From 8e744073810117e9752399270fad5a96a286fc82 Mon Sep 17 00:00:00 2001
From: Rob Winch <rwinch@vmware.com>
Date: Sat, 31 Aug 2013 11:55:24 -0500
Subject: [PATCH] SEC-2296: HttpServletRequest.login should throw
 ServletException if already authenticated

See throws documentation at
http://docs.oracle.com/javaee/6/api/javax/servlet/http/HttpServletRequest.html#login%28java.lang.String,%20java.lang.String%29
---
 .../HttpServlet3RequestFactory.java           | 13 +++++++++++--
 ...yContextHolderAwareRequestFilterTests.java | 19 ++++++++++++++++++-
 2 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/web/src/main/java/org/springframework/security/web/servletapi/HttpServlet3RequestFactory.java b/web/src/main/java/org/springframework/security/web/servletapi/HttpServlet3RequestFactory.java
index 2c453e5d0e..171d5332de 100644
--- a/web/src/main/java/org/springframework/security/web/servletapi/HttpServlet3RequestFactory.java
+++ b/web/src/main/java/org/springframework/security/web/servletapi/HttpServlet3RequestFactory.java
@@ -162,8 +162,7 @@ final class HttpServlet3RequestFactory implements HttpServletRequestFactory {
                 logger.debug("authenticationEntryPoint is null, so allowing original HttpServletRequest to handle authenticate");
                 return super.authenticate(response);
             }
-            Principal userPrincipal = getUserPrincipal();
-            if(userPrincipal != null) {
+            if(isAuthenticated()) {
                 return true;
             }
             entryPoint.commence(this, response, new AuthenticationCredentialsNotFoundException("User is not Authenticated"));
@@ -171,6 +170,11 @@ final class HttpServlet3RequestFactory implements HttpServletRequestFactory {
         }
 
         public void login(String username, String password) throws ServletException {
+            if(isAuthenticated()) {
+                throw new ServletException("Cannot perform login for '"
+                        + username + "' already authenticated as '"
+                        + getRemoteUser() + "'");
+            }
             AuthenticationManager authManager = authenticationManager;
             if(authManager == null) {
                 logger.debug("authenticationManager is null, so allowing original HttpServletRequest to handle login");
@@ -199,6 +203,11 @@ final class HttpServlet3RequestFactory implements HttpServletRequestFactory {
                 logoutHandler.logout(this, response, authentication);
             }
         }
+
+        private boolean isAuthenticated() {
+            Principal userPrincipal = getUserPrincipal();
+            return userPrincipal != null;
+        }
     }
 
     private static class SecurityContextAsyncContext implements AsyncContext {
diff --git a/web/src/test/java/org/springframework/security/web/servletapi/SecurityContextHolderAwareRequestFilterTests.java b/web/src/test/java/org/springframework/security/web/servletapi/SecurityContextHolderAwareRequestFilterTests.java
index 7c9e490c4b..cf0a51bd5f 100644
--- a/web/src/test/java/org/springframework/security/web/servletapi/SecurityContextHolderAwareRequestFilterTests.java
+++ b/web/src/test/java/org/springframework/security/web/servletapi/SecurityContextHolderAwareRequestFilterTests.java
@@ -16,6 +16,7 @@
 
 package org.springframework.security.web.servletapi;
 
+import static junit.framework.Assert.fail;
 import static org.fest.assertions.Assertions.assertThat;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyString;
@@ -172,11 +173,27 @@ public class SecurityContextHolderAwareRequestFilterTests {
         verify(request, times(0)).login(anyString(),anyString());
     }
 
+    // SEC-2296
+    @Test
+    public void loginWithExstingUser() throws Exception {
+        TestingAuthenticationToken expectedAuth = new TestingAuthenticationToken("user", "password","ROLE_USER");
+        when(authenticationManager.authenticate(any(UsernamePasswordAuthenticationToken.class))).thenReturn(new TestingAuthenticationToken("newuser","not be found","ROLE_USER"));
+        SecurityContextHolder.getContext().setAuthentication(expectedAuth);
+
+        try {
+            wrappedRequest().login(expectedAuth.getName(),String.valueOf(expectedAuth.getCredentials()));
+            fail("Expected Exception");
+        } catch(ServletException success) {
+            assertThat(SecurityContextHolder.getContext().getAuthentication()).isSameAs(expectedAuth);
+            verifyZeroInteractions(authenticationEntryPoint, logoutHandler);
+            verify(request, times(0)).login(anyString(),anyString());
+        }
+    }
+
     @Test
     public void loginFail() throws Exception {
         AuthenticationException authException = new BadCredentialsException("Invalid");
         when(authenticationManager.authenticate(any(UsernamePasswordAuthenticationToken.class))).thenThrow(authException);
-        SecurityContextHolder.getContext().setAuthentication(new TestingAuthenticationToken("should","be cleared","ROLE_USER"));
 
         try {
             wrappedRequest().login("invalid","credentials");