Skip to content

Commit 864a9b2

Browse files
committed
Fix ProviderManager.copyDetails Changes Authentication Type
Closes gh-18027
1 parent 1213dbe commit 864a9b2

File tree

2 files changed

+63
-8
lines changed

2 files changed

+63
-8
lines changed

core/src/main/java/org/springframework/security/authentication/ProviderManager.java

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ public Authentication authenticate(Authentication authentication) throws Authent
182182
try {
183183
result = provider.authenticate(authentication);
184184
if (result != null) {
185-
result = copyDetails(authentication, result);
185+
copyDetails(authentication, result);
186186
break;
187187
}
188188
}
@@ -277,14 +277,10 @@ private void prepareException(AuthenticationException ex, Authentication auth) {
277277
* @param source source authentication
278278
* @param dest the destination authentication object
279279
*/
280-
private Authentication copyDetails(Authentication source, Authentication dest) {
281-
if (source.getDetails() == null) {
282-
return dest;
280+
private void copyDetails(Authentication source, Authentication dest) {
281+
if ((dest instanceof AbstractAuthenticationToken token) && (dest.getDetails() == null)) {
282+
token.setDetails(source.getDetails());
283283
}
284-
if (dest.getDetails() != null) {
285-
return dest;
286-
}
287-
return dest.toBuilder().details(source.getDetails()).build();
288284
}
289285

290286
public List<AuthenticationProvider> getProviders() {

core/src/test/java/org/springframework/security/authentication/ProviderManagerTests.java

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import java.util.Collection;
2222
import java.util.List;
2323

24+
import org.jspecify.annotations.Nullable;
2425
import org.junit.jupiter.api.Test;
2526

2627
import org.springframework.context.MessageSource;
@@ -162,6 +163,20 @@ void detailsAreSetOnAuthenticationTokenIfNotAlreadySetByProvider() {
162163
assertThat(result.getDetails()).isSameAs(details);
163164
}
164165

166+
// gh-18027
167+
@Test
168+
void authenticationIsSameWhenDetailsSetAndAuthenticationToBuilderIsDefault() {
169+
Authentication customAuthentication = new DefaultToBuilderAuthentication();
170+
AuthenticationProvider provider = mock(AuthenticationProvider.class);
171+
given(provider.supports(any())).willReturn(true);
172+
given(provider.authenticate(any())).willReturn(customAuthentication);
173+
TestingAuthenticationToken request = createAuthenticationToken();
174+
request.setDetails(new Object());
175+
ProviderManager authMgr = new ProviderManager(provider);
176+
Authentication result = authMgr.authenticate(request);
177+
assertThat(result).isSameAs(customAuthentication);
178+
}
179+
165180
@Test
166181
void authenticationExceptionIsIgnoredIfLaterProviderAuthenticates() {
167182
Authentication result = new TestingAuthenticationToken("user", "pass", "FACTOR");
@@ -356,4 +371,48 @@ public boolean supports(Class<?> authentication) {
356371

357372
}
358373

374+
/**
375+
* Represents a custom {@link Authentication} that does not override
376+
* {@link #toBuilder()}. We should remain passive to previous versions of Spring
377+
* Security and not change the {@link Authentication} type.
378+
*/
379+
private static final class DefaultToBuilderAuthentication implements Authentication {
380+
381+
@Override
382+
public Collection<? extends GrantedAuthority> getAuthorities() {
383+
return List.of();
384+
}
385+
386+
@Override
387+
public @Nullable Object getCredentials() {
388+
return null;
389+
}
390+
391+
@Override
392+
public @Nullable Object getDetails() {
393+
return null;
394+
}
395+
396+
@Override
397+
public @Nullable Object getPrincipal() {
398+
return null;
399+
}
400+
401+
@Override
402+
public boolean isAuthenticated() {
403+
return false;
404+
}
405+
406+
@Override
407+
public void setAuthenticated(boolean isAuthenticated) throws IllegalArgumentException {
408+
409+
}
410+
411+
@Override
412+
public String getName() {
413+
return "";
414+
}
415+
416+
}
417+
359418
}

0 commit comments

Comments
 (0)