瀏覽代碼

Validate redirect_uri on dynamic client registration

Closes gh-392
Joe Grandja 4 年之前
父節點
當前提交
c7815939d2

+ 0 - 15
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);
-			}
-		}
 	}
 }

+ 27 - 1
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<String>) scope).contains(DEFAULT_AUTHORIZED_SCOPE);
 	}
 
+	private static boolean isValidRedirectUris(List<String> 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()));
 

+ 0 - 10
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

+ 64 - 0
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();