From 7ad4ebd07ad0c3c8a507cf68bb35308b6595042a Mon Sep 17 00:00:00 2001 From: ch4mpy Date: Mon, 26 Sep 2022 09:30:03 -1000 Subject: [PATCH] Allow authentication details to be set by converter Prevent JwtAuthenticationProvider from setting authentication details when jwtAuthenticationConverter returned an authentication instance with non null details. Closes gh-11822 --- .../JwtAuthenticationProvider.java | 7 +++- .../JwtAuthenticationProviderTests.java | 37 +++++++++++++++++-- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/authentication/JwtAuthenticationProvider.java b/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/authentication/JwtAuthenticationProvider.java index 000457b6f8e..cdca4ffc0cc 100644 --- a/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/authentication/JwtAuthenticationProvider.java +++ b/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/authentication/JwtAuthenticationProvider.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2022 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. @@ -56,6 +56,7 @@ * * @author Josh Cummings * @author Joe Grandja + * @author Jerome Wacongne ch4mp@c4-soft.com * @since 5.1 * @see AuthenticationProvider * @see JwtDecoder @@ -86,7 +87,9 @@ public Authentication authenticate(Authentication authentication) throws Authent BearerTokenAuthenticationToken bearer = (BearerTokenAuthenticationToken) authentication; Jwt jwt = getJwt(bearer); AbstractAuthenticationToken token = this.jwtAuthenticationConverter.convert(jwt); - token.setDetails(bearer.getDetails()); + if (token.getDetails() == null) { + token.setDetails(bearer.getDetails()); + } this.logger.debug("Authenticated token"); return token; } diff --git a/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/authentication/JwtAuthenticationProviderTests.java b/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/authentication/JwtAuthenticationProviderTests.java index 53457361380..b4438ba28fb 100644 --- a/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/authentication/JwtAuthenticationProviderTests.java +++ b/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/authentication/JwtAuthenticationProviderTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2022 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. @@ -25,6 +25,7 @@ import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.core.convert.converter.Converter; +import org.springframework.security.authentication.AbstractAuthenticationToken; import org.springframework.security.core.AuthenticationException; import org.springframework.security.oauth2.core.OAuth2AuthenticationException; import org.springframework.security.oauth2.jwt.BadJwtException; @@ -43,12 +44,13 @@ * Tests for {@link JwtAuthenticationProvider} * * @author Josh Cummings + * @author Jerome Wacongne ch4mp@c4-soft.com */ @ExtendWith(MockitoExtension.class) public class JwtAuthenticationProviderTests { @Mock - Converter jwtAuthenticationConverter; + Converter jwtAuthenticationConverter; @Mock JwtDecoder jwtDecoder; @@ -107,17 +109,46 @@ public void authenticateWhenDecoderFailsGenericallyThenThrowsGenericException() @Test public void authenticateWhenConverterReturnsAuthenticationThenProviderPropagatesIt() { + BearerTokenAuthenticationToken token = this.authentication(); + Jwt jwt = TestJwts.jwt().build(); + JwtAuthenticationToken authentication = new JwtAuthenticationToken(jwt); + given(this.jwtDecoder.decode(token.getToken())).willReturn(jwt); + given(this.jwtAuthenticationConverter.convert(jwt)).willReturn(authentication); + + assertThat(this.provider.authenticate(token)).isEqualTo(authentication); + } + + @Test + public void authenticateWhenConverterDoesNotSetAuthenticationDetailsThenProviderSetsItWithTokenDetails() { + BearerTokenAuthenticationToken token = this.authentication(); + Object details = mock(Object.class); + token.setDetails(details); + Jwt jwt = TestJwts.jwt().build(); + JwtAuthenticationToken authentication = new JwtAuthenticationToken(jwt); + given(this.jwtDecoder.decode(token.getToken())).willReturn(jwt); + given(this.jwtAuthenticationConverter.convert(jwt)).willReturn(authentication); + // @formatter:off + assertThat(this.provider.authenticate(token)) + .isEqualTo(authentication).hasFieldOrPropertyWithValue("details", + details); + // @formatter:on + } + + @Test + public void authenticateWhenConverterSetsAuthenticationDetailsThenProviderDoesNotOverwriteIt() { BearerTokenAuthenticationToken token = this.authentication(); Object details = mock(Object.class); token.setDetails(details); Jwt jwt = TestJwts.jwt().build(); JwtAuthenticationToken authentication = new JwtAuthenticationToken(jwt); + Object expectedDetails = "To be kept as is"; + authentication.setDetails(expectedDetails); given(this.jwtDecoder.decode(token.getToken())).willReturn(jwt); given(this.jwtAuthenticationConverter.convert(jwt)).willReturn(authentication); // @formatter:off assertThat(this.provider.authenticate(token)) .isEqualTo(authentication).hasFieldOrPropertyWithValue("details", - details); + expectedDetails); // @formatter:on }