From 2a867997e20f33dfbc3bcb5fe5f1bf750e4e3448 Mon Sep 17 00:00:00 2001 From: Joe Grandja Date: Mon, 14 Jan 2019 13:11:52 -0500 Subject: [PATCH] Polish gh-6415 --- ...thorizationCodeAuthenticationProvider.java | 1 + ...tionCodeReactiveAuthenticationManager.java | 1 + .../OidcIdTokenDecoderFactory.java | 22 ++++---- .../ReactiveOidcIdTokenDecoderFactory.java | 22 ++++---- .../OidcIdTokenDecoderFactoryTests.java | 51 ++++++++----------- ...eactiveOidcIdTokenDecoderFactoryTests.java | 49 +++++++----------- 6 files changed, 67 insertions(+), 79 deletions(-) diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/authentication/OidcAuthorizationCodeAuthenticationProvider.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/authentication/OidcAuthorizationCodeAuthenticationProvider.java index 8ed7f8a31f..1103a6aabc 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/authentication/OidcAuthorizationCodeAuthenticationProvider.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/authentication/OidcAuthorizationCodeAuthenticationProvider.java @@ -66,6 +66,7 @@ import java.util.Map; * @see OAuth2AccessTokenResponseClient * @see OidcUserService * @see OidcUser + * @see OidcIdTokenDecoderFactory * @see Section 3.1 Authorization Code Grant Flow * @see Section 3.1.3.1 Token Request * @see Section 3.1.3.3 Token Response diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/authentication/OidcAuthorizationCodeReactiveAuthenticationManager.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/authentication/OidcAuthorizationCodeReactiveAuthenticationManager.java index a5669f28e1..043ce83aba 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/authentication/OidcAuthorizationCodeReactiveAuthenticationManager.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/authentication/OidcAuthorizationCodeReactiveAuthenticationManager.java @@ -66,6 +66,7 @@ import java.util.Map; * @see ReactiveOAuth2AccessTokenResponseClient * @see ReactiveOAuth2UserService * @see OAuth2User + * @see ReactiveOidcIdTokenDecoderFactory * @see Section 4.1 Authorization Code Grant Flow * @see Section 4.1.3 Access Token Request * @see Section 4.1.4 Access Token Response diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/authentication/OidcIdTokenDecoderFactory.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/authentication/OidcIdTokenDecoderFactory.java index 4463121970..5d1c317872 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/authentication/OidcIdTokenDecoderFactory.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/authentication/OidcIdTokenDecoderFactory.java @@ -19,6 +19,7 @@ import org.springframework.security.oauth2.client.registration.ClientRegistratio import org.springframework.security.oauth2.core.OAuth2AuthenticationException; import org.springframework.security.oauth2.core.OAuth2Error; import org.springframework.security.oauth2.core.OAuth2TokenValidator; +import org.springframework.security.oauth2.core.oidc.OidcIdToken; import org.springframework.security.oauth2.jwt.Jwt; import org.springframework.security.oauth2.jwt.JwtDecoder; import org.springframework.security.oauth2.jwt.JwtDecoderFactory; @@ -33,14 +34,16 @@ import java.util.function.Function; import static org.springframework.security.oauth2.jwt.JwtProcessors.withJwkSetUri; /** - * Provides a default or custom implementation for {@link OAuth2TokenValidator} + * A {@link JwtDecoderFactory factory} that provides a {@link JwtDecoder} + * used for {@link OidcIdToken} signature verification. + * The provided {@link JwtDecoder} is associated to a specific {@link ClientRegistration}. * * @author Joe Grandja * @author Rafael Dominguez * @since 5.2 - * - * @see OAuth2TokenValidator - * @see Jwt + * @see JwtDecoderFactory + * @see ClientRegistration + * @see OidcIdToken */ public final class OidcIdTokenDecoderFactory implements JwtDecoderFactory { private static final String MISSING_SIGNATURE_VERIFIER_ERROR_CODE = "missing_signature_verifier"; @@ -49,7 +52,7 @@ public final class OidcIdTokenDecoderFactory implements JwtDecoderFactory { if (!StringUtils.hasText(clientRegistration.getProviderDetails().getJwkSetUri())) { OAuth2Error oauth2Error = new OAuth2Error( @@ -63,19 +66,20 @@ public final class OidcIdTokenDecoderFactory implements JwtDecoderFactory jwtValidator = jwtValidatorFactory.apply(clientRegistration); + OAuth2TokenValidator jwtValidator = this.jwtValidatorFactory.apply(clientRegistration); jwtDecoder.setJwtValidator(jwtValidator); return jwtDecoder; }); } /** - * Allows user customization for the {@link OAuth2TokenValidator} + * Sets the factory that provides an {@link OAuth2TokenValidator}, which is used by the {@link JwtDecoder}. + * The default is {@link OidcIdTokenValidator}. * - * @param jwtValidatorFactory + * @param jwtValidatorFactory the factory that provides an {@link OAuth2TokenValidator} */ public final void setJwtValidatorFactory(Function> jwtValidatorFactory) { - Assert.notNull(jwtValidatorFactory, "jwtValidatorFactory cannot be null."); + Assert.notNull(jwtValidatorFactory, "jwtValidatorFactory cannot be null"); this.jwtValidatorFactory = jwtValidatorFactory; } } diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/authentication/ReactiveOidcIdTokenDecoderFactory.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/authentication/ReactiveOidcIdTokenDecoderFactory.java index b3da55282e..605bda5317 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/authentication/ReactiveOidcIdTokenDecoderFactory.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/authentication/ReactiveOidcIdTokenDecoderFactory.java @@ -19,6 +19,7 @@ import org.springframework.security.oauth2.client.registration.ClientRegistratio import org.springframework.security.oauth2.core.OAuth2AuthenticationException; import org.springframework.security.oauth2.core.OAuth2Error; import org.springframework.security.oauth2.core.OAuth2TokenValidator; +import org.springframework.security.oauth2.core.oidc.OidcIdToken; import org.springframework.security.oauth2.jwt.Jwt; import org.springframework.security.oauth2.jwt.NimbusReactiveJwtDecoder; import org.springframework.security.oauth2.jwt.ReactiveJwtDecoder; @@ -31,14 +32,16 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.function.Function; /** - * Provides a default or custom reactive implementation for {@link OAuth2TokenValidator} + * A {@link ReactiveJwtDecoderFactory factory} that provides a {@link ReactiveJwtDecoder} + * used for {@link OidcIdToken} signature verification. + * The provided {@link ReactiveJwtDecoder} is associated to a specific {@link ClientRegistration}. * * @author Joe Grandja * @author Rafael Dominguez * @since 5.2 - * - * @see OAuth2TokenValidator - * @see Jwt + * @see ReactiveJwtDecoderFactory + * @see ClientRegistration + * @see OidcIdToken */ public final class ReactiveOidcIdTokenDecoderFactory implements ReactiveJwtDecoderFactory { private static final String MISSING_SIGNATURE_VERIFIER_ERROR_CODE = "missing_signature_verifier"; @@ -47,7 +50,7 @@ public final class ReactiveOidcIdTokenDecoderFactory implements ReactiveJwtDecod @Override public ReactiveJwtDecoder createDecoder(ClientRegistration clientRegistration) { - Assert.notNull(clientRegistration, "clientRegistration cannot be null."); + Assert.notNull(clientRegistration, "clientRegistration cannot be null"); return this.jwtDecoders.computeIfAbsent(clientRegistration.getRegistrationId(), key -> { if (!StringUtils.hasText(clientRegistration.getProviderDetails().getJwkSetUri())) { OAuth2Error oauth2Error = new OAuth2Error( @@ -61,19 +64,20 @@ public final class ReactiveOidcIdTokenDecoderFactory implements ReactiveJwtDecod } NimbusReactiveJwtDecoder jwtDecoder = new NimbusReactiveJwtDecoder( clientRegistration.getProviderDetails().getJwkSetUri()); - OAuth2TokenValidator jwtValidator = jwtValidatorFactory.apply(clientRegistration); + OAuth2TokenValidator jwtValidator = this.jwtValidatorFactory.apply(clientRegistration); jwtDecoder.setJwtValidator(jwtValidator); return jwtDecoder; }); } /** - * Allows user customization for the {@link OAuth2TokenValidator} + * Sets the factory that provides an {@link OAuth2TokenValidator}, which is used by the {@link ReactiveJwtDecoder}. + * The default is {@link OidcIdTokenValidator}. * - * @param jwtValidatorFactory + * @param jwtValidatorFactory the factory that provides an {@link OAuth2TokenValidator} */ public final void setJwtValidatorFactory(Function> jwtValidatorFactory) { - Assert.notNull(jwtValidatorFactory, "jwtValidatorFactory cannot be null."); + Assert.notNull(jwtValidatorFactory, "jwtValidatorFactory cannot be null"); this.jwtValidatorFactory = jwtValidatorFactory; } } diff --git a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/oidc/authentication/OidcIdTokenDecoderFactoryTests.java b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/oidc/authentication/OidcIdTokenDecoderFactoryTests.java index 707230b2e1..f58ec2c5d9 100644 --- a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/oidc/authentication/OidcIdTokenDecoderFactoryTests.java +++ b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/oidc/authentication/OidcIdTokenDecoderFactoryTests.java @@ -23,15 +23,12 @@ import org.springframework.security.oauth2.core.OAuth2AuthenticationException; import org.springframework.security.oauth2.core.OAuth2TokenValidator; import org.springframework.security.oauth2.jwt.Jwt; -import java.time.Duration; import java.util.function.Function; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; +import static org.mockito.Mockito.*; /** * @author Joe Grandja @@ -45,55 +42,47 @@ public class OidcIdTokenDecoderFactoryTests { private OidcIdTokenDecoderFactory idTokenDecoderFactory; + private Function> defaultJwtValidatorFactory = OidcIdTokenValidator::new; + @Before public void setUp() { - idTokenDecoderFactory = new OidcIdTokenDecoderFactory(); + this.idTokenDecoderFactory = new OidcIdTokenDecoderFactory(); } @Test - public void setJwtValidatorFactoryWhenNullThenThrowIllegalArgumentException(){ - assertThatThrownBy(()-> idTokenDecoderFactory.setJwtValidatorFactory(null)) + public void setJwtValidatorFactoryWhenNullThenThrowIllegalArgumentException() { + assertThatThrownBy(() -> this.idTokenDecoderFactory.setJwtValidatorFactory(null)) .isInstanceOf(IllegalArgumentException.class); } @Test - public void createDecoderWhenClientRegistrationNullThenThrowIllegalArgumentException(){ - assertThatThrownBy(() -> idTokenDecoderFactory.createDecoder(null)) + public void createDecoderWhenClientRegistrationNullThenThrowIllegalArgumentException() { + assertThatThrownBy(() -> this.idTokenDecoderFactory.createDecoder(null)) .isInstanceOf(IllegalArgumentException.class); } @Test - public void createDecoderWhenJwkSetUriEmptyThenThrowOAuth2AuthenticationException(){ - assertThatThrownBy(()-> idTokenDecoderFactory.createDecoder(registration.jwkSetUri(null).build())) - .isInstanceOf(OAuth2AuthenticationException.class); + public void createDecoderWhenJwkSetUriEmptyThenThrowOAuth2AuthenticationException() { + assertThatThrownBy(() -> this.idTokenDecoderFactory.createDecoder(this.registration.jwkSetUri(null).build())) + .isInstanceOf(OAuth2AuthenticationException.class); } @Test - public void createDecoderWhenClientRegistrationValidThenReturnDecoder(){ - assertThat(idTokenDecoderFactory.createDecoder(registration.build())) + public void createDecoderWhenClientRegistrationValidThenReturnDecoder() { + assertThat(this.idTokenDecoderFactory.createDecoder(this.registration.build())) .isNotNull(); } @Test - public void createDecoderWhenCustomJwtValidatorFactorySetThenApplied(){ - Function> customValidator = mock(Function.class); - idTokenDecoderFactory.setJwtValidatorFactory(customValidator); + public void createDecoderWhenCustomJwtValidatorFactorySetThenApplied() { + Function> customJwtValidatorFactory = mock(Function.class); + this.idTokenDecoderFactory.setJwtValidatorFactory(customJwtValidatorFactory); - when(customValidator.apply(any(ClientRegistration.class))) - .thenReturn(customJwtValidatorFactory.apply(registration.build())); + when(customJwtValidatorFactory.apply(any(ClientRegistration.class))) + .thenReturn(this.defaultJwtValidatorFactory.apply(this.registration.build())); - idTokenDecoderFactory.createDecoder(registration.build()); + this.idTokenDecoderFactory.createDecoder(this.registration.build()); - verify(customValidator).apply(any(ClientRegistration.class)); + verify(customJwtValidatorFactory).apply(any(ClientRegistration.class)); } - - private Function> customJwtValidatorFactory = (c) -> { - OidcIdTokenValidator idTokenValidator = new OidcIdTokenValidator(c); - if (c.getRegistrationId().equals("registration-id1")) { - idTokenValidator.setClockSkew(Duration.ofSeconds(30)); - } else if (c.getRegistrationId().equals("registration-id2")) { - idTokenValidator.setClockSkew(Duration.ofSeconds(70)); - } - return idTokenValidator; - }; } diff --git a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/oidc/authentication/ReactiveOidcIdTokenDecoderFactoryTests.java b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/oidc/authentication/ReactiveOidcIdTokenDecoderFactoryTests.java index 1c80773b63..8712d22c37 100644 --- a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/oidc/authentication/ReactiveOidcIdTokenDecoderFactoryTests.java +++ b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/oidc/authentication/ReactiveOidcIdTokenDecoderFactoryTests.java @@ -23,15 +23,12 @@ import org.springframework.security.oauth2.core.OAuth2AuthenticationException; import org.springframework.security.oauth2.core.OAuth2TokenValidator; import org.springframework.security.oauth2.jwt.Jwt; -import java.time.Duration; import java.util.function.Function; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; +import static org.mockito.Mockito.*; /** * @author Joe Grandja @@ -45,55 +42,47 @@ public class ReactiveOidcIdTokenDecoderFactoryTests { private ReactiveOidcIdTokenDecoderFactory idTokenDecoderFactory; + private Function> defaultJwtValidatorFactory = OidcIdTokenValidator::new; + @Before public void setUp() { - idTokenDecoderFactory = new ReactiveOidcIdTokenDecoderFactory(); + this.idTokenDecoderFactory = new ReactiveOidcIdTokenDecoderFactory(); } @Test - public void setJwtValidatorFactoryWhenNullThenThrowIllegalArgumentException(){ - assertThatThrownBy(()-> idTokenDecoderFactory.setJwtValidatorFactory(null)) + public void setJwtValidatorFactoryWhenNullThenThrowIllegalArgumentException() { + assertThatThrownBy(() -> this.idTokenDecoderFactory.setJwtValidatorFactory(null)) .isInstanceOf(IllegalArgumentException.class); } @Test - public void createDecoderWhenClientRegistrationNullThenThrowIllegalArgumentException(){ - assertThatThrownBy(() -> idTokenDecoderFactory.createDecoder(null)) + public void createDecoderWhenClientRegistrationNullThenThrowIllegalArgumentException() { + assertThatThrownBy(() -> this.idTokenDecoderFactory.createDecoder(null)) .isInstanceOf(IllegalArgumentException.class); } @Test - public void createDecoderWhenJwkSetUriEmptyThenThrowOAuth2AuthenticationException(){ - assertThatThrownBy(()-> idTokenDecoderFactory.createDecoder(registration.jwkSetUri(null).build())) + public void createDecoderWhenJwkSetUriEmptyThenThrowOAuth2AuthenticationException() { + assertThatThrownBy(() -> this.idTokenDecoderFactory.createDecoder(this.registration.jwkSetUri(null).build())) .isInstanceOf(OAuth2AuthenticationException.class); } @Test - public void createDecoderWhenClientRegistrationValidThenReturnDecoder(){ - assertThat(idTokenDecoderFactory.createDecoder(registration.build())) + public void createDecoderWhenClientRegistrationValidThenReturnDecoder() { + assertThat(this.idTokenDecoderFactory.createDecoder(this.registration.build())) .isNotNull(); } @Test - public void createDecoderWhenCustomJwtValidatorFactorySetThenApplied(){ - Function> customValidator = mock(Function.class); - idTokenDecoderFactory.setJwtValidatorFactory(customValidator); + public void createDecoderWhenCustomJwtValidatorFactorySetThenApplied() { + Function> customJwtValidatorFactory = mock(Function.class); + this.idTokenDecoderFactory.setJwtValidatorFactory(customJwtValidatorFactory); - when(customValidator.apply(any(ClientRegistration.class))) - .thenReturn(customJwtValidatorFactory.apply(registration.build())); + when(customJwtValidatorFactory.apply(any(ClientRegistration.class))) + .thenReturn(this.defaultJwtValidatorFactory.apply(this.registration.build())); - idTokenDecoderFactory.createDecoder(registration.build()); + this.idTokenDecoderFactory.createDecoder(this.registration.build()); - verify(customValidator).apply(any(ClientRegistration.class)); + verify(customJwtValidatorFactory).apply(any(ClientRegistration.class)); } - - private Function> customJwtValidatorFactory = (c) -> { - OidcIdTokenValidator idTokenValidator = new OidcIdTokenValidator(c); - if (c.getRegistrationId().equals("registration-id1")) { - idTokenValidator.setClockSkew(Duration.ofSeconds(30)); - } else if (c.getRegistrationId().equals("registration-id2")) { - idTokenValidator.setClockSkew(Duration.ofSeconds(70)); - } - return idTokenValidator; - }; }