From 0a4fdc64f309d294b1bbcc6d1ede1eaacb657a43 Mon Sep 17 00:00:00 2001 From: Takashi Norimatsu Date: Tue, 4 May 2021 19:57:33 +0900 Subject: [PATCH] KEYCLOAK-17929 SecureSigningAlgorithmForSignedJwtEnforceExecutor polishing for FAPI --- ...gAlgorithmForSignedJwtEnforceExecutor.java | 41 ++++++++++++++++--- ...thmForSignedJwtEnforceExecutorFactory.java | 9 +++- .../client/AbstractClientPoliciesTest.java | 7 ++++ .../testsuite/client/ClientPoliciesTest.java | 5 ++- 4 files changed, 54 insertions(+), 8 deletions(-) diff --git a/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureSigningAlgorithmForSignedJwtEnforceExecutor.java b/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureSigningAlgorithmForSignedJwtEnforceExecutor.java index f94acf1828..00b9241c00 100644 --- a/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureSigningAlgorithmForSignedJwtEnforceExecutor.java +++ b/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureSigningAlgorithmForSignedJwtEnforceExecutor.java @@ -17,6 +17,9 @@ package org.keycloak.services.clientpolicy.executor; +import java.util.List; +import java.util.Optional; + import org.jboss.logging.Logger; import org.jboss.resteasy.spi.HttpRequest; import org.keycloak.OAuth2Constants; @@ -27,22 +30,51 @@ import org.keycloak.jose.jws.JWSInputException; import org.keycloak.models.KeycloakSession; import org.keycloak.services.clientpolicy.ClientPolicyContext; import org.keycloak.services.clientpolicy.ClientPolicyException; +import org.keycloak.services.clientpolicy.executor.SecureClientAuthEnforceExecutor.Configuration; -public class SecureSigningAlgorithmForSignedJwtEnforceExecutor implements ClientPolicyExecutorProvider { +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.annotation.JsonProperty; + +public class SecureSigningAlgorithmForSignedJwtEnforceExecutor implements ClientPolicyExecutorProvider { private static final Logger logger = Logger.getLogger(SecureSigningAlgorithmForSignedJwtEnforceExecutor.class); private final KeycloakSession session; + private Configuration configuration; public SecureSigningAlgorithmForSignedJwtEnforceExecutor(KeycloakSession session) { this.session = session; } + @Override + public void setupConfiguration(SecureSigningAlgorithmForSignedJwtEnforceExecutor.Configuration config) { + this.configuration = config; + } + + @Override + public Class getExecutorConfigurationClass() { + return Configuration.class; + } + @Override public String getProviderId() { return SecureSigningAlgorithmForSignedJwtEnforceExecutorFactory.PROVIDER_ID; } + @JsonIgnoreProperties(ignoreUnknown = true) + public static class Configuration extends ClientPolicyExecutorConfiguration { + @JsonProperty("require-client-assertion") + protected Boolean requireClientAssertion; + + public Boolean isRequireClientAssertion() { + return requireClientAssertion; + } + + public void setRequireClientAssertion(Boolean augment) { + this.requireClientAssertion = augment; + } + } + @Override public void executeOnEvent(ClientPolicyContext context) throws ClientPolicyException { switch (context.getEvent()) { @@ -51,6 +83,8 @@ public class SecureSigningAlgorithmForSignedJwtEnforceExecutor implements Client case TOKEN_REVOKE: case TOKEN_INTROSPECT: case LOGOUT_REQUEST: + boolean isRequireClientAssertion = Optional.ofNullable(configuration.isRequireClientAssertion()).orElse(Boolean.FALSE).booleanValue(); + if (!isRequireClientAssertion) break; HttpRequest req = session.getContext().getContextObject(HttpRequest.class); String clientAssertion = req.getDecodedFormParameters().getFirst(OAuth2Constants.CLIENT_ASSERTION); JWSInput jws = null; @@ -68,11 +102,6 @@ public class SecureSigningAlgorithmForSignedJwtEnforceExecutor implements Client } private void verifySecureSigningAlgorithm(String signatureAlgorithm) throws ClientPolicyException { - if (signatureAlgorithm == null) { - logger.trace("Signing algorithm not specified explicitly."); - return; - } - // Please change also SecureSigningAlgorithmForSignedJwtEnforceExecutorFactory.getHelpText() if you are changing any algorithms here. switch (signatureAlgorithm) { case Algorithm.PS256: diff --git a/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureSigningAlgorithmForSignedJwtEnforceExecutorFactory.java b/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureSigningAlgorithmForSignedJwtEnforceExecutorFactory.java index ce29da5130..6041595ab6 100644 --- a/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureSigningAlgorithmForSignedJwtEnforceExecutorFactory.java +++ b/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureSigningAlgorithmForSignedJwtEnforceExecutorFactory.java @@ -22,6 +22,8 @@ import org.keycloak.models.KeycloakSession; import org.keycloak.models.KeycloakSessionFactory; import org.keycloak.provider.ProviderConfigProperty; +import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -29,6 +31,11 @@ public class SecureSigningAlgorithmForSignedJwtEnforceExecutorFactory implements public static final String PROVIDER_ID = "securesignalgjwt-enforce-executor"; + public static final String REQUIRE_CLIENT_ASSERTION = "require-client-assertion"; + + private static final ProviderConfigProperty REQUIRE_CLIENT_ASSERTION_PROPERTY = new ProviderConfigProperty( + REQUIRE_CLIENT_ASSERTION, null, null, ProviderConfigProperty.BOOLEAN_TYPE, false); + @Override public ClientPolicyExecutorProvider create(KeycloakSession session) { return new SecureSigningAlgorithmForSignedJwtEnforceExecutor(session); @@ -58,7 +65,7 @@ public class SecureSigningAlgorithmForSignedJwtEnforceExecutorFactory implements @Override public List getConfigProperties() { - return Collections.emptyList(); + return new ArrayList<>(Arrays.asList(REQUIRE_CLIENT_ASSERTION_PROPERTY)); } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/AbstractClientPoliciesTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/AbstractClientPoliciesTest.java index b199b96a37..4e64c636f9 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/AbstractClientPoliciesTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/AbstractClientPoliciesTest.java @@ -128,6 +128,7 @@ import org.keycloak.services.clientpolicy.executor.SecureRequestObjectExecutorFa import org.keycloak.services.clientpolicy.executor.SecureResponseTypeExecutorFactory; import org.keycloak.services.clientpolicy.executor.SecureSessionEnforceExecutorFactory; import org.keycloak.services.clientpolicy.executor.SecureSigningAlgorithmEnforceExecutorFactory; +import org.keycloak.services.clientpolicy.executor.SecureSigningAlgorithmForSignedJwtEnforceExecutor; import org.keycloak.services.clientpolicy.executor.SecureSigningAlgorithmForSignedJwtEnforceExecutorFactory; import org.keycloak.testsuite.AbstractKeycloakTest; import org.keycloak.testsuite.AssertEvents; @@ -866,6 +867,12 @@ public abstract class AbstractClientPoliciesTest extends AbstractKeycloakTest { return config; } + protected Object createSecureSigningAlgorithmForSignedJwtEnforceExecutorConfig(Boolean requireClientAssertion) { + SecureSigningAlgorithmForSignedJwtEnforceExecutor.Configuration config = new SecureSigningAlgorithmForSignedJwtEnforceExecutor.Configuration(); + config.setRequireClientAssertion(requireClientAssertion); + return config; + } + // Client Policies CRUD Operation protected static class ClientPoliciesBuilder { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientPoliciesTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientPoliciesTest.java index d8c675b415..9b3c5165dd 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientPoliciesTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientPoliciesTest.java @@ -91,6 +91,7 @@ import org.keycloak.services.clientpolicy.executor.SecureRequestObjectExecutorFa import org.keycloak.services.clientpolicy.executor.SecureResponseTypeExecutorFactory; import org.keycloak.services.clientpolicy.executor.SecureSessionEnforceExecutorFactory; import org.keycloak.services.clientpolicy.executor.SecureSigningAlgorithmEnforceExecutorFactory; +import org.keycloak.services.clientpolicy.executor.SecureSigningAlgorithmForSignedJwtEnforceExecutor; import org.keycloak.services.clientpolicy.executor.SecureSigningAlgorithmForSignedJwtEnforceExecutorFactory; import org.keycloak.services.clientpolicy.executor.SecureRequestObjectExecutor; import org.keycloak.testsuite.admin.ApiUtil; @@ -1255,7 +1256,7 @@ public class ClientPoliciesTest extends AbstractClientPoliciesTest { // register profiles String json = (new ClientProfilesBuilder()).addProfile( (new ClientProfileBuilder()).createProfile(PROFILE_NAME, "Ensimmainen Profiili", Boolean.FALSE, null) - .addExecutor(SecureSigningAlgorithmForSignedJwtEnforceExecutorFactory.PROVIDER_ID, null) + .addExecutor(SecureSigningAlgorithmForSignedJwtEnforceExecutorFactory.PROVIDER_ID, createSecureSigningAlgorithmForSignedJwtEnforceExecutorConfig(Boolean.TRUE)) .toRepresentation() ).toString(); updateProfiles(json); @@ -1350,6 +1351,8 @@ public class ClientPoliciesTest extends AbstractClientPoliciesTest { (new ClientProfileBuilder()).createProfile(PROFILE_NAME, "Az Elso Profil", Boolean.FALSE, null) .addExecutor(HolderOfKeyEnforceExecutorFactory.PROVIDER_ID, createHolderOfKeyEnforceExecutorConfig(Boolean.TRUE)) + .addExecutor(SecureSigningAlgorithmForSignedJwtEnforceExecutorFactory.PROVIDER_ID, + createSecureSigningAlgorithmForSignedJwtEnforceExecutorConfig(Boolean.FALSE)) .toRepresentation() ).toString(); updateProfiles(json);