diff --git a/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureResponseTypeExecutor.java b/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureResponseTypeExecutor.java index e161cfcfc5..0350ce83f5 100644 --- a/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureResponseTypeExecutor.java +++ b/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 Takashi Norimatsu */ -public class SecureResponseTypeExecutor implements ClientPolicyExecutorProvider { +public class SecureResponseTypeExecutor implements ClientPolicyExecutorProvider { 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 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 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())); + } + } diff --git a/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureResponseTypeExecutorFactory.java b/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureResponseTypeExecutorFactory.java index 7d05e07ca9..14125cff8e 100644 --- a/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureResponseTypeExecutorFactory.java +++ b/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 getConfigProperties() { - return Collections.emptyList(); + return new ArrayList<>(Arrays.asList(AUTO_CONFIGURE_PROPERTY, ALLOW_TOKEN_RESPONSE_TYPE_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 f43182c4dd..1e56b066de 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 @@ -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); 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 a0ac1d5f37..19e9a8c2f1 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 @@ -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.nonce("cie8cjcwiw"); + oauth.responseType(OIDCResponseType.CODE + " " + OIDCResponseType.ID_TOKEN); + 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); - oauth.nonce("vbwe566fsfffds"); + // update profiles + 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);