Remove support for "plain" code_challenge_method parameter

Closes gh-756
This commit is contained in:
Joe Grandja
2022-05-24 19:37:21 -04:00
parent c4406cda67
commit ca2ffb0756
9 changed files with 19 additions and 122 deletions

View File

@@ -113,8 +113,6 @@ final class CodeVerifierAuthenticator {
private static boolean codeVerifierValid(String codeVerifier, String codeChallenge, String codeChallengeMethod) {
if (!StringUtils.hasText(codeVerifier)) {
return false;
} else if (!StringUtils.hasText(codeChallengeMethod) || "plain".equals(codeChallengeMethod)) {
return codeVerifier.equals(codeChallenge);
} else if ("S256".equals(codeChallengeMethod)) {
try {
MessageDigest md = MessageDigest.getInstance("SHA-256");

View File

@@ -210,7 +210,7 @@ public final class OAuth2AuthorizationCodeRequestAuthenticationProvider implemen
if (StringUtils.hasText(codeChallenge)) {
String codeChallengeMethod = (String) authorizationCodeRequestAuthentication.getAdditionalParameters().get(PkceParameterNames.CODE_CHALLENGE_METHOD);
if (StringUtils.hasText(codeChallengeMethod)) {
if (!"S256".equals(codeChallengeMethod) && !"plain".equals(codeChallengeMethod)) {
if (!"S256".equals(codeChallengeMethod)) {
throwError(OAuth2ErrorCodes.INVALID_REQUEST, PkceParameterNames.CODE_CHALLENGE_METHOD, PKCE_ERROR_URI,
authorizationCodeRequestAuthentication, registeredClient, null);
}

View File

@@ -94,7 +94,6 @@ public final class OAuth2AuthorizationServerMetadataEndpointFilter extends OnceP
.tokenRevocationEndpointAuthenticationMethods(clientAuthenticationMethods())
.tokenIntrospectionEndpoint(asUrl(issuer, this.providerSettings.getTokenIntrospectionEndpoint()))
.tokenIntrospectionEndpointAuthenticationMethods(clientAuthenticationMethods())
.codeChallengeMethod("plain")
.codeChallengeMethod("S256")
.build();

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2020-2021 the original author or authors.
* Copyright 2020-2022 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.
@@ -59,7 +59,6 @@ public class OAuth2AuthorizationServerMetadataTests {
.tokenRevocationEndpointAuthenticationMethod(ClientAuthenticationMethod.CLIENT_SECRET_BASIC.getValue())
.tokenIntrospectionEndpoint("https://example.com/issuer1/oauth2/introspect")
.tokenIntrospectionEndpointAuthenticationMethod(ClientAuthenticationMethod.CLIENT_SECRET_BASIC.getValue())
.codeChallengeMethod("plain")
.codeChallengeMethod("S256")
.claim("a-claim", "a-value")
.build();
@@ -76,7 +75,7 @@ public class OAuth2AuthorizationServerMetadataTests {
assertThat(authorizationServerMetadata.getTokenRevocationEndpointAuthenticationMethods()).containsExactly(ClientAuthenticationMethod.CLIENT_SECRET_BASIC.getValue());
assertThat(authorizationServerMetadata.getTokenIntrospectionEndpoint()).isEqualTo(url("https://example.com/issuer1/oauth2/introspect"));
assertThat(authorizationServerMetadata.getTokenIntrospectionEndpointAuthenticationMethods()).containsExactly(ClientAuthenticationMethod.CLIENT_SECRET_BASIC.getValue());
assertThat(authorizationServerMetadata.getCodeChallengeMethods()).containsExactlyInAnyOrder("plain", "S256");
assertThat(authorizationServerMetadata.getCodeChallengeMethods()).containsExactly("S256");
assertThat(authorizationServerMetadata.getClaimAsString("a-claim")).isEqualTo("a-value");
}

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2020-2021 the original author or authors.
* Copyright 2020-2022 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.
@@ -103,7 +103,7 @@ public class OAuth2AuthorizationServerMetadataHttpMessageConverterTests {
+ " \"revocation_endpoint_auth_methods_supported\": [\"client_secret_basic\"],\n"
+ " \"introspection_endpoint\": \"https://example.com/issuer1/oauth2/introspect\",\n"
+ " \"introspection_endpoint_auth_methods_supported\": [\"client_secret_basic\"],\n"
+ " \"code_challenge_methods_supported\": [\"plain\",\"S256\"],\n"
+ " \"code_challenge_methods_supported\": [\"S256\"],\n"
+ " \"custom_claim\": \"value\",\n"
+ " \"custom_collection_claim\": [\"value1\", \"value2\"]\n"
+ "}\n";
@@ -125,7 +125,7 @@ public class OAuth2AuthorizationServerMetadataHttpMessageConverterTests {
assertThat(authorizationServerMetadata.getTokenRevocationEndpointAuthenticationMethods()).containsExactly(ClientAuthenticationMethod.CLIENT_SECRET_BASIC.getValue());
assertThat(authorizationServerMetadata.getTokenIntrospectionEndpoint()).isEqualTo(new URL("https://example.com/issuer1/oauth2/introspect"));
assertThat(authorizationServerMetadata.getTokenIntrospectionEndpointAuthenticationMethods()).containsExactly(ClientAuthenticationMethod.CLIENT_SECRET_BASIC.getValue());
assertThat(authorizationServerMetadata.getCodeChallengeMethods()).containsExactlyInAnyOrder("plain", "S256");
assertThat(authorizationServerMetadata.getCodeChallengeMethods()).containsExactly("S256");
assertThat(authorizationServerMetadata.getClaimAsString("custom_claim")).isEqualTo("value");
assertThat(authorizationServerMetadata.getClaimAsStringList("custom_collection_claim")).containsExactlyInAnyOrder("value1", "value2");
}
@@ -172,7 +172,6 @@ public class OAuth2AuthorizationServerMetadataHttpMessageConverterTests {
.tokenRevocationEndpointAuthenticationMethod(ClientAuthenticationMethod.CLIENT_SECRET_BASIC.getValue())
.tokenIntrospectionEndpoint("https://example.com/issuer1/oauth2/introspect")
.tokenIntrospectionEndpointAuthenticationMethod(ClientAuthenticationMethod.CLIENT_SECRET_BASIC.getValue())
.codeChallengeMethod("plain")
.codeChallengeMethod("S256")
.claim("custom_claim", "value")
.claim("custom_collection_claim", Arrays.asList("value1", "value2"))
@@ -194,7 +193,7 @@ public class OAuth2AuthorizationServerMetadataHttpMessageConverterTests {
assertThat(authorizationServerMetadataResponse).contains("\"revocation_endpoint_auth_methods_supported\":[\"client_secret_basic\"]");
assertThat(authorizationServerMetadataResponse).contains("\"introspection_endpoint\":\"https://example.com/issuer1/oauth2/introspect\"");
assertThat(authorizationServerMetadataResponse).contains("\"introspection_endpoint_auth_methods_supported\":[\"client_secret_basic\"]");
assertThat(authorizationServerMetadataResponse).contains("\"code_challenge_methods_supported\":[\"plain\",\"S256\"]");
assertThat(authorizationServerMetadataResponse).contains("\"code_challenge_methods_supported\":[\"S256\"]");
assertThat(authorizationServerMetadataResponse).contains("\"custom_claim\":\"value\"");
assertThat(authorizationServerMetadataResponse).contains("\"custom_collection_claim\":[\"value1\",\"value2\"]");
}

View File

@@ -54,9 +54,6 @@ import static org.mockito.Mockito.when;
* @author Daniel Garnier-Moiroux
*/
public class ClientSecretAuthenticationProviderTests {
private static final String PLAIN_CODE_VERIFIER = "pkce-key";
private static final String PLAIN_CODE_CHALLENGE = PLAIN_CODE_VERIFIER;
// See RFC 7636: Appendix B. Example for the S256 code_challenge_method
// https://tools.ietf.org/html/rfc7636#appendix-B
private static final String S256_CODE_VERIFIER = "dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk";
@@ -231,12 +228,12 @@ public class ClientSecretAuthenticationProviderTests {
.thenReturn(registeredClient);
OAuth2Authorization authorization = TestOAuth2Authorizations
.authorization(registeredClient, createPkceAuthorizationParametersPlain())
.authorization(registeredClient, createPkceAuthorizationParametersS256())
.build();
when(this.authorizationService.findByToken(eq(AUTHORIZATION_CODE), eq(AUTHORIZATION_CODE_TOKEN_TYPE)))
.thenReturn(authorization);
Map<String, Object> parameters = createPkceTokenParameters(PLAIN_CODE_VERIFIER);
Map<String, Object> parameters = createPkceTokenParameters(S256_CODE_VERIFIER);
parameters.put(OAuth2ParameterNames.CODE, "invalid-code");
OAuth2ClientAuthenticationToken authentication = new OAuth2ClientAuthenticationToken(
@@ -258,7 +255,7 @@ public class ClientSecretAuthenticationProviderTests {
.thenReturn(registeredClient);
OAuth2Authorization authorization = TestOAuth2Authorizations
.authorization(registeredClient, createPkceAuthorizationParametersPlain())
.authorization(registeredClient, createPkceAuthorizationParametersS256())
.build();
when(this.authorizationService.findByToken(eq(AUTHORIZATION_CODE), eq(AUTHORIZATION_CODE_TOKEN_TYPE)))
.thenReturn(authorization);
@@ -317,13 +314,6 @@ public class ClientSecretAuthenticationProviderTests {
return parameters;
}
private static Map<String, Object> createPkceAuthorizationParametersPlain() {
Map<String, Object> parameters = new HashMap<>();
parameters.put(PkceParameterNames.CODE_CHALLENGE_METHOD, "plain");
parameters.put(PkceParameterNames.CODE_CHALLENGE, PLAIN_CODE_CHALLENGE);
return parameters;
}
private static Map<String, Object> createPkceAuthorizationParametersS256() {
Map<String, Object> parameters = new HashMap<>();
parameters.put(PkceParameterNames.CODE_CHALLENGE_METHOD, "S256");

View File

@@ -48,9 +48,6 @@ import static org.mockito.Mockito.when;
* @author Daniel Garnier-Moiroux
*/
public class PublicClientAuthenticationProviderTests {
private static final String PLAIN_CODE_VERIFIER = "pkce-key";
private static final String PLAIN_CODE_CHALLENGE = PLAIN_CODE_VERIFIER;
// See RFC 7636: Appendix B. Example for the S256 code_challenge_method
// https://tools.ietf.org/html/rfc7636#appendix-B
private static final String S256_CODE_VERIFIER = "dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk";
@@ -131,12 +128,12 @@ public class PublicClientAuthenticationProviderTests {
.thenReturn(registeredClient);
OAuth2Authorization authorization = TestOAuth2Authorizations
.authorization(registeredClient, createPkceAuthorizationParametersPlain())
.authorization(registeredClient, createPkceAuthorizationParametersS256())
.build();
when(this.authorizationService.findByToken(eq(AUTHORIZATION_CODE), eq(AUTHORIZATION_CODE_TOKEN_TYPE)))
.thenReturn(authorization);
Map<String, Object> parameters = createPkceTokenParameters(PLAIN_CODE_VERIFIER);
Map<String, Object> parameters = createPkceTokenParameters(S256_CODE_VERIFIER);
parameters.put(OAuth2ParameterNames.CODE, "invalid-code");
OAuth2ClientAuthenticationToken authentication =
@@ -163,7 +160,7 @@ public class PublicClientAuthenticationProviderTests {
when(this.authorizationService.findByToken(eq(AUTHORIZATION_CODE), eq(AUTHORIZATION_CODE_TOKEN_TYPE)))
.thenReturn(authorization);
Map<String, Object> parameters = createPkceTokenParameters(PLAIN_CODE_VERIFIER);
Map<String, Object> parameters = createPkceTokenParameters(S256_CODE_VERIFIER);
OAuth2ClientAuthenticationToken authentication =
new OAuth2ClientAuthenticationToken(registeredClient.getClientId(), ClientAuthenticationMethod.NONE, null, parameters);
@@ -184,7 +181,7 @@ public class PublicClientAuthenticationProviderTests {
.thenReturn(registeredClient);
OAuth2Authorization authorization = TestOAuth2Authorizations
.authorization(registeredClient, createPkceAuthorizationParametersPlain())
.authorization(registeredClient, createPkceAuthorizationParametersS256())
.build();
when(this.authorizationService.findByToken(eq(AUTHORIZATION_CODE), eq(AUTHORIZATION_CODE_TOKEN_TYPE)))
.thenReturn(authorization);
@@ -203,32 +200,6 @@ public class PublicClientAuthenticationProviderTests {
});
}
@Test
public void authenticateWhenPlainMethodAndInvalidCodeVerifierThenThrowOAuth2AuthenticationException() {
RegisteredClient registeredClient = TestRegisteredClients.registeredPublicClient().build();
when(this.registeredClientRepository.findByClientId(eq(registeredClient.getClientId())))
.thenReturn(registeredClient);
OAuth2Authorization authorization = TestOAuth2Authorizations
.authorization(registeredClient, createPkceAuthorizationParametersPlain())
.build();
when(this.authorizationService.findByToken(eq(AUTHORIZATION_CODE), eq(AUTHORIZATION_CODE_TOKEN_TYPE)))
.thenReturn(authorization);
Map<String, Object> parameters = createPkceTokenParameters("invalid-code-verifier");
OAuth2ClientAuthenticationToken authentication =
new OAuth2ClientAuthenticationToken(registeredClient.getClientId(), ClientAuthenticationMethod.NONE, null, parameters);
assertThatThrownBy(() -> this.authenticationProvider.authenticate(authentication))
.isInstanceOf(OAuth2AuthenticationException.class)
.extracting(ex -> ((OAuth2AuthenticationException) ex).getError())
.satisfies(error -> {
assertThat(error.getErrorCode()).isEqualTo(OAuth2ErrorCodes.INVALID_GRANT);
assertThat(error.getDescription()).contains(PkceParameterNames.CODE_VERIFIER);
});
}
@Test
public void authenticateWhenS256MethodAndInvalidCodeVerifierThenThrowOAuth2AuthenticationException() {
RegisteredClient registeredClient = TestRegisteredClients.registeredPublicClient().build();
@@ -255,58 +226,6 @@ public class PublicClientAuthenticationProviderTests {
});
}
@Test
public void authenticateWhenPlainMethodAndValidCodeVerifierThenAuthenticated() {
RegisteredClient registeredClient = TestRegisteredClients.registeredPublicClient().build();
when(this.registeredClientRepository.findByClientId(eq(registeredClient.getClientId())))
.thenReturn(registeredClient);
OAuth2Authorization authorization = TestOAuth2Authorizations
.authorization(registeredClient, createPkceAuthorizationParametersPlain())
.build();
when(this.authorizationService.findByToken(eq(AUTHORIZATION_CODE), eq(AUTHORIZATION_CODE_TOKEN_TYPE)))
.thenReturn(authorization);
Map<String, Object> parameters = createPkceTokenParameters(PLAIN_CODE_VERIFIER);
OAuth2ClientAuthenticationToken authentication =
new OAuth2ClientAuthenticationToken(registeredClient.getClientId(), ClientAuthenticationMethod.NONE, null, parameters);
OAuth2ClientAuthenticationToken authenticationResult =
(OAuth2ClientAuthenticationToken) this.authenticationProvider.authenticate(authentication);
assertThat(authenticationResult.isAuthenticated()).isTrue();
assertThat(authenticationResult.getPrincipal().toString()).isEqualTo(registeredClient.getClientId());
assertThat(authenticationResult.getCredentials()).isNull();
assertThat(authenticationResult.getRegisteredClient()).isEqualTo(registeredClient);
}
@Test
public void authenticateWhenMissingMethodThenDefaultPlainMethodAndAuthenticated() {
RegisteredClient registeredClient = TestRegisteredClients.registeredPublicClient().build();
when(this.registeredClientRepository.findByClientId(eq(registeredClient.getClientId())))
.thenReturn(registeredClient);
Map<String, Object> authorizationRequestAdditionalParameters = createPkceAuthorizationParametersPlain();
authorizationRequestAdditionalParameters.remove(PkceParameterNames.CODE_CHALLENGE_METHOD);
OAuth2Authorization authorization = TestOAuth2Authorizations
.authorization(registeredClient, authorizationRequestAdditionalParameters)
.build();
when(this.authorizationService.findByToken(eq(AUTHORIZATION_CODE), eq(AUTHORIZATION_CODE_TOKEN_TYPE)))
.thenReturn(authorization);
Map<String, Object> parameters = createPkceTokenParameters(PLAIN_CODE_VERIFIER);
OAuth2ClientAuthenticationToken authentication =
new OAuth2ClientAuthenticationToken(registeredClient.getClientId(), ClientAuthenticationMethod.NONE, null, parameters);
OAuth2ClientAuthenticationToken authenticationResult =
(OAuth2ClientAuthenticationToken) this.authenticationProvider.authenticate(authentication);
assertThat(authenticationResult.isAuthenticated()).isTrue();
assertThat(authenticationResult.getPrincipal().toString()).isEqualTo(registeredClient.getClientId());
assertThat(authenticationResult.getCredentials()).isNull();
assertThat(authenticationResult.getRegisteredClient()).isEqualTo(registeredClient);
}
@Test
public void authenticateWhenS256MethodAndValidCodeVerifierThenAuthenticated() {
RegisteredClient registeredClient = TestRegisteredClients.registeredPublicClient().build();
@@ -338,7 +257,7 @@ public class PublicClientAuthenticationProviderTests {
when(this.registeredClientRepository.findByClientId(eq(registeredClient.getClientId())))
.thenReturn(registeredClient);
Map<String, Object> authorizationRequestAdditionalParameters = createPkceAuthorizationParametersPlain();
Map<String, Object> authorizationRequestAdditionalParameters = createPkceAuthorizationParametersS256();
// This should never happen: the Authorization endpoint should not allow it
authorizationRequestAdditionalParameters.put(PkceParameterNames.CODE_CHALLENGE_METHOD, "unsupported-challenge-method");
OAuth2Authorization authorization = TestOAuth2Authorizations
@@ -347,7 +266,7 @@ public class PublicClientAuthenticationProviderTests {
when(this.authorizationService.findByToken(eq(AUTHORIZATION_CODE), eq(AUTHORIZATION_CODE_TOKEN_TYPE)))
.thenReturn(authorization);
Map<String, Object> parameters = createPkceTokenParameters(PLAIN_CODE_VERIFIER);
Map<String, Object> parameters = createPkceTokenParameters(S256_CODE_VERIFIER);
OAuth2ClientAuthenticationToken authentication =
new OAuth2ClientAuthenticationToken(registeredClient.getClientId(), ClientAuthenticationMethod.NONE, null, parameters);
@@ -372,13 +291,6 @@ public class PublicClientAuthenticationProviderTests {
return parameters;
}
private static Map<String, Object> createPkceAuthorizationParametersPlain() {
Map<String, Object> parameters = new HashMap<>();
parameters.put(PkceParameterNames.CODE_CHALLENGE_METHOD, "plain");
parameters.put(PkceParameterNames.CODE_CHALLENGE, PLAIN_CODE_CHALLENGE);
return parameters;
}
private static Map<String, Object> createPkceAuthorizationParametersS256() {
Map<String, Object> parameters = new HashMap<>();
parameters.put(PkceParameterNames.CODE_CHALLENGE_METHOD, "S256");

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2020-2021 the original author or authors.
* Copyright 2020-2022 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.
@@ -260,7 +260,7 @@ public class OAuth2AuthorizationEndpointFilterTests {
OAuth2ErrorCodes.INVALID_REQUEST,
request -> {
request.addParameter(PkceParameterNames.CODE_CHALLENGE_METHOD, "S256");
request.addParameter(PkceParameterNames.CODE_CHALLENGE_METHOD, "plain");
request.addParameter(PkceParameterNames.CODE_CHALLENGE_METHOD, "S256");
});
}

View File

@@ -132,7 +132,7 @@ public class OAuth2AuthorizationServerMetadataEndpointFilterTests {
assertThat(authorizationServerMetadataResponse).contains("\"revocation_endpoint_auth_methods_supported\":[\"client_secret_basic\",\"client_secret_post\",\"client_secret_jwt\",\"private_key_jwt\"]");
assertThat(authorizationServerMetadataResponse).contains("\"introspection_endpoint\":\"https://example.com/issuer1/oauth2/v1/introspect\"");
assertThat(authorizationServerMetadataResponse).contains("\"introspection_endpoint_auth_methods_supported\":[\"client_secret_basic\",\"client_secret_post\",\"client_secret_jwt\",\"private_key_jwt\"]");
assertThat(authorizationServerMetadataResponse).contains("\"code_challenge_methods_supported\":[\"plain\",\"S256\"]");
assertThat(authorizationServerMetadataResponse).contains("\"code_challenge_methods_supported\":[\"S256\"]");
}
@Test