From 864a9b2fb39ce9ad4dee7d3a1c338e6ebe175e1f Mon Sep 17 00:00:00 2001 From: Rob Winch <362503+rwinch@users.noreply.github.com> Date: Fri, 10 Oct 2025 10:51:04 -0500 Subject: [PATCH] Fix ProviderManager.copyDetails Changes Authentication Type Closes gh-18027 --- .../authentication/ProviderManager.java | 12 ++-- .../authentication/ProviderManagerTests.java | 59 +++++++++++++++++++ 2 files changed, 63 insertions(+), 8 deletions(-) diff --git a/core/src/main/java/org/springframework/security/authentication/ProviderManager.java b/core/src/main/java/org/springframework/security/authentication/ProviderManager.java index 7167943a33..d90bfe5bad 100644 --- a/core/src/main/java/org/springframework/security/authentication/ProviderManager.java +++ b/core/src/main/java/org/springframework/security/authentication/ProviderManager.java @@ -182,7 +182,7 @@ public class ProviderManager implements AuthenticationManager, MessageSourceAwar try { result = provider.authenticate(authentication); if (result != null) { - result = copyDetails(authentication, result); + copyDetails(authentication, result); break; } } @@ -277,14 +277,10 @@ public class ProviderManager implements AuthenticationManager, MessageSourceAwar * @param source source authentication * @param dest the destination authentication object */ - private Authentication copyDetails(Authentication source, Authentication dest) { - if (source.getDetails() == null) { - return dest; + private void copyDetails(Authentication source, Authentication dest) { + if ((dest instanceof AbstractAuthenticationToken token) && (dest.getDetails() == null)) { + token.setDetails(source.getDetails()); } - if (dest.getDetails() != null) { - return dest; - } - return dest.toBuilder().details(source.getDetails()).build(); } public List getProviders() { diff --git a/core/src/test/java/org/springframework/security/authentication/ProviderManagerTests.java b/core/src/test/java/org/springframework/security/authentication/ProviderManagerTests.java index 69fff2567c..4c66bf5bae 100644 --- a/core/src/test/java/org/springframework/security/authentication/ProviderManagerTests.java +++ b/core/src/test/java/org/springframework/security/authentication/ProviderManagerTests.java @@ -21,6 +21,7 @@ import java.util.Arrays; import java.util.Collection; import java.util.List; +import org.jspecify.annotations.Nullable; import org.junit.jupiter.api.Test; import org.springframework.context.MessageSource; @@ -162,6 +163,20 @@ public class ProviderManagerTests { assertThat(result.getDetails()).isSameAs(details); } + // gh-18027 + @Test + void authenticationIsSameWhenDetailsSetAndAuthenticationToBuilderIsDefault() { + Authentication customAuthentication = new DefaultToBuilderAuthentication(); + AuthenticationProvider provider = mock(AuthenticationProvider.class); + given(provider.supports(any())).willReturn(true); + given(provider.authenticate(any())).willReturn(customAuthentication); + TestingAuthenticationToken request = createAuthenticationToken(); + request.setDetails(new Object()); + ProviderManager authMgr = new ProviderManager(provider); + Authentication result = authMgr.authenticate(request); + assertThat(result).isSameAs(customAuthentication); + } + @Test void authenticationExceptionIsIgnoredIfLaterProviderAuthenticates() { Authentication result = new TestingAuthenticationToken("user", "pass", "FACTOR"); @@ -356,4 +371,48 @@ public class ProviderManagerTests { } + /** + * Represents a custom {@link Authentication} that does not override + * {@link #toBuilder()}. We should remain passive to previous versions of Spring + * Security and not change the {@link Authentication} type. + */ + private static final class DefaultToBuilderAuthentication implements Authentication { + + @Override + public Collection getAuthorities() { + return List.of(); + } + + @Override + public @Nullable Object getCredentials() { + return null; + } + + @Override + public @Nullable Object getDetails() { + return null; + } + + @Override + public @Nullable Object getPrincipal() { + return null; + } + + @Override + public boolean isAuthenticated() { + return false; + } + + @Override + public void setAuthenticated(boolean isAuthenticated) throws IllegalArgumentException { + + } + + @Override + public String getName() { + return ""; + } + + } + }