diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/authorization/OAuth2AuthorizationServerConfigurer.java b/oauth2-authorization-server/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/authorization/OAuth2AuthorizationServerConfigurer.java index 2fd64f2d..d70aae08 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/authorization/OAuth2AuthorizationServerConfigurer.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/authorization/OAuth2AuthorizationServerConfigurer.java @@ -35,12 +35,12 @@ import org.springframework.security.config.annotation.web.configurers.ExceptionH import org.springframework.security.crypto.password.PasswordEncoder; import org.springframework.security.oauth2.jwt.JwtEncoder; import org.springframework.security.oauth2.jwt.NimbusJwsEncoder; -import org.springframework.security.oauth2.server.authorization.InMemoryOAuth2AuthorizationService; import org.springframework.security.oauth2.server.authorization.InMemoryOAuth2AuthorizationConsentService; +import org.springframework.security.oauth2.server.authorization.InMemoryOAuth2AuthorizationService; import org.springframework.security.oauth2.server.authorization.JwtEncodingContext; +import org.springframework.security.oauth2.server.authorization.OAuth2AuthorizationConsentService; import org.springframework.security.oauth2.server.authorization.OAuth2AuthorizationService; import org.springframework.security.oauth2.server.authorization.OAuth2TokenCustomizer; -import org.springframework.security.oauth2.server.authorization.OAuth2AuthorizationConsentService; import org.springframework.security.oauth2.server.authorization.authentication.OAuth2AuthorizationCodeAuthenticationProvider; import org.springframework.security.oauth2.server.authorization.authentication.OAuth2ClientAuthenticationProvider; import org.springframework.security.oauth2.server.authorization.authentication.OAuth2ClientCredentialsAuthenticationProvider; @@ -79,6 +79,7 @@ import org.springframework.util.StringUtils; * @see AbstractHttpConfigurer * @see RegisteredClientRepository * @see OAuth2AuthorizationService + * @see OAuth2AuthorizationConsentService * @see OAuth2AuthorizationEndpointFilter * @see OAuth2TokenEndpointFilter * @see OAuth2TokenIntrospectionEndpointFilter @@ -138,7 +139,7 @@ public final class OAuth2AuthorizationServerConfigurer authorizationConsentService(OAuth2AuthorizationConsentService authorizationConsentService) { @@ -160,17 +161,17 @@ public final class OAuth2AuthorizationServerConfigurer - *
  • {@code client_id} the client identifier
    • - *
  • {@code scope} the space separated list of scopes present in the authorization request
    • - *
  • {@code state} a CSRF protection token
    • + *
  • {@code client_id} - the client identifier
    • + *
  • {@code scope} - the space separated list of scopes present in the authorization request
    • + *
  • {@code state} - a CSRF protection token
    • * * * In general, the consent page should create a form that submits @@ -181,14 +182,13 @@ public final class OAuth2AuthorizationServerConfigurerIt must be submitted to {@link ProviderSettings#authorizationEndpoint()} *
  • It must include the received {@code client_id} as an HTTP parameter
    • *
  • It must include the received {@code state} as an HTTP parameter
    • - *
  • It must include the list of {@code scope}s the {@code Resource Owners} - * consents to as an HTTP parameter
    • - *
  • It must include the {@code consent_action} parameter, with value either + *
  • It must include the list of {@code scope}s the {@code Resource Owner} + * consented to as an HTTP parameter
    • + *
  • It must include the {@code consent_action} parameter, with a value either * {@code approve} or {@code cancel} as an HTTP parameter
    • * * - * - * @param consentPage the consent page to redirect to if consent is required (e.g. "/consent") + * @param consentPage the consent page to redirect to if consent is required (e.g. "/oauth2/consent") * @return the {@link OAuth2AuthorizationServerConfigurer} for further configuration */ public OAuth2AuthorizationServerConfigurer consentPage(String consentPage) { @@ -316,9 +316,8 @@ public final class OAuth2AuthorizationServerConfigurer * When authorizing access for a given client, the resource owner may only grant a subset of the authorities @@ -130,7 +130,7 @@ public final class OAuth2AuthorizationConsent implements Serializable { /** * A builder for {@link OAuth2AuthorizationConsent}. */ - public final static class Builder implements Serializable { + public static final class Builder implements Serializable { private static final long serialVersionUID = Version.SERIAL_VERSION_UID; private final String registeredClientId; @@ -151,10 +151,10 @@ public final class OAuth2AuthorizationConsent implements Serializable { /** * Adds a scope to the collection of {@code authorities} in the resulting {@link OAuth2AuthorizationConsent}, - * wrapping it in a SimpleGrantedAuthority, prefixed by {@code SCOPE_}. For example, a + * wrapping it in a {@link SimpleGrantedAuthority}, prefixed by {@code SCOPE_}. For example, a * {@code message.write} scope would be stored as {@code SCOPE_message.write}. * - * @param scope the {@code scope} + * @param scope the scope * @return the {@code Builder} for further configuration */ public Builder scope(String scope) { diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/OAuth2AuthorizationConsentService.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/OAuth2AuthorizationConsentService.java index 185a69d1..04f6607c 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/OAuth2AuthorizationConsentService.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/OAuth2AuthorizationConsentService.java @@ -29,6 +29,7 @@ import java.security.Principal; * @see OAuth2AuthorizationConsent */ public interface OAuth2AuthorizationConsentService { + /** * Saves the {@link OAuth2AuthorizationConsent}. * @@ -53,4 +54,5 @@ public interface OAuth2AuthorizationConsentService { */ @Nullable OAuth2AuthorizationConsent findById(String registeredClientId, String principalName); + } diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilter.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilter.java index 596473c3..7876146d 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilter.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilter.java @@ -86,6 +86,7 @@ import org.springframework.web.util.UriComponentsBuilder; * @see OAuth2AuthorizationService * @see OAuth2AuthorizationConsentService * @see OAuth2Authorization + * @see OAuth2AuthorizationConsent * @see Section 4.1 Authorization Code Grant * @see Section 4.1.1 Authorization Request * @see Section 4.1.2 Authorization Response @@ -139,10 +140,7 @@ public class OAuth2AuthorizationEndpointFilter extends OncePerRequestFilter { @Deprecated public OAuth2AuthorizationEndpointFilter(RegisteredClientRepository registeredClientRepository, OAuth2AuthorizationService authorizationService, String authorizationEndpointUri) { - this(registeredClientRepository, - authorizationService, - new InMemoryOAuth2AuthorizationConsentService(), - authorizationEndpointUri); + this(registeredClientRepository, authorizationService, new InMemoryOAuth2AuthorizationConsentService(), authorizationEndpointUri); } /** @@ -162,19 +160,19 @@ public class OAuth2AuthorizationEndpointFilter extends OncePerRequestFilter { * * @param registeredClientRepository the repository of registered clients * @param authorizationService the authorization service - * @param consentService the consent service + * @param authorizationConsentService the authorization consent service * @param authorizationEndpointUri the endpoint {@code URI} for authorization requests */ public OAuth2AuthorizationEndpointFilter(RegisteredClientRepository registeredClientRepository, - OAuth2AuthorizationService authorizationService, OAuth2AuthorizationConsentService consentService, + OAuth2AuthorizationService authorizationService, OAuth2AuthorizationConsentService authorizationConsentService, String authorizationEndpointUri) { Assert.notNull(registeredClientRepository, "registeredClientRepository cannot be null"); Assert.notNull(authorizationService, "authorizationService cannot be null"); - Assert.notNull(consentService, "consentService cannot be null"); + Assert.notNull(authorizationConsentService, "authorizationConsentService cannot be null"); Assert.hasText(authorizationEndpointUri, "authorizationEndpointUri cannot be empty"); this.registeredClientRepository = registeredClientRepository; this.authorizationService = authorizationService; - this.authorizationConsentService = consentService; + this.authorizationConsentService = authorizationConsentService; RequestMatcher authorizationRequestGetMatcher = new AntPathRequestMatcher( authorizationEndpointUri, HttpMethod.GET.name()); @@ -196,14 +194,14 @@ public class OAuth2AuthorizationEndpointFilter extends OncePerRequestFilter { } /** - * Specify the URL to redirect Resource Owners to if consent is required. A default consent + * Specify the URI to redirect Resource Owners to if consent is required. A default consent * page will be generated when this attribute is not specified. * - * @param customConsentUri the URI of the custom consent page to redirect to if consent is required (e.g. "/consent") + * @param userConsentUri the URI of the custom consent page to redirect to if consent is required (e.g. "/oauth2/consent") * @see org.springframework.security.config.annotation.web.configurers.oauth2.server.authorization.OAuth2AuthorizationServerConfigurer#consentPage(String) */ - public final void setUserConsentUri(String customConsentUri) { - this.userConsentUri = customConsentUri; + public final void setUserConsentUri(String userConsentUri) { + this.userConsentUri = userConsentUri; } @Override @@ -259,8 +257,9 @@ public class OAuth2AuthorizationEndpointFilter extends OncePerRequestFilter { .attribute(Principal.class.getName(), principal) .attribute(OAuth2AuthorizationRequest.class.getName(), authorizationRequest); - OAuth2AuthorizationConsent previousConsent = this.authorizationConsentService.findById(registeredClient.getClientId(), principal.getName()); - if (requireUserConsent(registeredClient, authorizationRequest, previousConsent)) { + OAuth2AuthorizationConsent currentAuthorizationConsent = this.authorizationConsentService.findById( + registeredClient.getId(), principal.getName()); + if (requireUserConsent(registeredClient, authorizationRequest, currentAuthorizationConsent)) { String state = this.stateGenerator.generateKey(); OAuth2Authorization authorization = builder .attribute(OAuth2ParameterNames.STATE, state) @@ -269,16 +268,16 @@ public class OAuth2AuthorizationEndpointFilter extends OncePerRequestFilter { // TODO Need to remove 'in-flight' authorization if consent step is not completed (e.g. approved or cancelled) - if (this.hasCustomUserConsentPage()) { - String redirect = UriComponentsBuilder + if (hasCustomUserConsentPage()) { + String redirectUri = UriComponentsBuilder .fromUriString(this.userConsentUri) .queryParam(OAuth2ParameterNames.SCOPE, String.join(" ", authorizationRequest.getScopes())) .queryParam(OAuth2ParameterNames.CLIENT_ID, registeredClient.getClientId()) .queryParam(OAuth2ParameterNames.STATE, state) .toUriString(); - this.redirectStrategy.sendRedirect(request, response, redirect); + this.redirectStrategy.sendRedirect(request, response, redirectUri); } else { - UserConsentPage.displayConsent(request, response, registeredClient, authorization, previousConsent); + UserConsentPage.displayConsent(request, response, registeredClient, authorization, currentAuthorizationConsent); } } else { Instant issuedAt = Instant.now(); @@ -305,10 +304,12 @@ public class OAuth2AuthorizationEndpointFilter extends OncePerRequestFilter { } private boolean hasCustomUserConsentPage() { - return this.userConsentUri != null; + return StringUtils.hasText(this.userConsentUri); } - private boolean requireUserConsent(RegisteredClient registeredClient, OAuth2AuthorizationRequest authorizationRequest, OAuth2AuthorizationConsent previousConsent) { + private static boolean requireUserConsent(RegisteredClient registeredClient, OAuth2AuthorizationRequest authorizationRequest, + OAuth2AuthorizationConsent currentAuthorizationConsent) { + if (!registeredClient.getClientSettings().requireUserConsent()) { return false; } @@ -318,8 +319,8 @@ public class OAuth2AuthorizationEndpointFilter extends OncePerRequestFilter { return false; } - if (previousConsent != null && - previousConsent.getScopes().containsAll(authorizationRequest.getScopes())) { + if (currentAuthorizationConsent != null && + currentAuthorizationConsent.getScopes().containsAll(authorizationRequest.getScopes())) { return false; } @@ -364,16 +365,18 @@ public class OAuth2AuthorizationEndpointFilter extends OncePerRequestFilter { authorizedScopes.add(OidcScopes.OPENID); } - OAuth2AuthorizationConsent previousConsent = this.authorizationConsentService.findById( - userConsentRequestContext.getClientId(), - userConsentRequestContext.getAuthorization().getPrincipalName() - ); - for (String requestedScope : userConsentRequestContext.getAuthorizationRequest().getScopes()) { - if (previousConsent != null && previousConsent.getScopes().contains(requestedScope)) { - authorizedScopes.add(requestedScope); + OAuth2AuthorizationConsent currentAuthorizationConsent = this.authorizationConsentService.findById( + userConsentRequestContext.getAuthorization().getRegisteredClientId(), + userConsentRequestContext.getAuthorization().getPrincipalName()); + if (currentAuthorizationConsent != null) { + Set currentAuthorizedScopes = currentAuthorizationConsent.getScopes(); + for (String requestedScope : userConsentRequestContext.getAuthorizationRequest().getScopes()) { + if (currentAuthorizedScopes.contains(requestedScope)) { + authorizedScopes.add(requestedScope); + } } } - saveAuthorizationConsent(previousConsent, userConsentRequestContext); + saveAuthorizationConsent(currentAuthorizationConsent, userConsentRequestContext); OAuth2Authorization authorization = OAuth2Authorization.from(userConsentRequestContext.getAuthorization()) .token(authorizationCode) @@ -388,26 +391,25 @@ public class OAuth2AuthorizationEndpointFilter extends OncePerRequestFilter { authorizationCode, userConsentRequestContext.getAuthorizationRequest().getState()); } - private void saveAuthorizationConsent(OAuth2AuthorizationConsent previousConsent, UserConsentRequestContext userConsentRequestContext) { + private void saveAuthorizationConsent(OAuth2AuthorizationConsent currentAuthorizationConsent, UserConsentRequestContext userConsentRequestContext) { if (CollectionUtils.isEmpty(userConsentRequestContext.getScopes())) { return; } - OAuth2AuthorizationConsent.Builder userConsentBuilder; - if (previousConsent == null) { - userConsentBuilder = OAuth2AuthorizationConsent.withId( - userConsentRequestContext.getClientId(), - userConsentRequestContext.getAuthorization().getPrincipalName() - ); + OAuth2AuthorizationConsent.Builder authorizationConsentBuilder; + if (currentAuthorizationConsent == null) { + authorizationConsentBuilder = OAuth2AuthorizationConsent.withId( + userConsentRequestContext.getAuthorization().getRegisteredClientId(), + userConsentRequestContext.getAuthorization().getPrincipalName()); } else { - userConsentBuilder = OAuth2AuthorizationConsent.from(previousConsent); + authorizationConsentBuilder = OAuth2AuthorizationConsent.from(currentAuthorizationConsent); } for (String authorizedScope : userConsentRequestContext.getScopes()) { - userConsentBuilder.scope(authorizedScope); + authorizationConsentBuilder.scope(authorizedScope); } - OAuth2AuthorizationConsent userConsent = userConsentBuilder.build(); - this.authorizationConsentService.save(userConsent); + OAuth2AuthorizationConsent authorizationConsent = authorizationConsentBuilder.build(); + this.authorizationConsentService.save(authorizationConsent); } private void validateAuthorizationRequest(OAuth2AuthorizationRequestContext authorizationRequestContext) { @@ -815,9 +817,9 @@ public class OAuth2AuthorizationEndpointFilter extends OncePerRequestFilter { private static void displayConsent(HttpServletRequest request, HttpServletResponse response, RegisteredClient registeredClient, OAuth2Authorization authorization, - OAuth2AuthorizationConsent previousConsent) throws IOException { + OAuth2AuthorizationConsent currentAuthorizationConsent) throws IOException { - String consentPage = generateConsentPage(request, registeredClient, authorization, previousConsent); + String consentPage = generateConsentPage(request, registeredClient, authorization, currentAuthorizationConsent); response.setContentType(TEXT_HTML_UTF8.toString()); response.setContentLength(consentPage.getBytes(StandardCharsets.UTF_8).length); response.getWriter().write(consentPage); @@ -832,17 +834,26 @@ public class OAuth2AuthorizationEndpointFilter extends OncePerRequestFilter { } private static String generateConsentPage(HttpServletRequest request, - RegisteredClient registeredClient, OAuth2Authorization authorization, OAuth2AuthorizationConsent previousConsent) { + RegisteredClient registeredClient, OAuth2Authorization authorization, + OAuth2AuthorizationConsent currentAuthorizationConsent) { + + Set currentAuthorizedScopes; + if (currentAuthorizationConsent != null) { + currentAuthorizedScopes = currentAuthorizationConsent.getScopes(); + } else { + currentAuthorizedScopes = Collections.emptySet(); + } + OAuth2AuthorizationRequest authorizationRequest = authorization.getAttribute( OAuth2AuthorizationRequest.class.getName()); - Set scopes = new HashSet<>(); - Set previouslyApprovedScopes = new HashSet<>(); + Set scopesToAuthorize = new HashSet<>(); + Set scopesPreviouslyAuthorized = new HashSet<>(); for (String scope : authorizationRequest.getScopes()) { - if (previousConsent != null && previousConsent.getScopes().contains(scope)) { - previouslyApprovedScopes.add(scope); + if (currentAuthorizedScopes.contains(scope)) { + scopesPreviouslyAuthorized.add(scope); } else if (!scope.equals(OidcScopes.OPENID)) { // openid scope does not require consent - scopes.add(scope); + scopesToAuthorize.add(scope); } } @@ -879,16 +890,16 @@ public class OAuth2AuthorizationEndpointFilter extends OncePerRequestFilter { builder.append(" "); builder.append(" "); - for (String scope : scopes) { + for (String scope : scopesToAuthorize) { builder.append("
    "); builder.append(" "); builder.append(" "); builder.append("
    "); } - if (!previouslyApprovedScopes.isEmpty()) { + if (!scopesPreviouslyAuthorized.isEmpty()) { builder.append("

    You have already granted the following permissions to the above app:

    "); - for (String scope : previouslyApprovedScopes) { + for (String scope : scopesPreviouslyAuthorized) { builder.append("
    "); builder.append(" "); builder.append(" "); diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/authorization/OAuth2AuthorizationCodeGrantTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/authorization/OAuth2AuthorizationCodeGrantTests.java index a52e27c1..d4ec8db6 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/authorization/OAuth2AuthorizationCodeGrantTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/authorization/OAuth2AuthorizationCodeGrantTests.java @@ -122,7 +122,7 @@ public class OAuth2AuthorizationCodeGrantTests { private static ProviderSettings providerSettings; private static HttpMessageConverter accessTokenHttpResponseConverter = new OAuth2AccessTokenResponseHttpMessageConverter(); - private static String consentPage = "/custom-consent"; + private static String consentPage = "/oauth2/consent"; @Rule public final SpringTestRule spring = new SpringTestRule(); @@ -360,15 +360,13 @@ public class OAuth2AuthorizationCodeGrantTests { .getResponse() .getContentAsString(); - assertThat(consentPage).contains("Consent required"); assertThat(consentPage).contains(scopeCheckbox("message.read")); assertThat(consentPage).contains(scopeCheckbox("message.write")); } @Test - public void requestWhenConsentRequestReturnAccessTokenResponse() throws Exception { - final String stateParameter = "consent-state"; + public void requestWhenConsentRequestThenReturnAccessTokenResponse() throws Exception { this.spring.register(AuthorizationServerConfiguration.class).autowire(); RegisteredClient registeredClient = TestRegisteredClients.registeredClient() @@ -381,14 +379,15 @@ public class OAuth2AuthorizationCodeGrantTests { .build(); when(registeredClientRepository.findByClientId(eq(registeredClient.getClientId()))) .thenReturn(registeredClient); - OAuth2Authorization stateTokenAuthorization = TestOAuth2Authorizations.authorization(registeredClient) + OAuth2Authorization authorization = TestOAuth2Authorizations.authorization(registeredClient) .principalName("user") .build(); + final String stateParameter = "state"; + when(authorizationService.findByToken( - eq(stateParameter), - eq(new OAuth2TokenType(OAuth2ParameterNames.STATE)))) - .thenReturn(stateTokenAuthorization); + eq(stateParameter), eq(new OAuth2TokenType(OAuth2ParameterNames.STATE)))) + .thenReturn(authorization); MvcResult mvcResult = this.mvc.perform(post(OAuth2AuthorizationEndpointFilter.DEFAULT_AUTHORIZATION_ENDPOINT_URI) .param(OAuth2ParameterNames.CLIENT_ID, registeredClient.getClientId()) @@ -480,10 +479,10 @@ public class OAuth2AuthorizationCodeGrantTests { private static String getAuthorizationHeader(RegisteredClient registeredClient) throws Exception { String clientId = registeredClient.getClientId(); - String secret = registeredClient.getClientSecret(); + String clientSecret = registeredClient.getClientSecret(); clientId = URLEncoder.encode(clientId, StandardCharsets.UTF_8.name()); - secret = URLEncoder.encode(secret, StandardCharsets.UTF_8.name()); - String credentialsString = clientId + ":" + secret; + clientSecret = URLEncoder.encode(clientSecret, StandardCharsets.UTF_8.name()); + String credentialsString = clientId + ":" + clientSecret; byte[] encodedBytes = Base64.getEncoder().encode(credentialsString.getBytes(StandardCharsets.UTF_8)); return "Basic " + new String(encodedBytes, StandardCharsets.UTF_8); } diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/InMemoryOAuth2AuthorizationConsentServiceTest.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/InMemoryOAuth2AuthorizationConsentServiceTest.java deleted file mode 100644 index 505d8cd8..00000000 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/InMemoryOAuth2AuthorizationConsentServiceTest.java +++ /dev/null @@ -1,152 +0,0 @@ -/* - * Copyright 2020-2021 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. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.springframework.security.oauth2.server.authorization; - -import org.junit.Before; -import org.junit.Test; -import org.springframework.security.core.authority.SimpleGrantedAuthority; - -import java.util.List; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; - -/** - * Tests for {@link InMemoryOAuth2AuthorizationConsentService}. - * - * @author Daniel Garnier-Moiroux - */ -public class InMemoryOAuth2AuthorizationConsentServiceTest { - private InMemoryOAuth2AuthorizationConsentService consentService; - - private static final String CLIENT_ID = "client-id"; - private static final String PRINCIPAL_NAME = "principal-name"; - private static final OAuth2AuthorizationConsent CONSENT = OAuth2AuthorizationConsent - .withId(CLIENT_ID, PRINCIPAL_NAME) - .authority(new SimpleGrantedAuthority("some.authority")) - .build(); - - @Before - public void setUp() throws Exception { - this.consentService = new InMemoryOAuth2AuthorizationConsentService(); - this.consentService.save(CONSENT); - } - - @Test - public void constructorVaragsWhenAuthorizationConsentNullThenThrowIllegalArgumentException() { - assertThatIllegalArgumentException() - .isThrownBy(() -> new InMemoryOAuth2AuthorizationConsentService((OAuth2AuthorizationConsent) null)) - .withMessage("authorizationConsent cannot be null"); - } - - @Test - public void constructorListWhenAuthorizationConsentsNullThenThrowIllegalArgumentException() { - assertThatIllegalArgumentException() - .isThrownBy(() -> new InMemoryOAuth2AuthorizationConsentService((List) null)) - .withMessage("authorizationConsents cannot be null"); - } - - @Test - public void constructorWhenDuplicateAuthorizationConsentsThenThrowIllegalArgumentException() { - OAuth2AuthorizationConsent authorizationConsent = OAuth2AuthorizationConsent.withId("client-id", "principal-name") - .scope("thing.write") // must have at least one scope - .build(); - - assertThatIllegalArgumentException() - .isThrownBy(() -> new InMemoryOAuth2AuthorizationConsentService(authorizationConsent, authorizationConsent)) - .withMessage("The authorizationConsent must be unique. Found duplicate, with registered client id: [client-id] and principal name: [principal-name]"); - } - - @Test - public void saveWhenConsentNullThenThrowIllegalArgumentException() { - assertThatIllegalArgumentException() - .isThrownBy(() -> this.consentService.save(null)) - .withMessage("authorizationConsent cannot be null"); - } - - @Test - public void saveWhenConsentNewThenSaved() { - OAuth2AuthorizationConsent expectedConsent = OAuth2AuthorizationConsent - .withId("new-client", "new-principal") - .authority(new SimpleGrantedAuthority("new.authority")) - .build(); - - this.consentService.save(expectedConsent); - - OAuth2AuthorizationConsent consent = - this.consentService.findById("new-client", "new-principal"); - assertThat(consent).isEqualTo(expectedConsent); - } - - @Test - public void saveWhenConsentExistsThenUpdated() { - OAuth2AuthorizationConsent expectedConsent = OAuth2AuthorizationConsent - .from(CONSENT) - .authority(new SimpleGrantedAuthority("new.authority")) - .build(); - - this.consentService.save(expectedConsent); - - OAuth2AuthorizationConsent consent = - this.consentService.findById(CLIENT_ID, PRINCIPAL_NAME); - assertThat(consent).isEqualTo(expectedConsent); - assertThat(consent).isNotEqualTo(CONSENT); - - } - - @Test - public void removeNullThenThrowIllegalArgumentException() { - assertThatIllegalArgumentException() - .isThrownBy(() -> this.consentService.remove(null)) - .withMessage("authorizationConsent cannot be null"); - } - - @Test - public void removeWhenConsentProvidedThenRemoved() { - this.consentService.remove(CONSENT); - - assertThat(this.consentService.findById(CLIENT_ID, PRINCIPAL_NAME)) - .isNull(); - } - - @Test - public void findWhenRegisteredClientIdNullThenThrowIllegalArgumentException() { - assertThatIllegalArgumentException() - .isThrownBy(() -> this.consentService.findById(null, "some-user")) - .withMessage("registeredClientId cannot be empty"); - } - - @Test - public void findWhenPrincipalNameNullThenThrowIllegalArgumentException() { - assertThatIllegalArgumentException() - .isThrownBy(() -> this.consentService.findById("some-client", null)) - .withMessage("principalName cannot be empty"); - } - - @Test - public void findWhenConsentExistsThenFound() { - assertThat(this.consentService.findById(CLIENT_ID, PRINCIPAL_NAME)) - .isEqualTo(CONSENT); - } - - @Test - public void findWhenConsentDoesNotExistThenNull() { - this.consentService.save(CONSENT); - - assertThat(this.consentService.findById("unknown-client", PRINCIPAL_NAME)).isNull(); - assertThat(this.consentService.findById(CLIENT_ID, "unkown-user")).isNull(); - } -} diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/InMemoryOAuth2AuthorizationConsentServiceTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/InMemoryOAuth2AuthorizationConsentServiceTests.java new file mode 100644 index 00000000..2b7f85f2 --- /dev/null +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/InMemoryOAuth2AuthorizationConsentServiceTests.java @@ -0,0 +1,148 @@ +/* + * Copyright 2020-2021 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.security.oauth2.server.authorization; + +import java.util.List; + +import org.junit.Before; +import org.junit.Test; + +import org.springframework.security.core.authority.SimpleGrantedAuthority; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; + +/** + * Tests for {@link InMemoryOAuth2AuthorizationConsentService}. + * + * @author Daniel Garnier-Moiroux + */ +public class InMemoryOAuth2AuthorizationConsentServiceTests { + private static final String REGISTERED_CLIENT_ID = "registered-client-id"; + private static final String PRINCIPAL_NAME = "principal-name"; + private static final OAuth2AuthorizationConsent AUTHORIZATION_CONSENT = + OAuth2AuthorizationConsent.withId(REGISTERED_CLIENT_ID, PRINCIPAL_NAME) + .authority(new SimpleGrantedAuthority("some.authority")) + .build(); + + private InMemoryOAuth2AuthorizationConsentService authorizationConsentService; + + @Before + public void setUp() { + this.authorizationConsentService = new InMemoryOAuth2AuthorizationConsentService(); + this.authorizationConsentService.save(AUTHORIZATION_CONSENT); + } + + @Test + public void constructorVarargsWhenAuthorizationConsentNullThenThrowIllegalArgumentException() { + assertThatIllegalArgumentException() + .isThrownBy(() -> new InMemoryOAuth2AuthorizationConsentService((OAuth2AuthorizationConsent) null)) + .withMessage("authorizationConsent cannot be null"); + } + + @Test + public void constructorListWhenAuthorizationConsentsNullThenThrowIllegalArgumentException() { + assertThatIllegalArgumentException() + .isThrownBy(() -> new InMemoryOAuth2AuthorizationConsentService((List) null)) + .withMessage("authorizationConsents cannot be null"); + } + + @Test + public void constructorWhenDuplicateAuthorizationConsentsThenThrowIllegalArgumentException() { + assertThatIllegalArgumentException() + .isThrownBy(() -> new InMemoryOAuth2AuthorizationConsentService(AUTHORIZATION_CONSENT, AUTHORIZATION_CONSENT)) + .withMessage("The authorizationConsent must be unique. Found duplicate, with registered client id: [registered-client-id] and principal name: [principal-name]"); + } + + @Test + public void saveWhenAuthorizationConsentNullThenThrowIllegalArgumentException() { + assertThatIllegalArgumentException() + .isThrownBy(() -> this.authorizationConsentService.save(null)) + .withMessage("authorizationConsent cannot be null"); + } + + @Test + public void saveWhenAuthorizationConsentNewThenSaved() { + OAuth2AuthorizationConsent expectedAuthorizationConsent = + OAuth2AuthorizationConsent.withId("new-client", "new-principal") + .authority(new SimpleGrantedAuthority("new.authority")) + .build(); + + this.authorizationConsentService.save(expectedAuthorizationConsent); + + OAuth2AuthorizationConsent authorizationConsent = + this.authorizationConsentService.findById("new-client", "new-principal"); + assertThat(authorizationConsent).isEqualTo(expectedAuthorizationConsent); + } + + @Test + public void saveWhenAuthorizationConsentExistsThenUpdated() { + OAuth2AuthorizationConsent expectedAuthorizationConsent = + OAuth2AuthorizationConsent.from(AUTHORIZATION_CONSENT) + .authority(new SimpleGrantedAuthority("new.authority")) + .build(); + + this.authorizationConsentService.save(expectedAuthorizationConsent); + + OAuth2AuthorizationConsent authorizationConsent = + this.authorizationConsentService.findById( + AUTHORIZATION_CONSENT.getRegisteredClientId(), AUTHORIZATION_CONSENT.getPrincipalName()); + assertThat(authorizationConsent).isEqualTo(expectedAuthorizationConsent); + assertThat(authorizationConsent).isNotEqualTo(AUTHORIZATION_CONSENT); + } + + @Test + public void removeWhenAuthorizationConsentNullThenThrowIllegalArgumentException() { + assertThatIllegalArgumentException() + .isThrownBy(() -> this.authorizationConsentService.remove(null)) + .withMessage("authorizationConsent cannot be null"); + } + + @Test + public void removeWhenAuthorizationConsentProvidedThenRemoved() { + this.authorizationConsentService.remove(AUTHORIZATION_CONSENT); + assertThat(this.authorizationConsentService.findById( + AUTHORIZATION_CONSENT.getRegisteredClientId(), AUTHORIZATION_CONSENT.getPrincipalName())) + .isNull(); + } + + @Test + public void findByIdWhenRegisteredClientIdNullThenThrowIllegalArgumentException() { + assertThatIllegalArgumentException() + .isThrownBy(() -> this.authorizationConsentService.findById(null, "some-user")) + .withMessage("registeredClientId cannot be empty"); + } + + @Test + public void findByIdWhenPrincipalNameNullThenThrowIllegalArgumentException() { + assertThatIllegalArgumentException() + .isThrownBy(() -> this.authorizationConsentService.findById("some-client", null)) + .withMessage("principalName cannot be empty"); + } + + @Test + public void findByIdWhenAuthorizationConsentExistsThenFound() { + assertThat(this.authorizationConsentService.findById(REGISTERED_CLIENT_ID, PRINCIPAL_NAME)) + .isEqualTo(AUTHORIZATION_CONSENT); + } + + @Test + public void findByIdWhenAuthorizationConsentDoesNotExistThenNull() { + this.authorizationConsentService.save(AUTHORIZATION_CONSENT); + assertThat(this.authorizationConsentService.findById("unknown-client", PRINCIPAL_NAME)).isNull(); + assertThat(this.authorizationConsentService.findById(REGISTERED_CLIENT_ID, "unknown-user")).isNull(); + } +} diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/OAuth2AuthorizationConsentTest.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/OAuth2AuthorizationConsentTests.java similarity index 59% rename from oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/OAuth2AuthorizationConsentTest.java rename to oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/OAuth2AuthorizationConsentTests.java index 4772d538..af71f258 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/OAuth2AuthorizationConsentTest.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/OAuth2AuthorizationConsentTests.java @@ -16,6 +16,7 @@ package org.springframework.security.oauth2.server.authorization; import org.junit.Test; + import org.springframework.security.core.authority.SimpleGrantedAuthority; import static org.assertj.core.api.Assertions.assertThat; @@ -26,7 +27,8 @@ import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException * * @author Daniel Garnier-Moiroux */ -public class OAuth2AuthorizationConsentTest { +public class OAuth2AuthorizationConsentTests { + @Test public void fromWhenAuthorizationConsentNullThenThrowIllegalArgumentException() { assertThatIllegalArgumentException() @@ -35,14 +37,14 @@ public class OAuth2AuthorizationConsentTest { } @Test - public void withClientIdAndPrincipalWhenClientIdNullThenThrowIllegalArgumentException() { + public void withIdWhenRegisteredClientIdNullThenThrowIllegalArgumentException() { assertThatIllegalArgumentException() .isThrownBy(() -> OAuth2AuthorizationConsent.withId(null, "some-user")) .withMessage("registeredClientId cannot be empty"); } @Test - public void withClientIdAndPrincipalWhenPrincipalNullThenThrowIllegalArgumentException() { + public void withIdWhenPrincipalNameNullThenThrowIllegalArgumentException() { assertThatIllegalArgumentException() .isThrownBy(() -> OAuth2AuthorizationConsent.withId("some-client", null)) .withMessage("principalName cannot be empty"); @@ -58,21 +60,21 @@ public class OAuth2AuthorizationConsentTest { @Test public void buildWhenAllAttributesAreProvidedThenAllAttributesAreSet() { - OAuth2AuthorizationConsent consent = OAuth2AuthorizationConsent - .withId("some-client", "some-user") - .scope("resource.read") - .scope("resource.write") - .authority(new SimpleGrantedAuthority("CLAIM_email")) - .build(); - - assertThat(consent.getPrincipalName()).isEqualTo("some-user"); - assertThat(consent.getRegisteredClientId()).isEqualTo("some-client"); - assertThat(consent.getScopes()) + OAuth2AuthorizationConsent authorizationConsent = + OAuth2AuthorizationConsent.withId("some-client", "some-user") + .scope("resource.read") + .scope("resource.write") + .authority(new SimpleGrantedAuthority("CLAIM_email")) + .build(); + + assertThat(authorizationConsent.getRegisteredClientId()).isEqualTo("some-client"); + assertThat(authorizationConsent.getPrincipalName()).isEqualTo("some-user"); + assertThat(authorizationConsent.getScopes()) .containsExactlyInAnyOrder( "resource.read", "resource.write" ); - assertThat(consent.getAuthorities()) + assertThat(authorizationConsent.getAuthorities()) .containsExactlyInAnyOrder( new SimpleGrantedAuthority("SCOPE_resource.read"), new SimpleGrantedAuthority("SCOPE_resource.write"), @@ -82,18 +84,20 @@ public class OAuth2AuthorizationConsentTest { @Test public void fromWhenAuthorizationConsentProvidedThenCopied() { - OAuth2AuthorizationConsent previousConsent = OAuth2AuthorizationConsent - .withId("some-client", "some-principal") - .scope("first.scope") - .scope("second.scope") - .authority(new SimpleGrantedAuthority("CLAIM_email")) - .build(); - - OAuth2AuthorizationConsent consent = OAuth2AuthorizationConsent.from(previousConsent).build(); - - assertThat(consent.getPrincipalName()).isEqualTo("some-principal"); - assertThat(consent.getRegisteredClientId()).isEqualTo("some-client"); - assertThat(consent.getAuthorities()) + OAuth2AuthorizationConsent previousAuthorizationConsent = + OAuth2AuthorizationConsent.withId("some-client", "some-principal") + .scope("first.scope") + .scope("second.scope") + .authority(new SimpleGrantedAuthority("CLAIM_email")) + .build(); + + OAuth2AuthorizationConsent authorizationConsent = + OAuth2AuthorizationConsent.from(previousAuthorizationConsent) + .build(); + + assertThat(authorizationConsent.getRegisteredClientId()).isEqualTo("some-client"); + assertThat(authorizationConsent.getPrincipalName()).isEqualTo("some-principal"); + assertThat(authorizationConsent.getAuthorities()) .containsExactlyInAnyOrder( new SimpleGrantedAuthority("SCOPE_first.scope"), new SimpleGrantedAuthority("SCOPE_second.scope"), @@ -103,15 +107,16 @@ public class OAuth2AuthorizationConsentTest { @Test public void authoritiesThenCustomizesAuthorities() { - OAuth2AuthorizationConsent consent = OAuth2AuthorizationConsent - .withId("some-client", "some-user") - .authority(new SimpleGrantedAuthority("some.authority")) - .authorities(authorities -> { - authorities.clear(); - authorities.add(new SimpleGrantedAuthority("other.authority")); - }) - .build(); - - assertThat(consent.getAuthorities()).containsExactly(new SimpleGrantedAuthority("other.authority")); + OAuth2AuthorizationConsent authorizationConsent = + OAuth2AuthorizationConsent.withId("some-client", "some-user") + .authority(new SimpleGrantedAuthority("some.authority")) + .authorities(authorities -> { + authorities.clear(); + authorities.add(new SimpleGrantedAuthority("other.authority")); + }) + .build(); + + assertThat(authorizationConsent.getAuthorities()).containsExactly(new SimpleGrantedAuthority("other.authority")); } + } diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilterTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilterTests.java index 37293a7d..1dd20ef7 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilterTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilterTests.java @@ -33,6 +33,7 @@ import org.junit.After; import org.junit.Before; import org.junit.Test; import org.mockito.ArgumentCaptor; + import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; @@ -88,16 +89,17 @@ public class OAuth2AuthorizationEndpointFilterTests { private static final String PKCE_ERROR_URI = "https://tools.ietf.org/html/rfc7636%23section-4.4.1"; private RegisteredClientRepository registeredClientRepository; private OAuth2AuthorizationService authorizationService; + private OAuth2AuthorizationConsentService authorizationConsentService; private OAuth2AuthorizationEndpointFilter filter; - private OAuth2AuthorizationConsentService consentService; private TestingAuthenticationToken authentication; @Before public void setUp() { this.registeredClientRepository = mock(RegisteredClientRepository.class); this.authorizationService = mock(OAuth2AuthorizationService.class); - this.consentService = mock(OAuth2AuthorizationConsentService.class); - this.filter = new OAuth2AuthorizationEndpointFilter(this.registeredClientRepository, this.authorizationService, this.consentService); + this.authorizationConsentService = mock(OAuth2AuthorizationConsentService.class); + this.filter = new OAuth2AuthorizationEndpointFilter( + this.registeredClientRepository, this.authorizationService, this.authorizationConsentService); this.authentication = new TestingAuthenticationToken("principalName", "password"); this.authentication.setAuthenticated(true); SecurityContext securityContext = SecurityContextHolder.createEmptyContext(); @@ -112,28 +114,28 @@ public class OAuth2AuthorizationEndpointFilterTests { @Test public void constructorWhenRegisteredClientRepositoryNullThenThrowIllegalArgumentException() { - assertThatThrownBy(() -> new OAuth2AuthorizationEndpointFilter(null, this.authorizationService, this.consentService)) + assertThatThrownBy(() -> new OAuth2AuthorizationEndpointFilter(null, this.authorizationService, this.authorizationConsentService)) .isInstanceOf(IllegalArgumentException.class) .hasMessage("registeredClientRepository cannot be null"); } @Test public void constructorWhenAuthorizationServiceNullThenThrowIllegalArgumentException() { - assertThatThrownBy(() -> new OAuth2AuthorizationEndpointFilter(this.registeredClientRepository, null, this.consentService)) + assertThatThrownBy(() -> new OAuth2AuthorizationEndpointFilter(this.registeredClientRepository, null, this.authorizationConsentService)) .isInstanceOf(IllegalArgumentException.class) .hasMessage("authorizationService cannot be null"); } @Test - public void constructorWhenConsentServiceNullThenThrowIllegalArgumentException() { + public void constructorWhenAuthorizationConsentServiceNullThenThrowIllegalArgumentException() { assertThatThrownBy(() -> new OAuth2AuthorizationEndpointFilter(this.registeredClientRepository, this.authorizationService, (OAuth2AuthorizationConsentService) null)) .isInstanceOf(IllegalArgumentException.class) - .hasMessage("consentService cannot be null"); + .hasMessage("authorizationConsentService cannot be null"); } @Test public void constructorWhenAuthorizationEndpointUriNullThenThrowIllegalArgumentException() { - assertThatThrownBy(() -> new OAuth2AuthorizationEndpointFilter(this.registeredClientRepository, this.authorizationService, this.consentService, null)) + assertThatThrownBy(() -> new OAuth2AuthorizationEndpointFilter(this.registeredClientRepository, this.authorizationService, this.authorizationConsentService, null)) .isInstanceOf(IllegalArgumentException.class) .hasMessage("authorizationEndpointUri cannot be empty"); } @@ -592,7 +594,7 @@ public class OAuth2AuthorizationEndpointFilterTests { RegisteredClient registeredClient = TestRegisteredClients.registeredClient() .clientSettings(clientSettings -> clientSettings.requireUserConsent(true)) .build(); - when(this.registeredClientRepository.findByClientId((eq(registeredClient.getClientId())))) + when(this.registeredClientRepository.findByClientId(eq(registeredClient.getClientId()))) .thenReturn(registeredClient); MockHttpServletRequest request = createAuthorizationRequest(registeredClient); @@ -601,6 +603,8 @@ public class OAuth2AuthorizationEndpointFilterTests { this.filter.doFilter(request, response, filterChain); + verifyNoInteractions(filterChain); + ArgumentCaptor authorizationCaptor = ArgumentCaptor.forClass(OAuth2Authorization.class); verify(this.authorizationService).save(authorizationCaptor.capture()); @@ -640,7 +644,7 @@ public class OAuth2AuthorizationEndpointFilterTests { }) .clientSettings(clientSettings -> clientSettings.requireUserConsent(true)) .build(); - when(this.registeredClientRepository.findByClientId((eq(registeredClient.getClientId())))) + when(this.registeredClientRepository.findByClientId(eq(registeredClient.getClientId()))) .thenReturn(registeredClient); MockHttpServletRequest request = createAuthorizationRequest(registeredClient); @@ -656,8 +660,6 @@ public class OAuth2AuthorizationEndpointFilterTests { assertThat(response.getContentAsString()).contains(scopeCheckbox("message.read")); assertThat(response.getContentAsString()).contains(scopeCheckbox("message.write")); - - verifyNoInteractions(filterChain); } @Test @@ -673,19 +675,17 @@ public class OAuth2AuthorizationEndpointFilterTests { }) .clientSettings(clientSettings -> clientSettings.requireUserConsent(true)) .build(); - when(this.registeredClientRepository.findByClientId((eq(registeredClient.getClientId())))) + when(this.registeredClientRepository.findByClientId(eq(registeredClient.getClientId()))) .thenReturn(registeredClient); - OAuth2AuthorizationConsent previousConsent = createConsent( + OAuth2AuthorizationConsent previousConsent = createAuthorizationConsent( registeredClient.getClientId(), this.authentication.getName(), Arrays.asList(previouslyApprovedScope, unrelatedPreviouslyApprovedScope) ); - when(this.consentService.findById( - eq(registeredClient.getClientId()), - eq(this.authentication.getName()))) + when(this.authorizationConsentService.findById( + eq(registeredClient.getId()), eq(this.authentication.getName()))) .thenReturn(previousConsent); - MockHttpServletRequest request = createAuthorizationRequest(registeredClient); MockHttpServletResponse response = new MockHttpServletResponse(); FilterChain filterChain = mock(FilterChain.class); @@ -704,7 +704,7 @@ public class OAuth2AuthorizationEndpointFilterTests { @Test public void doFilterWhenUserConsentRequiredAndCustomConsentUriAndAuthorizationRequestThenRedirects() throws Exception { - this.filter.setUserConsentUri("/consent"); + this.filter.setUserConsentUri("/oauth2/consent"); RegisteredClient registeredClient = TestRegisteredClients.registeredClient() .scopes(scopes -> { scopes.clear(); @@ -713,7 +713,7 @@ public class OAuth2AuthorizationEndpointFilterTests { }) .clientSettings(clientSettings -> clientSettings.requireUserConsent(true)) .build(); - when(this.registeredClientRepository.findByClientId((eq(registeredClient.getClientId())))) + when(this.registeredClientRepository.findByClientId(eq(registeredClient.getClientId()))) .thenReturn(registeredClient); MockHttpServletRequest request = createAuthorizationRequest(registeredClient); @@ -736,7 +736,7 @@ public class OAuth2AuthorizationEndpointFilterTests { String[] redirectScopes = consentRedirectUri.getQueryParams().getFirst(OAuth2ParameterNames.SCOPE).split(" "); String redirectState = consentRedirectUri.getQueryParams().getFirst(OAuth2ParameterNames.STATE); - assertThat(consentRedirectUri.getPath()).isEqualTo("/consent"); + assertThat(consentRedirectUri.getPath()).isEqualTo("/oauth2/consent"); assertThat(consentRedirectUri.getQueryParams().getFirst(OAuth2ParameterNames.CLIENT_ID)).isEqualTo(registeredClient.getClientId()); assertThat(redirectScopes).containsExactlyInAnyOrder("message.read", "message.write"); assertThat(redirectState).isEqualTo(authorization.getAttribute(OAuth2ParameterNames.STATE)); @@ -752,15 +752,14 @@ public class OAuth2AuthorizationEndpointFilterTests { }) .clientSettings(clientSettings -> clientSettings.requireUserConsent(true)) .build(); - when(this.registeredClientRepository.findByClientId((eq(registeredClient.getClientId())))) + when(this.registeredClientRepository.findByClientId(eq(registeredClient.getClientId()))) .thenReturn(registeredClient); - OAuth2AuthorizationConsent previousConsent = createConsent( + OAuth2AuthorizationConsent authorizationConsent = createAuthorizationConsent( registeredClient.getClientId(), this.authentication.getName(), Arrays.asList("message.read", "message.write") ); - when(this.consentService.findById( - eq(registeredClient.getClientId()), - eq(this.authentication.getName()))) - .thenReturn(previousConsent); + when(this.authorizationConsentService.findById( + eq(registeredClient.getId()), eq(this.authentication.getName()))) + .thenReturn(authorizationConsent); MockHttpServletRequest request = createAuthorizationRequest(registeredClient); MockHttpServletResponse response = new MockHttpServletResponse(); @@ -1011,7 +1010,7 @@ public class OAuth2AuthorizationEndpointFilterTests { .build(); when(this.authorizationService.findByToken(eq("state"), eq(STATE_TOKEN_TYPE))) .thenReturn(authorization); - when(this.registeredClientRepository.findByClientId((eq(registeredClient.getClientId())))) + when(this.registeredClientRepository.findByClientId(eq(registeredClient.getClientId()))) .thenReturn(registeredClient); MockHttpServletRequest request = createUserConsentRequest(registeredClient); @@ -1020,13 +1019,13 @@ public class OAuth2AuthorizationEndpointFilterTests { this.filter.doFilter(request, response, filterChain); - ArgumentCaptor consentCaptor = ArgumentCaptor.forClass(OAuth2AuthorizationConsent.class); + ArgumentCaptor authorizationConsentCaptor = ArgumentCaptor.forClass(OAuth2AuthorizationConsent.class); - verify(this.consentService).save(consentCaptor.capture()); - OAuth2AuthorizationConsent consent = consentCaptor.getValue(); - assertThat(consent.getPrincipalName()).isEqualTo(this.authentication.getName()); - assertThat(consent.getRegisteredClientId()).isEqualTo(registeredClient.getClientId()); - assertThat(consent.getScopes()).containsExactlyInAnyOrder("message.read", "message.write"); + verify(this.authorizationConsentService).save(authorizationConsentCaptor.capture()); + OAuth2AuthorizationConsent authorizationConsent = authorizationConsentCaptor.getValue(); + assertThat(authorizationConsent.getPrincipalName()).isEqualTo(this.authentication.getName()); + assertThat(authorizationConsent.getRegisteredClientId()).isEqualTo(registeredClient.getId()); + assertThat(authorizationConsent.getScopes()).containsExactlyInAnyOrder("message.read", "message.write"); } @Test @@ -1041,7 +1040,7 @@ public class OAuth2AuthorizationEndpointFilterTests { .build(); when(this.authorizationService.findByToken(eq("state"), eq(STATE_TOKEN_TYPE))) .thenReturn(authorization); - when(this.registeredClientRepository.findByClientId((eq(registeredClient.getClientId())))) + when(this.registeredClientRepository.findByClientId(eq(registeredClient.getClientId()))) .thenReturn(registeredClient); MockHttpServletRequest request = createUserConsentRequest(registeredClient); @@ -1050,7 +1049,7 @@ public class OAuth2AuthorizationEndpointFilterTests { this.filter.doFilter(request, response, filterChain); - verify(this.consentService, never()).save(any()); + verify(this.authorizationConsentService, never()).save(any()); } @Test @@ -1069,18 +1068,17 @@ public class OAuth2AuthorizationEndpointFilterTests { .build(); when(this.authorizationService.findByToken(eq("state"), eq(STATE_TOKEN_TYPE))) .thenReturn(authorization); - when(this.registeredClientRepository.findByClientId((eq(registeredClient.getClientId())))) + when(this.registeredClientRepository.findByClientId(eq(registeredClient.getClientId()))) .thenReturn(registeredClient); - OAuth2AuthorizationConsent previousConsent = - createConsent( + OAuth2AuthorizationConsent previousAuthorizationConsent = + createAuthorizationConsent( registeredClient.getClientId(), this.authentication.getName(), Collections.singleton("message.read") ); - when(this.consentService.findById( - eq(registeredClient.getClientId()), - eq(this.authentication.getName()))) - .thenReturn(previousConsent); + when(this.authorizationConsentService.findById( + eq(registeredClient.getId()), eq(this.authentication.getName()))) + .thenReturn(previousAuthorizationConsent); MockHttpServletRequest request = createUserConsentRequest(registeredClient); MockHttpServletResponse response = new MockHttpServletResponse(); @@ -1088,13 +1086,13 @@ public class OAuth2AuthorizationEndpointFilterTests { this.filter.doFilter(request, response, filterChain); - ArgumentCaptor consentCaptor = ArgumentCaptor.forClass(OAuth2AuthorizationConsent.class); + ArgumentCaptor authorizationConsentCaptor = ArgumentCaptor.forClass(OAuth2AuthorizationConsent.class); - verify(this.consentService).save(consentCaptor.capture()); - OAuth2AuthorizationConsent consent = consentCaptor.getValue(); - assertThat(consent.getPrincipalName()).isEqualTo(this.authentication.getName()); - assertThat(consent.getRegisteredClientId()).isEqualTo(registeredClient.getClientId()); - assertThat(consent.getScopes()).containsExactlyInAnyOrder("message.read", "message.write"); + verify(this.authorizationConsentService).save(authorizationConsentCaptor.capture()); + OAuth2AuthorizationConsent authorizationConsent = authorizationConsentCaptor.getValue(); + assertThat(authorizationConsent.getRegisteredClientId()).isEqualTo(registeredClient.getClientId()); + assertThat(authorizationConsent.getPrincipalName()).isEqualTo(this.authentication.getName()); + assertThat(authorizationConsent.getScopes()).containsExactlyInAnyOrder("message.read", "message.write"); } @Test @@ -1116,18 +1114,17 @@ public class OAuth2AuthorizationEndpointFilterTests { .build(); when(this.authorizationService.findByToken(eq("state"), eq(STATE_TOKEN_TYPE))) .thenReturn(authorization); - when(this.registeredClientRepository.findByClientId((eq(registeredClient.getClientId())))) + when(this.registeredClientRepository.findByClientId(eq(registeredClient.getClientId()))) .thenReturn(registeredClient); - OAuth2AuthorizationConsent previousConsent = - createConsent( + OAuth2AuthorizationConsent previousAuthorizationConsent = + createAuthorizationConsent( registeredClient.getClientId(), this.authentication.getName(), Arrays.asList(previouslyApprovedScope, unrelatedPreviouslyApprovedScope) ); - when(this.consentService.findById( - eq(registeredClient.getClientId()), - eq(this.authentication.getName()))) - .thenReturn(previousConsent); + when(this.authorizationConsentService.findById( + eq(registeredClient.getId()), eq(this.authentication.getName()))) + .thenReturn(previousAuthorizationConsent); MockHttpServletRequest request = createUserConsentRequest(registeredClient); MockHttpServletResponse response = new MockHttpServletResponse(); @@ -1313,18 +1310,14 @@ public class OAuth2AuthorizationEndpointFilterTests { request.addParameter(PkceParameterNames.CODE_CHALLENGE_METHOD, "S256"); } - private static OAuth2AuthorizationConsent createConsent( - String registeredClientId, - String prinicpalName, - Collection scopes - ) { - OAuth2AuthorizationConsent.Builder consentBuilder = OAuth2AuthorizationConsent - .withId(registeredClientId, prinicpalName); + private static OAuth2AuthorizationConsent createAuthorizationConsent(String registeredClientId, + String principalName, Collection scopes) { + OAuth2AuthorizationConsent.Builder authorizationConsentBuilder = + OAuth2AuthorizationConsent.withId(registeredClientId, principalName); for (String scope : scopes) { - consentBuilder.scope(scope); + authorizationConsentBuilder.scope(scope); } - return consentBuilder.build(); - + return authorizationConsentBuilder.build(); } private static MockHttpServletRequest createUserConsentRequest(RegisteredClient registeredClient) { @@ -1357,4 +1350,5 @@ public class OAuth2AuthorizationEndpointFilterTests { scope ); } + } diff --git a/samples/boot/oauth2-integration/authorizationserver-custom-consent-page/src/main/java/sample/OAuth2AuthorizationServerCustomConsentPageApplication.java b/samples/boot/oauth2-integration/authorizationserver-custom-consent-page/src/main/java/sample/OAuth2AuthorizationServerCustomConsentPageApplication.java index 2ea9a1b9..1c56d205 100644 --- a/samples/boot/oauth2-integration/authorizationserver-custom-consent-page/src/main/java/sample/OAuth2AuthorizationServerCustomConsentPageApplication.java +++ b/samples/boot/oauth2-integration/authorizationserver-custom-consent-page/src/main/java/sample/OAuth2AuthorizationServerCustomConsentPageApplication.java @@ -1,5 +1,5 @@ /* - * Copyright 2020 the original author or authors. + * Copyright 2020-2021 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. @@ -20,7 +20,6 @@ import org.springframework.boot.autoconfigure.SpringBootApplication; /** * @author Daniel Garnier-Moiroux - * @since 0.1.2 */ @SpringBootApplication public class OAuth2AuthorizationServerCustomConsentPageApplication { diff --git a/samples/boot/oauth2-integration/authorizationserver-custom-consent-page/src/main/java/sample/config/AuthorizationServerConfig.java b/samples/boot/oauth2-integration/authorizationserver-custom-consent-page/src/main/java/sample/config/AuthorizationServerConfig.java index 1022ebbb..f2981971 100644 --- a/samples/boot/oauth2-integration/authorizationserver-custom-consent-page/src/main/java/sample/config/AuthorizationServerConfig.java +++ b/samples/boot/oauth2-integration/authorizationserver-custom-consent-page/src/main/java/sample/config/AuthorizationServerConfig.java @@ -15,10 +15,14 @@ */ package sample.config; +import java.util.UUID; + import com.nimbusds.jose.jwk.JWKSet; import com.nimbusds.jose.jwk.RSAKey; import com.nimbusds.jose.jwk.source.JWKSource; import com.nimbusds.jose.proc.SecurityContext; +import sample.jose.Jwks; + import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.core.Ordered; @@ -37,14 +41,10 @@ import org.springframework.security.oauth2.server.authorization.client.Registere import org.springframework.security.oauth2.server.authorization.config.ProviderSettings; import org.springframework.security.web.SecurityFilterChain; import org.springframework.security.web.util.matcher.RequestMatcher; -import sample.jose.Jwks; - -import java.util.UUID; /** * @author Joe Grandja * @author Daniel Garnier-Moiroux - * @since 0.0.1 */ @Configuration(proxyBeanMethods = false) public class AuthorizationServerConfig { @@ -54,7 +54,7 @@ public class AuthorizationServerConfig { public SecurityFilterChain authorizationServerSecurityFilterChain(HttpSecurity http) throws Exception { OAuth2AuthorizationServerConfigurer authorizationServerConfigurer = new OAuth2AuthorizationServerConfigurer<>(); - authorizationServerConfigurer.consentPage("/consent"); + authorizationServerConfigurer.consentPage("/oauth2/consent"); RequestMatcher endpointsMatcher = authorizationServerConfigurer .getEndpointsMatcher(); @@ -102,8 +102,9 @@ public class AuthorizationServerConfig { } @Bean - public OAuth2AuthorizationConsentService authorizationService() { + public OAuth2AuthorizationConsentService authorizationConsentService() { // Will be used by the ConsentController return new InMemoryOAuth2AuthorizationConsentService(); } + } diff --git a/samples/boot/oauth2-integration/authorizationserver-custom-consent-page/src/main/java/sample/config/DefaultSecurityConfig.java b/samples/boot/oauth2-integration/authorizationserver-custom-consent-page/src/main/java/sample/config/DefaultSecurityConfig.java index 9d569197..e70d509d 100644 --- a/samples/boot/oauth2-integration/authorizationserver-custom-consent-page/src/main/java/sample/config/DefaultSecurityConfig.java +++ b/samples/boot/oauth2-integration/authorizationserver-custom-consent-page/src/main/java/sample/config/DefaultSecurityConfig.java @@ -1,5 +1,5 @@ /* - * Copyright 2020 the original author or authors. + * Copyright 2020-2021 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. @@ -28,12 +28,11 @@ import static org.springframework.security.config.Customizer.withDefaults; /** * @author Joe Grandja - * @since 0.1.0 */ @EnableWebSecurity public class DefaultSecurityConfig { - // formatter:off + // @formatter:off @Bean SecurityFilterChain defaultSecurityFilterChain(HttpSecurity http) throws Exception { http @@ -43,7 +42,7 @@ public class DefaultSecurityConfig { .formLogin(withDefaults()); return http.build(); } - // formatter:on + // @formatter:on // @formatter:off @Bean diff --git a/samples/boot/oauth2-integration/authorizationserver-custom-consent-page/src/main/java/sample/jose/Jwks.java b/samples/boot/oauth2-integration/authorizationserver-custom-consent-page/src/main/java/sample/jose/Jwks.java index 70dc6581..1f936056 100644 --- a/samples/boot/oauth2-integration/authorizationserver-custom-consent-page/src/main/java/sample/jose/Jwks.java +++ b/samples/boot/oauth2-integration/authorizationserver-custom-consent-page/src/main/java/sample/jose/Jwks.java @@ -15,12 +15,6 @@ */ package sample.jose; -import com.nimbusds.jose.jwk.Curve; -import com.nimbusds.jose.jwk.ECKey; -import com.nimbusds.jose.jwk.OctetSequenceKey; -import com.nimbusds.jose.jwk.RSAKey; - -import javax.crypto.SecretKey; import java.security.KeyPair; import java.security.interfaces.ECPrivateKey; import java.security.interfaces.ECPublicKey; @@ -28,9 +22,15 @@ import java.security.interfaces.RSAPrivateKey; import java.security.interfaces.RSAPublicKey; import java.util.UUID; +import javax.crypto.SecretKey; + +import com.nimbusds.jose.jwk.Curve; +import com.nimbusds.jose.jwk.ECKey; +import com.nimbusds.jose.jwk.OctetSequenceKey; +import com.nimbusds.jose.jwk.RSAKey; + /** * @author Joe Grandja - * @since 0.1.0 */ public final class Jwks { diff --git a/samples/boot/oauth2-integration/authorizationserver-custom-consent-page/src/main/java/sample/jose/KeyGeneratorUtils.java b/samples/boot/oauth2-integration/authorizationserver-custom-consent-page/src/main/java/sample/jose/KeyGeneratorUtils.java index 3d8b911f..b101062e 100644 --- a/samples/boot/oauth2-integration/authorizationserver-custom-consent-page/src/main/java/sample/jose/KeyGeneratorUtils.java +++ b/samples/boot/oauth2-integration/authorizationserver-custom-consent-page/src/main/java/sample/jose/KeyGeneratorUtils.java @@ -15,8 +15,6 @@ */ package sample.jose; -import javax.crypto.KeyGenerator; -import javax.crypto.SecretKey; import java.math.BigInteger; import java.security.KeyPair; import java.security.KeyPairGenerator; @@ -25,9 +23,11 @@ import java.security.spec.ECParameterSpec; import java.security.spec.ECPoint; import java.security.spec.EllipticCurve; +import javax.crypto.KeyGenerator; +import javax.crypto.SecretKey; + /** * @author Joe Grandja - * @since 0.1.0 */ final class KeyGeneratorUtils { diff --git a/samples/boot/oauth2-integration/authorizationserver-custom-consent-page/src/main/java/sample/web/ConsentController.java b/samples/boot/oauth2-integration/authorizationserver-custom-consent-page/src/main/java/sample/web/AuthorizationConsentController.java similarity index 65% rename from samples/boot/oauth2-integration/authorizationserver-custom-consent-page/src/main/java/sample/web/ConsentController.java rename to samples/boot/oauth2-integration/authorizationserver-custom-consent-page/src/main/java/sample/web/AuthorizationConsentController.java index 4703b44a..9aa4f8f5 100644 --- a/samples/boot/oauth2-integration/authorizationserver-custom-consent-page/src/main/java/sample/web/ConsentController.java +++ b/samples/boot/oauth2-integration/authorizationserver-custom-consent-page/src/main/java/sample/web/AuthorizationConsentController.java @@ -16,6 +16,7 @@ package sample.web; import java.security.Principal; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.Map; @@ -25,6 +26,8 @@ import java.util.stream.Collectors; import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames; import org.springframework.security.oauth2.server.authorization.OAuth2AuthorizationConsent; import org.springframework.security.oauth2.server.authorization.OAuth2AuthorizationConsentService; +import org.springframework.security.oauth2.server.authorization.client.RegisteredClient; +import org.springframework.security.oauth2.server.authorization.client.RegisteredClientRepository; import org.springframework.stereotype.Controller; import org.springframework.ui.Model; import org.springframework.util.StringUtils; @@ -33,39 +36,46 @@ import org.springframework.web.bind.annotation.RequestParam; /** * @author Daniel Garnier-Moiroux - * @since 0.1.2 */ @Controller -public class ConsentController { - +public class AuthorizationConsentController { + private final RegisteredClientRepository registeredClientRepository; private final OAuth2AuthorizationConsentService authorizationConsentService; - public ConsentController(OAuth2AuthorizationConsentService authorizationConsentService) { + public AuthorizationConsentController(RegisteredClientRepository registeredClientRepository, + OAuth2AuthorizationConsentService authorizationConsentService) { + this.registeredClientRepository = registeredClientRepository; this.authorizationConsentService = authorizationConsentService; } - @GetMapping(value = "/consent") - public String consent( - Principal principal, - Model model, - @RequestParam(OAuth2ParameterNames.SCOPE) String scope, + @GetMapping(value = "/oauth2/consent") + public String consent(Principal principal, Model model, @RequestParam(OAuth2ParameterNames.CLIENT_ID) String clientId, - @RequestParam(OAuth2ParameterNames.STATE) String state - ) { + @RequestParam(OAuth2ParameterNames.SCOPE) String scope, + @RequestParam(OAuth2ParameterNames.STATE) String state) { + // Remove scopes that were already approved Set scopesToApprove = new HashSet<>(); Set previouslyApprovedScopes = new HashSet<>(); - OAuth2AuthorizationConsent previousConsent = this.authorizationConsentService.findById(clientId, principal.getName()); - for (String scopeFromRequest : StringUtils.delimitedListToStringArray(scope, " ")) { - if (previousConsent != null && previousConsent.getScopes().contains(scopeFromRequest)) { - previouslyApprovedScopes.add(scopeFromRequest); + RegisteredClient registeredClient = this.registeredClientRepository.findByClientId(clientId); + OAuth2AuthorizationConsent currentAuthorizationConsent = + this.authorizationConsentService.findById(registeredClient.getId(), principal.getName()); + Set authorizedScopes; + if (currentAuthorizationConsent != null) { + authorizedScopes = currentAuthorizationConsent.getScopes(); + } else { + authorizedScopes = Collections.emptySet(); + } + for (String requestedScope : StringUtils.delimitedListToStringArray(scope, " ")) { + if (authorizedScopes.contains(requestedScope)) { + previouslyApprovedScopes.add(requestedScope); } else { - scopesToApprove.add(scopeFromRequest); + scopesToApprove.add(requestedScope); } } - model.addAttribute("state", state); model.addAttribute("clientId", clientId); + model.addAttribute("state", state); model.addAttribute("scopes", withDescription(scopesToApprove)); model.addAttribute("previouslyApprovedScopes", withDescription(previouslyApprovedScopes)); model.addAttribute("principalName", principal.getName()); @@ -73,18 +83,15 @@ public class ConsentController { return "consent"; } - private Set withDescription(Set scopes) { + private static Set withDescription(Set scopes) { return scopes .stream() .map(ScopeWithDescription::new) .collect(Collectors.toSet()); } - private static class ScopeWithDescription { - public final String scope; - public final String description; - - private final static String DEFAULT_DESCRIPTION = "UNKNOWN SCOPE - We cannot provide information about this permission, use caution when granting this."; + public static class ScopeWithDescription { + private static final String DEFAULT_DESCRIPTION = "UNKNOWN SCOPE - We cannot provide information about this permission, use caution when granting this."; private static final Map scopeDescriptions = new HashMap<>(); static { scopeDescriptions.put( @@ -101,6 +108,9 @@ public class ConsentController { ); } + public final String scope; + public final String description; + ScopeWithDescription(String scope) { this.scope = scope; this.description = scopeDescriptions.getOrDefault(scope, DEFAULT_DESCRIPTION);