From 250a39cdbfe8f61996bd503812a006a9c807510d Mon Sep 17 00:00:00 2001 From: DingHao Date: Thu, 9 Jan 2025 10:02:08 +0800 Subject: [PATCH 1/2] Add default authorizationRequestBaseUri to DefaultOAuth2AuthorizationRequestResolver Closes gh-16383 Signed-off-by: DingHao --- .../DefaultOAuth2AuthorizationRequestResolver.java | 11 +++++++++++ ...faultOAuth2AuthorizationRequestResolverTests.java | 12 ++++++++++++ 2 files changed, 23 insertions(+) diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolver.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolver.java index a2297accd76..5ad816f7d4b 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolver.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolver.java @@ -86,6 +86,17 @@ public final class DefaultOAuth2AuthorizationRequestResolver implements OAuth2Au private Consumer authorizationRequestCustomizer = (customizer) -> { }; + /** + * Constructs a {@code DefaultOAuth2AuthorizationRequestResolver} using the provided + * parameters. + * @param clientRegistrationRepository the repository of client registrations + * authorization requests + */ + public DefaultOAuth2AuthorizationRequestResolver(ClientRegistrationRepository clientRegistrationRepository) { + this(clientRegistrationRepository, + OAuth2AuthorizationRequestRedirectFilter.DEFAULT_AUTHORIZATION_REQUEST_BASE_URI); + } + /** * Constructs a {@code DefaultOAuth2AuthorizationRequestResolver} using the provided * parameters. diff --git a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolverTests.java b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolverTests.java index a0abf7132e4..7c718c990e4 100644 --- a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolverTests.java +++ b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolverTests.java @@ -568,6 +568,18 @@ public void resolveWhenAuthorizationRequestCustomizerOverridesParameterThenQuery + "nonce=([a-zA-Z0-9\\-\\.\\_\\~]){43}&" + "appid=client-id"); } + @Test + public void resolveWhenAuthorizationRequestNoProvideAuthorizationRequestBaseUri() { + OAuth2AuthorizationRequestResolver resolver = new DefaultOAuth2AuthorizationRequestResolver( + this.clientRegistrationRepository); + String requestUri = this.authorizationRequestBaseUri + "/" + this.registration2.getRegistrationId(); + MockHttpServletRequest request = new MockHttpServletRequest("GET", requestUri); + request.setServletPath(requestUri); + OAuth2AuthorizationRequest authorizationRequest = resolver.resolve(request); + assertThat(authorizationRequest.getRedirectUri()) + .isEqualTo("http://localhost/login/oauth2/code/" + this.registration2.getRegistrationId()); + } + @Test public void resolveWhenAuthorizationRequestProvideCodeChallengeMethod() { ClientRegistration clientRegistration = this.pkceClientRegistration; From 82be3dda2dd3487b2b0bd8bfe6b19fd9d1f06d23 Mon Sep 17 00:00:00 2001 From: Rob Winch <362503+rwinch@users.noreply.github.com> Date: Fri, 27 Jun 2025 15:36:22 -0500 Subject: [PATCH 2/2] Fix cycle in DefaultOAuth2AuthorizationRequestResolver DefaultOAuth2AuthorizationRequestResolver should not depend on OAuth2AuthorizationRequestRedirectFilter because OAuth2AuthorizationRequestRedirectFilter already depends on DefaultOAuth2AuthorizationRequestResolver. OAuth2AuthorizationRequestRedirectFilter also takes advantage of the new constructor that defaults the base uri. Polishes gh-16384 --- .../web/DefaultOAuth2AuthorizationRequestResolver.java | 8 ++++++-- .../web/OAuth2AuthorizationRequestRedirectFilter.java | 9 +++------ .../DefaultOAuth2AuthorizationRequestResolverTests.java | 6 ++++++ 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolver.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolver.java index 5ad816f7d4b..4014e277426 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolver.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolver.java @@ -66,6 +66,11 @@ */ public final class DefaultOAuth2AuthorizationRequestResolver implements OAuth2AuthorizationRequestResolver { + /** + * The default base {@code URI} used for authorization requests. + */ + public static final String DEFAULT_AUTHORIZATION_REQUEST_BASE_URI = "/oauth2/authorization"; + private static final String REGISTRATION_ID_URI_VARIABLE_NAME = "registrationId"; private static final char PATH_DELIMITER = '/'; @@ -93,8 +98,7 @@ public final class DefaultOAuth2AuthorizationRequestResolver implements OAuth2Au * authorization requests */ public DefaultOAuth2AuthorizationRequestResolver(ClientRegistrationRepository clientRegistrationRepository) { - this(clientRegistrationRepository, - OAuth2AuthorizationRequestRedirectFilter.DEFAULT_AUTHORIZATION_REQUEST_BASE_URI); + this(clientRegistrationRepository, DEFAULT_AUTHORIZATION_REQUEST_BASE_URI); } /** diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/OAuth2AuthorizationRequestRedirectFilter.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/OAuth2AuthorizationRequestRedirectFilter.java index 65d0be31233..bbf9be13630 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/OAuth2AuthorizationRequestRedirectFilter.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/OAuth2AuthorizationRequestRedirectFilter.java @@ -87,7 +87,7 @@ public class OAuth2AuthorizationRequestRedirectFilter extends OncePerRequestFilt /** * The default base {@code URI} used for authorization requests. */ - public static final String DEFAULT_AUTHORIZATION_REQUEST_BASE_URI = "/oauth2/authorization"; + public static final String DEFAULT_AUTHORIZATION_REQUEST_BASE_URI = DefaultOAuth2AuthorizationRequestResolver.DEFAULT_AUTHORIZATION_REQUEST_BASE_URI; private final ThrowableAnalyzer throwableAnalyzer = new DefaultThrowableAnalyzer(); @@ -107,7 +107,7 @@ public class OAuth2AuthorizationRequestRedirectFilter extends OncePerRequestFilt * @param clientRegistrationRepository the repository of client registrations */ public OAuth2AuthorizationRequestRedirectFilter(ClientRegistrationRepository clientRegistrationRepository) { - this(clientRegistrationRepository, DEFAULT_AUTHORIZATION_REQUEST_BASE_URI); + this(new DefaultOAuth2AuthorizationRequestResolver(clientRegistrationRepository)); } /** @@ -119,10 +119,7 @@ public OAuth2AuthorizationRequestRedirectFilter(ClientRegistrationRepository cli */ public OAuth2AuthorizationRequestRedirectFilter(ClientRegistrationRepository clientRegistrationRepository, String authorizationRequestBaseUri) { - Assert.notNull(clientRegistrationRepository, "clientRegistrationRepository cannot be null"); - Assert.hasText(authorizationRequestBaseUri, "authorizationRequestBaseUri cannot be empty"); - this.authorizationRequestResolver = new DefaultOAuth2AuthorizationRequestResolver(clientRegistrationRepository, - authorizationRequestBaseUri); + this(new DefaultOAuth2AuthorizationRequestResolver(clientRegistrationRepository, authorizationRequestBaseUri)); } /** diff --git a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolverTests.java b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolverTests.java index 7c718c990e4..1382a0368f8 100644 --- a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolverTests.java +++ b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolverTests.java @@ -97,6 +97,12 @@ public void setUp() { this.authorizationRequestBaseUri); } + @Test + void authorizationRequestBaseUriEqualToRedirectFilter() { + assertThat(DefaultOAuth2AuthorizationRequestResolver.DEFAULT_AUTHORIZATION_REQUEST_BASE_URI) + .isEqualTo(OAuth2AuthorizationRequestRedirectFilter.DEFAULT_AUTHORIZATION_REQUEST_BASE_URI); + } + @Test public void constructorWhenClientRegistrationRepositoryIsNullThenThrowIllegalArgumentException() { assertThatIllegalArgumentException()