Skip to content

Commit 0cd5940

Browse files
jgrandjajgrandja
committed
Polish authorization error response encoding
Issue gh-1011
1 parent 30927ad commit 0cd5940

File tree

3 files changed

+50
-13
lines changed

3 files changed

+50
-13
lines changed

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

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -298,13 +298,12 @@ private void sendAuthorizationResponse(HttpServletRequest request, HttpServletRe
298298
UriComponentsBuilder uriBuilder = UriComponentsBuilder
299299
.fromUriString(authorizationCodeRequestAuthentication.getRedirectUri())
300300
.queryParam(OAuth2ParameterNames.CODE, authorizationCodeRequestAuthentication.getAuthorizationCode().getTokenValue());
301-
String redirectUri;
302301
if (StringUtils.hasText(authorizationCodeRequestAuthentication.getState())) {
303302
uriBuilder.queryParam(
304303
OAuth2ParameterNames.STATE,
305304
UriUtils.encode(authorizationCodeRequestAuthentication.getState(), StandardCharsets.UTF_8));
306305
}
307-
redirectUri = uriBuilder.build(true).toUriString(); // build(true) -> Components are explicitly encoded
306+
String redirectUri = uriBuilder.build(true).toUriString(); // build(true) -> Components are explicitly encoded
308307
this.redirectStrategy.sendRedirect(request, response, redirectUri);
309308
}
310309

@@ -331,18 +330,21 @@ private void sendErrorResponse(HttpServletRequest request, HttpServletResponse r
331330
.fromUriString(authorizationCodeRequestAuthentication.getRedirectUri())
332331
.queryParam(OAuth2ParameterNames.ERROR, error.getErrorCode());
333332
if (StringUtils.hasText(error.getDescription())) {
334-
uriBuilder.queryParam(OAuth2ParameterNames.ERROR_DESCRIPTION, error.getDescription());
333+
uriBuilder.queryParam(
334+
OAuth2ParameterNames.ERROR_DESCRIPTION,
335+
UriUtils.encode(error.getDescription(), StandardCharsets.UTF_8));
335336
}
336337
if (StringUtils.hasText(error.getUri())) {
337-
uriBuilder.queryParam(OAuth2ParameterNames.ERROR_URI, error.getUri());
338+
uriBuilder.queryParam(
339+
OAuth2ParameterNames.ERROR_URI,
340+
UriUtils.encode(error.getUri(), StandardCharsets.UTF_8));
338341
}
339-
String redirectUri;
340342
if (StringUtils.hasText(authorizationCodeRequestAuthentication.getState())) {
341343
uriBuilder.queryParam(
342344
OAuth2ParameterNames.STATE,
343345
UriUtils.encode(authorizationCodeRequestAuthentication.getState(), StandardCharsets.UTF_8));
344346
}
345-
redirectUri = uriBuilder.build(true).toUriString(); // build(true) -> Components are explicitly encoded
347+
String redirectUri = uriBuilder.build(true).toUriString(); // build(true) -> Components are explicitly encoded
346348
this.redirectStrategy.sendRedirect(request, response, redirectUri);
347349
}
348350

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,7 @@
127127
import org.springframework.util.StringUtils;
128128
import org.springframework.web.util.UriComponents;
129129
import org.springframework.web.util.UriComponentsBuilder;
130+
import org.springframework.web.util.UriUtils;
130131

131132
import static org.assertj.core.api.Assertions.assertThat;
132133
import static org.hamcrest.CoreMatchers.containsString;
@@ -455,6 +456,36 @@ public void requestWhenConfidentialClientWithPkceAndMissingCodeVerifierThenBadRe
455456
.andExpect(status().isBadRequest());
456457
}
457458

459+
// gh-1011
460+
@Test
461+
public void requestWhenConfidentialClientWithPkceAndMissingCodeChallengeThenErrorResponseEncoded() throws Exception {
462+
this.spring.register(AuthorizationServerConfiguration.class).autowire();
463+
464+
String redirectUri = "https://example.com/callback-1?param=encoded%20parameter%20value";
465+
RegisteredClient registeredClient = TestRegisteredClients.registeredClient()
466+
.redirectUris(redirectUris -> {
467+
redirectUris.clear();
468+
redirectUris.add(redirectUri);
469+
})
470+
.clientSettings(ClientSettings.builder().requireProofKey(true).build())
471+
.build();
472+
this.registeredClientRepository.save(registeredClient);
473+
474+
MultiValueMap<String, String> authorizationRequestParameters = getAuthorizationRequestParameters(registeredClient);
475+
MvcResult mvcResult = this.mvc.perform(get(DEFAULT_AUTHORIZATION_ENDPOINT_URI)
476+
.params(authorizationRequestParameters)
477+
.with(user("user")))
478+
.andExpect(status().is3xxRedirection())
479+
.andReturn();
480+
String redirectedUrl = mvcResult.getResponse().getRedirectedUrl();
481+
String expectedRedirectUri = redirectUri + "&" +
482+
"error=invalid_request&" +
483+
"error_description=" + UriUtils.encode("OAuth 2.0 Parameter: code_challenge", StandardCharsets.UTF_8) + "&" +
484+
"error_uri=" + UriUtils.encode("https://datatracker.ietf.org/doc/html/rfc7636#section-4.4.1", StandardCharsets.UTF_8) + "&" +
485+
"state=" + STATE_URL_ENCODED;
486+
assertThat(redirectedUrl).isEqualTo(expectedRedirectUri);
487+
}
488+
458489
@Test
459490
public void requestWhenCustomTokenGeneratorThenUsed() throws Exception {
460491
this.spring.register(AuthorizationServerConfigurationWithTokenGenerator.class).autowire();

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

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -280,12 +280,17 @@ public void doFilterWhenAuthorizationRequestMultipleCodeChallengeMethodThenInval
280280

281281
@Test
282282
public void doFilterWhenAuthorizationRequestAuthenticationExceptionThenErrorResponse() throws Exception {
283-
RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build();
283+
RegisteredClient registeredClient = TestRegisteredClients.registeredClient()
284+
.redirectUris(redirectUris -> {
285+
redirectUris.clear();
286+
redirectUris.add("https://example.com?param=encoded%20parameter%20value");
287+
})
288+
.build();
284289
OAuth2AuthorizationCodeRequestAuthenticationToken authorizationCodeRequestAuthentication =
285290
new OAuth2AuthorizationCodeRequestAuthenticationToken(
286291
AUTHORIZATION_URI, registeredClient.getClientId(), principal,
287-
registeredClient.getRedirectUris().iterator().next(), STATE, registeredClient.getScopes(), null);
288-
OAuth2Error error = new OAuth2Error("errorCode", "errorDescription", "errorUri");
292+
registeredClient.getRedirectUris().iterator().next(), "client state", registeredClient.getScopes(), null);
293+
OAuth2Error error = new OAuth2Error(OAuth2ErrorCodes.INVALID_REQUEST, "error description", "error uri");
289294
when(this.authenticationManager.authenticate(any()))
290295
.thenThrow(new OAuth2AuthorizationCodeRequestAuthenticationException(error, authorizationCodeRequestAuthentication));
291296

@@ -300,8 +305,7 @@ public void doFilterWhenAuthorizationRequestAuthenticationExceptionThenErrorResp
300305

301306
assertThat(response.getStatus()).isEqualTo(HttpStatus.FOUND.value());
302307
assertThat(response.getRedirectedUrl()).isEqualTo(
303-
request.getParameter(OAuth2ParameterNames.REDIRECT_URI) +
304-
"?error=errorCode&error_description=errorDescription&error_uri=errorUri&state=state");
308+
"https://example.com?param=encoded%20parameter%20value&error=invalid_request&error_description=error%20description&error_uri=error%20uri&state=client%20state");
305309
assertThat(SecurityContextHolder.getContext().getAuthentication()).isSameAs(this.principal);
306310
}
307311

@@ -546,7 +550,7 @@ public void doFilterWhenAuthorizationRequestAuthenticatedThenAuthorizationRespon
546550
OAuth2AuthorizationCodeRequestAuthenticationToken authorizationCodeRequestAuthenticationResult =
547551
new OAuth2AuthorizationCodeRequestAuthenticationToken(
548552
AUTHORIZATION_URI, registeredClient.getClientId(), principal, this.authorizationCode,
549-
registeredClient.getRedirectUris().iterator().next(), STATE, registeredClient.getScopes());
553+
registeredClient.getRedirectUris().iterator().next(), "client state", registeredClient.getScopes());
550554
authorizationCodeRequestAuthenticationResult.setAuthenticated(true);
551555
when(this.authenticationManager.authenticate(any()))
552556
.thenReturn(authorizationCodeRequestAuthenticationResult);
@@ -568,7 +572,7 @@ public void doFilterWhenAuthorizationRequestAuthenticatedThenAuthorizationRespon
568572
.isEqualTo(REMOTE_ADDRESS);
569573
assertThat(response.getStatus()).isEqualTo(HttpStatus.FOUND.value());
570574
assertThat(response.getRedirectedUrl()).isEqualTo(
571-
"https://example.com?param=encoded%20parameter%20value&code=code&state=state");
575+
"https://example.com?param=encoded%20parameter%20value&code=code&state=client%20state");
572576
}
573577

574578
@Test

0 commit comments

Comments
 (0)