Clean up external authentication system and have it use request attributes
authorlajoie <lajoie@ab3bd59b-922f-494d-bb5f-6f0a3c29deca>
Sun, 1 May 2011 19:47:46 +0000 (19:47 +0000)
committerlajoie <lajoie@ab3bd59b-922f-494d-bb5f-6f0a3c29deca>
Sun, 1 May 2011 19:47:46 +0000 (19:47 +0000)
Remove external authentication servlet as this isn't really used
Fix NPE in UsernamePrincipal

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

src/main/java/edu/internet2/middleware/shibboleth/idp/authn/UsernamePrincipal.java
src/main/java/edu/internet2/middleware/shibboleth/idp/authn/provider/ExternalAuthnSystemLoginHandler.java
src/main/java/edu/internet2/middleware/shibboleth/idp/authn/provider/ExternalAuthnSystemServlet.java [deleted file]
src/main/java/edu/internet2/middleware/shibboleth/idp/config/profile/authn/ExternalAuthnSystemLoginHandlerBeanDefinitionParser.java
src/main/java/edu/internet2/middleware/shibboleth/idp/config/profile/authn/ExternalAuthnSystemLoginHandlerFactoryBean.java
src/main/resources/schema/shibboleth-2.0-idp-profile-handler.xsd
src/main/webapp/WEB-INF/web.xml

index a686c0b..f10b528 100644 (file)
@@ -37,6 +37,9 @@ public class UsernamePrincipal implements Principal, Serializable {
      */
     public UsernamePrincipal(String principalName) {
         name = DatatypeHelper.safeTrimOrNullString(principalName);
+        if(name == null){
+            throw new IllegalArgumentException("principal name may not be null or empty");
+        }
     }
 
     /** {@inheritDoc} */
index decc572..7d6135b 100644 (file)
 package edu.internet2.middleware.shibboleth.idp.authn.provider;
 
 import java.io.IOException;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Map.Entry;
 
+import javax.servlet.RequestDispatcher;
+import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
-import org.opensaml.util.URLBuilder;
 import org.opensaml.xml.util.DatatypeHelper;
-import org.opensaml.xml.util.Pair;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -39,192 +33,103 @@ import edu.internet2.middleware.shibboleth.idp.util.HttpServletHelper;
 /**
  * A login handler meant to bridge between the IdP and an external, web-based, authentication service.
  * 
- * This login handler will redirect the user-agent to a context-relative path and include the following query
- * parameters: {@link #FORCE_AUTHN_PARAM}, {@link #PASSIVE_AUTHN_PARAM}, {@link #AUTHN_METHOD_PARAM}, and
+ * This login handler will forward the user-agent to a context-relative path and include the following request
+ * attributes: {@link #FORCE_AUTHN_PARAM}, {@link #PASSIVE_AUTHN_PARAM}, {@link #AUTHN_METHOD_PARAM}, and
  * {@link #RELYING_PARTY_PARAM}.
  * 
- * The external authentication service must be configured to protect the page to which the user-agent is authenticated.
- * This external service must populate the REMOTE_USER header with the principal name of the authenticated user. The
- * external authentication service may also indicate which authentication method it actually performed by populating the
- * HTTP header name {@value #AUTHN_METHOD_PARAM}.
+ * The external authentication system invocation Fileter/Servlet/JSP must, upon completion of authentication, set the
+ * appropriate {@link HttpServletRequest} attributes, as described by the
+ * {@link edu.internet2.middleware.shibboleth.idp.authn.LoginHandler} interface and then invoke
+ * {@link edu.internet2.middleware.shibboleth.idp.authn.AuthenticationEngine#returnToAuthenticationEngine(HttpServletRequest, HttpServletResponse)}
+ * .
  */
 public class ExternalAuthnSystemLoginHandler extends AbstractLoginHandler {
 
-    /**
-     * Query parameter that indicates whether the authentication request requires forced authentication. Parameter Name:
-     * * {@value}
-     */
+    /** Query parameter, {@value} , that indicates whether the authentication request requires forced authentication. */
     public static final String FORCE_AUTHN_PARAM = "forceAuthn";
 
-    /**
-     * Query parameter that indicates whether the authentication requires passive authentication. Parameter Name: * * *
-     * * {@value}
-     */
+    /** Query parameter, {@value} , that indicates whether the authentication requires passive authentication. */
     public static final String PASSIVE_AUTHN_PARAM = "isPassive";
 
-    /** Query parameter that provides which authentication method should be attempted. Parameter Name: {@value} */
+    /** Query parameter, {@value} , that provides which authentication method should be attempted. */
     public static final String AUTHN_METHOD_PARAM = "authnMethod";
 
-    /**
-     * Query parameter that provides the entity ID of the relying party that is requesting authentication. Parameter
-     * Name: {@value}
-     */
+    /** Query parameter, {@value} , that provides the entity ID of the relying party that is requesting authentication. */
     public static final String RELYING_PARTY_PARAM = "relyingParty";
 
     /** Class logger. */
     private final Logger log = LoggerFactory.getLogger(RemoteUserLoginHandler.class);
 
-    /** The context-relative path to the SSO-protected Servlet. Default value: {@value} */
-    private String protectedPath = "/Authn/External";
-
-    /** Static query parameters sent to the SSO-protected Servlet. */
-    private Map<String, String> queryParameters;
+    /** The context-relative path to the Filter, Servlet, or JSP that triggers the external authentication system. */
+    private String externalAuthnPath;
 
     /** Constructor. */
     public ExternalAuthnSystemLoginHandler() {
         super();
-        queryParameters = new HashMap<String, String>();
     }
 
     /**
-     * Get context-relative path to the SSO-protected Servlet.
+     * Get context-relative path to the Filter, Servlet, or JSP that triggers the external authentication system.
      * 
-     * @return context-relative path to the SSO-protected Servlet
+     * @return context-relative path to the Filter, Servlet, or JSP that triggers the external authentication system
      */
-    public String getProtectedPath() {
-        return protectedPath;
+    public String getExternalAuthnPath() {
+        return externalAuthnPath;
     }
 
     /**
-     * Set context-relative path to the SSO-protected Servlet. The given path may not contain fragments and/or query
-     * params.
+     * Set context-relative path to the Filter, Servlet, or JSP that triggers the external authentication system.
      * 
-     * @param path context-relative path to the SSO-protected Servlet, may not be null or empty
+     * @param path context-relative path to the Filter, Servlet, or JSP that triggers the external authentication
+     *            system, may not be null or empty
      */
-    public void setProtectedPath(String path) {
+    public void setExternalAuthnPath(String path) {
         String trimmedPath = DatatypeHelper.safeTrimOrNullString(path);
         if (trimmedPath == null) {
-            throw new IllegalArgumentException("Protected path may not be null or empty");
-        }
-
-        if (trimmedPath.contains("?")) {
-            throw new IllegalArgumentException("Protected path may not include query parameters");
-        }
-
-        if (trimmedPath.contains("#")) {
-            throw new IllegalArgumentException("Protected path may not include document fragements");
+            throw new IllegalArgumentException("External Authn path may not be null or empty");
         }
 
-        protectedPath = trimmedPath;
-    }
-
-    /**
-     * Gets the immutable set of query parameters sent to the SSO-protected Servlet.
-     * 
-     * @return immutable set of query parameters sent to the SSO-protected Servlet, never null
-     */
-    public Map<String, String> getQueryParameters() {
-        return queryParameters;
-    }
-
-    /**
-     * Sets the query parameters that will be sent to the SSO-protected Servlet.
-     * 
-     * @param params query parameters that will be sent to the SSO-protected Servlet, maybe null
-     */
-    public void setQueryParameters(Map<String, String> params) {
-        if(params == null || params.isEmpty()){
-            queryParameters = Collections.emptyMap();
-        }
-        
-        HashMap<String, String> newParams = new HashMap<String, String>();
-
-        String trimmedKeyName;
-        for (Entry<String, String> param : params.entrySet()) {
-            trimmedKeyName = DatatypeHelper.safeTrimOrNullString(param.getKey());
-            if (trimmedKeyName != null) {
-                newParams.put(trimmedKeyName, DatatypeHelper.safeTrimOrNullString(param.getValue()));
-            }
-        }
-
-        queryParameters = Collections.unmodifiableMap(newParams);
+        externalAuthnPath = trimmedPath;
     }
 
     /** {@inheritDoc} */
     public void login(HttpServletRequest httpRequest, HttpServletResponse httpResponse) {
 
-        // forward control to the servlet.
         try {
-            String profileUrl = buildRedirectUrl(httpRequest);
-
-            log.debug("Redirecting to {}", profileUrl);
-            httpResponse.sendRedirect(profileUrl);
+            log.debug("Forwarding authentication request to {}", externalAuthnPath);
+            populateRequestAttributes(httpRequest);
+            RequestDispatcher dispatcher = httpRequest.getRequestDispatcher(externalAuthnPath);
+            dispatcher.forward(httpRequest, httpResponse);
             return;
-        } catch (IOException ex) {
-            log.error("Unable to redirect to remote user authentication servlet.", ex);
-        }
-    }
-
-    /**
-     * Builds the URL that redirects to the external authentication service.
-     * 
-     * @param httpRequest current HTTP request
-     * 
-     * @return URL to which to redirect the user-agent
-     */
-    protected String buildRedirectUrl(HttpServletRequest httpRequest) {
-        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 (!protectedPath.startsWith("/")) {
-            pathBuilder.append("/");
+        } catch (IOException e) {
+            log.error("Unable to forward authentication request to external authentication system.", e);
+        } catch (ServletException e) {
+            log.error("Unable to forward authentication request to external authentication system.", e);
         }
-        pathBuilder.append(protectedPath);
-        urlBuilder.setPath(pathBuilder.toString());
-
-        urlBuilder.getQueryParams().addAll(buildQueryParameters(httpRequest));
-
-        return urlBuilder.buildURL();
     }
 
     /**
-     * Builds the query parameters that will be sent to the external authentication service.
+     * Sets the request attributes that will be sent to the external authentication service.
      * 
      * @param httpRequest current HTTP request
-     * 
-     * @return query parameters to be sent to the external authentication service
      */
-    protected List<Pair<String, String>> buildQueryParameters(HttpServletRequest httpRequest) {
+    protected void populateRequestAttributes(HttpServletRequest httpRequest) {
         LoginContext loginContext = HttpServletHelper.getLoginContext(httpRequest);
 
-        ArrayList<Pair<String, String>> params = new ArrayList<Pair<String, String>>();
-
-        for (Entry<String, String> staticParam : queryParameters.entrySet()) {
-            params.add(new Pair<String, String>(staticParam.getKey(), staticParam.getValue()));
-        }
-
         if (loginContext.isForceAuthRequired()) {
-            params.add(new Pair<String, String>(FORCE_AUTHN_PARAM, Boolean.TRUE.toString()));
+            httpRequest.setAttribute(FORCE_AUTHN_PARAM, Boolean.TRUE);
         } else {
-            params.add(new Pair<String, String>(FORCE_AUTHN_PARAM, Boolean.FALSE.toString()));
+            httpRequest.setAttribute(FORCE_AUTHN_PARAM, Boolean.FALSE);
         }
 
         if (loginContext.isPassiveAuthRequired()) {
-            params.add(new Pair<String, String>(PASSIVE_AUTHN_PARAM, Boolean.TRUE.toString()));
+            httpRequest.setAttribute(PASSIVE_AUTHN_PARAM, Boolean.TRUE);
         } else {
-            params.add(new Pair<String, String>(PASSIVE_AUTHN_PARAM, Boolean.FALSE.toString()));
+            httpRequest.setAttribute(PASSIVE_AUTHN_PARAM, Boolean.FALSE);
         }
 
-        params.add(new Pair<String, String>(AUTHN_METHOD_PARAM, loginContext.getAttemptedAuthnMethod()));
-
-        params.add(new Pair<String, String>(RELYING_PARTY_PARAM, loginContext.getRelyingPartyId()));
+        httpRequest.setAttribute(AUTHN_METHOD_PARAM, loginContext.getAttemptedAuthnMethod());
 
-        return params;
+        httpRequest.setAttribute(RELYING_PARTY_PARAM, loginContext.getRelyingPartyId());
     }
 }
\ No newline at end of file
diff --git a/src/main/java/edu/internet2/middleware/shibboleth/idp/authn/provider/ExternalAuthnSystemServlet.java b/src/main/java/edu/internet2/middleware/shibboleth/idp/authn/provider/ExternalAuthnSystemServlet.java
deleted file mode 100644 (file)
index 4d22dd8..0000000
+++ /dev/null
@@ -1,63 +0,0 @@
-/*
- * Copyright 2011 University Corporation for Advanced Internet Development, Inc.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package edu.internet2.middleware.shibboleth.idp.authn.provider;
-
-import java.io.IOException;
-
-import javax.servlet.ServletException;
-import javax.servlet.http.HttpServlet;
-import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletResponse;
-
-import org.opensaml.xml.util.DatatypeHelper;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import edu.internet2.middleware.shibboleth.idp.authn.AuthenticationEngine;
-import edu.internet2.middleware.shibboleth.idp.authn.LoginHandler;
-import edu.internet2.middleware.shibboleth.idp.authn.UsernamePrincipal;
-
-/**
- * Extracts the REMOTE_USER and, optionally, the method used to authentication the user and places the information in
- * request attributes used by the authentication engine.
- */
-public class ExternalAuthnSystemServlet extends HttpServlet {
-
-    /** Serial version UID. */
-    private static final long serialVersionUID = -6153665874235557534L;
-
-    /** Class logger. */
-    private final Logger log = LoggerFactory.getLogger(ExternalAuthnSystemServlet.class);
-
-    /** {@inheritDoc} */
-    protected void service(HttpServletRequest httpRequest, HttpServletResponse httpResponse) throws ServletException,
-            IOException {
-        String principalName = httpRequest.getRemoteUser();
-
-        log.debug("User identified as {} returning control back to authentication engine", principalName);
-        httpRequest.setAttribute(LoginHandler.PRINCIPAL_KEY, new UsernamePrincipal(principalName));
-
-        String authnMethod = DatatypeHelper.safeTrimOrNullString(httpRequest
-                .getHeader(ExternalAuthnSystemLoginHandler.AUTHN_METHOD_PARAM));
-        if (authnMethod != null) {
-            log.debug("User {} authenticated by the method {}", principalName, authnMethod);
-            httpRequest.setAttribute(LoginHandler.AUTHENTICATION_METHOD_KEY, authnMethod);
-        }
-
-        AuthenticationEngine.returnToAuthenticationEngine(httpRequest, httpResponse);
-    }
-}
\ No newline at end of file
index b01af27..95a835f 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright [2007] [University Corporation for Advanced Internet Development, Inc.]
+ * Copyright 2011 University Corporation for Advanced Internet Development, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
 
 package edu.internet2.middleware.shibboleth.idp.config.profile.authn;
 
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-
 import javax.xml.namespace.QName;
 
 import org.opensaml.xml.util.DatatypeHelper;
-import org.opensaml.xml.util.XMLHelper;
-import org.springframework.beans.factory.BeanCreationException;
 import org.springframework.beans.factory.support.BeanDefinitionBuilder;
 import org.w3c.dom.Element;
 
@@ -48,46 +41,7 @@ public class ExternalAuthnSystemLoginHandlerBeanDefinitionParser extends Abstrac
     protected void doParse(Element config, BeanDefinitionBuilder builder) {
         super.doParse(config, builder);
 
-        if (config.hasAttributeNS(null, "protectedServletPath")) {
-            builder.addPropertyValue("protectedServletPath",
-                    DatatypeHelper.safeTrimOrNullString(config.getAttributeNS(null, "protectedServletPath")));
-        } else {
-            builder.addPropertyValue("protectedServletPath", "/Authn/External");
-        }
-
-        List<Element> queryParamElements = XMLHelper.getChildElementsByTagNameNS(config,
-                ProfileHandlerNamespaceHandler.NAMESPACE, "QueryParam");
-        builder.addPropertyValue("queryParams", parseQueryParameters(queryParamElements));
-    }
-
-    /**
-     * Parses the query parameter elements, if any, in to a map.
-     * 
-     * @param queryParamElements query parameter elements, may be null or empty
-     * 
-     * @return the map of query elements indexed by the parameter name
-     */
-    protected Map<String, String> parseQueryParameters(List<Element> queryParamElements) {
-        if (queryParamElements == null || queryParamElements.isEmpty()) {
-            return Collections.emptyMap();
-        }
-
-        HashMap<String, String> params = new HashMap<String, String>();
-
-        String paramName;
-        String paramValue;
-        for (Element queryParamElement : queryParamElements) {
-            paramName = DatatypeHelper.safeTrimOrNullString(queryParamElement.getAttributeNS(null, "name"));
-            if (paramName == null) {
-                throw new BeanCreationException("Query parameter name may not be null or empty");
-            }
-            paramValue = DatatypeHelper.safeTrimOrNullString(queryParamElement.getAttributeNS(null, "value"));
-            if (paramValue == null) {
-                throw new BeanCreationException("Query parameter value may not be null or empty");
-            }
-            params.put(paramName, paramValue);
-        }
-
-        return params;
+        builder.addPropertyValue("externalAuthnPath",
+                DatatypeHelper.safeTrimOrNullString(config.getAttributeNS(null, "externalAuthnPath")));
     }
 }
\ No newline at end of file
index 2fb011d..1d55a68 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright [2007] [University Corporation for Advanced Internet Development, Inc.]
+ * Copyright 2011 University Corporation for Advanced Internet Development, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,8 +16,6 @@
 
 package edu.internet2.middleware.shibboleth.idp.config.profile.authn;
 
-import java.util.Map;
-
 import edu.internet2.middleware.shibboleth.idp.authn.provider.ExternalAuthnSystemLoginHandler;
 
 /**
@@ -25,11 +23,8 @@ import edu.internet2.middleware.shibboleth.idp.authn.provider.ExternalAuthnSyste
  */
 public class ExternalAuthnSystemLoginHandlerFactoryBean extends AbstractLoginHandlerFactoryBean {
 
-    /** Path to protected servlet. */
-    private String protectedServletPath;
-
-    /** Static query parameters added to the request to the external authentication system invocation. */
-    private Map<String, String> queryParams;
+    /** The context-relative path to the Filter, Servlet, or JSP that triggers the external authentication system. */
+    private String externalAuthnPath;
 
     /** {@inheritDoc} */
     public Class getObjectType() {
@@ -37,46 +32,28 @@ public class ExternalAuthnSystemLoginHandlerFactoryBean extends AbstractLoginHan
     }
 
     /**
-     * Gets the path to protected Servlet.
-     * 
-     * @return path to protected servlet
-     */
-    public String getProtectedServletPath() {
-        return protectedServletPath;
-    }
-
-    /**
-     * Sets the path to protected servlet.
-     * 
-     * @param path path to protected servlet
-     */
-    public void setProtectedServletPath(String path) {
-        protectedServletPath = path;
-    }
-
-    /**
-     * Gets the static query parameters added to the request to the external authentication system invocation.
+     * Get context-relative path to the Filter, Servlet, or JSP that triggers the external authentication system.
      * 
-     * @return static query parameters added to the request to the external authentication system invocation
+     * @return context-relative path to the Filter, Servlet, or JSP that triggers the external authentication system
      */
-    public Map<String, String> getQueryParams() {
-        return queryParams;
+    public String getExternalAuthnPath() {
+        return externalAuthnPath;
     }
 
     /**
-     * Sets the static query parameters added to the request to the external authentication system invocation.
+     * Set context-relative path to the Filter, Servlet, or JSP that triggers the external authentication system.
      * 
-     * @param params static query parameters added to the request to the external authentication system invocation
+     * @param path context-relative path to the Filter, Servlet, or JSP that triggers the external authentication
+     *            system, may not be null or empty
      */
-    public void setQueryParams(Map<String, String> params) {
-        queryParams = params;
+    public void setExternalAuthnPath(String path) {
+        externalAuthnPath = path;
     }
 
     /** {@inheritDoc} */
     protected Object createInstance() throws Exception {
         ExternalAuthnSystemLoginHandler handler = new ExternalAuthnSystemLoginHandler();
-        handler.setProtectedPath(getProtectedServletPath());
-        handler.setQueryParameters(queryParams);
+        handler.setExternalAuthnPath(getExternalAuthnPath());
         populateHandler(handler);
         return handler;
     }
index b30cd6b..1d8cb72 100644 (file)
     <xsd:complexType name="ExternalAuthn">
         <xsd:complexContent>
             <xsd:extension base="LoginHandlerType">
-                <xsd:sequence>
-                    <xsd:element name="QueryParam" minOccurs="0" maxOccurs="unbounded">
-                        <xsd:complexType>
-                            <xsd:attribute name="name" type="xsd:string" use="required" />
-                            <xsd:attribute name="value" type="xsd:string" use="required" />
-                        </xsd:complexType>                    
-                    </xsd:element>
-                </xsd:sequence>
-                <xsd:attribute name="protectedServletPath" type="xsd:string">
+                <xsd:attribute name="externalAuthnPath" type="xsd:string" use="required">
                     <xsd:annotation>
                         <xsd:documentation>
                             The servlet context path to the
index 89de743..2c214e9 100644 (file)
         <servlet-name>AuthenticationEngine</servlet-name>
         <url-pattern>/AuthnEngine</url-pattern>
     </servlet-mapping>
-    
-    <!-- Servlet protected by an external authentication service -->
-    <servlet>
-        <servlet-name>ExternalAuthHandler</servlet-name>
-        <servlet-class>edu.internet2.middleware.shibboleth.idp.authn.provider.ExternalAuthnSystemServlet</servlet-class>
-        <load-on-startup>3</load-on-startup>
-    </servlet>
-
-    <servlet-mapping>
-        <servlet-name>ExternalAuthHandler</servlet-name>
-        <url-pattern>/Authn/External</url-pattern>
-    </servlet-mapping>
 
     <!-- Servlet protected by container used for RemoteUser authentication -->
     <servlet>