Fix errors in URL creation when IdP context is bound to / - SIDP-417
authorlajoie <lajoie@ab3bd59b-922f-494d-bb5f-6f0a3c29deca>
Wed, 10 Nov 2010 23:04:03 +0000 (23:04 +0000)
committerlajoie <lajoie@ab3bd59b-922f-494d-bb5f-6f0a3c29deca>
Wed, 10 Nov 2010 23:04:03 +0000 (23:04 +0000)
git-svn-id: https://subversion.switch.ch/svn/shibboleth/java-idp/branches/REL_2@2966 ab3bd59b-922f-494d-bb5f-6f0a3c29deca

doc/RELEASE-NOTES.txt
src/main/java/edu/internet2/middleware/shibboleth/idp/authn/AuthenticationEngine.java
src/main/java/edu/internet2/middleware/shibboleth/idp/authn/provider/UsernamePasswordLoginHandler.java
src/main/java/edu/internet2/middleware/shibboleth/idp/authn/provider/UsernamePasswordLoginServlet.java
src/main/java/edu/internet2/middleware/shibboleth/idp/profile/saml1/ShibbolethSSOProfileHandler.java
src/main/java/edu/internet2/middleware/shibboleth/idp/profile/saml2/SSOProfileHandler.java
src/main/java/edu/internet2/middleware/shibboleth/idp/util/HttpServletHelper.java

index 7f1ccca..8522e4d 100644 (file)
@@ -1,5 +1,6 @@
 Changes in Release 2.2.1
 =============================================
+[SIDP-417] - Shib deployed to root web context, SSOProfileHandler forwards to "/null/AuthnEngine"
 [SIDP-421] - Error logging SOAP queries
 [SIDP-427] - Incorrect handling of returned authn error in SSO profile handlers
 [SIDP-428] - Address lifecycle issues around use of MetadataCredentialResolverFactory
index 3c55a0b..7f65fb8 100644 (file)
@@ -165,9 +165,8 @@ public class AuthenticationEngine extends HttpServlet {
             forwardRequest("/error.jsp", httpRequest, httpResponse);
         }
 
-        URLBuilder urlBuilder = HttpServletHelper.getServletContextUrl(httpRequest);
-        urlBuilder.setPath(urlBuilder.getPath() + loginContext.getProfileHandlerURL());
-        String profileUrl = urlBuilder.buildURL();
+        String profileUrl = HttpServletHelper.getContextRelativeUrl(httpRequest, loginContext.getProfileHandlerURL())
+                .buildURL();
         LOG.debug("Redirecting user to profile handler at {}", profileUrl);
         try {
             httpResponse.sendRedirect(profileUrl);
@@ -274,8 +273,8 @@ public class AuthenticationEngine extends HttpServlet {
      */
     protected Map<String, LoginHandler> determinePossibleLoginHandlers(Session idpSession, LoginContext loginContext)
             throws AuthenticationException {
-        Map<String, LoginHandler> supportedLoginHandlers = new HashMap<String, LoginHandler>(handlerManager
-                .getLoginHandlers());
+        Map<String, LoginHandler> supportedLoginHandlers = new HashMap<String, LoginHandler>(
+                handlerManager.getLoginHandlers());
         LOG.debug("Filtering configured LoginHandlers: {}", supportedLoginHandlers);
 
         // First, if the service provider requested a particular authentication method, filter out everything but
@@ -289,10 +288,9 @@ public class AuthenticationEngine extends HttpServlet {
                 supportedLoginHandlerEntry = supportedLoginHandlerItr.next();
                 if (!supportedLoginHandlerEntry.getKey().equals(AuthnContext.PREVIOUS_SESSION_AUTHN_CTX)
                         && !requestedMethods.contains(supportedLoginHandlerEntry.getKey())) {
-                    LOG
-                            .debug(
-                                    "Filtering out login handler for authentication {}, it does not provide a requested authentication method",
-                                    supportedLoginHandlerEntry.getKey());
+                    LOG.debug(
+                            "Filtering out login handler for authentication {}, it does not provide a requested authentication method",
+                            supportedLoginHandlerEntry.getKey());
                     supportedLoginHandlerItr.remove();
                 }
             }
@@ -341,8 +339,7 @@ public class AuthenticationEngine extends HttpServlet {
             }
         }
         if (currentAuthnMethods.isEmpty()) {
-            LOG
-                    .debug("Filtering out previous session login handler because there are no active authentication methods");
+            LOG.debug("Filtering out previous session login handler because there are no active authentication methods");
             supportedLoginHandlers.remove(AuthnContext.PREVIOUS_SESSION_AUTHN_CTX);
             return;
         }
@@ -359,8 +356,7 @@ public class AuthenticationEngine extends HttpServlet {
             }
 
             if (!retainPreviousSession) {
-                LOG
-                        .debug("Filtering out previous session login handler, no active authentication methods match required methods");
+                LOG.debug("Filtering out previous session login handler, no active authentication methods match required methods");
                 supportedLoginHandlers.remove(AuthnContext.PREVIOUS_SESSION_AUTHN_CTX);
                 return;
             }
@@ -474,8 +470,8 @@ public class AuthenticationEngine extends HttpServlet {
                 if (loginContext.getRequestedAuthenticationMethods().isEmpty()
                         || loginContext.getRequestedAuthenticationMethods().contains(
                                 authnMethod.getAuthenticationMethod())) {
-                    LOG.debug("Basing previous session authentication on active authentication method {}", authnMethod
-                            .getAuthenticationMethod());
+                    LOG.debug("Basing previous session authentication on active authentication method {}",
+                            authnMethod.getAuthenticationMethod());
                     loginContext.setAttemptedAuthnMethod(authnMethod.getAuthenticationMethod());
                     loginContext.setAuthenticationMethodInformation(authnMethod);
                     return loginHandler;
@@ -544,8 +540,8 @@ public class AuthenticationEngine extends HttpServlet {
 
             loginContext.setPrincipalAuthenticated(true);
             updateUserSession(loginContext, subject, actualAuthnMethod, httpRequest, httpResponse);
-            LOG.debug("User {} authenticated with method {}", loginContext.getPrincipalName(), loginContext
-                    .getAuthenticationMethod());
+            LOG.debug("User {} authenticated with method {}", loginContext.getPrincipalName(),
+                    loginContext.getAuthenticationMethod());
         } catch (AuthenticationException e) {
             LOG.error("Authentication failed with the error:", e);
             loginContext.setPrincipalAuthenticated(false);
@@ -579,8 +575,8 @@ public class AuthenticationEngine extends HttpServlet {
         String errorMessage = DatatypeHelper.safeTrimOrNullString((String) httpRequest
                 .getAttribute(LoginHandler.AUTHENTICATION_ERROR_KEY));
         if (errorMessage != null) {
-            LOG.error("Error returned from login handler for authentication method {}:\n{}", loginContext
-                    .getAttemptedAuthnMethod(), errorMessage);
+            LOG.error("Error returned from login handler for authentication method {}:\n{}",
+                    loginContext.getAttemptedAuthnMethod(), errorMessage);
             throw new AuthenticationException(errorMessage);
         }
 
@@ -687,7 +683,8 @@ public class AuthenticationEngine extends HttpServlet {
         idpSession.setSubject(mergeSubjects(idpSession.getSubject(), authenticationSubject));
 
         // Check if an existing authentication method was used (i.e. SSO occurred), if not record the new information
-        AuthenticationMethodInformation authnMethodInfo = idpSession.getAuthenticationMethods().get(authenticationMethod);
+        AuthenticationMethodInformation authnMethodInfo = idpSession.getAuthenticationMethods().get(
+                authenticationMethod);
         if (authnMethodInfo == null) {
             LOG.debug("Recording authentication and service information in Shibboleth session for principal: {}",
                     authenticationPrincipal.getName());
index 6f13563..1aca5e6 100644 (file)
@@ -21,7 +21,6 @@ import java.io.IOException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
-import org.opensaml.util.URLBuilder;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -32,46 +31,39 @@ import edu.internet2.middleware.shibboleth.idp.util.HttpServletHelper;
  * 
  * This login handler creates a {@link javax.security.auth.Subject} and binds it to the request as described in the
  * {@link edu.internet2.middleware.shibboleth.idp.authn.LoginHandler} documentation. If the JAAS module does not create
- * a principal for the user a {@link edu.internet2.middleware.shibboleth.idp.authn.UsernamePrincipal} is created, using the
- * entered username. If the <code>storeCredentialsInSubject</code> init parameter of the authentication servlet is set
- * to true a {@link UsernamePasswordCredential} is created, based on the entered username and password, and stored in
- * the Subject's private credentials.
+ * a principal for the user a {@link edu.internet2.middleware.shibboleth.idp.authn.UsernamePrincipal} is created, using
+ * the entered username. If the <code>storeCredentialsInSubject</code> init parameter of the authentication servlet is
+ * set to true a {@link UsernamePasswordCredential} is created, based on the entered username and password, and stored
+ * in the Subject's private credentials.
  */
 public class UsernamePasswordLoginHandler extends AbstractLoginHandler {
 
     /** Class logger. */
     private final Logger log = LoggerFactory.getLogger(UsernamePasswordLoginHandler.class);
 
-    /** The URL of the servlet used to perform authentication. */
-    private String authenticationServletURL;
+    /** The context-relative path of the servlet used to perform authentication. */
+    private String authenticationServletPath;
 
     /**
      * Constructor.
      * 
-     * @param servletURL URL to the authentication servlet
+     * @param servletPath context-relative path to the authentication servlet, may start with "/"
      */
-    public UsernamePasswordLoginHandler(String servletURL) {
+    public UsernamePasswordLoginHandler(String servletPath) {
         super();
         setSupportsPassive(false);
         setSupportsForceAuthentication(true);
-        authenticationServletURL = servletURL;
+        authenticationServletPath = servletPath;
     }
 
     /** {@inheritDoc} */
     public void login(final HttpServletRequest httpRequest, final HttpServletResponse httpResponse) {
         // forward control to the servlet.
         try {
-            URLBuilder urlBuilder = HttpServletHelper.getServletContextUrl(httpRequest);
-            
-            StringBuilder pathBuilder = new StringBuilder(urlBuilder.getPath());
-            if (!authenticationServletURL.startsWith("/")) {
-                pathBuilder.append("/");
-            }
-            pathBuilder.append(authenticationServletURL);
-            urlBuilder.setPath(pathBuilder.toString());
-
-            log.debug("Redirecting to {}", urlBuilder.buildURL());
-            httpResponse.sendRedirect(urlBuilder.buildURL());
+            String authnServletUrl = HttpServletHelper.getContextRelativeUrl(httpRequest, authenticationServletPath)
+                    .buildURL();
+            log.debug("Redirecting to {}", authnServletUrl);
+            httpResponse.sendRedirect(authnServletUrl);
             return;
         } catch (IOException ex) {
             log.error("Unable to redirect to authentication servlet.", ex);
index 3363c3e..57cc93f 100644 (file)
@@ -120,11 +120,13 @@ public class UsernamePasswordLoginServlet extends HttpServlet {
      */
     protected void redirectToLoginPage(HttpServletRequest request, HttpServletResponse response) {
 
-        String requestContext = DatatypeHelper.safeTrimOrNullString(request.getContextPath());
-        if (requestContext == null) {
-            requestContext = "/";
+        StringBuilder actionUrlBuilder = new StringBuilder();
+        if(!"".equals(request.getContextPath())){
+            actionUrlBuilder.append(request.getContextPath());
         }
-        request.setAttribute("actionUrl", requestContext + request.getServletPath());
+        actionUrlBuilder.append(request.getServletPath());
+        
+        request.setAttribute("actionUrl", actionUrlBuilder.toString());
 
         try {
             request.getRequestDispatcher(loginPage).forward(request, response);
index a0799e9..2cf60f9 100644 (file)
@@ -123,13 +123,13 @@ public class ShibbolethSSOProfileHandler extends AbstractSAML1ProfileHandler {
         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() || loginContext.getAuthenticationFailure() != null){
+        } else if (loginContext.isPrincipalAuthenticated() || loginContext.getAuthenticationFailure() != null) {
             log.debug("Incoming request contains a login context, processing as second leg of request");
             HttpServletHelper.unbindLoginContext(getStorageService(), servletContext, httpRequest, httpResponse);
             completeAuthenticationRequest(loginContext, inTransport, outTransport);
-        }else {
+        } else {
             log.debug("Incoming request contained a login context but principal was not authenticated, processing as first leg of request");
-            performAuthentication(inTransport, outTransport);            
+            performAuthentication(inTransport, outTransport);
         }
     }
 
@@ -167,9 +167,8 @@ public class ShibbolethSSOProfileHandler extends AbstractSAML1ProfileHandler {
                 .getServletContext(), httpRequest, httpResponse);
 
         try {
-            URLBuilder urlBuilder = HttpServletHelper.getServletContextUrl(httpRequest);
-            urlBuilder.setPath(urlBuilder.getPath() + authenticationManagerPath);
-            String authnEngineUrl = urlBuilder.buildURL();
+            String authnEngineUrl = HttpServletHelper.getContextRelativeUrl(httpRequest, authenticationManagerPath)
+                    .buildURL();
             log.debug("Redirecting user to authentication engine at {}", authnEngineUrl);
             httpResponse.sendRedirect(authnEngineUrl);
         } catch (IOException e) {
@@ -214,8 +213,8 @@ public class ShibbolethSSOProfileHandler extends AbstractSAML1ProfileHandler {
         requestContext.setMessageDecoder(decoder);
         try {
             decoder.decode(requestContext);
-            log.debug("Decoded Shibboleth SSO request from relying party '{}'", requestContext
-                    .getInboundMessageIssuer());
+            log.debug("Decoded Shibboleth SSO request from relying party '{}'",
+                    requestContext.getInboundMessageIssuer());
         } catch (MessageDecodingException e) {
             String msg = "Error decoding Shibboleth SSO request";
             log.warn(msg, e);
index b54b082..86a0095 100644 (file)
@@ -155,13 +155,13 @@ public class SSOProfileHandler extends AbstractSAML2ProfileHandler {
         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() || loginContext.getAuthenticationFailure() != null){
+        } else if (loginContext.isPrincipalAuthenticated() || loginContext.getAuthenticationFailure() != null) {
             log.debug("Incoming request contains a login context, processing as second leg of request");
             HttpServletHelper.unbindLoginContext(getStorageService(), servletContext, httpRequest, httpResponse);
             completeAuthenticationRequest(loginContext, inTransport, outTransport);
-        }else {
+        } else {
             log.debug("Incoming request contained a login context but principal was not authenticated, processing as first leg of request");
-            performAuthentication(inTransport, outTransport);            
+            performAuthentication(inTransport, outTransport);
         }
     }
 
@@ -205,9 +205,8 @@ public class SSOProfileHandler extends AbstractSAML2ProfileHandler {
             HttpServletHelper.bindLoginContext(loginContext, getStorageService(), httpRequest.getSession()
                     .getServletContext(), httpRequest, httpResponse);
 
-            URLBuilder urlBuilder = HttpServletHelper.getServletContextUrl(httpRequest);
-            urlBuilder.setPath(urlBuilder.getPath() + authenticationManagerPath);
-            String authnEngineUrl = urlBuilder.buildURL();
+            String authnEngineUrl = HttpServletHelper.getContextRelativeUrl(httpRequest, authenticationManagerPath)
+                    .buildURL();
             log.debug("Redirecting user to authentication engine at {}", authnEngineUrl);
             httpResponse.sendRedirect(authnEngineUrl);
         } catch (MarshallingException e) {
@@ -250,15 +249,13 @@ 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 {}",
-                                    requestedPrincipalName, loginContext.getPrincipalName());
+                    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));
                     throw new ProfileException("User failed authentication");
@@ -633,11 +630,10 @@ 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.");
             }
index 77aa449..78737d8 100644 (file)
@@ -622,4 +622,31 @@ public class HttpServletHelper {
         urlBuilder.setPath(httpRequest.getContextPath());
         return urlBuilder;
     }
+    
+    /**
+     * Builds a URL to a path that is meant to be relative to the Servlet context.
+     * 
+     * @param httpRequest current HTTP request
+     * @param path path relative to the context, may start with a "/"
+     * 
+     * @return URL builder containing the scheme, server name, server port, and full path
+     */
+    public static URLBuilder getContextRelativeUrl(HttpServletRequest httpRequest, String path){
+        URLBuilder urlBuilder = new URLBuilder();
+        urlBuilder.setScheme(httpRequest.getScheme());
+        urlBuilder.setHost(httpRequest.getServerName());
+        urlBuilder.setPort(httpRequest.getServerPort());
+        
+        StringBuilder pathBuilder = new StringBuilder();
+        if(!"".equals(httpRequest.getContextPath())){
+            pathBuilder.append(httpRequest.getContextPath());
+        }
+        if(!path.startsWith("/")){
+            pathBuilder.append("/");
+        }
+        pathBuilder.append(DatatypeHelper.safeTrim(path));
+        urlBuilder.setPath(pathBuilder.toString());
+        
+        return urlBuilder;
+    }
 }
\ No newline at end of file