From 793820acfa48edde0e85c8252882bee84fd9badd Mon Sep 17 00:00:00 2001 From: Josh Cummings <3627351+jzheaux@users.noreply.github.com> Date: Mon, 3 Nov 2025 13:31:30 -0700 Subject: [PATCH] Remove Authority Copying From Reactive We will re-address this when adding factors to ReactiveAuthenticationManager implementations. Issue gh-2603 --- .../AuthenticationWebFilter.java | 43 --------------- .../AuthenticationWebFilterTests.java | 52 ------------------- 2 files changed, 95 deletions(-) diff --git a/web/src/main/java/org/springframework/security/web/server/authentication/AuthenticationWebFilter.java b/web/src/main/java/org/springframework/security/web/server/authentication/AuthenticationWebFilter.java index 528ab64d54..3bbbdb108c 100644 --- a/web/src/main/java/org/springframework/security/web/server/authentication/AuthenticationWebFilter.java +++ b/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 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 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 onAuthenticationSuccess(Authentication authentication, WebFilterExchange webFilterExchange) { ServerWebExchange exchange = webFilterExchange.getExchange(); SecurityContextImpl securityContext = new SecurityContextImpl(); diff --git a/web/src/test/java/org/springframework/security/web/server/authentication/AuthenticationWebFilterTests.java b/web/src/test/java/org/springframework/security/web/server/authentication/AuthenticationWebFilterTests.java index 47d3b04f3c..7d33f04332 100644 --- a/web/src/test/java/org/springframework/security/web/server/authentication/AuthenticationWebFilterTests.java +++ b/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 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 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()))