KEYCLOAK-18139 SecureResponseTypeExecutor: polishing for FAPI 1 final

services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureResponseTypeExecutor.java

@@ -17,29 +17,73 @@
 
 package org.keycloak.services.clientpolicy.executor;
 
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+
 import org.jboss.logging.Logger;
 import org.keycloak.OAuthErrorException;
 import org.keycloak.models.KeycloakSession;
+import org.keycloak.protocol.oidc.OIDCConfigAttributes;
 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 SecureResponseTypeExecutor implements ClientPolicyExecutorProvider<ClientPolicyExecutorConfigurationRepresentation> {
+public class SecureResponseTypeExecutor implements ClientPolicyExecutorProvider<SecureResponseTypeExecutor.Configuration> {
 
     private static final Logger logger = Logger.getLogger(SecureResponseTypeExecutor.class);
 
     protected final KeycloakSession session;
+    private Configuration configuration;
 
     public SecureResponseTypeExecutor(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;
+
+        @JsonProperty("allow-token-response-type")
+        protected Boolean allowTokenResponseType;
+
+        public Boolean isAutoConfigure() {
+            return autoConfigure;
+        }
+
+        public void setAutoConfigure(Boolean autoConfigure) {
+            this.autoConfigure = autoConfigure;
+        }
+
+        public Boolean isAllowTokenResponseType() {
+            return allowTokenResponseType;
+        }
+
+        public void setAllowTokenResponseType(Boolean allowTokenResponseType) {
+            this.allowTokenResponseType = allowTokenResponseType;
+        }
+    }
+
     @Override
     public String getProviderId() {
         return SecureResponseTypeExecutorFactory.PROVIDER_ID;
@@ -48,6 +92,12 @@ public class SecureResponseTypeExecutor implements ClientPolicyExecutorProvider<
     @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(),
@@ -66,17 +116,51 @@ public class SecureResponseTypeExecutor implements ClientPolicyExecutorProvider<
             String redirectUri) throws ClientPolicyException {
         logger.trace("Authz Endpoint - authz request");
 
-        if (parsedResponseType.hasResponseType(OIDCResponseType.CODE) && parsedResponseType.hasResponseType(OIDCResponseType.ID_TOKEN)) {
+        if (isHybridFlow(parsedResponseType)) {
             if (parsedResponseType.hasResponseType(OIDCResponseType.TOKEN)) {
-                logger.trace("Passed. response_type = code id_token token");
+                if (isAllowTokenResponseType()) {
+                    logger.trace("Passed. response_type = code id_token token");
+                    return;
+                }
             } else {
                 logger.trace("Passed. response_type = code id_token");
+                return;
             }
-            return;
         }
 
         logger.tracev("invalid response_type = {0}", parsedResponseType);
         throw new ClientPolicyException(OAuthErrorException.INVALID_REQUEST, "invalid response_type");
     }
 
+    private boolean isHybridFlow(OIDCResponseType parsedResponseType) {
+        return parsedResponseType.hasResponseType(OIDCResponseType.CODE) && parsedResponseType.hasResponseType(OIDCResponseType.ID_TOKEN);
+    }
+
+    private boolean isAllowTokenResponseType() {
+        return configuration != null && Optional.ofNullable(configuration.isAllowTokenResponseType()).orElse(Boolean.FALSE).booleanValue();
+    }
+
+    private void autoConfigure(ClientRepresentation rep) {
+        if (isAutoConfigure()) {
+            Map<String, String> attributes = Optional.ofNullable(rep.getAttributes()).orElse(new HashMap<>());
+            attributes.put(OIDCConfigAttributes.ID_TOKEN_AS_DETACHED_SIGNATURE, Boolean.TRUE.toString());
+            rep.setAttributes(attributes);
+        }
+    }
+
+    private boolean isAutoConfigure() {
+        return configuration != null && Optional.ofNullable(configuration.isAutoConfigure()).orElse(Boolean.FALSE).booleanValue();
+    }
+
+    private void validate(ClientRepresentation rep) throws ClientPolicyException {
+        if (!isIdTokenAsDetachedSignature(rep)) {
+            throw new ClientPolicyException(OAuthErrorException.INVALID_CLIENT_METADATA, "Invalid client metadata: ID Token as detached signature in disabled");
+        }
+    }
+
+    private boolean isIdTokenAsDetachedSignature(ClientRepresentation rep) {
+        if (rep.getAttributes() == null) return false;
+        return Boolean.valueOf(Optional.ofNullable(rep.getAttributes().get(OIDCConfigAttributes.ID_TOKEN_AS_DETACHED_SIGNATURE)).orElse(Boolean.FALSE.toString()));
+    }
+
 }

services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureResponseTypeExecutorFactory.java

@@ -17,7 +17,8 @@
 
 package org.keycloak.services.clientpolicy.executor;
 
-import java.util.Collections;
+import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 
 import org.keycloak.Config.Scope;
@@ -32,6 +33,14 @@ public class SecureResponseTypeExecutorFactory implements ClientPolicyExecutorPr
 
     public static final String PROVIDER_ID = "secure-response-type";
 
+    public static final String AUTO_CONFIGURE = "auto-configure";
+    public static final String ALLOW_TOKEN_RESPONSE_TYPE = "allow-token-response-type";
+
+    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 use ID token returned from authorization endpoint as detached signature.", ProviderConfigProperty.BOOLEAN_TYPE, false);
+    private static final ProviderConfigProperty ALLOW_TOKEN_RESPONSE_TYPE_PROPERTY = new ProviderConfigProperty(
+            ALLOW_TOKEN_RESPONSE_TYPE, "Allow-token-response-type", "If On, then it allows an access token returned from authorization endpoint in hybrid flow.", ProviderConfigProperty.BOOLEAN_TYPE, false);
+
     @Override
     public ClientPolicyExecutorProvider create(KeycloakSession session) {
         return new SecureResponseTypeExecutor(session);
@@ -56,12 +65,12 @@ public class SecureResponseTypeExecutorFactory implements ClientPolicyExecutorPr
 
     @Override
     public String getHelpText() {
-        return "The executor checks whether the client sent its authorization request with code id_token or code id_token token in its response type by following Financial-grade API Security Profile : Read and Write API Security Profile.";
+        return "The executor checks whether the client sent its authorization request with code id_token or code id_token token in its response type depending on its setting.";
     }
 
     @Override
     public List<ProviderConfigProperty> getConfigProperties() {
-        return Collections.emptyList();
+        return new ArrayList<>(Arrays.asList(AUTO_CONFIGURE_PROPERTY, ALLOW_TOKEN_RESPONSE_TYPE_PROPERTY));
     }
 
 }

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/AbstractClientPoliciesTest.java

@@ -133,6 +133,7 @@ import org.keycloak.services.clientpolicy.executor.SecureClientAuthenticatorExec
 import org.keycloak.services.clientpolicy.executor.SecureClientUrisExecutorFactory;
 import org.keycloak.services.clientpolicy.executor.SecureRequestObjectExecutor;
 import org.keycloak.services.clientpolicy.executor.SecureRequestObjectExecutorFactory;
+import org.keycloak.services.clientpolicy.executor.SecureResponseTypeExecutor;
 import org.keycloak.services.clientpolicy.executor.SecureResponseTypeExecutorFactory;
 import org.keycloak.services.clientpolicy.executor.SecureSessionEnforceExecutorFactory;
 import org.keycloak.services.clientpolicy.executor.SecureSigningAlgorithmExecutor;
@@ -867,6 +868,13 @@ public abstract class AbstractClientPoliciesTest extends AbstractKeycloakTest {
         return config;
     }
 
+    protected SecureResponseTypeExecutor.Configuration createSecureResponseTypeExecutor(Boolean autoConfigure, Boolean allowTokenResponseType) {
+        SecureResponseTypeExecutor.Configuration config = new SecureResponseTypeExecutor.Configuration();
+        if (autoConfigure != null) config.setAutoConfigure(autoConfigure);
+        if (allowTokenResponseType != null) config.setAllowTokenResponseType(allowTokenResponseType);
+        return config;
+    }
+
     protected SecureSigningAlgorithmForSignedJwtExecutor.Configuration createSecureSigningAlgorithmForSignedJwtEnforceExecutorConfig(Boolean requireClientAssertion) {
         SecureSigningAlgorithmForSignedJwtExecutor.Configuration config = new SecureSigningAlgorithmForSignedJwtExecutor.Configuration();
         config.setRequireClientAssertion(requireClientAssertion);

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientPoliciesTest.java

@@ -65,6 +65,7 @@ import org.keycloak.protocol.oidc.OIDCConfigAttributes;
 import org.keycloak.protocol.oidc.OIDCLoginProtocol;
 import org.keycloak.protocol.oidc.utils.OIDCResponseType;
 import org.keycloak.representations.AccessToken;
+import org.keycloak.representations.IDToken;
 import org.keycloak.representations.RefreshToken;
 import org.keycloak.representations.idm.ClientRepresentation;
 import org.keycloak.representations.idm.CredentialRepresentation;
@@ -978,8 +979,8 @@ public class ClientPoliciesTest extends AbstractClientPoliciesTest {
         assertEquals(OAuthErrorException.INVALID_REQUEST, oauth.getCurrentQuery().get(OAuth2Constants.ERROR));
         assertEquals("invalid response_type", oauth.getCurrentQuery().get(OAuth2Constants.ERROR_DESCRIPTION));
 
-        oauth.responseType(OIDCResponseType.CODE + " " + OIDCResponseType.ID_TOKEN + " "  + OIDCResponseType.TOKEN);
+        oauth.responseType(OIDCResponseType.CODE + " " + OIDCResponseType.ID_TOKEN);
-        oauth.nonce("cie8cjcwiw");
+        oauth.nonce("vbwe566fsfffds");
         oauth.doLogin(TEST_USER_NAME, TEST_USER_PASSWORD);
 
         EventRepresentation loginEvent = events.expectLogin().client(clientId).assertEvent();
@@ -993,8 +994,16 @@ public class ClientPoliciesTest extends AbstractClientPoliciesTest {
         oauth.doLogout(res.getRefreshToken(), clientSecret);
         events.expectLogout(sessionId).client(clientId).clearDetails().assertEvent();
 
-        oauth.responseType(OIDCResponseType.CODE + " " + OIDCResponseType.ID_TOKEN);
+        // update profiles
-        oauth.nonce("vbwe566fsfffds");
+        json = (new ClientProfilesBuilder()).addProfile(
+                (new ClientProfileBuilder()).createProfile(PROFILE_NAME, "O Primeiro Perfil")
+                    .addExecutor(SecureResponseTypeExecutorFactory.PROVIDER_ID, createSecureResponseTypeExecutor(Boolean.FALSE, Boolean.TRUE))
+                    .toRepresentation()
+                ).toString();
+        updateProfiles(json);
+
+        oauth.responseType(OIDCResponseType.CODE + " " + OIDCResponseType.ID_TOKEN + " "  + OIDCResponseType.TOKEN); // token response type allowed
+        oauth.nonce("cie8cjcwiw");
         oauth.doLogin(TEST_USER_NAME, TEST_USER_PASSWORD);
 
         loginEvent = events.expectLogin().client(clientId).assertEvent();
@@ -1009,6 +1018,101 @@ public class ClientPoliciesTest extends AbstractClientPoliciesTest {
         events.expectLogout(sessionId).client(clientId).clearDetails().assertEvent();
     }
 
+    @Test
+    public void testSecureResponseTypeExecutorAllowTokenResponseType() throws Exception {
+        // register profiles
+        String json = (new ClientProfilesBuilder()).addProfile(
+                (new ClientProfileBuilder()).createProfile(PROFILE_NAME, "O Primeiro Perfil")
+                    .addExecutor(SecureResponseTypeExecutorFactory.PROVIDER_ID, createSecureResponseTypeExecutor(null, Boolean.TRUE))
+                    .toRepresentation()
+                ).toString();
+        updateProfiles(json);
+
+        // register policies
+        json = (new ClientPoliciesBuilder()).addPolicy(
+                (new ClientPolicyBuilder()).createPolicy(POLICY_NAME, "Den Forsta Policyn", Boolean.TRUE)
+                    .addCondition(ClientUpdaterContextConditionFactory.PROVIDER_ID,
+                        createClientUpdateContextConditionConfig(Arrays.asList(
+                                ClientUpdaterContextConditionFactory.BY_AUTHENTICATED_USER,
+                                ClientUpdaterContextConditionFactory.BY_INITIAL_ACCESS_TOKEN,
+                                ClientUpdaterContextConditionFactory.BY_REGISTRATION_ACCESS_TOKEN)))
+                    .addCondition(ClientRolesConditionFactory.PROVIDER_ID, 
+                            createClientRolesConditionConfig(Arrays.asList(SAMPLE_CLIENT_ROLE)))
+                    .addProfile(PROFILE_NAME)
+                    .toRepresentation()
+                ).toString();
+        updatePolicies(json);
+
+        // create by Admin REST API
+        try {
+            createClientByAdmin(generateSuffixedName("App-by-Admin"), (ClientRepresentation clientRep) -> {
+                    clientRep.setSecret("secret");
+            });
+            fail();
+        } catch (ClientPolicyException e) {
+            assertEquals(OAuthErrorException.INVALID_CLIENT_METADATA, e.getMessage());
+        }
+
+        // update profiles
+        json = (new ClientProfilesBuilder()).addProfile(
+                (new ClientProfileBuilder()).createProfile(PROFILE_NAME, "O Primeiro Perfil")
+                    .addExecutor(SecureResponseTypeExecutorFactory.PROVIDER_ID, createSecureResponseTypeExecutor(Boolean.TRUE, null))
+                    .toRepresentation()
+                ).toString();
+        updateProfiles(json);
+
+        String cId = null;
+        String clientId = generateSuffixedName(CLIENT_NAME);
+        String clientSecret = "secret";
+        try {
+            cId = createClientByAdmin(clientId, (ClientRepresentation clientRep) -> {
+                clientRep.setSecret(clientSecret);
+                clientRep.setStandardFlowEnabled(Boolean.TRUE);
+                clientRep.setImplicitFlowEnabled(Boolean.TRUE);
+                clientRep.setPublicClient(Boolean.FALSE);
+            });
+        } catch (ClientPolicyException e) {
+            fail();
+        }
+        ClientRepresentation cRep = getClientByAdmin(cId);
+        assertEquals(Boolean.TRUE.toString(), cRep.getAttributes().get(OIDCConfigAttributes.ID_TOKEN_AS_DETACHED_SIGNATURE));
+
+        adminClient.realm(REALM_NAME).clients().get(cId).roles().create(RoleBuilder.create().name(SAMPLE_CLIENT_ROLE).build());
+
+        oauth.clientId(clientId);
+        oauth.openLoginForm();
+        assertEquals(OAuthErrorException.INVALID_REQUEST, oauth.getCurrentQuery().get(OAuth2Constants.ERROR));
+        assertEquals("invalid response_type", oauth.getCurrentQuery().get(OAuth2Constants.ERROR_DESCRIPTION));
+
+        oauth.responseType(OIDCResponseType.CODE + " " + OIDCResponseType.ID_TOKEN);
+        oauth.nonce("LIVieviDie028f");
+        oauth.doLogin(TEST_USER_NAME, TEST_USER_PASSWORD);
+
+        EventRepresentation loginEvent = events.expectLogin().client(clientId).assertEvent();
+        String sessionId = loginEvent.getSessionId();
+        String codeId = loginEvent.getDetails().get(Details.CODE_ID);
+        String code = new OAuthClient.AuthorizationEndpointResponse(oauth).getCode();
+
+        IDToken idToken = oauth.verifyIDToken(new OAuthClient.AuthorizationEndpointResponse(oauth).getIdToken());
+        // confirm ID token as detached signature does not include authenticated user's claims
+        Assert.assertNull(idToken.getEmailVerified());
+        Assert.assertNull(idToken.getName());
+        Assert.assertNull(idToken.getPreferredUsername());
+        Assert.assertNull(idToken.getGivenName());
+        Assert.assertNull(idToken.getFamilyName());
+        Assert.assertNull(idToken.getEmail());
+        assertEquals("LIVieviDie028f", idToken.getNonce());
+        // confirm an access token not returned
+        Assert.assertNull(new OAuthClient.AuthorizationEndpointResponse(oauth).getAccessToken());
+
+        OAuthClient.AccessTokenResponse res = oauth.doAccessTokenRequest(code, clientSecret);
+        assertEquals(200, res.getStatusCode());
+        events.expectCodeToToken(codeId, sessionId).client(clientId).assertEvent();
+
+        oauth.doLogout(res.getRefreshToken(), clientSecret);
+        events.expectLogout(sessionId).client(clientId).clearDetails().assertEvent();
+    }
+
     @Test
     public void testSecureRequestObjectExecutor() throws Exception, URISyntaxException, IOException {
         Integer availablePeriod = Integer.valueOf(SecureRequestObjectExecutor.DEFAULT_AVAILABLE_PERIOD + 400);