Move Authority Propagation Into Filters
Given that the filters are the level at which the SecurityContextHolder is consulted, this commit moves the operation that ProviderManager was doing into each authentication filter. Issue gh-17862
This commit is contained in:
+2
@@ -16,6 +16,7 @@
|
||||
|
||||
package org.springframework.security.authentication;
|
||||
|
||||
import java.io.Serial;
|
||||
import java.security.Principal;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Collection;
|
||||
@@ -43,6 +44,7 @@ import org.springframework.util.Assert;
|
||||
*/
|
||||
public abstract class AbstractAuthenticationToken implements Authentication, CredentialsContainer {
|
||||
|
||||
@Serial
|
||||
private static final long serialVersionUID = -3194696462184782834L;
|
||||
|
||||
private final Collection<GrantedAuthority> authorities;
|
||||
|
||||
-15
@@ -27,7 +27,6 @@ import reactor.core.publisher.Mono;
|
||||
|
||||
import org.springframework.security.core.Authentication;
|
||||
import org.springframework.security.core.AuthenticationException;
|
||||
import org.springframework.security.core.context.ReactiveSecurityContextHolder;
|
||||
import org.springframework.util.Assert;
|
||||
|
||||
/**
|
||||
@@ -58,20 +57,6 @@ public class DelegatingReactiveAuthenticationManager implements ReactiveAuthenti
|
||||
|
||||
@Override
|
||||
public Mono<Authentication> authenticate(Authentication authentication) {
|
||||
return ReactiveSecurityContextHolder.getContext().flatMap((context) -> {
|
||||
Mono<Authentication> result = doAuthenticate(authentication);
|
||||
Authentication current = context.getAuthentication();
|
||||
if (current == null) {
|
||||
return result;
|
||||
}
|
||||
if (!current.isAuthenticated()) {
|
||||
return result;
|
||||
}
|
||||
return doAuthenticate(current).map((r) -> r.toBuilder().apply(current).build());
|
||||
}).switchIfEmpty(doAuthenticate(authentication));
|
||||
}
|
||||
|
||||
private Mono<Authentication> doAuthenticate(Authentication authentication) {
|
||||
Flux<ReactiveAuthenticationManager> result = Flux.fromIterable(this.delegates);
|
||||
Function<ReactiveAuthenticationManager, Mono<Authentication>> logging = (m) -> m.authenticate(authentication)
|
||||
.doOnError(AuthenticationException.class, (ex) -> ex.setAuthenticationRequest(authentication))
|
||||
|
||||
+8
-29
@@ -33,8 +33,6 @@ import org.springframework.security.core.Authentication;
|
||||
import org.springframework.security.core.AuthenticationException;
|
||||
import org.springframework.security.core.CredentialsContainer;
|
||||
import org.springframework.security.core.SpringSecurityMessageSource;
|
||||
import org.springframework.security.core.context.SecurityContextHolder;
|
||||
import org.springframework.security.core.context.SecurityContextHolderStrategy;
|
||||
import org.springframework.util.Assert;
|
||||
import org.springframework.util.CollectionUtils;
|
||||
|
||||
@@ -94,9 +92,6 @@ public class ProviderManager implements AuthenticationManager, MessageSourceAwar
|
||||
|
||||
private static final Log logger = LogFactory.getLog(ProviderManager.class);
|
||||
|
||||
private SecurityContextHolderStrategy securityContextHolderStrategy = SecurityContextHolder
|
||||
.getContextHolderStrategy();
|
||||
|
||||
private AuthenticationEventPublisher eventPublisher = new NullEventPublisher();
|
||||
|
||||
private List<AuthenticationProvider> providers = Collections.emptyList();
|
||||
@@ -187,7 +182,7 @@ public class ProviderManager implements AuthenticationManager, MessageSourceAwar
|
||||
try {
|
||||
result = provider.authenticate(authentication);
|
||||
if (result != null) {
|
||||
copyDetails(authentication, result);
|
||||
result = copyDetails(authentication, result);
|
||||
break;
|
||||
}
|
||||
}
|
||||
@@ -214,7 +209,6 @@ public class ProviderManager implements AuthenticationManager, MessageSourceAwar
|
||||
lastException = ex;
|
||||
}
|
||||
}
|
||||
result = applyPreviousAuthentication(result);
|
||||
if (result == null && this.parent != null) {
|
||||
// Allow the parent to try.
|
||||
try {
|
||||
@@ -271,20 +265,6 @@ public class ProviderManager implements AuthenticationManager, MessageSourceAwar
|
||||
throw lastException;
|
||||
}
|
||||
|
||||
private @Nullable Authentication applyPreviousAuthentication(@Nullable Authentication result) {
|
||||
if (result == null) {
|
||||
return null;
|
||||
}
|
||||
Authentication current = this.securityContextHolderStrategy.getContext().getAuthentication();
|
||||
if (current == null) {
|
||||
return result;
|
||||
}
|
||||
if (!current.isAuthenticated()) {
|
||||
return result;
|
||||
}
|
||||
return result.toBuilder().apply(current).build();
|
||||
}
|
||||
|
||||
@SuppressWarnings("deprecation")
|
||||
private void prepareException(AuthenticationException ex, Authentication auth) {
|
||||
ex.setAuthenticationRequest(auth);
|
||||
@@ -297,21 +277,20 @@ public class ProviderManager implements AuthenticationManager, MessageSourceAwar
|
||||
* @param source source authentication
|
||||
* @param dest the destination authentication object
|
||||
*/
|
||||
private void copyDetails(Authentication source, Authentication dest) {
|
||||
if ((dest instanceof AbstractAuthenticationToken token) && (dest.getDetails() == null)) {
|
||||
token.setDetails(source.getDetails());
|
||||
private Authentication copyDetails(Authentication source, Authentication dest) {
|
||||
if (source.getDetails() == null) {
|
||||
return dest;
|
||||
}
|
||||
if (dest.getDetails() != null) {
|
||||
return dest;
|
||||
}
|
||||
return dest.toBuilder().details(source.getDetails()).build();
|
||||
}
|
||||
|
||||
public List<AuthenticationProvider> getProviders() {
|
||||
return this.providers;
|
||||
}
|
||||
|
||||
public void setSecurityContextHolderStrategy(SecurityContextHolderStrategy securityContextHolderStrategy) {
|
||||
Assert.notNull(securityContextHolderStrategy, "securityContextHolderStrategy cannot be null");
|
||||
this.securityContextHolderStrategy = securityContextHolderStrategy;
|
||||
}
|
||||
|
||||
@Override
|
||||
public void setMessageSource(MessageSource messageSource) {
|
||||
this.messages = new MessageSourceAccessor(messageSource);
|
||||
|
||||
-21
@@ -27,13 +27,10 @@ import reactor.test.StepVerifier;
|
||||
|
||||
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 static org.assertj.core.api.Assertions.assertThat;
|
||||
import static org.mockito.ArgumentMatchers.any;
|
||||
import static org.mockito.BDDMockito.given;
|
||||
import static org.mockito.Mockito.mock;
|
||||
|
||||
/**
|
||||
* @author Rob Winch
|
||||
@@ -121,24 +118,6 @@ public class DelegatingReactiveAuthenticationManagerTests {
|
||||
assertThat(expected.getAuthenticationRequest()).isEqualTo(this.authentication);
|
||||
}
|
||||
|
||||
@Test
|
||||
void authenticateWhenPreviousAuthenticationThenApplies() {
|
||||
Authentication factorOne = new TestingAuthenticationToken("user", "pass", "FACTOR_ONE");
|
||||
Authentication factorTwo = new TestingAuthenticationToken("user", "pass", "FACTOR_TWO");
|
||||
ReactiveAuthenticationManager provider = mock(ReactiveAuthenticationManager.class);
|
||||
given(provider.authenticate(any())).willReturn(Mono.just(factorTwo));
|
||||
ReactiveAuthenticationManager manager = new DelegatingReactiveAuthenticationManager(provider);
|
||||
Authentication request = new TestingAuthenticationToken("user", "password");
|
||||
StepVerifier
|
||||
.create(manager.authenticate(request)
|
||||
.flatMapIterable(Authentication::getAuthorities)
|
||||
.map(GrantedAuthority::getAuthority)
|
||||
.contextWrite(ReactiveSecurityContextHolder.withAuthentication(factorOne)))
|
||||
.expectNext("FACTOR_TWO")
|
||||
.expectNext("FACTOR_ONE")
|
||||
.verifyComplete();
|
||||
}
|
||||
|
||||
private DelegatingReactiveAuthenticationManager managerWithContinueOnError() {
|
||||
DelegatingReactiveAuthenticationManager manager = new DelegatingReactiveAuthenticationManager(this.delegate1,
|
||||
this.delegate2);
|
||||
|
||||
+11
-28
@@ -18,17 +18,15 @@ package org.springframework.security.authentication;
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.Arrays;
|
||||
import java.util.Collection;
|
||||
import java.util.List;
|
||||
import java.util.Set;
|
||||
|
||||
import org.junit.jupiter.api.Test;
|
||||
|
||||
import org.springframework.context.MessageSource;
|
||||
import org.springframework.security.core.Authentication;
|
||||
import org.springframework.security.core.AuthenticationException;
|
||||
import org.springframework.security.core.authority.AuthorityUtils;
|
||||
import org.springframework.security.core.context.SecurityContextHolderStrategy;
|
||||
import org.springframework.security.core.context.SecurityContextImpl;
|
||||
import org.springframework.security.core.GrantedAuthority;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
|
||||
@@ -51,7 +49,7 @@ public class ProviderManagerTests {
|
||||
|
||||
@Test
|
||||
void authenticationFailsWithUnsupportedToken() {
|
||||
Authentication token = new AbstractAuthenticationToken(null) {
|
||||
Authentication token = new AbstractAuthenticationToken((Collection<? extends GrantedAuthority>) null) {
|
||||
@Override
|
||||
public Object getCredentials() {
|
||||
return "";
|
||||
@@ -82,24 +80,24 @@ public class ProviderManagerTests {
|
||||
|
||||
@Test
|
||||
void authenticationSucceedsWithSupportedTokenAndReturnsExpectedObject() {
|
||||
Authentication a = mock(Authentication.class);
|
||||
Authentication a = new TestingAuthenticationToken("user", "pass", "FACTOR");
|
||||
ProviderManager mgr = new ProviderManager(createProviderWhichReturns(a));
|
||||
AuthenticationEventPublisher publisher = mock(AuthenticationEventPublisher.class);
|
||||
mgr.setAuthenticationEventPublisher(publisher);
|
||||
Authentication result = mgr.authenticate(a);
|
||||
assertThat(result).isEqualTo(a);
|
||||
assertThat(result.getPrincipal()).isEqualTo(a.getPrincipal());
|
||||
verify(publisher).publishAuthenticationSuccess(result);
|
||||
}
|
||||
|
||||
@Test
|
||||
void authenticationSucceedsWhenFirstProviderReturnsNullButSecondAuthenticates() {
|
||||
Authentication a = mock(Authentication.class);
|
||||
Authentication a = new TestingAuthenticationToken("user", "pass", "FACTOR");
|
||||
ProviderManager mgr = new ProviderManager(
|
||||
Arrays.asList(createProviderWhichReturns(null), createProviderWhichReturns(a)));
|
||||
AuthenticationEventPublisher publisher = mock(AuthenticationEventPublisher.class);
|
||||
mgr.setAuthenticationEventPublisher(publisher);
|
||||
Authentication result = mgr.authenticate(a);
|
||||
assertThat(result).isSameAs(a);
|
||||
assertThat(result.getPrincipal()).isEqualTo(a.getPrincipal());
|
||||
verify(publisher).publishAuthenticationSuccess(result);
|
||||
}
|
||||
|
||||
@@ -166,11 +164,12 @@ public class ProviderManagerTests {
|
||||
|
||||
@Test
|
||||
void authenticationExceptionIsIgnoredIfLaterProviderAuthenticates() {
|
||||
Authentication authReq = mock(Authentication.class);
|
||||
Authentication result = new TestingAuthenticationToken("user", "pass", "FACTOR");
|
||||
ProviderManager mgr = new ProviderManager(
|
||||
createProviderWhichThrows(new BadCredentialsException("", new Throwable())),
|
||||
createProviderWhichReturns(authReq));
|
||||
assertThat(mgr.authenticate(mock(Authentication.class))).isSameAs(authReq);
|
||||
createProviderWhichReturns(result));
|
||||
Authentication request = new TestingAuthenticationToken("user", "pass");
|
||||
assertThat(mgr.authenticate(request).getPrincipal()).isEqualTo(result.getPrincipal());
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -314,22 +313,6 @@ public class ProviderManagerTests {
|
||||
verifyNoMoreInteractions(publisher); // Child should not publish (duplicate event)
|
||||
}
|
||||
|
||||
@Test
|
||||
void authenticateWhenPreviousAuthenticationThenApplies() {
|
||||
Authentication factorOne = new TestingAuthenticationToken("user", "pass", "FACTOR_ONE");
|
||||
Authentication factorTwo = new TestingAuthenticationToken("user", "pass", "FACTOR_TWO");
|
||||
SecurityContextHolderStrategy securityContextHolderStrategy = mock(SecurityContextHolderStrategy.class);
|
||||
given(securityContextHolderStrategy.getContext()).willReturn(new SecurityContextImpl(factorOne));
|
||||
AuthenticationProvider provider = mock(AuthenticationProvider.class);
|
||||
given(provider.authenticate(any())).willReturn(factorTwo);
|
||||
given(provider.supports(any())).willReturn(true);
|
||||
ProviderManager manager = new ProviderManager(provider);
|
||||
manager.setSecurityContextHolderStrategy(securityContextHolderStrategy);
|
||||
Authentication request = new TestingAuthenticationToken("user", "password");
|
||||
Set<String> authorities = AuthorityUtils.authorityListToSet(manager.authenticate(request).getAuthorities());
|
||||
assertThat(authorities).containsExactlyInAnyOrder("FACTOR_ONE", "FACTOR_TWO");
|
||||
}
|
||||
|
||||
private AuthenticationProvider createProviderWhichThrows(final AuthenticationException ex) {
|
||||
AuthenticationProvider provider = mock(AuthenticationProvider.class);
|
||||
given(provider.supports(any(Class.class))).willReturn(true);
|
||||
|
||||
Reference in New Issue
Block a user