https://bugs.internet2.edu/jira/browse/SIDP-258

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

diff --git a/doc/RELEASE-NOTES.txt b/doc/RELEASE-NOTES.txt
index 6a5fd05..e790d2c 100644 (file)
--- a/doc/RELEASE-NOTES.txt
+++ b/doc/RELEASE-NOTES.txt
@@ -2,6 +2,7 @@ Changes in Release 2.1.3
 =============================================
 [SIDP-244] - Error message on invalid ACS could be improved
 [SIDP-247] - Log Exception in UP LoginHandler Servlet
+[SIDP-258] - Authentication Engine does not check to ensure returned authenticaiton mechanism from Login Handler is acceptable to the SP
 [SIDP-263] - Suggest adding defaultSigningCredentialRef to the AnonymousRelyingParty element in the default config
 [SIDP-261] - IPAddressLoginHandler addresses comparison fails
 [SIDP-265] - Distinguish requested AuthMethod and default AuthMethod

diff --git a/src/main/java/edu/internet2/middleware/shibboleth/idp/authn/AuthenticationEngine.java b/src/main/java/edu/internet2/middleware/shibboleth/idp/authn/AuthenticationEngine.java
index 300ee22..25a952b 100644 (file)
--- a/src/main/java/edu/internet2/middleware/shibboleth/idp/authn/AuthenticationEngine.java
+++ b/src/main/java/edu/internet2/middleware/shibboleth/idp/authn/AuthenticationEngine.java
@@ -46,6 +46,7 @@ import org.opensaml.xml.util.Base64;
 import org.opensaml.xml.util.DatatypeHelper;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.slf4j.helpers.MessageFormatter;
 
 import edu.internet2.middleware.shibboleth.common.session.SessionManager;
 import edu.internet2.middleware.shibboleth.common.util.HttpHelper;
@@ -89,8 +90,8 @@ public class AuthenticationEngine extends HttpServlet {
 
     /** Class logger. */
     private static final Logger LOG = LoggerFactory.getLogger(AuthenticationEngine.class);
-    
-    //TODO remove once HttpServletHelper does redirects
+
+    // TODO remove once HttpServletHelper does redirects
     private static ServletContext context;
 
     /** Storage service used to store {@link LoginContext}s while authentication is in progress. */
@@ -128,8 +129,9 @@ public class AuthenticationEngine extends HttpServlet {
 
         handlerManager = HttpServletHelper.getProfileHandlerManager(config.getServletContext());
         sessionManager = HttpServletHelper.getSessionManager(config.getServletContext());
-        storageService = (StorageService<String, LoginContextEntry>) HttpServletHelper.getStorageService(config.getServletContext());
-        
+        storageService = (StorageService<String, LoginContextEntry>) HttpServletHelper.getStorageService(config
+                .getServletContext());
+
         context = config.getServletContext();
     }
 
@@ -157,15 +159,15 @@ public class AuthenticationEngine extends HttpServlet {
      * @param httpRequest current HTTP request
      * @param httpResponse current HTTP response
      */
-    public static void returnToProfileHandler(HttpServletRequest httpRequest,
-            HttpServletResponse httpResponse) {
+    public static void returnToProfileHandler(HttpServletRequest httpRequest, HttpServletResponse httpResponse) {
         LOG.debug("Returning control to login handler");
-        LoginContext loginContext = HttpServletHelper.unbindLoginContext(storageService, context, httpRequest, httpResponse);
+        LoginContext loginContext = HttpServletHelper.unbindLoginContext(storageService, context, httpRequest,
+                httpResponse);
         if (loginContext == null) {
             LOG.warn("No login context available, unable to return to profile handler");
             forwardRequest("/idp-error.jsp", httpRequest, httpResponse);
         }
-        
+
         HttpServletHelper.bindLoginContext(loginContext, httpRequest);
         LOG.debug("Returning control to profile handler at: {}", loginContext.getProfileHandlerURL());
         forwardRequest(loginContext.getProfileHandlerURL(), httpRequest, httpResponse);
@@ -273,9 +275,10 @@ public class AuthenticationEngine extends HttpServlet {
             LOG.debug("Authenticating user with login handler of type {}", loginHandler.getClass().getName());
             loginContext.setAuthenticationAttempted();
             loginContext.setAuthenticationEngineURL(HttpHelper.getRequestUriWithoutContext(httpRequest));
-            
+
             // Send the request to the login handler
-            HttpServletHelper.bindLoginContext(loginContext, storageService, getServletContext(), httpRequest, httpResponse);
+            HttpServletHelper.bindLoginContext(loginContext, storageService, getServletContext(), httpRequest,
+                    httpResponse);
             loginHandler.login(httpRequest, httpResponse);
         } catch (AuthenticationException e) {
             loginContext.setAuthenticationFailure(e);
@@ -449,11 +452,17 @@ public class AuthenticationEngine extends HttpServlet {
             // event that it supports multiple methods
             String actualAuthnMethod = DatatypeHelper.safeTrimOrNullString((String) httpRequest
                     .getAttribute(LoginHandler.AUTHENTICATION_METHOD_KEY));
-            if (actualAuthnMethod == null) {
-                actualAuthnMethod = loginContext.getAttemptedAuthnMethod();
+            if (actualAuthnMethod != null) {
+                if (!loginContext.getRequestedAuthenticationMethods().isEmpty()
+                        && !loginContext.getRequestedAuthenticationMethods().contains(actualAuthnMethod)) {
+                    String msg = MessageFormatter.format(
+                                    "Relying patry required an authentication method of '{}' but the login handler performed '{}'",
+                                    loginContext.getRequestedAuthenticationMethods(), actualAuthnMethod);
+                    LOG.error(msg);
+                    throw new AuthenticationException(msg);
+                }
             } else {
-                LOG.debug("Authentication method overriden by LoginHandler.  It was {} and is now {}", loginContext
-                        .getAttemptedAuthnMethod(), actualAuthnMethod);
+                actualAuthnMethod = loginContext.getAttemptedAuthnMethod();
             }
 
             // Check to make sure the login handler did the right thing
@@ -502,9 +511,10 @@ public class AuthenticationEngine extends HttpServlet {
                     .getAttemptedAuthnMethod(), errorMessage);
             throw new AuthenticationException(errorMessage);
         }
-        
-        AuthenticationException authnException = (AuthenticationException) httpRequest.getAttribute(LoginHandler.AUTHENTICATION_EXCEPTION_KEY);
-        if(authnException != null){
+
+        AuthenticationException authnException = (AuthenticationException) httpRequest
+                .getAttribute(LoginHandler.AUTHENTICATION_EXCEPTION_KEY);
+        if (authnException != null) {
             throw authnException;
         }