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
spring-projects · Dec 13, 2022 · 7ad4ebd · 7ad4ebd
1 parent c2d0ea3
commit 7ad4ebd
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 5 deletions.
diff --git a/...ngframework/security/oauth2/server/resource/authentication/JwtAuthenticationProvider.java b/...ngframework/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&#64;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/...mework/security/oauth2/server/resource/authentication/JwtAuthenticationProviderTests.java b/...mework/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&#64;c4-soft.com
  */
 @ExtendWith(MockitoExtension.class)
 public class JwtAuthenticationProviderTests {
 
 	@Mock
-	Converter<Jwt, JwtAuthenticationToken> jwtAuthenticationConverter;
+	Converter<Jwt, AbstractAuthenticationToken> 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
 	}