Browse Source

Remove UsernamePasswordAuthenticationToken check

This commit reverts 21dd050d7b69bf3a8efdb46100893d151fe8b15e.

Closes gh-10347
Steve Riesenberg 2 years ago
parent
commit
e0e6467d9b

+ 1 - 6
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
 		// Only reauthenticate if username doesn't match SecurityContextHolder and user
 		// isn't authenticated (see SEC-53)
 		// isn't authenticated (see SEC-53)
 		Authentication existingAuth = this.securityContextHolderStrategy.getContext().getAuthentication();
 		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;
 			return true;
 		}
 		}
 		// Handle unusual condition where an AnonymousAuthenticationToken is already
 		// Handle unusual condition where an AnonymousAuthenticationToken is already

+ 78 - 0
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.mock.web.MockHttpSession;
 import org.springframework.security.authentication.AuthenticationManager;
 import org.springframework.security.authentication.AuthenticationManager;
 import org.springframework.security.authentication.BadCredentialsException;
 import org.springframework.security.authentication.BadCredentialsException;
+import org.springframework.security.authentication.TestingAuthenticationToken;
 import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
 import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.authority.AuthorityUtils;
 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.never;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
 
 
 /**
 /**
  * Tests {@link BasicAuthenticationFilter}.
  * Tests {@link BasicAuthenticationFilter}.
@@ -410,4 +412,80 @@ public class BasicAuthenticationFilterTests {
 		assertThat(contextArg.getValue().getAuthentication().getName()).isEqualTo("rod");
 		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<Authentication> 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<Authentication> 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");
+	}
+
 }
 }