SEC-2078: AbstractPreAuthenticatedProcessingFilter requriesAuthentication support for non-String Principals

Previously, if the Principal returned by getPreAuthenticatedPrincipal was not a String,
it prevented requiresAuthentication from detecting when the Principal was the same.
This caused the need to authenticate the user for every request even when the Principal
did not change.

Now requiresAuthentication will check to see if the result of
getPreAuthenticatedPrincipal is equal to the current Authentication.getPrincipal().

Author: rwinch

web/src/main/java/org/springframework/security/web/authentication/preauth/AbstractPreAuthenticatedProcessingFilter.java

@@ -1,3 +1,15 @@
+/*
+ * Copyright 2002-2012 the original author or authors.
+ *
+ * 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 org.springframework.security.web.authentication.preauth;
 
 import java.io.IOException;
@@ -50,6 +62,7 @@
  *
  * @author Luke Taylor
  * @author Ruud Senden
+ * @author Rob Winch
  * @since 2.0
  */
 public abstract class AbstractPreAuthenticatedProcessingFilter extends GenericFilterBean implements
@@ -142,7 +155,11 @@ private boolean requiresAuthentication(HttpServletRequest request) {
 
         Object principal = getPreAuthenticatedPrincipal(request);
 
-        if (currentUser.getName().equals(principal)) {
+        if ((principal instanceof String) && currentUser.getName().equals(principal)) {
+            return false;
+        }
+
+        if(principal != null && principal.equals(currentUser.getPrincipal())) {
             return false;
         }
 

web/src/test/java/org/springframework/security/web/authentication/preauth/AbstractPreAuthenticatedProcessingFilterTests.java

@@ -1,8 +1,26 @@
+/*
+ * Copyright 2002-2012 the original author or authors.
+ *
+ * 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 org.springframework.security.web.authentication.preauth;
 
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 import static org.mockito.Matchers.any;
-import static org.mockito.Mockito.*;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyZeroInteractions;
+import static org.mockito.Mockito.when;
 
 import javax.servlet.FilterChain;
 import javax.servlet.ServletException;
@@ -19,9 +37,17 @@
 import org.springframework.security.authentication.AuthenticationManager;
 import org.springframework.security.authentication.BadCredentialsException;
 import org.springframework.security.authentication.TestingAuthenticationToken;
+import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
 import org.springframework.security.core.Authentication;
+import org.springframework.security.core.authority.AuthorityUtils;
 import org.springframework.security.core.context.SecurityContextHolder;
+import org.springframework.security.core.userdetails.User;
 
+/**
+ *
+ * @author Rob Winch
+ *
+ */
 public class AbstractPreAuthenticatedProcessingFilterTests {
     private AbstractPreAuthenticatedProcessingFilter filter;
 
@@ -123,6 +149,111 @@ public void nullPreAuthenticationPerservesPreviousUserCheckPrincipalChangesFalse
         assertEquals(authentication, SecurityContextHolder.getContext().getAuthentication());
     }
 
+    @Test
+    public void requiresAuthenticationFalsePrincipalString() throws Exception {
+        Object principal = "sameprincipal";
+        SecurityContextHolder.getContext().setAuthentication(new TestingAuthenticationToken(principal, "something", "ROLE_USER"));
+        MockHttpServletRequest request = new MockHttpServletRequest();
+        MockHttpServletResponse response = new MockHttpServletResponse();
+        MockFilterChain chain = new MockFilterChain();
+
+        ConcretePreAuthenticatedProcessingFilter filter = new ConcretePreAuthenticatedProcessingFilter();
+        filter.setCheckForPrincipalChanges(true);
+        filter.principal = principal;
+        AuthenticationManager am = mock(AuthenticationManager.class);
+        filter.setAuthenticationManager(am);
+        filter.afterPropertiesSet();
+
+        filter.doFilter(request, response, chain);
+
+        verifyZeroInteractions(am);
+    }
+
+    @Test
+    public void requiresAuthenticationTruePrincipalString() throws Exception {
+        Object currentPrincipal = "currentUser";
+        TestingAuthenticationToken authRequest = new TestingAuthenticationToken(currentPrincipal, "something", "ROLE_USER");
+        SecurityContextHolder.getContext().setAuthentication(authRequest);
+        MockHttpServletRequest request = new MockHttpServletRequest();
+        MockHttpServletResponse response = new MockHttpServletResponse();
+        MockFilterChain chain = new MockFilterChain();
+
+        ConcretePreAuthenticatedProcessingFilter filter = new ConcretePreAuthenticatedProcessingFilter();
+        filter.setCheckForPrincipalChanges(true);
+        filter.principal = "newUser";
+        AuthenticationManager am = mock(AuthenticationManager.class);
+        filter.setAuthenticationManager(am);
+        filter.afterPropertiesSet();
+
+        filter.doFilter(request, response, chain);
+
+        verify(am).authenticate(any(PreAuthenticatedAuthenticationToken.class));
+    }
+
+    // SEC-2078
+    @Test
+    public void requiresAuthenticationFalsePrincipalNotString() throws Exception {
+        Object principal = new Object();
+        SecurityContextHolder.getContext().setAuthentication(new TestingAuthenticationToken(principal, "something", "ROLE_USER"));
+        MockHttpServletRequest request = new MockHttpServletRequest();
+        MockHttpServletResponse response = new MockHttpServletResponse();
+        MockFilterChain chain = new MockFilterChain();
+
+        ConcretePreAuthenticatedProcessingFilter filter = new ConcretePreAuthenticatedProcessingFilter();
+        filter.setCheckForPrincipalChanges(true);
+        filter.principal = principal;
+        AuthenticationManager am = mock(AuthenticationManager.class);
+        filter.setAuthenticationManager(am);
+        filter.afterPropertiesSet();
+
+        filter.doFilter(request, response, chain);
+
+        verifyZeroInteractions(am);
+    }
+
+    @Test
+    public void requiresAuthenticationFalsePrincipalUser() throws Exception {
+        User currentPrincipal = new User("user","password", AuthorityUtils.createAuthorityList("ROLE_USER"));
+        UsernamePasswordAuthenticationToken currentAuthentication = new UsernamePasswordAuthenticationToken(
+                currentPrincipal, currentPrincipal.getPassword(), currentPrincipal.getAuthorities());
+        SecurityContextHolder.getContext().setAuthentication(currentAuthentication);
+        MockHttpServletRequest request = new MockHttpServletRequest();
+        MockHttpServletResponse response = new MockHttpServletResponse();
+        MockFilterChain chain = new MockFilterChain();
+
+        ConcretePreAuthenticatedProcessingFilter filter = new ConcretePreAuthenticatedProcessingFilter();
+        filter.setCheckForPrincipalChanges(true);
+        filter.principal = new User(currentPrincipal.getUsername(), currentPrincipal.getPassword(), AuthorityUtils.NO_AUTHORITIES);
+        AuthenticationManager am = mock(AuthenticationManager.class);
+        filter.setAuthenticationManager(am);
+        filter.afterPropertiesSet();
+
+        filter.doFilter(request, response, chain);
+
+        verifyZeroInteractions(am);
+    }
+
+    @Test
+    public void requiresAuthenticationTruePrincipalNotString() throws Exception {
+        Object currentPrincipal = new Object();
+        TestingAuthenticationToken authRequest = new TestingAuthenticationToken(currentPrincipal, "something", "ROLE_USER");
+        SecurityContextHolder.getContext().setAuthentication(authRequest);
+        MockHttpServletRequest request = new MockHttpServletRequest();
+        MockHttpServletResponse response = new MockHttpServletResponse();
+        MockFilterChain chain = new MockFilterChain();
+
+        ConcretePreAuthenticatedProcessingFilter filter = new ConcretePreAuthenticatedProcessingFilter();
+        filter.setCheckForPrincipalChanges(true);
+        filter.principal = new Object();
+        AuthenticationManager am = mock(AuthenticationManager.class);
+        filter.setAuthenticationManager(am);
+        filter.afterPropertiesSet();
+
+        filter.doFilter(request, response, chain);
+
+        verify(am).authenticate(any(PreAuthenticatedAuthenticationToken.class));
+    }
+
     private void testDoFilter(boolean grantAccess) throws Exception {
         MockHttpServletRequest req = new MockHttpServletRequest();
         MockHttpServletResponse res = new MockHttpServletResponse();
@@ -150,7 +281,7 @@ public Authentication answer(InvocationOnMock invocation) throws Throwable {
     }
 
     private static class ConcretePreAuthenticatedProcessingFilter extends AbstractPreAuthenticatedProcessingFilter {
-        private String principal = "testPrincipal";
+        private Object principal = "testPrincipal";
         private boolean initFilterBeanInvoked;
         protected Object getPreAuthenticatedPrincipal(HttpServletRequest httpRequest) {
             return principal;