Skip to content

Commit c781593

Browse files
jgrandjajgrandja
committed
Validate redirect_uri on dynamic client registration
Closes gh-392
1 parent 2c8d5a1 commit c781593

File tree

4 files changed

+91
-26
lines changed

4 files changed

+91
-26
lines changed

‎oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/core/oidc/OidcClientRegistration.java‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,6 @@
1616
package org.springframework.security.oauth2.core.oidc;
1717

1818
import java.io.Serializable;
19-
import java.net.URI;
20-
import java.net.URL;
2119
import java.time.Instant;
2220
import java.util.Collections;
2321
import java.util.LinkedHashMap;
@@ -307,9 +305,6 @@ private void validate() {
307305
Assert.notNull(this.claims.get(OidcClientMetadataClaimNames.REDIRECT_URIS), "redirect_uris cannot be null");
308306
Assert.isInstanceOf(List.class, this.claims.get(OidcClientMetadataClaimNames.REDIRECT_URIS), "redirect_uris must be of type List");
309307
Assert.notEmpty((List<?>) this.claims.get(OidcClientMetadataClaimNames.REDIRECT_URIS), "redirect_uris cannot be empty");
310-
((List<?>) this.claims.get(OidcClientMetadataClaimNames.REDIRECT_URIS)).forEach(
311-
url -> validateURL(url, "redirect_uri must be a valid URL")
312-
);
313308
if (this.claims.get(OidcClientMetadataClaimNames.GRANT_TYPES) != null) {
314309
Assert.isInstanceOf(List.class, this.claims.get(OidcClientMetadataClaimNames.GRANT_TYPES), "grant_types must be of type List");
315310
Assert.notEmpty((List<?>) this.claims.get(OidcClientMetadataClaimNames.GRANT_TYPES), "grant_types cannot be empty");
@@ -341,15 +336,5 @@ private void acceptClaimValues(String name, Consumer<List<String>> valuesConsume
341336
valuesConsumer.accept(values);
342337
}
343338

344-
private static void validateURL(Object url, String errorMessage) {
345-
if (URL.class.isAssignableFrom(url.getClass())) {
346-
return;
347-
}
348-
try {
349-
new URI(url.toString()).toURL();
350-
} catch (Exception ex) {
351-
throw new IllegalArgumentException(errorMessage, ex);
352-
}
353-
}
354339
}
355340
}

‎oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/OidcClientRegistrationAuthenticationProvider.java‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,12 @@
1515
*/
1616
package org.springframework.security.oauth2.server.authorization.oidc.authentication;
1717

18+
import java.net.URI;
19+
import java.net.URISyntaxException;
1820
import java.time.Instant;
1921
import java.util.Base64;
2022
import java.util.Collection;
23+
import java.util.List;
2124
import java.util.UUID;
2225

2326
import org.springframework.security.authentication.AuthenticationProvider;
@@ -110,6 +113,11 @@ public Authentication authenticate(Authentication authentication) throws Authent
110113
throw new OAuth2AuthenticationException(new OAuth2Error(OAuth2ErrorCodes.INSUFFICIENT_SCOPE));
111114
}
112115

116+
if (!isValidRedirectUris(clientRegistrationAuthentication.getClientRegistration().getRedirectUris())) {
117+
// TODO Add OAuth2ErrorCodes.INVALID_REDIRECT_URI
118+
throw new OAuth2AuthenticationException("invalid_redirect_uri");
119+
}
120+
113121
RegisteredClient registeredClient = create(clientRegistrationAuthentication.getClientRegistration());
114122
this.registeredClientRepository.save(registeredClient);
115123

@@ -135,6 +143,25 @@ private static boolean isAuthorized(OAuth2Authorization.Token<OAuth2AccessToken>
135143
return scope != null && ((Collection<String>) scope).contains(DEFAULT_AUTHORIZED_SCOPE);
136144
}
137145

146+
private static boolean isValidRedirectUris(List<String> redirectUris) {
147+
if (CollectionUtils.isEmpty(redirectUris)) {
148+
return true;
149+
}
150+
151+
for (String redirectUri : redirectUris) {
152+
try {
153+
URI validRedirectUri = new URI(redirectUri);
154+
if (validRedirectUri.getFragment() != null) {
155+
return false;
156+
}
157+
} catch (URISyntaxException ex) {
158+
return false;
159+
}
160+
}
161+
162+
return true;
163+
}
164+
138165
private static RegisteredClient create(OidcClientRegistration clientRegistration) {
139166
// @formatter:off
140167
RegisteredClient.Builder builder = RegisteredClient.withId(UUID.randomUUID().toString())
@@ -149,7 +176,6 @@ private static RegisteredClient create(OidcClientRegistration clientRegistration
149176
builder.clientAuthenticationMethod(ClientAuthenticationMethod.CLIENT_SECRET_BASIC);
150177
}
151178

152-
// TODO Validate redirect_uris and throw OAuth2ErrorCodes2.INVALID_REDIRECT_URI on error
153179
builder.redirectUris(redirectUris ->
154180
redirectUris.addAll(clientRegistration.getRedirectUris()));
155181

‎oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/core/oidc/OidcClientRegistrationTests.java‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -228,16 +228,6 @@ public void buildWhenRedirectUrisEmptyListThenThrowIllegalArgumentException() {
228228
.withMessage("redirect_uris cannot be empty");
229229
}
230230

231-
@Test
232-
public void buildWhenInvalidRedirectUriThenThrowIllegalArgumentException() {
233-
OidcClientRegistration.Builder builder = OidcClientRegistration.builder()
234-
.redirectUri("invalid-uri");
235-
236-
assertThatIllegalArgumentException()
237-
.isThrownBy(builder::build)
238-
.withMessage("redirect_uri must be a valid URL");
239-
}
240-
241231
@Test
242232
public void buildWhenRedirectUrisAddingOrRemovingThenCorrectValues() {
243233
// @formatter:off

‎oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/OidcClientRegistrationAuthenticationProviderTests.java‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,70 @@ public void authenticateWhenAccessTokenNotAuthorizedThenThrowOAuth2Authenticatio
208208
eq(jwtAccessToken.getTokenValue()), eq(OAuth2TokenType.ACCESS_TOKEN));
209209
}
210210

211+
@Test
212+
public void authenticateWhenInvalidRedirectUriThenThrowOAuth2AuthenticationException() {
213+
Jwt jwt = createJwt();
214+
OAuth2AccessToken jwtAccessToken = new OAuth2AccessToken(OAuth2AccessToken.TokenType.BEARER,
215+
jwt.getTokenValue(), jwt.getIssuedAt(),
216+
jwt.getExpiresAt(), jwt.getClaim(OAuth2ParameterNames.SCOPE));
217+
RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build();
218+
OAuth2Authorization authorization = TestOAuth2Authorizations.authorization(
219+
registeredClient, jwtAccessToken, jwt.getClaims()).build();
220+
when(this.authorizationService.findByToken(
221+
eq(jwtAccessToken.getTokenValue()), eq(OAuth2TokenType.ACCESS_TOKEN)))
222+
.thenReturn(authorization);
223+
224+
JwtAuthenticationToken principal = new JwtAuthenticationToken(
225+
jwt, AuthorityUtils.createAuthorityList("SCOPE_client.create"));
226+
// @formatter:off
227+
OidcClientRegistration clientRegistration = OidcClientRegistration.builder()
228+
.redirectUri("invalid uri")
229+
.build();
230+
// @formatter:on
231+
232+
OidcClientRegistrationAuthenticationToken authentication = new OidcClientRegistrationAuthenticationToken(
233+
principal, clientRegistration);
234+
235+
assertThatThrownBy(() -> this.authenticationProvider.authenticate(authentication))
236+
.isInstanceOf(OAuth2AuthenticationException.class)
237+
.extracting(ex -> ((OAuth2AuthenticationException) ex).getError()).extracting("errorCode")
238+
.isEqualTo("invalid_redirect_uri");
239+
verify(this.authorizationService).findByToken(
240+
eq(jwtAccessToken.getTokenValue()), eq(OAuth2TokenType.ACCESS_TOKEN));
241+
}
242+
243+
@Test
244+
public void authenticateWhenRedirectUriContainsFragmentThenThrowOAuth2AuthenticationException() {
245+
Jwt jwt = createJwt();
246+
OAuth2AccessToken jwtAccessToken = new OAuth2AccessToken(OAuth2AccessToken.TokenType.BEARER,
247+
jwt.getTokenValue(), jwt.getIssuedAt(),
248+
jwt.getExpiresAt(), jwt.getClaim(OAuth2ParameterNames.SCOPE));
249+
RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build();
250+
OAuth2Authorization authorization = TestOAuth2Authorizations.authorization(
251+
registeredClient, jwtAccessToken, jwt.getClaims()).build();
252+
when(this.authorizationService.findByToken(
253+
eq(jwtAccessToken.getTokenValue()), eq(OAuth2TokenType.ACCESS_TOKEN)))
254+
.thenReturn(authorization);
255+
256+
JwtAuthenticationToken principal = new JwtAuthenticationToken(
257+
jwt, AuthorityUtils.createAuthorityList("SCOPE_client.create"));
258+
// @formatter:off
259+
OidcClientRegistration clientRegistration = OidcClientRegistration.builder()
260+
.redirectUri("https://client.example.com#fragment")
261+
.build();
262+
// @formatter:on
263+
264+
OidcClientRegistrationAuthenticationToken authentication = new OidcClientRegistrationAuthenticationToken(
265+
principal, clientRegistration);
266+
267+
assertThatThrownBy(() -> this.authenticationProvider.authenticate(authentication))
268+
.isInstanceOf(OAuth2AuthenticationException.class)
269+
.extracting(ex -> ((OAuth2AuthenticationException) ex).getError()).extracting("errorCode")
270+
.isEqualTo("invalid_redirect_uri");
271+
verify(this.authorizationService).findByToken(
272+
eq(jwtAccessToken.getTokenValue()), eq(OAuth2TokenType.ACCESS_TOKEN));
273+
}
274+
211275
@Test
212276
public void authenticateWhenValidAccessTokenThenReturnClientRegistration() {
213277
Jwt jwt = createJwt();

0 commit comments

Comments
 (0)