From 0dfe5cb44a91f69110c37b47b857340f4ea33509 Mon Sep 17 00:00:00 2001 From: Dmitriy Dubson Date: Thu, 19 Aug 2021 14:33:21 -0400 Subject: [PATCH] Fix cancel consent functionality on default consent page - Fix also applies to custom consent sample Closes gh-393 --- .../OAuth2AuthorizationEndpointFilter.java | 15 +- .../OAuth2AuthorizationCodeGrantTests.java | 3 +- ...Auth2AuthorizationEndpointFilterTests.java | 3 +- ...orizationserver-custom-consent-page.gradle | 4 + .../config/AuthorizationServerConfig.java | 6 +- .../src/main/resources/templates/consent.html | 14 +- ...rverCustomConsentPageApplicationTests.java | 132 ++++++++++++++++++ ...uth2-integrated-authorizationserver.gradle | 1 + ...OAuth2AuthorizationServerConsentTests.java | 119 ++++++++++++++++ 9 files changed, 284 insertions(+), 13 deletions(-) create mode 100644 samples/boot/oauth2-integration/authorizationserver-custom-consent-page/src/test/java/sample/OAuth2AuthorizationServerCustomConsentPageApplicationTests.java create mode 100644 samples/boot/oauth2-integration/authorizationserver/src/test/java/sample/OAuth2AuthorizationServerConsentTests.java 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 c280db99..1790d8fd 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 @@ -65,6 +65,7 @@ import org.springframework.web.util.UriComponentsBuilder; * @author Paurav Munshi * @author Daniel Garnier-Moiroux * @author Anoop Garlapati + * @author Dmitriy Dubson * @since 0.0.1 * @see AuthenticationManager * @see OAuth2AuthorizationCodeRequestAuthenticationProvider @@ -332,6 +333,12 @@ public final class OAuth2AuthorizationEndpointFilter extends OncePerRequestFilte builder.append(" "); builder.append(" "); builder.append(" Consent required"); + builder.append(" "); builder.append(""); builder.append(""); builder.append("
"); @@ -350,13 +357,13 @@ public final class OAuth2AuthorizationEndpointFilter extends OncePerRequestFilte builder.append("
"); builder.append("
"); builder.append("
"); - builder.append("
"); + builder.append(" "); builder.append(" "); builder.append(" "); for (String scope : scopesToAuthorize) { builder.append("
"); - builder.append(" "); + builder.append(" "); builder.append(" "); builder.append("
"); } @@ -372,10 +379,10 @@ public final class OAuth2AuthorizationEndpointFilter extends OncePerRequestFilte } builder.append("
"); - builder.append(" "); + builder.append(" "); builder.append("
"); builder.append("
"); - builder.append(" "); + builder.append(" "); 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 0bd9a4a7..760847d8 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,6 +122,7 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers. * * @author Joe Grandja * @author Daniel Garnier-Moiroux + * @author Dmitriy Dubson */ public class OAuth2AuthorizationCodeGrantTests { private static final String DEFAULT_AUTHORIZATION_ENDPOINT_URI = "/oauth2/authorize"; @@ -556,7 +557,7 @@ public class OAuth2AuthorizationCodeGrantTests { private static String scopeCheckbox(String scope) { return MessageFormat.format( - "", + "", scope ); } 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 77488125..26304eeb 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 @@ -73,6 +73,7 @@ import static org.mockito.Mockito.when; * @author Joe Grandja * @author Daniel Garnier-Moiroux * @author Anoop Garlapati + * @author Dmitriy Dubson * @since 0.0.1 */ public class OAuth2AuthorizationEndpointFilterTests { @@ -574,7 +575,7 @@ public class OAuth2AuthorizationEndpointFilterTests { private static String scopeCheckbox(String scope) { return MessageFormat.format( - "", + "", scope ); } diff --git a/samples/boot/oauth2-integration/authorizationserver-custom-consent-page/spring-security-samples-boot-oauth2-integrated-authorizationserver-custom-consent-page.gradle b/samples/boot/oauth2-integration/authorizationserver-custom-consent-page/spring-security-samples-boot-oauth2-integrated-authorizationserver-custom-consent-page.gradle index 744b63d1..b58a36aa 100644 --- a/samples/boot/oauth2-integration/authorizationserver-custom-consent-page/spring-security-samples-boot-oauth2-integrated-authorizationserver-custom-consent-page.gradle +++ b/samples/boot/oauth2-integration/authorizationserver-custom-consent-page/spring-security-samples-boot-oauth2-integrated-authorizationserver-custom-consent-page.gradle @@ -5,4 +5,8 @@ dependencies { compile 'org.springframework.boot:spring-boot-starter-thymeleaf' compile 'org.springframework.boot:spring-boot-starter-security' compile project(':spring-security-oauth2-authorization-server') + + testCompile 'org.springframework.boot:spring-boot-starter-test' + testCompile 'org.springframework.security:spring-security-test' + testCompile 'net.sourceforge.htmlunit:htmlunit' } 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 e6c59264..f4f27ccb 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 @@ -21,8 +21,6 @@ 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; @@ -42,6 +40,7 @@ import org.springframework.security.oauth2.server.authorization.config.ClientSet 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; /** * @author Joe Grandja @@ -49,6 +48,7 @@ import org.springframework.security.web.util.matcher.RequestMatcher; */ @Configuration(proxyBeanMethods = false) public class AuthorizationServerConfig { + private static final String CUSTOM_CONSENT_PAGE_URI = "/oauth2/consent"; @Bean @Order(Ordered.HIGHEST_PRECEDENCE) @@ -57,7 +57,7 @@ public class AuthorizationServerConfig { new OAuth2AuthorizationServerConfigurer<>(); authorizationServerConfigurer .authorizationEndpoint(authorizationEndpoint -> - authorizationEndpoint.consentPage("/oauth2/consent")); + authorizationEndpoint.consentPage(CUSTOM_CONSENT_PAGE_URI)); RequestMatcher endpointsMatcher = authorizationServerConfigurer .getEndpointsMatcher(); diff --git a/samples/boot/oauth2-integration/authorizationserver-custom-consent-page/src/main/resources/templates/consent.html b/samples/boot/oauth2-integration/authorizationserver-custom-consent-page/src/main/resources/templates/consent.html index 59d52628..1fda2b50 100644 --- a/samples/boot/oauth2-integration/authorizationserver-custom-consent-page/src/main/resources/templates/consent.html +++ b/samples/boot/oauth2-integration/authorizationserver-custom-consent-page/src/main/resources/templates/consent.html @@ -5,12 +5,18 @@ - Consent required + Custom consent page - Consent required +
@@ -33,7 +39,7 @@
-
+ @@ -59,12 +65,12 @@
-
-
diff --git a/samples/boot/oauth2-integration/authorizationserver-custom-consent-page/src/test/java/sample/OAuth2AuthorizationServerCustomConsentPageApplicationTests.java b/samples/boot/oauth2-integration/authorizationserver-custom-consent-page/src/test/java/sample/OAuth2AuthorizationServerCustomConsentPageApplicationTests.java new file mode 100644 index 00000000..55bb805e --- /dev/null +++ b/samples/boot/oauth2-integration/authorizationserver-custom-consent-page/src/test/java/sample/OAuth2AuthorizationServerCustomConsentPageApplicationTests.java @@ -0,0 +1,132 @@ +/* + * 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 sample; + +import java.io.IOException; +import java.util.List; +import java.util.stream.Collectors; + +import com.gargoylesoftware.htmlunit.WebClient; +import com.gargoylesoftware.htmlunit.WebResponse; +import com.gargoylesoftware.htmlunit.html.DomElement; +import com.gargoylesoftware.htmlunit.html.HtmlCheckBoxInput; +import com.gargoylesoftware.htmlunit.html.HtmlPage; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.context.TestConfiguration; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Import; +import org.springframework.http.HttpStatus; +import org.springframework.security.oauth2.server.authorization.InMemoryOAuth2AuthorizationService; +import org.springframework.security.oauth2.server.authorization.OAuth2AuthorizationConsentService; +import org.springframework.security.oauth2.server.authorization.OAuth2AuthorizationService; +import org.springframework.security.test.context.support.WithMockUser; +import org.springframework.test.context.junit4.SpringRunner; +import org.springframework.web.util.UriComponentsBuilder; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.when; + +/** + * Consent page integration tests for the sample Authorization Server serving a custom Consent page + * + * @author Dmitriy Dubson + */ +@RunWith(SpringRunner.class) +@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) +@Import({OAuth2AuthorizationServerCustomConsentPageApplicationTests.InMemoryOAuth2AuthorizationServiceTestConfiguration.class}) +@AutoConfigureMockMvc +public class OAuth2AuthorizationServerCustomConsentPageApplicationTests { + @Autowired + private WebClient webClient; + + @MockBean + private OAuth2AuthorizationConsentService mockAuthorizationConsentService; + + private final String testConsentResultEndpoint = "http://127.0.0.1/login/oauth2/code/messaging-client-oidc"; + + private final String authorizeEndpoint = UriComponentsBuilder + .fromPath("/oauth2/authorize") + .queryParam("response_type", "code") + .queryParam("client_id", "messaging-client") + .queryParam("scope", "openid message.read message.write") + .queryParam("state", "state") + .queryParam("redirect_uri", testConsentResultEndpoint) + .toUriString(); + + @Before + public void setUp() { + this.webClient.getOptions().setThrowExceptionOnFailingStatusCode(false); + this.webClient.getOptions().setRedirectEnabled(true); + this.webClient.getCookieManager().clearCookies(); + when(mockAuthorizationConsentService.findById(any(), any())).thenReturn(null); + } + + @Test + @WithMockUser("user1") + public void whenUserConsentsToAllScopesThenReturnAuthorizationCode() throws IOException { + final HtmlPage consentPage = webClient.getPage(authorizeEndpoint); + assertThat(consentPage.getTitleText()).isEqualTo("Custom consent page - Consent required"); + + List scopes = consentPage.querySelectorAll("input[name='scope']").stream() + .map(scope -> (HtmlCheckBoxInput) scope).collect(Collectors.toList()); + for (HtmlCheckBoxInput scope : scopes) { + scope.click(); + } + + assertThat(scopes.stream().map(DomElement::getId).collect(Collectors.toList())) + .containsExactlyInAnyOrder("openid", "message.read", "message.write"); + assertThat(scopes.stream().allMatch(HtmlCheckBoxInput::isChecked)).isTrue(); + + DomElement submitConsentButton = consentPage.querySelector("button[id='submit-consent']"); + this.webClient.getOptions().setRedirectEnabled(false); + + WebResponse approveConsentResponse = submitConsentButton.click().getWebResponse(); + assertThat(approveConsentResponse.getStatusCode()).isEqualTo(HttpStatus.MOVED_PERMANENTLY.value()); + String location = approveConsentResponse.getResponseHeaderValue("location"); + assertThat(location).startsWith(testConsentResultEndpoint); + assertThat(location).contains("code="); + } + + @Test + @WithMockUser("user1") + public void whenUserCancelsApprovingConsentThenReturnAnAccessDeniedError() throws IOException { + final HtmlPage consentPage = webClient.getPage(authorizeEndpoint); + assertThat(consentPage.getTitleText()).isEqualTo("Custom consent page - Consent required"); + + DomElement cancelConsentButton = consentPage.querySelector("button[id='cancel-consent']"); + this.webClient.getOptions().setRedirectEnabled(false); + + WebResponse cancelConsentResponse = cancelConsentButton.click().getWebResponse(); + assertThat(cancelConsentResponse.getStatusCode()).isEqualTo(HttpStatus.MOVED_PERMANENTLY.value()); + String location = cancelConsentResponse.getResponseHeaderValue("location"); + assertThat(location).contains("error=access_denied"); + } + + @TestConfiguration + static class InMemoryOAuth2AuthorizationServiceTestConfiguration { + @Bean + public OAuth2AuthorizationService authorizationService() { + return new InMemoryOAuth2AuthorizationService(); + } + } +} diff --git a/samples/boot/oauth2-integration/authorizationserver/spring-security-samples-boot-oauth2-integrated-authorizationserver.gradle b/samples/boot/oauth2-integration/authorizationserver/spring-security-samples-boot-oauth2-integrated-authorizationserver.gradle index a404f478..2a24a81a 100644 --- a/samples/boot/oauth2-integration/authorizationserver/spring-security-samples-boot-oauth2-integrated-authorizationserver.gradle +++ b/samples/boot/oauth2-integration/authorizationserver/spring-security-samples-boot-oauth2-integrated-authorizationserver.gradle @@ -8,5 +8,6 @@ dependencies { runtimeOnly 'com.h2database:h2' testCompile 'org.springframework.boot:spring-boot-starter-test' + testCompile 'org.springframework.security:spring-security-test' testCompile 'net.sourceforge.htmlunit:htmlunit' } diff --git a/samples/boot/oauth2-integration/authorizationserver/src/test/java/sample/OAuth2AuthorizationServerConsentTests.java b/samples/boot/oauth2-integration/authorizationserver/src/test/java/sample/OAuth2AuthorizationServerConsentTests.java new file mode 100644 index 00000000..7e90d057 --- /dev/null +++ b/samples/boot/oauth2-integration/authorizationserver/src/test/java/sample/OAuth2AuthorizationServerConsentTests.java @@ -0,0 +1,119 @@ +/* + * 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 sample; + +import java.io.IOException; +import java.util.List; +import java.util.stream.Collectors; + +import com.gargoylesoftware.htmlunit.WebClient; +import com.gargoylesoftware.htmlunit.WebResponse; +import com.gargoylesoftware.htmlunit.html.DomElement; +import com.gargoylesoftware.htmlunit.html.HtmlCheckBoxInput; +import com.gargoylesoftware.htmlunit.html.HtmlPage; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.http.HttpStatus; +import org.springframework.security.oauth2.server.authorization.OAuth2AuthorizationConsentService; +import org.springframework.security.test.context.support.WithMockUser; +import org.springframework.test.context.junit4.SpringRunner; +import org.springframework.web.util.UriComponentsBuilder; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.when; + +/** + * Consent screen integration tests for the sample Authorization Server + * + * @author Dmitriy Dubson + */ +@RunWith(SpringRunner.class) +@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) +@AutoConfigureMockMvc +public class OAuth2AuthorizationServerConsentTests { + @Autowired + private WebClient webClient; + + @MockBean + private OAuth2AuthorizationConsentService mockAuthorizationConsentService; + + private final String testConsentResultEndpoint = "http://127.0.0.1/login/oauth2/code/messaging-client-oidc"; + + private final String authorizeEndpoint = UriComponentsBuilder + .fromPath("/oauth2/authorize") + .queryParam("response_type", "code") + .queryParam("client_id", "messaging-client") + .queryParam("scope", "openid message.read message.write") + .queryParam("state", "state") + .queryParam("redirect_uri", testConsentResultEndpoint) + .toUriString(); + + @Before + public void setUp() { + this.webClient.getOptions().setThrowExceptionOnFailingStatusCode(false); + this.webClient.getOptions().setRedirectEnabled(true); + this.webClient.getCookieManager().clearCookies(); + when(mockAuthorizationConsentService.findById(any(), any())).thenReturn(null); + } + + @Test + @WithMockUser("user1") + public void whenUserConsentsToAllScopesThenReturnAuthorizationCode() throws IOException { + final HtmlPage consentPage = webClient.getPage(authorizeEndpoint); + assertThat(consentPage.getTitleText()).isEqualTo("Consent required"); + + List scopes = consentPage.querySelectorAll("input[name='scope']").stream() + .map(scope -> (HtmlCheckBoxInput) scope).collect(Collectors.toList()); + for (HtmlCheckBoxInput scope : scopes) { + scope.click(); + } + + assertThat(scopes.stream().map(DomElement::getId).collect(Collectors.toList())) + .containsExactlyInAnyOrder("message.read", "message.write"); + assertThat(scopes.stream().allMatch(HtmlCheckBoxInput::isChecked)).isTrue(); + + DomElement submitConsentButton = consentPage.querySelector("button[id='submit-consent']"); + this.webClient.getOptions().setRedirectEnabled(false); + + WebResponse approveConsentResponse = submitConsentButton.click().getWebResponse(); + assertThat(approveConsentResponse.getStatusCode()).isEqualTo(HttpStatus.MOVED_PERMANENTLY.value()); + String location = approveConsentResponse.getResponseHeaderValue("location"); + assertThat(location).startsWith(testConsentResultEndpoint); + assertThat(location).contains("code="); + } + + @Test + @WithMockUser("user1") + public void whenUserCancelsApprovingConsentThenReturnAnAccessDeniedError() throws IOException { + final HtmlPage consentPage = webClient.getPage(authorizeEndpoint); + assertThat(consentPage.getTitleText()).isEqualTo("Consent required"); + + DomElement cancelConsentButton = consentPage.querySelector("button[id='cancel-consent']"); + this.webClient.getOptions().setRedirectEnabled(false); + + WebResponse cancelConsentResponse = cancelConsentButton.click().getWebResponse(); + assertThat(cancelConsentResponse.getStatusCode()).isEqualTo(HttpStatus.MOVED_PERMANENTLY.value()); + String location = cancelConsentResponse.getResponseHeaderValue("location"); + assertThat(location).contains("error=access_denied"); + } + +}