From d973f5f80c31324a8899df1df071efca072685f3 Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Tue, 4 Dec 2012 10:47:29 -0600 Subject: [PATCH] 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(). --- ...tractPreAuthenticatedProcessingFilter.java | 25 +++- ...PreAuthenticatedProcessingFilterTests.java | 137 +++++++++++++++++- 2 files changed, 155 insertions(+), 7 deletions(-) diff --git a/web/src/main/java/org/springframework/security/web/authentication/preauth/AbstractPreAuthenticatedProcessingFilter.java b/web/src/main/java/org/springframework/security/web/authentication/preauth/AbstractPreAuthenticatedProcessingFilter.java index 312a17802b..deaa90b664 100755 --- a/web/src/main/java/org/springframework/security/web/authentication/preauth/AbstractPreAuthenticatedProcessingFilter.java +++ b/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 @@ import org.springframework.web.filter.GenericFilterBean; * * @author Luke Taylor * @author Ruud Senden + * @author Rob Winch * @since 2.0 */ public abstract class AbstractPreAuthenticatedProcessingFilter extends GenericFilterBean implements @@ -142,7 +155,11 @@ public abstract class AbstractPreAuthenticatedProcessingFilter extends GenericFi 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; } @@ -233,9 +250,9 @@ public abstract class AbstractPreAuthenticatedProcessingFilter extends GenericFi } /** - * If set, the pre-authenticated principal will be checked on each request and compared - * against the name of the current Authentication object. If a change is detected, - * the user will be reauthenticated. + * If set, the pre-authenticated principal will be checked on each request and compared against the name of the + * current Authentication object. A check to determine if {@link Authentication#getPrincipal()} is equal + * to the principal will also be performed. If a change is detected, the user will be reauthenticated. * * @param checkForPrincipalChanges */ diff --git a/web/src/test/java/org/springframework/security/web/authentication/preauth/AbstractPreAuthenticatedProcessingFilterTests.java b/web/src/test/java/org/springframework/security/web/authentication/preauth/AbstractPreAuthenticatedProcessingFilterTests.java index 6dc9c8a5e4..bb8ebf7b32 100644 --- a/web/src/test/java/org/springframework/security/web/authentication/preauth/AbstractPreAuthenticatedProcessingFilterTests.java +++ b/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.mock.web.MockHttpServletResponse; 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 class AbstractPreAuthenticatedProcessingFilterTests { 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 class AbstractPreAuthenticatedProcessingFilterTests { } private static class ConcretePreAuthenticatedProcessingFilter extends AbstractPreAuthenticatedProcessingFilter { - private String principal = "testPrincipal"; + private Object principal = "testPrincipal"; private boolean initFilterBeanInvoked; protected Object getPreAuthenticatedPrincipal(HttpServletRequest httpRequest) { return principal;