diff --git a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/services/clientpolicy/executor/TestPKCEEnforceExecutor.java b/services/src/main/java/org/keycloak/services/clientpolicy/executor/PKCEEnforceExecutor.java similarity index 91% rename from testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/services/clientpolicy/executor/TestPKCEEnforceExecutor.java rename to services/src/main/java/org/keycloak/services/clientpolicy/executor/PKCEEnforceExecutor.java index af7229124f..cf00a14b39 100644 --- a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/services/clientpolicy/executor/TestPKCEEnforceExecutor.java +++ b/services/src/main/java/org/keycloak/services/clientpolicy/executor/PKCEEnforceExecutor.java @@ -15,7 +15,7 @@ * limitations under the License. */ -package org.keycloak.testsuite.services.clientpolicy.executor; +package org.keycloak.services.clientpolicy.executor; import java.security.MessageDigest; 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.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_VERIFIER_PATTERN = Pattern.compile("^[0-9a-zA-Z\\-\\.~_]+$"); - public TestPKCEEnforceExecutor(KeycloakSession session, ComponentModel componentModel) { + public PKCEEnforceExecutor(KeycloakSession session, ComponentModel componentModel) { super(session, componentModel); } @@ -99,8 +99,13 @@ public class TestPKCEEnforceExecutor extends AbstractAugumentingClientRegistrati 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 - 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"); } @@ -116,16 +121,9 @@ public class TestPKCEEnforceExecutor extends AbstractAugumentingClientRegistrati } - private void executeOnTokenRequest( - MultivaluedMap 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 boolean isAcceptableCodeChallengeMethod(String method) { + return OAuth2Constants.PKCE_METHOD_S256.equals(method); + } private boolean isValidPkceCodeChallenge(String codeChallenge) { if (codeChallenge.length() < OIDCLoginProtocol.PKCE_CODE_CHALLENGE_MIN_LENGTH) { @@ -138,6 +136,17 @@ public class TestPKCEEnforceExecutor extends AbstractAugumentingClientRegistrati return m.matches(); } + private void executeOnTokenRequest( + MultivaluedMap 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 { // check whether code verifier is specified if (codeVerifier == null) { @@ -146,7 +155,6 @@ public class TestPKCEEnforceExecutor extends AbstractAugumentingClientRegistrati verifyCodeVerifier(codeVerifier, codeChallenge, codeChallengeMethod); } - private void verifyCodeVerifier(String codeVerifier, String codeChallenge, String codeChallengeMethod) throws ClientPolicyException { // check whether code verifier is formatted along with the PKCE specification diff --git a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/services/clientpolicy/executor/TestPKCEEnforceExecutorFactory.java b/services/src/main/java/org/keycloak/services/clientpolicy/executor/PKCEEnforceExecutorFactory.java similarity index 81% rename from testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/services/clientpolicy/executor/TestPKCEEnforceExecutorFactory.java rename to services/src/main/java/org/keycloak/services/clientpolicy/executor/PKCEEnforceExecutorFactory.java index 6af332d4c8..6d6bc75279 100644 --- a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/services/clientpolicy/executor/TestPKCEEnforceExecutorFactory.java +++ b/services/src/main/java/org/keycloak/services/clientpolicy/executor/PKCEEnforceExecutorFactory.java @@ -15,7 +15,7 @@ * limitations under the License. */ -package org.keycloak.testsuite.services.clientpolicy.executor; +package org.keycloak.services.clientpolicy.executor; 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.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 public ClientPolicyExecutorProvider create(KeycloakSession session, ComponentModel model) { - return new TestPKCEEnforceExecutor(session, model); + return new PKCEEnforceExecutor(session, model); } @Override @@ -55,7 +55,7 @@ public class TestPKCEEnforceExecutorFactory extends AbstractAugumentingClientReg @Override public String getHelpText() { - return null; + return "It makes the client enforce Proof Key for Code Exchange operation."; } @Override diff --git a/services/src/main/resources/META-INF/services/org.keycloak.services.clientpolicy.executor.ClientPolicyExecutorProviderFactory b/services/src/main/resources/META-INF/services/org.keycloak.services.clientpolicy.executor.ClientPolicyExecutorProviderFactory index 8a76969799..3aa581ca37 100644 --- a/services/src/main/resources/META-INF/services/org.keycloak.services.clientpolicy.executor.ClientPolicyExecutorProviderFactory +++ b/services/src/main/resources/META-INF/services/org.keycloak.services.clientpolicy.executor.ClientPolicyExecutorProviderFactory @@ -1,3 +1,4 @@ org.keycloak.services.clientpolicy.executor.SecureResponseTypeExecutorFactory org.keycloak.services.clientpolicy.executor.SecureRequestObjectExecutorFactory -org.keycloak.services.clientpolicy.executor.SecureClientAuthEnforceExecutorFactory \ No newline at end of file +org.keycloak.services.clientpolicy.executor.SecureClientAuthEnforceExecutorFactory +org.keycloak.services.clientpolicy.executor.PKCEEnforceExecutorFactory diff --git a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/resources/META-INF/services/org.keycloak.services.clientpolicy.executor.ClientPolicyExecutorProviderFactory b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/resources/META-INF/services/org.keycloak.services.clientpolicy.executor.ClientPolicyExecutorProviderFactory deleted file mode 100644 index f58265e5c8..0000000000 --- a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/resources/META-INF/services/org.keycloak.services.clientpolicy.executor.ClientPolicyExecutorProviderFactory +++ /dev/null @@ -1 +0,0 @@ -org.keycloak.testsuite.services.clientpolicy.executor.TestPKCEEnforceExecutorFactory \ No newline at end of file diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientPolicyBasicsTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientPolicyBasicsTest.java index 1a9b87a3f7..42573e1793 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientPolicyBasicsTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientPolicyBasicsTest.java @@ -83,6 +83,7 @@ import org.keycloak.services.clientpolicy.condition.ClientUpdateContextCondition import org.keycloak.services.clientpolicy.condition.ClientRolesConditionFactory; import org.keycloak.services.clientpolicy.executor.ClientPolicyExecutorProvider; 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.SecureRequestObjectExecutorFactory; 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.rest.resource.TestingOIDCEndpointsApplicationResource.AuthorizationEndpointRequestObject; import org.keycloak.testsuite.services.clientpolicy.condition.TestRaiseExeptionConditionFactory; -import org.keycloak.testsuite.services.clientpolicy.executor.TestPKCEEnforceExecutorFactory; import org.keycloak.testsuite.util.OAuthClient; import com.fasterxml.jackson.databind.JsonNode; @@ -409,11 +409,11 @@ public class ClientPolicyBasicsTest extends AbstractKeycloakTest { createPolicy(policyName, DefaultClientPolicyProviderFactory.PROVIDER_ID, null, null, null); logger.info("... Created Policy : " + policyName); - createExecutor("TestPKCEEnforceExecutor", TestPKCEEnforceExecutorFactory.PROVIDER_ID, null, (ComponentRepresentation provider) -> { + createExecutor("PKCEEnforceExecutor", PKCEEnforceExecutorFactory.PROVIDER_ID, null, (ComponentRepresentation provider) -> { setExecutorAugmentActivate(provider); }); - registerExecutor("TestPKCEEnforceExecutor", policyName); - logger.info("... Registered Executor : TestPKCEEnforceExecutor"); + registerExecutor("PKCEEnforceExecutor", policyName); + logger.info("... Registered Executor : PKCEEnforceExecutor"); String clientId = "Zahlungs-App"; String clientSecret = "secret"; @@ -477,15 +477,15 @@ public class ClientPolicyBasicsTest extends AbstractKeycloakTest { try { successfulLoginAndLogout(clientId, clientSecret); - createExecutor("TestPKCEEnforceExecutor", TestPKCEEnforceExecutorFactory.PROVIDER_ID, null, (ComponentRepresentation provider) -> { + createExecutor("PKCEEnforceExecutor", PKCEEnforceExecutorFactory.PROVIDER_ID, null, (ComponentRepresentation provider) -> { setExecutorAugmentDeactivate(provider); }); - registerExecutor("TestPKCEEnforceExecutor", policyName); - logger.info("... Registered Executor : TestPKCEEnforceExecutor"); + registerExecutor("PKCEEnforceExecutor", policyName); + logger.info("... Registered Executor : PKCEEnforceExecutor"); failLoginByNotFollowingPKCE(clientId); - updateExecutor("TestPKCEEnforceExecutor", (ComponentRepresentation provider) -> { + updateExecutor("PKCEEnforceExecutor", (ComponentRepresentation provider) -> { setExecutorAugmentActivate(provider); }); @@ -495,8 +495,8 @@ public class ClientPolicyBasicsTest extends AbstractKeycloakTest { assertEquals(false, getClientByAdmin(cid).isServiceAccountsEnabled()); assertEquals(OAuth2Constants.PKCE_METHOD_S256, OIDCAdvancedConfigWrapper.fromClientRepresentation(getClientByAdmin(cid)).getPkceCodeChallengeMethod()); - deleteExecutor("TestPKCEEnforceExecutor", policyName); - logger.info("... Deleted Executor : TestPKCEEnforceExecutor"); + deleteExecutor("PKCEEnforceExecutor", policyName); + logger.info("... Deleted Executor : PKCEEnforceExecutor"); updateClientByAdmin(cid, (ClientRepresentation clientRep) -> { OIDCAdvancedConfigWrapper.fromClientRepresentation(clientRep).setPkceCodeChallengeMethod(null); @@ -547,11 +547,11 @@ public class ClientPolicyBasicsTest extends AbstractKeycloakTest { registerCondition(CLIENTROLES_CONDITION_BETA_NAME, policyBetaName); 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); }); - registerExecutor("TestPKCEEnforceExecutor-beta", policyBetaName); - logger.info("... Registered Executor : TestPKCEEnforceExecutor-beta"); + registerExecutor("PKCEEnforceExecutor-beta", policyBetaName); + logger.info("... Registered Executor : PKCEEnforceExecutor-beta"); String clientAlphaId = "Alpha-App"; String clientAlphaSecret = "secretAlpha"; @@ -872,11 +872,11 @@ public class ClientPolicyBasicsTest extends AbstractKeycloakTest { registerExecutor("SecureClientAuthEnforceExecutor", policyName); logger.info("... Registered Executor : SecureClientAuthEnforceExecutor"); - createExecutor("TestPKCEEnforceExecutor", TestPKCEEnforceExecutorFactory.PROVIDER_ID, null, (ComponentRepresentation provider) -> { + createExecutor("PKCEEnforceExecutor", PKCEEnforceExecutorFactory.PROVIDER_ID, null, (ComponentRepresentation provider) -> { setExecutorAugmentActivate(provider); }); - registerExecutor("TestPKCEEnforceExecutor", policyName); - logger.info("... Registered Executor : TestPKCEEnforceExecutor"); + registerExecutor("PKCEEnforceExecutor", policyName); + logger.info("... Registered Executor : PKCEEnforceExecutor"); } @@ -1155,5 +1155,4 @@ public class ClientPolicyBasicsTest extends AbstractKeycloakTest { private void setExecutorAugmentedClientAuthMethod(ComponentRepresentation provider, String augmentedClientAuthMethod) { provider.getConfig().putSingle(SecureClientAuthEnforceExecutorFactory.CLIENT_AUTHNS_AUGMENT, augmentedClientAuthMethod); } - }