Skip to content

Include AuthenticationRequest in AuthenticationException #16505

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Mar 22, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2024 the original author or authors.
* Copyright 2002-2025 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -26,6 +26,7 @@
import reactor.core.publisher.Mono;

import org.springframework.security.core.Authentication;
import org.springframework.security.core.AuthenticationException;
import org.springframework.util.Assert;

/**
Expand Down Expand Up @@ -58,6 +59,7 @@ public DelegatingReactiveAuthenticationManager(List<ReactiveAuthenticationManage
public Mono<Authentication> authenticate(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))
.doOnError(this.logger::debug);

return ((this.continueOnError) ? result.concatMapDelayError(logging) : result.concatMap(logging)).next();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2020 the original author or authors.
* Copyright 2002-2025 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -202,6 +202,7 @@ public Authentication authenticate(Authentication authentication) throws Authent
throw ex;
}
catch (AuthenticationException ex) {
ex.setAuthenticationRequest(authentication);
logger.debug(LogMessage.format("Authentication failed with provider %s since %s",
provider.getClass().getSimpleName(), ex.getMessage()));
lastException = ex;
Expand Down Expand Up @@ -265,6 +266,7 @@ public Authentication authenticate(Authentication authentication) throws Authent

@SuppressWarnings("deprecation")
private void prepareException(AuthenticationException ex, Authentication auth) {
ex.setAuthenticationRequest(auth);
this.eventPublisher.publishAuthenticationFailure(ex, auth);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@

import java.io.Serial;

import org.springframework.util.Assert;

/**
* Abstract superclass for all exceptions related to an {@link Authentication} object
* being invalid for whatever reason.
Expand All @@ -29,6 +31,8 @@ public abstract class AuthenticationException extends RuntimeException {
@Serial
private static final long serialVersionUID = 2018827803361503060L;

private Authentication authenticationRequest;

/**
* Constructs an {@code AuthenticationException} with the specified message and root
* cause.
Expand All @@ -48,4 +52,31 @@ public AuthenticationException(String msg) {
super(msg);
}

/**
* Get the {@link Authentication} object representing the failed authentication
* attempt.
* <p>
* This field captures the authentication request that was attempted but ultimately
* failed, providing critical information for diagnosing the failure and facilitating
* debugging
* @since 6.5
*/
public Authentication getAuthenticationRequest() {
return this.authenticationRequest;
}

/**
* Set the {@link Authentication} object representing the failed authentication
* attempt.
* <p>
* The provided {@code authenticationRequest} should not be null
* @param authenticationRequest the authentication request associated with the failed
* authentication attempt
* @since 6.5
*/
public void setAuthenticationRequest(Authentication authenticationRequest) {
Assert.notNull(authenticationRequest, "authenticationRequest cannot be null");
this.authenticationRequest = authenticationRequest;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
* @see CoreJackson2Module
*/
@JsonTypeInfo(use = JsonTypeInfo.Id.CLASS, include = JsonTypeInfo.As.PROPERTY)
@JsonIgnoreProperties(ignoreUnknown = true, value = { "cause", "stackTrace" })
@JsonIgnoreProperties(ignoreUnknown = true, value = { "cause", "stackTrace", "authenticationRequest" })
class BadCredentialsExceptionMixin {

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2024 the original author or authors.
* Copyright 2002-2025 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -26,6 +26,7 @@
import reactor.test.StepVerifier;

import org.springframework.security.core.Authentication;
import org.springframework.security.core.AuthenticationException;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.any;
Expand Down Expand Up @@ -108,6 +109,15 @@ public void authenticateWhenContinueOnErrorAndDelegate1NotEmptyThenReturnsNotEmp
assertThat(manager.authenticate(this.authentication).block()).isEqualTo(this.authentication);
}

@Test
void whenAccountStatusExceptionThenAuthenticationRequestIsIncluded() {
AuthenticationException expected = new LockedException("");
given(this.delegate1.authenticate(any())).willReturn(Mono.error(expected));
ReactiveAuthenticationManager manager = new DelegatingReactiveAuthenticationManager(this.delegate1);
StepVerifier.create(manager.authenticate(this.authentication)).expectError(LockedException.class).verify();
assertThat(expected.getAuthenticationRequest()).isEqualTo(this.authentication);
}

private DelegatingReactiveAuthenticationManager managerWithContinueOnError() {
DelegatingReactiveAuthenticationManager manager = new DelegatingReactiveAuthenticationManager(this.delegate1,
this.delegate2);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;

import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -47,7 +46,7 @@
public class ProviderManagerTests {

@Test
public void authenticationFailsWithUnsupportedToken() {
void authenticationFailsWithUnsupportedToken() {
Authentication token = new AbstractAuthenticationToken(null) {
@Override
public Object getCredentials() {
Expand All @@ -65,7 +64,7 @@ public Object getPrincipal() {
}

@Test
public void credentialsAreClearedByDefault() {
void credentialsAreClearedByDefault() {
UsernamePasswordAuthenticationToken token = UsernamePasswordAuthenticationToken.unauthenticated("Test",
"Password");
ProviderManager mgr = makeProviderManager();
Expand All @@ -78,8 +77,8 @@ public void credentialsAreClearedByDefault() {
}

@Test
public void authenticationSucceedsWithSupportedTokenAndReturnsExpectedObject() {
final Authentication a = mock(Authentication.class);
void authenticationSucceedsWithSupportedTokenAndReturnsExpectedObject() {
Authentication a = mock(Authentication.class);
ProviderManager mgr = new ProviderManager(createProviderWhichReturns(a));
AuthenticationEventPublisher publisher = mock(AuthenticationEventPublisher.class);
mgr.setAuthenticationEventPublisher(publisher);
Expand All @@ -89,8 +88,8 @@ public void authenticationSucceedsWithSupportedTokenAndReturnsExpectedObject() {
}

@Test
public void authenticationSucceedsWhenFirstProviderReturnsNullButSecondAuthenticates() {
final Authentication a = mock(Authentication.class);
void authenticationSucceedsWhenFirstProviderReturnsNullButSecondAuthenticates() {
Authentication a = mock(Authentication.class);
ProviderManager mgr = new ProviderManager(
Arrays.asList(createProviderWhichReturns(null), createProviderWhichReturns(a)));
AuthenticationEventPublisher publisher = mock(AuthenticationEventPublisher.class);
Expand All @@ -101,24 +100,24 @@ public void authenticationSucceedsWhenFirstProviderReturnsNullButSecondAuthentic
}

@Test
public void testStartupFailsIfProvidersNotSetAsList() {
void testStartupFailsIfProvidersNotSetAsList() {
assertThatIllegalArgumentException().isThrownBy(() -> new ProviderManager((List<AuthenticationProvider>) null));
}

@Test
public void testStartupFailsIfProvidersNotSetAsVarargs() {
void testStartupFailsIfProvidersNotSetAsVarargs() {
assertThatIllegalArgumentException().isThrownBy(() -> new ProviderManager((AuthenticationProvider) null));
}

@Test
public void testStartupFailsIfProvidersContainNullElement() {
void testStartupFailsIfProvidersContainNullElement() {
assertThatIllegalArgumentException()
.isThrownBy(() -> new ProviderManager(Arrays.asList(mock(AuthenticationProvider.class), null)));
}

// gh-8689
@Test
public void constructorWhenUsingListOfThenNoException() {
void constructorWhenUsingListOfThenNoException() {
List<AuthenticationProvider> providers = spy(ArrayList.class);
// List.of(null) in JDK 9 throws a NullPointerException
given(providers.contains(eq(null))).willThrow(NullPointerException.class);
Expand All @@ -127,7 +126,7 @@ public void constructorWhenUsingListOfThenNoException() {
}

@Test
public void detailsAreNotSetOnAuthenticationTokenIfAlreadySetByProvider() {
void detailsAreNotSetOnAuthenticationTokenIfAlreadySetByProvider() {
Object requestDetails = "(Request Details)";
final Object resultDetails = "(Result Details)";
// A provider which sets the details object
Expand All @@ -151,7 +150,7 @@ public boolean supports(Class<?> authentication) {
}

@Test
public void detailsAreSetOnAuthenticationTokenIfNotAlreadySetByProvider() {
void detailsAreSetOnAuthenticationTokenIfNotAlreadySetByProvider() {
Object details = new Object();
ProviderManager authMgr = makeProviderManager();
TestingAuthenticationToken request = createAuthenticationToken();
Expand All @@ -162,16 +161,16 @@ public void detailsAreSetOnAuthenticationTokenIfNotAlreadySetByProvider() {
}

@Test
public void authenticationExceptionIsIgnoredIfLaterProviderAuthenticates() {
final Authentication authReq = mock(Authentication.class);
void authenticationExceptionIsIgnoredIfLaterProviderAuthenticates() {
Authentication authReq = mock(Authentication.class);
ProviderManager mgr = new ProviderManager(
createProviderWhichThrows(new BadCredentialsException("", new Throwable())),
createProviderWhichReturns(authReq));
assertThat(mgr.authenticate(mock(Authentication.class))).isSameAs(authReq);
}

@Test
public void authenticationExceptionIsRethrownIfNoLaterProviderAuthenticates() {
void authenticationExceptionIsRethrownIfNoLaterProviderAuthenticates() {
ProviderManager mgr = new ProviderManager(Arrays
.asList(createProviderWhichThrows(new BadCredentialsException("")), createProviderWhichReturns(null)));
assertThatExceptionOfType(BadCredentialsException.class)
Expand All @@ -180,7 +179,7 @@ public void authenticationExceptionIsRethrownIfNoLaterProviderAuthenticates() {

// SEC-546
@Test
public void accountStatusExceptionPreventsCallsToSubsequentProviders() {
void accountStatusExceptionPreventsCallsToSubsequentProviders() {
AuthenticationProvider iThrowAccountStatusException = createProviderWhichThrows(new AccountStatusException("") {
});
AuthenticationProvider otherProvider = mock(AuthenticationProvider.class);
Expand All @@ -191,48 +190,47 @@ public void accountStatusExceptionPreventsCallsToSubsequentProviders() {
}

@Test
public void parentAuthenticationIsUsedIfProvidersDontAuthenticate() {
void parentAuthenticationIsUsedIfProvidersDontAuthenticate() {
AuthenticationManager parent = mock(AuthenticationManager.class);
Authentication authReq = mock(Authentication.class);
given(parent.authenticate(authReq)).willReturn(authReq);
ProviderManager mgr = new ProviderManager(Collections.singletonList(mock(AuthenticationProvider.class)),
parent);
ProviderManager mgr = new ProviderManager(List.of(mock(AuthenticationProvider.class)), parent);
assertThat(mgr.authenticate(authReq)).isSameAs(authReq);
}

@Test
public void parentIsNotCalledIfAccountStatusExceptionIsThrown() {
void parentIsNotCalledIfAccountStatusExceptionIsThrown() {
AuthenticationProvider iThrowAccountStatusException = createProviderWhichThrows(
new AccountStatusException("", new Throwable()) {
});
AuthenticationManager parent = mock(AuthenticationManager.class);
ProviderManager mgr = new ProviderManager(Collections.singletonList(iThrowAccountStatusException), parent);
ProviderManager mgr = new ProviderManager(List.of(iThrowAccountStatusException), parent);
assertThatExceptionOfType(AccountStatusException.class)
.isThrownBy(() -> mgr.authenticate(mock(Authentication.class)));
verifyNoInteractions(parent);
}

@Test
public void providerNotFoundFromParentIsIgnored() {
void providerNotFoundFromParentIsIgnored() {
final Authentication authReq = mock(Authentication.class);
AuthenticationEventPublisher publisher = mock(AuthenticationEventPublisher.class);
AuthenticationManager parent = mock(AuthenticationManager.class);
given(parent.authenticate(authReq)).willThrow(new ProviderNotFoundException(""));
// Set a provider that throws an exception - this is the exception we expect to be
// propagated
ProviderManager mgr = new ProviderManager(
Collections.singletonList(createProviderWhichThrows(new BadCredentialsException(""))), parent);
ProviderManager mgr = new ProviderManager(List.of(createProviderWhichThrows(new BadCredentialsException(""))),
parent);
mgr.setAuthenticationEventPublisher(publisher);
assertThatExceptionOfType(BadCredentialsException.class).isThrownBy(() -> mgr.authenticate(authReq))
.satisfies((ex) -> verify(publisher).publishAuthenticationFailure(ex, authReq));
}

@Test
public void authenticationExceptionFromParentOverridesPreviousOnes() {
void authenticationExceptionFromParentOverridesPreviousOnes() {
AuthenticationManager parent = mock(AuthenticationManager.class);
ProviderManager mgr = new ProviderManager(
Collections.singletonList(createProviderWhichThrows(new BadCredentialsException(""))), parent);
final Authentication authReq = mock(Authentication.class);
ProviderManager mgr = new ProviderManager(List.of(createProviderWhichThrows(new BadCredentialsException(""))),
parent);
Authentication authReq = mock(Authentication.class);
AuthenticationEventPublisher publisher = mock(AuthenticationEventPublisher.class);
mgr.setAuthenticationEventPublisher(publisher);
// Set a provider that throws an exception - this is the exception we expect to be
Expand All @@ -244,21 +242,48 @@ public void authenticationExceptionFromParentOverridesPreviousOnes() {
}

@Test
public void statusExceptionIsPublished() {
void statusExceptionIsPublished() {
AuthenticationManager parent = mock(AuthenticationManager.class);
final LockedException expected = new LockedException("");
ProviderManager mgr = new ProviderManager(Collections.singletonList(createProviderWhichThrows(expected)),
parent);
final Authentication authReq = mock(Authentication.class);
LockedException expected = new LockedException("");
ProviderManager mgr = new ProviderManager(List.of(createProviderWhichThrows(expected)), parent);
Authentication authReq = mock(Authentication.class);
AuthenticationEventPublisher publisher = mock(AuthenticationEventPublisher.class);
mgr.setAuthenticationEventPublisher(publisher);
assertThatExceptionOfType(LockedException.class).isThrownBy(() -> mgr.authenticate(authReq));
verify(publisher).publishAuthenticationFailure(expected, authReq);
}

@Test
void whenAccountStatusExceptionThenAuthenticationRequestIsIncluded() {
AuthenticationException expected = new LockedException("");
ProviderManager mgr = new ProviderManager(createProviderWhichThrows(expected));
Authentication authReq = mock(Authentication.class);
assertThatExceptionOfType(LockedException.class).isThrownBy(() -> mgr.authenticate(authReq));
assertThat(expected.getAuthenticationRequest()).isEqualTo(authReq);
}

@Test
void whenInternalServiceAuthenticationExceptionThenAuthenticationRequestIsIncluded() {
AuthenticationException expected = new InternalAuthenticationServiceException("");
ProviderManager mgr = new ProviderManager(createProviderWhichThrows(expected));
Authentication authReq = mock(Authentication.class);
assertThatExceptionOfType(InternalAuthenticationServiceException.class)
.isThrownBy(() -> mgr.authenticate(authReq));
assertThat(expected.getAuthenticationRequest()).isEqualTo(authReq);
}

@Test
void whenAuthenticationExceptionThenAuthenticationRequestIsIncluded() {
AuthenticationException expected = new BadCredentialsException("");
ProviderManager mgr = new ProviderManager(createProviderWhichThrows(expected));
Authentication authReq = mock(Authentication.class);
assertThatExceptionOfType(BadCredentialsException.class).isThrownBy(() -> mgr.authenticate(authReq));
assertThat(expected.getAuthenticationRequest()).isEqualTo(authReq);
}

// SEC-2367
@Test
public void providerThrowsInternalAuthenticationServiceException() {
void providerThrowsInternalAuthenticationServiceException() {
InternalAuthenticationServiceException expected = new InternalAuthenticationServiceException("Expected");
ProviderManager mgr = new ProviderManager(Arrays.asList(createProviderWhichThrows(expected),
createProviderWhichThrows(new BadCredentialsException("Oops"))), null);
Expand All @@ -269,15 +294,15 @@ public void providerThrowsInternalAuthenticationServiceException() {

// gh-6281
@Test
public void authenticateWhenFailsInParentAndPublishesThenChildDoesNotPublish() {
void authenticateWhenFailsInParentAndPublishesThenChildDoesNotPublish() {
BadCredentialsException badCredentialsExParent = new BadCredentialsException("Bad Credentials in parent");
ProviderManager parentMgr = new ProviderManager(createProviderWhichThrows(badCredentialsExParent));
ProviderManager childMgr = new ProviderManager(Collections.singletonList(
createProviderWhichThrows(new BadCredentialsException("Bad Credentials in child"))), parentMgr);
ProviderManager childMgr = new ProviderManager(
List.of(createProviderWhichThrows(new BadCredentialsException("Bad Credentials in child"))), parentMgr);
AuthenticationEventPublisher publisher = mock(AuthenticationEventPublisher.class);
parentMgr.setAuthenticationEventPublisher(publisher);
childMgr.setAuthenticationEventPublisher(publisher);
final Authentication authReq = mock(Authentication.class);
Authentication authReq = mock(Authentication.class);
assertThatExceptionOfType(BadCredentialsException.class).isThrownBy(() -> childMgr.authenticate(authReq))
.isSameAs(badCredentialsExParent);
verify(publisher).publishAuthenticationFailure(badCredentialsExParent, authReq); // Parent
Expand Down
Loading