diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/core/oidc/OidcClientRegistration.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/core/oidc/OidcClientRegistration.java index e098f262..e98183a3 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/core/oidc/OidcClientRegistration.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/core/oidc/OidcClientRegistration.java @@ -16,8 +16,6 @@ package org.springframework.security.oauth2.core.oidc; import java.io.Serializable; -import java.net.URI; -import java.net.URL; import java.time.Instant; import java.util.Collections; import java.util.LinkedHashMap; @@ -307,9 +305,6 @@ public final class OidcClientRegistration implements OidcClientMetadataClaimAcce Assert.notNull(this.claims.get(OidcClientMetadataClaimNames.REDIRECT_URIS), "redirect_uris cannot be null"); Assert.isInstanceOf(List.class, this.claims.get(OidcClientMetadataClaimNames.REDIRECT_URIS), "redirect_uris must be of type List"); Assert.notEmpty((List) this.claims.get(OidcClientMetadataClaimNames.REDIRECT_URIS), "redirect_uris cannot be empty"); - ((List) this.claims.get(OidcClientMetadataClaimNames.REDIRECT_URIS)).forEach( - url -> validateURL(url, "redirect_uri must be a valid URL") - ); if (this.claims.get(OidcClientMetadataClaimNames.GRANT_TYPES) != null) { Assert.isInstanceOf(List.class, this.claims.get(OidcClientMetadataClaimNames.GRANT_TYPES), "grant_types must be of type List"); Assert.notEmpty((List) this.claims.get(OidcClientMetadataClaimNames.GRANT_TYPES), "grant_types cannot be empty"); @@ -341,15 +336,5 @@ public final class OidcClientRegistration implements OidcClientMetadataClaimAcce valuesConsumer.accept(values); } - private static void validateURL(Object url, String errorMessage) { - if (URL.class.isAssignableFrom(url.getClass())) { - return; - } - try { - new URI(url.toString()).toURL(); - } catch (Exception ex) { - throw new IllegalArgumentException(errorMessage, ex); - } - } } } 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 edab9783..5cd7981c 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 @@ -15,9 +15,12 @@ */ package org.springframework.security.oauth2.server.authorization.oidc.authentication; +import java.net.URI; +import java.net.URISyntaxException; import java.time.Instant; import java.util.Base64; import java.util.Collection; +import java.util.List; import java.util.UUID; import org.springframework.security.authentication.AuthenticationProvider; @@ -110,6 +113,11 @@ public final class OidcClientRegistrationAuthenticationProvider implements Authe throw new OAuth2AuthenticationException(new OAuth2Error(OAuth2ErrorCodes.INSUFFICIENT_SCOPE)); } + if (!isValidRedirectUris(clientRegistrationAuthentication.getClientRegistration().getRedirectUris())) { + // TODO Add OAuth2ErrorCodes.INVALID_REDIRECT_URI + throw new OAuth2AuthenticationException("invalid_redirect_uri"); + } + RegisteredClient registeredClient = create(clientRegistrationAuthentication.getClientRegistration()); this.registeredClientRepository.save(registeredClient); @@ -135,6 +143,25 @@ public final class OidcClientRegistrationAuthenticationProvider implements Authe return scope != null && ((Collection) scope).contains(DEFAULT_AUTHORIZED_SCOPE); } + private static boolean isValidRedirectUris(List redirectUris) { + if (CollectionUtils.isEmpty(redirectUris)) { + return true; + } + + for (String redirectUri : redirectUris) { + try { + URI validRedirectUri = new URI(redirectUri); + if (validRedirectUri.getFragment() != null) { + return false; + } + } catch (URISyntaxException ex) { + return false; + } + } + + return true; + } + private static RegisteredClient create(OidcClientRegistration clientRegistration) { // @formatter:off RegisteredClient.Builder builder = RegisteredClient.withId(UUID.randomUUID().toString()) @@ -149,7 +176,6 @@ public final class OidcClientRegistrationAuthenticationProvider implements Authe builder.clientAuthenticationMethod(ClientAuthenticationMethod.CLIENT_SECRET_BASIC); } - // TODO Validate redirect_uris and throw OAuth2ErrorCodes2.INVALID_REDIRECT_URI on error builder.redirectUris(redirectUris -> redirectUris.addAll(clientRegistration.getRedirectUris())); diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/core/oidc/OidcClientRegistrationTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/core/oidc/OidcClientRegistrationTests.java index ac0d0aa5..3b9bc938 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/core/oidc/OidcClientRegistrationTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/core/oidc/OidcClientRegistrationTests.java @@ -228,16 +228,6 @@ public class OidcClientRegistrationTests { .withMessage("redirect_uris cannot be empty"); } - @Test - public void buildWhenInvalidRedirectUriThenThrowIllegalArgumentException() { - OidcClientRegistration.Builder builder = OidcClientRegistration.builder() - .redirectUri("invalid-uri"); - - assertThatIllegalArgumentException() - .isThrownBy(builder::build) - .withMessage("redirect_uri must be a valid URL"); - } - @Test public void buildWhenRedirectUrisAddingOrRemovingThenCorrectValues() { // @formatter:off 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 cff3d447..2b341910 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 @@ -208,6 +208,70 @@ public class OidcClientRegistrationAuthenticationProviderTests { eq(jwtAccessToken.getTokenValue()), eq(OAuth2TokenType.ACCESS_TOKEN)); } + @Test + public void authenticateWhenInvalidRedirectUriThenThrowOAuth2AuthenticationException() { + Jwt jwt = createJwt(); + OAuth2AccessToken jwtAccessToken = new OAuth2AccessToken(OAuth2AccessToken.TokenType.BEARER, + jwt.getTokenValue(), jwt.getIssuedAt(), + jwt.getExpiresAt(), jwt.getClaim(OAuth2ParameterNames.SCOPE)); + RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build(); + OAuth2Authorization authorization = TestOAuth2Authorizations.authorization( + registeredClient, jwtAccessToken, jwt.getClaims()).build(); + when(this.authorizationService.findByToken( + eq(jwtAccessToken.getTokenValue()), eq(OAuth2TokenType.ACCESS_TOKEN))) + .thenReturn(authorization); + + JwtAuthenticationToken principal = new JwtAuthenticationToken( + jwt, AuthorityUtils.createAuthorityList("SCOPE_client.create")); + // @formatter:off + OidcClientRegistration clientRegistration = OidcClientRegistration.builder() + .redirectUri("invalid uri") + .build(); + // @formatter:on + + OidcClientRegistrationAuthenticationToken authentication = new OidcClientRegistrationAuthenticationToken( + principal, clientRegistration); + + assertThatThrownBy(() -> this.authenticationProvider.authenticate(authentication)) + .isInstanceOf(OAuth2AuthenticationException.class) + .extracting(ex -> ((OAuth2AuthenticationException) ex).getError()).extracting("errorCode") + .isEqualTo("invalid_redirect_uri"); + verify(this.authorizationService).findByToken( + eq(jwtAccessToken.getTokenValue()), eq(OAuth2TokenType.ACCESS_TOKEN)); + } + + @Test + public void authenticateWhenRedirectUriContainsFragmentThenThrowOAuth2AuthenticationException() { + Jwt jwt = createJwt(); + OAuth2AccessToken jwtAccessToken = new OAuth2AccessToken(OAuth2AccessToken.TokenType.BEARER, + jwt.getTokenValue(), jwt.getIssuedAt(), + jwt.getExpiresAt(), jwt.getClaim(OAuth2ParameterNames.SCOPE)); + RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build(); + OAuth2Authorization authorization = TestOAuth2Authorizations.authorization( + registeredClient, jwtAccessToken, jwt.getClaims()).build(); + when(this.authorizationService.findByToken( + eq(jwtAccessToken.getTokenValue()), eq(OAuth2TokenType.ACCESS_TOKEN))) + .thenReturn(authorization); + + JwtAuthenticationToken principal = new JwtAuthenticationToken( + jwt, AuthorityUtils.createAuthorityList("SCOPE_client.create")); + // @formatter:off + OidcClientRegistration clientRegistration = OidcClientRegistration.builder() + .redirectUri("https://client.example.com#fragment") + .build(); + // @formatter:on + + OidcClientRegistrationAuthenticationToken authentication = new OidcClientRegistrationAuthenticationToken( + principal, clientRegistration); + + assertThatThrownBy(() -> this.authenticationProvider.authenticate(authentication)) + .isInstanceOf(OAuth2AuthenticationException.class) + .extracting(ex -> ((OAuth2AuthenticationException) ex).getError()).extracting("errorCode") + .isEqualTo("invalid_redirect_uri"); + verify(this.authorizationService).findByToken( + eq(jwtAccessToken.getTokenValue()), eq(OAuth2TokenType.ACCESS_TOKEN)); + } + @Test public void authenticateWhenValidAccessTokenThenReturnClientRegistration() { Jwt jwt = createJwt();