From e0e6467d9b1203b38d152b40281265e0816af8be Mon Sep 17 00:00:00 2001 From: Steve Riesenberg Date: Tue, 20 Sep 2022 11:57:27 -0500 Subject: [PATCH] Remove UsernamePasswordAuthenticationToken check This commit reverts 21dd050d7b69bf3a8efdb46100893d151fe8b15e. Closes gh-10347 --- .../www/BasicAuthenticationFilter.java | 7 +- .../www/BasicAuthenticationFilterTests.java | 78 +++++++++++++++++++ 2 files changed, 79 insertions(+), 6 deletions(-) diff --git a/web/src/main/java/org/springframework/security/web/authentication/www/BasicAuthenticationFilter.java b/web/src/main/java/org/springframework/security/web/authentication/www/BasicAuthenticationFilter.java index 4b680efc0e..99f3639301 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/www/BasicAuthenticationFilter.java +++ b/web/src/main/java/org/springframework/security/web/authentication/www/BasicAuthenticationFilter.java @@ -206,12 +206,7 @@ public class BasicAuthenticationFilter extends OncePerRequestFilter { // Only reauthenticate if username doesn't match SecurityContextHolder and user // isn't authenticated (see SEC-53) Authentication existingAuth = this.securityContextHolderStrategy.getContext().getAuthentication(); - if (existingAuth == null || !existingAuth.isAuthenticated()) { - return true; - } - // Limit username comparison to providers which use usernames (ie - // UsernamePasswordAuthenticationToken) (see SEC-348) - if (existingAuth instanceof UsernamePasswordAuthenticationToken && !existingAuth.getName().equals(username)) { + if (existingAuth == null || !existingAuth.getName().equals(username) || !existingAuth.isAuthenticated()) { return true; } // Handle unusual condition where an AnonymousAuthenticationToken is already diff --git a/web/src/test/java/org/springframework/security/web/authentication/www/BasicAuthenticationFilterTests.java b/web/src/test/java/org/springframework/security/web/authentication/www/BasicAuthenticationFilterTests.java index f901438a15..ca29009479 100644 --- a/web/src/test/java/org/springframework/security/web/authentication/www/BasicAuthenticationFilterTests.java +++ b/web/src/test/java/org/springframework/security/web/authentication/www/BasicAuthenticationFilterTests.java @@ -33,6 +33,7 @@ import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.mock.web.MockHttpSession; 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; @@ -55,6 +56,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; /** * Tests {@link BasicAuthenticationFilter}. @@ -410,4 +412,80 @@ public class BasicAuthenticationFilterTests { assertThat(contextArg.getValue().getAuthentication().getName()).isEqualTo("rod"); } + @Test + public void doFilterWhenUsernameDoesNotChangeThenAuthenticationIsNotRequired() throws Exception { + SecurityContextHolderStrategy securityContextHolderStrategy = SecurityContextHolder.getContextHolderStrategy(); + SecurityContext securityContext = securityContextHolderStrategy.createEmptyContext(); + Authentication authentication = UsernamePasswordAuthenticationToken.authenticated("rod", "koala", + AuthorityUtils.createAuthorityList("USER")); + securityContext.setAuthentication(authentication); + securityContextHolderStrategy.setContext(securityContext); + + String token = "rod:koala"; + MockHttpServletRequest request = new MockHttpServletRequest(); + request.addHeader("Authorization", "Basic " + CodecTestUtils.encodeBase64(token)); + FilterChain filterChain = mock(FilterChain.class); + MockHttpServletResponse response = new MockHttpServletResponse(); + this.filter.doFilter(request, response, filterChain); + assertThat(response.getStatus()).isEqualTo(200); + + verify(this.manager, never()).authenticate(any(Authentication.class)); + verify(filterChain).doFilter(any(ServletRequest.class), any(ServletResponse.class)); + verifyNoMoreInteractions(this.manager, filterChain); + } + + @Test + public void doFilterWhenUsernameChangesThenAuthenticationIsRequired() throws Exception { + SecurityContextHolderStrategy securityContextHolderStrategy = SecurityContextHolder.getContextHolderStrategy(); + SecurityContext securityContext = securityContextHolderStrategy.createEmptyContext(); + Authentication authentication = UsernamePasswordAuthenticationToken.authenticated("user", "password", + AuthorityUtils.createAuthorityList("USER")); + securityContext.setAuthentication(authentication); + securityContextHolderStrategy.setContext(securityContext); + + String token = "rod:koala"; + MockHttpServletRequest request = new MockHttpServletRequest(); + request.addHeader("Authorization", "Basic " + CodecTestUtils.encodeBase64(token)); + FilterChain filterChain = mock(FilterChain.class); + MockHttpServletResponse response = new MockHttpServletResponse(); + this.filter.doFilter(request, response, filterChain); + assertThat(response.getStatus()).isEqualTo(200); + + ArgumentCaptor authenticationCaptor = ArgumentCaptor.forClass(Authentication.class); + verify(this.manager).authenticate(authenticationCaptor.capture()); + verify(filterChain).doFilter(any(ServletRequest.class), any(ServletResponse.class)); + verifyNoMoreInteractions(this.manager, filterChain); + + Authentication authenticationRequest = authenticationCaptor.getValue(); + assertThat(authenticationRequest).isInstanceOf(UsernamePasswordAuthenticationToken.class); + assertThat(authenticationRequest.getName()).isEqualTo("rod"); + } + + @Test + public void doFilterWhenUsernameChangesAndNotUsernamePasswordAuthenticationTokenThenAuthenticationIsRequired() + throws Exception { + SecurityContextHolderStrategy securityContextHolderStrategy = SecurityContextHolder.getContextHolderStrategy(); + SecurityContext securityContext = securityContextHolderStrategy.createEmptyContext(); + Authentication authentication = new TestingAuthenticationToken("user", "password", "USER"); + securityContext.setAuthentication(authentication); + securityContextHolderStrategy.setContext(securityContext); + + String token = "rod:koala"; + MockHttpServletRequest request = new MockHttpServletRequest(); + request.addHeader("Authorization", "Basic " + CodecTestUtils.encodeBase64(token)); + FilterChain filterChain = mock(FilterChain.class); + MockHttpServletResponse response = new MockHttpServletResponse(); + this.filter.doFilter(request, response, filterChain); + assertThat(response.getStatus()).isEqualTo(200); + + ArgumentCaptor authenticationCaptor = ArgumentCaptor.forClass(Authentication.class); + verify(this.manager).authenticate(authenticationCaptor.capture()); + verify(filterChain).doFilter(any(ServletRequest.class), any(ServletResponse.class)); + verifyNoMoreInteractions(this.manager, filterChain); + + Authentication authenticationRequest = authenticationCaptor.getValue(); + assertThat(authenticationRequest).isInstanceOf(UsernamePasswordAuthenticationToken.class); + assertThat(authenticationRequest.getName()).isEqualTo("rod"); + } + }