From e37424c6374ceb150419768a1b2cf2ce2f37e507 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] 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 5ad816f7d4..4014e27742 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 @@ import org.springframework.web.util.UriComponentsBuilder; */ 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 65d0be3123..bbf9be1363 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 class OAuth2AuthorizationRequestRedirectFilter extends OncePerRequestFilt */ 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 7c718c990e..1382a0368f 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 class DefaultOAuth2AuthorizationRequestResolverTests { this.authorizationRequestBaseUri); } + @Test + void authorizationRequestBaseUriEqualToRedirectFilter() { + assertThat(DefaultOAuth2AuthorizationRequestResolver.DEFAULT_AUTHORIZATION_REQUEST_BASE_URI) + .isEqualTo(OAuth2AuthorizationRequestRedirectFilter.DEFAULT_AUTHORIZATION_REQUEST_BASE_URI); + } + @Test public void constructorWhenClientRegistrationRepositoryIsNullThenThrowIllegalArgumentException() { assertThatIllegalArgumentException()