KEYCLOAK-19539 FAPI 2.0 Baseline : Reject Implicit Grant

2021-10-13 12:28:14 +09:00 · 2021-10-13 12:28:14 +09:00 · 7f5e94db87
commit 7f5e94db87
parent 0eec2f4c7a
6 changed files with 263 additions and 0 deletions
--- a/docs/documentation/server_admin/topics/clients/client-policies.adoc
+++ b/docs/documentation/server_admin/topics/clients/client-policies.adoc
@ -125,6 +125,7 @@ One of several purposes for this executor is to realize the security requirement
 * Enforce <<_secret_rotation, Client Secret Rotation>>
 * Enforce Client Registration Access Token
 * Enforce checking if a client is the one to which an intent was issued in a use case where an intent is issued before starting an authorization code flow to get an access token like UK OpenBanking
+* Enforce prohibiting implicit and hybrid flow

 [[_client_policy_profile]]
 === Profile
--- a/services/src/main/java/org/keycloak/services/clientpolicy/executor/RejectImplicitGrantExecutor.java
+++ b/services/src/main/java/org/keycloak/services/clientpolicy/executor/RejectImplicitGrantExecutor.java
@ -0,0 +1,115 @@
+/*
+ * Copyright 2021 Red Hat, Inc. and/or its affiliates
+ * and other contributors as indicated by the @author tags.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.keycloak.services.clientpolicy.executor;
+
+import org.keycloak.OAuthErrorException;
+import org.keycloak.models.KeycloakSession;
+import org.keycloak.protocol.oidc.endpoints.request.AuthorizationEndpointRequest;
+import org.keycloak.protocol.oidc.utils.OIDCResponseType;
+import org.keycloak.representations.idm.ClientPolicyExecutorConfigurationRepresentation;
+import org.keycloak.representations.idm.ClientRepresentation;
+import org.keycloak.services.clientpolicy.ClientPolicyContext;
+import org.keycloak.services.clientpolicy.ClientPolicyException;
+import org.keycloak.services.clientpolicy.context.AuthorizationRequestContext;
+import org.keycloak.services.clientpolicy.context.ClientCRUDContext;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+
+/**
+ * @author <a href="mailto:takashi.norimatsu.ws@hitachi.com">Takashi Norimatsu</a>
+ */
+public class RejectImplicitGrantExecutor implements ClientPolicyExecutorProvider<RejectImplicitGrantExecutor.Configuration> {
+
+    private final KeycloakSession session;
+    private Configuration configuration;
+
+    public RejectImplicitGrantExecutor(KeycloakSession session) {
+        this.session = session;
+    }
+
+    @Override
+    public void setupConfiguration(Configuration config) {
+        this.configuration = config;
+    }
+
+    @Override
+    public Class<Configuration> getExecutorConfigurationClass() {
+        return Configuration.class;
+    }
+
+    public static class Configuration extends ClientPolicyExecutorConfigurationRepresentation {
+        @JsonProperty("auto-configure")
+        protected Boolean autoConfigure;
+
+        public Boolean isAutoConfigure() {
+            return autoConfigure;
+        }
+
+        public void setAutoConfigure(Boolean autoConfigure) {
+            this.autoConfigure = autoConfigure;
+        }
+    }
+
+    @Override
+    public String getProviderId() {
+        return RejectImplicitGrantExecutorFactory.PROVIDER_ID;
+    }
+
+    @Override
+    public void executeOnEvent(ClientPolicyContext context) throws ClientPolicyException {
+        switch (context.getEvent()) {
+            case REGISTER:
+            case UPDATE:
+                ClientCRUDContext clientUpdateContext = (ClientCRUDContext)context;
+                autoConfigure(clientUpdateContext.getProposedClientRepresentation());
+                validate(clientUpdateContext.getProposedClientRepresentation());
+                break;
+            case AUTHORIZATION_REQUEST:
+                AuthorizationRequestContext authorizationRequestContext = (AuthorizationRequestContext)context;
+                executeOnAuthorizationRequest(authorizationRequestContext.getparsedResponseType(),
+                    authorizationRequestContext.getAuthorizationEndpointRequest(),
+                    authorizationRequestContext.getRedirectUri());
+                return;
+            default:
+                return;
+        }
+    }
+
+    private void autoConfigure(ClientRepresentation rep) {
+        if (configuration.isAutoConfigure())
+            rep.setImplicitFlowEnabled(Boolean.FALSE);
+    }
+
+    private void validate(ClientRepresentation rep) throws ClientPolicyException {
+        boolean isImplicitFlowEnabled = rep.isImplicitFlowEnabled().booleanValue();
+        if (!isImplicitFlowEnabled) return;
+        throw new ClientPolicyException(OAuthErrorException.INVALID_CLIENT_METADATA, "Invalid client metadata: implicit flow enabled");
+    }
+
+    private void executeOnAuthorizationRequest(
+            OIDCResponseType parsedResponseType,
+            AuthorizationEndpointRequest request,
+            String redirectUri) throws ClientPolicyException {
+        // Before client policies operation, Authorization Endpoint logic has already checked whether implicit/hybrid flow is activated for a client.
+        // This method rejects implicit grant regardless of client setting for allowing implicit grant.
+        if (parsedResponseType.isImplicitOrHybridFlow()) {
+            throw new ClientPolicyException(OAuthErrorException.INVALID_GRANT, "Implicit/Hybrid flow is prohibited.");
+        }
+    }
+
+}
--- a/services/src/main/java/org/keycloak/services/clientpolicy/executor/RejectImplicitGrantExecutorFactory.java
+++ b/services/src/main/java/org/keycloak/services/clientpolicy/executor/RejectImplicitGrantExecutorFactory.java
@ -0,0 +1,72 @@
+/*
+ * Copyright 2021 Red Hat, Inc. and/or its affiliates
+ * and other contributors as indicated by the @author tags.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.keycloak.services.clientpolicy.executor;
+
+import java.util.Collections;
+import java.util.List;
+
+import org.keycloak.Config.Scope;
+import org.keycloak.models.KeycloakSession;
+import org.keycloak.models.KeycloakSessionFactory;
+import org.keycloak.provider.ProviderConfigProperty;
+
+/**
+ * @author <a href="mailto:takashi.norimatsu.ws@hitachi.com">Takashi Norimatsu</a>
+ */
+public class RejectImplicitGrantExecutorFactory implements ClientPolicyExecutorProviderFactory {
+
+    public static final String PROVIDER_ID = "reject-implicit-grant";
+
+    public static final String AUTO_CONFIGURE = "auto-configure";
+
+    private static final ProviderConfigProperty AUTO_CONFIGURE_PROPERTY = new ProviderConfigProperty(
+            AUTO_CONFIGURE, "Auto-configure", "If On, then the during client creation or update, the configuration of the client will be auto-configured to reject an implicit grant/hybrid flow.", ProviderConfigProperty.BOOLEAN_TYPE, false);
+
+    @Override
+    public ClientPolicyExecutorProvider create(KeycloakSession session) {
+        return new RejectImplicitGrantExecutor(session);
+    }
+
+    @Override
+    public void init(Scope config) {
+    }
+
+    @Override
+    public void postInit(KeycloakSessionFactory factory) {
+    }
+
+    @Override
+    public void close() {
+    }
+
+    @Override
+    public String getId() {
+        return PROVIDER_ID;
+    }
+
+    @Override
+    public String getHelpText() {
+        return "It makes keycloak to reject an implicit grant / hybrid flow.";
+    }
+
+    @Override
+    public List<ProviderConfigProperty> getConfigProperties() {
+        return Collections.singletonList(AUTO_CONFIGURE_PROPERTY);
+    }
+
+}
--- 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
@ -20,3 +20,4 @@ org.keycloak.services.clientpolicy.executor.RejectRequestExecutorFactory
 org.keycloak.services.clientpolicy.executor.IntentClientBindCheckExecutorFactory
 org.keycloak.services.clientpolicy.executor.SuppressRefreshTokenRotationExecutorFactory
 org.keycloak.services.clientpolicy.executor.RegistrationAccessTokenRotationDisabledExecutorFactory
+org.keycloak.services.clientpolicy.executor.RejectImplicitGrantExecutorFactory
--- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/policies/ClientPoliciesTest.java
+++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/policies/ClientPoliciesTest.java
@ -38,6 +38,7 @@ import static org.keycloak.testsuite.util.ClientPoliciesUtil.createHolderOfKeyEn
 import static org.keycloak.testsuite.util.ClientPoliciesUtil.createIntentClientBindCheckExecutorConfig;
 import static org.keycloak.testsuite.util.ClientPoliciesUtil.createPKCEEnforceExecutorConfig;
 import static org.keycloak.testsuite.util.ClientPoliciesUtil.createRejectisResourceOwnerPasswordCredentialsGrantExecutorConfig;
+import static org.keycloak.testsuite.util.ClientPoliciesUtil.createRejectImplicitGrantExecutorConfig;
 import static org.keycloak.testsuite.util.ClientPoliciesUtil.createSecureClientAuthenticatorExecutorConfig;
 import static org.keycloak.testsuite.util.ClientPoliciesUtil.createSecureSigningAlgorithmForSignedJwtEnforceExecutorConfig;
 import static org.keycloak.testsuite.util.ClientPoliciesUtil.createTestRaiseExeptionConditionConfig;
@ -103,6 +104,7 @@ import org.keycloak.services.clientpolicy.executor.FullScopeDisabledExecutorFact
 import org.keycloak.services.clientpolicy.executor.HolderOfKeyEnforcerExecutorFactory;
 import org.keycloak.services.clientpolicy.executor.IntentClientBindCheckExecutorFactory;
 import org.keycloak.services.clientpolicy.executor.PKCEEnforcerExecutorFactory;
+import org.keycloak.services.clientpolicy.executor.RejectImplicitGrantExecutorFactory;
 import org.keycloak.services.clientpolicy.executor.RejectRequestExecutorFactory;
 import org.keycloak.services.clientpolicy.executor.RejectResourceOwnerPasswordCredentialsGrantExecutorFactory;
 import org.keycloak.services.clientpolicy.executor.SecureClientAuthenticatorExecutorFactory;
@ -1156,4 +1158,69 @@ public class ClientPoliciesTest extends AbstractClientPoliciesTest {
        assertEquals(OAuthErrorException.INVALID_REQUEST, oauth.getCurrentFragment().get(OAuth2Constants.ERROR));
        assertEquals("no claim for an intent value for ID token" , oauth.getCurrentFragment().get(OAuth2Constants.ERROR_DESCRIPTION));
    }
+
+    @Test
+    public void testRejectImplicitGrantExecutor() throws Exception {
+
+        String clientId = generateSuffixedName(CLIENT_NAME);
+        String clientSecret = "secret";
+
+        createClientByAdmin(clientId, (ClientRepresentation clientRep) -> {
+            clientRep.setSecret(clientSecret);
+            clientRep.setStandardFlowEnabled(Boolean.TRUE);
+            clientRep.setImplicitFlowEnabled(Boolean.TRUE);
+            clientRep.setPublicClient(Boolean.FALSE);
+        });
+
+        // register profiles
+        String json = (new ClientProfilesBuilder()).addProfile(
+                (new ClientProfileBuilder()).createProfile(PROFILE_NAME, "Az Elso Profil")
+                    .addExecutor(RejectImplicitGrantExecutorFactory.PROVIDER_ID,
+                        createRejectImplicitGrantExecutorConfig(Boolean.TRUE))
+                    .toRepresentation()
+                ).toString();
+        updateProfiles(json);
+
+        // register policies
+        json = (new ClientPoliciesBuilder()).addPolicy(
+                (new ClientPolicyBuilder()).createPolicy(POLICY_NAME, "Az Elso Politika", Boolean.TRUE)
+                    .addCondition(AnyClientConditionFactory.PROVIDER_ID, 
+                        createAnyClientConditionConfig())
+                    .addProfile(PROFILE_NAME)
+                    .toRepresentation()
+                ).toString();
+        updatePolicies(json);
+
+        try {
+            String expectedErrorDescription = "Implicit/Hybrid flow is prohibited.";
+            oauth.clientId(clientId);
+
+            // implicit grant
+            testProhibitedImplicitOrHybridFlow(false, OIDCResponseType.TOKEN, null, OAuthErrorException.INVALID_GRANT, expectedErrorDescription);
+
+            // hybrid grant
+            testProhibitedImplicitOrHybridFlow(true, OIDCResponseType.TOKEN + " " + OIDCResponseType.ID_TOKEN, "exsefweag", OAuthErrorException.INVALID_GRANT, expectedErrorDescription);
+
+            // hybrid grant
+            testProhibitedImplicitOrHybridFlow(true, OIDCResponseType.TOKEN + " " + OIDCResponseType.CODE, "exsefweag", OAuthErrorException.INVALID_GRANT, expectedErrorDescription);
+
+            // hybrid grant
+            testProhibitedImplicitOrHybridFlow(true, OIDCResponseType.TOKEN + " " + OIDCResponseType.CODE + " " + OIDCResponseType.ID_TOKEN, "exsefweag", OAuthErrorException.INVALID_GRANT, expectedErrorDescription);
+
+        } finally {
+            // revert test client instance settings the same as OAuthClient.init
+            oauth.openid(true);
+            oauth.responseType(OIDCResponseType.CODE);
+            oauth.nonce(null);
+        }
+    }
+
+    private void testProhibitedImplicitOrHybridFlow(boolean isOpenid, String responseType, String nonce, String expectedError, String expectedErrorDescription) {
+        oauth.openid(isOpenid);
+        oauth.responseType(responseType);
+        oauth.nonce(nonce);
+        oauth.openLoginForm();
+        assertEquals(expectedError, oauth.getCurrentFragment().get(OAuth2Constants.ERROR));
+        assertEquals(expectedErrorDescription, oauth.getCurrentFragment().get(OAuth2Constants.ERROR_DESCRIPTION));
+    }
 }
--- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/util/ClientPoliciesUtil.java
+++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/util/ClientPoliciesUtil.java
@ -44,6 +44,7 @@ import org.keycloak.services.clientpolicy.executor.HolderOfKeyEnforcerExecutor;
 import org.keycloak.services.clientpolicy.executor.IntentClientBindCheckExecutor;
 import org.keycloak.services.clientpolicy.executor.PKCEEnforcerExecutor;
 import org.keycloak.services.clientpolicy.executor.RejectResourceOwnerPasswordCredentialsGrantExecutor;
+import org.keycloak.services.clientpolicy.executor.RejectImplicitGrantExecutor;
 import org.keycloak.services.clientpolicy.executor.SecureClientAuthenticatorExecutor;
 import org.keycloak.services.clientpolicy.executor.SecureRequestObjectExecutor;
 import org.keycloak.services.clientpolicy.executor.SecureResponseTypeExecutor;
@ -221,6 +222,12 @@ public final class ClientPoliciesUtil {
        return config;
    }

+    public static RejectImplicitGrantExecutor.Configuration createRejectImplicitGrantExecutorConfig(Boolean autoConfigure) {
+        RejectImplicitGrantExecutor.Configuration config = new RejectImplicitGrantExecutor.Configuration();
+        config.setAutoConfigure(autoConfigure);
+        return config;
+    }
+
    public static IntentClientBindCheckExecutor.Configuration createIntentClientBindCheckExecutorConfig(String intentName, String endpoint) {
        IntentClientBindCheckExecutor.Configuration config = new IntentClientBindCheckExecutor.Configuration();
        config.setIntentName(intentName);