From 3f8e69211f096e2a011f66b2d3d70ca75cf4c227 Mon Sep 17 00:00:00 2001 From: "mhyeon.lee" Date: Tue, 10 Jul 2018 23:40:42 +0900 Subject: [PATCH] Fix OAuth2 ClientRegistration scope can be null Allows scope of OAuth2 ClientRegistration to be null. - The scope setting in the RFC document is defined as Optional. https://tools.ietf.org/html/rfc6749#section-4.1.1 > scope: OPTIONAL. > The scope of the access request as described by Section 3.3. - When the client omits the scope parameter, validation is determined by the authorization server. https://tools.ietf.org/html/rfc6749#section-3.3 > If the client omits the scope parameter when requesting authorization, the authorization server MUST either process the request using a pre-defined default value or fail the request indicating an invalid scope. The authorization server SHOULD document its scope requirements and default value (if defined). Fixes gh-5494 --- .../registration/ClientRegistration.java | 4 +-- .../registration/ClientRegistrationTests.java | 27 +++++++++++++++---- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientRegistration.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientRegistration.java index bf3f5e198c..bf390f8b94 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientRegistration.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientRegistration.java @@ -463,10 +463,9 @@ public final class ClientRegistration { Assert.hasText(this.clientSecret, "clientSecret cannot be empty"); Assert.notNull(this.clientAuthenticationMethod, "clientAuthenticationMethod cannot be null"); Assert.hasText(this.redirectUriTemplate, "redirectUriTemplate cannot be empty"); - Assert.notEmpty(this.scopes, "scopes cannot be empty"); Assert.hasText(this.authorizationUri, "authorizationUri cannot be empty"); Assert.hasText(this.tokenUri, "tokenUri cannot be empty"); - if (this.scopes.contains(OidcScopes.OPENID)) { + if (this.scopes != null && this.scopes.contains(OidcScopes.OPENID)) { // OIDC Clients need to verify/validate the ID Token Assert.hasText(this.jwkSetUri, "jwkSetUri cannot be empty"); } @@ -479,7 +478,6 @@ public final class ClientRegistration { Assert.hasText(this.registrationId, "registrationId cannot be empty"); Assert.hasText(this.clientId, "clientId cannot be empty"); Assert.hasText(this.redirectUriTemplate, "redirectUriTemplate cannot be empty"); - Assert.notEmpty(this.scopes, "scopes cannot be empty"); Assert.hasText(this.authorizationUri, "authorizationUri cannot be empty"); Assert.hasText(this.clientName, "clientName cannot be empty"); } diff --git a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/registration/ClientRegistrationTests.java b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/registration/ClientRegistrationTests.java index 95f23bd833..0a9c096950 100644 --- a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/registration/ClientRegistrationTests.java +++ b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/registration/ClientRegistrationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -165,8 +165,9 @@ public class ClientRegistrationTests { .build(); } - @Test(expected = IllegalArgumentException.class) - public void buildWhenAuthorizationCodeGrantScopeIsNullThenThrowIllegalArgumentException() { + // gh-5494 + @Test + public void buildWhenAuthorizationCodeGrantScopeIsNullThenScopeNotRequired() { ClientRegistration.withRegistrationId(REGISTRATION_ID) .clientId(CLIENT_ID) .clientSecret(CLIENT_SECRET) @@ -260,6 +261,21 @@ public class ClientRegistrationTests { .build(); } + // gh-5494 + @Test + public void buildWhenAuthorizationCodeGrantScopeIsNullThenJwkSetUriNotRequired() { + ClientRegistration.withRegistrationId(REGISTRATION_ID) + .clientId(CLIENT_ID) + .clientSecret(CLIENT_SECRET) + .clientAuthenticationMethod(ClientAuthenticationMethod.BASIC) + .authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE) + .redirectUriTemplate(REDIRECT_URI) + .authorizationUri(AUTHORIZATION_URI) + .tokenUri(TOKEN_URI) + .clientName(CLIENT_NAME) + .build(); + } + @Test public void buildWhenImplicitGrantAllAttributesProvidedThenAllAttributesAreSet() { ClientRegistration registration = ClientRegistration.withRegistrationId(REGISTRATION_ID) @@ -316,8 +332,9 @@ public class ClientRegistrationTests { .build(); } - @Test(expected = IllegalArgumentException.class) - public void buildWhenImplicitGrantScopeIsNullThenThrowIllegalArgumentException() { + // gh-5494 + @Test + public void buildWhenImplicitGrantScopeIsNullThenScopeNotRequired() { ClientRegistration.withRegistrationId(REGISTRATION_ID) .clientId(CLIENT_ID) .authorizationGrantType(AuthorizationGrantType.IMPLICIT)