Ver Fonte

Remove Authority Copying From Reactive

We will re-address this when adding factors to
ReactiveAuthenticationManager implementations.

Issue gh-2603
Josh Cummings há 6 dias atrás
pai
commit
793820acfa

+ 0 - 43
web/src/main/java/org/springframework/security/web/server/authentication/AuthenticationWebFilter.java

@@ -16,10 +16,7 @@
 
 package org.springframework.security.web.server.authentication;
 
-import java.lang.reflect.Method;
-import java.util.Set;
 import java.util.function.Function;
-import java.util.stream.Collectors;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -30,7 +27,6 @@ import org.springframework.security.authentication.ReactiveAuthenticationManager
 import org.springframework.security.authentication.ReactiveAuthenticationManagerResolver;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.AuthenticationException;
-import org.springframework.security.core.GrantedAuthority;
 import org.springframework.security.core.context.ReactiveSecurityContextHolder;
 import org.springframework.security.core.context.SecurityContextImpl;
 import org.springframework.security.web.server.WebFilterExchange;
@@ -126,51 +122,12 @@ public class AuthenticationWebFilter implements WebFilter {
 			.flatMap((authenticationManager) -> authenticationManager.authenticate(token))
 			.switchIfEmpty(Mono
 				.defer(() -> Mono.error(new IllegalStateException("No provider found for " + token.getClass()))))
-			.flatMap(this::applyCurrentAuthenication)
 			.flatMap(
 					(authentication) -> onAuthenticationSuccess(authentication, new WebFilterExchange(exchange, chain)))
 			.doOnError(AuthenticationException.class,
 					(ex) -> logger.debug(LogMessage.format("Authentication failed: %s", ex.getMessage()), ex));
 	}
 
-	private Mono<Authentication> applyCurrentAuthenication(Authentication result) {
-		return ReactiveSecurityContextHolder.getContext().map((context) -> {
-			Authentication current = context.getAuthentication();
-			if (current == null) {
-				return result;
-			}
-			if (!current.isAuthenticated()) {
-				return result;
-			}
-			if (!declaresToBuilder(result)) {
-				return result;
-			}
-			return result.toBuilder()
-			// @formatter:off
-				.authorities((a) -> {
-					Set<String> newAuthorities = a.stream()
-						.map(GrantedAuthority::getAuthority)
-						.collect(Collectors.toUnmodifiableSet());
-					for (GrantedAuthority currentAuthority : current.getAuthorities()) {
-						if (!newAuthorities.contains(currentAuthority.getAuthority())) {
-							a.add(currentAuthority);
-						}
-					}
-				})
-				.build();
-				// @formatter:on
-		}).switchIfEmpty(Mono.just(result));
-	}
-
-	private static boolean declaresToBuilder(Authentication authentication) {
-		for (Method method : authentication.getClass().getDeclaredMethods()) {
-			if (method.getName().equals("toBuilder") && method.getParameterTypes().length == 0) {
-				return true;
-			}
-		}
-		return false;
-	}
-
 	protected Mono<Void> onAuthenticationSuccess(Authentication authentication, WebFilterExchange webFilterExchange) {
 		ServerWebExchange exchange = webFilterExchange.getExchange();
 		SecurityContextImpl securityContext = new SecurityContextImpl();

+ 0 - 52
web/src/test/java/org/springframework/security/web/server/authentication/AuthenticationWebFilterTests.java

@@ -25,10 +25,8 @@ import org.mockito.junit.jupiter.MockitoExtension;
 import reactor.core.publisher.Mono;
 
 import org.springframework.security.authentication.BadCredentialsException;
-import org.springframework.security.authentication.NonBuildableAuthenticationToken;
 import org.springframework.security.authentication.ReactiveAuthenticationManager;
 import org.springframework.security.authentication.ReactiveAuthenticationManagerResolver;
-import org.springframework.security.authentication.SecurityAssertions;
 import org.springframework.security.authentication.TestingAuthenticationToken;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.GrantedAuthority;
@@ -178,31 +176,6 @@ public class AuthenticationWebFilterTests {
 		assertThat(result.getResponseCookies()).isEmpty();
 	}
 
-	@Test
-	public void filterWhenAuthenticatedThenCombinesAuthorities() {
-		String ROLE_EXISTING = "ROLE_EXISTING";
-		TestingAuthenticationToken existingAuthn = new TestingAuthenticationToken("username", "password",
-				ROLE_EXISTING);
-		given(this.authenticationManager.authenticate(any()))
-			.willReturn(Mono.just(new TestingAuthenticationToken("user", "password", "TEST")));
-		given(this.securityContextRepository.save(any(), any())).willReturn(Mono.empty());
-		this.filter = new AuthenticationWebFilter(this.authenticationManager);
-		this.filter.setSecurityContextRepository(this.securityContextRepository);
-		WebTestClient client = WebTestClientBuilder.bindToWebFilters(new RunAsWebFilter(existingAuthn), this.filter)
-			.build();
-		client.get()
-			.uri("/")
-			.headers((headers) -> headers.setBasicAuth("test", "this"))
-			.exchange()
-			.expectStatus()
-			.isOk();
-		ArgumentCaptor<SecurityContext> context = ArgumentCaptor.forClass(SecurityContext.class);
-		verify(this.securityContextRepository).save(any(), context.capture());
-		Authentication authentication = context.getValue().getAuthentication();
-		assertThat(authentication.getAuthorities()).extracting(GrantedAuthority::getAuthority)
-			.containsExactlyInAnyOrder(ROLE_EXISTING, "TEST");
-	}
-
 	/**
 	 * This is critical to avoid adding duplicate GrantedAuthority instances with the
 	 * same' authority when the issuedAt is too old and a new instance is requested.
@@ -232,31 +205,6 @@ public class AuthenticationWebFilterTests {
 			.containsExactly(DefaultEqualsGrantedAuthority.AUTHORITY);
 	}
 
-	@Test
-	void doFilterWhenNotOverridingToBuilderThenDoesNotMergeAuthorities() throws Exception {
-		TestingAuthenticationToken existingAuthn = new TestingAuthenticationToken("username", "password", "FACTORONE");
-		given(this.authenticationManager.authenticate(any()))
-			.willReturn(Mono.just(new NonBuildableAuthenticationToken("user", "password", "FACTORTWO")));
-		given(this.securityContextRepository.save(any(), any())).willReturn(Mono.empty());
-		this.filter = new AuthenticationWebFilter(this.authenticationManager);
-		this.filter.setSecurityContextRepository(this.securityContextRepository);
-		WebTestClient client = WebTestClientBuilder.bindToWebFilters(new RunAsWebFilter(existingAuthn), this.filter)
-			.build();
-		client.get()
-			.uri("/")
-			.headers((headers) -> headers.setBasicAuth("test", "this"))
-			.exchange()
-			.expectStatus()
-			.isOk();
-		ArgumentCaptor<SecurityContext> context = ArgumentCaptor.forClass(SecurityContext.class);
-		verify(this.securityContextRepository).save(any(), context.capture());
-		Authentication authentication = context.getValue().getAuthentication();
-		SecurityAssertions.assertThat(authentication)
-			.authorities()
-			.extracting(GrantedAuthority::getAuthority)
-			.containsExactly("FACTORTWO");
-	}
-
 	@Test
 	public void filterWhenAuthenticationManagerResolverDefaultsAndAuthenticationFailThenUnauthorized() {
 		given(this.authenticationManager.authenticate(any()))