KEYCLOAK-14201 Client Policy - Executor : Enforce Proof Key for Code Exchange (PKCE)

This commit is contained in:
Takashi Norimatsu 2020-10-15 11:30:59 +09:00 committed by Marek Posolda
parent f2856385bd
commit a63814da67
5 changed files with 47 additions and 40 deletions

View file

@ -15,7 +15,7 @@
* limitations under the License. * limitations under the License.
*/ */
package org.keycloak.testsuite.services.clientpolicy.executor; package org.keycloak.services.clientpolicy.executor;
import java.security.MessageDigest; import java.security.MessageDigest;
import java.util.regex.Matcher; import java.util.regex.Matcher;
@ -44,14 +44,14 @@ import org.keycloak.services.clientpolicy.ClientPolicyException;
import org.keycloak.services.clientpolicy.TokenRequestContext; import org.keycloak.services.clientpolicy.TokenRequestContext;
import org.keycloak.services.clientpolicy.executor.AbstractAugumentingClientRegistrationPolicyExecutor; import org.keycloak.services.clientpolicy.executor.AbstractAugumentingClientRegistrationPolicyExecutor;
public class TestPKCEEnforceExecutor extends AbstractAugumentingClientRegistrationPolicyExecutor { public class PKCEEnforceExecutor extends AbstractAugumentingClientRegistrationPolicyExecutor {
private static final Logger logger = Logger.getLogger(TestPKCEEnforceExecutor.class); private static final Logger logger = Logger.getLogger(PKCEEnforceExecutor.class);
private static final Pattern VALID_CODE_CHALLENGE_PATTERN = Pattern.compile("^[0-9a-zA-Z\\-\\.~_]+$"); private static final Pattern VALID_CODE_CHALLENGE_PATTERN = Pattern.compile("^[0-9a-zA-Z\\-\\.~_]+$");
private static final Pattern VALID_CODE_VERIFIER_PATTERN = Pattern.compile("^[0-9a-zA-Z\\-\\.~_]+$"); private static final Pattern VALID_CODE_VERIFIER_PATTERN = Pattern.compile("^[0-9a-zA-Z\\-\\.~_]+$");
public TestPKCEEnforceExecutor(KeycloakSession session, ComponentModel componentModel) { public PKCEEnforceExecutor(KeycloakSession session, ComponentModel componentModel) {
super(session, componentModel); super(session, componentModel);
} }
@ -99,8 +99,13 @@ public class TestPKCEEnforceExecutor extends AbstractAugumentingClientRegistrati
throw new ClientPolicyException(OAuthErrorException.INVALID_REQUEST, "Missing parameter: code_challenge_method"); throw new ClientPolicyException(OAuthErrorException.INVALID_REQUEST, "Missing parameter: code_challenge_method");
} }
// check whether acceptable code challenge method is specified
if (!isAcceptableCodeChallengeMethod(codeChallengeMethod)) {
throw new ClientPolicyException(OAuthErrorException.INVALID_REQUEST, "Invalid parameter: invalid code_challenge_method");
}
// check whether specified code challenge method is configured one in advance // check whether specified code challenge method is configured one in advance
if (!codeChallengeMethod.equals(pkceCodeChallengeMethod)) { if (pkceCodeChallengeMethod != null && !codeChallengeMethod.equals(pkceCodeChallengeMethod)) {
throw new ClientPolicyException(OAuthErrorException.INVALID_REQUEST, "Invalid parameter: code challenge method is not configured one"); throw new ClientPolicyException(OAuthErrorException.INVALID_REQUEST, "Invalid parameter: code challenge method is not configured one");
} }
@ -116,16 +121,9 @@ public class TestPKCEEnforceExecutor extends AbstractAugumentingClientRegistrati
} }
private void executeOnTokenRequest( private boolean isAcceptableCodeChallengeMethod(String method) {
MultivaluedMap<String, String> params, return OAuth2Constants.PKCE_METHOD_S256.equals(method);
OAuth2CodeParser.ParseResult parseResult) throws ClientPolicyException { }
String codeVerifier = params.getFirst(OAuth2Constants.CODE_VERIFIER);
OAuth2Code codeData = parseResult.getCodeData();
String codeChallenge = codeData.getCodeChallenge();
String codeChallengeMethod = codeData.getCodeChallengeMethod();
checkParamsForPkceEnforcedClient(codeVerifier, codeChallenge, codeChallengeMethod);
};
private boolean isValidPkceCodeChallenge(String codeChallenge) { private boolean isValidPkceCodeChallenge(String codeChallenge) {
if (codeChallenge.length() < OIDCLoginProtocol.PKCE_CODE_CHALLENGE_MIN_LENGTH) { if (codeChallenge.length() < OIDCLoginProtocol.PKCE_CODE_CHALLENGE_MIN_LENGTH) {
@ -138,6 +136,17 @@ public class TestPKCEEnforceExecutor extends AbstractAugumentingClientRegistrati
return m.matches(); return m.matches();
} }
private void executeOnTokenRequest(
MultivaluedMap<String, String> params,
OAuth2CodeParser.ParseResult parseResult) throws ClientPolicyException {
String codeVerifier = params.getFirst(OAuth2Constants.CODE_VERIFIER);
OAuth2Code codeData = parseResult.getCodeData();
String codeChallenge = codeData.getCodeChallenge();
String codeChallengeMethod = codeData.getCodeChallengeMethod();
checkParamsForPkceEnforcedClient(codeVerifier, codeChallenge, codeChallengeMethod);
};
private void checkParamsForPkceEnforcedClient(String codeVerifier, String codeChallenge, String codeChallengeMethod) throws ClientPolicyException { private void checkParamsForPkceEnforcedClient(String codeVerifier, String codeChallenge, String codeChallengeMethod) throws ClientPolicyException {
// check whether code verifier is specified // check whether code verifier is specified
if (codeVerifier == null) { if (codeVerifier == null) {
@ -146,7 +155,6 @@ public class TestPKCEEnforceExecutor extends AbstractAugumentingClientRegistrati
verifyCodeVerifier(codeVerifier, codeChallenge, codeChallengeMethod); verifyCodeVerifier(codeVerifier, codeChallenge, codeChallengeMethod);
} }
private void verifyCodeVerifier(String codeVerifier, String codeChallenge, String codeChallengeMethod) throws ClientPolicyException { private void verifyCodeVerifier(String codeVerifier, String codeChallenge, String codeChallengeMethod) throws ClientPolicyException {
// check whether code verifier is formatted along with the PKCE specification // check whether code verifier is formatted along with the PKCE specification

View file

@ -15,7 +15,7 @@
* limitations under the License. * limitations under the License.
*/ */
package org.keycloak.testsuite.services.clientpolicy.executor; package org.keycloak.services.clientpolicy.executor;
import java.util.List; import java.util.List;
@ -27,13 +27,13 @@ import org.keycloak.provider.ProviderConfigProperty;
import org.keycloak.services.clientpolicy.executor.AbstractAugumentingClientRegistrationPolicyExecutorFactory; import org.keycloak.services.clientpolicy.executor.AbstractAugumentingClientRegistrationPolicyExecutorFactory;
import org.keycloak.services.clientpolicy.executor.ClientPolicyExecutorProvider; import org.keycloak.services.clientpolicy.executor.ClientPolicyExecutorProvider;
public class TestPKCEEnforceExecutorFactory extends AbstractAugumentingClientRegistrationPolicyExecutorFactory { public class PKCEEnforceExecutorFactory extends AbstractAugumentingClientRegistrationPolicyExecutorFactory {
public static final String PROVIDER_ID = "test-pkce-enforce-executor"; public static final String PROVIDER_ID = "pkce-enforce-executor";
@Override @Override
public ClientPolicyExecutorProvider create(KeycloakSession session, ComponentModel model) { public ClientPolicyExecutorProvider create(KeycloakSession session, ComponentModel model) {
return new TestPKCEEnforceExecutor(session, model); return new PKCEEnforceExecutor(session, model);
} }
@Override @Override
@ -55,7 +55,7 @@ public class TestPKCEEnforceExecutorFactory extends AbstractAugumentingClientReg
@Override @Override
public String getHelpText() { public String getHelpText() {
return null; return "It makes the client enforce Proof Key for Code Exchange operation.";
} }
@Override @Override

View file

@ -1,3 +1,4 @@
org.keycloak.services.clientpolicy.executor.SecureResponseTypeExecutorFactory org.keycloak.services.clientpolicy.executor.SecureResponseTypeExecutorFactory
org.keycloak.services.clientpolicy.executor.SecureRequestObjectExecutorFactory org.keycloak.services.clientpolicy.executor.SecureRequestObjectExecutorFactory
org.keycloak.services.clientpolicy.executor.SecureClientAuthEnforceExecutorFactory org.keycloak.services.clientpolicy.executor.SecureClientAuthEnforceExecutorFactory
org.keycloak.services.clientpolicy.executor.PKCEEnforceExecutorFactory

View file

@ -83,6 +83,7 @@ import org.keycloak.services.clientpolicy.condition.ClientUpdateContextCondition
import org.keycloak.services.clientpolicy.condition.ClientRolesConditionFactory; import org.keycloak.services.clientpolicy.condition.ClientRolesConditionFactory;
import org.keycloak.services.clientpolicy.executor.ClientPolicyExecutorProvider; import org.keycloak.services.clientpolicy.executor.ClientPolicyExecutorProvider;
import org.keycloak.services.clientpolicy.executor.SecureClientAuthEnforceExecutorFactory; import org.keycloak.services.clientpolicy.executor.SecureClientAuthEnforceExecutorFactory;
import org.keycloak.services.clientpolicy.executor.PKCEEnforceExecutorFactory;
import org.keycloak.services.clientpolicy.executor.SecureRequestObjectExecutor; import org.keycloak.services.clientpolicy.executor.SecureRequestObjectExecutor;
import org.keycloak.services.clientpolicy.executor.SecureRequestObjectExecutorFactory; import org.keycloak.services.clientpolicy.executor.SecureRequestObjectExecutorFactory;
import org.keycloak.services.clientpolicy.executor.SecureResponseTypeExecutorFactory; import org.keycloak.services.clientpolicy.executor.SecureResponseTypeExecutorFactory;
@ -94,7 +95,6 @@ import org.keycloak.testsuite.client.resources.TestApplicationResourceUrls;
import org.keycloak.testsuite.client.resources.TestOIDCEndpointsApplicationResource; import org.keycloak.testsuite.client.resources.TestOIDCEndpointsApplicationResource;
import org.keycloak.testsuite.rest.resource.TestingOIDCEndpointsApplicationResource.AuthorizationEndpointRequestObject; import org.keycloak.testsuite.rest.resource.TestingOIDCEndpointsApplicationResource.AuthorizationEndpointRequestObject;
import org.keycloak.testsuite.services.clientpolicy.condition.TestRaiseExeptionConditionFactory; import org.keycloak.testsuite.services.clientpolicy.condition.TestRaiseExeptionConditionFactory;
import org.keycloak.testsuite.services.clientpolicy.executor.TestPKCEEnforceExecutorFactory;
import org.keycloak.testsuite.util.OAuthClient; import org.keycloak.testsuite.util.OAuthClient;
import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.JsonNode;
@ -409,11 +409,11 @@ public class ClientPolicyBasicsTest extends AbstractKeycloakTest {
createPolicy(policyName, DefaultClientPolicyProviderFactory.PROVIDER_ID, null, null, null); createPolicy(policyName, DefaultClientPolicyProviderFactory.PROVIDER_ID, null, null, null);
logger.info("... Created Policy : " + policyName); logger.info("... Created Policy : " + policyName);
createExecutor("TestPKCEEnforceExecutor", TestPKCEEnforceExecutorFactory.PROVIDER_ID, null, (ComponentRepresentation provider) -> { createExecutor("PKCEEnforceExecutor", PKCEEnforceExecutorFactory.PROVIDER_ID, null, (ComponentRepresentation provider) -> {
setExecutorAugmentActivate(provider); setExecutorAugmentActivate(provider);
}); });
registerExecutor("TestPKCEEnforceExecutor", policyName); registerExecutor("PKCEEnforceExecutor", policyName);
logger.info("... Registered Executor : TestPKCEEnforceExecutor"); logger.info("... Registered Executor : PKCEEnforceExecutor");
String clientId = "Zahlungs-App"; String clientId = "Zahlungs-App";
String clientSecret = "secret"; String clientSecret = "secret";
@ -477,15 +477,15 @@ public class ClientPolicyBasicsTest extends AbstractKeycloakTest {
try { try {
successfulLoginAndLogout(clientId, clientSecret); successfulLoginAndLogout(clientId, clientSecret);
createExecutor("TestPKCEEnforceExecutor", TestPKCEEnforceExecutorFactory.PROVIDER_ID, null, (ComponentRepresentation provider) -> { createExecutor("PKCEEnforceExecutor", PKCEEnforceExecutorFactory.PROVIDER_ID, null, (ComponentRepresentation provider) -> {
setExecutorAugmentDeactivate(provider); setExecutorAugmentDeactivate(provider);
}); });
registerExecutor("TestPKCEEnforceExecutor", policyName); registerExecutor("PKCEEnforceExecutor", policyName);
logger.info("... Registered Executor : TestPKCEEnforceExecutor"); logger.info("... Registered Executor : PKCEEnforceExecutor");
failLoginByNotFollowingPKCE(clientId); failLoginByNotFollowingPKCE(clientId);
updateExecutor("TestPKCEEnforceExecutor", (ComponentRepresentation provider) -> { updateExecutor("PKCEEnforceExecutor", (ComponentRepresentation provider) -> {
setExecutorAugmentActivate(provider); setExecutorAugmentActivate(provider);
}); });
@ -495,8 +495,8 @@ public class ClientPolicyBasicsTest extends AbstractKeycloakTest {
assertEquals(false, getClientByAdmin(cid).isServiceAccountsEnabled()); assertEquals(false, getClientByAdmin(cid).isServiceAccountsEnabled());
assertEquals(OAuth2Constants.PKCE_METHOD_S256, OIDCAdvancedConfigWrapper.fromClientRepresentation(getClientByAdmin(cid)).getPkceCodeChallengeMethod()); assertEquals(OAuth2Constants.PKCE_METHOD_S256, OIDCAdvancedConfigWrapper.fromClientRepresentation(getClientByAdmin(cid)).getPkceCodeChallengeMethod());
deleteExecutor("TestPKCEEnforceExecutor", policyName); deleteExecutor("PKCEEnforceExecutor", policyName);
logger.info("... Deleted Executor : TestPKCEEnforceExecutor"); logger.info("... Deleted Executor : PKCEEnforceExecutor");
updateClientByAdmin(cid, (ClientRepresentation clientRep) -> { updateClientByAdmin(cid, (ClientRepresentation clientRep) -> {
OIDCAdvancedConfigWrapper.fromClientRepresentation(clientRep).setPkceCodeChallengeMethod(null); OIDCAdvancedConfigWrapper.fromClientRepresentation(clientRep).setPkceCodeChallengeMethod(null);
@ -547,11 +547,11 @@ public class ClientPolicyBasicsTest extends AbstractKeycloakTest {
registerCondition(CLIENTROLES_CONDITION_BETA_NAME, policyBetaName); registerCondition(CLIENTROLES_CONDITION_BETA_NAME, policyBetaName);
logger.info("... Registered Condition : " + CLIENTROLES_CONDITION_BETA_NAME); logger.info("... Registered Condition : " + CLIENTROLES_CONDITION_BETA_NAME);
createExecutor("TestPKCEEnforceExecutor-beta", TestPKCEEnforceExecutorFactory.PROVIDER_ID, null, (ComponentRepresentation provider) -> { createExecutor("PKCEEnforceExecutor-beta", PKCEEnforceExecutorFactory.PROVIDER_ID, null, (ComponentRepresentation provider) -> {
setExecutorAugmentActivate(provider); setExecutorAugmentActivate(provider);
}); });
registerExecutor("TestPKCEEnforceExecutor-beta", policyBetaName); registerExecutor("PKCEEnforceExecutor-beta", policyBetaName);
logger.info("... Registered Executor : TestPKCEEnforceExecutor-beta"); logger.info("... Registered Executor : PKCEEnforceExecutor-beta");
String clientAlphaId = "Alpha-App"; String clientAlphaId = "Alpha-App";
String clientAlphaSecret = "secretAlpha"; String clientAlphaSecret = "secretAlpha";
@ -872,11 +872,11 @@ public class ClientPolicyBasicsTest extends AbstractKeycloakTest {
registerExecutor("SecureClientAuthEnforceExecutor", policyName); registerExecutor("SecureClientAuthEnforceExecutor", policyName);
logger.info("... Registered Executor : SecureClientAuthEnforceExecutor"); logger.info("... Registered Executor : SecureClientAuthEnforceExecutor");
createExecutor("TestPKCEEnforceExecutor", TestPKCEEnforceExecutorFactory.PROVIDER_ID, null, (ComponentRepresentation provider) -> { createExecutor("PKCEEnforceExecutor", PKCEEnforceExecutorFactory.PROVIDER_ID, null, (ComponentRepresentation provider) -> {
setExecutorAugmentActivate(provider); setExecutorAugmentActivate(provider);
}); });
registerExecutor("TestPKCEEnforceExecutor", policyName); registerExecutor("PKCEEnforceExecutor", policyName);
logger.info("... Registered Executor : TestPKCEEnforceExecutor"); logger.info("... Registered Executor : PKCEEnforceExecutor");
} }
@ -1155,5 +1155,4 @@ public class ClientPolicyBasicsTest extends AbstractKeycloakTest {
private void setExecutorAugmentedClientAuthMethod(ComponentRepresentation provider, String augmentedClientAuthMethod) { private void setExecutorAugmentedClientAuthMethod(ComponentRepresentation provider, String augmentedClientAuthMethod) {
provider.getConfig().putSingle(SecureClientAuthEnforceExecutorFactory.CLIENT_AUTHNS_AUGMENT, augmentedClientAuthMethod); provider.getConfig().putSingle(SecureClientAuthEnforceExecutorFactory.CLIENT_AUTHNS_AUGMENT, augmentedClientAuthMethod);
} }
} }