Skip to content

Commit ce8721f

Browse files
author
Dmitriy Dubson
committed
Fix cancel consent functionality on default consent page
- Fix also applies to custom consent sample Closes gh-393
1 parent 8e8979a commit ce8721f

File tree

9 files changed

+284
-13
lines changed

9 files changed

+284
-13
lines changed

oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilter.java

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@
6565
* @author Paurav Munshi
6666
* @author Daniel Garnier-Moiroux
6767
* @author Anoop Garlapati
68+
* @author Dmitriy Dubson
6869
* @since 0.0.1
6970
* @see AuthenticationManager
7071
* @see OAuth2AuthorizationCodeRequestAuthenticationProvider
@@ -332,6 +333,12 @@ private static String generateConsentPage(HttpServletRequest request,
332333
builder.append(" <meta name=\"viewport\" content=\"width=device-width, initial-scale=1, shrink-to-fit=no\">");
333334
builder.append(" <link rel=\"stylesheet\" href=\"https://stackpath.bootstrapcdn.com/bootstrap/4.5.2/css/bootstrap.min.css\" integrity=\"sha384-JcKb8q3iqJ61gNV9KGb8thSsNjpSL0n8PARn9HuZOnIxN0hoP+VmmDGMN5t9UJ0Z\" crossorigin=\"anonymous\">");
334335
builder.append(" <title>Consent required</title>");
336+
builder.append(" <script>");
337+
builder.append(" function cancelConsent() {");
338+
builder.append(" document.consent_form.reset();");
339+
builder.append(" document.consent_form.submit();");
340+
builder.append(" }");
341+
builder.append(" </script>");
335342
builder.append("</head>");
336343
builder.append("<body>");
337344
builder.append("<div class=\"container\">");
@@ -350,13 +357,13 @@ private static String generateConsentPage(HttpServletRequest request,
350357
builder.append(" </div>");
351358
builder.append(" <div class=\"row\">");
352359
builder.append(" <div class=\"col text-center\">");
353-
builder.append(" <form method=\"post\" action=\"" + request.getRequestURI() + "\">");
360+
builder.append(" <form name=\"consent_form\" method=\"post\" action=\"" + request.getRequestURI() + "\">");
354361
builder.append(" <input type=\"hidden\" name=\"client_id\" value=\"" + clientId + "\">");
355362
builder.append(" <input type=\"hidden\" name=\"state\" value=\"" + state + "\">");
356363

357364
for (String scope : scopesToAuthorize) {
358365
builder.append(" <div class=\"form-group form-check py-1\">");
359-
builder.append(" <input class=\"form-check-input\" type=\"checkbox\" name=\"scope\" value=\"" + scope + "\" id=\"" + scope + "\">");
366+
builder.append(" <input class=\"form-check-input scope-to-accept\" type=\"checkbox\" name=\"scope\" value=\"" + scope + "\" id=\"" + scope + "\">");
360367
builder.append(" <label class=\"form-check-label\" for=\"" + scope + "\">" + scope + "</label>");
361368
builder.append(" </div>");
362369
}
@@ -372,10 +379,10 @@ private static String generateConsentPage(HttpServletRequest request,
372379
}
373380

374381
builder.append(" <div class=\"form-group pt-3\">");
375-
builder.append(" <button class=\"btn btn-primary btn-lg\" type=\"submit\">Submit Consent</button>");
382+
builder.append(" <button class=\"btn btn-primary btn-lg\" type=\"submit\" id=\"submit-consent\">Submit Consent</button>");
376383
builder.append(" </div>");
377384
builder.append(" <div class=\"form-group\">");
378-
builder.append(" <button class=\"btn btn-link regular\" type=\"reset\">Cancel</button>");
385+
builder.append(" <button class=\"btn btn-link regular\" type=\"button\" onclick=\"cancelConsent();\" id=\"cancel-consent\">Cancel</button>");
379386
builder.append(" </div>");
380387
builder.append(" </form>");
381388
builder.append(" </div>");

oauth2-authorization-server/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/authorization/OAuth2AuthorizationCodeGrantTests.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,7 @@
122122
*
123123
* @author Joe Grandja
124124
* @author Daniel Garnier-Moiroux
125+
* @author Dmitriy Dubson
125126
*/
126127
public class OAuth2AuthorizationCodeGrantTests {
127128
private static final String DEFAULT_AUTHORIZATION_ENDPOINT_URI = "/oauth2/authorize";
@@ -556,7 +557,7 @@ private static String getAuthorizationHeader(RegisteredClient registeredClient)
556557

557558
private static String scopeCheckbox(String scope) {
558559
return MessageFormat.format(
559-
"<input class=\"form-check-input\" type=\"checkbox\" name=\"scope\" value=\"{0}\" id=\"{0}\">",
560+
"<input class=\"form-check-input scope-to-accept\" type=\"checkbox\" name=\"scope\" value=\"{0}\" id=\"{0}\">",
560561
scope
561562
);
562563
}

oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilterTests.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@
7373
* @author Joe Grandja
7474
* @author Daniel Garnier-Moiroux
7575
* @author Anoop Garlapati
76+
* @author Dmitriy Dubson
7677
* @since 0.0.1
7778
*/
7879
public class OAuth2AuthorizationEndpointFilterTests {
@@ -574,7 +575,7 @@ private static OAuth2AuthorizationCodeRequestAuthenticationToken.Builder authori
574575

575576
private static String scopeCheckbox(String scope) {
576577
return MessageFormat.format(
577-
"<input class=\"form-check-input\" type=\"checkbox\" name=\"scope\" value=\"{0}\" id=\"{0}\">",
578+
"<input class=\"form-check-input scope-to-accept\" type=\"checkbox\" name=\"scope\" value=\"{0}\" id=\"{0}\">",
578579
scope
579580
);
580581
}

samples/boot/oauth2-integration/authorizationserver-custom-consent-page/spring-security-samples-boot-oauth2-integrated-authorizationserver-custom-consent-page.gradle

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,4 +5,8 @@ dependencies {
55
compile 'org.springframework.boot:spring-boot-starter-thymeleaf'
66
compile 'org.springframework.boot:spring-boot-starter-security'
77
compile project(':spring-security-oauth2-authorization-server')
8+
9+
testCompile 'org.springframework.boot:spring-boot-starter-test'
10+
testCompile 'org.springframework.security:spring-security-test'
11+
testCompile 'net.sourceforge.htmlunit:htmlunit'
812
}

samples/boot/oauth2-integration/authorizationserver-custom-consent-page/src/main/java/sample/config/AuthorizationServerConfig.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,6 @@
2121
import com.nimbusds.jose.jwk.RSAKey;
2222
import com.nimbusds.jose.jwk.source.JWKSource;
2323
import com.nimbusds.jose.proc.SecurityContext;
24-
import sample.jose.Jwks;
25-
2624
import org.springframework.context.annotation.Bean;
2725
import org.springframework.context.annotation.Configuration;
2826
import org.springframework.core.Ordered;
@@ -42,13 +40,15 @@
4240
import org.springframework.security.oauth2.server.authorization.config.ProviderSettings;
4341
import org.springframework.security.web.SecurityFilterChain;
4442
import org.springframework.security.web.util.matcher.RequestMatcher;
43+
import sample.jose.Jwks;
4544

4645
/**
4746
* @author Joe Grandja
4847
* @author Daniel Garnier-Moiroux
4948
*/
5049
@Configuration(proxyBeanMethods = false)
5150
public class AuthorizationServerConfig {
51+
private static final String CUSTOM_CONSENT_PAGE_URI = "/oauth2/consent";
5252

5353
@Bean
5454
@Order(Ordered.HIGHEST_PRECEDENCE)
@@ -57,7 +57,7 @@ public SecurityFilterChain authorizationServerSecurityFilterChain(HttpSecurity h
5757
new OAuth2AuthorizationServerConfigurer<>();
5858
authorizationServerConfigurer
5959
.authorizationEndpoint(authorizationEndpoint ->
60-
authorizationEndpoint.consentPage("/oauth2/consent"));
60+
authorizationEndpoint.consentPage(CUSTOM_CONSENT_PAGE_URI));
6161

6262
RequestMatcher endpointsMatcher = authorizationServerConfigurer
6363
.getEndpointsMatcher();

samples/boot/oauth2-integration/authorizationserver-custom-consent-page/src/main/resources/templates/consent.html

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,18 @@
55
<meta name="viewport" content="width=device-width, initial-scale=1, shrink-to-fit=no">
66
<link rel="stylesheet" href="https://stackpath.bootstrapcdn.com/bootstrap/4.5.2/css/bootstrap.min.css"
77
integrity="sha384-JcKb8q3iqJ61gNV9KGb8thSsNjpSL0n8PARn9HuZOnIxN0hoP+VmmDGMN5t9UJ0Z" crossorigin="anonymous">
8-
<title>Consent required</title>
8+
<title>Custom consent page - Consent required</title>
99
<style>
1010
body {
1111
background-color: aliceblue;
1212
}
1313
</style>
14+
<script>
15+
function cancelConsent() {
16+
document.consent_form.reset();
17+
document.consent_form.submit();
18+
}
19+
</script>
1420
</head>
1521
<body>
1622
<div class="container">
@@ -33,7 +39,7 @@ <h1 class="text-center text-primary">App permissions</h1>
3339
</div>
3440
<div class="row">
3541
<div class="col text-center">
36-
<form method="post" action="/oauth2/authorize">
42+
<form name="consent_form" method="post" action="/oauth2/authorize">
3743
<input type="hidden" name="client_id" th:value="${clientId}">
3844
<input type="hidden" name="state" th:value="${state}">
3945

@@ -59,12 +65,12 @@ <h1 class="text-center text-primary">App permissions</h1>
5965
</div>
6066

6167
<div class="form-group pt-3">
62-
<button class="btn btn-primary btn-lg" type="submit">
68+
<button class="btn btn-primary btn-lg" type="submit" id="submit-consent">
6369
Submit Consent
6470
</button>
6571
</div>
6672
<div class="form-group">
67-
<button class="btn btn-link regular" type="reset">
73+
<button class="btn btn-link regular" type="button" id="cancel-consent" onclick="cancelConsent();">
6874
Cancel
6975
</button>
7076
</div>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
/*
2+
* Copyright 2020-2021 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package sample;
17+
18+
import java.io.IOException;
19+
import java.util.List;
20+
import java.util.stream.Collectors;
21+
22+
import com.gargoylesoftware.htmlunit.WebClient;
23+
import com.gargoylesoftware.htmlunit.WebResponse;
24+
import com.gargoylesoftware.htmlunit.html.DomElement;
25+
import com.gargoylesoftware.htmlunit.html.HtmlCheckBoxInput;
26+
import com.gargoylesoftware.htmlunit.html.HtmlPage;
27+
import org.junit.Before;
28+
import org.junit.Test;
29+
import org.junit.runner.RunWith;
30+
import org.springframework.beans.factory.annotation.Autowired;
31+
import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc;
32+
import org.springframework.boot.test.context.SpringBootTest;
33+
import org.springframework.boot.test.context.TestConfiguration;
34+
import org.springframework.boot.test.mock.mockito.MockBean;
35+
import org.springframework.context.annotation.Bean;
36+
import org.springframework.context.annotation.Import;
37+
import org.springframework.http.HttpStatus;
38+
import org.springframework.security.oauth2.server.authorization.InMemoryOAuth2AuthorizationService;
39+
import org.springframework.security.oauth2.server.authorization.OAuth2AuthorizationConsentService;
40+
import org.springframework.security.oauth2.server.authorization.OAuth2AuthorizationService;
41+
import org.springframework.security.test.context.support.WithMockUser;
42+
import org.springframework.test.context.junit4.SpringRunner;
43+
import org.springframework.web.util.UriComponentsBuilder;
44+
45+
import static org.assertj.core.api.Assertions.assertThat;
46+
import static org.mockito.ArgumentMatchers.any;
47+
import static org.mockito.Mockito.when;
48+
49+
/**
50+
* Consent page integration tests for the sample Authorization Server serving a custom Consent page
51+
*
52+
* @author Dmitriy Dubson
53+
*/
54+
@RunWith(SpringRunner.class)
55+
@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
56+
@Import({OAuth2AuthorizationServerCustomConsentPageApplicationTests.InMemoryOAuth2AuthorizationServiceTestConfiguration.class})
57+
@AutoConfigureMockMvc
58+
public class OAuth2AuthorizationServerCustomConsentPageApplicationTests {
59+
@Autowired
60+
private WebClient webClient;
61+
62+
@MockBean
63+
private OAuth2AuthorizationConsentService mockAuthorizationConsentService;
64+
65+
private final String testConsentResultEndpoint = "http://127.0.0.1/login/oauth2/code/messaging-client-oidc";
66+
67+
private final String authorizeEndpoint = UriComponentsBuilder
68+
.fromPath("/oauth2/authorize")
69+
.queryParam("response_type", "code")
70+
.queryParam("client_id", "messaging-client")
71+
.queryParam("scope", "openid message.read message.write")
72+
.queryParam("state", "state")
73+
.queryParam("redirect_uri", testConsentResultEndpoint)
74+
.toUriString();
75+
76+
@Before
77+
public void setUp() {
78+
this.webClient.getOptions().setThrowExceptionOnFailingStatusCode(false);
79+
this.webClient.getOptions().setRedirectEnabled(true);
80+
this.webClient.getCookieManager().clearCookies();
81+
when(mockAuthorizationConsentService.findById(any(), any())).thenReturn(null);
82+
}
83+
84+
@Test
85+
@WithMockUser("user1")
86+
public void whenUserConsentsToAllScopesThenReturnAuthorizationCode() throws IOException {
87+
final HtmlPage consentPage = webClient.getPage(authorizeEndpoint);
88+
assertThat(consentPage.getTitleText()).isEqualTo("Custom consent page - Consent required");
89+
90+
List<HtmlCheckBoxInput> scopes = consentPage.querySelectorAll("input[name='scope']").stream()
91+
.map(scope -> (HtmlCheckBoxInput) scope).collect(Collectors.toList());
92+
for (HtmlCheckBoxInput scope : scopes) {
93+
scope.click();
94+
}
95+
96+
assertThat(scopes.stream().map(DomElement::getId).collect(Collectors.toList()))
97+
.containsExactlyInAnyOrder("openid", "message.read", "message.write");
98+
assertThat(scopes.stream().allMatch(HtmlCheckBoxInput::isChecked)).isTrue();
99+
100+
DomElement submitConsentButton = consentPage.querySelector("button[id='submit-consent']");
101+
this.webClient.getOptions().setRedirectEnabled(false);
102+
103+
WebResponse approveConsentResponse = submitConsentButton.click().getWebResponse();
104+
assertThat(approveConsentResponse.getStatusCode()).isEqualTo(HttpStatus.MOVED_PERMANENTLY.value());
105+
String location = approveConsentResponse.getResponseHeaderValue("location");
106+
assertThat(location).startsWith(testConsentResultEndpoint);
107+
assertThat(location).contains("code=");
108+
}
109+
110+
@Test
111+
@WithMockUser("user1")
112+
public void whenUserCancelsApprovingConsentThenReturnAnAccessDeniedError() throws IOException {
113+
final HtmlPage consentPage = webClient.getPage(authorizeEndpoint);
114+
assertThat(consentPage.getTitleText()).isEqualTo("Custom consent page - Consent required");
115+
116+
DomElement cancelConsentButton = consentPage.querySelector("button[id='cancel-consent']");
117+
this.webClient.getOptions().setRedirectEnabled(false);
118+
119+
WebResponse cancelConsentResponse = cancelConsentButton.click().getWebResponse();
120+
assertThat(cancelConsentResponse.getStatusCode()).isEqualTo(HttpStatus.MOVED_PERMANENTLY.value());
121+
String location = cancelConsentResponse.getResponseHeaderValue("location");
122+
assertThat(location).contains("error=access_denied");
123+
}
124+
125+
@TestConfiguration
126+
static class InMemoryOAuth2AuthorizationServiceTestConfiguration {
127+
@Bean
128+
public OAuth2AuthorizationService authorizationService() {
129+
return new InMemoryOAuth2AuthorizationService();
130+
}
131+
}
132+
}

samples/boot/oauth2-integration/authorizationserver/spring-security-samples-boot-oauth2-integrated-authorizationserver.gradle

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,5 +8,6 @@ dependencies {
88
runtimeOnly 'com.h2database:h2'
99

1010
testCompile 'org.springframework.boot:spring-boot-starter-test'
11+
testCompile 'org.springframework.security:spring-security-test'
1112
testCompile 'net.sourceforge.htmlunit:htmlunit'
1213
}

0 commit comments

Comments
 (0)