[JIRA] Created: (SIDP-548) Incorrect handling of returned authn error in SSO profile handlers

[marian.cvecka@idp.protectnetwork.org (JIRA)]

Incorrect handling of returned authn error in SSO profile handlers
------------------------------------------------------------------

                 Key: SIDP-548
                 URL: https://issues.shibboleth.net/jira/browse/SIDP-548
             Project: Shibboleth IdP 2 - Java
          Issue Type: Bug
          Components: SAML 1, SAML 2
    Affects Versions: 2.2.0
            Reporter: marian.cvecka at idp.protectnetwork.org
            Assignee: Chad La Joie
             Fix For: 2.2.1


I think there's a bug in the conditionals that run in the processRequest method in the SAML 1 and 2 SSO profile handlers. They use the LoginContext.isPrincipalAuthenticated() method to determine whether to treat the request as the "first" or "second" leg, but this breaks if the LoginHandler returns to the profile handler with an error rather than authenticating the user.

The code in both the protocol versions looks like this:

        if (loginContext == null) {
            log.debug("Incoming request does not contain a login context, processing as first leg of request");
            performAuthentication(inTransport, outTransport);
        }else if(!loginContext.isPrincipalAuthenticated()){
            log.debug("Incoming request contained a login context but principal was not authenticated, processing as first leg of request");
            performAuthentication(inTransport, outTransport);            
        } else {
            log.debug("Incoming request contains a login context, processing as second leg of request");
            HttpServletHelper.unbindLoginContext(getStorageService(), servletContext, httpRequest, httpResponse);
            completeAuthenticationRequest(loginContext, inTransport, outTransport);
        }

When there's a failure in the LoginHandler, it returns control to the profile handler, but with a login context in place, and the middle branch runs instead of the last branch. That fails because the request URL no longer has the SAML request content. It's supposed to fall into the last branch where it would look for a login error inside completeAuthenticationRequest, but it never gets a chance.

I'm not 100% certain what the fix is yet. Maybe the presence of a login context is sufficient to send it to the second leg? Or it needs to check for the error case explicitly as well as using isPrincipalAuthenticated().

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira