Expose login exception to login.jsp page in order to allow better message customizati...

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

diff --git a/doc/RELEASE-NOTES.txt b/doc/RELEASE-NOTES.txt
index 21bee76..592d332 100644 (file)
--- a/doc/RELEASE-NOTES.txt
+++ b/doc/RELEASE-NOTES.txt
@@ -1,5 +1,6 @@
 Changes in Release 2.2.0
 =============================================
+[SIDP-368] - Provide more acurate login error to servlet when Username/Password login authentication has failed.
 [SIDP-365] - Expose uptime of IdP web application with status handler
 [SIDP-362] - Only log exception message without stack trace for expired SAML messages
 [SIDP-360] - Session isn't being set within the attribute request context during a SAML1 attribute query

diff --git a/src/main/java/edu/internet2/middleware/shibboleth/idp/authn/provider/UsernamePasswordLoginServlet.java b/src/main/java/edu/internet2/middleware/shibboleth/idp/authn/provider/UsernamePasswordLoginServlet.java
index 0dda801..fca93b6 100644 (file)
--- a/src/main/java/edu/internet2/middleware/shibboleth/idp/authn/provider/UsernamePasswordLoginServlet.java
+++ b/src/main/java/edu/internet2/middleware/shibboleth/idp/authn/provider/UsernamePasswordLoginServlet.java
@@ -18,8 +18,6 @@ package edu.internet2.middleware.shibboleth.idp.authn.provider;
 
 import java.io.IOException;
 import java.security.Principal;
-import java.util.ArrayList;
-import java.util.List;
 import java.util.Set;
 
 import javax.security.auth.Subject;
@@ -28,6 +26,7 @@ import javax.security.auth.callback.CallbackHandler;
 import javax.security.auth.callback.NameCallback;
 import javax.security.auth.callback.PasswordCallback;
 import javax.security.auth.callback.UnsupportedCallbackException;
+import javax.security.auth.login.LoginException;
 import javax.servlet.ServletConfig;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServlet;
@@ -35,7 +34,6 @@ import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
 import org.opensaml.xml.util.DatatypeHelper;
-import org.opensaml.xml.util.Pair;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -99,16 +97,17 @@ public class UsernamePasswordLoginServlet extends HttpServlet {
         String password = request.getParameter(passwordAttribute);
 
         if (username == null || password == null) {
-            redirectToLoginPage(request, response, null);
+            redirectToLoginPage(request, response);
             return;
         }
 
-        if (authenticateUser(request, username, password)) {
+        try {
+            authenticateUser(request, username, password);
             AuthenticationEngine.returnToAuthenticationEngine(request, response);
-        } else {
-            List<Pair<String, String>> queryParams = new ArrayList<Pair<String, String>>();
-            queryParams.add(new Pair<String, String>(failureParam, "true"));
-            redirectToLoginPage(request, response, queryParams);
+        } catch (LoginException e) {
+            request.setAttribute(failureParam, true);
+            request.setAttribute(LoginHandler.AUTHENTICATION_EXCEPTION_KEY, e);
+            redirectToLoginPage(request, response);
         }
     }
 
@@ -117,10 +116,8 @@ public class UsernamePasswordLoginServlet extends HttpServlet {
      * 
      * @param request current request
      * @param response current response
-     * @param queryParams query parameters to pass to the login page
      */
-    protected void redirectToLoginPage(HttpServletRequest request, HttpServletResponse response,
-            List<Pair<String, String>> queryParams) {
+    protected void redirectToLoginPage(HttpServletRequest request, HttpServletResponse response) {
 
         String requestContext = DatatypeHelper.safeTrimOrNullString(request.getContextPath());
         if (requestContext == null) {
@@ -128,12 +125,6 @@ public class UsernamePasswordLoginServlet extends HttpServlet {
         }
         request.setAttribute("actionUrl", requestContext + request.getServletPath());
 
-        if (queryParams != null) {
-            for (Pair<String, String> param : queryParams) {
-                request.setAttribute(param.getFirst(), param.getSecond());
-            }
-        }
-
         try {
             request.getRequestDispatcher(loginPage).forward(request, response);
             log.debug("Redirecting to login page {}", loginPage);
@@ -152,9 +143,9 @@ public class UsernamePasswordLoginServlet extends HttpServlet {
      * @param username the principal name of the user to be authenticated
      * @param password the password of the user to be authenticated
      * 
-     * @return true of authentication succeeds, false if not
+     * @throws LoginException thrown if there is a problem authenticating the user
      */
-    protected boolean authenticateUser(HttpServletRequest request, String username, String password) {
+    protected void authenticateUser(HttpServletRequest request, String username, String password) throws LoginException {
         try {
             log.debug("Attempting to authenticate user {}", username);
 
@@ -180,11 +171,12 @@ public class UsernamePasswordLoginServlet extends HttpServlet {
 
             Subject userSubject = new Subject(false, principals, publicCredentials, privateCredentials);
             request.setAttribute(LoginHandler.SUBJECT_KEY, userSubject);
-
-            return true;
+        } catch (LoginException e) {
+            log.debug("User authentication for " + username + " failed", e);
+            throw e;
         } catch (Throwable e) {
             log.debug("User authentication for " + username + " failed", e);
-            return false;
+            throw new LoginException("unknown authentication error");
         }
     }
 

diff --git a/src/main/webapp/login.jsp b/src/main/webapp/login.jsp
index 7d8719b..b3db7e2 100644 (file)
--- a/src/main/webapp/login.jsp
+++ b/src/main/webapp/login.jsp
@@ -1,4 +1,5 @@
 <%@ page import="edu.internet2.middleware.shibboleth.idp.authn.LoginContext" %>
+<%@ page import="edu.internet2.middleware.shibboleth.idp.authn.LoginHandler" %>
 <%@ page import="edu.internet2.middleware.shibboleth.idp.session.*" %>
 <%@ page import="edu.internet2.middleware.shibboleth.idp.util.HttpServletHelper" %>
 <%@ page import="org.opensaml.saml2.metadata.*" %>
@@ -25,7 +26,7 @@
                Is Forced Authentication: <%= loginContext.isForceAuthRequired() %><br/>
                </p>
                
-               <% if ("true".equals(request.getAttribute("loginFailed"))) { %>
+               <% if (request.getAttribute(LoginHandler.AUTHENTICATION_EXCEPTION_KEY) != null) { %>
                <p><font color="red">Authentication Failed</font></p>
                <% } %>