Identity.login adds a hardcoded message

[fernando_jmt]

Hi.

I just try the Seam security, I did an authenticator something like this:

public boolean authenticate(String username, String password, Set<String> roles) {
 try {

 User currentUser = (User) entityManager.createQuery("select u from User u where u.username=:username and u.password=:password")
 .setParameter("username", username)
 .setParameter("password", password)
 .getSingleResult();

 if (currentUser.getRoles() != null) {
 for (Role role : currentUser.getRoles())
 roles.add(role.getName());
 }
 facesMessages.addFromResourceBundle("User.loggedIn", new Object[]{currentUser.getFullName()});
 return true;
 }
 catch (NoResultException ex) {
 facesMessages.addFromResourceBundle("Login.error");
 return false;
 }
 }

I also created the respective page for this authentication. But when the authentication fails, I am getting two messages:
a) "Login.error" value (messages.properties)
b) Login failed (hardcoded).


Then I saw that in the Identity.java you have:

public String login()
 {
 try
 {
 authenticate();
 log.debug("Login successful for: " + getUsername());
 return "success";
 }
 catch (LoginException ex)
 {
 log.debug("Login failed for:" + getUsername(), ex);
 FacesMessages.instance().add("Login failed");
 return null;
 }
 }

I think the line for "FacesMessages.instance().add("Login failed")" is not useful in the most cases, especially for i18n applications.


I suggest to:
a) remove this hardcode message
b) add the posibility to configure the respective i18n message.

I think the option a) is better. What do you think?

Thanks in advance.

Best regards.

[hstang]

You could provide your own implementation of login() instead of using's Identity. That way you can customize the messages however you want. e.g.

Identity.instance().authenticate()

I'm with you in thinking exposing login() in the design of the API with hard coded inputs as you mentioned was a bad idea, especially when used in examples where people think it is some sort of pattern.

[hstang]

....got cut off earlier...just want to finish by saying it's probably an easy fix to removing the hard code and provide option for customizing a message.

[gavin.king]

Yes, is should be configurable like all the other build-in facesmessages in Seam. I'll fix that now.

[gavin.king]

Done in CVS

[fernando_jmt]

It sounds great. Thanks.

[chris1]

mmhh, maybe this is a tricky one: The severity of the login failed msg is SEVERITY_INFO but I need a SEVERITY_ERROR. How can I achieve this?

-Chris

[hstang]

Probably try not depend on Identity.login(). Roll your own.

[tarantula]

Hey guys,

When you say configurable does this mean the Identity.instance().login() message can be disabled?

If so, how?

I would like to stick to the Identity API for security but not if it adds a Faces message when one is not needed and leaves me no option to suppress it.

Since the message is global it shows up when h:messages (globalOnly=true) is on the page.

+1 for more control of Seam messages.

Ian

[shane.bryzak]

I've just committed a change to cvs that will allow you to override the message key used for the login success/failure messages. I.e. if you don't want a message, set loginSuccessfulKey to an empty string, e.g.

<security:identity authenticate-method="#{authenticator.authenticate}"
 security-rules="#{securityRules}"
 loginSuccessfulKey=""/>

[gavin.king]

-1. messages.properties is the place to config messages.

[shane.bryzak]

I've rolled back the changes previously mentioned... Gavin has been so kind as to modify FacesMessages to not add blank messages so my changes are redundant.