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;