Send back a valid authn statement upon successful authn even if attribute resolution...
authorlajoie <lajoie@ab3bd59b-922f-494d-bb5f-6f0a3c29deca>
Thu, 24 Sep 2009 11:58:35 +0000 (11:58 +0000)
committerlajoie <lajoie@ab3bd59b-922f-494d-bb5f-6f0a3c29deca>
Thu, 24 Sep 2009 11:58:35 +0000 (11:58 +0000)
git-svn-id: https://subversion.switch.ch/svn/shibboleth/java-idp/branches/REL_2@2891 ab3bd59b-922f-494d-bb5f-6f0a3c29deca

doc/RELEASE-NOTES.txt
src/main/java/edu/internet2/middleware/shibboleth/idp/profile/saml1/AbstractSAML1ProfileHandler.java
src/main/java/edu/internet2/middleware/shibboleth/idp/profile/saml1/ShibbolethSSOProfileHandler.java
src/main/java/edu/internet2/middleware/shibboleth/idp/profile/saml2/AbstractSAML2ProfileHandler.java
src/main/java/edu/internet2/middleware/shibboleth/idp/profile/saml2/SSOProfileHandler.java

index 50c7749..b6661c1 100644 (file)
@@ -3,6 +3,7 @@ Changes in Release 2.1.4
 [SIDP-340] - Default tc-config.xml causes TCNonPortableObjectError
 [SIDP-348] - Remove Terracotta Configuration from IdP Install
 [SIDP-249] - LoginContext is not removed from StorageService after Authentication Completes
+[SIDP-351] - Attribute resolution errors shouldn't prevent valid authn statement being returned
 
 Changes in Release 2.1.3
 =============================================
index d2920cc..8c23354 100644 (file)
@@ -360,6 +360,10 @@ public abstract class AbstractSAML1ProfileHandler extends AbstractSAMLProfileHan
      */
     protected NameIdentifier buildNameId(BaseSAML1ProfileRequestContext<?, ?, ?> requestContext)
             throws ProfileException {
+        if(requestContext.getAttributes() == null){
+            return null;
+        }
+        
         log.debug("Attemping to build NameIdentifier for principal '{}' in response to request from relying party '{}",
                 requestContext.getPrincipalName(), requestContext.getInboundMessageIssuer());
 
@@ -493,8 +497,6 @@ public abstract class AbstractSAML1ProfileHandler extends AbstractSAMLProfileHan
      * Resolved the attributes for the principal.
      * 
      * @param requestContext current request context
-     * 
-     * @throws ProfileException thrown if attributes can not be resolved
      */
     protected void resolveAttributes(BaseSAML1ProfileRequestContext<?, ?, ?> requestContext) throws ProfileException {
         AbstractSAML1ProfileConfiguration profileConfiguration = requestContext.getProfileConfiguration();
@@ -507,12 +509,8 @@ public abstract class AbstractSAML1ProfileHandler extends AbstractSAMLProfileHan
 
             requestContext.setAttributes(principalAttributes);
         } catch (AttributeRequestException e) {
-            requestContext.setFailureStatus(buildStatus(StatusCode.RESPONDER, null, "Error resolving attributes"));
-            String msg = MessageFormatter.format(
-                    "Error resolving attributes for principal '{}' for SAML request from relying party '{}'",
-                    requestContext.getPrincipalName(), requestContext.getInboundMessageIssuer());
-            log.error(msg, e);
-            throw new ProfileException(msg, e);
+            log.warn("Error resolving attributes for principal '{}'.  No name identifier or attribute statement will be included in response",
+                    requestContext.getPrincipalName());
         }
     }
 
@@ -529,6 +527,10 @@ public abstract class AbstractSAML1ProfileHandler extends AbstractSAMLProfileHan
     protected AttributeStatement buildAttributeStatement(BaseSAML1ProfileRequestContext<?, ?, ?> requestContext,
             String subjectConfMethod) throws ProfileException {
 
+        if(requestContext.getAttributes() == null){
+            return null;
+        }
+        
         log.debug(
                 "Creating attribute statement about principal '{}'in response to SAML request from relying party '{}'",
                 requestContext.getPrincipalName(), requestContext.getInboundMessageIssuer());
index e76e8db..fdb3288 100644 (file)
@@ -85,9 +85,6 @@ public class ShibbolethSSOProfileHandler extends AbstractSAML1ProfileHandler {
      * Constructor.
      * 
      * @param authnManagerPath path to the authentication manager servlet
-     * 
-     * @throws IllegalArgumentException thrown if either the authentication manager path or encoding binding URI are
-     *             null or empty
      */
     public ShibbolethSSOProfileHandler(String authnManagerPath) {
         if (DatatypeHelper.isEmpty(authnManagerPath)) {
@@ -253,16 +250,16 @@ public class ShibbolethSSOProfileHandler extends AbstractSAML1ProfileHandler {
             }
 
             resolveAttributes(requestContext);
-
+            
             ArrayList<Statement> statements = new ArrayList<Statement>();
             statements.add(buildAuthenticationStatement(requestContext));
             if (requestContext.getProfileConfiguration().includeAttributeStatement()) {
-                AttributeStatement attributeStatement = buildAttributeStatement(requestContext,
-                        "urn:oasis:names:tc:SAML:1.0:cm:bearer");
-                if (attributeStatement != null) {
-                    requestContext.setReleasedAttributes(requestContext.getAttributes().keySet());
-                    statements.add(attributeStatement);
-                }
+                    AttributeStatement attributeStatement = buildAttributeStatement(requestContext,
+                            "urn:oasis:names:tc:SAML:1.0:cm:bearer");
+                    if (attributeStatement != null) {
+                        requestContext.setReleasedAttributes(requestContext.getAttributes().keySet());
+                        statements.add(attributeStatement);
+                    }
             }
 
             samlResponse = buildResponse(requestContext, statements);
index 6bfb0bd..70ff9ab 100644 (file)
@@ -462,12 +462,8 @@ public abstract class AbstractSAML2ProfileHandler extends AbstractSAMLProfileHan
 
             requestContext.setAttributes(principalAttributes);
         } catch (AttributeRequestException e) {
-            requestContext.setFailureStatus(buildStatus(StatusCode.RESPONDER_URI, null, "Error resolving attributes"));
-            String msg = MessageFormatter.format(
-                    "Error resolving attributes for principal '{}' for SAML request from relying party '{}'",
-                    requestContext.getPrincipalName(), requestContext.getInboundMessageIssuer());
-            log.error(msg, e);
-            throw new ProfileException(msg, e);
+            log.warn("Error resolving attributes for principal '{}'.  No name identifier or attribute statement will be included in response",
+                    requestContext.getPrincipalName());
         }
     }
 
@@ -482,6 +478,10 @@ public abstract class AbstractSAML2ProfileHandler extends AbstractSAMLProfileHan
      */
     protected AttributeStatement buildAttributeStatement(BaseSAML2ProfileRequestContext<?, ?, ?> requestContext)
             throws ProfileException {
+        if(requestContext.getAttributes() == null){
+            return null;
+        }
+        
         log.debug("Creating attribute statement in response to SAML request '{}' from relying party '{}'",
                 requestContext.getInboundSAMLMessageId(), requestContext.getInboundMessageIssuer());
 
@@ -826,9 +826,13 @@ public abstract class AbstractSAML2ProfileHandler extends AbstractSAMLProfileHan
      *             name ID attribute or because there are no supported name formats
      */
     protected NameID buildNameId(BaseSAML2ProfileRequestContext<?, ?, ?> requestContext) throws ProfileException {
+        if(requestContext.getAttributes() == null){
+            return null;
+        }
+        
         log.debug("Building assertion NameID for principal/relying party:{}/{}", requestContext.getPrincipalName(),
                 requestContext.getInboundMessageIssuer());
-
+        
         // Check if AuthnRequest includes an explicit NameIDPolicy Format.
         String requiredNameFormat = null;
         if (requestContext.getInboundSAMLMessage() instanceof AuthnRequest) {
index 63dd3ed..4aa3ae1 100644 (file)
@@ -169,7 +169,8 @@ public class SSOProfileHandler extends AbstractSAML2ProfileHandler {
             RelyingPartyConfiguration rpConfig = getRelyingPartyConfiguration(relyingPartyId);
             ProfileConfiguration ssoConfig = rpConfig.getProfileConfiguration(getProfileId());
             if (ssoConfig == null) {
-                String msg = MessageFormatter.format("SAML 2 SSO profile is not configured for relying party '{}'", requestContext.getInboundMessageIssuer());
+                String msg = MessageFormatter.format("SAML 2 SSO profile is not configured for relying party '{}'",
+                        requestContext.getInboundMessageIssuer());
                 log.warn(msg);
                 throw new ProfileException(msg);
             }
@@ -180,7 +181,7 @@ public class SSOProfileHandler extends AbstractSAML2ProfileHandler {
             loginContext.setAuthenticationEngineURL(authenticationManagerPath);
             loginContext.setProfileHandlerURL(HttpHelper.getRequestUriWithoutContext(servletRequest));
             loginContext.setDefaultAuthenticationMethod(rpConfig.getDefaultAuthenticationMethod());
-            
+
             HttpServletHelper.bindLoginContext(loginContext, servletRequest);
             RequestDispatcher dispatcher = servletRequest.getRequestDispatcher(authenticationManagerPath);
             dispatcher.forward(servletRequest, ((HttpServletResponseAdapter) outTransport).getWrappedResponse());
@@ -209,7 +210,7 @@ public class SSOProfileHandler extends AbstractSAML2ProfileHandler {
             throws ProfileException {
         HttpServletRequest httpRequest = ((HttpServletRequestAdapter) inTransport).getWrappedRequest();
         Saml2LoginContext loginContext = (Saml2LoginContext) HttpServletHelper.getLoginContext(httpRequest);
-        
+
         SSORequestContext requestContext = buildRequestContext(loginContext, inTransport, outTransport);
 
         checkSamlVersion(requestContext);
@@ -228,14 +229,11 @@ public class SSOProfileHandler extends AbstractSAML2ProfileHandler {
             }
 
             if (requestContext.getSubjectNameIdentifier() != null) {
-                log
-                        .debug("Authentication request contained a subject with a name identifier, resolving principal from NameID");
+                log.debug("Authentication request contained a subject with a name identifier, resolving principal from NameID");
                 resolvePrincipal(requestContext);
                 String requestedPrincipalName = requestContext.getPrincipalName();
                 if (!DatatypeHelper.safeEquals(loginContext.getPrincipalName(), requestedPrincipalName)) {
-                    log
-                            .warn(
-                                    "Authentication request identified principal {} but authentication mechanism identified principal {}",
+                    log.warn("Authentication request identified principal {} but authentication mechanism identified principal {}",
                                     requestedPrincipalName, loginContext.getPrincipalName());
                     requestContext.setFailureStatus(buildStatus(StatusCode.RESPONDER_URI, StatusCode.AUTHN_FAILED_URI,
                             null));
@@ -244,7 +242,7 @@ public class SSOProfileHandler extends AbstractSAML2ProfileHandler {
             }
 
             resolveAttributes(requestContext);
-
+            
             ArrayList<Statement> statements = new ArrayList<Statement>();
             statements.add(buildAuthnStatement(requestContext));
             if (requestContext.getProfileConfiguration().includeAttributeStatement()) {
@@ -279,8 +277,8 @@ public class SSOProfileHandler extends AbstractSAML2ProfileHandler {
     protected void decodeRequest(SSORequestContext requestContext, HTTPInTransport inTransport,
             HTTPOutTransport outTransport) throws ProfileException {
         if (log.isDebugEnabled()) {
-            log.debug("Decoding message with decoder binding '{}'",
-                    getInboundMessageDecoder(requestContext).getBindingURI());
+            log.debug("Decoding message with decoder binding '{}'", getInboundMessageDecoder(requestContext)
+                    .getBindingURI());
         }
 
         requestContext.setCommunicationProfileId(getProfileId());
@@ -303,18 +301,18 @@ public class SSOProfileHandler extends AbstractSAML2ProfileHandler {
             log.debug("Decoded request from relying party '{}'", requestContext.getInboundMessageIssuer());
 
             if (!(requestContext.getInboundSAMLMessage() instanceof AuthnRequest)) {
-                log.warn("Incomming message was not a AuthnRequest, it was a '{}'",
-                        requestContext.getInboundSAMLMessage().getClass().getName());
+                log.warn("Incomming message was not a AuthnRequest, it was a '{}'", requestContext
+                        .getInboundSAMLMessage().getClass().getName());
                 requestContext.setFailureStatus(buildStatus(StatusCode.REQUESTER_URI, null,
                         "Invalid SAML AuthnRequest message."));
                 throw new ProfileException("Invalid SAML AuthnRequest message.");
             }
         } catch (MessageDecodingException e) {
-            String msg = "Error decoding authentication request message"; 
+            String msg = "Error decoding authentication request message";
             log.warn(msg, e);
             throw new ProfileException(msg, e);
         } catch (SecurityException e) {
-            String msg = "Message did not meet security requirements"; 
+            String msg = "Message did not meet security requirements";
             log.warn(msg, e);
             throw new ProfileException(msg, e);
         }
@@ -482,8 +480,8 @@ public class SSOProfileHandler extends AbstractSAML2ProfileHandler {
                 }
             }
         }
-        
-        if(authnContext.getAuthnContextClassRef() == null || authnContext.getAuthnContextDeclRef() == null){
+
+        if (authnContext.getAuthnContextClassRef() == null || authnContext.getAuthnContextDeclRef() == null) {
             AuthnContextClassRef ref = authnContextClassRefBuilder.buildObject();
             ref.setAuthnContextClassRef(loginContext.getAuthenticationMethod());
             authnContext.setAuthnContextClassRef(ref);
@@ -527,8 +525,11 @@ public class SSOProfileHandler extends AbstractSAML2ProfileHandler {
                 } else {
                     endpoint.setBinding(getSupportedOutboundBindings().get(0));
                 }
-                log.warn("Generating endpoint for anonymous relying party self-identified as '{}', ACS url '{}' and binding '{}'",
-                        new Object[] {requestContext.getInboundMessageIssuer(), endpoint.getLocation(), endpoint.getBinding(), });
+                log
+                        .warn(
+                                "Generating endpoint for anonymous relying party self-identified as '{}', ACS url '{}' and binding '{}'",
+                                new Object[] { requestContext.getInboundMessageIssuer(), endpoint.getLocation(),
+                                        endpoint.getBinding(), });
             } else {
                 log.warn("Unable to generate endpoint for anonymous party.  No ACS url provided.");
             }