8 Replies Latest reply on Jan 13, 2014 2:36 AM by bodzolca

    Seam Login - flawed logic or do I need to extend Credentials?

    kragoth

      Ok, so I have a page that has the following fields. Username, Password, and a checkbox for accepting the terms and conditions. The problem I have is this. (And maybe a side effect of not using a password in dev mode but I want to try to understand what I'm supposed to do to fix this :S)


      If a user types in their username, leaves password blank (dev user has no password) and does not check the check box then in authenticate method (I have set this up in components.xml) then authentication fails... as per the code below.


          public boolean authenticate() {
              log.info("Logging in as user: " + credentials.getUsername()
                  + ". Privacy aggreement accepted? " + agreedToPrivacy);
      
              if (!agreedToPrivacy) {
                  FacesMessagesUtils
                      .addErrorFromBundle("security.privacy.not.checked");
                  return false;
              }
      
              ....other checks here to the database or LDAP maybe.
      



      So I return false from the authenticate method and the user is left on the login screen with an error. So far all good.


      So, now the user checks the checkbox and clicks login. I expected that this should work and successfully log the user in. Instead I just get a LoginException and the user can never get off the login screen unless they change their username (to something completely wrong) try to login (which fails). Then fix their username back up and login again.


      So, I've looked into the Seam source to try work this out and I think I sorta know what's going on.


      In the Identity.login() method it calls the authenticate method and if that fails it throws a LoginException. This exception is caught and then this line of code executes.


      credentials.invalidate();
      



      However, there is no way to make the credentials valid again unless you change the username or password as the only place where invalid is set to false is inside the setUsername or setPassword methods in the Credentials class. The problem is that invalid is set to false if and only if the set username changes.


      So, my question is this. Is it really logical to assume that the only reason a person's credentials failed to authenticate is because they typed their username and/or password wrong. I could think of a whole bunch of reasons why authentication can fail outside of the username/password being incorrect. Authentication server timeout, time based restrictions on login (valid login hours), multiple logins detected (systems that only allow you to be connected on 1 session), a brand new account that hasn't been activated yet. I'm sure there's more but, these are a few I can think of in 30 secs.


      Anyways, I'm more then willing to extend Credentials if i have to (Not sure IF I can do it but, I can go read about it). But, just wondering what other people's thoughts are on this.

        • 1. Re: Seam Login - flawed logic or do I need to extend Credentials?
          kragoth

          Well, a quick look into extending Credentials brings me to another issue. There is no nice way for me to set invalid to false. I can't do what is done in the setPassword and setUsername methods because the invalid variable is private.  So, I have to do some hack to make my credentials valid again when someone checks the checkbox. Reflection to set the invalid flag or set the username to some junk name and back again.


          Is there any reason for this restriction?

          • 2. Re: Seam Login - flawed logic or do I need to extend Credentials?
            kragoth

            Has anyone got any ideas about this?

            • 3. Re: Seam Login - flawed logic or do I need to extend Credentials?
              nickarls

              Could this be related to the tip-box in the manual under 15.3.2?

              • 4. Re: Seam Login - flawed logic or do I need to extend Credentials?
                kragoth

                After reading your post I thought hmm, am I doing the very bad thing of not reading the doco first. But, I've read doco and even more importantly I've gone through the Seam code. Sure the event gets raised but that's not the problem.


                As described above, the problem is that Seam does not provide a way of telling the Credentials component that it is valid again after a failed login. The only way Seam sets the invalid flag to false is if you change the username and/or the password. (And I've been saying that username/password combo is not the only way a login can fail, so why is this they only thing that is allowed to change the invalid flag to false)


                I would have thought that raising the event should flag the Credentials as no longer being invalid. But, this is not how it works.

                • 5. Re: Seam Login - flawed logic or do I need to extend Credentials?
                  nickarls

                  Tried nuking org.jboss.seam.security.credentials from the contexts?

                  • 6. Re: Seam Login - flawed logic or do I need to extend Credentials?
                    kragoth

                    I don't want to nuke it, I want all the details to stay as they are, but allow me to update the credentials to valid status when the user checks the checkbox. (Which I have done using a work around for now).


                    My problem is that the Seam thinks that the only way a Credential can be made valid is if you change the username and/or password. I mean sure I could just grab the info in the credentials, nuke the current one, recreate and copy the data across... but why do all that work. I can effectivly work around the problem by just setting the username to blank when they check the box and then resetting it to what it was. I just think this is a really stupid way to have to work around the problem.

                    • 7. Re: Seam Login - flawed logic or do I need to extend Credentials?
                      rhoppe

                      Hi,


                      for me the solution was to reset password to an empty string in case of failed login:


                      @Observer(Identity.EVENT_LOGIN_FAILED)
                      public void loginFailed(javax.security.auth.login.LoginException loginEx) {
                           identity.getCredentials().setPassword("");
                      }
                      



                      This forces to invalidate the credentials.

                      • 8. Re: Seam Login - flawed logic or do I need to extend Credentials?
                        bodzolca
                        So, my question is this. Is it really logical to assume that the only reason a person's credentials failed to authenticate is because they typed their username and/or password wrong. I could think of a whole bunch of reasons why authentication can fail outside of the username/password being incorrect. Authentication server timeout, time based restrictions on login (valid login hours), multiple logins detected (systems that only allow you to be connected on 1 session), a brand new account that hasn't been activated yet.

                         

                        Right you are! I'm adding some custom single-sign-on scenarios to your list. Really strange to introduce a state variable that is so poorly thought out.

                         

                        Anyway, thx for your analysis and solution.