First round of bug fixes on Shib SSO, requests properly flow through profile handler...

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

diff --git a/src/edu/internet2/middleware/shibboleth/idp/authn/LoginContext.java b/src/edu/internet2/middleware/shibboleth/idp/authn/LoginContext.java
index b451bef..dc34585 100644 (file)
--- a/src/edu/internet2/middleware/shibboleth/idp/authn/LoginContext.java
+++ b/src/edu/internet2/middleware/shibboleth/idp/authn/LoginContext.java
@@ -93,9 +93,13 @@ public class LoginContext implements Serializable {
 
     /** The session id. */
     private String sessionID;
+    
+    /** List of request authentication methods. */
+    private ArrayList<String> requestAuthenticationMethods;
 
     /** Creates a new instance of LoginContext. */
     public LoginContext() {
+        requestAuthenticationMethods = new ArrayList<String>();
     }
 
     /**
@@ -105,9 +109,9 @@ public class LoginContext implements Serializable {
      * @param passive if the authentication manager must not interact with the users UI.
      */
     public LoginContext(boolean force, boolean passive) {
-
         forceAuth = force;
         passiveAuth = passive;
+        requestAuthenticationMethods = new ArrayList<String>();
     }
 
     /**
@@ -374,6 +378,6 @@ public class LoginContext implements Serializable {
      * @return an list of authentication method identifiers
      */
     public List<String> getRequestedAuthenticationMethods() {
-        return new ArrayList<String>();
+        return requestAuthenticationMethods;
     }
 }
\ No newline at end of file

diff --git a/src/edu/internet2/middleware/shibboleth/idp/config/profile/AbstractAuthenticationHandlerBeanDefinitionParser.java b/src/edu/internet2/middleware/shibboleth/idp/config/profile/AbstractAuthenticationHandlerBeanDefinitionParser.java
index 2b5815e..dbf0e6e 100644 (file)
--- a/src/edu/internet2/middleware/shibboleth/idp/config/profile/AbstractAuthenticationHandlerBeanDefinitionParser.java
+++ b/src/edu/internet2/middleware/shibboleth/idp/config/profile/AbstractAuthenticationHandlerBeanDefinitionParser.java
@@ -19,6 +19,7 @@ package edu.internet2.middleware.shibboleth.idp.config.profile;
 import java.util.ArrayList;
 import java.util.List;
 
+import org.apache.log4j.Logger;
 import org.opensaml.xml.util.DatatypeHelper;
 import org.opensaml.xml.util.XMLHelper;
 import org.springframework.beans.factory.support.BeanDefinitionBuilder;
@@ -30,17 +31,30 @@ import org.w3c.dom.Element;
  */
 public abstract class AbstractAuthenticationHandlerBeanDefinitionParser extends AbstractSingleBeanDefinitionParser {
 
+    /** Class logger. */
+    private static Logger log = Logger.getLogger(AbstractAuthenticationHandlerBeanDefinitionParser.class);
+
     /** {@inheritDoc} */
     protected void doParse(Element config, BeanDefinitionBuilder builder) {
-        builder.addPropertyValue("authenticationDuration", Integer.parseInt(config.getAttributeNS(null,
-                "authenticationDuration")));
 
+        int duration = Integer.parseInt(config.getAttributeNS(null, "authenticationDuration"));
+        if (log.isDebugEnabled()) {
+            log.debug("Authentication handler declared duration of " + duration + " minutes");
+        }
+        builder.addPropertyValue("authenticationDuration", duration);
+
+        String authnMethod;
         ArrayList<String> authnMethods = new ArrayList<String>();
         List<Element> authnMethodElems = XMLHelper.getChildElementsByTagNameNS(config,
                 ProfileHandlerNamespaceHandler.NAMESPACE, "AuthenticationMethod");
         for (Element authnMethodElem : authnMethodElems) {
-            authnMethods.add(DatatypeHelper.safeTrimOrNullString(authnMethodElem.getTextContent()));
+            authnMethod = DatatypeHelper.safeTrimOrNullString(authnMethodElem.getTextContent());
+            if (log.isDebugEnabled()) {
+                log.debug("Authentication handler declared support for authentication method " + authnMethod);
+            }
+            authnMethods.add(authnMethod);
         }
+        builder.addPropertyValue("authenticationMethods", authnMethods);
     }
 
     /** {@inheritDoc} */

diff --git a/src/edu/internet2/middleware/shibboleth/idp/config/profile/ProfileHandlerGroupBeanDefinitionParser.java b/src/edu/internet2/middleware/shibboleth/idp/config/profile/ProfileHandlerGroupBeanDefinitionParser.java
index bb683da..09d7334 100644 (file)
--- a/src/edu/internet2/middleware/shibboleth/idp/config/profile/ProfileHandlerGroupBeanDefinitionParser.java
+++ b/src/edu/internet2/middleware/shibboleth/idp/config/profile/ProfileHandlerGroupBeanDefinitionParser.java
@@ -21,6 +21,7 @@ import java.util.Map;
 
 import javax.xml.namespace.QName;
 
+import org.apache.log4j.Logger;
 import org.opensaml.xml.util.XMLHelper;
 import org.springframework.beans.factory.support.AbstractBeanDefinition;
 import org.springframework.beans.factory.support.BeanDefinitionBuilder;
@@ -34,6 +35,9 @@ import edu.internet2.middleware.shibboleth.common.config.SpringConfigurationUtil
  * Spring bean definition parser for profile handler root element.
  */
 public class ProfileHandlerGroupBeanDefinitionParser extends AbstractBeanDefinitionParser {
+    
+    /** Class logger. */
+    private static Logger log = Logger.getLogger(ProfileHandlerGroupBeanDefinitionParser.class);
 
     /** Schema type name. */
     public static final QName SCHEMA_TYPE = new QName(ProfileHandlerNamespaceHandler.NAMESPACE, "ProfileHandlerGroup");
@@ -46,12 +50,21 @@ public class ProfileHandlerGroupBeanDefinitionParser extends AbstractBeanDefinit
         List<Element> children;
 
         children = configChildren.get(new QName(ProfileHandlerNamespaceHandler.NAMESPACE, "ErrorHandler"));
+        if(log.isDebugEnabled()){
+            log.debug(children.size() + " error handler definitions found");
+        }
         builder.addPropertyValue("errorHandler", SpringConfigurationUtils.parseCustomElement(children.get(0), context));
 
         children = configChildren.get(new QName(ProfileHandlerNamespaceHandler.NAMESPACE, "ProfileHandler"));
+        if(log.isDebugEnabled()){
+            log.debug(children.size() + " profile handler definitions found");
+        }
         builder.addPropertyValue("profileHandlers", SpringConfigurationUtils.parseCustomElements(children, context));
 
         children = configChildren.get(new QName(ProfileHandlerNamespaceHandler.NAMESPACE, "AuthenticationHandler"));
+        if(log.isDebugEnabled()){
+            log.debug(children.size() + " authentication handler definitions found");
+        }
         builder.addPropertyValue("authenticationHandlers", SpringConfigurationUtils.parseCustomElements(children,
                 context));
 

diff --git a/src/edu/internet2/middleware/shibboleth/idp/profile/IdPProfileHandlerManager.java b/src/edu/internet2/middleware/shibboleth/idp/profile/IdPProfileHandlerManager.java
index 99b236a..554d209 100644 (file)
--- a/src/edu/internet2/middleware/shibboleth/idp/profile/IdPProfileHandlerManager.java
+++ b/src/edu/internet2/middleware/shibboleth/idp/profile/IdPProfileHandlerManager.java
@@ -27,7 +27,6 @@ import javax.servlet.http.HttpServletRequest;
 
 import org.apache.log4j.Logger;
 import org.opensaml.util.resource.Resource;
-import org.opensaml.util.resource.ResourceException;
 import org.opensaml.xml.util.Pair;
 import org.springframework.context.ApplicationContext;
 
@@ -105,7 +104,7 @@ public class IdPProfileHandlerManager extends BaseReloadableService implements P
 
         String requestPath = ((HttpServletRequest) request).getPathInfo();
         if (log.isDebugEnabled()) {
-            log.debug("Looking up profile handler for request path: " + requestPath);
+            log.debug(getServiceName() + ": Looking up profile handler for request path: " + requestPath);
         }
         Lock readLock = getReadWriteLock().readLock();
         readLock.lock();
@@ -114,12 +113,12 @@ public class IdPProfileHandlerManager extends BaseReloadableService implements P
 
         if (handler != null) {
             if (log.isDebugEnabled()) {
-                log.debug("Located profile handler of the following type for request path " + requestPath + ": "
-                        + handler.getClass().getName());
+                log.debug(getServiceName() + ": Located profile handler of the following type for request path "
+                        + requestPath + ": " + handler.getClass().getName());
             }
         } else {
             if (log.isDebugEnabled()) {
-                log.debug("No profile handler registered for request path " + requestPath);
+                log.debug(getServiceName() + ": No profile handler registered for request path " + requestPath);
             }
         }
         return handler;
@@ -143,17 +142,45 @@ public class IdPProfileHandlerManager extends BaseReloadableService implements P
      * @return authentication method URI and handler appropriate for given login context
      */
     public Pair<String, AuthenticationHandler> getAuthenticationHandler(LoginContext loginContext) {
+        if (log.isDebugEnabled()) {
+            log.debug(getServiceName() + ": Looking up authentication method for relying party "
+                    + loginContext.getRelyingPartyId());
+        }
         List<String> requestedMethods = loginContext.getRequestedAuthenticationMethods();
         if (requestedMethods != null) {
             AuthenticationHandler candidateHandler;
             for (String requestedMethod : requestedMethods) {
+                if (log.isDebugEnabled()) {
+                    log.debug(getServiceName() + ": Checking for authentication handler for method " + requestedMethod
+                            + " which was requested for relying party " + loginContext.getRelyingPartyId());
+                }
                 candidateHandler = authenticationHandlers.get(requestedMethod);
                 if (candidateHandler != null) {
-                    if (loginContext.getPassiveAuth() && candidateHandler.supportsPassive()) {
-                        return new Pair<String, AuthenticationHandler>(requestedMethod, candidateHandler);
+                    if (log.isDebugEnabled()) {
+                        log.debug(getServiceName() + ": Authentication handler for method " + requestedMethod
+                                + " for relying party " + loginContext.getRelyingPartyId()
+                                + " found.  Checking if it meets othe criteria.");
+                    }
+                    if(loginContext.getPassiveAuth() && !candidateHandler.supportsPassive()){
+                        if (log.isDebugEnabled()) {
+                            log.debug(getServiceName() + ": Authentication handler for method " + requestedMethod
+                                    + " for relying party " + loginContext.getRelyingPartyId()
+                                    + " does not meet required support for passive auth.  Skipping it");
+                        }
+                        continue;
+                    }
+                    
+                    if (log.isDebugEnabled()) {
+                        log.debug(getServiceName() + ": Authentication handler for method " + requestedMethod
+                                + " for relying party " + loginContext.getRelyingPartyId()
+                                + " meets all requirements, using it.");
                     }
+                    return new Pair<String, AuthenticationHandler>(requestedMethod, candidateHandler);
                 }
             }
+        } else {
+            log.error(getServiceName() + ": No requested authentication methods for relying party "
+                    + loginContext.getRelyingPartyId());
         }
 
         return null;
@@ -169,54 +196,84 @@ public class IdPProfileHandlerManager extends BaseReloadableService implements P
     }
 
     /** {@inheritDoc} */
-    protected void newContextCreated(ApplicationContext newServiceContext) throws ResourceException {
+    protected void newContextCreated(ApplicationContext newServiceContext) {
         if (log.isDebugEnabled()) {
-            log.debug("Loading new configuration into service");
+            log.debug(getServiceName() + ": Loading new configuration into service");
         }
-        String[] errorBeanNames = newServiceContext.getBeanNamesForType(AbstractErrorHandler.class);
-        String[] profileBeanNames = newServiceContext.getBeanNamesForType(AbstractRequestURIMappedProfileHandler.class);
-        String[] authnBeanNames = newServiceContext.getBeanNamesForType(AuthenticationHandler.class);
-
         Lock writeLock = getReadWriteLock().writeLock();
         writeLock.lock();
+        loadNewErrorHandler(newServiceContext);
+        loadNewProfileHandlers(newServiceContext);
+        loadNewAuthenticationHandlers(newServiceContext);
+        writeLock.unlock();
+    }
+
+    /**
+     * Reads the new error handler from the newly created application context and loads it into this manager.
+     * 
+     * @param newServiceContext newly created application context
+     */
+    protected void loadNewErrorHandler(ApplicationContext newServiceContext) {
+        String[] errorBeanNames = newServiceContext.getBeanNamesForType(AbstractErrorHandler.class);
+        if (log.isDebugEnabled()) {
+            log.debug(getServiceName() + ": Loading " + errorBeanNames.length + " new error handler.");
+        }
 
         errorHandler = (AbstractErrorHandler) newServiceContext.getBean(errorBeanNames[0]);
         if (log.isDebugEnabled()) {
-            log.debug("Loaded new error handler of type: " + errorHandler.getClass().getName());
+            log.debug(getServiceName() + ": Loaded new error handler of type: " + errorHandler.getClass().getName());
         }
+    }
 
-        profileHandlers.clear();
+    /**
+     * Reads the new profile handlers from the newly created application context and loads it into this manager.
+     * 
+     * @param newServiceContext newly created application context
+     */
+    protected void loadNewProfileHandlers(ApplicationContext newServiceContext) {
+        String[] profileBeanNames = newServiceContext.getBeanNamesForType(AbstractRequestURIMappedProfileHandler.class);
         if (log.isDebugEnabled()) {
-            log.debug(profileBeanNames.length + " profile handlers loaded");
+            log.debug(getServiceName() + ": Loading " + profileBeanNames.length + " new profile handlers.");
         }
+
+        profileHandlers.clear();
         AbstractRequestURIMappedProfileHandler profileHandler;
         for (String profileBeanName : profileBeanNames) {
             profileHandler = (AbstractRequestURIMappedProfileHandler) newServiceContext.getBean(profileBeanName);
             for (String requestPath : profileHandler.getRequestPaths()) {
                 profileHandlers.put(requestPath, profileHandler);
                 if (log.isDebugEnabled()) {
-                    log.debug("Request path " + requestPath + " mapped to profile handler of type: "
-                            + profileHandler.getClass().getName());
+                    log.debug(getServiceName() + ": Loaded profile handler of type "
+                                    + profileHandler.getClass().getName() + " handling requests to request path "
+                                    + requestPath);
                 }
             }
         }
+    }
 
-        authenticationHandlers.clear();
+    /**
+     * Reads the new authentication handlers from the newly created application context and loads it into this manager.
+     * 
+     * @param newServiceContext newly created application context
+     */
+    protected void loadNewAuthenticationHandlers(ApplicationContext newServiceContext) {
+        String[] authnBeanNames = newServiceContext.getBeanNamesForType(AuthenticationHandler.class);
         if (log.isDebugEnabled()) {
-            log.debug(authnBeanNames.length + " authentication handlers loaded");
+            log.debug(getServiceName() + ": Loading " + authnBeanNames.length + " new authentication handlers.");
         }
+
+        authenticationHandlers.clear();
         AuthenticationHandler authnHandler;
         for (String authnBeanName : authnBeanNames) {
             authnHandler = (AuthenticationHandler) newServiceContext.getBean(authnBeanName);
+            if (log.isDebugEnabled()) {
+                log.debug(getServiceName() + ": Loading authentication handler of type "
+                        + authnHandler.getClass().getName() + " supporting authentication methods: "
+                        + authnHandler.getSupportedAuthenticationMethods());
+            }
             for (String authnMethod : authnHandler.getSupportedAuthenticationMethods()) {
                 authenticationHandlers.put(authnMethod, authnHandler);
-                if (log.isDebugEnabled()) {
-                    log.debug("Authentication method " + authnMethod + " mapped to authentication handler of type: "
-                            + authnHandler.getClass().getName());
-                }
             }
         }
-
-        writeLock.unlock();
     }
 }
\ No newline at end of file

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 57e0e36..87e1e74 100644 (file)
--- a/src/edu/internet2/middleware/shibboleth/idp/profile/saml1/ShibbolethSSOProfileHandler.java
+++ b/src/edu/internet2/middleware/shibboleth/idp/profile/saml1/ShibbolethSSOProfileHandler.java
@@ -67,7 +67,6 @@ public class ShibbolethSSOProfileHandler extends AbstractSAML1ProfileHandler {
      * Constructor.
      * 
      * @param authnManagerPath path to the authentication manager servlet
-     * @param encoder URI of the encoding binding
      * 
      * @throws IllegalArgumentException thrown if either the authentication manager path or encoding binding URI are
      *             null or empty