From 8d7f8b3420b77ee44061591604f3ccc99be90441 Mon Sep 17 00:00:00 2001 From: Daniel Garnier-Moiroux Date: Thu, 20 Oct 2022 14:42:38 +0200 Subject: [PATCH 1/8] Improve customizing OIDC UserInfo endpoint Closes gh-785 --- .../src/docs/asciidoc/protocol-endpoints.adoc | 20 ++- .../OidcUserInfoEndpointConfigurer.java | 162 +++++++++++++++++- .../oidc/web/OidcUserInfoEndpointFilter.java | 75 ++++++-- .../web/configurers/OidcUserInfoTests.java | 139 +++++++++++++-- .../web/OidcUserInfoEndpointFilterTests.java | 113 +++++++++++- 5 files changed, 474 insertions(+), 35 deletions(-) diff --git a/docs/src/docs/asciidoc/protocol-endpoints.adoc b/docs/src/docs/asciidoc/protocol-endpoints.adoc index 56347119..8371e278 100644 --- a/docs/src/docs/asciidoc/protocol-endpoints.adoc +++ b/docs/src/docs/asciidoc/protocol-endpoints.adoc @@ -285,21 +285,37 @@ public SecurityFilterChain authorizationServerSecurityFilterChain(HttpSecurity h .oidc(oidc -> oidc .userInfoEndpoint(userInfoEndpoint -> - userInfoEndpoint.userInfoMapper(userInfoMapper) <1> + userInfoEndpoint + .userInfoRequestConverter(userInfoRequestConverter) <1> + .userInfoRequestConverters(userInfoRequestConvertersConsumer) <2> + .authenticationProvider(authenticationProvider) <3> + .authenticationProviders(authenticationProvidersConsumer) <4> + .userInfoResponseHandler(userInfoResponseHandler) <5> + .errorResponseHandler(errorResponseHandler) <6> + .userInfoMapper(userInfoMapper) <7> ) ); return http.build(); } ---- -<1> `userInfoMapper()`: The `Function` used to extract claims from `OidcUserInfoAuthenticationContext` to an instance of `OidcUserInfo`. +<1> `userInfoRequestConverter()`: Adds an `AuthenticationConverter` (_pre-processor_) used when attempting to extract an https://openid.net/specs/openid-connect-core-1_0.html#UserInfoRequest[UserInfo request] from `HttpServletRequest` to an instance of `OidcUserInfoAuthenticationToken`. +<2> `userInfoRequestConverters()`: Sets the `Consumer` providing access to the `List` of default and (optionally) added ``AuthenticationConverter``'s allowing the ability to add, remove, or customize a specific `AuthenticationConverter`. +<3> `authenticationProvider()`: Adds an `AuthenticationProvider` (_main processor_) used for authenticating the `OidcUserInfoAuthenticationToken`. +<4> `authenticationProviders()`: Sets the `Consumer` providing access to the `List` of default and (optionally) added ``AuthenticationProvider``'s allowing the ability to add, remove, or customize a specific `AuthenticationProvider`. +<5> `revocationResponseHandler()`: The `AuthenticationSuccessHandler` (_post-processor_) used for handling an "`authenticated`" `OidcUserInfoAuthenticationToken` and returning the https://openid.net/specs/openid-connect-core-1_0.html#UserInfoResponse[UserInfo response]. +<6> `userInfoResponseHandler()`: The `AuthenticationFailureHandler` (_post-processor_) used for handling an `OAuth2AuthenticationException` and returning the https://openid.net/specs/openid-connect-core-1_0.html#UserInfoError[UserInfo Error response]. +<7> `userInfoMapper()`: The `Function` used to extract claims from `OidcUserInfoAuthenticationContext` to an instance of `OidcUserInfo`. `OidcUserInfoEndpointConfigurer` configures the `OidcUserInfoEndpointFilter` and registers it with the OAuth2 authorization server `SecurityFilterChain` `@Bean`. `OidcUserInfoEndpointFilter` is the `Filter` that processes https://openid.net/specs/openid-connect-core-1_0.html#UserInfoRequest[UserInfo requests] and returns the https://openid.net/specs/openid-connect-core-1_0.html#UserInfoResponse[OidcUserInfo response]. `OidcUserInfoEndpointFilter` is configured with the following defaults: +* `*AuthenticationConverter*` -- An internal implementation that obtains the `Authentication` from the `SecurityContext` and wraps the principal in an `OidcUserInfoAuthenticationToken`. * `*AuthenticationManager*` -- An `AuthenticationManager` composed of `OidcUserInfoAuthenticationProvider`, which is associated with an internal implementation of `userInfoMapper` that extracts https://openid.net/specs/openid-connect-core-1_0.html#StandardClaims[standard claims] from the https://openid.net/specs/openid-connect-core-1_0.html#IDToken[ID Token] based on the https://openid.net/specs/openid-connect-core-1_0.html#ScopeClaims[scopes requested] during authorization. +* `*AuthenticationSuccessHandler*` -- An internal implementation that handles an "`authenticated`" `OidcUserInfoAuthenticationToken` and returns the UserInfo response. +* `*AuthenticationFailureHandler*` -- An internal implementation that uses the `OAuth2Error` associated with the `OAuth2AuthenticationException` and returns the `OAuth2Error` response. [TIP] You can customize the ID Token by providing an xref:core-model-components.adoc#oauth2-token-customizer[`OAuth2TokenCustomizer`] `@Bean`. diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcUserInfoEndpointConfigurer.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcUserInfoEndpointConfigurer.java index b50e3422..52a1a917 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcUserInfoEndpointConfigurer.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcUserInfoEndpointConfigurer.java @@ -15,13 +15,23 @@ */ package org.springframework.security.oauth2.server.authorization.config.annotation.web.configurers; +import java.util.ArrayList; +import java.util.List; +import java.util.function.Consumer; import java.util.function.Function; +import javax.servlet.http.HttpServletRequest; + import org.springframework.http.HttpMethod; import org.springframework.security.authentication.AuthenticationManager; +import org.springframework.security.authentication.AuthenticationProvider; import org.springframework.security.config.annotation.ObjectPostProcessor; import org.springframework.security.config.annotation.web.builders.HttpSecurity; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.oauth2.core.OAuth2AccessToken; +import org.springframework.security.oauth2.core.OAuth2AuthenticationException; +import org.springframework.security.oauth2.core.OAuth2Error; import org.springframework.security.oauth2.core.oidc.OidcIdToken; import org.springframework.security.oauth2.core.oidc.OidcUserInfo; import org.springframework.security.oauth2.server.authorization.oidc.authentication.OidcUserInfoAuthenticationContext; @@ -29,21 +39,33 @@ import org.springframework.security.oauth2.server.authorization.oidc.authenticat import org.springframework.security.oauth2.server.authorization.oidc.authentication.OidcUserInfoAuthenticationToken; import org.springframework.security.oauth2.server.authorization.oidc.web.OidcUserInfoEndpointFilter; import org.springframework.security.oauth2.server.authorization.settings.AuthorizationServerSettings; +import org.springframework.security.oauth2.server.authorization.web.authentication.DelegatingAuthenticationConverter; import org.springframework.security.web.access.intercept.FilterSecurityInterceptor; +import org.springframework.security.web.authentication.AuthenticationConverter; +import org.springframework.security.web.authentication.AuthenticationFailureHandler; +import org.springframework.security.web.authentication.AuthenticationSuccessHandler; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; import org.springframework.security.web.util.matcher.OrRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; +import org.springframework.util.Assert; /** * Configurer for OpenID Connect 1.0 UserInfo Endpoint. * * @author Steve Riesenberg + * @author Daniel Garnier-Moiroux * @since 0.2.1 * @see OidcConfigurer#userInfoEndpoint * @see OidcUserInfoEndpointFilter */ public final class OidcUserInfoEndpointConfigurer extends AbstractOAuth2Configurer { private RequestMatcher requestMatcher; + private final List userInfoRequestConverters = new ArrayList<>(); + private Consumer> userInfoRequestConvertersConsumer = (authenticationConverters) -> {}; + private final List authenticationProviders = new ArrayList<>(); + private Consumer> authenticationProvidersConsumer = (authenticationProviders) -> {}; + private AuthenticationSuccessHandler userInfoResponseHandler; + private AuthenticationFailureHandler errorResponseHandler; private Function userInfoMapper; /** @@ -53,6 +75,91 @@ public final class OidcUserInfoEndpointConfigurer extends AbstractOAuth2Configur super(objectPostProcessor); } + /** + * Sets the {@link AuthenticationConverter} used when attempting to extract the OAuth2 Access Token from {@link HttpServletRequest} + * to an instance of {@link OidcUserInfoAuthenticationToken} used for authenticating the User Info request. + * + * @param userInfoRequestConverter the {@link AuthenticationConverter} used when attempting to extract an OIDC User Info from {@link HttpServletRequest} + * @return the {@link OidcUserInfoEndpointConfigurer} for further configuration + * @since 0.4.0 + */ + public OidcUserInfoEndpointConfigurer userInfoRequestConverter(AuthenticationConverter userInfoRequestConverter) { + Assert.notNull(userInfoRequestConverter, "userInfoRequestConverter cannot be null"); + this.userInfoRequestConverters.add(userInfoRequestConverter); + return this; + } + + /** + * Sets the {@code Consumer} providing access to the {@code List} of default + * and (optionally) added {@link #userInfoRequestConverter(AuthenticationConverter) AuthenticationConverter}'s + * allowing the ability to add, remove, or customize a specific {@link AuthenticationConverter}. + * + * @param userInfoRequestConvertersConsumer the {@code Consumer} providing access to the {@code List} of default and (optionally) added {@link AuthenticationConverter}'s + * @return the {@link OidcUserInfoEndpointConfigurer} for further configuration + * @since 0.4.0 + */ + public OidcUserInfoEndpointConfigurer userInfoRequestConverters( + Consumer> userInfoRequestConvertersConsumer) { + Assert.notNull(userInfoRequestConvertersConsumer, "userInfoRequestConvertersConsumer cannot be null"); + this.userInfoRequestConvertersConsumer = userInfoRequestConvertersConsumer; + return this; + } + + /** + * Adds an {@link AuthenticationProvider} used for authenticating a type of {@link OidcUserInfoAuthenticationToken}. + * + * @param authenticationProvider a {@link AuthenticationProvider} used for authenticating a type of {@link OidcUserInfoAuthenticationToken} + * @return the {@link OidcUserInfoEndpointConfigurer} for further configuration + * @since 0.4.0 + */ + public OidcUserInfoEndpointConfigurer authenticationProvider(AuthenticationProvider authenticationProvider) { + Assert.notNull(authenticationProvider, "authenticationProvider cannot be null"); + this.authenticationProviders.add(authenticationProvider); + return this; + } + + /** + * Sets the {@code Consumer} providing access to the {@code List} of default + * and (optionally) added {@link #authenticationProvider(AuthenticationProvider) AuthenticationProvider}'s + * allowing the ability to add, remove, or customize a specific {@link AuthenticationProvider}. + * + * @param authenticationProvidersConsumer the {@code Consumer} providing access to the {@code List} of default and (optionally) added {@link AuthenticationProvider}'s + * @return the {@link OidcUserInfoEndpointConfigurer} for further configuration + * @since 0.4.0 + */ + public OidcUserInfoEndpointConfigurer authenticationProviders( + Consumer> authenticationProvidersConsumer) { + Assert.notNull(authenticationProvidersConsumer, "authenticationProvidersConsumer cannot be null"); + this.authenticationProvidersConsumer = authenticationProvidersConsumer; + return this; + } + + /** + * Sets the {@link AuthenticationSuccessHandler} used for handling an {@link OidcUserInfoAuthenticationToken} and + * returning the {@link OidcUserInfo User Info Response}. + * + * @param userInfoResponseHandler the {@link AuthenticationSuccessHandler} used for handling an {@link OidcUserInfoAuthenticationToken} + * @return the {@link OidcUserInfoEndpointConfigurer} for further configuration + * @since 0.4.0 + */ + public OidcUserInfoEndpointConfigurer userInfoResponseHandler(AuthenticationSuccessHandler userInfoResponseHandler) { + this.userInfoResponseHandler = userInfoResponseHandler; + return this; + } + + /** + * Sets the {@link AuthenticationFailureHandler} used for handling an {@link OAuth2AuthenticationException} and + * returning the {@link OAuth2Error Error Response}. + * + * @param errorResponseHandler the {@link AuthenticationFailureHandler} used for handling an {@link OAuth2AuthenticationException} + * @return the {@link OidcUserInfoEndpointConfigurer} for further configuration + * @since 0.4.0 + */ + public OidcUserInfoEndpointConfigurer errorResponseHandler(AuthenticationFailureHandler errorResponseHandler) { + this.errorResponseHandler = errorResponseHandler; + return this; + } + /** * Sets the {@link Function} used to extract claims from {@link OidcUserInfoAuthenticationContext} * to an instance of {@link OidcUserInfo} for the UserInfo response. @@ -67,9 +174,9 @@ public final class OidcUserInfoEndpointConfigurer extends AbstractOAuth2Configur * * * @param userInfoMapper the {@link Function} used to extract claims from {@link OidcUserInfoAuthenticationContext} to an instance of {@link OidcUserInfo} - * @return the {@link OidcUserInfoEndpointConfigurer} for further configuration */ - public OidcUserInfoEndpointConfigurer userInfoMapper(Function userInfoMapper) { + public OidcUserInfoEndpointConfigurer userInfoMapper( + Function userInfoMapper) { this.userInfoMapper = userInfoMapper; return this; } @@ -82,13 +189,15 @@ public final class OidcUserInfoEndpointConfigurer extends AbstractOAuth2Configur new AntPathRequestMatcher(userInfoEndpointUri, HttpMethod.GET.name()), new AntPathRequestMatcher(userInfoEndpointUri, HttpMethod.POST.name())); - OidcUserInfoAuthenticationProvider oidcUserInfoAuthenticationProvider = - new OidcUserInfoAuthenticationProvider( - OAuth2ConfigurerUtils.getAuthorizationService(httpSecurity)); - if (this.userInfoMapper != null) { - oidcUserInfoAuthenticationProvider.setUserInfoMapper(this.userInfoMapper); + List authenticationProviders = createDefaultAuthenticationProviders(httpSecurity); + + if (!this.authenticationProviders.isEmpty()) { + authenticationProviders.addAll(0, this.authenticationProviders); } - httpSecurity.authenticationProvider(postProcess(oidcUserInfoAuthenticationProvider)); + this.authenticationProvidersConsumer.accept(authenticationProviders); + + authenticationProviders.forEach(authenticationProvider -> + httpSecurity.authenticationProvider(postProcess(authenticationProvider))); } @Override @@ -100,6 +209,19 @@ public final class OidcUserInfoEndpointConfigurer extends AbstractOAuth2Configur new OidcUserInfoEndpointFilter( authenticationManager, authorizationServerSettings.getOidcUserInfoEndpoint()); + List authenticationConverters = createDefaultAuthenticationConverters(); + if (!this.userInfoRequestConverters.isEmpty()) { + authenticationConverters.addAll(0, this.userInfoRequestConverters); + } + this.userInfoRequestConvertersConsumer.accept(authenticationConverters); + oidcUserInfoEndpointFilter.setAuthenticationConverter( + new DelegatingAuthenticationConverter(authenticationConverters)); + if (this.userInfoResponseHandler != null) { + oidcUserInfoEndpointFilter.setAuthenticationSuccessHandler(this.userInfoResponseHandler); + } + if (this.errorResponseHandler != null) { + oidcUserInfoEndpointFilter.setAuthenticationFailureHandler(this.errorResponseHandler); + } httpSecurity.addFilterAfter(postProcess(oidcUserInfoEndpointFilter), FilterSecurityInterceptor.class); } @@ -108,4 +230,28 @@ public final class OidcUserInfoEndpointConfigurer extends AbstractOAuth2Configur return this.requestMatcher; } + private static List createDefaultAuthenticationConverters() { + List authenticationConverters = new ArrayList<>(); + authenticationConverters.add( + (request) -> { + Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); + return new OidcUserInfoAuthenticationToken(authentication); + } + ); + return authenticationConverters; + } + + private List createDefaultAuthenticationProviders(HttpSecurity httpSecurity) { + List authenticationProviders = new ArrayList<>(); + + OidcUserInfoAuthenticationProvider oidcUserInfoAuthenticationProvider = new OidcUserInfoAuthenticationProvider( + OAuth2ConfigurerUtils.getAuthorizationService(httpSecurity)); + if (this.userInfoMapper != null) { + oidcUserInfoAuthenticationProvider.setUserInfoMapper(this.userInfoMapper); + } + authenticationProviders.add(oidcUserInfoAuthenticationProvider); + + return authenticationProviders; + } + } diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/web/OidcUserInfoEndpointFilter.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/web/OidcUserInfoEndpointFilter.java index 2c60e5f6..1ca5b2c3 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/web/OidcUserInfoEndpointFilter.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/web/OidcUserInfoEndpointFilter.java @@ -28,6 +28,7 @@ import org.springframework.http.converter.HttpMessageConverter; import org.springframework.http.server.ServletServerHttpResponse; import org.springframework.security.authentication.AuthenticationManager; import org.springframework.security.core.Authentication; +import org.springframework.security.core.AuthenticationException; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.oauth2.core.OAuth2AuthenticationException; import org.springframework.security.oauth2.core.OAuth2Error; @@ -36,6 +37,9 @@ import org.springframework.security.oauth2.core.http.converter.OAuth2ErrorHttpMe import org.springframework.security.oauth2.core.oidc.OidcUserInfo; import org.springframework.security.oauth2.server.authorization.oidc.authentication.OidcUserInfoAuthenticationToken; import org.springframework.security.oauth2.server.authorization.oidc.http.converter.OidcUserInfoHttpMessageConverter; +import org.springframework.security.web.authentication.AuthenticationConverter; +import org.springframework.security.web.authentication.AuthenticationFailureHandler; +import org.springframework.security.web.authentication.AuthenticationSuccessHandler; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; import org.springframework.security.web.util.matcher.OrRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; @@ -61,11 +65,16 @@ public final class OidcUserInfoEndpointFilter extends OncePerRequestFilter { private final AuthenticationManager authenticationManager; private final RequestMatcher userInfoEndpointMatcher; + private AuthenticationConverter authenticationConverter = this::createAuthentication; + private final HttpMessageConverter userInfoHttpMessageConverter = new OidcUserInfoHttpMessageConverter(); private final HttpMessageConverter errorHttpResponseConverter = new OAuth2ErrorHttpMessageConverter(); + private AuthenticationSuccessHandler authenticationSuccessHandler = this::sendUserInfoResponse; + private AuthenticationFailureHandler authenticationFailureHandler = this::sendErrorResponse; + /** * Constructs an {@code OidcUserInfoEndpointFilter} using the provided parameters. * @@ -100,34 +109,77 @@ public final class OidcUserInfoEndpointFilter extends OncePerRequestFilter { } try { - Authentication principal = SecurityContextHolder.getContext().getAuthentication(); - - OidcUserInfoAuthenticationToken userInfoAuthentication = new OidcUserInfoAuthenticationToken(principal); + Authentication userInfoAuthentication = this.authenticationConverter.convert(request); OidcUserInfoAuthenticationToken userInfoAuthenticationResult = (OidcUserInfoAuthenticationToken) this.authenticationManager.authenticate(userInfoAuthentication); - sendUserInfoResponse(response, userInfoAuthenticationResult.getUserInfo()); - + this.authenticationSuccessHandler.onAuthenticationSuccess(request, response, userInfoAuthenticationResult); } catch (OAuth2AuthenticationException ex) { - sendErrorResponse(response, ex.getError()); + this.authenticationFailureHandler.onAuthenticationFailure(request, response, ex); } catch (Exception ex) { OAuth2Error error = new OAuth2Error( OAuth2ErrorCodes.INVALID_REQUEST, "OpenID Connect 1.0 UserInfo Error: " + ex.getMessage(), "https://openid.net/specs/openid-connect-core-1_0.html#UserInfoError"); - sendErrorResponse(response, error); + this.authenticationFailureHandler.onAuthenticationFailure(request, response, + new OAuth2AuthenticationException(error)); } finally { SecurityContextHolder.clearContext(); } } - private void sendUserInfoResponse(HttpServletResponse response, OidcUserInfo userInfo) throws IOException { - ServletServerHttpResponse httpResponse = new ServletServerHttpResponse(response); - this.userInfoHttpMessageConverter.write(userInfo, null, httpResponse); + /** + * Sets the {@link AuthenticationConverter} used when attempting to extract the OAuth2 Access Token from {@link HttpServletRequest} + * to an instance of {@link OidcUserInfoAuthenticationToken} used for authenticating the User Info request. + * + * @param authenticationConverter the {@link AuthenticationConverter} used when attempting to extract an OIDC User Info from {@link HttpServletRequest} + * @since 0.4.0 + */ + public void setAuthenticationConverter(AuthenticationConverter authenticationConverter) { + Assert.notNull(authenticationConverter, "authenticationConverter cannot be null"); + this.authenticationConverter = authenticationConverter; } - private void sendErrorResponse(HttpServletResponse response, OAuth2Error error) throws IOException { + /** + * Sets the {@link AuthenticationSuccessHandler} used for handling an {@link OidcUserInfoAuthenticationToken} and + * returning the {@link OidcUserInfo OIDC User Info}. + * + * @param authenticationSuccessHandler the {@link AuthenticationSuccessHandler} for handling an {@link OidcUserInfoAuthenticationToken} + * @since 0.4.0 + */ + public void setAuthenticationSuccessHandler(AuthenticationSuccessHandler authenticationSuccessHandler) { + Assert.notNull(authenticationSuccessHandler, "authenticationSuccessHandler cannot be null"); + this.authenticationSuccessHandler = authenticationSuccessHandler; + } + + /** + * Sets the {@link AuthenticationFailureHandler} used for handling an {@link OAuth2AuthenticationException} + * and returning the {@link OAuth2Error Error Response}. + * + * @param authenticationFailureHandler the {@link AuthenticationFailureHandler} used for handling an {@link OAuth2AuthenticationException} + * @since 0.4.0 + */ + public void setAuthenticationFailureHandler(AuthenticationFailureHandler authenticationFailureHandler) { + Assert.notNull(authenticationFailureHandler, "authenticationFailureHandler cannot be null"); + this.authenticationFailureHandler = authenticationFailureHandler; + } + + private Authentication createAuthentication(HttpServletRequest request) { + Authentication principal = SecurityContextHolder.getContext().getAuthentication(); + return new OidcUserInfoAuthenticationToken(principal); + } + + private void sendUserInfoResponse(HttpServletRequest request, HttpServletResponse response, + Authentication authentication) throws IOException { + OidcUserInfoAuthenticationToken userInfoAuthenticationToken = (OidcUserInfoAuthenticationToken) authentication; + ServletServerHttpResponse httpResponse = new ServletServerHttpResponse(response); + this.userInfoHttpMessageConverter.write(userInfoAuthenticationToken.getUserInfo(), null, httpResponse); + } + + private void sendErrorResponse(HttpServletRequest request, HttpServletResponse response, + AuthenticationException authenticationException) throws IOException { + OAuth2Error error = ((OAuth2AuthenticationException) authenticationException).getError(); HttpStatus httpStatus = HttpStatus.BAD_REQUEST; if (error.getErrorCode().equals(OAuth2ErrorCodes.INVALID_TOKEN)) { httpStatus = HttpStatus.UNAUTHORIZED; @@ -138,4 +190,5 @@ public final class OidcUserInfoEndpointFilter extends OncePerRequestFilter { httpResponse.setStatusCode(httpStatus); this.errorHttpResponseConverter.write(error, null, httpResponse); } + } diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcUserInfoTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcUserInfoTests.java index ee9363e8..07d464bf 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcUserInfoTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcUserInfoTests.java @@ -19,9 +19,13 @@ import java.time.Instant; import java.util.Arrays; import java.util.Collections; import java.util.HashSet; +import java.util.List; import java.util.Set; +import java.util.function.Consumer; import java.util.function.Function; +import javax.servlet.http.HttpServletResponse; + import com.nimbusds.jose.jwk.JWKSet; import com.nimbusds.jose.jwk.source.ImmutableJWKSet; import com.nimbusds.jose.jwk.source.JWKSource; @@ -30,10 +34,13 @@ import org.junit.Before; import org.junit.BeforeClass; import org.junit.Rule; import org.junit.Test; +import org.mockito.ArgumentCaptor; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Bean; import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpStatus; +import org.springframework.security.authentication.AuthenticationProvider; import org.springframework.security.config.Customizer; import org.springframework.security.config.annotation.web.builders.HttpSecurity; import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity; @@ -61,11 +68,15 @@ import org.springframework.security.oauth2.server.authorization.client.Registere import org.springframework.security.oauth2.server.authorization.client.TestRegisteredClients; import org.springframework.security.oauth2.server.authorization.config.annotation.web.configuration.OAuth2AuthorizationServerConfiguration; import org.springframework.security.oauth2.server.authorization.oidc.authentication.OidcUserInfoAuthenticationContext; +import org.springframework.security.oauth2.server.authorization.oidc.authentication.OidcUserInfoAuthenticationProvider; import org.springframework.security.oauth2.server.authorization.oidc.authentication.OidcUserInfoAuthenticationToken; import org.springframework.security.oauth2.server.authorization.settings.AuthorizationServerSettings; import org.springframework.security.oauth2.server.authorization.test.SpringTestRule; import org.springframework.security.oauth2.server.resource.authentication.JwtAuthenticationToken; import org.springframework.security.web.SecurityFilterChain; +import org.springframework.security.web.authentication.AuthenticationConverter; +import org.springframework.security.web.authentication.AuthenticationFailureHandler; +import org.springframework.security.web.authentication.AuthenticationSuccessHandler; import org.springframework.security.web.context.HttpSessionSecurityContextRepository; import org.springframework.security.web.context.SecurityContextRepository; import org.springframework.security.web.util.matcher.RequestMatcher; @@ -74,8 +85,15 @@ import org.springframework.test.web.servlet.MvcResult; import org.springframework.test.web.servlet.ResultMatcher; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.reset; import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; @@ -99,17 +117,48 @@ public class OidcUserInfoTests { @Autowired private JwtEncoder jwtEncoder; + @Autowired + private JwtDecoder jwtDecoder; + @Autowired private OAuth2AuthorizationService authorizationService; + private static AuthenticationConverter authenticationConverter; + + private static Consumer> authenticationConvertersConsumer; + + private static AuthenticationProvider authenticationProvider; + + private static Consumer> authenticationProvidersConsumer; + + private static AuthenticationSuccessHandler authenticationSuccessHandler; + + private static AuthenticationFailureHandler authenticationFailureHandler; + + private static Function userInfoMapper; + @BeforeClass public static void init() { securityContextRepository = spy(new HttpSessionSecurityContextRepository()); + authenticationConverter = mock(AuthenticationConverter.class); + authenticationConvertersConsumer = mock(Consumer.class); + authenticationProvider = mock(AuthenticationProvider.class); + authenticationProvidersConsumer = mock(Consumer.class); + authenticationSuccessHandler = mock(AuthenticationSuccessHandler.class); + authenticationFailureHandler = mock(AuthenticationFailureHandler.class); + userInfoMapper = mock(Function.class); } @Before public void setup() { reset(securityContextRepository); + reset(authenticationConverter); + reset(authenticationConvertersConsumer); + reset(authenticationProvider); + reset(authenticationProvidersConsumer); + reset(authenticationSuccessHandler); + reset(authenticationFailureHandler); + reset(userInfoMapper); } @Test @@ -145,19 +194,89 @@ public class OidcUserInfoTests { } @Test - public void requestWhenSignedJwtAndCustomUserInfoMapperThenMapJwtClaimsToUserInfoResponse() throws Exception { + public void requestWhenUserInfoEndpointCustomizedThenUsed() throws Exception { this.spring.register(CustomUserInfoConfiguration.class).autowire(); OAuth2Authorization authorization = createAuthorization(); this.authorizationService.save(authorization); + when(userInfoMapper.apply(any())).thenReturn(createUserInfo()); + OAuth2AccessToken accessToken = authorization.getAccessToken().getToken(); // @formatter:off this.mvc.perform(get(DEFAULT_OIDC_USER_INFO_ENDPOINT_URI) .header(HttpHeaders.AUTHORIZATION, "Bearer " + accessToken.getTokenValue())) - .andExpect(status().is2xxSuccessful()) - .andExpectAll(userInfoResponse()); + .andExpect(status().is2xxSuccessful()); // @formatter:on + verify(userInfoMapper).apply(any()); + verify(authenticationConverter).convert(any()); + verify(authenticationSuccessHandler).onAuthenticationSuccess(any(), any(), any()); + verifyNoInteractions(authenticationFailureHandler); + + ArgumentCaptor> authenticationProvidersCaptor = ArgumentCaptor.forClass(List.class); + verify(authenticationProvidersConsumer).accept(authenticationProvidersCaptor.capture()); + List authenticationProviders = authenticationProvidersCaptor.getValue(); + assertThat(authenticationProviders).hasSize(2).allMatch(provider -> + provider == authenticationProvider || + provider instanceof OidcUserInfoAuthenticationProvider + ); + + ArgumentCaptor> authenticationConvertersCaptor = ArgumentCaptor.forClass(List.class); + verify(authenticationConvertersConsumer).accept(authenticationConvertersCaptor.capture()); + List authenticationConverters = authenticationConvertersCaptor.getValue(); + assertThat(authenticationConverters).hasSize(2).allMatch(AuthenticationConverter.class::isInstance); + } + + @Test + public void requestWhenUserInfoEndpointCustomizedThenAuthenticationProviderUsed() throws Exception { + this.spring.register(CustomUserInfoConfiguration.class).autowire(); + + OAuth2Authorization authorization = createAuthorization(); + this.authorizationService.save(authorization); + + when(authenticationProvider.supports(eq(OidcUserInfoAuthenticationToken.class))).thenReturn(true); + String tokenValue = authorization.getAccessToken().getToken().getTokenValue(); + Jwt jwt = this.jwtDecoder.decode(tokenValue); + OidcUserInfoAuthenticationToken oidcUserInfoAuthentication = new OidcUserInfoAuthenticationToken( + new JwtAuthenticationToken(jwt), createUserInfo()); + when(authenticationProvider.authenticate(any())).thenReturn(oidcUserInfoAuthentication); + + OAuth2AccessToken accessToken = authorization.getAccessToken().getToken(); + // @formatter:off + this.mvc.perform(get(DEFAULT_OIDC_USER_INFO_ENDPOINT_URI) + .header(HttpHeaders.AUTHORIZATION, "Bearer " + accessToken.getTokenValue())) + .andExpect(status().is2xxSuccessful()); + // @formatter:on + verify(authenticationSuccessHandler).onAuthenticationSuccess(any(), any(), any()); + verify(authenticationProvider).authenticate(any()); + verifyNoInteractions(authenticationFailureHandler); + verifyNoInteractions(userInfoMapper); + } + + @Test + public void requestWhenUserInfoEndpointCustomizedAndErrorThenUsed() throws Exception { + this.spring.register(CustomUserInfoConfiguration.class).autowire(); + when(userInfoMapper.apply(any())).thenReturn(createUserInfo()); + doAnswer( + invocation -> { + HttpServletResponse response = invocation.getArgument(1); + response.setStatus(HttpStatus.UNAUTHORIZED.value()); + response.getWriter().write("unauthorized"); + return null; + } + ).when(authenticationFailureHandler).onAuthenticationFailure(any(), any(), any()); + + OAuth2AccessToken accessToken = createAuthorization().getAccessToken().getToken(); + + + // @formatter:off + this.mvc.perform(get(DEFAULT_OIDC_USER_INFO_ENDPOINT_URI) + .header(HttpHeaders.AUTHORIZATION, "Bearer " + accessToken.getTokenValue())) + .andExpect(status().is4xxClientError()); + // @formatter:on + verify(authenticationFailureHandler).onAuthenticationFailure(any(), any(), any()); + verifyNoInteractions(authenticationSuccessHandler); + verifyNoInteractions(userInfoMapper); } // gh-482 @@ -271,14 +390,6 @@ public class OidcUserInfoTests { RequestMatcher endpointsMatcher = authorizationServerConfigurer .getEndpointsMatcher(); - // Custom User Info Mapper that retrieves claims from a signed JWT - Function userInfoMapper = context -> { - OidcUserInfoAuthenticationToken authentication = context.getAuthentication(); - JwtAuthenticationToken principal = (JwtAuthenticationToken) authentication.getPrincipal(); - - return new OidcUserInfo(principal.getToken().getClaims()); - }; - // @formatter:off http .requestMatcher(endpointsMatcher) @@ -290,6 +401,12 @@ public class OidcUserInfoTests { .apply(authorizationServerConfigurer) .oidc(oidc -> oidc .userInfoEndpoint(userInfo -> userInfo + .userInfoRequestConverter(authenticationConverter) + .userInfoRequestConverters(authenticationConvertersConsumer) + .authenticationProvider(authenticationProvider) + .authenticationProviders(authenticationProvidersConsumer) + .userInfoResponseHandler(authenticationSuccessHandler) + .errorResponseHandler(authenticationFailureHandler) .userInfoMapper(userInfoMapper) ) ); diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/oidc/web/OidcUserInfoEndpointFilterTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/oidc/web/OidcUserInfoEndpointFilterTests.java index 528285c5..ad7497c6 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/oidc/web/OidcUserInfoEndpointFilterTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/oidc/web/OidcUserInfoEndpointFilterTests.java @@ -44,6 +44,9 @@ import org.springframework.security.oauth2.jwt.JoseHeaderNames; import org.springframework.security.oauth2.jwt.Jwt; import org.springframework.security.oauth2.server.authorization.oidc.authentication.OidcUserInfoAuthenticationToken; import org.springframework.security.oauth2.server.resource.authentication.JwtAuthenticationToken; +import org.springframework.security.web.authentication.AuthenticationConverter; +import org.springframework.security.web.authentication.AuthenticationFailureHandler; +import org.springframework.security.web.authentication.AuthenticationSuccessHandler; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; @@ -84,6 +87,27 @@ public class OidcUserInfoEndpointFilterTests { .withMessage("userInfoEndpointUri cannot be empty"); } + @Test + public void setAuthenticationConverterNullThenThrowIllegalArgumentException() { + assertThatIllegalArgumentException() + .isThrownBy(() -> this.filter.setAuthenticationConverter(null)) + .withMessage("authenticationConverter cannot be null"); + } + + @Test + public void setAuthenticationSuccessHandlerNullThenThrowIllegalArgumentException() { + assertThatIllegalArgumentException() + .isThrownBy(() -> this.filter.setAuthenticationSuccessHandler(null)) + .withMessage("authenticationSuccessHandler cannot be null"); + } + + @Test + public void setAuthenticationFailureHandlerNullThenThrowIllegalArgumentException() { + assertThatIllegalArgumentException() + .isThrownBy(() -> this.filter.setAuthenticationFailureHandler(null)) + .withMessage("authenticationFailureHandler cannot be null"); + } + @Test public void doFilterWhenNotUserInfoRequestThenNotProcessed() throws Exception { String requestUri = "/path"; @@ -145,11 +169,21 @@ public class OidcUserInfoEndpointFilterTests { @Test public void doFilterWhenUserInfoRequestInvalidTokenThenUnauthorizedError() throws Exception { + doFilterWhenAuthenticationExceptionThenError(OAuth2ErrorCodes.INVALID_TOKEN, HttpStatus.UNAUTHORIZED); + } + + @Test + public void doFilterWhenUserInfoRequestInsufficientScopeThenForbiddenError() throws Exception { + doFilterWhenAuthenticationExceptionThenError(OAuth2ErrorCodes.INSUFFICIENT_SCOPE, HttpStatus.FORBIDDEN); + } + + private void doFilterWhenAuthenticationExceptionThenError(String oauth2ErrorCode, HttpStatus httpStatus) + throws Exception { Authentication principal = new TestingAuthenticationToken("principal", "credentials"); SecurityContextHolder.getContext().setAuthentication(principal); when(this.authenticationManager.authenticate(any())) - .thenThrow(new OAuth2AuthenticationException(OAuth2ErrorCodes.INVALID_TOKEN)); + .thenThrow(new OAuth2AuthenticationException(oauth2ErrorCode)); String requestUri = DEFAULT_OIDC_USER_INFO_ENDPOINT_URI; MockHttpServletRequest request = new MockHttpServletRequest("GET", requestUri); @@ -161,9 +195,82 @@ public class OidcUserInfoEndpointFilterTests { verifyNoInteractions(filterChain); - assertThat(response.getStatus()).isEqualTo(HttpStatus.UNAUTHORIZED.value()); + assertThat(response.getStatus()).isEqualTo(httpStatus.value()); OAuth2Error error = readError(response); - assertThat(error.getErrorCode()).isEqualTo(OAuth2ErrorCodes.INVALID_TOKEN); + assertThat(error.getErrorCode()).isEqualTo(oauth2ErrorCode); + } + + @Test + public void doFilterWhenCustomAuthenticationConverterThenUses() throws Exception { + Authentication principal = new TestingAuthenticationToken("principal", "credentials"); + OidcUserInfoAuthenticationToken authentication = new OidcUserInfoAuthenticationToken(principal); + AuthenticationConverter authenticationConverter = mock(AuthenticationConverter.class); + this.filter.setAuthenticationConverter(authenticationConverter); + + when(authenticationConverter.convert(any())).thenReturn(authentication); + when(this.authenticationManager.authenticate(any())).thenReturn( + new OidcUserInfoAuthenticationToken(principal, createUserInfo()) + ); + + String requestUri = DEFAULT_OIDC_USER_INFO_ENDPOINT_URI; + MockHttpServletRequest request = new MockHttpServletRequest("GET", requestUri); + request.setServletPath(requestUri); + MockHttpServletResponse response = new MockHttpServletResponse(); + FilterChain filterChain = mock(FilterChain.class); + + this.filter.doFilter(request, response, filterChain); + + verify(authenticationConverter).convert(request); + verify(this.authenticationManager).authenticate(authentication); + assertUserInfoResponse(response.getContentAsString()); + } + + @Test + public void doFilterWhenCustomAuthenticationSuccessHandlerThenUses() throws Exception { + AuthenticationSuccessHandler successHandler = mock(AuthenticationSuccessHandler.class); + this.filter.setAuthenticationSuccessHandler(successHandler); + + Authentication principal = new TestingAuthenticationToken("principal", "credentials"); + SecurityContextHolder.getContext().setAuthentication(principal); + + OidcUserInfoAuthenticationToken authentication = new OidcUserInfoAuthenticationToken(principal, createUserInfo()); + when(this.authenticationManager.authenticate(any())).thenReturn(authentication); + + String requestUri = DEFAULT_OIDC_USER_INFO_ENDPOINT_URI; + MockHttpServletRequest request = new MockHttpServletRequest("GET", requestUri); + request.setServletPath(requestUri); + MockHttpServletResponse response = new MockHttpServletResponse(); + FilterChain filterChain = mock(FilterChain.class); + + this.filter.doFilter(request, response, filterChain); + + verifyNoInteractions(filterChain); + verify(successHandler).onAuthenticationSuccess(request, response, authentication); + } + + @Test + public void doFilterWhenCustomFailureHandlerThenUses() throws Exception { + AuthenticationFailureHandler failureHandler = mock(AuthenticationFailureHandler.class); + this.filter.setAuthenticationFailureHandler(failureHandler); + + Authentication principal = new TestingAuthenticationToken("principal", "credentials"); + SecurityContextHolder.getContext().setAuthentication(principal); + + OAuth2AuthenticationException authenticationException = + new OAuth2AuthenticationException(OAuth2ErrorCodes.INVALID_TOKEN); + when(this.authenticationManager.authenticate(any())).thenThrow(authenticationException); + + String requestUri = DEFAULT_OIDC_USER_INFO_ENDPOINT_URI; + MockHttpServletRequest request = new MockHttpServletRequest("GET", requestUri); + request.setServletPath(requestUri); + MockHttpServletResponse response = new MockHttpServletResponse(); + FilterChain filterChain = mock(FilterChain.class); + + this.filter.doFilter(request, response, filterChain); + + verifyNoInteractions(filterChain); + + verify(failureHandler).onAuthenticationFailure(request, response, authenticationException); } private OAuth2Error readError(MockHttpServletResponse response) throws Exception { From 2ba711c83a08607a19963bc2fa40dbb4afcd8f21 Mon Sep 17 00:00:00 2001 From: Joe Grandja Date: Thu, 27 Oct 2022 16:33:42 -0400 Subject: [PATCH 2/8] Polish gh-929 --- .../src/docs/asciidoc/protocol-endpoints.adoc | 10 +++---- .../OidcUserInfoEndpointConfigurer.java | 29 ++++++++++--------- .../oidc/web/OidcUserInfoEndpointFilter.java | 24 +++++++-------- .../web/configurers/OidcUserInfoTests.java | 10 ++++--- .../web/OidcUserInfoEndpointFilterTests.java | 14 ++++----- 5 files changed, 45 insertions(+), 42 deletions(-) diff --git a/docs/src/docs/asciidoc/protocol-endpoints.adoc b/docs/src/docs/asciidoc/protocol-endpoints.adoc index 8371e278..dd5dd6c4 100644 --- a/docs/src/docs/asciidoc/protocol-endpoints.adoc +++ b/docs/src/docs/asciidoc/protocol-endpoints.adoc @@ -271,7 +271,7 @@ public SecurityFilterChain authorizationServerSecurityFilterChain(HttpSecurity h `OidcUserInfoEndpointConfigurer` provides the ability to customize the https://openid.net/specs/openid-connect-core-1_0.html#UserInfo[OpenID Connect 1.0 UserInfo endpoint]. It defines extension points that let you customize the https://openid.net/specs/openid-connect-core-1_0.html#UserInfoResponse[UserInfo response]. -`OidcUserInfoEndpointConfigurer` provides the following configuration option: +`OidcUserInfoEndpointConfigurer` provides the following configuration options: [source,java] ---- @@ -303,8 +303,8 @@ public SecurityFilterChain authorizationServerSecurityFilterChain(HttpSecurity h <2> `userInfoRequestConverters()`: Sets the `Consumer` providing access to the `List` of default and (optionally) added ``AuthenticationConverter``'s allowing the ability to add, remove, or customize a specific `AuthenticationConverter`. <3> `authenticationProvider()`: Adds an `AuthenticationProvider` (_main processor_) used for authenticating the `OidcUserInfoAuthenticationToken`. <4> `authenticationProviders()`: Sets the `Consumer` providing access to the `List` of default and (optionally) added ``AuthenticationProvider``'s allowing the ability to add, remove, or customize a specific `AuthenticationProvider`. -<5> `revocationResponseHandler()`: The `AuthenticationSuccessHandler` (_post-processor_) used for handling an "`authenticated`" `OidcUserInfoAuthenticationToken` and returning the https://openid.net/specs/openid-connect-core-1_0.html#UserInfoResponse[UserInfo response]. -<6> `userInfoResponseHandler()`: The `AuthenticationFailureHandler` (_post-processor_) used for handling an `OAuth2AuthenticationException` and returning the https://openid.net/specs/openid-connect-core-1_0.html#UserInfoError[UserInfo Error response]. +<5> `userInfoResponseHandler()`: The `AuthenticationSuccessHandler` (_post-processor_) used for handling an "`authenticated`" `OidcUserInfoAuthenticationToken` and returning the https://openid.net/specs/openid-connect-core-1_0.html#UserInfoResponse[UserInfo response]. +<6> `errorResponseHandler()`: The `AuthenticationFailureHandler` (_post-processor_) used for handling an `OAuth2AuthenticationException` and returning the https://openid.net/specs/openid-connect-core-1_0.html#UserInfoError[UserInfo Error response]. <7> `userInfoMapper()`: The `Function` used to extract claims from `OidcUserInfoAuthenticationContext` to an instance of `OidcUserInfo`. `OidcUserInfoEndpointConfigurer` configures the `OidcUserInfoEndpointFilter` and registers it with the OAuth2 authorization server `SecurityFilterChain` `@Bean`. @@ -312,9 +312,9 @@ public SecurityFilterChain authorizationServerSecurityFilterChain(HttpSecurity h `OidcUserInfoEndpointFilter` is configured with the following defaults: -* `*AuthenticationConverter*` -- An internal implementation that obtains the `Authentication` from the `SecurityContext` and wraps the principal in an `OidcUserInfoAuthenticationToken`. +* `*AuthenticationConverter*` -- An internal implementation that obtains the `Authentication` from the `SecurityContext` and creates an `OidcUserInfoAuthenticationToken` with the principal. * `*AuthenticationManager*` -- An `AuthenticationManager` composed of `OidcUserInfoAuthenticationProvider`, which is associated with an internal implementation of `userInfoMapper` that extracts https://openid.net/specs/openid-connect-core-1_0.html#StandardClaims[standard claims] from the https://openid.net/specs/openid-connect-core-1_0.html#IDToken[ID Token] based on the https://openid.net/specs/openid-connect-core-1_0.html#ScopeClaims[scopes requested] during authorization. -* `*AuthenticationSuccessHandler*` -- An internal implementation that handles an "`authenticated`" `OidcUserInfoAuthenticationToken` and returns the UserInfo response. +* `*AuthenticationSuccessHandler*` -- An internal implementation that handles an "`authenticated`" `OidcUserInfoAuthenticationToken` and returns the `OidcUserInfo` response. * `*AuthenticationFailureHandler*` -- An internal implementation that uses the `OAuth2Error` associated with the `OAuth2AuthenticationException` and returns the `OAuth2Error` response. [TIP] diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcUserInfoEndpointConfigurer.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcUserInfoEndpointConfigurer.java index 52a1a917..26538044 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcUserInfoEndpointConfigurer.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcUserInfoEndpointConfigurer.java @@ -61,7 +61,7 @@ import org.springframework.util.Assert; public final class OidcUserInfoEndpointConfigurer extends AbstractOAuth2Configurer { private RequestMatcher requestMatcher; private final List userInfoRequestConverters = new ArrayList<>(); - private Consumer> userInfoRequestConvertersConsumer = (authenticationConverters) -> {}; + private Consumer> userInfoRequestConvertersConsumer = (userInfoRequestConverters) -> {}; private final List authenticationProviders = new ArrayList<>(); private Consumer> authenticationProvidersConsumer = (authenticationProviders) -> {}; private AuthenticationSuccessHandler userInfoResponseHandler; @@ -76,10 +76,10 @@ public final class OidcUserInfoEndpointConfigurer extends AbstractOAuth2Configur } /** - * Sets the {@link AuthenticationConverter} used when attempting to extract the OAuth2 Access Token from {@link HttpServletRequest} - * to an instance of {@link OidcUserInfoAuthenticationToken} used for authenticating the User Info request. + * Adds an {@link AuthenticationConverter} used when attempting to extract an UserInfo Request from {@link HttpServletRequest} + * to an instance of {@link OidcUserInfoAuthenticationToken} used for authenticating the request. * - * @param userInfoRequestConverter the {@link AuthenticationConverter} used when attempting to extract an OIDC User Info from {@link HttpServletRequest} + * @param userInfoRequestConverter an {@link AuthenticationConverter} used when attempting to extract an UserInfo Request from {@link HttpServletRequest} * @return the {@link OidcUserInfoEndpointConfigurer} for further configuration * @since 0.4.0 */ @@ -106,9 +106,9 @@ public final class OidcUserInfoEndpointConfigurer extends AbstractOAuth2Configur } /** - * Adds an {@link AuthenticationProvider} used for authenticating a type of {@link OidcUserInfoAuthenticationToken}. + * Adds an {@link AuthenticationProvider} used for authenticating an {@link OidcUserInfoAuthenticationToken}. * - * @param authenticationProvider a {@link AuthenticationProvider} used for authenticating a type of {@link OidcUserInfoAuthenticationToken} + * @param authenticationProvider an {@link AuthenticationProvider} used for authenticating an {@link OidcUserInfoAuthenticationToken} * @return the {@link OidcUserInfoEndpointConfigurer} for further configuration * @since 0.4.0 */ @@ -135,8 +135,8 @@ public final class OidcUserInfoEndpointConfigurer extends AbstractOAuth2Configur } /** - * Sets the {@link AuthenticationSuccessHandler} used for handling an {@link OidcUserInfoAuthenticationToken} and - * returning the {@link OidcUserInfo User Info Response}. + * Sets the {@link AuthenticationSuccessHandler} used for handling an {@link OidcUserInfoAuthenticationToken} + * and returning the {@link OidcUserInfo UserInfo Response}. * * @param userInfoResponseHandler the {@link AuthenticationSuccessHandler} used for handling an {@link OidcUserInfoAuthenticationToken} * @return the {@link OidcUserInfoEndpointConfigurer} for further configuration @@ -148,8 +148,8 @@ public final class OidcUserInfoEndpointConfigurer extends AbstractOAuth2Configur } /** - * Sets the {@link AuthenticationFailureHandler} used for handling an {@link OAuth2AuthenticationException} and - * returning the {@link OAuth2Error Error Response}. + * Sets the {@link AuthenticationFailureHandler} used for handling an {@link OAuth2AuthenticationException} + * and returning the {@link OAuth2Error Error Response}. * * @param errorResponseHandler the {@link AuthenticationFailureHandler} used for handling an {@link OAuth2AuthenticationException} * @return the {@link OidcUserInfoEndpointConfigurer} for further configuration @@ -190,12 +190,10 @@ public final class OidcUserInfoEndpointConfigurer extends AbstractOAuth2Configur new AntPathRequestMatcher(userInfoEndpointUri, HttpMethod.POST.name())); List authenticationProviders = createDefaultAuthenticationProviders(httpSecurity); - if (!this.authenticationProviders.isEmpty()) { authenticationProviders.addAll(0, this.authenticationProviders); } this.authenticationProvidersConsumer.accept(authenticationProviders); - authenticationProviders.forEach(authenticationProvider -> httpSecurity.authenticationProvider(postProcess(authenticationProvider))); } @@ -232,20 +230,23 @@ public final class OidcUserInfoEndpointConfigurer extends AbstractOAuth2Configur private static List createDefaultAuthenticationConverters() { List authenticationConverters = new ArrayList<>(); + authenticationConverters.add( (request) -> { Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); return new OidcUserInfoAuthenticationToken(authentication); } ); + return authenticationConverters; } private List createDefaultAuthenticationProviders(HttpSecurity httpSecurity) { List authenticationProviders = new ArrayList<>(); - OidcUserInfoAuthenticationProvider oidcUserInfoAuthenticationProvider = new OidcUserInfoAuthenticationProvider( - OAuth2ConfigurerUtils.getAuthorizationService(httpSecurity)); + OidcUserInfoAuthenticationProvider oidcUserInfoAuthenticationProvider = + new OidcUserInfoAuthenticationProvider( + OAuth2ConfigurerUtils.getAuthorizationService(httpSecurity)); if (this.userInfoMapper != null) { oidcUserInfoAuthenticationProvider.setUserInfoMapper(this.userInfoMapper); } diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/web/OidcUserInfoEndpointFilter.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/web/OidcUserInfoEndpointFilter.java index 1ca5b2c3..355f3b4f 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/web/OidcUserInfoEndpointFilter.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/web/OidcUserInfoEndpointFilter.java @@ -35,6 +35,7 @@ import org.springframework.security.oauth2.core.OAuth2Error; import org.springframework.security.oauth2.core.OAuth2ErrorCodes; import org.springframework.security.oauth2.core.http.converter.OAuth2ErrorHttpMessageConverter; import org.springframework.security.oauth2.core.oidc.OidcUserInfo; +import org.springframework.security.oauth2.server.authorization.oidc.authentication.OidcUserInfoAuthenticationProvider; import org.springframework.security.oauth2.server.authorization.oidc.authentication.OidcUserInfoAuthenticationToken; import org.springframework.security.oauth2.server.authorization.oidc.http.converter.OidcUserInfoHttpMessageConverter; import org.springframework.security.web.authentication.AuthenticationConverter; @@ -51,8 +52,10 @@ import org.springframework.web.filter.OncePerRequestFilter; * * @author Ido Salomon * @author Steve Riesenberg + * @author Daniel Garnier-Moiroux * @since 0.2.1 * @see OidcUserInfo + * @see OidcUserInfoAuthenticationProvider * @see 5.3. UserInfo Endpoint */ public final class OidcUserInfoEndpointFilter extends OncePerRequestFilter { @@ -64,14 +67,11 @@ public final class OidcUserInfoEndpointFilter extends OncePerRequestFilter { private final AuthenticationManager authenticationManager; private final RequestMatcher userInfoEndpointMatcher; - - private AuthenticationConverter authenticationConverter = this::createAuthentication; - private final HttpMessageConverter userInfoHttpMessageConverter = new OidcUserInfoHttpMessageConverter(); private final HttpMessageConverter errorHttpResponseConverter = new OAuth2ErrorHttpMessageConverter(); - + private AuthenticationConverter authenticationConverter = this::createAuthentication; private AuthenticationSuccessHandler authenticationSuccessHandler = this::sendUserInfoResponse; private AuthenticationFailureHandler authenticationFailureHandler = this::sendErrorResponse; @@ -111,8 +111,8 @@ public final class OidcUserInfoEndpointFilter extends OncePerRequestFilter { try { Authentication userInfoAuthentication = this.authenticationConverter.convert(request); - OidcUserInfoAuthenticationToken userInfoAuthenticationResult = - (OidcUserInfoAuthenticationToken) this.authenticationManager.authenticate(userInfoAuthentication); + Authentication userInfoAuthenticationResult = + this.authenticationManager.authenticate(userInfoAuthentication); this.authenticationSuccessHandler.onAuthenticationSuccess(request, response, userInfoAuthenticationResult); } catch (OAuth2AuthenticationException ex) { @@ -130,10 +130,10 @@ public final class OidcUserInfoEndpointFilter extends OncePerRequestFilter { } /** - * Sets the {@link AuthenticationConverter} used when attempting to extract the OAuth2 Access Token from {@link HttpServletRequest} - * to an instance of {@link OidcUserInfoAuthenticationToken} used for authenticating the User Info request. + * Sets the {@link AuthenticationConverter} used when attempting to extract an UserInfo Request from {@link HttpServletRequest} + * to an instance of {@link OidcUserInfoAuthenticationToken} used for authenticating the request. * - * @param authenticationConverter the {@link AuthenticationConverter} used when attempting to extract an OIDC User Info from {@link HttpServletRequest} + * @param authenticationConverter the {@link AuthenticationConverter} used when attempting to extract an UserInfo Request from {@link HttpServletRequest} * @since 0.4.0 */ public void setAuthenticationConverter(AuthenticationConverter authenticationConverter) { @@ -142,10 +142,10 @@ public final class OidcUserInfoEndpointFilter extends OncePerRequestFilter { } /** - * Sets the {@link AuthenticationSuccessHandler} used for handling an {@link OidcUserInfoAuthenticationToken} and - * returning the {@link OidcUserInfo OIDC User Info}. + * Sets the {@link AuthenticationSuccessHandler} used for handling an {@link OidcUserInfoAuthenticationToken} + * and returning the {@link OidcUserInfo UserInfo Response}. * - * @param authenticationSuccessHandler the {@link AuthenticationSuccessHandler} for handling an {@link OidcUserInfoAuthenticationToken} + * @param authenticationSuccessHandler the {@link AuthenticationSuccessHandler} used for handling an {@link OidcUserInfoAuthenticationToken} * @since 0.4.0 */ public void setAuthenticationSuccessHandler(AuthenticationSuccessHandler authenticationSuccessHandler) { diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcUserInfoTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcUserInfoTests.java index 07d464bf..b42b7763 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcUserInfoTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcUserInfoTests.java @@ -208,6 +208,7 @@ public class OidcUserInfoTests { .header(HttpHeaders.AUTHORIZATION, "Bearer " + accessToken.getTokenValue())) .andExpect(status().is2xxSuccessful()); // @formatter:on + verify(userInfoMapper).apply(any()); verify(authenticationConverter).convert(any()); verify(authenticationSuccessHandler).onAuthenticationSuccess(any(), any(), any()); @@ -228,7 +229,7 @@ public class OidcUserInfoTests { } @Test - public void requestWhenUserInfoEndpointCustomizedThenAuthenticationProviderUsed() throws Exception { + public void requestWhenUserInfoEndpointCustomizedWithAuthenticationProviderThenUsed() throws Exception { this.spring.register(CustomUserInfoConfiguration.class).autowire(); OAuth2Authorization authorization = createAuthorization(); @@ -247,6 +248,7 @@ public class OidcUserInfoTests { .header(HttpHeaders.AUTHORIZATION, "Bearer " + accessToken.getTokenValue())) .andExpect(status().is2xxSuccessful()); // @formatter:on + verify(authenticationSuccessHandler).onAuthenticationSuccess(any(), any(), any()); verify(authenticationProvider).authenticate(any()); verifyNoInteractions(authenticationFailureHandler); @@ -254,8 +256,9 @@ public class OidcUserInfoTests { } @Test - public void requestWhenUserInfoEndpointCustomizedAndErrorThenUsed() throws Exception { + public void requestWhenUserInfoEndpointCustomizedWithAuthenticationFailureHandlerThenUsed() throws Exception { this.spring.register(CustomUserInfoConfiguration.class).autowire(); + when(userInfoMapper.apply(any())).thenReturn(createUserInfo()); doAnswer( invocation -> { @@ -267,13 +270,12 @@ public class OidcUserInfoTests { ).when(authenticationFailureHandler).onAuthenticationFailure(any(), any(), any()); OAuth2AccessToken accessToken = createAuthorization().getAccessToken().getToken(); - - // @formatter:off this.mvc.perform(get(DEFAULT_OIDC_USER_INFO_ENDPOINT_URI) .header(HttpHeaders.AUTHORIZATION, "Bearer " + accessToken.getTokenValue())) .andExpect(status().is4xxClientError()); // @formatter:on + verify(authenticationFailureHandler).onAuthenticationFailure(any(), any(), any()); verifyNoInteractions(authenticationSuccessHandler); verifyNoInteractions(userInfoMapper); diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/oidc/web/OidcUserInfoEndpointFilterTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/oidc/web/OidcUserInfoEndpointFilterTests.java index ad7497c6..60e7b939 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/oidc/web/OidcUserInfoEndpointFilterTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/oidc/web/OidcUserInfoEndpointFilterTests.java @@ -88,21 +88,21 @@ public class OidcUserInfoEndpointFilterTests { } @Test - public void setAuthenticationConverterNullThenThrowIllegalArgumentException() { + public void setAuthenticationConverterWhenNullThenThrowIllegalArgumentException() { assertThatIllegalArgumentException() .isThrownBy(() -> this.filter.setAuthenticationConverter(null)) .withMessage("authenticationConverter cannot be null"); } @Test - public void setAuthenticationSuccessHandlerNullThenThrowIllegalArgumentException() { + public void setAuthenticationSuccessHandlerWhenNullThenThrowIllegalArgumentException() { assertThatIllegalArgumentException() .isThrownBy(() -> this.filter.setAuthenticationSuccessHandler(null)) .withMessage("authenticationSuccessHandler cannot be null"); } @Test - public void setAuthenticationFailureHandlerNullThenThrowIllegalArgumentException() { + public void setAuthenticationFailureHandlerWhenNullThenThrowIllegalArgumentException() { assertThatIllegalArgumentException() .isThrownBy(() -> this.filter.setAuthenticationFailureHandler(null)) .withMessage("authenticationFailureHandler cannot be null"); @@ -201,7 +201,7 @@ public class OidcUserInfoEndpointFilterTests { } @Test - public void doFilterWhenCustomAuthenticationConverterThenUses() throws Exception { + public void doFilterWhenCustomAuthenticationConverterThenUsed() throws Exception { Authentication principal = new TestingAuthenticationToken("principal", "credentials"); OidcUserInfoAuthenticationToken authentication = new OidcUserInfoAuthenticationToken(principal); AuthenticationConverter authenticationConverter = mock(AuthenticationConverter.class); @@ -220,13 +220,14 @@ public class OidcUserInfoEndpointFilterTests { this.filter.doFilter(request, response, filterChain); + verifyNoInteractions(filterChain); verify(authenticationConverter).convert(request); verify(this.authenticationManager).authenticate(authentication); assertUserInfoResponse(response.getContentAsString()); } @Test - public void doFilterWhenCustomAuthenticationSuccessHandlerThenUses() throws Exception { + public void doFilterWhenCustomAuthenticationSuccessHandlerThenUsed() throws Exception { AuthenticationSuccessHandler successHandler = mock(AuthenticationSuccessHandler.class); this.filter.setAuthenticationSuccessHandler(successHandler); @@ -249,7 +250,7 @@ public class OidcUserInfoEndpointFilterTests { } @Test - public void doFilterWhenCustomFailureHandlerThenUses() throws Exception { + public void doFilterWhenCustomAuthenticationFailureHandlerThenUsed() throws Exception { AuthenticationFailureHandler failureHandler = mock(AuthenticationFailureHandler.class); this.filter.setAuthenticationFailureHandler(failureHandler); @@ -269,7 +270,6 @@ public class OidcUserInfoEndpointFilterTests { this.filter.doFilter(request, response, filterChain); verifyNoInteractions(filterChain); - verify(failureHandler).onAuthenticationFailure(request, response, authenticationException); } From efbfdc234c7feb63383c0987be91d9874aa0e7e7 Mon Sep 17 00:00:00 2001 From: Daniel Garnier-Moiroux Date: Thu, 27 Oct 2022 14:11:11 +0200 Subject: [PATCH 3/8] Improve customizing OIDC Client Registration endpoint Related gh-696 Closes gh-946 --- .../src/docs/asciidoc/protocol-endpoints.adoc | 18 +- ...cClientRegistrationEndpointConfigurer.java | 170 ++++++++++++++++-- .../OidcClientRegistrationEndpointFilter.java | 78 ++++++-- .../OidcClientRegistrationTests.java | 132 ++++++++++++++ ...ClientRegistrationEndpointFilterTests.java | 161 +++++++++++++---- 5 files changed, 498 insertions(+), 61 deletions(-) diff --git a/docs/src/docs/asciidoc/protocol-endpoints.adoc b/docs/src/docs/asciidoc/protocol-endpoints.adoc index dd5dd6c4..6e5c3f9a 100644 --- a/docs/src/docs/asciidoc/protocol-endpoints.adoc +++ b/docs/src/docs/asciidoc/protocol-endpoints.adoc @@ -367,12 +367,26 @@ public SecurityFilterChain authorizationServerSecurityFilterChain(HttpSecurity h authorizationServerConfigurer .oidc(oidc -> oidc - .clientRegistrationEndpoint(Customizer.withDefaults()) + .clientRegistrationEndpoint(clientRegistrationEndpoint -> + clientRegistrationEndpoint + .clientRegistrationRequestConverter(clientRegistrationRequestConverter) <1> + .clientRegistrationRequestConverters(clientRegistrationRequestConvertersConsumers) <2> + .authenticationProvider(authenticationProvider) <3> + .authenticationProviders(authenticationProvidersConsumer) <4> + .clientRegistrationResponseHandler(clientRegistrationResponseHandler) <5> + .errorResponseHandler(errorResponseHandler) <6> + ) ); return http.build(); } ---- +<1> `clientRegistrationRequestConverter()`: Adds an `AuthenticationConverter` (_pre-processor_) used when attempting to extract a https://openid.net/specs/openid-connect-registration-1_0.html#RegistrationRequest[Client Registration Request] or https://openid.net/specs/openid-connect-registration-1_0.html#ReadRequest[Client Read Request] from `HttpServletRequest` to an instance of `OidcClientRegistrationAuthenticationToken`. +<2> `clientRegistrationRequestConverters()`: Sets the `Consumer` providing access to the `List` of default and (optionally) added ``AuthenticationConverter``'s allowing the ability to add, remove, or customize a specific `AuthenticationConverter`. +<3> `authenticationProvider()`: Adds an `AuthenticationProvider` (_main processor_) used for authenticating the `OidcClientRegistrationAuthenticationToken`. +<4> `authenticationProviders()`: Sets the `Consumer` providing access to the `List` of default and (optionally) added ``AuthenticationProvider``'s allowing the ability to add, remove, or customize a specific `AuthenticationProvider`. +<5> `clientRegistrationResponseHandler()`: The `AuthenticationSuccessHandler` (_post-processor_) used for handling an "`authenticated`" `OidcClientRegistrationAuthenticationToken` and returning the https://openid.net/specs/openid-connect-registration-1_0.html#RegistrationResponse[Client Registration Response] or https://openid.net/specs/openid-connect-registration-1_0.html#ReadResponse[Client Read Response]. +<6> `errorResponseHandler()`: The `AuthenticationFailureHandler` (_post-processor_) used for handling an `OAuth2AuthenticationException` and returning the https://openid.net/specs/openid-connect-registration-1_0.html#RegistrationError[Client Registration Error Response] or https://openid.net/specs/openid-connect-registration-1_0.html#ReadError[Client Read Error Response]. [NOTE] The OpenID Connect 1.0 Client Registration endpoint is disabled by default because many deployments do not require dynamic client registration. @@ -387,6 +401,8 @@ The OpenID Connect 1.0 Client Registration endpoint is disabled by default becau * `*AuthenticationConverter*` -- An `OidcClientRegistrationAuthenticationConverter`. * `*AuthenticationManager*` -- An `AuthenticationManager` composed of `OidcClientRegistrationAuthenticationProvider` and `OidcClientConfigurationAuthenticationProvider`. +* `*AuthenticationSuccessHandler*` -- An internal implementation that handles an "`authenticated`" `OidcClientRegistrationAuthenticationToken` and returns the Client Registration or Client Read response. +* `*AuthenticationFailureHandler*` -- An internal implementation that uses the `OAuth2Error` associated with the `OAuth2AuthenticationException` and returns the `OAuth2Error` response. The OpenID Connect 1.0 Client Registration endpoint is an https://openid.net/specs/openid-connect-registration-1_0.html#ClientRegistration[OAuth2 protected resource], which *REQUIRES* an access token to be sent as a bearer token in the Client Registration (or Client Read) request. diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcClientRegistrationEndpointConfigurer.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcClientRegistrationEndpointConfigurer.java index d631f85a..a8bf0c95 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcClientRegistrationEndpointConfigurer.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcClientRegistrationEndpointConfigurer.java @@ -15,29 +15,53 @@ */ package org.springframework.security.oauth2.server.authorization.config.annotation.web.configurers; +import java.util.ArrayList; +import java.util.List; +import java.util.function.Consumer; + +import javax.servlet.http.HttpServletRequest; + import org.springframework.http.HttpMethod; import org.springframework.security.authentication.AuthenticationManager; +import org.springframework.security.authentication.AuthenticationProvider; import org.springframework.security.config.annotation.ObjectPostProcessor; import org.springframework.security.config.annotation.web.builders.HttpSecurity; +import org.springframework.security.oauth2.core.OAuth2AuthenticationException; +import org.springframework.security.oauth2.core.OAuth2Error; +import org.springframework.security.oauth2.core.oidc.OidcUserInfo; import org.springframework.security.oauth2.server.authorization.oidc.authentication.OidcClientConfigurationAuthenticationProvider; import org.springframework.security.oauth2.server.authorization.oidc.authentication.OidcClientRegistrationAuthenticationProvider; +import org.springframework.security.oauth2.server.authorization.oidc.authentication.OidcClientRegistrationAuthenticationToken; import org.springframework.security.oauth2.server.authorization.oidc.web.OidcClientRegistrationEndpointFilter; +import org.springframework.security.oauth2.server.authorization.oidc.web.authentication.OidcClientRegistrationAuthenticationConverter; import org.springframework.security.oauth2.server.authorization.settings.AuthorizationServerSettings; +import org.springframework.security.oauth2.server.authorization.web.authentication.DelegatingAuthenticationConverter; import org.springframework.security.web.access.intercept.FilterSecurityInterceptor; +import org.springframework.security.web.authentication.AuthenticationConverter; +import org.springframework.security.web.authentication.AuthenticationFailureHandler; +import org.springframework.security.web.authentication.AuthenticationSuccessHandler; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; import org.springframework.security.web.util.matcher.OrRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; +import org.springframework.util.Assert; /** * Configurer for OpenID Connect Dynamic Client Registration 1.0 Endpoint. * * @author Joe Grandja + * @author Daniel Garnier-Moiroux * @since 0.2.0 * @see OidcConfigurer#clientRegistrationEndpoint * @see OidcClientRegistrationEndpointFilter */ public final class OidcClientRegistrationEndpointConfigurer extends AbstractOAuth2Configurer { private RequestMatcher requestMatcher; + private final List clientRegistrationRequestConverters = new ArrayList<>(); + private Consumer> clientRegistrationRequestConvertersConsumer = (authenticationConverters) -> {}; + private final List authenticationProviders = new ArrayList<>(); + private Consumer> authenticationProvidersConsumer = (authenticationProviders) -> {}; + private AuthenticationSuccessHandler clientRegistrationResponseHandler; + private AuthenticationFailureHandler errorResponseHandler; /** * Restrict for internal use only. @@ -46,6 +70,93 @@ public final class OidcClientRegistrationEndpointConfigurer extends AbstractOAut super(objectPostProcessor); } + /** + * Sets the {@link AuthenticationConverter} used when attempting to extract the OIDC Client Registration Request + * from {@link HttpServletRequest} to an instance of {@link OidcClientRegistrationAuthenticationToken} used for + * creating the Client Registration or returning the Client Read Response. + * + * @param clientRegistrationRequestConverter the {@link AuthenticationConverter} used when attempting to extract an + * OIDC Client Registration Request from {@link HttpServletRequest} + * @return the {@link OidcClientRegistrationEndpointConfigurer} for further configuration + * @since 0.4.0 + */ + public OidcClientRegistrationEndpointConfigurer clientRegistrationRequestConverter( + AuthenticationConverter clientRegistrationRequestConverter) { + Assert.notNull(clientRegistrationRequestConverter, "clientRegistrationRequestConverter cannot be null"); + this.clientRegistrationRequestConverters.add(clientRegistrationRequestConverter); + return this; + } + + /** + * Sets the {@code Consumer} providing access to the {@code List} of default + * and (optionally) added {@link #clientRegistrationRequestConverter(AuthenticationConverter) AuthenticationConverter}'s + * allowing the ability to add, remove, or customize a specific {@link AuthenticationConverter}. + * + * @param clientRegistrationRequestConvertersConsumer the {@code Consumer} providing access to the {@code List} of default and (optionally) added {@link AuthenticationConverter}'s + * @return the {@link OidcUserInfoEndpointConfigurer} for further configuration + * @since 0.4.0 + */ + public OidcClientRegistrationEndpointConfigurer clientRegistrationRequestConverters(Consumer> clientRegistrationRequestConvertersConsumer) { + Assert.notNull(clientRegistrationRequestConvertersConsumer, "clientRegistrationRequestConvertersConsumer cannot be null"); + this.clientRegistrationRequestConvertersConsumer = clientRegistrationRequestConvertersConsumer; + return this; + } + + /** + * Adds an {@link AuthenticationProvider} used for authenticating a type of {@link OidcClientRegistrationAuthenticationToken}. + * + * @param authenticationProvider a {@link AuthenticationProvider} used for authenticating a type of {@link OidcClientRegistrationAuthenticationToken} + * @return the {@link OidcClientRegistrationEndpointConfigurer} for further configuration + * @since 0.4.0 + */ + public OidcClientRegistrationEndpointConfigurer authenticationProvider(AuthenticationProvider authenticationProvider) { + Assert.notNull(authenticationProvider, "authenticationProvider cannot be null"); + this.authenticationProviders.add(authenticationProvider); + return this; + } + + /** + * Sets the {@code Consumer} providing access to the {@code List} of default + * and (optionally) added {@link #authenticationProvider(AuthenticationProvider) AuthenticationProvider}'s + * allowing the ability to add, remove, or customize a specific {@link AuthenticationProvider}. + * + * @param authenticationProvidersConsumer the {@code Consumer} providing access to the {@code List} of default and (optionally) added {@link AuthenticationProvider}'s + * @return the {@link OidcClientRegistrationEndpointConfigurer} for further configuration + * @since 0.4.0 + */ + public OidcClientRegistrationEndpointConfigurer authenticationProviders( + Consumer> authenticationProvidersConsumer) { + Assert.notNull(authenticationProvidersConsumer, "authenticationProvidersConsumer cannot be null"); + this.authenticationProvidersConsumer = authenticationProvidersConsumer; + return this; + } + + /** + * Sets the {@link AuthenticationSuccessHandler} used for handling an {@link OidcClientRegistrationAuthenticationToken} and + * returning the {@link OidcUserInfo User Info Response}. + * + * @param clientRegistrationResponseHandler the {@link AuthenticationSuccessHandler} used for handling an {@link OidcClientRegistrationAuthenticationToken} + * @return the {@link OidcClientRegistrationEndpointConfigurer} for further configuration + * @since 0.4.0 + */ + public OidcClientRegistrationEndpointConfigurer clientRegistrationResponseHandler(AuthenticationSuccessHandler clientRegistrationResponseHandler) { + this.clientRegistrationResponseHandler = clientRegistrationResponseHandler; + return this; + } + + /** + * Sets the {@link AuthenticationFailureHandler} used for handling an {@link OAuth2AuthenticationException} and + * returning the {@link OAuth2Error Error Response}. + * + * @param errorResponseHandler the {@link AuthenticationFailureHandler} used for handling an {@link OAuth2AuthenticationException} + * @return the {@link OidcClientRegistrationEndpointConfigurer} for further configuration + * @since 0.4.0 + */ + public OidcClientRegistrationEndpointConfigurer errorResponseHandler(AuthenticationFailureHandler errorResponseHandler) { + this.errorResponseHandler = errorResponseHandler; + return this; + } + @Override void init(HttpSecurity httpSecurity) { AuthorizationServerSettings authorizationServerSettings = OAuth2ConfigurerUtils.getAuthorizationServerSettings(httpSecurity); @@ -54,18 +165,15 @@ public final class OidcClientRegistrationEndpointConfigurer extends AbstractOAut new AntPathRequestMatcher(authorizationServerSettings.getOidcClientRegistrationEndpoint(), HttpMethod.GET.name()) ); - OidcClientRegistrationAuthenticationProvider oidcClientRegistrationAuthenticationProvider = - new OidcClientRegistrationAuthenticationProvider( - OAuth2ConfigurerUtils.getRegisteredClientRepository(httpSecurity), - OAuth2ConfigurerUtils.getAuthorizationService(httpSecurity), - OAuth2ConfigurerUtils.getTokenGenerator(httpSecurity)); - httpSecurity.authenticationProvider(postProcess(oidcClientRegistrationAuthenticationProvider)); + List authenticationProviders = createDefaultAuthenticationProviders(httpSecurity); - OidcClientConfigurationAuthenticationProvider oidcClientConfigurationAuthenticationProvider = - new OidcClientConfigurationAuthenticationProvider( - OAuth2ConfigurerUtils.getRegisteredClientRepository(httpSecurity), - OAuth2ConfigurerUtils.getAuthorizationService(httpSecurity)); - httpSecurity.authenticationProvider(postProcess(oidcClientConfigurationAuthenticationProvider)); + if (!this.authenticationProviders.isEmpty()) { + authenticationProviders.addAll(0, this.authenticationProviders); + } + this.authenticationProvidersConsumer.accept(authenticationProviders); + + authenticationProviders.forEach(authenticationProvider -> + httpSecurity.authenticationProvider(postProcess(authenticationProvider))); } @Override @@ -77,6 +185,22 @@ public final class OidcClientRegistrationEndpointConfigurer extends AbstractOAut new OidcClientRegistrationEndpointFilter( authenticationManager, authorizationServerSettings.getOidcClientRegistrationEndpoint()); + + List authenticationConverters = createDefaultAuthenticationConverters(); + if (!this.clientRegistrationRequestConverters.isEmpty()) { + authenticationConverters.addAll(0, this.clientRegistrationRequestConverters); + } + this.clientRegistrationRequestConvertersConsumer.accept(authenticationConverters); + oidcClientRegistrationEndpointFilter.setAuthenticationConverter( + new DelegatingAuthenticationConverter(authenticationConverters)); + + if (this.clientRegistrationResponseHandler != null) { + oidcClientRegistrationEndpointFilter + .setAuthenticationSuccessHandler(this.clientRegistrationResponseHandler); + } + if (this.errorResponseHandler != null) { + oidcClientRegistrationEndpointFilter.setAuthenticationFailureHandler(this.errorResponseHandler); + } httpSecurity.addFilterAfter(postProcess(oidcClientRegistrationEndpointFilter), FilterSecurityInterceptor.class); } @@ -85,4 +209,28 @@ public final class OidcClientRegistrationEndpointConfigurer extends AbstractOAut return this.requestMatcher; } + private static List createDefaultAuthenticationProviders(HttpSecurity httpSecurity) { + List authenticationProviders = new ArrayList<>(); + + OidcClientRegistrationAuthenticationProvider oidcClientRegistrationAuthenticationProvider = + new OidcClientRegistrationAuthenticationProvider( + OAuth2ConfigurerUtils.getRegisteredClientRepository(httpSecurity), + OAuth2ConfigurerUtils.getAuthorizationService(httpSecurity), + OAuth2ConfigurerUtils.getTokenGenerator(httpSecurity)); + authenticationProviders.add(oidcClientRegistrationAuthenticationProvider); + + OidcClientConfigurationAuthenticationProvider oidcClientConfigurationAuthenticationProvider = + new OidcClientConfigurationAuthenticationProvider( + OAuth2ConfigurerUtils.getRegisteredClientRepository(httpSecurity), + OAuth2ConfigurerUtils.getAuthorizationService(httpSecurity)); + authenticationProviders.add(oidcClientConfigurationAuthenticationProvider); + return authenticationProviders; + } + + private static List createDefaultAuthenticationConverters() { + List authenticationConverters = new ArrayList<>(); + authenticationConverters.add(new OidcClientRegistrationAuthenticationConverter()); + return authenticationConverters; + } + } diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/web/OidcClientRegistrationEndpointFilter.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/web/OidcClientRegistrationEndpointFilter.java index af408b70..f737b01d 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/web/OidcClientRegistrationEndpointFilter.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/web/OidcClientRegistrationEndpointFilter.java @@ -27,6 +27,8 @@ import org.springframework.http.HttpStatus; import org.springframework.http.converter.HttpMessageConverter; import org.springframework.http.server.ServletServerHttpResponse; import org.springframework.security.authentication.AuthenticationManager; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.AuthenticationException; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.oauth2.core.OAuth2AuthenticationException; import org.springframework.security.oauth2.core.OAuth2Error; @@ -40,6 +42,8 @@ import org.springframework.security.oauth2.server.authorization.oidc.authenticat import org.springframework.security.oauth2.server.authorization.oidc.http.converter.OidcClientRegistrationHttpMessageConverter; import org.springframework.security.oauth2.server.authorization.oidc.web.authentication.OidcClientRegistrationAuthenticationConverter; import org.springframework.security.web.authentication.AuthenticationConverter; +import org.springframework.security.web.authentication.AuthenticationFailureHandler; +import org.springframework.security.web.authentication.AuthenticationSuccessHandler; import org.springframework.security.web.util.matcher.AndRequestMatcher; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; import org.springframework.security.web.util.matcher.OrRequestMatcher; @@ -53,6 +57,7 @@ import org.springframework.web.filter.OncePerRequestFilter; * * @author Ovidiu Popa * @author Joe Grandja + * @author Daniel Garnier-Moiroux * @since 0.1.1 * @see OidcClientRegistration * @see OidcClientRegistrationAuthenticationConverter @@ -69,11 +74,13 @@ public final class OidcClientRegistrationEndpointFilter extends OncePerRequestFi private final AuthenticationManager authenticationManager; private final RequestMatcher clientRegistrationEndpointMatcher; + private AuthenticationConverter authenticationConverter = new OidcClientRegistrationAuthenticationConverter(); private final HttpMessageConverter clientRegistrationHttpMessageConverter = new OidcClientRegistrationHttpMessageConverter(); private final HttpMessageConverter errorHttpResponseConverter = new OAuth2ErrorHttpMessageConverter(); - private AuthenticationConverter authenticationConverter; + private AuthenticationSuccessHandler authenticationSuccessHandler = this::sendClientRegistrationResponse; + private AuthenticationFailureHandler authenticationFailureHandler = this::sendErrorResponse; /** * Constructs an {@code OidcClientRegistrationEndpointFilter} using the provided parameters. @@ -99,7 +106,6 @@ public final class OidcClientRegistrationEndpointFilter extends OncePerRequestFi new AntPathRequestMatcher( clientRegistrationEndpointUri, HttpMethod.POST.name()), createClientConfigurationMatcher(clientRegistrationEndpointUri)); - this.authenticationConverter = new OidcClientRegistrationAuthenticationConverter(); } private static RequestMatcher createClientConfigurationMatcher(String clientRegistrationEndpointUri) { @@ -130,33 +136,77 @@ public final class OidcClientRegistrationEndpointFilter extends OncePerRequestFi OidcClientRegistrationAuthenticationToken clientRegistrationAuthenticationResult = (OidcClientRegistrationAuthenticationToken) this.authenticationManager.authenticate(clientRegistrationAuthentication); - HttpStatus httpStatus = HttpStatus.OK; - if (clientRegistrationAuthentication.getClientRegistration() != null) { - httpStatus = HttpStatus.CREATED; - } - - sendClientRegistrationResponse(response, httpStatus, clientRegistrationAuthenticationResult.getClientRegistration()); - + this.authenticationSuccessHandler.onAuthenticationSuccess(request, response, clientRegistrationAuthenticationResult); } catch (OAuth2AuthenticationException ex) { - sendErrorResponse(response, ex.getError()); + this.authenticationFailureHandler.onAuthenticationFailure(request, response, ex); } catch (Exception ex) { OAuth2Error error = new OAuth2Error( OAuth2ErrorCodes.INVALID_REQUEST, "OpenID Client Registration Error: " + ex.getMessage(), "https://openid.net/specs/openid-connect-registration-1_0.html#RegistrationError"); - sendErrorResponse(response, error); + this.authenticationFailureHandler.onAuthenticationFailure(request, response, + new OAuth2AuthenticationException(error)); } finally { SecurityContextHolder.clearContext(); } } - private void sendClientRegistrationResponse(HttpServletResponse response, HttpStatus httpStatus, OidcClientRegistration clientRegistration) throws IOException { + /** + * Sets the {@link AuthenticationConverter} used when attempting to extract the OIDC Client Registration Request + * from {@link HttpServletRequest} to an instance of {@link OidcClientRegistrationAuthenticationToken} used for + * creating the Client Registration or returning the Client Read Response. + * + * @param authenticationConverter the {@link AuthenticationConverter} used when attempting to extract an + * OIDC Client Registration Request from {@link HttpServletRequest} + * @since 0.4.0 + */ + public void setAuthenticationConverter(AuthenticationConverter authenticationConverter) { + Assert.notNull(authenticationConverter, "authenticationConverter cannot be null"); + this.authenticationConverter = authenticationConverter; + } + + /** + * Sets the {@link AuthenticationSuccessHandler} used for handling an {@link OidcClientRegistrationAuthenticationToken} + * and returning the {@link OidcClientRegistration Client Registration Response}. + * + * @param authenticationSuccessHandler the {@link AuthenticationSuccessHandler} used for handling an {@link OidcClientRegistrationAuthenticationToken} + * @see 0.4.0 + */ + public void setAuthenticationSuccessHandler(AuthenticationSuccessHandler authenticationSuccessHandler) { + Assert.notNull(authenticationSuccessHandler, "authenticationSuccessHandler cannot be null"); + this.authenticationSuccessHandler = authenticationSuccessHandler; + } + + /** + * Sets the {@link AuthenticationFailureHandler} used for handling an + * {@link OAuth2AuthenticationException} and returning the {@link OAuth2Error Error + * Response}. + * @param authenticationFailureHandler the {@link AuthenticationFailureHandler} used + * for handling an {@link OAuth2AuthenticationException} + * @since 0.4.0 + */ + public void setAuthenticationFailureHandler(AuthenticationFailureHandler authenticationFailureHandler) { + Assert.notNull(authenticationFailureHandler, "authenticationFailureHandler cannot be null"); + this.authenticationFailureHandler = authenticationFailureHandler; + } + + private void sendClientRegistrationResponse(HttpServletRequest request, HttpServletResponse response, + Authentication authentication) throws IOException { + OidcClientRegistration clientRegistration = ((OidcClientRegistrationAuthenticationToken) authentication) + .getClientRegistration(); ServletServerHttpResponse httpResponse = new ServletServerHttpResponse(response); - httpResponse.setStatusCode(httpStatus); + if (HttpMethod.POST.name().equals(request.getMethod())) { + httpResponse.setStatusCode(HttpStatus.CREATED); + } + else { + httpResponse.setStatusCode(HttpStatus.OK); + } this.clientRegistrationHttpMessageConverter.write(clientRegistration, null, httpResponse); } - private void sendErrorResponse(HttpServletResponse response, OAuth2Error error) throws IOException { + private void sendErrorResponse(HttpServletRequest request, HttpServletResponse response, + AuthenticationException authenticationException) throws IOException { + OAuth2Error error = ((OAuth2AuthenticationException) authenticationException).getError(); HttpStatus httpStatus = HttpStatus.BAD_REQUEST; if (OAuth2ErrorCodes.INVALID_TOKEN.equals(error.getErrorCode())) { httpStatus = HttpStatus.UNAUTHORIZED; diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcClientRegistrationTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcClientRegistrationTests.java index 2fb92bf6..787a143b 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcClientRegistrationTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcClientRegistrationTests.java @@ -18,6 +18,10 @@ package org.springframework.security.oauth2.server.authorization.config.annotati import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.Collections; +import java.util.List; +import java.util.function.Consumer; + +import javax.servlet.http.HttpServletResponse; import com.nimbusds.jose.jwk.JWKSet; import com.nimbusds.jose.jwk.source.JWKSource; @@ -30,6 +34,7 @@ import org.junit.Before; import org.junit.BeforeClass; import org.junit.Rule; import org.junit.Test; +import org.mockito.ArgumentCaptor; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Bean; @@ -37,6 +42,7 @@ import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.http.converter.HttpMessageConverter; +import org.springframework.http.server.ServletServerHttpResponse; import org.springframework.jdbc.core.JdbcOperations; import org.springframework.jdbc.core.JdbcTemplate; import org.springframework.jdbc.datasource.embedded.EmbeddedDatabase; @@ -45,6 +51,7 @@ import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseType; import org.springframework.mock.http.MockHttpOutputMessage; import org.springframework.mock.http.client.MockClientHttpResponse; import org.springframework.mock.web.MockHttpServletResponse; +import org.springframework.security.authentication.AuthenticationProvider; import org.springframework.security.config.Customizer; import org.springframework.security.config.annotation.web.builders.HttpSecurity; import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity; @@ -54,6 +61,7 @@ import org.springframework.security.crypto.password.PasswordEncoder; import org.springframework.security.oauth2.core.AuthorizationGrantType; import org.springframework.security.oauth2.core.ClientAuthenticationMethod; import org.springframework.security.oauth2.core.OAuth2AccessToken; +import org.springframework.security.oauth2.core.OAuth2AuthenticationException; import org.springframework.security.oauth2.core.endpoint.OAuth2AccessTokenResponse; import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationResponseType; import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames; @@ -76,11 +84,17 @@ import org.springframework.security.oauth2.server.authorization.client.Registere import org.springframework.security.oauth2.server.authorization.client.TestRegisteredClients; import org.springframework.security.oauth2.server.authorization.config.annotation.web.configuration.OAuth2AuthorizationServerConfiguration; import org.springframework.security.oauth2.server.authorization.oidc.OidcClientRegistration; +import org.springframework.security.oauth2.server.authorization.oidc.authentication.OidcClientConfigurationAuthenticationProvider; +import org.springframework.security.oauth2.server.authorization.oidc.authentication.OidcClientRegistrationAuthenticationProvider; +import org.springframework.security.oauth2.server.authorization.oidc.authentication.OidcClientRegistrationAuthenticationToken; import org.springframework.security.oauth2.server.authorization.oidc.http.converter.OidcClientRegistrationHttpMessageConverter; import org.springframework.security.oauth2.server.authorization.settings.AuthorizationServerSettings; import org.springframework.security.oauth2.server.authorization.settings.ClientSettings; import org.springframework.security.oauth2.server.authorization.test.SpringTestRule; import org.springframework.security.web.SecurityFilterChain; +import org.springframework.security.web.authentication.AuthenticationConverter; +import org.springframework.security.web.authentication.AuthenticationFailureHandler; +import org.springframework.security.web.authentication.AuthenticationSuccessHandler; import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.test.web.servlet.MockMvc; import org.springframework.test.web.servlet.MvcResult; @@ -88,6 +102,14 @@ import org.springframework.web.util.UriComponentsBuilder; import static org.assertj.core.api.Assertions.assertThat; import static org.hamcrest.CoreMatchers.containsString; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.reset; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; +import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.jwt; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header; @@ -127,6 +149,18 @@ public class OidcClientRegistrationTests { @Autowired private AuthorizationServerSettings authorizationServerSettings; + private static AuthenticationConverter authenticationConverter; + + private static Consumer> authenticationConvertersConsumer; + + private static AuthenticationProvider authenticationProvider; + + private static Consumer> authenticationProvidersConsumer; + + private static AuthenticationSuccessHandler authenticationSuccessHandler; + + private static AuthenticationFailureHandler authenticationFailureHandler; + private MockWebServer server; private String clientJwkSetUrl; @@ -144,6 +178,12 @@ public class OidcClientRegistrationTests { .addScript("org/springframework/security/oauth2/server/authorization/oauth2-authorization-schema.sql") .addScript("org/springframework/security/oauth2/server/authorization/client/oauth2-registered-client-schema.sql") .build(); + authenticationConverter = mock(AuthenticationConverter.class); + authenticationConvertersConsumer = mock(Consumer.class); + authenticationProvider = mock(AuthenticationProvider.class); + authenticationProvidersConsumer = mock(Consumer.class); + authenticationSuccessHandler = mock(AuthenticationSuccessHandler.class); + authenticationFailureHandler = mock(AuthenticationFailureHandler.class); } @Before @@ -157,6 +197,7 @@ public class OidcClientRegistrationTests { .setBody(clientJwkSet.toString()); // @formatter:on this.server.enqueue(response); + when(authenticationProvider.supports(OidcClientRegistrationAuthenticationToken.class)).thenReturn(true); } @After @@ -164,6 +205,12 @@ public class OidcClientRegistrationTests { this.server.shutdown(); jdbcOperations.update("truncate table oauth2_authorization"); jdbcOperations.update("truncate table oauth2_registered_client"); + reset(authenticationConverter); + reset(authenticationConvertersConsumer); + reset(authenticationProvider); + reset(authenticationProvidersConsumer); + reset(authenticationSuccessHandler); + reset(authenticationFailureHandler); } @AfterClass @@ -260,6 +307,65 @@ public class OidcClientRegistrationTests { assertThat(clientConfigurationResponse.getRegistrationAccessToken()).isNull(); } + @Test + public void requestWhenUserInfoEndpointCustomizedThenUsed() throws Exception { + this.spring.register(CustomClientRegistrationConfiguration.class).autowire(); + + // @formatter:off + OidcClientRegistration clientRegistration = OidcClientRegistration.builder() + .clientName("client-name") + .redirectUri("https://client.example.com") + .grantType(AuthorizationGrantType.AUTHORIZATION_CODE.getValue()) + .grantType(AuthorizationGrantType.CLIENT_CREDENTIALS.getValue()) + .scope("scope1") + .scope("scope2") + .build(); + // @formatter:on + + doAnswer(invocation -> { + HttpServletResponse response = invocation.getArgument(1, HttpServletResponse.class); + ServletServerHttpResponse httpResponse = new ServletServerHttpResponse(response); + httpResponse.setStatusCode(HttpStatus.CREATED); + new OidcClientRegistrationHttpMessageConverter().write(clientRegistration, null, httpResponse); + return null; + }).when(authenticationSuccessHandler).onAuthenticationSuccess(any(), any(), any()); + + registerClient(clientRegistration); + + verify(authenticationConverter).convert(any()); + ArgumentCaptor> authenticationConvertersCaptor = ArgumentCaptor + .forClass(List.class); + verify(authenticationConvertersConsumer).accept(authenticationConvertersCaptor.capture()); + List authenticationConverters = authenticationConvertersCaptor.getValue(); + assertThat(authenticationConverters).hasSize(2).contains(authenticationConverter); + + verify(authenticationProvider).authenticate(any()); + ArgumentCaptor> authenticationProvidersCaptor = ArgumentCaptor + .forClass(List.class); + verify(authenticationProvidersConsumer).accept(authenticationProvidersCaptor.capture()); + List authenticationProviders = authenticationProvidersCaptor.getValue(); + assertThat(authenticationProviders).hasSize(3) + .allMatch(provider -> provider == authenticationProvider + || provider instanceof OidcClientRegistrationAuthenticationProvider + || provider instanceof OidcClientConfigurationAuthenticationProvider); + + verify(authenticationSuccessHandler).onAuthenticationSuccess(any(), any(), any()); + verifyNoInteractions(authenticationFailureHandler); + } + + @Test + public void requestWhenUserInfoEndpointCustomizedAndErrorThenUsed() throws Exception { + this.spring.register(CustomClientRegistrationConfiguration.class).autowire(); + + when(authenticationProvider.authenticate(any())).thenThrow(new OAuth2AuthenticationException("error")); + + this.mvc.perform(get(DEFAULT_OIDC_CLIENT_REGISTRATION_ENDPOINT_URI) + .param(OAuth2ParameterNames.CLIENT_ID, "invalid").with(jwt())); + + verify(authenticationFailureHandler).onAuthenticationFailure(any(), any(), any()); + verifyNoInteractions(authenticationSuccessHandler); + } + private OidcClientRegistration registerClient(OidcClientRegistration clientRegistration) throws Exception { // ***** (1) Obtain the "initial" access token used for registering the client @@ -352,6 +458,32 @@ public class OidcClientRegistrationTests { return clientRegistrationHttpMessageConverter.read(OidcClientRegistration.class, httpResponse); } + @EnableWebSecurity + static class CustomClientRegistrationConfiguration extends AuthorizationServerConfiguration { + + @Bean + @Override + public SecurityFilterChain authorizationServerSecurityFilterChain(HttpSecurity http) throws Exception { + OAuth2AuthorizationServerConfigurer authorizationServerConfigurer = new OAuth2AuthorizationServerConfigurer(); + authorizationServerConfigurer.oidc(oidc -> oidc.clientRegistrationEndpoint( + clientRegistration -> clientRegistration.clientRegistrationRequestConverter(authenticationConverter) + .clientRegistrationRequestConverters(authenticationConvertersConsumer) + .authenticationProvider(authenticationProvider) + .authenticationProviders(authenticationProvidersConsumer) + .clientRegistrationResponseHandler(authenticationSuccessHandler) + .errorResponseHandler(authenticationFailureHandler))); + RequestMatcher endpointsMatcher = authorizationServerConfigurer.getEndpointsMatcher(); + + http.requestMatcher(endpointsMatcher) + .authorizeRequests(authorizeRequests -> authorizeRequests.anyRequest().authenticated()) + .csrf(csrf -> csrf.ignoringRequestMatchers(endpointsMatcher)) + .oauth2ResourceServer(OAuth2ResourceServerConfigurer::jwt).apply(authorizationServerConfigurer); + return http.build(); + + } + + } + @EnableWebSecurity static class AuthorizationServerConfiguration { diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/oidc/web/OidcClientRegistrationEndpointFilterTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/oidc/web/OidcClientRegistrationEndpointFilterTests.java index 5a25a587..f6de9fe9 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/oidc/web/OidcClientRegistrationEndpointFilterTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/oidc/web/OidcClientRegistrationEndpointFilterTests.java @@ -15,10 +15,12 @@ */ package org.springframework.security.oauth2.server.authorization.oidc.web; +import java.io.IOException; import java.time.Instant; import java.util.Collections; import javax.servlet.FilterChain; +import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -33,6 +35,8 @@ import org.springframework.mock.http.client.MockClientHttpResponse; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.security.authentication.AuthenticationManager; +import org.springframework.security.authentication.TestingAuthenticationToken; +import org.springframework.security.core.Authentication; import org.springframework.security.core.authority.AuthorityUtils; import org.springframework.security.core.context.SecurityContext; import org.springframework.security.core.context.SecurityContextHolder; @@ -54,10 +58,14 @@ import org.springframework.security.oauth2.server.authorization.oidc.OidcClientR import org.springframework.security.oauth2.server.authorization.oidc.authentication.OidcClientRegistrationAuthenticationToken; import org.springframework.security.oauth2.server.authorization.oidc.http.converter.OidcClientRegistrationHttpMessageConverter; import org.springframework.security.oauth2.server.resource.authentication.JwtAuthenticationToken; +import org.springframework.security.web.authentication.AuthenticationConverter; +import org.springframework.security.web.authentication.AuthenticationFailureHandler; +import org.springframework.security.web.authentication.AuthenticationSuccessHandler; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; @@ -68,6 +76,7 @@ import static org.mockito.Mockito.when; * * @author Ovidiu Popa * @author Joe Grandja + * @author Daniel Garnier-Moiroux */ public class OidcClientRegistrationEndpointFilterTests { private static final String DEFAULT_OIDC_CLIENT_REGISTRATION_ENDPOINT_URI = "/connect/register"; @@ -103,6 +112,27 @@ public class OidcClientRegistrationEndpointFilterTests { .withMessage("clientRegistrationEndpointUri cannot be empty"); } + @Test + public void setAuthenticationConverterWhenNullThenThrowIllegalArgumentException() { + assertThatIllegalArgumentException() + .isThrownBy(() -> this.filter.setAuthenticationConverter(null)) + .withMessage("authenticationConverter cannot be null"); + } + + @Test + public void setAuthenticationSuccessHandlerWhenNullThenThrowIllegalArgumentException() { + assertThatIllegalArgumentException() + .isThrownBy(() -> this.filter.setAuthenticationSuccessHandler(null)) + .withMessage("authenticationSuccessHandler cannot be null"); + } + + @Test + public void setAuthenticationFailureHandlerWhenNullThenThrowIllegalArgumentException() { + assertThatIllegalArgumentException() + .isThrownBy(() -> this.filter.setAuthenticationFailureHandler(null)) + .withMessage("authenticationFailureHandler cannot be null"); + } + @Test public void doFilterWhenNotClientRegistrationRequestThenNotProcessed() throws Exception { String requestUri = "/path"; @@ -203,25 +233,13 @@ public class OidcClientRegistrationEndpointFilterTests { @Test public void doFilterWhenClientRegistrationRequestValidThenSuccessResponse() throws Exception { // @formatter:off - OidcClientRegistration.Builder clientRegistrationBuilder = OidcClientRegistration.builder() - .clientName("client-name") - .redirectUri("https://client.example.com") - .grantType(AuthorizationGrantType.AUTHORIZATION_CODE.getValue()) - .grantType(AuthorizationGrantType.CLIENT_CREDENTIALS.getValue()) - .scope("scope1") - .scope("scope2"); + OidcClientRegistration expectedClientRegistrationResponse = createClientRegistration(); - OidcClientRegistration clientRegistrationRequest = clientRegistrationBuilder.build(); - - OidcClientRegistration expectedClientRegistrationResponse = clientRegistrationBuilder - .clientId("client-id") - .clientIdIssuedAt(Instant.now()) - .clientSecret("client-secret") - .tokenEndpointAuthenticationMethod(ClientAuthenticationMethod.CLIENT_SECRET_BASIC.getValue()) - .responseType(OAuth2AuthorizationResponseType.CODE.getValue()) - .idTokenSignedResponseAlgorithm(SignatureAlgorithm.RS256.getName()) - .registrationAccessToken("registration-access-token") - .registrationClientUrl("https://auth-server:9000/connect/register?client_id=client-id") + OidcClientRegistration clientRegistrationRequest = OidcClientRegistration.builder() + .clientName(expectedClientRegistrationResponse.getClientName()) + .redirectUris(redirectUris -> redirectUris.addAll(expectedClientRegistrationResponse.getRedirectUris())) + .grantTypes(grantTypes -> grantTypes.addAll(expectedClientRegistrationResponse.getGrantTypes())) + .scopes(scopes -> scopes.addAll(expectedClientRegistrationResponse.getScopes())) .build(); // @formatter:on @@ -353,6 +371,27 @@ public class OidcClientRegistrationEndpointFilterTests { OAuth2ErrorCodes.INVALID_CLIENT, HttpStatus.UNAUTHORIZED); } + @Test + public void doFilterWhenCustomAuthenticationFailureHandlerThenUsed() throws Exception { + AuthenticationFailureHandler authenticationFailureHandler = mock(AuthenticationFailureHandler.class); + this.filter.setAuthenticationFailureHandler(authenticationFailureHandler); + + when(this.authenticationManager.authenticate(any())) + .thenThrow(new OAuth2AuthenticationException(OAuth2ErrorCodes.INVALID_TOKEN)); + + String requestUri = DEFAULT_OIDC_CLIENT_REGISTRATION_ENDPOINT_URI; + MockHttpServletRequest request = new MockHttpServletRequest("GET", requestUri); + request.setServletPath(requestUri); + request.setParameter(OAuth2ParameterNames.CLIENT_ID, "client1"); + MockHttpServletResponse response = new MockHttpServletResponse(); + FilterChain filterChain = mock(FilterChain.class); + + this.filter.doFilter(request, response, filterChain); + + verify(authenticationFailureHandler).onAuthenticationFailure(eq(request), eq(response), + any(OAuth2AuthenticationException.class)); + } + private void doFilterWhenClientConfigurationRequestInvalidThenError( String errorCode, HttpStatus status) throws Exception { Jwt jwt = createJwt("client.read"); @@ -384,23 +423,7 @@ public class OidcClientRegistrationEndpointFilterTests { @Test public void doFilterWhenClientConfigurationRequestValidThenSuccessResponse() throws Exception { - // @formatter:off - OidcClientRegistration expectedClientRegistrationResponse = OidcClientRegistration.builder() - .clientId("client-id") - .clientIdIssuedAt(Instant.now()) - .clientSecret("client-secret") - .clientName("client-name") - .redirectUri("https://client.example.com") - .grantType(AuthorizationGrantType.AUTHORIZATION_CODE.getValue()) - .grantType(AuthorizationGrantType.CLIENT_CREDENTIALS.getValue()) - .tokenEndpointAuthenticationMethod(ClientAuthenticationMethod.CLIENT_SECRET_BASIC.getValue()) - .responseType(OAuth2AuthorizationResponseType.CODE.getValue()) - .idTokenSignedResponseAlgorithm(SignatureAlgorithm.RS256.getName()) - .scope("scope1") - .scope("scope2") - .registrationClientUrl("https://auth-server:9000/connect/register?client_id=client-id") - .build(); - // @formatter:on + OidcClientRegistration expectedClientRegistrationResponse = createClientRegistration(); Jwt jwt = createJwt("client.read"); JwtAuthenticationToken principal = new JwtAuthenticationToken( @@ -452,6 +475,74 @@ public class OidcClientRegistrationEndpointFilterTests { .isEqualTo(expectedClientRegistrationResponse.getRegistrationClientUrl()); } + @Test + public void doFilterWhenCustomAuthenticationSuccessHandlerThenUsed() throws Exception { + OidcClientRegistration expectedClientRegistrationResponse = createClientRegistration(); + Authentication principal = new TestingAuthenticationToken("principal", "Credentials"); + + OidcClientRegistrationAuthenticationToken clientRegistrationAuthenticationResult = + new OidcClientRegistrationAuthenticationToken(principal, expectedClientRegistrationResponse); + + when(this.authenticationManager.authenticate(any())).thenReturn(clientRegistrationAuthenticationResult); + AuthenticationSuccessHandler successHandler = mock(AuthenticationSuccessHandler.class); + this.filter.setAuthenticationSuccessHandler(successHandler); + + SecurityContext securityContext = SecurityContextHolder.createEmptyContext(); + securityContext.setAuthentication(principal); + SecurityContextHolder.setContext(securityContext); + + String requestUri = DEFAULT_OIDC_CLIENT_REGISTRATION_ENDPOINT_URI; + MockHttpServletRequest request = new MockHttpServletRequest("GET", requestUri); + request.setServletPath(requestUri); + request.setParameter(OAuth2ParameterNames.CLIENT_ID, expectedClientRegistrationResponse.getClientId()); + + MockHttpServletResponse response = new MockHttpServletResponse(); + FilterChain filterChain = mock(FilterChain.class); + + this.filter.doFilter(request, response, filterChain); + + verify(successHandler).onAuthenticationSuccess(request, response, clientRegistrationAuthenticationResult); + } + + private static OidcClientRegistration createClientRegistration() { + // @formatter:off + OidcClientRegistration expectedClientRegistrationResponse = OidcClientRegistration.builder() + .clientId("client-id") + .clientIdIssuedAt(Instant.now()) + .clientSecret("client-secret") + .clientName("client-name") + .redirectUri("https://client.example.com") + .grantType(AuthorizationGrantType.AUTHORIZATION_CODE.getValue()) + .grantType(AuthorizationGrantType.CLIENT_CREDENTIALS.getValue()) + .tokenEndpointAuthenticationMethod(ClientAuthenticationMethod.CLIENT_SECRET_BASIC.getValue()) + .responseType(OAuth2AuthorizationResponseType.CODE.getValue()) + .idTokenSignedResponseAlgorithm(SignatureAlgorithm.RS256.getName()) + .scope("scope1") + .scope("scope2") + .registrationClientUrl("https://auth-server:9000/connect/register?client_id=client-id") + .build(); + return expectedClientRegistrationResponse; + // @formatter:on + } + + @Test + public void doFilterWhenCustomAuthenticationConverterThenUsed() throws ServletException, IOException { + AuthenticationConverter authenticationConverter = mock(AuthenticationConverter.class); + this.filter.setAuthenticationConverter(authenticationConverter); + + String requestUri = DEFAULT_OIDC_CLIENT_REGISTRATION_ENDPOINT_URI; + MockHttpServletRequest request = new MockHttpServletRequest("GET", requestUri); + request.setServletPath(requestUri); + request.setParameter(OAuth2ParameterNames.CLIENT_ID, "client-id"); + + MockHttpServletResponse response = new MockHttpServletResponse(); + FilterChain filterChain = mock(FilterChain.class); + + this.filter.doFilter(request, response, filterChain); + + verify(authenticationConverter).convert(request); + } + private OAuth2Error readError(MockHttpServletResponse response) throws Exception { MockClientHttpResponse httpResponse = new MockClientHttpResponse( response.getContentAsByteArray(), HttpStatus.valueOf(response.getStatus())); From bfd7a09c3b4e70c51d10eb9b4b53fb0daf81df2b Mon Sep 17 00:00:00 2001 From: Joe Grandja Date: Fri, 28 Oct 2022 15:37:41 -0400 Subject: [PATCH 4/8] Polish gh-946 --- .../src/docs/asciidoc/protocol-endpoints.adoc | 16 ++-- ...cClientRegistrationEndpointConfigurer.java | 53 +++++----- .../OidcClientRegistrationEndpointFilter.java | 31 +++--- .../OidcClientRegistrationTests.java | 54 +++++++---- ...ClientRegistrationEndpointFilterTests.java | 96 +++++++++---------- 5 files changed, 130 insertions(+), 120 deletions(-) diff --git a/docs/src/docs/asciidoc/protocol-endpoints.adoc b/docs/src/docs/asciidoc/protocol-endpoints.adoc index 6e5c3f9a..7a3222b3 100644 --- a/docs/src/docs/asciidoc/protocol-endpoints.adoc +++ b/docs/src/docs/asciidoc/protocol-endpoints.adoc @@ -353,8 +353,10 @@ The guide xref:guides/how-to-userinfo.adoc#how-to-userinfo[How-to: Customize the [[oidc-client-registration-endpoint]] == OpenID Connect 1.0 Client Registration Endpoint -`OidcClientRegistrationEndpointConfigurer` configures the https://openid.net/specs/openid-connect-registration-1_0.html#ClientRegistration[OpenID Connect 1.0 Client Registration endpoint]. -The following example shows how to enable (disabled by default) the OpenID Connect 1.0 Client Registration endpoint: +`OidcClientRegistrationEndpointConfigurer` provides the ability to customize the https://openid.net/specs/openid-connect-registration-1_0.html#ClientRegistration[OpenID Connect 1.0 Client Registration endpoint]. +It defines extension points that let you customize the pre-processing, main processing, and post-processing logic for https://openid.net/specs/openid-connect-registration-1_0.html#RegistrationRequest[Client Registration requests] or https://openid.net/specs/openid-connect-registration-1_0.html#ReadRequest[Client Read requests]. + +`OidcClientRegistrationEndpointConfigurer` provides the following configuration options: [source,java] ---- @@ -375,18 +377,18 @@ public SecurityFilterChain authorizationServerSecurityFilterChain(HttpSecurity h .authenticationProviders(authenticationProvidersConsumer) <4> .clientRegistrationResponseHandler(clientRegistrationResponseHandler) <5> .errorResponseHandler(errorResponseHandler) <6> - ) + ) ); return http.build(); } ---- -<1> `clientRegistrationRequestConverter()`: Adds an `AuthenticationConverter` (_pre-processor_) used when attempting to extract a https://openid.net/specs/openid-connect-registration-1_0.html#RegistrationRequest[Client Registration Request] or https://openid.net/specs/openid-connect-registration-1_0.html#ReadRequest[Client Read Request] from `HttpServletRequest` to an instance of `OidcClientRegistrationAuthenticationToken`. +<1> `clientRegistrationRequestConverter()`: Adds an `AuthenticationConverter` (_pre-processor_) used when attempting to extract a https://openid.net/specs/openid-connect-registration-1_0.html#RegistrationRequest[Client Registration request] or https://openid.net/specs/openid-connect-registration-1_0.html#ReadRequest[Client Read request] from `HttpServletRequest` to an instance of `OidcClientRegistrationAuthenticationToken`. <2> `clientRegistrationRequestConverters()`: Sets the `Consumer` providing access to the `List` of default and (optionally) added ``AuthenticationConverter``'s allowing the ability to add, remove, or customize a specific `AuthenticationConverter`. <3> `authenticationProvider()`: Adds an `AuthenticationProvider` (_main processor_) used for authenticating the `OidcClientRegistrationAuthenticationToken`. <4> `authenticationProviders()`: Sets the `Consumer` providing access to the `List` of default and (optionally) added ``AuthenticationProvider``'s allowing the ability to add, remove, or customize a specific `AuthenticationProvider`. -<5> `clientRegistrationResponseHandler()`: The `AuthenticationSuccessHandler` (_post-processor_) used for handling an "`authenticated`" `OidcClientRegistrationAuthenticationToken` and returning the https://openid.net/specs/openid-connect-registration-1_0.html#RegistrationResponse[Client Registration Response] or https://openid.net/specs/openid-connect-registration-1_0.html#ReadResponse[Client Read Response]. -<6> `errorResponseHandler()`: The `AuthenticationFailureHandler` (_post-processor_) used for handling an `OAuth2AuthenticationException` and returning the https://openid.net/specs/openid-connect-registration-1_0.html#RegistrationError[Client Registration Error Response] or https://openid.net/specs/openid-connect-registration-1_0.html#ReadError[Client Read Error Response]. +<5> `clientRegistrationResponseHandler()`: The `AuthenticationSuccessHandler` (_post-processor_) used for handling an "`authenticated`" `OidcClientRegistrationAuthenticationToken` and returning the https://openid.net/specs/openid-connect-registration-1_0.html#RegistrationResponse[Client Registration response] or https://openid.net/specs/openid-connect-registration-1_0.html#ReadResponse[Client Read response]. +<6> `errorResponseHandler()`: The `AuthenticationFailureHandler` (_post-processor_) used for handling an `OAuth2AuthenticationException` and returning the https://openid.net/specs/openid-connect-registration-1_0.html#RegistrationError[Client Registration Error response] or https://openid.net/specs/openid-connect-registration-1_0.html#ReadError[Client Read Error response]. [NOTE] The OpenID Connect 1.0 Client Registration endpoint is disabled by default because many deployments do not require dynamic client registration. @@ -401,7 +403,7 @@ The OpenID Connect 1.0 Client Registration endpoint is disabled by default becau * `*AuthenticationConverter*` -- An `OidcClientRegistrationAuthenticationConverter`. * `*AuthenticationManager*` -- An `AuthenticationManager` composed of `OidcClientRegistrationAuthenticationProvider` and `OidcClientConfigurationAuthenticationProvider`. -* `*AuthenticationSuccessHandler*` -- An internal implementation that handles an "`authenticated`" `OidcClientRegistrationAuthenticationToken` and returns the Client Registration or Client Read response. +* `*AuthenticationSuccessHandler*` -- An internal implementation that handles an "`authenticated`" `OidcClientRegistrationAuthenticationToken` and returns the `OidcClientRegistration` response. * `*AuthenticationFailureHandler*` -- An internal implementation that uses the `OAuth2Error` associated with the `OAuth2AuthenticationException` and returns the `OAuth2Error` response. The OpenID Connect 1.0 Client Registration endpoint is an https://openid.net/specs/openid-connect-registration-1_0.html#ClientRegistration[OAuth2 protected resource], which *REQUIRES* an access token to be sent as a bearer token in the Client Registration (or Client Read) request. diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcClientRegistrationEndpointConfigurer.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcClientRegistrationEndpointConfigurer.java index a8bf0c95..4e5d3f06 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcClientRegistrationEndpointConfigurer.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcClientRegistrationEndpointConfigurer.java @@ -28,7 +28,7 @@ import org.springframework.security.config.annotation.ObjectPostProcessor; import org.springframework.security.config.annotation.web.builders.HttpSecurity; import org.springframework.security.oauth2.core.OAuth2AuthenticationException; import org.springframework.security.oauth2.core.OAuth2Error; -import org.springframework.security.oauth2.core.oidc.OidcUserInfo; +import org.springframework.security.oauth2.server.authorization.oidc.OidcClientRegistration; import org.springframework.security.oauth2.server.authorization.oidc.authentication.OidcClientConfigurationAuthenticationProvider; import org.springframework.security.oauth2.server.authorization.oidc.authentication.OidcClientRegistrationAuthenticationProvider; import org.springframework.security.oauth2.server.authorization.oidc.authentication.OidcClientRegistrationAuthenticationToken; @@ -46,7 +46,7 @@ import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.util.Assert; /** - * Configurer for OpenID Connect Dynamic Client Registration 1.0 Endpoint. + * Configurer for OpenID Connect 1.0 Dynamic Client Registration Endpoint. * * @author Joe Grandja * @author Daniel Garnier-Moiroux @@ -57,7 +57,7 @@ import org.springframework.util.Assert; public final class OidcClientRegistrationEndpointConfigurer extends AbstractOAuth2Configurer { private RequestMatcher requestMatcher; private final List clientRegistrationRequestConverters = new ArrayList<>(); - private Consumer> clientRegistrationRequestConvertersConsumer = (authenticationConverters) -> {}; + private Consumer> clientRegistrationRequestConvertersConsumer = (clientRegistrationRequestConverters) -> {}; private final List authenticationProviders = new ArrayList<>(); private Consumer> authenticationProvidersConsumer = (authenticationProviders) -> {}; private AuthenticationSuccessHandler clientRegistrationResponseHandler; @@ -71,12 +71,10 @@ public final class OidcClientRegistrationEndpointConfigurer extends AbstractOAut } /** - * Sets the {@link AuthenticationConverter} used when attempting to extract the OIDC Client Registration Request - * from {@link HttpServletRequest} to an instance of {@link OidcClientRegistrationAuthenticationToken} used for - * creating the Client Registration or returning the Client Read Response. + * Adds an {@link AuthenticationConverter} used when attempting to extract a Client Registration Request from {@link HttpServletRequest} + * to an instance of {@link OidcClientRegistrationAuthenticationToken} used for authenticating the request. * - * @param clientRegistrationRequestConverter the {@link AuthenticationConverter} used when attempting to extract an - * OIDC Client Registration Request from {@link HttpServletRequest} + * @param clientRegistrationRequestConverter an {@link AuthenticationConverter} used when attempting to extract a Client Registration Request from {@link HttpServletRequest} * @return the {@link OidcClientRegistrationEndpointConfigurer} for further configuration * @since 0.4.0 */ @@ -96,16 +94,17 @@ public final class OidcClientRegistrationEndpointConfigurer extends AbstractOAut * @return the {@link OidcUserInfoEndpointConfigurer} for further configuration * @since 0.4.0 */ - public OidcClientRegistrationEndpointConfigurer clientRegistrationRequestConverters(Consumer> clientRegistrationRequestConvertersConsumer) { + public OidcClientRegistrationEndpointConfigurer clientRegistrationRequestConverters( + Consumer> clientRegistrationRequestConvertersConsumer) { Assert.notNull(clientRegistrationRequestConvertersConsumer, "clientRegistrationRequestConvertersConsumer cannot be null"); this.clientRegistrationRequestConvertersConsumer = clientRegistrationRequestConvertersConsumer; return this; } /** - * Adds an {@link AuthenticationProvider} used for authenticating a type of {@link OidcClientRegistrationAuthenticationToken}. + * Adds an {@link AuthenticationProvider} used for authenticating an {@link OidcClientRegistrationAuthenticationToken}. * - * @param authenticationProvider a {@link AuthenticationProvider} used for authenticating a type of {@link OidcClientRegistrationAuthenticationToken} + * @param authenticationProvider an {@link AuthenticationProvider} used for authenticating an {@link OidcClientRegistrationAuthenticationToken} * @return the {@link OidcClientRegistrationEndpointConfigurer} for further configuration * @since 0.4.0 */ @@ -132,8 +131,8 @@ public final class OidcClientRegistrationEndpointConfigurer extends AbstractOAut } /** - * Sets the {@link AuthenticationSuccessHandler} used for handling an {@link OidcClientRegistrationAuthenticationToken} and - * returning the {@link OidcUserInfo User Info Response}. + * Sets the {@link AuthenticationSuccessHandler} used for handling an {@link OidcClientRegistrationAuthenticationToken} + * and returning the {@link OidcClientRegistration Client Registration Response}. * * @param clientRegistrationResponseHandler the {@link AuthenticationSuccessHandler} used for handling an {@link OidcClientRegistrationAuthenticationToken} * @return the {@link OidcClientRegistrationEndpointConfigurer} for further configuration @@ -145,8 +144,8 @@ public final class OidcClientRegistrationEndpointConfigurer extends AbstractOAut } /** - * Sets the {@link AuthenticationFailureHandler} used for handling an {@link OAuth2AuthenticationException} and - * returning the {@link OAuth2Error Error Response}. + * Sets the {@link AuthenticationFailureHandler} used for handling an {@link OAuth2AuthenticationException} + * and returning the {@link OAuth2Error Error Response}. * * @param errorResponseHandler the {@link AuthenticationFailureHandler} used for handling an {@link OAuth2AuthenticationException} * @return the {@link OidcClientRegistrationEndpointConfigurer} for further configuration @@ -160,18 +159,17 @@ public final class OidcClientRegistrationEndpointConfigurer extends AbstractOAut @Override void init(HttpSecurity httpSecurity) { AuthorizationServerSettings authorizationServerSettings = OAuth2ConfigurerUtils.getAuthorizationServerSettings(httpSecurity); + String clientRegistrationEndpointUri = authorizationServerSettings.getOidcClientRegistrationEndpoint(); this.requestMatcher = new OrRequestMatcher( - new AntPathRequestMatcher(authorizationServerSettings.getOidcClientRegistrationEndpoint(), HttpMethod.POST.name()), - new AntPathRequestMatcher(authorizationServerSettings.getOidcClientRegistrationEndpoint(), HttpMethod.GET.name()) + new AntPathRequestMatcher(clientRegistrationEndpointUri, HttpMethod.POST.name()), + new AntPathRequestMatcher(clientRegistrationEndpointUri, HttpMethod.GET.name()) ); List authenticationProviders = createDefaultAuthenticationProviders(httpSecurity); - if (!this.authenticationProviders.isEmpty()) { authenticationProviders.addAll(0, this.authenticationProviders); } this.authenticationProvidersConsumer.accept(authenticationProviders); - authenticationProviders.forEach(authenticationProvider -> httpSecurity.authenticationProvider(postProcess(authenticationProvider))); } @@ -185,7 +183,6 @@ public final class OidcClientRegistrationEndpointConfigurer extends AbstractOAut new OidcClientRegistrationEndpointFilter( authenticationManager, authorizationServerSettings.getOidcClientRegistrationEndpoint()); - List authenticationConverters = createDefaultAuthenticationConverters(); if (!this.clientRegistrationRequestConverters.isEmpty()) { authenticationConverters.addAll(0, this.clientRegistrationRequestConverters); @@ -193,7 +190,6 @@ public final class OidcClientRegistrationEndpointConfigurer extends AbstractOAut this.clientRegistrationRequestConvertersConsumer.accept(authenticationConverters); oidcClientRegistrationEndpointFilter.setAuthenticationConverter( new DelegatingAuthenticationConverter(authenticationConverters)); - if (this.clientRegistrationResponseHandler != null) { oidcClientRegistrationEndpointFilter .setAuthenticationSuccessHandler(this.clientRegistrationResponseHandler); @@ -209,6 +205,14 @@ public final class OidcClientRegistrationEndpointConfigurer extends AbstractOAut return this.requestMatcher; } + private static List createDefaultAuthenticationConverters() { + List authenticationConverters = new ArrayList<>(); + + authenticationConverters.add(new OidcClientRegistrationAuthenticationConverter()); + + return authenticationConverters; + } + private static List createDefaultAuthenticationProviders(HttpSecurity httpSecurity) { List authenticationProviders = new ArrayList<>(); @@ -224,13 +228,8 @@ public final class OidcClientRegistrationEndpointConfigurer extends AbstractOAut OAuth2ConfigurerUtils.getRegisteredClientRepository(httpSecurity), OAuth2ConfigurerUtils.getAuthorizationService(httpSecurity)); authenticationProviders.add(oidcClientConfigurationAuthenticationProvider); + return authenticationProviders; } - private static List createDefaultAuthenticationConverters() { - List authenticationConverters = new ArrayList<>(); - authenticationConverters.add(new OidcClientRegistrationAuthenticationConverter()); - return authenticationConverters; - } - } diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/web/OidcClientRegistrationEndpointFilter.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/web/OidcClientRegistrationEndpointFilter.java index f737b01d..031755d3 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/web/OidcClientRegistrationEndpointFilter.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/web/OidcClientRegistrationEndpointFilter.java @@ -74,11 +74,11 @@ public final class OidcClientRegistrationEndpointFilter extends OncePerRequestFi private final AuthenticationManager authenticationManager; private final RequestMatcher clientRegistrationEndpointMatcher; - private AuthenticationConverter authenticationConverter = new OidcClientRegistrationAuthenticationConverter(); private final HttpMessageConverter clientRegistrationHttpMessageConverter = new OidcClientRegistrationHttpMessageConverter(); private final HttpMessageConverter errorHttpResponseConverter = new OAuth2ErrorHttpMessageConverter(); + private AuthenticationConverter authenticationConverter = new OidcClientRegistrationAuthenticationConverter(); private AuthenticationSuccessHandler authenticationSuccessHandler = this::sendClientRegistrationResponse; private AuthenticationFailureHandler authenticationFailureHandler = this::sendErrorResponse; @@ -130,11 +130,10 @@ public final class OidcClientRegistrationEndpointFilter extends OncePerRequestFi } try { - OidcClientRegistrationAuthenticationToken clientRegistrationAuthentication = - (OidcClientRegistrationAuthenticationToken) this.authenticationConverter.convert(request); + Authentication clientRegistrationAuthentication = this.authenticationConverter.convert(request); - OidcClientRegistrationAuthenticationToken clientRegistrationAuthenticationResult = - (OidcClientRegistrationAuthenticationToken) this.authenticationManager.authenticate(clientRegistrationAuthentication); + Authentication clientRegistrationAuthenticationResult = + this.authenticationManager.authenticate(clientRegistrationAuthentication); this.authenticationSuccessHandler.onAuthenticationSuccess(request, response, clientRegistrationAuthenticationResult); } catch (OAuth2AuthenticationException ex) { @@ -142,7 +141,7 @@ public final class OidcClientRegistrationEndpointFilter extends OncePerRequestFi } catch (Exception ex) { OAuth2Error error = new OAuth2Error( OAuth2ErrorCodes.INVALID_REQUEST, - "OpenID Client Registration Error: " + ex.getMessage(), + "OpenID Connect 1.0 Client Registration Error: " + ex.getMessage(), "https://openid.net/specs/openid-connect-registration-1_0.html#RegistrationError"); this.authenticationFailureHandler.onAuthenticationFailure(request, response, new OAuth2AuthenticationException(error)); @@ -152,12 +151,10 @@ public final class OidcClientRegistrationEndpointFilter extends OncePerRequestFi } /** - * Sets the {@link AuthenticationConverter} used when attempting to extract the OIDC Client Registration Request - * from {@link HttpServletRequest} to an instance of {@link OidcClientRegistrationAuthenticationToken} used for - * creating the Client Registration or returning the Client Read Response. + * Sets the {@link AuthenticationConverter} used when attempting to extract a Client Registration Request from {@link HttpServletRequest} + * to an instance of {@link OidcClientRegistrationAuthenticationToken} used for authenticating the request. * - * @param authenticationConverter the {@link AuthenticationConverter} used when attempting to extract an - * OIDC Client Registration Request from {@link HttpServletRequest} + * @param authenticationConverter an {@link AuthenticationConverter} used when attempting to extract a Client Registration Request from {@link HttpServletRequest} * @since 0.4.0 */ public void setAuthenticationConverter(AuthenticationConverter authenticationConverter) { @@ -178,11 +175,10 @@ public final class OidcClientRegistrationEndpointFilter extends OncePerRequestFi } /** - * Sets the {@link AuthenticationFailureHandler} used for handling an - * {@link OAuth2AuthenticationException} and returning the {@link OAuth2Error Error - * Response}. - * @param authenticationFailureHandler the {@link AuthenticationFailureHandler} used - * for handling an {@link OAuth2AuthenticationException} + * Sets the {@link AuthenticationFailureHandler} used for handling an {@link OAuth2AuthenticationException} + * and returning the {@link OAuth2Error Error Response}. + * + * @param authenticationFailureHandler the {@link AuthenticationFailureHandler} used for handling an {@link OAuth2AuthenticationException} * @since 0.4.0 */ public void setAuthenticationFailureHandler(AuthenticationFailureHandler authenticationFailureHandler) { @@ -197,8 +193,7 @@ public final class OidcClientRegistrationEndpointFilter extends OncePerRequestFi ServletServerHttpResponse httpResponse = new ServletServerHttpResponse(response); if (HttpMethod.POST.name().equals(request.getMethod())) { httpResponse.setStatusCode(HttpStatus.CREATED); - } - else { + } else { httpResponse.setStatusCode(HttpStatus.OK); } this.clientRegistrationHttpMessageConverter.write(clientRegistration, null, httpResponse); diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcClientRegistrationTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcClientRegistrationTests.java index 787a143b..06ffbe78 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcClientRegistrationTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcClientRegistrationTests.java @@ -88,6 +88,7 @@ import org.springframework.security.oauth2.server.authorization.oidc.authenticat import org.springframework.security.oauth2.server.authorization.oidc.authentication.OidcClientRegistrationAuthenticationProvider; import org.springframework.security.oauth2.server.authorization.oidc.authentication.OidcClientRegistrationAuthenticationToken; import org.springframework.security.oauth2.server.authorization.oidc.http.converter.OidcClientRegistrationHttpMessageConverter; +import org.springframework.security.oauth2.server.authorization.oidc.web.authentication.OidcClientRegistrationAuthenticationConverter; import org.springframework.security.oauth2.server.authorization.settings.AuthorizationServerSettings; import org.springframework.security.oauth2.server.authorization.settings.ClientSettings; import org.springframework.security.oauth2.server.authorization.test.SpringTestRule; @@ -308,7 +309,7 @@ public class OidcClientRegistrationTests { } @Test - public void requestWhenUserInfoEndpointCustomizedThenUsed() throws Exception { + public void requestWhenClientRegistrationEndpointCustomizedThenUsed() throws Exception { this.spring.register(CustomClientRegistrationConfiguration.class).autowire(); // @formatter:off @@ -333,15 +334,17 @@ public class OidcClientRegistrationTests { registerClient(clientRegistration); verify(authenticationConverter).convert(any()); - ArgumentCaptor> authenticationConvertersCaptor = ArgumentCaptor - .forClass(List.class); + ArgumentCaptor> authenticationConvertersCaptor = + ArgumentCaptor.forClass(List.class); verify(authenticationConvertersConsumer).accept(authenticationConvertersCaptor.capture()); List authenticationConverters = authenticationConvertersCaptor.getValue(); - assertThat(authenticationConverters).hasSize(2).contains(authenticationConverter); + assertThat(authenticationConverters).hasSize(2) + .allMatch(converter -> converter == authenticationConverter + || converter instanceof OidcClientRegistrationAuthenticationConverter); verify(authenticationProvider).authenticate(any()); - ArgumentCaptor> authenticationProvidersCaptor = ArgumentCaptor - .forClass(List.class); + ArgumentCaptor> authenticationProvidersCaptor = + ArgumentCaptor.forClass(List.class); verify(authenticationProvidersConsumer).accept(authenticationProvidersCaptor.capture()); List authenticationProviders = authenticationProvidersCaptor.getValue(); assertThat(authenticationProviders).hasSize(3) @@ -354,7 +357,7 @@ public class OidcClientRegistrationTests { } @Test - public void requestWhenUserInfoEndpointCustomizedAndErrorThenUsed() throws Exception { + public void requestWhenClientRegistrationEndpointCustomizedWithAuthenticationFailureHandlerThenUsed() throws Exception { this.spring.register(CustomClientRegistrationConfiguration.class).autowire(); when(authenticationProvider.authenticate(any())).thenThrow(new OAuth2AuthenticationException("error")); @@ -461,27 +464,38 @@ public class OidcClientRegistrationTests { @EnableWebSecurity static class CustomClientRegistrationConfiguration extends AuthorizationServerConfiguration { + // @formatter:off @Bean @Override public SecurityFilterChain authorizationServerSecurityFilterChain(HttpSecurity http) throws Exception { - OAuth2AuthorizationServerConfigurer authorizationServerConfigurer = new OAuth2AuthorizationServerConfigurer(); - authorizationServerConfigurer.oidc(oidc -> oidc.clientRegistrationEndpoint( - clientRegistration -> clientRegistration.clientRegistrationRequestConverter(authenticationConverter) - .clientRegistrationRequestConverters(authenticationConvertersConsumer) - .authenticationProvider(authenticationProvider) - .authenticationProviders(authenticationProvidersConsumer) - .clientRegistrationResponseHandler(authenticationSuccessHandler) - .errorResponseHandler(authenticationFailureHandler))); + OAuth2AuthorizationServerConfigurer authorizationServerConfigurer = + new OAuth2AuthorizationServerConfigurer(); + authorizationServerConfigurer + .oidc(oidc -> + oidc + .clientRegistrationEndpoint(clientRegistration -> + clientRegistration + .clientRegistrationRequestConverter(authenticationConverter) + .clientRegistrationRequestConverters(authenticationConvertersConsumer) + .authenticationProvider(authenticationProvider) + .authenticationProviders(authenticationProvidersConsumer) + .clientRegistrationResponseHandler(authenticationSuccessHandler) + .errorResponseHandler(authenticationFailureHandler) + ) + ); RequestMatcher endpointsMatcher = authorizationServerConfigurer.getEndpointsMatcher(); - http.requestMatcher(endpointsMatcher) - .authorizeRequests(authorizeRequests -> authorizeRequests.anyRequest().authenticated()) + http + .requestMatcher(endpointsMatcher) + .authorizeRequests(authorizeRequests -> + authorizeRequests.anyRequest().authenticated() + ) .csrf(csrf -> csrf.ignoringRequestMatchers(endpointsMatcher)) - .oauth2ResourceServer(OAuth2ResourceServerConfigurer::jwt).apply(authorizationServerConfigurer); + .oauth2ResourceServer(OAuth2ResourceServerConfigurer::jwt) + .apply(authorizationServerConfigurer); return http.build(); - } - + // @formatter:on } @EnableWebSecurity diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/oidc/web/OidcClientRegistrationEndpointFilterTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/oidc/web/OidcClientRegistrationEndpointFilterTests.java index f6de9fe9..f5e9890e 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/oidc/web/OidcClientRegistrationEndpointFilterTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/oidc/web/OidcClientRegistrationEndpointFilterTests.java @@ -371,27 +371,6 @@ public class OidcClientRegistrationEndpointFilterTests { OAuth2ErrorCodes.INVALID_CLIENT, HttpStatus.UNAUTHORIZED); } - @Test - public void doFilterWhenCustomAuthenticationFailureHandlerThenUsed() throws Exception { - AuthenticationFailureHandler authenticationFailureHandler = mock(AuthenticationFailureHandler.class); - this.filter.setAuthenticationFailureHandler(authenticationFailureHandler); - - when(this.authenticationManager.authenticate(any())) - .thenThrow(new OAuth2AuthenticationException(OAuth2ErrorCodes.INVALID_TOKEN)); - - String requestUri = DEFAULT_OIDC_CLIENT_REGISTRATION_ENDPOINT_URI; - MockHttpServletRequest request = new MockHttpServletRequest("GET", requestUri); - request.setServletPath(requestUri); - request.setParameter(OAuth2ParameterNames.CLIENT_ID, "client1"); - MockHttpServletResponse response = new MockHttpServletResponse(); - FilterChain filterChain = mock(FilterChain.class); - - this.filter.doFilter(request, response, filterChain); - - verify(authenticationFailureHandler).onAuthenticationFailure(eq(request), eq(response), - any(OAuth2AuthenticationException.class)); - } - private void doFilterWhenClientConfigurationRequestInvalidThenError( String errorCode, HttpStatus status) throws Exception { Jwt jwt = createJwt("client.read"); @@ -475,6 +454,24 @@ public class OidcClientRegistrationEndpointFilterTests { .isEqualTo(expectedClientRegistrationResponse.getRegistrationClientUrl()); } + @Test + public void doFilterWhenCustomAuthenticationConverterThenUsed() throws ServletException, IOException { + AuthenticationConverter authenticationConverter = mock(AuthenticationConverter.class); + this.filter.setAuthenticationConverter(authenticationConverter); + + String requestUri = DEFAULT_OIDC_CLIENT_REGISTRATION_ENDPOINT_URI; + MockHttpServletRequest request = new MockHttpServletRequest("GET", requestUri); + request.setServletPath(requestUri); + request.setParameter(OAuth2ParameterNames.CLIENT_ID, "client-id"); + + MockHttpServletResponse response = new MockHttpServletResponse(); + FilterChain filterChain = mock(FilterChain.class); + + this.filter.doFilter(request, response, filterChain); + + verify(authenticationConverter).convert(request); + } + @Test public void doFilterWhenCustomAuthenticationSuccessHandlerThenUsed() throws Exception { OidcClientRegistration expectedClientRegistrationResponse = createClientRegistration(); @@ -504,43 +501,25 @@ public class OidcClientRegistrationEndpointFilterTests { verify(successHandler).onAuthenticationSuccess(request, response, clientRegistrationAuthenticationResult); } - private static OidcClientRegistration createClientRegistration() { - // @formatter:off - OidcClientRegistration expectedClientRegistrationResponse = OidcClientRegistration.builder() - .clientId("client-id") - .clientIdIssuedAt(Instant.now()) - .clientSecret("client-secret") - .clientName("client-name") - .redirectUri("https://client.example.com") - .grantType(AuthorizationGrantType.AUTHORIZATION_CODE.getValue()) - .grantType(AuthorizationGrantType.CLIENT_CREDENTIALS.getValue()) - .tokenEndpointAuthenticationMethod(ClientAuthenticationMethod.CLIENT_SECRET_BASIC.getValue()) - .responseType(OAuth2AuthorizationResponseType.CODE.getValue()) - .idTokenSignedResponseAlgorithm(SignatureAlgorithm.RS256.getName()) - .scope("scope1") - .scope("scope2") - .registrationClientUrl("https://auth-server:9000/connect/register?client_id=client-id") - .build(); - return expectedClientRegistrationResponse; - // @formatter:on - } - @Test - public void doFilterWhenCustomAuthenticationConverterThenUsed() throws ServletException, IOException { - AuthenticationConverter authenticationConverter = mock(AuthenticationConverter.class); - this.filter.setAuthenticationConverter(authenticationConverter); + public void doFilterWhenCustomAuthenticationFailureHandlerThenUsed() throws Exception { + AuthenticationFailureHandler authenticationFailureHandler = mock(AuthenticationFailureHandler.class); + this.filter.setAuthenticationFailureHandler(authenticationFailureHandler); + + when(this.authenticationManager.authenticate(any())) + .thenThrow(new OAuth2AuthenticationException(OAuth2ErrorCodes.INVALID_TOKEN)); String requestUri = DEFAULT_OIDC_CLIENT_REGISTRATION_ENDPOINT_URI; MockHttpServletRequest request = new MockHttpServletRequest("GET", requestUri); request.setServletPath(requestUri); - request.setParameter(OAuth2ParameterNames.CLIENT_ID, "client-id"); - + request.setParameter(OAuth2ParameterNames.CLIENT_ID, "client1"); MockHttpServletResponse response = new MockHttpServletResponse(); FilterChain filterChain = mock(FilterChain.class); this.filter.doFilter(request, response, filterChain); - verify(authenticationConverter).convert(request); + verify(authenticationFailureHandler).onAuthenticationFailure(eq(request), eq(response), + any(OAuth2AuthenticationException.class)); } private OAuth2Error readError(MockHttpServletResponse response) throws Exception { @@ -562,6 +541,27 @@ public class OidcClientRegistrationEndpointFilterTests { return this.clientRegistrationHttpMessageConverter.read(OidcClientRegistration.class, httpResponse); } + private static OidcClientRegistration createClientRegistration() { + // @formatter:off + OidcClientRegistration clientRegistration = OidcClientRegistration.builder() + .clientId("client-id") + .clientIdIssuedAt(Instant.now()) + .clientSecret("client-secret") + .clientName("client-name") + .redirectUri("https://client.example.com") + .grantType(AuthorizationGrantType.AUTHORIZATION_CODE.getValue()) + .grantType(AuthorizationGrantType.CLIENT_CREDENTIALS.getValue()) + .tokenEndpointAuthenticationMethod(ClientAuthenticationMethod.CLIENT_SECRET_BASIC.getValue()) + .responseType(OAuth2AuthorizationResponseType.CODE.getValue()) + .idTokenSignedResponseAlgorithm(SignatureAlgorithm.RS256.getName()) + .scope("scope1") + .scope("scope2") + .registrationClientUrl("https://auth-server:9000/connect/register?client_id=client-id") + .build(); + return clientRegistration; + // @formatter:on + } + private static Jwt createJwt(String scope) { // @formatter:off JwsHeader jwsHeader = TestJwsHeaders.jwsHeader() From 11ce8ef201eda14a8e8e4d271deb07cf7addbebd Mon Sep 17 00:00:00 2001 From: Joe Grandja Date: Fri, 28 Oct 2022 18:04:17 -0400 Subject: [PATCH 5/8] Polish gh-929 --- docs/src/docs/asciidoc/protocol-endpoints.adoc | 2 +- .../web/configurers/OidcUserInfoEndpointConfigurer.java | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/src/docs/asciidoc/protocol-endpoints.adoc b/docs/src/docs/asciidoc/protocol-endpoints.adoc index 7a3222b3..dc0cf2a9 100644 --- a/docs/src/docs/asciidoc/protocol-endpoints.adoc +++ b/docs/src/docs/asciidoc/protocol-endpoints.adoc @@ -269,7 +269,7 @@ public SecurityFilterChain authorizationServerSecurityFilterChain(HttpSecurity h == OpenID Connect 1.0 UserInfo Endpoint `OidcUserInfoEndpointConfigurer` provides the ability to customize the https://openid.net/specs/openid-connect-core-1_0.html#UserInfo[OpenID Connect 1.0 UserInfo endpoint]. -It defines extension points that let you customize the https://openid.net/specs/openid-connect-core-1_0.html#UserInfoResponse[UserInfo response]. +It defines extension points that let you customize the pre-processing, main processing, and post-processing logic for https://openid.net/specs/openid-connect-core-1_0.html#UserInfoRequest[UserInfo requests]. `OidcUserInfoEndpointConfigurer` provides the following configuration options: diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcUserInfoEndpointConfigurer.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcUserInfoEndpointConfigurer.java index 26538044..6a4efb0d 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcUserInfoEndpointConfigurer.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcUserInfoEndpointConfigurer.java @@ -174,6 +174,7 @@ public final class OidcUserInfoEndpointConfigurer extends AbstractOAuth2Configur * * * @param userInfoMapper the {@link Function} used to extract claims from {@link OidcUserInfoAuthenticationContext} to an instance of {@link OidcUserInfo} + * @return the {@link OidcUserInfoEndpointConfigurer} for further configuration */ public OidcUserInfoEndpointConfigurer userInfoMapper( Function userInfoMapper) { From 356d669a78ea860f9e018791e21fc600eb30e4b5 Mon Sep 17 00:00:00 2001 From: Jonah Back Date: Mon, 24 Oct 2022 12:45:32 -0700 Subject: [PATCH 6/8] Fix URL encoding for authorization request state parameter Closes gh-875 --- .../OAuth2AuthorizationEndpointFilter.java | 23 +++++++++++----- .../TestOAuth2Authorizations.java | 27 ++++++++++++------- .../OAuth2AuthorizationCodeGrantTests.java | 26 ++++++++++-------- ...Auth2AuthorizationEndpointFilterTests.java | 21 ++++++++------- 4 files changed, 61 insertions(+), 36 deletions(-) diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilter.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilter.java index 6f1bc191..e11c43ba 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilter.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilter.java @@ -18,6 +18,7 @@ package org.springframework.security.oauth2.server.authorization.web; import java.io.IOException; import java.nio.charset.StandardCharsets; import java.util.Arrays; +import java.util.HashMap; import java.util.HashSet; import java.util.Set; @@ -251,11 +252,13 @@ public final class OAuth2AuthorizationEndpointFilter extends OncePerRequestFilte String state = authorizationConsentAuthentication.getState(); if (hasConsentUri()) { - String redirectUri = UriComponentsBuilder.fromUriString(resolveConsentUri(request)) + UriComponentsBuilder uriBuilder = UriComponentsBuilder.fromUriString(resolveConsentUri(request)) .queryParam(OAuth2ParameterNames.SCOPE, String.join(" ", requestedScopes)) .queryParam(OAuth2ParameterNames.CLIENT_ID, clientId) - .queryParam(OAuth2ParameterNames.STATE, state) - .toUriString(); + .queryParam(OAuth2ParameterNames.STATE, "{state}"); + HashMap queryParameters = new HashMap<>(1); + queryParameters.put(OAuth2ParameterNames.STATE, state); + String redirectUri = uriBuilder.build(queryParameters).toString(); this.redirectStrategy.sendRedirect(request, response, redirectUri); } else { DefaultConsentPage.displayConsent(request, response, clientId, principal, requestedScopes, authorizedScopes, state); @@ -288,9 +291,12 @@ public final class OAuth2AuthorizationEndpointFilter extends OncePerRequestFilte .fromUriString(authorizationCodeRequestAuthentication.getRedirectUri()) .queryParam(OAuth2ParameterNames.CODE, authorizationCodeRequestAuthentication.getAuthorizationCode().getTokenValue()); if (StringUtils.hasText(authorizationCodeRequestAuthentication.getState())) { - uriBuilder.queryParam(OAuth2ParameterNames.STATE, authorizationCodeRequestAuthentication.getState()); + uriBuilder.queryParam(OAuth2ParameterNames.STATE, "{state}"); } - this.redirectStrategy.sendRedirect(request, response, uriBuilder.toUriString()); + HashMap queryParams = new HashMap<>(); + queryParams.put(OAuth2ParameterNames.STATE, authorizationCodeRequestAuthentication.getState()); + String redirectUri = uriBuilder.build(queryParams).toString(); + this.redirectStrategy.sendRedirect(request, response, redirectUri); } private void sendErrorResponse(HttpServletRequest request, HttpServletResponse response, @@ -318,9 +324,12 @@ public final class OAuth2AuthorizationEndpointFilter extends OncePerRequestFilte uriBuilder.queryParam(OAuth2ParameterNames.ERROR_URI, error.getUri()); } if (StringUtils.hasText(authorizationCodeRequestAuthentication.getState())) { - uriBuilder.queryParam(OAuth2ParameterNames.STATE, authorizationCodeRequestAuthentication.getState()); + uriBuilder.queryParam(OAuth2ParameterNames.STATE, "{state}"); } - this.redirectStrategy.sendRedirect(request, response, uriBuilder.toUriString()); + HashMap queryParams = new HashMap<>(); + queryParams.put(OAuth2ParameterNames.STATE, authorizationCodeRequestAuthentication.getState()); + String redirectUri = uriBuilder.build(queryParams).toString(); + this.redirectStrategy.sendRedirect(request, response, redirectUri); } /** diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/TestOAuth2Authorizations.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/TestOAuth2Authorizations.java index 2d95add5..58031b64 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/TestOAuth2Authorizations.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/TestOAuth2Authorizations.java @@ -39,44 +39,53 @@ import org.springframework.util.CollectionUtils; public class TestOAuth2Authorizations { public static OAuth2Authorization.Builder authorization() { - return authorization(TestRegisteredClients.registeredClient().build()); + return authorization(TestRegisteredClients.registeredClient().build(), "state"); + } + + public static OAuth2Authorization.Builder authorization(RegisteredClient registeredClient, String state) { + return authorization(registeredClient, Collections.emptyMap(), state); } public static OAuth2Authorization.Builder authorization(RegisteredClient registeredClient) { - return authorization(registeredClient, Collections.emptyMap()); + return authorization(registeredClient, Collections.emptyMap(), "state"); } public static OAuth2Authorization.Builder authorization(RegisteredClient registeredClient, - Map authorizationRequestAdditionalParameters) { + Map authorizationRequestAdditionalParameters, String state) { OAuth2AuthorizationCode authorizationCode = new OAuth2AuthorizationCode( "code", Instant.now(), Instant.now().plusSeconds(120)); OAuth2AccessToken accessToken = new OAuth2AccessToken( OAuth2AccessToken.TokenType.BEARER, "access-token", Instant.now(), Instant.now().plusSeconds(300)); - return authorization(registeredClient, authorizationCode, accessToken, Collections.emptyMap(), authorizationRequestAdditionalParameters); + return authorization(registeredClient, authorizationCode, accessToken, Collections.emptyMap(), authorizationRequestAdditionalParameters, state); + } + + public static OAuth2Authorization.Builder authorization(RegisteredClient registeredClient, + Map authorizationRequestAdditionalParameters) { + return authorization(registeredClient, authorizationRequestAdditionalParameters, "state"); } public static OAuth2Authorization.Builder authorization(RegisteredClient registeredClient, OAuth2AuthorizationCode authorizationCode) { - return authorization(registeredClient, authorizationCode, null, Collections.emptyMap(), Collections.emptyMap()); + return authorization(registeredClient, authorizationCode, null, Collections.emptyMap(), Collections.emptyMap(), "state"); } public static OAuth2Authorization.Builder authorization(RegisteredClient registeredClient, OAuth2AccessToken accessToken, Map accessTokenClaims) { OAuth2AuthorizationCode authorizationCode = new OAuth2AuthorizationCode( "code", Instant.now(), Instant.now().plusSeconds(120)); - return authorization(registeredClient, authorizationCode, accessToken, accessTokenClaims, Collections.emptyMap()); + return authorization(registeredClient, authorizationCode, accessToken, accessTokenClaims, Collections.emptyMap(), "state"); } private static OAuth2Authorization.Builder authorization(RegisteredClient registeredClient, OAuth2AuthorizationCode authorizationCode, OAuth2AccessToken accessToken, - Map accessTokenClaims, Map authorizationRequestAdditionalParameters) { + Map accessTokenClaims, Map authorizationRequestAdditionalParameters, String state) { OAuth2AuthorizationRequest authorizationRequest = OAuth2AuthorizationRequest.authorizationCode() .authorizationUri("https://provider.com/oauth2/authorize") .clientId(registeredClient.getClientId()) .redirectUri(registeredClient.getRedirectUris().iterator().next()) .scopes(registeredClient.getScopes()) .additionalParameters(authorizationRequestAdditionalParameters) - .state("state") + .state(state) .build(); OAuth2Authorization.Builder builder = OAuth2Authorization.withRegisteredClient(registeredClient) .id("id") @@ -84,7 +93,7 @@ public class TestOAuth2Authorizations { .authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE) .authorizedScopes(authorizationRequest.getScopes()) .token(authorizationCode) - .attribute(OAuth2ParameterNames.STATE, "state") + .attribute(OAuth2ParameterNames.STATE, state) .attribute(OAuth2AuthorizationRequest.class.getName(), authorizationRequest) .attribute(Principal.class.getName(), new TestingAuthenticationToken("principal", null, "ROLE_A", "ROLE_B")); diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2AuthorizationCodeGrantTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2AuthorizationCodeGrantTests.java index fcd79619..adef041d 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2AuthorizationCodeGrantTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2AuthorizationCodeGrantTests.java @@ -159,6 +159,9 @@ public class OAuth2AuthorizationCodeGrantTests { private static final String S256_CODE_VERIFIER = "dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk"; private static final String S256_CODE_CHALLENGE = "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM"; private static final String AUTHORITIES_CLAIM = "authorities"; + private static final String STATE_URL_UNENCODED = "awrD0fCnEcTUPFgmyy2SU89HZNcnAJ60ZW6l39YI0KyVjmIZ+004pwm9j55li7BoydXYysH4enZMF21Q"; + private static final String STATE_URL_ENCODED = "awrD0fCnEcTUPFgmyy2SU89HZNcnAJ60ZW6l39YI0KyVjmIZ%2B004pwm9j55li7BoydXYysH4enZMF21Q"; + private static final OAuth2TokenType AUTHORIZATION_CODE_TOKEN_TYPE = new OAuth2TokenType(OAuth2ParameterNames.CODE); private static final OAuth2TokenType STATE_TOKEN_TYPE = new OAuth2TokenType(OAuth2ParameterNames.STATE); @@ -290,7 +293,7 @@ public class OAuth2AuthorizationCodeGrantTests { .andExpect(status().is3xxRedirection()) .andReturn(); String redirectedUrl = mvcResult.getResponse().getRedirectedUrl(); - assertThat(redirectedUrl).matches("https://example.com\\?code=.{15,}&state=state"); + assertThat(redirectedUrl).matches("https://example.com\\?code=.{15,}&state="+STATE_URL_ENCODED); String authorizationCode = extractParameterFromRedirectUri(redirectedUrl, "code"); OAuth2Authorization authorization = this.authorizationService.findByToken(authorizationCode, AUTHORIZATION_CODE_TOKEN_TYPE); @@ -382,7 +385,7 @@ public class OAuth2AuthorizationCodeGrantTests { .andExpect(status().is3xxRedirection()) .andReturn(); String redirectedUrl = mvcResult.getResponse().getRedirectedUrl(); - assertThat(redirectedUrl).matches("https://example.com\\?code=.{15,}&state=state"); + assertThat(redirectedUrl).matches("https://example.com\\?code=.{15,}&state=" + STATE_URL_ENCODED); String authorizationCode = extractParameterFromRedirectUri(redirectedUrl, "code"); OAuth2Authorization authorizationCodeAuthorization = this.authorizationService.findByToken(authorizationCode, AUTHORIZATION_CODE_TOKEN_TYPE); @@ -426,7 +429,7 @@ public class OAuth2AuthorizationCodeGrantTests { .andExpect(status().is3xxRedirection()) .andReturn(); String redirectedUrl = mvcResult.getResponse().getRedirectedUrl(); - assertThat(redirectedUrl).matches("https://example.com\\?code=.{15,}&state=state"); + assertThat(redirectedUrl).matches("https://example.com\\?code=.{15,}&state=" + STATE_URL_ENCODED); String authorizationCode = extractParameterFromRedirectUri(redirectedUrl, "code"); OAuth2Authorization authorizationCodeAuthorization = this.authorizationService.findByToken(authorizationCode, AUTHORIZATION_CODE_TOKEN_TYPE); @@ -499,8 +502,9 @@ public class OAuth2AuthorizationCodeGrantTests { .build(); this.registeredClientRepository.save(registeredClient); - OAuth2Authorization authorization = TestOAuth2Authorizations.authorization(registeredClient) + OAuth2Authorization authorization = TestOAuth2Authorizations.authorization(registeredClient, STATE_URL_UNENCODED) .principalName("user") + .attribute(OAuth2ParameterNames.STATE, STATE_URL_UNENCODED) .build(); this.authorizationService.save(authorization); @@ -508,13 +512,13 @@ public class OAuth2AuthorizationCodeGrantTests { .param(OAuth2ParameterNames.CLIENT_ID, registeredClient.getClientId()) .param(OAuth2ParameterNames.SCOPE, "message.read") .param(OAuth2ParameterNames.SCOPE, "message.write") - .param(OAuth2ParameterNames.STATE, "state") + .param(OAuth2ParameterNames.STATE, STATE_URL_UNENCODED) .with(user("user"))) .andExpect(status().is3xxRedirection()) .andReturn(); String redirectedUrl = mvcResult.getResponse().getRedirectedUrl(); - assertThat(redirectedUrl).matches("https://example.com\\?code=.{15,}&state=state"); + assertThat(redirectedUrl).matches("https://example.com\\?code=.{15,}&state=" + STATE_URL_ENCODED); String authorizationCode = extractParameterFromRedirectUri(redirectedUrl, "code"); OAuth2Authorization authorizationCodeAuthorization = this.authorizationService.findByToken(authorizationCode, AUTHORIZATION_CODE_TOKEN_TYPE); @@ -580,20 +584,20 @@ public class OAuth2AuthorizationCodeGrantTests { .build(); this.registeredClientRepository.save(registeredClient); - OAuth2Authorization authorization = TestOAuth2Authorizations.authorization(registeredClient) + OAuth2Authorization authorization = TestOAuth2Authorizations.authorization(registeredClient, STATE_URL_UNENCODED) .build(); this.authorizationService.save(authorization); MvcResult mvcResult = this.mvc.perform(post(DEFAULT_AUTHORIZATION_ENDPOINT_URI) .param(OAuth2ParameterNames.CLIENT_ID, registeredClient.getClientId()) .param("authority", "authority-1 authority-2") - .param(OAuth2ParameterNames.STATE, "state") + .param(OAuth2ParameterNames.STATE, STATE_URL_UNENCODED) .with(user("principal"))) .andExpect(status().is3xxRedirection()) .andReturn(); String redirectedUrl = mvcResult.getResponse().getRedirectedUrl(); - assertThat(redirectedUrl).matches("https://example.com\\?code=.{15,}&state=state"); + assertThat(redirectedUrl).matches("https://example.com\\?code=.{15,}&state=" + STATE_URL_ENCODED); String authorizationCode = extractParameterFromRedirectUri(redirectedUrl, "code"); OAuth2Authorization authorizationCodeAuthorization = this.authorizationService.findByToken(authorizationCode, AUTHORIZATION_CODE_TOKEN_TYPE); @@ -631,7 +635,7 @@ public class OAuth2AuthorizationCodeGrantTests { OAuth2AuthorizationCodeRequestAuthenticationToken authorizationCodeRequestAuthenticationResult = new OAuth2AuthorizationCodeRequestAuthenticationToken( "https://provider.com/oauth2/authorize", registeredClient.getClientId(), principal, authorizationCode, - registeredClient.getRedirectUris().iterator().next(), "state", registeredClient.getScopes()); + registeredClient.getRedirectUris().iterator().next(), STATE_URL_UNENCODED, registeredClient.getScopes()); when(authorizationRequestConverter.convert(any())).thenReturn(authorizationCodeRequestAuthenticationResult); when(authorizationRequestAuthenticationProvider.supports(eq(OAuth2AuthorizationCodeRequestAuthenticationToken.class))).thenReturn(true); when(authorizationRequestAuthenticationProvider.authenticate(any())).thenReturn(authorizationCodeRequestAuthenticationResult); @@ -718,7 +722,7 @@ public class OAuth2AuthorizationCodeGrantTests { parameters.set(OAuth2ParameterNames.REDIRECT_URI, registeredClient.getRedirectUris().iterator().next()); parameters.set(OAuth2ParameterNames.SCOPE, StringUtils.collectionToDelimitedString(registeredClient.getScopes(), " ")); - parameters.set(OAuth2ParameterNames.STATE, "state"); + parameters.set(OAuth2ParameterNames.STATE, STATE_URL_UNENCODED); return parameters; } diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilterTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilterTests.java index e4038667..2736de26 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilterTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilterTests.java @@ -85,6 +85,9 @@ public class OAuth2AuthorizationEndpointFilterTests { private static final String AUTHORIZATION_URI = "https://provider.com/oauth2/authorize"; private static final String STATE = "state"; private static final String REMOTE_ADDRESS = "remote-address"; + private static final String STATE_URL_UNENCODED = "awrD0fCnEcTUPFgmyy2SU89HZNcnAJ60ZW6l39YI0KyVjmIZ+004pwm9j55li7BoydXYysH4enZMF21Q"; + private static final String STATE_URL_ENCODED = "awrD0fCnEcTUPFgmyy2SU89HZNcnAJ60ZW6l39YI0KyVjmIZ%2B004pwm9j55li7BoydXYysH4enZMF21Q"; + private AuthenticationManager authenticationManager; private OAuth2AuthorizationEndpointFilter filter; private TestingAuthenticationToken principal; @@ -284,7 +287,7 @@ public class OAuth2AuthorizationEndpointFilterTests { OAuth2AuthorizationCodeRequestAuthenticationToken authorizationCodeRequestAuthentication = new OAuth2AuthorizationCodeRequestAuthenticationToken( AUTHORIZATION_URI, registeredClient.getClientId(), principal, - registeredClient.getRedirectUris().iterator().next(), STATE, registeredClient.getScopes(), null); + registeredClient.getRedirectUris().iterator().next(), STATE_URL_UNENCODED, registeredClient.getScopes(), null); OAuth2Error error = new OAuth2Error("errorCode", "errorDescription", "errorUri"); when(this.authenticationManager.authenticate(any())) .thenThrow(new OAuth2AuthorizationCodeRequestAuthenticationException(error, authorizationCodeRequestAuthentication)); @@ -299,7 +302,7 @@ public class OAuth2AuthorizationEndpointFilterTests { verifyNoInteractions(filterChain); assertThat(response.getStatus()).isEqualTo(HttpStatus.FOUND.value()); - assertThat(response.getRedirectedUrl()).isEqualTo("https://example.com?error=errorCode&error_description=errorDescription&error_uri=errorUri&state=state"); + assertThat(response.getRedirectedUrl()).isEqualTo("https://example.com?error=errorCode&error_description=errorDescription&error_uri=errorUri&state=" + STATE_URL_ENCODED); assertThat(SecurityContextHolder.getContext().getAuthentication()).isSameAs(this.principal); } @@ -443,7 +446,7 @@ public class OAuth2AuthorizationEndpointFilterTests { OAuth2AuthorizationConsentAuthenticationToken authorizationConsentAuthenticationResult = new OAuth2AuthorizationConsentAuthenticationToken( AUTHORIZATION_URI, registeredClient.getClientId(), principal, - STATE, new HashSet<>(), null); // No scopes previously approved + STATE_URL_UNENCODED, new HashSet<>(), null); // No scopes previously approved authorizationConsentAuthenticationResult.setAuthenticated(true); when(this.authenticationManager.authenticate(any())) .thenReturn(authorizationConsentAuthenticationResult); @@ -459,7 +462,7 @@ public class OAuth2AuthorizationEndpointFilterTests { verifyNoInteractions(filterChain); assertThat(response.getStatus()).isEqualTo(HttpStatus.FOUND.value()); - assertThat(response.getRedirectedUrl()).isEqualTo("http://localhost/oauth2/custom-consent?scope=scope1%20scope2&client_id=client-1&state=state"); + assertThat(response.getRedirectedUrl()).isEqualTo("http://localhost/oauth2/custom-consent?scope=scope1%20scope2&client_id=client-1&state=" + STATE_URL_ENCODED); } @Test @@ -539,7 +542,7 @@ public class OAuth2AuthorizationEndpointFilterTests { OAuth2AuthorizationCodeRequestAuthenticationToken authorizationCodeRequestAuthenticationResult = new OAuth2AuthorizationCodeRequestAuthenticationToken( AUTHORIZATION_URI, registeredClient.getClientId(), principal, this.authorizationCode, - registeredClient.getRedirectUris().iterator().next(), STATE, registeredClient.getScopes()); + registeredClient.getRedirectUris().iterator().next(), STATE_URL_UNENCODED, registeredClient.getScopes()); authorizationCodeRequestAuthenticationResult.setAuthenticated(true); when(this.authenticationManager.authenticate(any())) .thenReturn(authorizationCodeRequestAuthenticationResult); @@ -560,7 +563,7 @@ public class OAuth2AuthorizationEndpointFilterTests { .extracting(WebAuthenticationDetails::getRemoteAddress) .isEqualTo(REMOTE_ADDRESS); assertThat(response.getStatus()).isEqualTo(HttpStatus.FOUND.value()); - assertThat(response.getRedirectedUrl()).isEqualTo("https://example.com?code=code&state=state"); + assertThat(response.getRedirectedUrl()).isEqualTo("https://example.com?code=code&state=" + STATE_URL_ENCODED); } @Test @@ -575,7 +578,7 @@ public class OAuth2AuthorizationEndpointFilterTests { OAuth2AuthorizationCodeRequestAuthenticationToken authorizationCodeRequestAuthenticationResult = new OAuth2AuthorizationCodeRequestAuthenticationToken( AUTHORIZATION_URI, registeredClient.getClientId(), principal, this.authorizationCode, - registeredClient.getRedirectUris().iterator().next(), STATE, registeredClient.getScopes()); + registeredClient.getRedirectUris().iterator().next(), STATE_URL_UNENCODED, registeredClient.getScopes()); authorizationCodeRequestAuthenticationResult.setAuthenticated(true); when(this.authenticationManager.authenticate(any())) .thenReturn(authorizationCodeRequestAuthenticationResult); @@ -591,7 +594,7 @@ public class OAuth2AuthorizationEndpointFilterTests { verifyNoInteractions(filterChain); assertThat(response.getStatus()).isEqualTo(HttpStatus.FOUND.value()); - assertThat(response.getRedirectedUrl()).isEqualTo("https://example.com?code=code&state=state"); + assertThat(response.getRedirectedUrl()).isEqualTo("https://example.com?code=code&state=" + STATE_URL_ENCODED); } private void doFilterWhenAuthorizationRequestInvalidParameterThenError(RegisteredClient registeredClient, @@ -634,7 +637,7 @@ public class OAuth2AuthorizationEndpointFilterTests { request.addParameter(OAuth2ParameterNames.REDIRECT_URI, registeredClient.getRedirectUris().iterator().next()); request.addParameter(OAuth2ParameterNames.SCOPE, StringUtils.collectionToDelimitedString(registeredClient.getScopes(), " ")); - request.addParameter(OAuth2ParameterNames.STATE, "state"); + request.addParameter(OAuth2ParameterNames.STATE, STATE_URL_UNENCODED); return request; } From 4eb25c163f0e26403a9381f23e9863f2317ecd13 Mon Sep 17 00:00:00 2001 From: Joe Grandja Date: Mon, 31 Oct 2022 11:36:56 -0400 Subject: [PATCH 7/8] Polish gh-920 --- .../OAuth2AuthorizationEndpointFilter.java | 27 ++++++++------- .../TestOAuth2Authorizations.java | 33 +++++++------------ .../OAuth2AuthorizationCodeGrantTests.java | 28 ++++++++++++---- ...Auth2AuthorizationEndpointFilterTests.java | 21 +++++------- 4 files changed, 59 insertions(+), 50 deletions(-) diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilter.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilter.java index e11c43ba..3e507fa1 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilter.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilter.java @@ -20,6 +20,7 @@ import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.HashMap; import java.util.HashSet; +import java.util.Map; import java.util.Set; import javax.servlet.FilterChain; @@ -252,13 +253,11 @@ public final class OAuth2AuthorizationEndpointFilter extends OncePerRequestFilte String state = authorizationConsentAuthentication.getState(); if (hasConsentUri()) { - UriComponentsBuilder uriBuilder = UriComponentsBuilder.fromUriString(resolveConsentUri(request)) + String redirectUri = UriComponentsBuilder.fromUriString(resolveConsentUri(request)) .queryParam(OAuth2ParameterNames.SCOPE, String.join(" ", requestedScopes)) .queryParam(OAuth2ParameterNames.CLIENT_ID, clientId) - .queryParam(OAuth2ParameterNames.STATE, "{state}"); - HashMap queryParameters = new HashMap<>(1); - queryParameters.put(OAuth2ParameterNames.STATE, state); - String redirectUri = uriBuilder.build(queryParameters).toString(); + .queryParam(OAuth2ParameterNames.STATE, state) + .toUriString(); this.redirectStrategy.sendRedirect(request, response, redirectUri); } else { DefaultConsentPage.displayConsent(request, response, clientId, principal, requestedScopes, authorizedScopes, state); @@ -290,12 +289,15 @@ public final class OAuth2AuthorizationEndpointFilter extends OncePerRequestFilte UriComponentsBuilder uriBuilder = UriComponentsBuilder .fromUriString(authorizationCodeRequestAuthentication.getRedirectUri()) .queryParam(OAuth2ParameterNames.CODE, authorizationCodeRequestAuthentication.getAuthorizationCode().getTokenValue()); + String redirectUri; if (StringUtils.hasText(authorizationCodeRequestAuthentication.getState())) { uriBuilder.queryParam(OAuth2ParameterNames.STATE, "{state}"); + Map queryParams = new HashMap<>(); + queryParams.put(OAuth2ParameterNames.STATE, authorizationCodeRequestAuthentication.getState()); + redirectUri = uriBuilder.build(queryParams).toString(); + } else { + redirectUri = uriBuilder.toUriString(); } - HashMap queryParams = new HashMap<>(); - queryParams.put(OAuth2ParameterNames.STATE, authorizationCodeRequestAuthentication.getState()); - String redirectUri = uriBuilder.build(queryParams).toString(); this.redirectStrategy.sendRedirect(request, response, redirectUri); } @@ -323,12 +325,15 @@ public final class OAuth2AuthorizationEndpointFilter extends OncePerRequestFilte if (StringUtils.hasText(error.getUri())) { uriBuilder.queryParam(OAuth2ParameterNames.ERROR_URI, error.getUri()); } + String redirectUri; if (StringUtils.hasText(authorizationCodeRequestAuthentication.getState())) { uriBuilder.queryParam(OAuth2ParameterNames.STATE, "{state}"); + Map queryParams = new HashMap<>(); + queryParams.put(OAuth2ParameterNames.STATE, authorizationCodeRequestAuthentication.getState()); + redirectUri = uriBuilder.build(queryParams).toString(); + } else { + redirectUri = uriBuilder.toUriString(); } - HashMap queryParams = new HashMap<>(); - queryParams.put(OAuth2ParameterNames.STATE, authorizationCodeRequestAuthentication.getState()); - String redirectUri = uriBuilder.build(queryParams).toString(); this.redirectStrategy.sendRedirect(request, response, redirectUri); } diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/TestOAuth2Authorizations.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/TestOAuth2Authorizations.java index 58031b64..fce02bd8 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/TestOAuth2Authorizations.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/TestOAuth2Authorizations.java @@ -39,53 +39,44 @@ import org.springframework.util.CollectionUtils; public class TestOAuth2Authorizations { public static OAuth2Authorization.Builder authorization() { - return authorization(TestRegisteredClients.registeredClient().build(), "state"); - } - - public static OAuth2Authorization.Builder authorization(RegisteredClient registeredClient, String state) { - return authorization(registeredClient, Collections.emptyMap(), state); + return authorization(TestRegisteredClients.registeredClient().build()); } public static OAuth2Authorization.Builder authorization(RegisteredClient registeredClient) { - return authorization(registeredClient, Collections.emptyMap(), "state"); - } - - public static OAuth2Authorization.Builder authorization(RegisteredClient registeredClient, - Map authorizationRequestAdditionalParameters, String state) { - OAuth2AuthorizationCode authorizationCode = new OAuth2AuthorizationCode( - "code", Instant.now(), Instant.now().plusSeconds(120)); - OAuth2AccessToken accessToken = new OAuth2AccessToken( - OAuth2AccessToken.TokenType.BEARER, "access-token", Instant.now(), Instant.now().plusSeconds(300)); - return authorization(registeredClient, authorizationCode, accessToken, Collections.emptyMap(), authorizationRequestAdditionalParameters, state); + return authorization(registeredClient, Collections.emptyMap()); } public static OAuth2Authorization.Builder authorization(RegisteredClient registeredClient, Map authorizationRequestAdditionalParameters) { - return authorization(registeredClient, authorizationRequestAdditionalParameters, "state"); + OAuth2AuthorizationCode authorizationCode = new OAuth2AuthorizationCode( + "code", Instant.now(), Instant.now().plusSeconds(120)); + OAuth2AccessToken accessToken = new OAuth2AccessToken( + OAuth2AccessToken.TokenType.BEARER, "access-token", Instant.now(), Instant.now().plusSeconds(300)); + return authorization(registeredClient, authorizationCode, accessToken, Collections.emptyMap(), authorizationRequestAdditionalParameters); } public static OAuth2Authorization.Builder authorization(RegisteredClient registeredClient, OAuth2AuthorizationCode authorizationCode) { - return authorization(registeredClient, authorizationCode, null, Collections.emptyMap(), Collections.emptyMap(), "state"); + return authorization(registeredClient, authorizationCode, null, Collections.emptyMap(), Collections.emptyMap()); } public static OAuth2Authorization.Builder authorization(RegisteredClient registeredClient, OAuth2AccessToken accessToken, Map accessTokenClaims) { OAuth2AuthorizationCode authorizationCode = new OAuth2AuthorizationCode( "code", Instant.now(), Instant.now().plusSeconds(120)); - return authorization(registeredClient, authorizationCode, accessToken, accessTokenClaims, Collections.emptyMap(), "state"); + return authorization(registeredClient, authorizationCode, accessToken, accessTokenClaims, Collections.emptyMap()); } private static OAuth2Authorization.Builder authorization(RegisteredClient registeredClient, OAuth2AuthorizationCode authorizationCode, OAuth2AccessToken accessToken, - Map accessTokenClaims, Map authorizationRequestAdditionalParameters, String state) { + Map accessTokenClaims, Map authorizationRequestAdditionalParameters) { OAuth2AuthorizationRequest authorizationRequest = OAuth2AuthorizationRequest.authorizationCode() .authorizationUri("https://provider.com/oauth2/authorize") .clientId(registeredClient.getClientId()) .redirectUri(registeredClient.getRedirectUris().iterator().next()) .scopes(registeredClient.getScopes()) .additionalParameters(authorizationRequestAdditionalParameters) - .state(state) + .state("state") .build(); OAuth2Authorization.Builder builder = OAuth2Authorization.withRegisteredClient(registeredClient) .id("id") @@ -93,7 +84,7 @@ public class TestOAuth2Authorizations { .authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE) .authorizedScopes(authorizationRequest.getScopes()) .token(authorizationCode) - .attribute(OAuth2ParameterNames.STATE, state) + .attribute(OAuth2ParameterNames.STATE, "consent-state") .attribute(OAuth2AuthorizationRequest.class.getName(), authorizationRequest) .attribute(Principal.class.getName(), new TestingAuthenticationToken("principal", null, "ROLE_A", "ROLE_B")); diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2AuthorizationCodeGrantTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2AuthorizationCodeGrantTests.java index adef041d..e65ab9cd 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2AuthorizationCodeGrantTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2AuthorizationCodeGrantTests.java @@ -69,6 +69,7 @@ import org.springframework.security.crypto.password.PasswordEncoder; import org.springframework.security.oauth2.core.AuthorizationGrantType; import org.springframework.security.oauth2.core.OAuth2Token; import org.springframework.security.oauth2.core.endpoint.OAuth2AccessTokenResponse; +import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest; import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationResponseType; import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames; import org.springframework.security.oauth2.core.endpoint.PkceParameterNames; @@ -293,7 +294,7 @@ public class OAuth2AuthorizationCodeGrantTests { .andExpect(status().is3xxRedirection()) .andReturn(); String redirectedUrl = mvcResult.getResponse().getRedirectedUrl(); - assertThat(redirectedUrl).matches("https://example.com\\?code=.{15,}&state="+STATE_URL_ENCODED); + assertThat(redirectedUrl).matches("https://example.com\\?code=.{15,}&state=" + STATE_URL_ENCODED); String authorizationCode = extractParameterFromRedirectUri(redirectedUrl, "code"); OAuth2Authorization authorization = this.authorizationService.findByToken(authorizationCode, AUTHORIZATION_CODE_TOKEN_TYPE); @@ -502,9 +503,16 @@ public class OAuth2AuthorizationCodeGrantTests { .build(); this.registeredClientRepository.save(registeredClient); - OAuth2Authorization authorization = TestOAuth2Authorizations.authorization(registeredClient, STATE_URL_UNENCODED) + OAuth2Authorization authorization = TestOAuth2Authorizations.authorization(registeredClient) .principalName("user") - .attribute(OAuth2ParameterNames.STATE, STATE_URL_UNENCODED) + .build(); + OAuth2AuthorizationRequest authorizationRequest = authorization.getAttribute(OAuth2AuthorizationRequest.class.getName()); + OAuth2AuthorizationRequest updatedAuthorizationRequest = + OAuth2AuthorizationRequest.from(authorizationRequest) + .state(STATE_URL_UNENCODED) + .build(); + authorization = OAuth2Authorization.from(authorization) + .attribute(OAuth2AuthorizationRequest.class.getName(), updatedAuthorizationRequest) .build(); this.authorizationService.save(authorization); @@ -512,7 +520,7 @@ public class OAuth2AuthorizationCodeGrantTests { .param(OAuth2ParameterNames.CLIENT_ID, registeredClient.getClientId()) .param(OAuth2ParameterNames.SCOPE, "message.read") .param(OAuth2ParameterNames.SCOPE, "message.write") - .param(OAuth2ParameterNames.STATE, STATE_URL_UNENCODED) + .param(OAuth2ParameterNames.STATE, authorization.getAttribute(OAuth2ParameterNames.STATE)) .with(user("user"))) .andExpect(status().is3xxRedirection()) .andReturn(); @@ -584,14 +592,22 @@ public class OAuth2AuthorizationCodeGrantTests { .build(); this.registeredClientRepository.save(registeredClient); - OAuth2Authorization authorization = TestOAuth2Authorizations.authorization(registeredClient, STATE_URL_UNENCODED) + OAuth2Authorization authorization = TestOAuth2Authorizations.authorization(registeredClient) + .build(); + OAuth2AuthorizationRequest authorizationRequest = authorization.getAttribute(OAuth2AuthorizationRequest.class.getName()); + OAuth2AuthorizationRequest updatedAuthorizationRequest = + OAuth2AuthorizationRequest.from(authorizationRequest) + .state(STATE_URL_UNENCODED) + .build(); + authorization = OAuth2Authorization.from(authorization) + .attribute(OAuth2AuthorizationRequest.class.getName(), updatedAuthorizationRequest) .build(); this.authorizationService.save(authorization); MvcResult mvcResult = this.mvc.perform(post(DEFAULT_AUTHORIZATION_ENDPOINT_URI) .param(OAuth2ParameterNames.CLIENT_ID, registeredClient.getClientId()) .param("authority", "authority-1 authority-2") - .param(OAuth2ParameterNames.STATE, STATE_URL_UNENCODED) + .param(OAuth2ParameterNames.STATE, authorization.getAttribute(OAuth2ParameterNames.STATE)) .with(user("principal"))) .andExpect(status().is3xxRedirection()) .andReturn(); diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilterTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilterTests.java index 2736de26..e4038667 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilterTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilterTests.java @@ -85,9 +85,6 @@ public class OAuth2AuthorizationEndpointFilterTests { private static final String AUTHORIZATION_URI = "https://provider.com/oauth2/authorize"; private static final String STATE = "state"; private static final String REMOTE_ADDRESS = "remote-address"; - private static final String STATE_URL_UNENCODED = "awrD0fCnEcTUPFgmyy2SU89HZNcnAJ60ZW6l39YI0KyVjmIZ+004pwm9j55li7BoydXYysH4enZMF21Q"; - private static final String STATE_URL_ENCODED = "awrD0fCnEcTUPFgmyy2SU89HZNcnAJ60ZW6l39YI0KyVjmIZ%2B004pwm9j55li7BoydXYysH4enZMF21Q"; - private AuthenticationManager authenticationManager; private OAuth2AuthorizationEndpointFilter filter; private TestingAuthenticationToken principal; @@ -287,7 +284,7 @@ public class OAuth2AuthorizationEndpointFilterTests { OAuth2AuthorizationCodeRequestAuthenticationToken authorizationCodeRequestAuthentication = new OAuth2AuthorizationCodeRequestAuthenticationToken( AUTHORIZATION_URI, registeredClient.getClientId(), principal, - registeredClient.getRedirectUris().iterator().next(), STATE_URL_UNENCODED, registeredClient.getScopes(), null); + registeredClient.getRedirectUris().iterator().next(), STATE, registeredClient.getScopes(), null); OAuth2Error error = new OAuth2Error("errorCode", "errorDescription", "errorUri"); when(this.authenticationManager.authenticate(any())) .thenThrow(new OAuth2AuthorizationCodeRequestAuthenticationException(error, authorizationCodeRequestAuthentication)); @@ -302,7 +299,7 @@ public class OAuth2AuthorizationEndpointFilterTests { verifyNoInteractions(filterChain); assertThat(response.getStatus()).isEqualTo(HttpStatus.FOUND.value()); - assertThat(response.getRedirectedUrl()).isEqualTo("https://example.com?error=errorCode&error_description=errorDescription&error_uri=errorUri&state=" + STATE_URL_ENCODED); + assertThat(response.getRedirectedUrl()).isEqualTo("https://example.com?error=errorCode&error_description=errorDescription&error_uri=errorUri&state=state"); assertThat(SecurityContextHolder.getContext().getAuthentication()).isSameAs(this.principal); } @@ -446,7 +443,7 @@ public class OAuth2AuthorizationEndpointFilterTests { OAuth2AuthorizationConsentAuthenticationToken authorizationConsentAuthenticationResult = new OAuth2AuthorizationConsentAuthenticationToken( AUTHORIZATION_URI, registeredClient.getClientId(), principal, - STATE_URL_UNENCODED, new HashSet<>(), null); // No scopes previously approved + STATE, new HashSet<>(), null); // No scopes previously approved authorizationConsentAuthenticationResult.setAuthenticated(true); when(this.authenticationManager.authenticate(any())) .thenReturn(authorizationConsentAuthenticationResult); @@ -462,7 +459,7 @@ public class OAuth2AuthorizationEndpointFilterTests { verifyNoInteractions(filterChain); assertThat(response.getStatus()).isEqualTo(HttpStatus.FOUND.value()); - assertThat(response.getRedirectedUrl()).isEqualTo("http://localhost/oauth2/custom-consent?scope=scope1%20scope2&client_id=client-1&state=" + STATE_URL_ENCODED); + assertThat(response.getRedirectedUrl()).isEqualTo("http://localhost/oauth2/custom-consent?scope=scope1%20scope2&client_id=client-1&state=state"); } @Test @@ -542,7 +539,7 @@ public class OAuth2AuthorizationEndpointFilterTests { OAuth2AuthorizationCodeRequestAuthenticationToken authorizationCodeRequestAuthenticationResult = new OAuth2AuthorizationCodeRequestAuthenticationToken( AUTHORIZATION_URI, registeredClient.getClientId(), principal, this.authorizationCode, - registeredClient.getRedirectUris().iterator().next(), STATE_URL_UNENCODED, registeredClient.getScopes()); + registeredClient.getRedirectUris().iterator().next(), STATE, registeredClient.getScopes()); authorizationCodeRequestAuthenticationResult.setAuthenticated(true); when(this.authenticationManager.authenticate(any())) .thenReturn(authorizationCodeRequestAuthenticationResult); @@ -563,7 +560,7 @@ public class OAuth2AuthorizationEndpointFilterTests { .extracting(WebAuthenticationDetails::getRemoteAddress) .isEqualTo(REMOTE_ADDRESS); assertThat(response.getStatus()).isEqualTo(HttpStatus.FOUND.value()); - assertThat(response.getRedirectedUrl()).isEqualTo("https://example.com?code=code&state=" + STATE_URL_ENCODED); + assertThat(response.getRedirectedUrl()).isEqualTo("https://example.com?code=code&state=state"); } @Test @@ -578,7 +575,7 @@ public class OAuth2AuthorizationEndpointFilterTests { OAuth2AuthorizationCodeRequestAuthenticationToken authorizationCodeRequestAuthenticationResult = new OAuth2AuthorizationCodeRequestAuthenticationToken( AUTHORIZATION_URI, registeredClient.getClientId(), principal, this.authorizationCode, - registeredClient.getRedirectUris().iterator().next(), STATE_URL_UNENCODED, registeredClient.getScopes()); + registeredClient.getRedirectUris().iterator().next(), STATE, registeredClient.getScopes()); authorizationCodeRequestAuthenticationResult.setAuthenticated(true); when(this.authenticationManager.authenticate(any())) .thenReturn(authorizationCodeRequestAuthenticationResult); @@ -594,7 +591,7 @@ public class OAuth2AuthorizationEndpointFilterTests { verifyNoInteractions(filterChain); assertThat(response.getStatus()).isEqualTo(HttpStatus.FOUND.value()); - assertThat(response.getRedirectedUrl()).isEqualTo("https://example.com?code=code&state=" + STATE_URL_ENCODED); + assertThat(response.getRedirectedUrl()).isEqualTo("https://example.com?code=code&state=state"); } private void doFilterWhenAuthorizationRequestInvalidParameterThenError(RegisteredClient registeredClient, @@ -637,7 +634,7 @@ public class OAuth2AuthorizationEndpointFilterTests { request.addParameter(OAuth2ParameterNames.REDIRECT_URI, registeredClient.getRedirectUris().iterator().next()); request.addParameter(OAuth2ParameterNames.SCOPE, StringUtils.collectionToDelimitedString(registeredClient.getScopes(), " ")); - request.addParameter(OAuth2ParameterNames.STATE, STATE_URL_UNENCODED); + request.addParameter(OAuth2ParameterNames.STATE, "state"); return request; } From 6dc3944eef3d73d9d76c863114f24ca7d1b1337c Mon Sep 17 00:00:00 2001 From: Joe Grandja Date: Mon, 31 Oct 2022 14:35:45 -0400 Subject: [PATCH 8/8] Add OidcClientRegistrationAuthenticationProvider.setRegisteredClientConverter() Closes gh-696 --- ...ntConfigurationAuthenticationProvider.java | 3 ++- ...entRegistrationAuthenticationProvider.java | 20 +++++++++++++++---- ...lientOidcClientRegistrationConverter.java} | 2 +- ...gistrationAuthenticationProviderTests.java | 7 +++++++ 4 files changed, 26 insertions(+), 6 deletions(-) rename oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/{OidcClientRegistrationConverter.java => RegisteredClientOidcClientRegistrationConverter.java} (96%) diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/OidcClientConfigurationAuthenticationProvider.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/OidcClientConfigurationAuthenticationProvider.java index 575e4593..5ce732b7 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/OidcClientConfigurationAuthenticationProvider.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/OidcClientConfigurationAuthenticationProvider.java @@ -46,6 +46,7 @@ import org.springframework.util.StringUtils; * @since 0.4.0 * @see RegisteredClientRepository * @see OAuth2AuthorizationService + * @see OidcClientRegistrationAuthenticationToken * @see OidcClientRegistrationAuthenticationProvider * @see 4. Client Configuration Endpoint */ @@ -67,7 +68,7 @@ public final class OidcClientConfigurationAuthenticationProvider implements Auth Assert.notNull(authorizationService, "authorizationService cannot be null"); this.registeredClientRepository = registeredClientRepository; this.authorizationService = authorizationService; - this.clientRegistrationConverter = new OidcClientRegistrationConverter(); + this.clientRegistrationConverter = new RegisteredClientOidcClientRegistrationConverter(); } @Override diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/OidcClientRegistrationAuthenticationProvider.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/OidcClientRegistrationAuthenticationProvider.java index e7ec6233..81e88c79 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/OidcClientRegistrationAuthenticationProvider.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/OidcClientRegistrationAuthenticationProvider.java @@ -74,6 +74,7 @@ import org.springframework.util.StringUtils; * @see RegisteredClientRepository * @see OAuth2AuthorizationService * @see OAuth2TokenGenerator + * @see OidcClientRegistrationAuthenticationToken * @see OidcClientConfigurationAuthenticationProvider * @see 3. Client Registration Endpoint */ @@ -84,7 +85,7 @@ public final class OidcClientRegistrationAuthenticationProvider implements Authe private final OAuth2AuthorizationService authorizationService; private final OAuth2TokenGenerator tokenGenerator; private final Converter clientRegistrationConverter; - private final Converter registeredClientConverter; + private Converter registeredClientConverter; /** * Constructs an {@code OidcClientRegistrationAuthenticationProvider} using the provided parameters. @@ -102,8 +103,8 @@ public final class OidcClientRegistrationAuthenticationProvider implements Authe this.registeredClientRepository = registeredClientRepository; this.authorizationService = authorizationService; this.tokenGenerator = tokenGenerator; - this.clientRegistrationConverter = new OidcClientRegistrationConverter(); - this.registeredClientConverter = new RegisteredClientConverter(); + this.clientRegistrationConverter = new RegisteredClientOidcClientRegistrationConverter(); + this.registeredClientConverter = new OidcClientRegistrationRegisteredClientConverter(); } @Override @@ -147,6 +148,17 @@ public final class OidcClientRegistrationAuthenticationProvider implements Authe return OidcClientRegistrationAuthenticationToken.class.isAssignableFrom(authentication); } + /** + * Sets the {@link Converter} used for converting an {@link OidcClientRegistration} to a {@link RegisteredClient}. + * + * @param registeredClientConverter the {@link Converter} used for converting an {@link OidcClientRegistration} to a {@link RegisteredClient} + * @since 0.4.0 + */ + public void setRegisteredClientConverter(Converter registeredClientConverter) { + Assert.notNull(registeredClientConverter, "registeredClientConverter cannot be null"); + this.registeredClientConverter = registeredClientConverter; + } + private OidcClientRegistrationAuthenticationToken registerClient(OidcClientRegistrationAuthenticationToken clientRegistrationAuthentication, OAuth2Authorization authorization) { @@ -293,7 +305,7 @@ public final class OidcClientRegistrationAuthenticationProvider implements Authe throw new OAuth2AuthenticationException(error); } - private static final class RegisteredClientConverter implements Converter { + private static final class OidcClientRegistrationRegisteredClientConverter implements Converter { private static final StringKeyGenerator CLIENT_ID_GENERATOR = new Base64StringKeyGenerator( Base64.getUrlEncoder().withoutPadding(), 32); private static final StringKeyGenerator CLIENT_SECRET_GENERATOR = new Base64StringKeyGenerator( diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/OidcClientRegistrationConverter.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/RegisteredClientOidcClientRegistrationConverter.java similarity index 96% rename from oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/OidcClientRegistrationConverter.java rename to oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/RegisteredClientOidcClientRegistrationConverter.java index b7e16d4e..75aa17c9 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/OidcClientRegistrationConverter.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/RegisteredClientOidcClientRegistrationConverter.java @@ -31,7 +31,7 @@ import org.springframework.web.util.UriComponentsBuilder; * @author Joe Grandja * @since 0.4.0 */ -final class OidcClientRegistrationConverter implements Converter { +final class RegisteredClientOidcClientRegistrationConverter implements Converter { @Override public OidcClientRegistration convert(RegisteredClient registeredClient) { diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/OidcClientRegistrationAuthenticationProviderTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/OidcClientRegistrationAuthenticationProviderTests.java index e471bd8a..e5b6210b 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/OidcClientRegistrationAuthenticationProviderTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/OidcClientRegistrationAuthenticationProviderTests.java @@ -134,6 +134,13 @@ public class OidcClientRegistrationAuthenticationProviderTests { .withMessage("tokenGenerator cannot be null"); } + @Test + public void setRegisteredClientConverterWhenNullThenThrowIllegalArgumentException() { + assertThatIllegalArgumentException() + .isThrownBy(() -> this.authenticationProvider.setRegisteredClientConverter(null)) + .withMessage("registeredClientConverter cannot be null"); + } + @Test public void supportsWhenTypeOidcClientRegistrationAuthenticationTokenThenReturnTrue() { assertThat(this.authenticationProvider.supports(OidcClientRegistrationAuthenticationToken.class)).isTrue();