Implement no attribute push more effeciently

git-svn-id: https://subversion.switch.ch/svn/shibboleth/java-idp/trunk@2312 ab3bd59b-922f-494d-bb5f-6f0a3c29deca

diff --git a/src/edu/internet2/middleware/shibboleth/idp/profile/saml1/AbstractSAML1ProfileHandler.java b/src/edu/internet2/middleware/shibboleth/idp/profile/saml1/AbstractSAML1ProfileHandler.java
index 2baea5f..9266f31 100644 (file)
--- a/src/edu/internet2/middleware/shibboleth/idp/profile/saml1/AbstractSAML1ProfileHandler.java
+++ b/src/edu/internet2/middleware/shibboleth/idp/profile/saml1/AbstractSAML1ProfileHandler.java
@@ -348,7 +348,7 @@ public abstract class AbstractSAML1ProfileHandler extends AbstractSAMLProfileHan
                 }
             }
             requestContext.setFailureStatus(buildStatus(StatusCode.RESPONDER, null,
-            "Unable to construct NameIdentifier"));
+                    "Unable to construct NameIdentifier"));
             throw new ProfileException("No principal attribute supported encoding into the a supported name ID format.");
         } catch (AttributeEncodingException e) {
             log.error("Unable to construct NameIdentifier", e);
@@ -483,6 +483,35 @@ 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(SAML1ProfileRequestContext requestContext) throws ProfileException {
+        AbstractSAML1ProfileConfiguration profileConfiguration = requestContext.getProfileConfiguration();
+        SAML1AttributeAuthority attributeAuthority = profileConfiguration.getAttributeAuthority();
+
+        try {
+            if (log.isDebugEnabled()) {
+                log.debug("Resolving attributes for principal " + requestContext.getPrincipalName()
+                        + " of SAML request from relying party " + requestContext.getRelyingPartyId());
+            }
+            Map<String, BaseAttribute> principalAttributes = attributeAuthority
+                    .getAttributes(buildAttributeRequestContext(requestContext));
+
+            requestContext.setPrincipalAttributes(principalAttributes);
+        } catch (AttributeRequestException e) {
+            log.error("Error resolving attributes for SAML request from relying party "
+                    + requestContext.getRelyingPartyId(), e);
+            requestContext.setFailureStatus(buildStatus(StatusCode.RESPONDER, null, "Error resolving attributes"));
+            throw new ProfileException("Error resolving attributes for SAML request from relying party "
+                    + requestContext.getRelyingPartyId(), e);
+        }
+    }
+
+    /**
      * Executes a query for attributes and builds a SAML attribute statement from the results.
      * 
      * @param requestContext current request context
@@ -504,21 +533,13 @@ public abstract class AbstractSAML1ProfileHandler extends AbstractSAMLProfileHan
         SAML1AttributeAuthority attributeAuthority = profileConfiguration.getAttributeAuthority();
 
         try {
-            if (log.isDebugEnabled()) {
-                log.debug("Resolving attributes for principal " + requestContext.getPrincipalName()
-                        + " of SAML request from relying party " + requestContext.getRelyingPartyId());
-            }
-            Map<String, BaseAttribute> principalAttributes = attributeAuthority
-                    .getAttributes(buildAttributeRequestContext(requestContext));
-
-            requestContext.setPrincipalAttributes(principalAttributes);
-
             AttributeStatement statment;
             if (requestContext.getSamlRequest() instanceof AttributeQuery) {
                 statment = attributeAuthority.buildAttributeStatement((AttributeQuery) requestContext.getSamlRequest(),
-                        principalAttributes.values());
+                        requestContext.getPrincipalAttributes().values());
             } else {
-                statment = attributeAuthority.buildAttributeStatement(null, principalAttributes.values());
+                statment = attributeAuthority.buildAttributeStatement(null, requestContext.getPrincipalAttributes()
+                        .values());
             }
 
             Subject statementSubject = buildSubject(requestContext, subjectConfMethod);
@@ -526,11 +547,10 @@ public abstract class AbstractSAML1ProfileHandler extends AbstractSAMLProfileHan
 
             return statment;
         } catch (AttributeRequestException e) {
-            log.error("Error resolving attributes for SAML request from relying party "
-                    + requestContext.getRelyingPartyId(), e);
+            log.error("Error encoding attributes for principal " + requestContext.getPrincipalName(), e);
             requestContext.setFailureStatus(buildStatus(StatusCode.RESPONDER, null, "Error resolving attributes"));
-            throw new ProfileException("Error resolving attributes for SAML request from relying party "
-                    + requestContext.getRelyingPartyId(), e);
+            throw new ProfileException("Error encoding attributes for principal " + requestContext.getPrincipalName(),
+                    e);
         }
     }
 
@@ -686,13 +706,13 @@ public abstract class AbstractSAML1ProfileHandler extends AbstractSAMLProfileHan
         auditLogEntry.setPrincipalName(context.getPrincipalName());
         auditLogEntry.setAssertingPartyId(context.getAssertingPartyId());
         auditLogEntry.setRelyingPartyId(context.getRelyingPartyId());
-        if(context.getMessageDecoder() != null){
+        if (context.getMessageDecoder() != null) {
             auditLogEntry.setRequestBinding(context.getMessageDecoder().getBindingURI());
         }
         auditLogEntry.setRequestId(null);
         auditLogEntry.setResponseBinding(context.getMessageEncoder().getBindingURI());
         auditLogEntry.setResponseId(context.getSamlResponse().getID());
-        if(context.getPrincipalAttributes() != null){
+        if (context.getPrincipalAttributes() != null) {
             auditLogEntry.getReleasedAttributes().addAll(context.getPrincipalAttributes().keySet());
         }
         getAduitLog().log(Level.CRITICAL, auditLogEntry);

diff --git a/src/edu/internet2/middleware/shibboleth/idp/profile/saml1/AttributeQueryProfileHandler.java b/src/edu/internet2/middleware/shibboleth/idp/profile/saml1/AttributeQueryProfileHandler.java
index 14a6e78..c42fd79 100644 (file)
--- a/src/edu/internet2/middleware/shibboleth/idp/profile/saml1/AttributeQueryProfileHandler.java
+++ b/src/edu/internet2/middleware/shibboleth/idp/profile/saml1/AttributeQueryProfileHandler.java
@@ -77,6 +77,7 @@ public class AttributeQueryProfileHandler extends AbstractSAML1ProfileHandler {
             }
 
             resolvePrincipal(requestContext);
+            resolveAttributes(requestContext);
 
             ArrayList<Statement> statements = new ArrayList<Statement>();
             statements.add(buildAttributeStatement(requestContext, "urn:oasis:names:tc:SAML:1.0:cm:sender-vouches"));

diff --git a/src/edu/internet2/middleware/shibboleth/idp/profile/saml1/ShibbolethSSOProfileHandler.java b/src/edu/internet2/middleware/shibboleth/idp/profile/saml1/ShibbolethSSOProfileHandler.java
index f58129e..425b212 100644 (file)
--- a/src/edu/internet2/middleware/shibboleth/idp/profile/saml1/ShibbolethSSOProfileHandler.java
+++ b/src/edu/internet2/middleware/shibboleth/idp/profile/saml1/ShibbolethSSOProfileHandler.java
@@ -198,16 +198,13 @@ public class ShibbolethSSOProfileHandler extends AbstractSAML1ProfileHandler {
                 requestContext.setFailureStatus(buildStatus(StatusCode.RESPONDER, null, "User failed authentication"));
                 throw new ProfileException("User failed authentication");
             }
-
-            //TODO currently attribute query must come first in order to get the principal's attributes, fix this
-            AttributeStatement attributeStatement = buildAttributeStatement(requestContext, "urn:oasis:names:tc:SAML:1.0:cm:bearer");
-            AuthenticationStatement authnStatement = buildAuthenticationStatement(requestContext);
+            
+            resolveAttributes(requestContext);
             
             ArrayList<Statement> statements = new ArrayList<Statement>();
-            //TODO make this more effecient
-            statements.add(authnStatement);
+            statements.add(buildAuthenticationStatement(requestContext));
             if (requestContext.getProfileConfiguration().includeAttributeStatement()) {
-                statements.add(attributeStatement);
+                statements.add(buildAttributeStatement(requestContext, "urn:oasis:names:tc:SAML:1.0:cm:bearer"));
             }
 
             samlResponse = buildResponse(requestContext, statements);

diff --git a/src/edu/internet2/middleware/shibboleth/idp/profile/saml2/AbstractSAML2ProfileHandler.java b/src/edu/internet2/middleware/shibboleth/idp/profile/saml2/AbstractSAML2ProfileHandler.java
index 885b1c9..7f4b7bf 100644 (file)
--- a/src/edu/internet2/middleware/shibboleth/idp/profile/saml2/AbstractSAML2ProfileHandler.java
+++ b/src/edu/internet2/middleware/shibboleth/idp/profile/saml2/AbstractSAML2ProfileHandler.java
@@ -319,6 +319,37 @@ public abstract class AbstractSAML2ProfileHandler extends AbstractSAMLProfileHan
         response.setVersion(SAMLVersion.VERSION_20);
         response.setIssuer(buildEntityIssuer(requestContext));
     }
+    
+    /**
+     * Resolves the attributes for the principal.
+     * 
+     * @param requestContext current request context
+     * 
+     * @throws ProfileException thrown if there is a problem resolved attributes
+     */
+    protected void resolveAttributes(SAML2ProfileRequestContext requestContext) throws ProfileException{
+        AbstractSAML2ProfileConfiguration profileConfiguration = requestContext.getProfileConfiguration();
+        SAML2AttributeAuthority attributeAuthority = profileConfiguration.getAttributeAuthority();
+
+        try {
+            if (log.isDebugEnabled()) {
+                log.debug("Resolving attributes for principal " + requestContext.getPrincipalName()
+                        + " of SAML request " + requestContext.getSamlRequest().getID() + " from relying party "
+                        + requestContext.getRelyingPartyId());
+            }
+            Map<String, BaseAttribute> principalAttributes = attributeAuthority
+                    .getAttributes(buildAttributeRequestContext(requestContext));
+
+            requestContext.setPrincipalAttributes(principalAttributes);
+        } catch (AttributeRequestException e) {
+            log.error("Error resolving attributes for SAML request " + requestContext.getSamlRequest().getID()
+                    + " from relying party " + requestContext.getRelyingPartyId(), e);
+            requestContext.setFailureStatus(buildStatus(StatusCode.RESPONDER_URI, null, "Error resolving attributes"));
+            throw new ProfileException("Error resolving attributes for SAML request "
+                    + requestContext.getSamlRequest().getID() + " from relying party "
+                    + requestContext.getRelyingPartyId(), e);
+        }
+    }
 
     /**
      * Executes a query for attributes and builds a SAML attribute statement from the results.
@@ -331,40 +362,24 @@ public abstract class AbstractSAML2ProfileHandler extends AbstractSAMLProfileHan
      */
     protected AttributeStatement buildAttributeStatement(SAML2ProfileRequestContext requestContext)
             throws ProfileException {
-
         if (log.isDebugEnabled()) {
             log.debug("Creating attribute statement in response to SAML request "
                     + requestContext.getSamlRequest().getID() + " from relying party "
                     + requestContext.getRelyingPartyId());
         }
-
         AbstractSAML2ProfileConfiguration profileConfiguration = requestContext.getProfileConfiguration();
         SAML2AttributeAuthority attributeAuthority = profileConfiguration.getAttributeAuthority();
-
         try {
-            if (log.isDebugEnabled()) {
-                log.debug("Resolving attributes for principal " + requestContext.getPrincipalName()
-                        + " of SAML request " + requestContext.getSamlRequest().getID() + " from relying party "
-                        + requestContext.getRelyingPartyId());
-            }
-            Map<String, BaseAttribute> principalAttributes = attributeAuthority
-                    .getAttributes(buildAttributeRequestContext(requestContext));
-
-            requestContext.setPrincipalAttributes(principalAttributes);
-
             if (requestContext.getSamlRequest() instanceof AttributeQuery) {
                 return attributeAuthority.buildAttributeStatement((AttributeQuery) requestContext.getSamlRequest(),
-                        principalAttributes.values());
+                        requestContext.getPrincipalAttributes().values());
             } else {
-                return attributeAuthority.buildAttributeStatement(null, principalAttributes.values());
+                return attributeAuthority.buildAttributeStatement(null, requestContext.getPrincipalAttributes().values());
             }
         } catch (AttributeRequestException e) {
-            log.error("Error resolving attributes for SAML request " + requestContext.getSamlRequest().getID()
-                    + " from relying party " + requestContext.getRelyingPartyId(), e);
+            log.error("Error encoding attributes for principal " + requestContext.getPrincipalName(), e);
             requestContext.setFailureStatus(buildStatus(StatusCode.RESPONDER_URI, null, "Error resolving attributes"));
-            throw new ProfileException("Error resolving attributes for SAML request "
-                    + requestContext.getSamlRequest().getID() + " from relying party "
-                    + requestContext.getRelyingPartyId(), e);
+            throw new ProfileException("Error encoding attributes for principal " + requestContext.getPrincipalName(), e);
         }
     }
 

diff --git a/src/edu/internet2/middleware/shibboleth/idp/profile/saml2/AttributeQueryProfileHandler.java b/src/edu/internet2/middleware/shibboleth/idp/profile/saml2/AttributeQueryProfileHandler.java
index 39de7f1..38318ad 100644 (file)
--- a/src/edu/internet2/middleware/shibboleth/idp/profile/saml2/AttributeQueryProfileHandler.java
+++ b/src/edu/internet2/middleware/shibboleth/idp/profile/saml2/AttributeQueryProfileHandler.java
@@ -78,6 +78,8 @@ public class AttributeQueryProfileHandler extends AbstractSAML2ProfileHandler {
 
             // Resolve attribute query name id to principal name and place in context
             resolvePrincipal(requestContext);
+            
+            resolveAttributes(requestContext);
 
             // Lookup principal name and attributes, create attribute statement from information
             ArrayList<Statement> statements = new ArrayList<Statement>();

diff --git a/src/edu/internet2/middleware/shibboleth/idp/profile/saml2/SSOProfileHandler.java b/src/edu/internet2/middleware/shibboleth/idp/profile/saml2/SSOProfileHandler.java
index 6a47578..477ae47 100644 (file)
--- a/src/edu/internet2/middleware/shibboleth/idp/profile/saml2/SSOProfileHandler.java
+++ b/src/edu/internet2/middleware/shibboleth/idp/profile/saml2/SSOProfileHandler.java
@@ -34,9 +34,7 @@ import org.opensaml.common.binding.decoding.MessageDecoder;
 import org.opensaml.common.binding.encoding.MessageEncoder;
 import org.opensaml.common.binding.security.SAMLSecurityPolicy;
 import org.opensaml.common.xml.SAMLConstants;
-import org.opensaml.saml2.core.SubjectLocality;
 import org.opensaml.saml2.binding.AuthnResponseEndpointSelector;
-import org.opensaml.saml2.core.AttributeStatement;
 import org.opensaml.saml2.core.AuthnContext;
 import org.opensaml.saml2.core.AuthnContextClassRef;
 import org.opensaml.saml2.core.AuthnContextDeclRef;
@@ -46,6 +44,7 @@ import org.opensaml.saml2.core.RequestedAuthnContext;
 import org.opensaml.saml2.core.Response;
 import org.opensaml.saml2.core.Statement;
 import org.opensaml.saml2.core.StatusCode;
+import org.opensaml.saml2.core.SubjectLocality;
 import org.opensaml.saml2.metadata.AssertionConsumerService;
 import org.opensaml.saml2.metadata.Endpoint;
 import org.opensaml.saml2.metadata.provider.MetadataProviderException;
@@ -221,19 +220,18 @@ public class SSOProfileHandler extends AbstractSAML2ProfileHandler {
         Response samlResponse;
         try {
             if (loginContext.getPrincipalName() == null) {
+                log.error("User's login context did not contain a principal, user considered unauthenticiated.");
                 requestContext
                         .setFailureStatus(buildStatus(StatusCode.RESPONDER_URI, StatusCode.AUTHN_FAILED_URI, null));
                 throw new ProfileException("User failed authentication");
             }
-
-            AuthnStatement authnStatement = buildAuthnStatement(requestContext);
-            AttributeStatement attributeStatement = buildAttributeStatement(requestContext);
+            
+            resolveAttributes(requestContext);
             
             ArrayList<Statement> statements = new ArrayList<Statement>();
-            statements.add(authnStatement);
-            //TODO this isn't very effecient, support this flag better
+            statements.add(buildAuthnStatement(requestContext));
             if(requestContext.getProfileConfiguration().includeAttributeStatement()){
-                statements.add(attributeStatement);
+                statements.add(buildAttributeStatement(requestContext));
             }
 
             samlResponse = buildResponse(requestContext, "urn:oasis:names:tc:SAML:2.0:cm:bearer", statements);