Skip to content

Commit 1cca9c5

Browse files
jgrandjajgrandja
committed
Enable PKCE by default in authorization server
Closes gh-18020
1 parent 469ed09 commit 1cca9c5

File tree

8 files changed

+127
-92
lines changed

8 files changed

+127
-92
lines changed

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

Lines changed: 37 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import java.time.temporal.ChronoUnit;
2727
import java.util.Arrays;
2828
import java.util.Base64;
29+
import java.util.HashMap;
2930
import java.util.HashSet;
3031
import java.util.List;
3132
import java.util.Map;
@@ -359,7 +360,7 @@ public void requestWhenTokenRequestValidThenReturnAccessTokenResponse() throws E
359360
RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build();
360361
this.registeredClientRepository.save(registeredClient);
361362

362-
OAuth2Authorization authorization = TestOAuth2Authorizations.authorization(registeredClient).build();
363+
OAuth2Authorization authorization = createAuthorization(registeredClient);
363364
this.authorizationService.save(authorization);
364365

365366
OAuth2AccessTokenResponse accessTokenResponse = assertTokenRequestReturnsAccessTokenResponse(registeredClient,
@@ -384,7 +385,7 @@ public void requestWhenTokenRequestCustomEndpointThenReturnAccessTokenResponse()
384385
RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build();
385386
this.registeredClientRepository.save(registeredClient);
386387

387-
OAuth2Authorization authorization = TestOAuth2Authorizations.authorization(registeredClient).build();
388+
OAuth2Authorization authorization = createAuthorization(registeredClient);
388389
this.authorizationService.save(authorization);
389390

390391
assertTokenRequestReturnsAccessTokenResponse(registeredClient, authorization,
@@ -433,8 +434,6 @@ public void requestWhenPublicClientWithPkceThenReturnAccessTokenResponse() throw
433434
MvcResult mvcResult = this.mvc
434435
.perform(get(DEFAULT_AUTHORIZATION_ENDPOINT_URI)
435436
.queryParams(getAuthorizationRequestParameters(registeredClient))
436-
.queryParam(PkceParameterNames.CODE_CHALLENGE, S256_CODE_CHALLENGE)
437-
.queryParam(PkceParameterNames.CODE_CHALLENGE_METHOD, "S256")
438437
.with(user("user")))
439438
.andExpect(status().is3xxRedirection())
440439
.andReturn();
@@ -451,8 +450,7 @@ public void requestWhenPublicClientWithPkceThenReturnAccessTokenResponse() throw
451450
this.mvc
452451
.perform(post(DEFAULT_TOKEN_ENDPOINT_URI)
453452
.params(getTokenRequestParameters(registeredClient, authorizationCodeAuthorization))
454-
.param(OAuth2ParameterNames.CLIENT_ID, registeredClient.getClientId())
455-
.param(PkceParameterNames.CODE_VERIFIER, S256_CODE_VERIFIER))
453+
.param(OAuth2ParameterNames.CLIENT_ID, registeredClient.getClientId()))
456454
.andExpect(header().string(HttpHeaders.CACHE_CONTROL, containsString("no-store")))
457455
.andExpect(header().string(HttpHeaders.PRAGMA, containsString("no-cache")))
458456
.andExpect(status().isOk())
@@ -487,8 +485,6 @@ public void requestWhenPublicClientWithPkceAndCustomRefreshTokenGeneratorThenRet
487485
MvcResult mvcResult = this.mvc
488486
.perform(get(DEFAULT_AUTHORIZATION_ENDPOINT_URI)
489487
.queryParams(getAuthorizationRequestParameters(registeredClient))
490-
.queryParam(PkceParameterNames.CODE_CHALLENGE, S256_CODE_CHALLENGE)
491-
.queryParam(PkceParameterNames.CODE_CHALLENGE_METHOD, "S256")
492488
.with(user("user")))
493489
.andExpect(status().is3xxRedirection())
494490
.andReturn();
@@ -505,8 +501,7 @@ public void requestWhenPublicClientWithPkceAndCustomRefreshTokenGeneratorThenRet
505501
this.mvc
506502
.perform(post(DEFAULT_TOKEN_ENDPOINT_URI)
507503
.params(getTokenRequestParameters(registeredClient, authorizationCodeAuthorization))
508-
.param(OAuth2ParameterNames.CLIENT_ID, registeredClient.getClientId())
509-
.param(PkceParameterNames.CODE_VERIFIER, S256_CODE_VERIFIER))
504+
.param(OAuth2ParameterNames.CLIENT_ID, registeredClient.getClientId()))
510505
.andExpect(header().string(HttpHeaders.CACHE_CONTROL, containsString("no-store")))
511506
.andExpect(header().string(HttpHeaders.PRAGMA, containsString("no-cache")))
512507
.andExpect(status().isOk())
@@ -542,11 +537,11 @@ public void requestWhenPublicClientWithPkceAndEmptyCodeThenBadRequest() throws E
542537
tokenRequestParameters.set(OAuth2ParameterNames.CODE, "");
543538
tokenRequestParameters.set(OAuth2ParameterNames.REDIRECT_URI,
544539
registeredClient.getRedirectUris().iterator().next());
540+
tokenRequestParameters.set(PkceParameterNames.CODE_VERIFIER, S256_CODE_VERIFIER);
545541

546542
this.mvc
547543
.perform(post(DEFAULT_TOKEN_ENDPOINT_URI).params(tokenRequestParameters)
548-
.param(OAuth2ParameterNames.CLIENT_ID, registeredClient.getClientId())
549-
.param(PkceParameterNames.CODE_VERIFIER, S256_CODE_VERIFIER))
544+
.param(OAuth2ParameterNames.CLIENT_ID, registeredClient.getClientId()))
550545
.andExpect(status().isBadRequest());
551546
}
552547

@@ -561,8 +556,6 @@ public void requestWhenConfidentialClientWithPkceAndMissingCodeVerifierThenBadRe
561556
registeredClient);
562557
MvcResult mvcResult = this.mvc
563558
.perform(get(DEFAULT_AUTHORIZATION_ENDPOINT_URI).queryParams(authorizationRequestParameters)
564-
.queryParam(PkceParameterNames.CODE_CHALLENGE, S256_CODE_CHALLENGE)
565-
.queryParam(PkceParameterNames.CODE_CHALLENGE_METHOD, "S256")
566559
.with(user("user")))
567560
.andExpect(status().is3xxRedirection())
568561
.andReturn();
@@ -577,9 +570,12 @@ public void requestWhenConfidentialClientWithPkceAndMissingCodeVerifierThenBadRe
577570
assertThat(authorizationCodeAuthorization.getAuthorizationGrantType())
578571
.isEqualTo(AuthorizationGrantType.AUTHORIZATION_CODE);
579572

573+
MultiValueMap<String, String> tokenRequestParameters = getTokenRequestParameters(registeredClient,
574+
authorizationCodeAuthorization);
575+
tokenRequestParameters.remove(PkceParameterNames.CODE_VERIFIER);
576+
580577
this.mvc
581-
.perform(post(DEFAULT_TOKEN_ENDPOINT_URI)
582-
.params(getTokenRequestParameters(registeredClient, authorizationCodeAuthorization))
578+
.perform(post(DEFAULT_TOKEN_ENDPOINT_URI).params(tokenRequestParameters)
583579
.param(OAuth2ParameterNames.CLIENT_ID, registeredClient.getClientId())
584580
.header(HttpHeaders.AUTHORIZATION, getAuthorizationHeader(registeredClient)))
585581
.andExpect(status().isBadRequest());
@@ -595,11 +591,12 @@ public void requestWhenConfidentialClientWithPkceAndMissingCodeChallengeThenErro
595591
RegisteredClient registeredClient = TestRegisteredClients.registeredClient().redirectUris((redirectUris) -> {
596592
redirectUris.clear();
597593
redirectUris.add(redirectUri);
598-
}).clientSettings(ClientSettings.builder().requireProofKey(true).build()).build();
594+
}).build();
599595
this.registeredClientRepository.save(registeredClient);
600596

601597
MultiValueMap<String, String> authorizationRequestParameters = getAuthorizationRequestParameters(
602598
registeredClient);
599+
authorizationRequestParameters.remove(PkceParameterNames.CODE_CHALLENGE);
603600
MvcResult mvcResult = this.mvc
604601
.perform(get(DEFAULT_AUTHORIZATION_ENDPOINT_URI).queryParams(authorizationRequestParameters)
605602
.with(user("user")))
@@ -618,11 +615,14 @@ public void requestWhenConfidentialClientWithPkceAndMissingCodeChallengeButCodeV
618615
throws Exception {
619616
this.spring.register(AuthorizationServerConfiguration.class).autowire();
620617

621-
RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build();
618+
RegisteredClient registeredClient = TestRegisteredClients.registeredClient()
619+
.clientSettings(ClientSettings.builder().requireProofKey(false).build())
620+
.build();
622621
this.registeredClientRepository.save(registeredClient);
623622

624623
MultiValueMap<String, String> authorizationRequestParameters = getAuthorizationRequestParameters(
625624
registeredClient);
625+
authorizationRequestParameters.remove(PkceParameterNames.CODE_CHALLENGE);
626626
MvcResult mvcResult = this.mvc
627627
.perform(get(DEFAULT_AUTHORIZATION_ENDPOINT_URI).queryParams(authorizationRequestParameters)
628628
.with(user("user")))
@@ -642,7 +642,6 @@ public void requestWhenConfidentialClientWithPkceAndMissingCodeChallengeButCodeV
642642
this.mvc
643643
.perform(post(DEFAULT_TOKEN_ENDPOINT_URI)
644644
.params(getTokenRequestParameters(registeredClient, authorizationCodeAuthorization))
645-
.param(PkceParameterNames.CODE_VERIFIER, S256_CODE_VERIFIER)
646645
.header(HttpHeaders.AUTHORIZATION, getAuthorizationHeader(registeredClient)))
647646
.andExpect(status().isBadRequest());
648647
}
@@ -654,7 +653,7 @@ public void requestWhenCustomTokenGeneratorThenUsed() throws Exception {
654653
RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build();
655654
this.registeredClientRepository.save(registeredClient);
656655

657-
OAuth2Authorization authorization = TestOAuth2Authorizations.authorization(registeredClient).build();
656+
OAuth2Authorization authorization = createAuthorization(registeredClient);
658657
this.authorizationService.save(authorization);
659658

660659
this.mvc
@@ -704,10 +703,14 @@ public void requestWhenConsentRequestThenReturnAccessTokenResponse() throws Exce
704703
OAuth2Authorization authorization = TestOAuth2Authorizations.authorization(registeredClient)
705704
.principalName("user")
706705
.build();
706+
Map<String, Object> additionalParameters = new HashMap<>();
707+
additionalParameters.put(PkceParameterNames.CODE_CHALLENGE, S256_CODE_CHALLENGE);
708+
additionalParameters.put(PkceParameterNames.CODE_CHALLENGE_METHOD, "S256");
707709
OAuth2AuthorizationRequest authorizationRequest = authorization
708710
.getAttribute(OAuth2AuthorizationRequest.class.getName());
709711
OAuth2AuthorizationRequest updatedAuthorizationRequest = OAuth2AuthorizationRequest.from(authorizationRequest)
710712
.state(STATE_URL_UNENCODED)
713+
.additionalParameters(additionalParameters)
711714
.build();
712715
authorization = OAuth2Authorization.from(authorization)
713716
.attribute(OAuth2AuthorizationRequest.class.getName(), updatedAuthorizationRequest)
@@ -793,7 +796,7 @@ public void requestWhenCustomConsentCustomizerConfiguredThenUsed() throws Except
793796
.build();
794797
this.registeredClientRepository.save(registeredClient);
795798

796-
OAuth2Authorization authorization = TestOAuth2Authorizations.authorization(registeredClient).build();
799+
OAuth2Authorization authorization = createAuthorization(registeredClient);
797800
OAuth2AuthorizationRequest authorizationRequest = authorization
798801
.getAttribute(OAuth2AuthorizationRequest.class.getName());
799802
OAuth2AuthorizationRequest updatedAuthorizationRequest = OAuth2AuthorizationRequest.from(authorizationRequest)
@@ -906,8 +909,6 @@ public void requestWhenClientObtainsAccessTokenThenClientAuthenticationNotPersis
906909
MvcResult mvcResult = this.mvc
907910
.perform(get(DEFAULT_AUTHORIZATION_ENDPOINT_URI)
908911
.queryParams(getAuthorizationRequestParameters(registeredClient))
909-
.queryParam(PkceParameterNames.CODE_CHALLENGE, S256_CODE_CHALLENGE)
910-
.queryParam(PkceParameterNames.CODE_CHALLENGE_METHOD, "S256")
911912
.with(user("user")))
912913
.andExpect(status().is3xxRedirection())
913914
.andReturn();
@@ -926,8 +927,7 @@ public void requestWhenClientObtainsAccessTokenThenClientAuthenticationNotPersis
926927
mvcResult = this.mvc
927928
.perform(post(DEFAULT_TOKEN_ENDPOINT_URI)
928929
.params(getTokenRequestParameters(registeredClient, authorizationCodeAuthorization))
929-
.param(OAuth2ParameterNames.CLIENT_ID, registeredClient.getClientId())
930-
.param(PkceParameterNames.CODE_VERIFIER, S256_CODE_VERIFIER))
930+
.param(OAuth2ParameterNames.CLIENT_ID, registeredClient.getClientId()))
931931
.andExpect(header().string(HttpHeaders.CACHE_CONTROL, containsString("no-store")))
932932
.andExpect(header().string(HttpHeaders.PRAGMA, containsString("no-cache")))
933933
.andExpect(status().isOk())
@@ -956,8 +956,6 @@ public void requestWhenAuthorizationAndTokenRequestIncludesIssuerPathThenIssuerR
956956
MvcResult mvcResult = this.mvc
957957
.perform(get(issuer.concat(DEFAULT_AUTHORIZATION_ENDPOINT_URI))
958958
.queryParams(getAuthorizationRequestParameters(registeredClient))
959-
.queryParam(PkceParameterNames.CODE_CHALLENGE, S256_CODE_CHALLENGE)
960-
.queryParam(PkceParameterNames.CODE_CHALLENGE_METHOD, "S256")
961959
.with(user("user")))
962960
.andExpect(status().is3xxRedirection())
963961
.andReturn();
@@ -969,8 +967,7 @@ public void requestWhenAuthorizationAndTokenRequestIncludesIssuerPathThenIssuerR
969967
this.mvc
970968
.perform(post(issuer.concat(DEFAULT_TOKEN_ENDPOINT_URI))
971969
.params(getTokenRequestParameters(registeredClient, authorizationCodeAuthorization))
972-
.param(OAuth2ParameterNames.CLIENT_ID, registeredClient.getClientId())
973-
.param(PkceParameterNames.CODE_VERIFIER, S256_CODE_VERIFIER))
970+
.param(OAuth2ParameterNames.CLIENT_ID, registeredClient.getClientId()))
974971
.andExpect(header().string(HttpHeaders.CACHE_CONTROL, containsString("no-store")))
975972
.andExpect(header().string(HttpHeaders.PRAGMA, containsString("no-cache")))
976973
.andExpect(status().isOk())
@@ -994,7 +991,7 @@ public void requestWhenTokenRequestWithDPoPProofThenReturnDPoPBoundAccessToken()
994991
RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build();
995992
this.registeredClientRepository.save(registeredClient);
996993

997-
OAuth2Authorization authorization = TestOAuth2Authorizations.authorization(registeredClient).build();
994+
OAuth2Authorization authorization = createAuthorization(registeredClient);
998995
this.authorizationService.save(authorization);
999996

1000997
String tokenEndpointUri = "http://localhost" + DEFAULT_TOKEN_ENDPOINT_URI;
@@ -1025,8 +1022,6 @@ public void requestWhenPushedAuthorizationRequestThenReturnAccessTokenResponse()
10251022

10261023
MvcResult mvcResult = this.mvc
10271024
.perform(post("/oauth2/par").params(getAuthorizationRequestParameters(registeredClient))
1028-
.param(PkceParameterNames.CODE_CHALLENGE, S256_CODE_CHALLENGE)
1029-
.param(PkceParameterNames.CODE_CHALLENGE_METHOD, "S256")
10301025
.header(HttpHeaders.AUTHORIZATION, getAuthorizationHeader(registeredClient)))
10311026
.andExpect(header().string(HttpHeaders.CACHE_CONTROL, containsString("no-store")))
10321027
.andExpect(header().string(HttpHeaders.PRAGMA, containsString("no-cache")))
@@ -1053,7 +1048,6 @@ public void requestWhenPushedAuthorizationRequestThenReturnAccessTokenResponse()
10531048
.perform(post(DEFAULT_TOKEN_ENDPOINT_URI)
10541049
.params(getTokenRequestParameters(registeredClient, authorizationCodeAuthorization))
10551050
.param(OAuth2ParameterNames.CLIENT_ID, registeredClient.getClientId())
1056-
.param(PkceParameterNames.CODE_VERIFIER, S256_CODE_VERIFIER)
10571051
.header(HttpHeaders.AUTHORIZATION, getAuthorizationHeader(registeredClient)))
10581052
.andExpect(header().string(HttpHeaders.CACHE_CONTROL, containsString("no-store")))
10591053
.andExpect(header().string(HttpHeaders.PRAGMA, containsString("no-cache")))
@@ -1077,6 +1071,13 @@ public void requestWhenPushedAuthorizationRequestThenReturnAccessTokenResponse()
10771071
.isEqualTo(true);
10781072
}
10791073

1074+
private static OAuth2Authorization createAuthorization(RegisteredClient registeredClient) {
1075+
Map<String, Object> additionalParameters = new HashMap<>();
1076+
additionalParameters.put(PkceParameterNames.CODE_CHALLENGE, S256_CODE_CHALLENGE);
1077+
additionalParameters.put(PkceParameterNames.CODE_CHALLENGE_METHOD, "S256");
1078+
return TestOAuth2Authorizations.authorization(registeredClient, additionalParameters).build();
1079+
}
1080+
10801081
private static String generateDPoPProof(String tokenEndpointUri) {
10811082
// @formatter:off
10821083
Map<String, Object> publicJwk = TestJwks.DEFAULT_EC_JWK
@@ -1105,6 +1106,8 @@ private static MultiValueMap<String, String> getAuthorizationRequestParameters(R
11051106
parameters.set(OAuth2ParameterNames.SCOPE,
11061107
StringUtils.collectionToDelimitedString(registeredClient.getScopes(), " "));
11071108
parameters.set(OAuth2ParameterNames.STATE, STATE_URL_UNENCODED);
1109+
parameters.set(PkceParameterNames.CODE_CHALLENGE, S256_CODE_CHALLENGE);
1110+
parameters.set(PkceParameterNames.CODE_CHALLENGE_METHOD, "S256");
11081111
return parameters;
11091112
}
11101113

@@ -1115,6 +1118,7 @@ private static MultiValueMap<String, String> getTokenRequestParameters(Registere
11151118
parameters.set(OAuth2ParameterNames.CODE,
11161119
authorization.getToken(OAuth2AuthorizationCode.class).getToken().getTokenValue());
11171120
parameters.set(OAuth2ParameterNames.REDIRECT_URI, registeredClient.getRedirectUris().iterator().next());
1121+
parameters.set(PkceParameterNames.CODE_VERIFIER, S256_CODE_VERIFIER);
11181122
return parameters;
11191123
}
11201124

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@
8989
import org.springframework.security.oauth2.server.authorization.http.converter.OAuth2TokenIntrospectionHttpMessageConverter;
9090
import org.springframework.security.oauth2.server.authorization.jackson2.TestingAuthenticationTokenMixin;
9191
import org.springframework.security.oauth2.server.authorization.settings.AuthorizationServerSettings;
92+
import org.springframework.security.oauth2.server.authorization.settings.ClientSettings;
9293
import org.springframework.security.oauth2.server.authorization.settings.OAuth2TokenFormat;
9394
import org.springframework.security.oauth2.server.authorization.settings.TokenSettings;
9495
import org.springframework.security.oauth2.server.authorization.token.OAuth2TokenClaimsContext;
@@ -310,6 +311,7 @@ public void requestWhenObtainReferenceAccessTokenAndIntrospectThenActive() throw
310311
.build();
311312
RegisteredClient authorizedRegisteredClient = TestRegisteredClients.registeredClient()
312313
.tokenSettings(tokenSettings)
314+
.clientSettings(ClientSettings.builder().requireProofKey(false).build())
313315
.build();
314316
// @formatter:on
315317
this.registeredClientRepository.save(authorizedRegisteredClient);

0 commit comments

Comments
 (0)