From 82af0b6af606ef4ca8e83a84a3dadca911bddfe4 Mon Sep 17 00:00:00 2001 From: rmartinc Date: Tue, 30 Jan 2024 12:18:24 +0100 Subject: [PATCH] Initial client policies integration for SAML Closes #26654 Signed-off-by: rmartinc --- .../clientpolicy/ClientPolicyEvent.java | 4 +- .../keycloak/protocol/saml/SamlService.java | 22 + .../condition/ClientProtocolCondition.java | 123 +++++ .../ClientProtocolConditionFactory.java | 84 ++++ .../condition/ClientRolesCondition.java | 2 + .../ClientUpdaterContextCondition.java | 2 + .../ClientUpdaterSourceGroupsCondition.java | 14 +- .../ClientUpdaterSourceHostsCondition.java | 2 + .../ClientUpdaterSourceRolesCondition.java | 15 +- .../context/AbstractSamlRequestContext.java | 74 +++ .../context/SamlAuthnRequestContext.java | 42 ++ .../context/SamlLogoutRequestContext.java | 42 ++ .../SamlAvoidRedirectBindingExecutor.java | 106 ++++ ...mlAvoidRedirectBindingExecutorFactory.java | 71 +++ .../SamlSecureClientUrisExecutor.java | 203 ++++++++ .../SamlSecureClientUrisExecutorFactory.java | 81 ++++ .../SamlSignatureEnforcerExecutor.java | 84 ++++ .../SamlSignatureEnforcerExecutorFactory.java | 71 +++ ...ition.ClientPolicyConditionProviderFactory | 3 +- ...ecutor.ClientPolicyExecutorProviderFactory | 5 +- .../keycloak-default-client-profiles.json | 20 +- .../keycloak/testsuite/util/SamlClient.java | 14 +- .../policies/AbstractClientPoliciesTest.java | 3 +- .../ClientPoliciesLoadUpdateTest.java | 2 +- .../SamlClientPoliciesExecutorTest.java | 458 ++++++++++++++++++ .../testsuite/util/ClientPoliciesUtil.java | 6 +- 26 files changed, 1532 insertions(+), 21 deletions(-) create mode 100644 services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientProtocolCondition.java create mode 100644 services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientProtocolConditionFactory.java create mode 100644 services/src/main/java/org/keycloak/services/clientpolicy/context/AbstractSamlRequestContext.java create mode 100644 services/src/main/java/org/keycloak/services/clientpolicy/context/SamlAuthnRequestContext.java create mode 100644 services/src/main/java/org/keycloak/services/clientpolicy/context/SamlLogoutRequestContext.java create mode 100644 services/src/main/java/org/keycloak/services/clientpolicy/executor/SamlAvoidRedirectBindingExecutor.java create mode 100644 services/src/main/java/org/keycloak/services/clientpolicy/executor/SamlAvoidRedirectBindingExecutorFactory.java create mode 100644 services/src/main/java/org/keycloak/services/clientpolicy/executor/SamlSecureClientUrisExecutor.java create mode 100644 services/src/main/java/org/keycloak/services/clientpolicy/executor/SamlSecureClientUrisExecutorFactory.java create mode 100644 services/src/main/java/org/keycloak/services/clientpolicy/executor/SamlSignatureEnforcerExecutor.java create mode 100644 services/src/main/java/org/keycloak/services/clientpolicy/executor/SamlSignatureEnforcerExecutorFactory.java create mode 100644 testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/policies/SamlClientPoliciesExecutorTest.java diff --git a/server-spi/src/main/java/org/keycloak/services/clientpolicy/ClientPolicyEvent.java b/server-spi/src/main/java/org/keycloak/services/clientpolicy/ClientPolicyEvent.java index 3224f8ae33..4cc8779a62 100644 --- a/server-spi/src/main/java/org/keycloak/services/clientpolicy/ClientPolicyEvent.java +++ b/server-spi/src/main/java/org/keycloak/services/clientpolicy/ClientPolicyEvent.java @@ -52,6 +52,8 @@ public enum ClientPolicyEvent { DEVICE_TOKEN_REQUEST, DEVICE_TOKEN_RESPONSE, RESOURCE_OWNER_PASSWORD_CREDENTIALS_REQUEST, - RESOURCE_OWNER_PASSWORD_CREDENTIALS_RESPONSE + RESOURCE_OWNER_PASSWORD_CREDENTIALS_RESPONSE, + SAML_AUTHN_REQUEST, + SAML_LOGOUT_REQUEST, } diff --git a/services/src/main/java/org/keycloak/protocol/saml/SamlService.java b/services/src/main/java/org/keycloak/protocol/saml/SamlService.java index 3d87c5e89e..92bb1cbc7c 100755 --- a/services/src/main/java/org/keycloak/protocol/saml/SamlService.java +++ b/services/src/main/java/org/keycloak/protocol/saml/SamlService.java @@ -96,6 +96,10 @@ import org.keycloak.saml.processing.web.util.RedirectBindingUtil; import org.keycloak.saml.validators.DestinationValidator; import org.keycloak.services.ErrorPage; import org.keycloak.services.Urls; +import org.keycloak.services.clientpolicy.ClientPolicyContext; +import org.keycloak.services.clientpolicy.ClientPolicyException; +import org.keycloak.services.clientpolicy.context.SamlAuthnRequestContext; +import org.keycloak.services.clientpolicy.context.SamlLogoutRequestContext; import org.keycloak.services.managers.AuthenticationManager; import org.keycloak.services.managers.RealmManager; import org.keycloak.services.messages.Messages; @@ -260,6 +264,16 @@ public class SamlService extends AuthorizationEndpointBase { return response; } + protected Response triggerSamlEvent(ClientPolicyContext ctx) { + try { + session.clientPolicy().triggerOnEvent(ctx); + } catch (ClientPolicyException cpe) { + logger.warnf("Error in client policies processing the request: %s - %s", cpe.getError(), cpe.getErrorDetail()); + return error(session, null, Response.Status.BAD_REQUEST, Messages.INVALID_REQUEST); + } + return null; + } + protected Response handleSamlRequest(String samlRequest, String relayState) { SAMLDocumentHolder documentHolder = extractRequestDocument(samlRequest); if (documentHolder == null) { @@ -318,9 +332,17 @@ public class SamlService extends AuthorizationEndpointBase { if (samlObject instanceof AuthnRequestType) { // Get the SAML Request Message AuthnRequestType authn = (AuthnRequestType) samlObject; + Response response = triggerSamlEvent(new SamlAuthnRequestContext(authn, client, getBindingType())); + if (response != null) { + return response; + } return loginRequest(relayState, authn, client); } else if (samlObject instanceof LogoutRequestType) { LogoutRequestType logout = (LogoutRequestType) samlObject; + Response response = triggerSamlEvent(new SamlLogoutRequestContext(logout, client, getBindingType())); + if (response != null) { + return response; + } return logoutRequest(logout, client, relayState); } else { throw new IllegalStateException("Invalid SAML object"); diff --git a/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientProtocolCondition.java b/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientProtocolCondition.java new file mode 100644 index 0000000000..c606e197a0 --- /dev/null +++ b/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientProtocolCondition.java @@ -0,0 +1,123 @@ +/* + * Copyright 2024 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.condition; + +import org.keycloak.models.ClientModel; +import org.keycloak.models.KeycloakSession; +import org.keycloak.representations.idm.ClientPolicyConditionConfigurationRepresentation; +import org.keycloak.representations.idm.ClientRepresentation; +import org.keycloak.services.clientpolicy.ClientPolicyContext; +import org.keycloak.services.clientpolicy.ClientPolicyException; +import org.keycloak.services.clientpolicy.ClientPolicyVote; +import org.keycloak.services.clientpolicy.context.ClientCRUDContext; + +/** + * + * @author rmartinc + */ +public class ClientProtocolCondition extends AbstractClientPolicyConditionProvider { + + public static class Configuration extends ClientPolicyConditionConfigurationRepresentation { + + protected String protocol; + + public Configuration() { + protocol = null; + } + + public Configuration(String protocol) { + this.protocol = protocol; + } + + public String getProtocol() { + return protocol; + } + + public void setProtocol(String protocol) { + this.protocol = protocol; + } + } + + public ClientProtocolCondition(KeycloakSession session) { + super(session); + } + + @Override + public String getProviderId() { + return ClientAccessTypeConditionFactory.PROVIDER_ID; + } + + @Override + public Class getConditionConfigurationClass() { + return Configuration.class; + } + + @Override + public ClientPolicyVote applyPolicy(ClientPolicyContext context) throws ClientPolicyException { + switch (context.getEvent()) { + case AUTHORIZATION_REQUEST: + case TOKEN_REQUEST: + case TOKEN_RESPONSE: + case SERVICE_ACCOUNT_TOKEN_REQUEST: + case SERVICE_ACCOUNT_TOKEN_RESPONSE: + case TOKEN_REFRESH: + case TOKEN_REFRESH_RESPONSE: + case TOKEN_REVOKE: + case TOKEN_INTROSPECT: + case USERINFO_REQUEST: + case LOGOUT_REQUEST: + case UPDATE: + case UPDATED: + case REGISTERED: + case SAML_AUTHN_REQUEST: + case SAML_LOGOUT_REQUEST: + if (isCorrectProtocolFromContext()) { + return ClientPolicyVote.YES; + } + return ClientPolicyVote.NO; + case REGISTER: + if (isCorrectProtocolFromRepresentation((ClientCRUDContext)context)) { + return ClientPolicyVote.YES; + } + return ClientPolicyVote.NO; + default: + return ClientPolicyVote.ABSTAIN; + } + } + + public boolean isCorrectProtocolFromContext() { + ClientModel client = session.getContext().getClient(); + if (client != null) { + String protocol = client.getProtocol(); + if (protocol != null) { + return protocol.equals(configuration.getProtocol()); + } + } + return false; + } + + public boolean isCorrectProtocolFromRepresentation(ClientCRUDContext context) { + ClientRepresentation clientRep = context.getProposedClientRepresentation(); + if (clientRep != null) { + String protocol = clientRep.getProtocol(); + if (protocol != null) { + return protocol.equals(configuration.getProtocol()); + } + } + return false; + } +} diff --git a/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientProtocolConditionFactory.java b/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientProtocolConditionFactory.java new file mode 100644 index 0000000000..b765bf4014 --- /dev/null +++ b/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientProtocolConditionFactory.java @@ -0,0 +1,84 @@ +/* + * Copyright 2024 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.condition; + +import java.util.LinkedList; +import java.util.List; +import org.keycloak.Config; +import org.keycloak.models.KeycloakSession; +import org.keycloak.models.KeycloakSessionFactory; +import org.keycloak.protocol.LoginProtocol; +import org.keycloak.provider.ProviderConfigProperty; +import org.keycloak.provider.ProviderConfigurationBuilder; + +/** + * + * @author rmartinc + */ +public class ClientProtocolConditionFactory implements ClientPolicyConditionProviderFactory { + + public static final String PROVIDER_ID = "client-type"; + + private List loginProtocols; + + @Override + public ClientPolicyConditionProvider create(KeycloakSession session) { + return new ClientProtocolCondition(session); + } + + @Override + public void postInit(KeycloakSessionFactory factory) { + try (KeycloakSession session = factory.create()) { + loginProtocols = new LinkedList<>(session.listProviderIds(LoginProtocol.class)); + } + } + + @Override + public String getId() { + return PROVIDER_ID; + } + + @Override + public String getHelpText() { + return "Condition that uses the client's protocol (OpenID Connect, SAML) to determine whether the policy is applied."; + } + + @Override + public void init(Config.Scope config) { + // no-op + } + + @Override + public void close() { + // no-op + } + + @Override + public List getConfigProperties() { + return ProviderConfigurationBuilder.create() + .property() + .name("protocol") + .type(ProviderConfigProperty.LIST_TYPE) + .options(loginProtocols) + .defaultValue(loginProtocols.iterator().next()) + .label("Client protocol") + .helpText("What client login protocol the condition will apply on.") + .add() + .build(); + } +} diff --git a/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientRolesCondition.java b/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientRolesCondition.java index a6e1ac42bf..8dccc27fe6 100644 --- a/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientRolesCondition.java +++ b/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientRolesCondition.java @@ -91,6 +91,8 @@ public class ClientRolesCondition extends AbstractClientPolicyConditionProvider< case PUSHED_AUTHORIZATION_REQUEST: case REGISTERED: case UPDATED: + case SAML_AUTHN_REQUEST: + case SAML_LOGOUT_REQUEST: if (isRolesMatched(session.getContext().getClient())) return ClientPolicyVote.YES; return ClientPolicyVote.NO; default: diff --git a/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientUpdaterContextCondition.java b/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientUpdaterContextCondition.java index fd0aec5b23..2e6bc841f0 100644 --- a/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientUpdaterContextCondition.java +++ b/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientUpdaterContextCondition.java @@ -73,6 +73,8 @@ public class ClientUpdaterContextCondition extends AbstractClientPolicyCondition switch (context.getEvent()) { case REGISTER: case UPDATE: + case REGISTERED: + case UPDATED: if (isAuthMethodMatched((ClientCRUDContext)context)) return ClientPolicyVote.YES; return ClientPolicyVote.NO; default: diff --git a/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientUpdaterSourceGroupsCondition.java b/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientUpdaterSourceGroupsCondition.java index ee39f8baee..e7eae61785 100644 --- a/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientUpdaterSourceGroupsCondition.java +++ b/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientUpdaterSourceGroupsCondition.java @@ -33,10 +33,14 @@ import org.keycloak.services.clientpolicy.ClientPolicyContext; import org.keycloak.services.clientpolicy.ClientPolicyException; import org.keycloak.services.clientpolicy.ClientPolicyVote; import org.keycloak.services.clientpolicy.context.AdminClientRegisterContext; +import org.keycloak.services.clientpolicy.context.AdminClientRegisteredContext; import org.keycloak.services.clientpolicy.context.AdminClientUpdateContext; +import org.keycloak.services.clientpolicy.context.AdminClientUpdatedContext; import org.keycloak.services.clientpolicy.context.ClientCRUDContext; import org.keycloak.services.clientpolicy.context.DynamicClientRegisterContext; +import org.keycloak.services.clientpolicy.context.DynamicClientRegisteredContext; import org.keycloak.services.clientpolicy.context.DynamicClientUpdateContext; +import org.keycloak.services.clientpolicy.context.DynamicClientUpdatedContext; /** * @author Takashi Norimatsu @@ -76,17 +80,19 @@ public class ClientUpdaterSourceGroupsCondition extends AbstractClientPolicyCond public ClientPolicyVote applyPolicy(ClientPolicyContext context) throws ClientPolicyException { switch (context.getEvent()) { case REGISTER: - if (context instanceof AdminClientRegisterContext) { + case REGISTERED: + if (context instanceof AdminClientRegisterContext || context instanceof AdminClientRegisteredContext) { return getVoteForGroupsMatched(((ClientCRUDContext)context).getAuthenticatedUser()); - } else if (context instanceof DynamicClientRegisterContext) { + } else if (context instanceof DynamicClientRegisterContext || context instanceof DynamicClientRegisteredContext) { return getVoteForGroupsMatched(((ClientCRUDContext)context).getToken()); } else { throw new ClientPolicyException(OAuthErrorException.SERVER_ERROR, "unexpected context type."); } case UPDATE: - if (context instanceof AdminClientUpdateContext) { + case UPDATED: + if (context instanceof AdminClientUpdateContext || context instanceof AdminClientUpdatedContext) { return getVoteForGroupsMatched(((ClientCRUDContext)context).getAuthenticatedUser()); - } else if (context instanceof DynamicClientUpdateContext) { + } else if (context instanceof DynamicClientUpdateContext || context instanceof DynamicClientUpdatedContext) { return getVoteForGroupsMatched(((ClientCRUDContext)context).getToken()); } else { throw new ClientPolicyException(OAuthErrorException.SERVER_ERROR, "unexpected context type."); diff --git a/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientUpdaterSourceHostsCondition.java b/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientUpdaterSourceHostsCondition.java index de25eb004a..ead4c7be08 100644 --- a/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientUpdaterSourceHostsCondition.java +++ b/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientUpdaterSourceHostsCondition.java @@ -73,6 +73,8 @@ public class ClientUpdaterSourceHostsCondition extends AbstractClientPolicyCondi switch (context.getEvent()) { case REGISTER: case UPDATE: + case REGISTERED: + case UPDATED: if (isHostMatched()) return ClientPolicyVote.YES; return ClientPolicyVote.NO; default: diff --git a/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientUpdaterSourceRolesCondition.java b/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientUpdaterSourceRolesCondition.java index 899cdf7cce..0e6a677c47 100644 --- a/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientUpdaterSourceRolesCondition.java +++ b/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientUpdaterSourceRolesCondition.java @@ -35,10 +35,14 @@ import org.keycloak.services.clientpolicy.ClientPolicyContext; import org.keycloak.services.clientpolicy.ClientPolicyException; import org.keycloak.services.clientpolicy.ClientPolicyVote; import org.keycloak.services.clientpolicy.context.AdminClientRegisterContext; +import org.keycloak.services.clientpolicy.context.AdminClientRegisteredContext; import org.keycloak.services.clientpolicy.context.AdminClientUpdateContext; +import org.keycloak.services.clientpolicy.context.AdminClientUpdatedContext; import org.keycloak.services.clientpolicy.context.ClientCRUDContext; import org.keycloak.services.clientpolicy.context.DynamicClientRegisterContext; +import org.keycloak.services.clientpolicy.context.DynamicClientRegisteredContext; import org.keycloak.services.clientpolicy.context.DynamicClientUpdateContext; +import org.keycloak.services.clientpolicy.context.DynamicClientUpdatedContext; /** @@ -79,17 +83,20 @@ public class ClientUpdaterSourceRolesCondition extends AbstractClientPolicyCondi public ClientPolicyVote applyPolicy(ClientPolicyContext context) throws ClientPolicyException { switch (context.getEvent()) { case REGISTER: - if (context instanceof AdminClientRegisterContext) { + case REGISTERED: + if (context instanceof AdminClientRegisterContext || context instanceof AdminClientRegisteredContext) { return getVoteForRolesMatched(((ClientCRUDContext)context).getAuthenticatedUser()); - } else if (context instanceof DynamicClientRegisterContext) { + } else if (context instanceof DynamicClientRegisterContext || context instanceof DynamicClientRegisteredContext) { return getVoteForRolesMatched(((ClientCRUDContext)context).getToken()); } else { throw new ClientPolicyException(OAuthErrorException.SERVER_ERROR, "unexpected context type."); } + case UPDATE: - if (context instanceof AdminClientUpdateContext) { + case UPDATED: + if (context instanceof AdminClientUpdateContext || context instanceof AdminClientUpdatedContext) { return getVoteForRolesMatched(((ClientCRUDContext)context).getAuthenticatedUser()); - } else if (context instanceof DynamicClientUpdateContext) { + } else if (context instanceof DynamicClientUpdateContext || context instanceof DynamicClientUpdatedContext) { return getVoteForRolesMatched(((ClientCRUDContext)context).getToken()); } else { throw new ClientPolicyException(OAuthErrorException.SERVER_ERROR, "unexpected context type."); diff --git a/services/src/main/java/org/keycloak/services/clientpolicy/context/AbstractSamlRequestContext.java b/services/src/main/java/org/keycloak/services/clientpolicy/context/AbstractSamlRequestContext.java new file mode 100644 index 0000000000..e4696c90cd --- /dev/null +++ b/services/src/main/java/org/keycloak/services/clientpolicy/context/AbstractSamlRequestContext.java @@ -0,0 +1,74 @@ +/* + * Copyright 2024 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.context; + +import org.keycloak.models.ClientModel; +import org.keycloak.services.clientpolicy.ClientPolicyContext; +import org.keycloak.services.clientpolicy.ClientPolicyEvent; + + +/** + *

Abstract saml request context for any SAML request received. The context + * will have the type object received, the client model and binding type + * the client used to connect.

+ * + * @author rmartinc + * @param The saml request type + */ +public abstract class AbstractSamlRequestContext implements ClientPolicyContext { + + protected final T request; + protected final ClientModel client; + protected final String protocolBinding; + + public AbstractSamlRequestContext(final T request, final ClientModel client, final String protocolBinding) { + this.request = request; + this.client = client; + this.protocolBinding = protocolBinding; + } + + /** + * {@inheritDoc} + */ + @Override + public abstract ClientPolicyEvent getEvent(); + + /** + * Getter for the SAML request received. + * @return The SAML request type + */ + public T getRequest() { + return request; + } + + /** + * Getter for the client model doing the request. + * @return The client model + */ + public ClientModel getClient() { + return client; + } + + /** + * Getter for the protocol binding type that is processing the request. + * @return The keycloak protocol binding type. + */ + public String getProtocolBinding() { + return protocolBinding; + } +} diff --git a/services/src/main/java/org/keycloak/services/clientpolicy/context/SamlAuthnRequestContext.java b/services/src/main/java/org/keycloak/services/clientpolicy/context/SamlAuthnRequestContext.java new file mode 100644 index 0000000000..781e7bbe27 --- /dev/null +++ b/services/src/main/java/org/keycloak/services/clientpolicy/context/SamlAuthnRequestContext.java @@ -0,0 +1,42 @@ +/* + * Copyright 2024 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.context; + +import org.keycloak.dom.saml.v2.protocol.AuthnRequestType; +import org.keycloak.models.ClientModel; +import org.keycloak.services.clientpolicy.ClientPolicyEvent; + +/** + *

Context for the saml authn request.

+ * + * @author rmartinc + */ +public class SamlAuthnRequestContext extends AbstractSamlRequestContext { + + public SamlAuthnRequestContext(final AuthnRequestType request, final ClientModel client, final String protocolBinding) { + super(request, client, protocolBinding); + } + + /** + * {@inheritDoc} + */ + @Override + public ClientPolicyEvent getEvent() { + return ClientPolicyEvent.SAML_AUTHN_REQUEST; + } +} diff --git a/services/src/main/java/org/keycloak/services/clientpolicy/context/SamlLogoutRequestContext.java b/services/src/main/java/org/keycloak/services/clientpolicy/context/SamlLogoutRequestContext.java new file mode 100644 index 0000000000..051805edce --- /dev/null +++ b/services/src/main/java/org/keycloak/services/clientpolicy/context/SamlLogoutRequestContext.java @@ -0,0 +1,42 @@ +/* + * Copyright 2024 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.context; + +import org.keycloak.dom.saml.v2.protocol.LogoutRequestType; +import org.keycloak.models.ClientModel; +import org.keycloak.services.clientpolicy.ClientPolicyEvent; + +/** + *

Context for the saml logout request.

+ * + * @author rmartinc + */ +public class SamlLogoutRequestContext extends AbstractSamlRequestContext { + + public SamlLogoutRequestContext(final LogoutRequestType request, final ClientModel client, final String protocolBinding) { + super(request, client, protocolBinding); + } + + /** + * {@inheritDoc} + */ + @Override + public ClientPolicyEvent getEvent() { + return ClientPolicyEvent.SAML_LOGOUT_REQUEST; + } +} diff --git a/services/src/main/java/org/keycloak/services/clientpolicy/executor/SamlAvoidRedirectBindingExecutor.java b/services/src/main/java/org/keycloak/services/clientpolicy/executor/SamlAvoidRedirectBindingExecutor.java new file mode 100644 index 0000000000..e9d1a5ae14 --- /dev/null +++ b/services/src/main/java/org/keycloak/services/clientpolicy/executor/SamlAvoidRedirectBindingExecutor.java @@ -0,0 +1,106 @@ +/* + * Copyright 2024 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.net.URI; +import org.keycloak.OAuthErrorException; +import org.keycloak.models.ClientModel; +import org.keycloak.models.KeycloakSession; +import org.keycloak.protocol.saml.SamlClient; +import org.keycloak.protocol.saml.SamlProtocol; +import org.keycloak.representations.idm.ClientPolicyExecutorConfigurationRepresentation; +import org.keycloak.saml.common.constants.JBossSAMLURIConstants; +import org.keycloak.services.clientpolicy.ClientPolicyContext; +import org.keycloak.services.clientpolicy.ClientPolicyException; +import org.keycloak.services.clientpolicy.context.AdminClientRegisteredContext; +import org.keycloak.services.clientpolicy.context.AdminClientUpdatedContext; +import org.keycloak.services.clientpolicy.context.SamlAuthnRequestContext; +import org.keycloak.services.clientpolicy.context.SamlLogoutRequestContext; + +/** + * + * @author rmartinc + */ +public class SamlAvoidRedirectBindingExecutor implements ClientPolicyExecutorProvider { + + public SamlAvoidRedirectBindingExecutor(KeycloakSession session) { + } + + @Override + public String getProviderId() { + return SamlAvoidRedirectBindingExecutorFactory.PROVIDER_ID; + } + + @Override + public void executeOnEvent(ClientPolicyContext context) throws ClientPolicyException { + switch (context.getEvent()) { + case REGISTERED -> { + confirmPostBindingIsForced(((AdminClientRegisteredContext)context).getTargetClient()); + } + case UPDATED -> { + confirmPostBindingIsForced(((AdminClientUpdatedContext)context).getTargetClient()); + } + case SAML_AUTHN_REQUEST -> { + confirmRedirectBindingIsNotUsed((SamlAuthnRequestContext) context); + } + case SAML_LOGOUT_REQUEST -> { + confirmRedirectBindingIsNotUsed((SamlLogoutRequestContext) context); + } + } + } + + private void confirmPostBindingIsForced(ClientModel client) throws ClientPolicyException { + if (SamlProtocol.LOGIN_PROTOCOL.equals(client.getProtocol())) { + SamlClient samlClient = new SamlClient(client); + if (!samlClient.forcePostBinding()) { + throw new ClientPolicyException(OAuthErrorException.INVALID_CLIENT_METADATA, "Force POST binding is not enabled"); + } + } + } + + private void confirmRedirectBindingIsNotUsed(SamlAuthnRequestContext context) throws ClientPolicyException { + SamlClient samlClient = new SamlClient(context.getClient()); + if (samlClient.forcePostBinding()) { + return; + } + URI requestedBinding = context.getRequest().getProtocolBinding(); + if (requestedBinding == null) { + // no request binding explicitly requested so using the one used by the request + if (context.getProtocolBinding().equals(SamlProtocol.SAML_REDIRECT_BINDING)) { + throw new ClientPolicyException(OAuthErrorException.INVALID_REQUEST, "REDIRECT binding is used for the login request and it is not allowed."); + } + } else { + // explicit request binding request check it's not redirect or artifact+redirect + if (JBossSAMLURIConstants.SAML_HTTP_REDIRECT_BINDING.get().equals(requestedBinding.toString()) + || (JBossSAMLURIConstants.SAML_HTTP_ARTIFACT_BINDING.get().equals(requestedBinding.toString()) + && context.getProtocolBinding().equals(SamlProtocol.SAML_REDIRECT_BINDING))) { + throw new ClientPolicyException(OAuthErrorException.INVALID_REQUEST, "REDIRECT binding is used for the login request and it is not allowed."); + } + } + } + + private void confirmRedirectBindingIsNotUsed(SamlLogoutRequestContext context) throws ClientPolicyException { + SamlClient samlClient = new SamlClient(context.getClient()); + if (samlClient.forcePostBinding()) { + return; + } + if (context.getProtocolBinding().equals(SamlProtocol.SAML_REDIRECT_BINDING)) { + throw new ClientPolicyException(OAuthErrorException.INVALID_REQUEST, "REDIRECT binding is used for the logout request and it is not allowed."); + } + } +} diff --git a/services/src/main/java/org/keycloak/services/clientpolicy/executor/SamlAvoidRedirectBindingExecutorFactory.java b/services/src/main/java/org/keycloak/services/clientpolicy/executor/SamlAvoidRedirectBindingExecutorFactory.java new file mode 100644 index 0000000000..ff2916e251 --- /dev/null +++ b/services/src/main/java/org/keycloak/services/clientpolicy/executor/SamlAvoidRedirectBindingExecutorFactory.java @@ -0,0 +1,71 @@ +/* + * Copyright 2024 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.List; +import org.keycloak.Config; +import org.keycloak.models.KeycloakSession; +import org.keycloak.models.KeycloakSessionFactory; +import org.keycloak.provider.ProviderConfigProperty; +import org.keycloak.provider.ProviderConfigurationBuilder; + +/** + *

Executor factory for SAML client that ensures REDIRECT is not used for responses + * and forces POST binding configuration option in the client creation/update.

+ * + * @author rmartinc + */ +public class SamlAvoidRedirectBindingExecutorFactory implements ClientPolicyExecutorProviderFactory { + + public static final String PROVIDER_ID = "saml-avoid-redirect"; + + @Override + public ClientPolicyExecutorProvider create(KeycloakSession session) { + return new SamlAvoidRedirectBindingExecutor(session); + } + + @Override + public void init(Config.Scope config) { + // no-op + } + + @Override + public void postInit(KeycloakSessionFactory factory) { + // no-op + } + + @Override + public void close() { + // no-op + } + + @Override + public String getId() { + return PROVIDER_ID; + } + + @Override + public String getHelpText() { + return "Executor to avoid the REDIRECT binding in a SAML client."; + } + + @Override + public List getConfigProperties() { + return ProviderConfigurationBuilder.create().build(); + } +} diff --git a/services/src/main/java/org/keycloak/services/clientpolicy/executor/SamlSecureClientUrisExecutor.java b/services/src/main/java/org/keycloak/services/clientpolicy/executor/SamlSecureClientUrisExecutor.java new file mode 100644 index 0000000000..fba8ef7668 --- /dev/null +++ b/services/src/main/java/org/keycloak/services/clientpolicy/executor/SamlSecureClientUrisExecutor.java @@ -0,0 +1,203 @@ +/* + * Copyright 2024 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 com.fasterxml.jackson.annotation.JsonProperty; +import java.net.URI; +import java.util.Collection; +import java.util.Map; +import org.keycloak.OAuthErrorException; +import org.keycloak.dom.saml.v2.protocol.AuthnRequestType; +import org.keycloak.models.ClientModel; +import org.keycloak.models.KeycloakSession; +import org.keycloak.protocol.oidc.utils.RedirectUtils; +import org.keycloak.protocol.saml.SamlProtocol; +import org.keycloak.representations.idm.ClientPolicyExecutorConfigurationRepresentation; +import org.keycloak.services.clientpolicy.ClientPolicyContext; +import org.keycloak.services.clientpolicy.ClientPolicyException; +import org.keycloak.services.clientpolicy.context.AdminClientRegisteredContext; +import org.keycloak.services.clientpolicy.context.AdminClientUpdatedContext; +import org.keycloak.services.clientpolicy.context.SamlAuthnRequestContext; +import org.keycloak.services.clientpolicy.context.SamlLogoutRequestContext; +import org.keycloak.utils.StringUtil; + +/** + * + * @author rmartinc + */ +public class SamlSecureClientUrisExecutor implements ClientPolicyExecutorProvider { + + private final KeycloakSession session; + private Configuration config; + + public static class Configuration extends ClientPolicyExecutorConfigurationRepresentation { + @JsonProperty("allow-wildcard-redirects") + protected boolean allowWildcardRedirects; + + public Configuration() { + this(false); + } + + public Configuration(boolean allowWildcardRedirects) { + this.allowWildcardRedirects = allowWildcardRedirects; + } + + public boolean isAllowWildcardResirects() { + return allowWildcardRedirects; + } + + public void setAllowWildcardResirects(boolean allowWildcardRedirects) { + this.allowWildcardRedirects = allowWildcardRedirects; + } + } + + public SamlSecureClientUrisExecutor(KeycloakSession session) { + this.session = session; + } + + @Override + public Class getExecutorConfigurationClass() { + return Configuration.class; + } + + @Override + public void setupConfiguration(Configuration config) { + this.config = config; + } + + @Override + public String getProviderId() { + return SamlSecureClientUrisExecutorFactory.PROVIDER_ID; + } + + @Override + public void executeOnEvent(ClientPolicyContext context) throws ClientPolicyException { + switch (context.getEvent()) { + case REGISTERED -> { + confirmSecureUris(((AdminClientRegisteredContext)context).getTargetClient()); + } + case UPDATED -> { + confirmSecureUris(((AdminClientUpdatedContext)context).getTargetClient()); + } + case SAML_AUTHN_REQUEST -> { + confirmLoginRedirectUri((SamlAuthnRequestContext) context); + } + case SAML_LOGOUT_REQUEST -> { + confirmLogoutRedirectUri((SamlLogoutRequestContext) context); + } + } + } + + private void confirmLoginRedirectUri(SamlAuthnRequestContext context) throws ClientPolicyException { + AuthnRequestType request = context.getRequest(); + URI uri = request.getAssertionConsumerServiceURL(); + if (uri != null) { + confirmSecureUri(uri.toString(), "AssertionConsumerServiceURL", OAuthErrorException.INVALID_REQUEST); + } else { + // use configuration for login, check URLs for login are all secure + ClientModel client = context.getClient(); + confirmSecureUri(client.getManagementUrl(), + "Master SAML Processing URL", OAuthErrorException.INVALID_REQUEST); + confirmSecureUri(client.getAttribute(SamlProtocol.SAML_ASSERTION_CONSUMER_URL_POST_ATTRIBUTE), + "Assertion Consumer Service POST Binding URL", OAuthErrorException.INVALID_REQUEST); + confirmSecureUri(client.getAttribute(SamlProtocol.SAML_ASSERTION_CONSUMER_URL_REDIRECT_ATTRIBUTE), + "Assertion Consumer Service Redirect Binding URL", OAuthErrorException.INVALID_REQUEST); + confirmSecureUri(client.getAttribute(SamlProtocol.SAML_ASSERTION_CONSUMER_URL_ARTIFACT_ATTRIBUTE), + "Artifact Binding URL", OAuthErrorException.INVALID_REQUEST); + } + } + + private void confirmLogoutRedirectUri(SamlLogoutRequestContext context) throws ClientPolicyException { + // check logout URLs are all secure + ClientModel client = context.getClient(); + confirmSecureUri(client.getManagementUrl(), + "Master SAML Processing URL", OAuthErrorException.INVALID_REQUEST); + confirmSecureUri(client.getAttribute(SamlProtocol.SAML_SINGLE_LOGOUT_SERVICE_URL_POST_ATTRIBUTE), + "Logout Service POST Binding URL", OAuthErrorException.INVALID_REQUEST); + confirmSecureUri(client.getAttribute(SamlProtocol.SAML_SINGLE_LOGOUT_SERVICE_URL_ARTIFACT_ATTRIBUTE), + "Logout Service ARTIFACT Binding URL", OAuthErrorException.INVALID_REQUEST); + confirmSecureUri(client.getAttribute(SamlProtocol.SAML_SINGLE_LOGOUT_SERVICE_URL_REDIRECT_ATTRIBUTE), + "Logout Service Redirect Binding URL", OAuthErrorException.INVALID_REQUEST); + confirmSecureUri(client.getAttribute(SamlProtocol.SAML_SINGLE_LOGOUT_SERVICE_URL_SOAP_ATTRIBUTE), + "Logout Service SOAP Binding URL", OAuthErrorException.INVALID_REQUEST); + } + + private void confirmSecureUri(String uri, String uriType) throws ClientPolicyException { + confirmSecureUri(uri, uriType, OAuthErrorException.INVALID_CLIENT_METADATA); + } + + private void confirmSecureUri(String uri, String uriType, String error) throws ClientPolicyException { + if (StringUtil.isBlank(uri)) { + return; + } + + if (!uri.startsWith("https:")) { // make this configurable? (allowed schemes...) + throw new ClientPolicyException(error, "Non secure scheme for " + uriType); + } + } + + private void confirmNoWildcard(String uri, String uriType) throws ClientPolicyException { + if (uri.endsWith("*") && !uri.contains("?") && !uri.contains("#")) { + throw new ClientPolicyException(OAuthErrorException.INVALID_CLIENT_METADATA, + "Unsecure wildcard redirect " + uri + " for " + uriType); + } + } + + private void confirmRedirectUris(Collection uris, String uriType) throws ClientPolicyException { + if (uris == null) { + return; + } + + for (String uri : uris) { + confirmSecureUri(uri, uriType); + if (!config.isAllowWildcardResirects()) { + confirmNoWildcard(uri, uriType); + } + } + } + + private void confirmSecureUris(ClientModel client) throws ClientPolicyException { + if (!SamlProtocol.LOGIN_PROTOCOL.equals(client.getProtocol())) { + return; + } + + confirmSecureUri(client.getRootUrl(), "Root URL"); + confirmSecureUri(client.getManagementUrl(), "Master SAML Processing URL"); + confirmSecureUri(client.getBaseUrl(), "Home URL"); + + confirmRedirectUris(RedirectUtils.resolveValidRedirects(session, client.getRootUrl(), client.getRedirectUris()), + "Valid redirect URIs"); + + Map attrs = Map.of( + SamlProtocol.SAML_ASSERTION_CONSUMER_URL_POST_ATTRIBUTE, "Assertion Consumer Service POST Binding URL", + SamlProtocol.SAML_ASSERTION_CONSUMER_URL_REDIRECT_ATTRIBUTE, "Assertion Consumer Service Redirect Binding URL", + SamlProtocol.SAML_ASSERTION_CONSUMER_URL_ARTIFACT_ATTRIBUTE, "Artifact Binding URL", + SamlProtocol.SAML_SINGLE_LOGOUT_SERVICE_URL_POST_ATTRIBUTE, "Logout Service POST Binding URL", + SamlProtocol.SAML_SINGLE_LOGOUT_SERVICE_URL_ARTIFACT_ATTRIBUTE, "Logout Service ARTIFACT Binding URL", + SamlProtocol.SAML_SINGLE_LOGOUT_SERVICE_URL_REDIRECT_ATTRIBUTE, "Logout Service Redirect Binding URL", + SamlProtocol.SAML_SINGLE_LOGOUT_SERVICE_URL_SOAP_ATTRIBUTE, "Logout Service SOAP Binding URL", + SamlProtocol.SAML_ARTIFACT_RESOLUTION_SERVICE_URL_ATTRIBUTE, "Artifact Resolution Service" + ); + + if (client.getAttributes() != null) { + for (Map.Entry attr : attrs.entrySet()) { + confirmSecureUri(client.getAttribute(attr.getKey()), attr.getValue()); + } + } + } +} diff --git a/services/src/main/java/org/keycloak/services/clientpolicy/executor/SamlSecureClientUrisExecutorFactory.java b/services/src/main/java/org/keycloak/services/clientpolicy/executor/SamlSecureClientUrisExecutorFactory.java new file mode 100644 index 0000000000..b37965cac9 --- /dev/null +++ b/services/src/main/java/org/keycloak/services/clientpolicy/executor/SamlSecureClientUrisExecutorFactory.java @@ -0,0 +1,81 @@ +/* + * Copyright 2024 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.List; +import org.keycloak.Config; +import org.keycloak.models.KeycloakSession; +import org.keycloak.models.KeycloakSessionFactory; +import org.keycloak.provider.ProviderConfigProperty; +import org.keycloak.provider.ProviderConfigurationBuilder; + +/** + *

Executor factory that enforces that all URLs configured in a SAML client + * are secure (https). It also enforces that no wildcard valid redirect URIs + * are configured on update/creation (wildcards can be allowed via + * allow-wildcard-redirects configuration property).

+ * + * @author rmartinc + */ +public class SamlSecureClientUrisExecutorFactory implements ClientPolicyExecutorProviderFactory { + + public static final String PROVIDER_ID = "saml-secure-client-uris"; + + @Override + public ClientPolicyExecutorProvider create(KeycloakSession session) { + return new SamlSecureClientUrisExecutor(session); + } + + @Override + public void init(Config.Scope config) { + // no-op + } + + @Override + public void postInit(KeycloakSessionFactory factory) { + // no-op + } + + @Override + public void close() { + // no-op + } + + @Override + public String getId() { + return PROVIDER_ID; + } + + @Override + public String getHelpText() { + return "Executor that enforces all URLs defined in the SAML client are https (TLS enabled). " + + "It also enforces that wildcard redirect URIs are not used (configurable)."; + } + + @Override + public List getConfigProperties() { + return ProviderConfigurationBuilder.create() + .property() + .name("allow-wildcard-redirects") + .type(ProviderConfigProperty.BOOLEAN_TYPE) + .label("Allow wildcard valid redirect URIs") + .helpText("Whether wildcard valid redirect URIs are allowed to be configured in the client.") + .add() + .build(); + } +} diff --git a/services/src/main/java/org/keycloak/services/clientpolicy/executor/SamlSignatureEnforcerExecutor.java b/services/src/main/java/org/keycloak/services/clientpolicy/executor/SamlSignatureEnforcerExecutor.java new file mode 100644 index 0000000000..1a6d6a0831 --- /dev/null +++ b/services/src/main/java/org/keycloak/services/clientpolicy/executor/SamlSignatureEnforcerExecutor.java @@ -0,0 +1,84 @@ +/* + * Copyright 2024 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.ClientModel; +import org.keycloak.models.KeycloakSession; +import org.keycloak.protocol.saml.SamlClient; +import org.keycloak.protocol.saml.SamlProtocol; +import org.keycloak.representations.idm.ClientPolicyExecutorConfigurationRepresentation; +import org.keycloak.services.clientpolicy.ClientPolicyContext; +import org.keycloak.services.clientpolicy.ClientPolicyException; +import org.keycloak.services.clientpolicy.context.AdminClientRegisteredContext; +import org.keycloak.services.clientpolicy.context.AdminClientUpdatedContext; +import org.keycloak.services.clientpolicy.context.SamlAuthnRequestContext; +import org.keycloak.services.clientpolicy.context.SamlLogoutRequestContext; + +/** + * + * @author rmartinc + */ +public class SamlSignatureEnforcerExecutor implements ClientPolicyExecutorProvider { + + public SamlSignatureEnforcerExecutor(KeycloakSession session) { + } + + @Override + public void executeOnEvent(ClientPolicyContext context) throws ClientPolicyException { + switch (context.getEvent()) { + case REGISTERED -> { + confirmSignaturesAreForcedRegister(((AdminClientRegisteredContext)context).getTargetClient()); + } + case UPDATED -> { + confirmSignaturesAreForcedRegister(((AdminClientUpdatedContext)context).getTargetClient()); + } + case SAML_AUTHN_REQUEST -> { + confirmSignaturesAreForced(((SamlAuthnRequestContext) context).getClient(), OAuthErrorException.INVALID_REQUEST); + } + case SAML_LOGOUT_REQUEST -> { + confirmSignaturesAreForced(((SamlLogoutRequestContext) context).getClient(), OAuthErrorException.INVALID_REQUEST); + } + } + } + + @Override + public String getProviderId() { + return SamlSignatureEnforcerExecutorFactory.PROVIDER_ID; + } + + private boolean signaturesAreForced(boolean clientSignature, boolean serverSignature, boolean assertionSignature) { + // ensure client is signed and server or asertion is signed + return clientSignature && (serverSignature || assertionSignature); + } + + private void confirmSignaturesAreForcedRegister(ClientModel client) throws ClientPolicyException { + if (SamlProtocol.LOGIN_PROTOCOL.equals(client.getProtocol())) { + confirmSignaturesAreForced(client, OAuthErrorException.INVALID_CLIENT_METADATA); + } + } + + private void confirmSignaturesAreForced(ClientModel client, String error) throws ClientPolicyException { + SamlClient samlClient = new SamlClient(client); + if (!signaturesAreForced(samlClient.requiresClientSignature(), samlClient.requiresRealmSignature(), + samlClient.requiresAssertionSignature())) { + throw new ClientPolicyException(error, + "Signatures not ensured for the client. Ensure Client signature required and Sign documents or Sign assertions are ON"); + } + } +} diff --git a/services/src/main/java/org/keycloak/services/clientpolicy/executor/SamlSignatureEnforcerExecutorFactory.java b/services/src/main/java/org/keycloak/services/clientpolicy/executor/SamlSignatureEnforcerExecutorFactory.java new file mode 100644 index 0000000000..d26bea755c --- /dev/null +++ b/services/src/main/java/org/keycloak/services/clientpolicy/executor/SamlSignatureEnforcerExecutorFactory.java @@ -0,0 +1,71 @@ +/* + * Copyright 2024 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.List; +import org.keycloak.Config; +import org.keycloak.models.KeycloakSession; +import org.keycloak.models.KeycloakSessionFactory; +import org.keycloak.provider.ProviderConfigProperty; +import org.keycloak.provider.ProviderConfigurationBuilder; + +/** + *

Policy executor that enforces client and server (full document or + * assertion) signature is ON.

+ * + * @author rmartinc + */ +public class SamlSignatureEnforcerExecutorFactory implements ClientPolicyExecutorProviderFactory { + + public static final String PROVIDER_ID = "saml-signature-enforcer"; + + @Override + public ClientPolicyExecutorProvider create(KeycloakSession session) { + return new SamlSignatureEnforcerExecutor(session); + } + + @Override + public void init(Config.Scope config) { + // no-op + } + + @Override + public void postInit(KeycloakSessionFactory factory) { + // no-op + } + + @Override + public void close() { + // no-op + } + + @Override + public String getId() { + return PROVIDER_ID; + } + + @Override + public String getHelpText() { + return "Executor to enforce that signatures are used in a SAML client."; + } + + @Override + public List getConfigProperties() { + return ProviderConfigurationBuilder.create().build(); + } +} diff --git a/services/src/main/resources/META-INF/services/org.keycloak.services.clientpolicy.condition.ClientPolicyConditionProviderFactory b/services/src/main/resources/META-INF/services/org.keycloak.services.clientpolicy.condition.ClientPolicyConditionProviderFactory index 0b5e3cffde..888c84ba0b 100644 --- a/services/src/main/resources/META-INF/services/org.keycloak.services.clientpolicy.condition.ClientPolicyConditionProviderFactory +++ b/services/src/main/resources/META-INF/services/org.keycloak.services.clientpolicy.condition.ClientPolicyConditionProviderFactory @@ -5,4 +5,5 @@ org.keycloak.services.clientpolicy.condition.ClientAccessTypeConditionFactory org.keycloak.services.clientpolicy.condition.ClientUpdaterSourceHostsConditionFactory org.keycloak.services.clientpolicy.condition.ClientUpdaterSourceGroupsConditionFactory org.keycloak.services.clientpolicy.condition.ClientUpdaterSourceRolesConditionFactory -org.keycloak.services.clientpolicy.condition.AnyClientConditionFactory \ No newline at end of file +org.keycloak.services.clientpolicy.condition.AnyClientConditionFactory +org.keycloak.services.clientpolicy.condition.ClientProtocolConditionFactory 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 4f0320c276..5b017e6255 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 @@ -24,4 +24,7 @@ org.keycloak.services.clientpolicy.executor.RejectImplicitGrantExecutorFactory org.keycloak.services.clientpolicy.executor.SecureParContentsExecutorFactory org.keycloak.services.clientpolicy.executor.DPoPBindEnforcerExecutorFactory org.keycloak.services.clientpolicy.executor.UseLightweightAccessTokenExecutorFactory -org.keycloak.services.clientpolicy.executor.SecureRedirectUrisEnforcerExecutorFactory \ No newline at end of file +org.keycloak.services.clientpolicy.executor.SecureRedirectUrisEnforcerExecutorFactory +org.keycloak.services.clientpolicy.executor.SamlSecureClientUrisExecutorFactory +org.keycloak.services.clientpolicy.executor.SamlAvoidRedirectBindingExecutorFactory +org.keycloak.services.clientpolicy.executor.SamlSignatureEnforcerExecutorFactory diff --git a/services/src/main/resources/keycloak-default-client-profiles.json b/services/src/main/resources/keycloak-default-client-profiles.json index 559f0a68a1..27cf329cb7 100644 --- a/services/src/main/resources/keycloak-default-client-profiles.json +++ b/services/src/main/resources/keycloak-default-client-profiles.json @@ -376,6 +376,24 @@ } } ] + }, + { + "name": "saml-security-profile", + "description": "Client profile that enforces SAML clients to be secure.", + "executors": [ + { + "executor": "saml-secure-client-uris", + "configuration": {} + }, + { + "executor": "saml-signature-enforcer", + "configuration": {} + }, + { + "executor": "saml-avoid-redirect", + "configuration": {} + } + ] } ] -} \ No newline at end of file +} diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/SamlClient.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/SamlClient.java index 57ebc24859..75ab46416d 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/SamlClient.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/SamlClient.java @@ -200,10 +200,13 @@ public class SamlClient { try { BaseSAML2BindingBuilder binding = new BaseSAML2BindingBuilder(); - if (privateKeyStr != null && publicKeyStr != null) { + if (privateKeyStr != null && (publicKeyStr != null || certificateStr != null)) { PrivateKey privateKey = org.keycloak.testsuite.util.KeyUtils.privateKeyFromString(privateKeyStr); - PublicKey publicKey = org.keycloak.testsuite.util.KeyUtils.publicKeyFromString(publicKeyStr); + PublicKey publicKey = publicKeyStr != null? org.keycloak.testsuite.util.KeyUtils.publicKeyFromString(publicKeyStr) : null; X509Certificate cert = org.keycloak.common.util.PemUtils.decodeCertificate(certificateStr); + if (publicKey == null) { + publicKey = cert.getPublicKey(); + } binding .signatureAlgorithm(SignatureAlgorithm.RSA_SHA256) .signWith(KeyUtils.createKeyId(privateKey), privateKey, publicKey, cert) @@ -329,10 +332,13 @@ public class SamlClient { public HttpUriRequest createSamlSignedRequest(URI samlEndpoint, String relayState, Document samlRequest, String privateKeyStr, String publicKeyStr, String certificateStr) { try { BaseSAML2BindingBuilder binding = new BaseSAML2BindingBuilder().relayState(relayState); - if (privateKeyStr != null && publicKeyStr != null) { + if (privateKeyStr != null && (publicKeyStr != null || certificateStr != null)) { PrivateKey privateKey = org.keycloak.testsuite.util.KeyUtils.privateKeyFromString(privateKeyStr); - PublicKey publicKey = org.keycloak.testsuite.util.KeyUtils.publicKeyFromString(publicKeyStr); + PublicKey publicKey = publicKeyStr != null? org.keycloak.testsuite.util.KeyUtils.publicKeyFromString(publicKeyStr) : null; X509Certificate cert = org.keycloak.common.util.PemUtils.decodeCertificate(certificateStr); + if (publicKey == null) { + publicKey = cert.getPublicKey(); + } binding.signatureAlgorithm(SignatureAlgorithm.RSA_SHA256) .signWith(KeyUtils.createKeyId(privateKey), privateKey, publicKey, cert) .signDocument(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/policies/AbstractClientPoliciesTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/policies/AbstractClientPoliciesTest.java index 7de341ac2e..dc9d2f452d 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/policies/AbstractClientPoliciesTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/policies/AbstractClientPoliciesTest.java @@ -204,6 +204,7 @@ public abstract class AbstractClientPoliciesTest extends AbstractKeycloakTest { protected static final String FAPI2_MESSAGE_SIGNING_PROFILE_NAME = "fapi-2-message-signing"; protected static final String OAUTH2_1_CONFIDENTIAL_CLIENT_PROFILE_NAME = "oauth-2-1-for-confidential-client"; protected static final String OAUTH2_1_PUBLIC_CLIENT_PROFILE_NAME = "oauth-2-1-for-public-client"; + protected static final String SAML_SECURITY_PROFILE_NAME = "saml-security-profile"; protected static final String ERR_MSG_MISSING_NONCE = "Missing parameter: nonce"; protected static final String ERR_MSG_MISSING_STATE = "Missing parameter: state"; @@ -338,7 +339,7 @@ public abstract class AbstractClientPoliciesTest extends AbstractKeycloakTest { ClientProfilesRepresentation actualProfilesRep = getProfilesWithGlobals(); // same profiles - assertExpectedProfiles(actualProfilesRep, Arrays.asList(FAPI1_BASELINE_PROFILE_NAME, FAPI1_ADVANCED_PROFILE_NAME, FAPI_CIBA_PROFILE_NAME, FAPI2_SECURITY_PROFILE_NAME, FAPI2_MESSAGE_SIGNING_PROFILE_NAME, OAUTH2_1_CONFIDENTIAL_CLIENT_PROFILE_NAME, OAUTH2_1_PUBLIC_CLIENT_PROFILE_NAME), Arrays.asList("ordinal-test-profile", "lack-of-builtin-field-test-profile")); + assertExpectedProfiles(actualProfilesRep, Arrays.asList(FAPI1_BASELINE_PROFILE_NAME, FAPI1_ADVANCED_PROFILE_NAME, FAPI_CIBA_PROFILE_NAME, FAPI2_SECURITY_PROFILE_NAME, FAPI2_MESSAGE_SIGNING_PROFILE_NAME, OAUTH2_1_CONFIDENTIAL_CLIENT_PROFILE_NAME, OAUTH2_1_PUBLIC_CLIENT_PROFILE_NAME, SAML_SECURITY_PROFILE_NAME), Arrays.asList("ordinal-test-profile", "lack-of-builtin-field-test-profile")); // each profile - fapi-1-baseline ClientProfileRepresentation actualProfileRep = getProfileRepresentation(actualProfilesRep, FAPI1_BASELINE_PROFILE_NAME, true); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/policies/ClientPoliciesLoadUpdateTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/policies/ClientPoliciesLoadUpdateTest.java index fc943142b4..7aaabeed8b 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/policies/ClientPoliciesLoadUpdateTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/policies/ClientPoliciesLoadUpdateTest.java @@ -84,7 +84,7 @@ public class ClientPoliciesLoadUpdateTest extends AbstractClientPoliciesTest { ClientProfilesRepresentation actualProfilesRep = getProfilesWithGlobals(); // same profiles - assertExpectedProfiles(actualProfilesRep, Arrays.asList(FAPI1_BASELINE_PROFILE_NAME, FAPI1_ADVANCED_PROFILE_NAME, FAPI_CIBA_PROFILE_NAME, FAPI2_SECURITY_PROFILE_NAME, FAPI2_MESSAGE_SIGNING_PROFILE_NAME, OAUTH2_1_CONFIDENTIAL_CLIENT_PROFILE_NAME, OAUTH2_1_PUBLIC_CLIENT_PROFILE_NAME), Collections.emptyList()); + assertExpectedProfiles(actualProfilesRep, Arrays.asList(FAPI1_BASELINE_PROFILE_NAME, FAPI1_ADVANCED_PROFILE_NAME, FAPI_CIBA_PROFILE_NAME, FAPI2_SECURITY_PROFILE_NAME, FAPI2_MESSAGE_SIGNING_PROFILE_NAME, OAUTH2_1_CONFIDENTIAL_CLIENT_PROFILE_NAME, OAUTH2_1_PUBLIC_CLIENT_PROFILE_NAME, SAML_SECURITY_PROFILE_NAME), Collections.emptyList()); // each profile - fapi-1-baseline ClientProfileRepresentation actualProfileRep = getProfileRepresentation(actualProfilesRep, FAPI1_BASELINE_PROFILE_NAME, true); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/policies/SamlClientPoliciesExecutorTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/policies/SamlClientPoliciesExecutorTest.java new file mode 100644 index 0000000000..e52a434fbc --- /dev/null +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/policies/SamlClientPoliciesExecutorTest.java @@ -0,0 +1,458 @@ +/* + * Copyright 2023 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.testsuite.client.policies; + +import jakarta.ws.rs.ClientErrorException; +import jakarta.ws.rs.core.Response; +import jakarta.ws.rs.core.UriBuilder; +import java.io.IOException; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.function.Function; +import org.apache.http.util.EntityUtils; +import org.hamcrest.MatcherAssert; +import org.hamcrest.Matchers; +import org.junit.Assert; +import org.junit.Test; +import org.keycloak.OAuthErrorException; +import org.keycloak.admin.client.resource.RealmResource; +import org.keycloak.models.Constants; +import org.keycloak.protocol.saml.SamlConfigAttributes; +import org.keycloak.protocol.saml.SamlProtocol; +import org.keycloak.representations.idm.ClientPoliciesRepresentation; +import org.keycloak.representations.idm.ClientPolicyExecutorConfigurationRepresentation; +import org.keycloak.representations.idm.ClientPolicyRepresentation; +import org.keycloak.representations.idm.ClientProfileRepresentation; +import org.keycloak.representations.idm.ClientProfilesRepresentation; +import org.keycloak.representations.idm.ClientRepresentation; +import org.keycloak.representations.idm.KeysMetadataRepresentation; +import org.keycloak.representations.idm.KeysMetadataRepresentation.KeyMetadataRepresentation; +import org.keycloak.representations.idm.OAuth2ErrorRepresentation; +import org.keycloak.representations.idm.RealmRepresentation; +import org.keycloak.saml.common.constants.JBossSAMLURIConstants; +import org.keycloak.saml.processing.core.saml.v2.common.SAMLDocumentHolder; +import org.keycloak.services.clientpolicy.condition.ClientProtocolCondition; +import org.keycloak.services.clientpolicy.condition.ClientProtocolConditionFactory; +import org.keycloak.services.clientpolicy.executor.SamlAvoidRedirectBindingExecutorFactory; +import org.keycloak.services.clientpolicy.executor.SamlSecureClientUrisExecutor; +import org.keycloak.services.clientpolicy.executor.SamlSecureClientUrisExecutorFactory; +import org.keycloak.services.clientpolicy.executor.SamlSignatureEnforcerExecutorFactory; +import org.keycloak.services.resources.RealmsResource; +import org.keycloak.testsuite.AbstractTestRealmKeycloakTest; +import org.keycloak.testsuite.util.ClientPoliciesUtil; +import org.keycloak.testsuite.util.SamlClient; +import org.keycloak.testsuite.util.SamlClientBuilder; + +/** + * + * @author rmartinc + */ +public class SamlClientPoliciesExecutorTest extends AbstractTestRealmKeycloakTest { + + private static final String PROFILE_POLICY_NAME = "test"; + private static final String PRIVATE_KEY = "MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQC7qZGaTn+o0pWr" + + "MS09ZVWiOPY1tKzqC+Zuvj9j4C46oNQOi1iSM0CRhhk9UUimYltnNsKoJzduk5hS" + + "02/0rYhPGwH1AENUQzpmHyBii1u3Ywi6rwb+wPY7EsDzSF7lmwiDAlVJtVhuif+J" + + "UFNlmP29wWFVaaM1JAK7wTgiOSoxeFqQPBwL4d+kjKm4S9LV6pOs798WEwvBJKtB" + + "bve/ZvQHmpbt3ftbBegaXQJUzsxr7xqo+DKI6RPAprn0v0W8pGHiMQ/WoTtan5ka" + + "R3LFSShtx3lX1WVNnT+0wBZcLEkQzaB1R6prwYlSQ6JaydjUqstInrgU2WUVwXJi" + + "MNR0ulB7AgMBAAECggEANfV/5DqGAmjqmBq/w1OL1+VBBhg5T+K0E0uotnMTV9A+" + + "qR/wC7mo6y7/ut3QYecOGRNpzSfZjHXr6oTZQCVVeElvup6kvWnHNO3mRe+EI6ra" + + "K7N/82hQZJPz3wAEKUj2nZTiKRt3nfEYBMeP8zqWWyVrcz+4qeL81jesiEqfkzFl" + + "gefsbBwYz7odNHyvYOYklrudKpfQQDff9zKEpPh9ou6TP5cHyPXNxN2HxSKjAsxN" + + "OO/zxrtJeuP1pLMhLJQZuQyAuckdkAy4Cv8K8x+r6/8PxIFjz1+6+I4uolnsSpS7" + + "+upOK+866XUIE6h6hlLTEp+XXds+/LFZaT+B/vBTwQKBgQDKCxNuwrRAD+JifB0t" + + "hvnew8kwW48dT0ZM2m3Fw85Vey2hVStZoR6tYuxtoXLBpJCjGpeFhxm1BPV38SNr" + + "78W98NduS57uU5iueByKY/twDws/AFSHpsLyMiWJBBwH8VRm0U4GmFxBlNCrU3TP" + + "ddJxAgy5XYSP+LG7LXr0jZ1oSQKBgQDtx1OhAR14urfnuQgsmH9G6bEXFmR71uL8" + + "OJBU0n/AB8bPQbisDRPcSkdF7KgQI5hJE+5+8aERmAZ1B//5wbZlmR/lmd+V/c/6" + + "BAxJkQicjVG5EhgliM73z4jm/85pYfkN9IgbZlB7vCVKfKWKSIwJ/pY039WN+2t4" + + "BsenhJ2aowKBgBnJCBXeq3pxjIbdKCwjSchwXEDbrowjDenJBrFyp+ao7c3lPL8X" + + "nP6r3ViwfiDQi9UFE8lq0JEVrO49zDN+SlJPZm8hH4tzB81cbugKkpBemyTTOfaG" + + "BeM7Gyc9awZoekkU9UxKLZwBDhCPehzwAIeDp3QQx1ZIewZUa5jCahBhAoGAd/Yg" + + "UxJk9Av/zIClhxpI3FX6alN5zqDTU7yV1LV+jjteKiJWMTdH1dQDsVt8TugmZHgR" + + "0ynEwUOZvmGS20bH5uoiFYxUKTAsRU7VhCgP2CvUFzLxy74B7TRfNWvJj5FGPawp" + + "Hum3oTWC+tl4CxQe0swGrBZhf4hg5+VDxVg6y1ECgYEAns+tbdeBJWV/r3Rh3e0C" + + "LetywsgNG02aIuZpIlT4VWKV5cIIq6d20C6I/EKhAZ0E56D8xX6xVmUBcY6Qv1zd" + + "7yOVwITM3P64G+ZtPkm3m3w7XRnaIEUHuwFWpdMPjfdipjelq0ltbRQMOiqYAQYR" + + "jfH4O0lZc8bo2TVGeQHpyg4="; + private static final String CERTIFICATE = "MIICyDCCAbCgAwIBAgIJAIiYjotcPTEkMA0GCSqGSIb3DQEBCwUAMBExDzANBgNV" + + "BAMTBmNsaWVudDAgFw0yNDAxMjYxMzM5MjNaGA8yMDc4MTAyOTEzMzkyM1owETEP" + + "MA0GA1UEAxMGY2xpZW50MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA" + + "u6mRmk5/qNKVqzEtPWVVojj2NbSs6gvmbr4/Y+AuOqDUDotYkjNAkYYZPVFIpmJb" + + "ZzbCqCc3bpOYUtNv9K2ITxsB9QBDVEM6Zh8gYotbt2MIuq8G/sD2OxLA80he5ZsI" + + "gwJVSbVYbon/iVBTZZj9vcFhVWmjNSQCu8E4IjkqMXhakDwcC+HfpIypuEvS1eqT" + + "rO/fFhMLwSSrQW73v2b0B5qW7d37WwXoGl0CVM7Ma+8aqPgyiOkTwKa59L9FvKRh" + + "4jEP1qE7Wp+ZGkdyxUkobcd5V9VlTZ0/tMAWXCxJEM2gdUeqa8GJUkOiWsnY1KrL" + + "SJ64FNllFcFyYjDUdLpQewIDAQABoyEwHzAdBgNVHQ4EFgQUplMyjmtmloAy8sTA" + + "CENFZugti98wDQYJKoZIhvcNAQELBQADggEBACXxwe1HJ0j56SgGueNSzfoUXwI4" + + "a0XUN73I3zuXOwBoSqJr7X17B0ZDrHAb+k1WOz1iIz6OA2Bi1p8rtYqn/rLAdCbQ" + + "fatlSzVrVkxc689LEOFiN9eGlfBpqX/VllY9DPzmMoPLa1v0Ya/AXIQlyURbe3Ve" + + "PHdhS8lScQi239FtSq1pKlRRzBsfTNwD7MbgY2kGPSKBqe9TuYqYTjc4r0XmjVO2" + + "ZI3mUuNOSpBrH2YY5umutjH4ZTJstzf82kp1m+/wsNM46ZvV4DCHxNUESONzZteW" + + "+9OgpVwAt9ltqlX6qFxq04S0pAA2AyLnDvMuIUgtdNn7jFCwqYCePnDWJfY="; + + @Override + public void configureTestRealm(RealmRepresentation testRealm) { + // no-op + } + + @Test + public void testSamlSecureClientUrisExecutor() throws Exception { + createClientProfileAndPolicyToTest(SamlSecureClientUrisExecutorFactory.PROVIDER_ID, null); + final RealmResource realm = testRealm(); + String clientId = null; + try { + final ClientRepresentation client = createSecureClient("test-saml-client"); + // test creation fails for non-https urls + testSamlSecureClient(client, c -> realm.clients().create(c)); + // create it + testClientOperation(client, null, c -> realm.clients().create(c)); + clientId = realm.clients().findByClientId(client.getClientId()).iterator().next().getId(); + client.setId(clientId); + // test update fails for non-https urls + testSamlSecureClient(client, c -> updateClient(realm, c)); + // test wildcard redirects are not valid + testRedirectUrisWildcard(client, c -> updateClient(realm, c)); + // test a login with https is valid + testAuthenticationPostSuccess(client); + // test a login with http is invalid + testAuthenticationPostError(client, "http://client.keycloak.org/saml/"); + } finally { + removeClientProfileAndPolicyToTest(); + if (clientId != null) { + realm.clients().get(clientId).remove(); + } + } + } + + @Test + public void testSamlAvoidRedirectBindingExecutor() throws Exception { + String clientId = null; + final RealmResource realm = testRealm(); + try { + final ClientRepresentation client = createSecureClient("test-saml-client"); + client.getAttributes().put(SamlConfigAttributes.SAML_FORCE_POST_BINDING, Boolean.FALSE.toString()); + + // create a client without the executor enabled + testClientOperation(client, null, c -> realm.clients().create(c)); + clientId = realm.clients().findByClientId(client.getClientId()).iterator().next().getId(); + + // enable the policy and check redirect is not allowed + createClientProfileAndPolicyToTest(SamlAvoidRedirectBindingExecutorFactory.PROVIDER_ID, null); + testAuthenticationRedirectPostError(client); + realm.clients().get(clientId).remove(); + + // test creation without signature fails + testClientOperation(client, "Force POST binding is not enabled", c -> realm.clients().create(c)); + // put force post binding + client.getAttributes().put(SamlConfigAttributes.SAML_FORCE_POST_BINDING, Boolean.TRUE.toString()); + testClientOperation(client, null, c -> realm.clients().create(c)); + // update without post binding fails + clientId = realm.clients().findByClientId(client.getClientId()).iterator().next().getId(); + client.setId(clientId); + client.getAttributes().put(SamlConfigAttributes.SAML_FORCE_POST_BINDING, Boolean.FALSE.toString()); + testClientOperation(client, "Force POST binding is not enabled", c -> updateClient(realm, client)); + // with force works OK + client.getAttributes().put(SamlConfigAttributes.SAML_FORCE_POST_BINDING, Boolean.TRUE.toString()); + testClientOperation(client, null, c -> updateClient(realm, client)); + // test a REDIRECT is forced to POST OK + testAuthenticationRedirectPostSuccess(client); + } finally { + removeClientProfileAndPolicyToTest(); + if (clientId != null) { + realm.clients().get(clientId).remove(); + } + } + } + + @Test + public void testSamlSignatureEnforcerExecutor() throws Exception { + String clientId = null; + final RealmResource realm = testRealm(); + try { + final ClientRepresentation client = createSecureClient("test-saml-client"); + client.getAttributes().put(SamlConfigAttributes.SAML_CLIENT_SIGNATURE_ATTRIBUTE, Boolean.FALSE.toString()); + + // create a client without client signature + testClientOperation(client, null, c -> realm.clients().create(c)); + clientId = realm.clients().findByClientId(client.getClientId()).iterator().next().getId(); + + // enable the policy and check login without signature is not allowed + createClientProfileAndPolicyToTest(SamlSignatureEnforcerExecutorFactory.PROVIDER_ID, null); + testAuthenticationPostError(client, client.getAdminUrl()); + realm.clients().get(clientId).remove(); + + // test creation + testSamlAttributeOperation(client, "Signatures not ensured for the client.", + SamlConfigAttributes.SAML_CLIENT_SIGNATURE_ATTRIBUTE, + Boolean.FALSE.toString(), Boolean.TRUE.toString(), c -> realm.clients().create(c)); + testSamlAttributeOperation(client, "Signatures not ensured for the client.", + SamlConfigAttributes.SAML_SERVER_SIGNATURE, + Boolean.FALSE.toString(), Boolean.FALSE.toString(), c -> realm.clients().create(c)); + testSamlAttributeOperation(client, "Signatures not ensured for the client.", + SamlConfigAttributes.SAML_ASSERTION_SIGNATURE, + Boolean.FALSE.toString(), Boolean.FALSE.toString(), c -> realm.clients().create(c)); + testSamlAttributeOperation(client, null, + SamlConfigAttributes.SAML_SERVER_SIGNATURE, + Boolean.TRUE.toString(), Boolean.TRUE.toString(), c -> realm.clients().create(c)); + clientId = realm.clients().findByClientId(client.getClientId()).iterator().next().getId(); + client.setId(clientId); + + // test update + testSamlAttributeOperation(client, "Signatures not ensured for the client.", + SamlConfigAttributes.SAML_CLIENT_SIGNATURE_ATTRIBUTE, + Boolean.FALSE.toString(), Boolean.TRUE.toString(), c -> updateClient(realm, c)); + testSamlAttributeOperation(client, "Signatures not ensured for the client.", + SamlConfigAttributes.SAML_SERVER_SIGNATURE, + Boolean.FALSE.toString(), Boolean.FALSE.toString(), c -> updateClient(realm, c)); + testSamlAttributeOperation(client, "Signatures not ensured for the client.", + SamlConfigAttributes.SAML_ASSERTION_SIGNATURE, + Boolean.FALSE.toString(), Boolean.FALSE.toString(), c -> updateClient(realm, c)); + + // test login is OK with signatures + client.getAttributes().put(SamlConfigAttributes.SAML_CLIENT_SIGNATURE_ATTRIBUTE, Boolean.TRUE.toString()); + client.getAttributes().put(SamlConfigAttributes.SAML_SERVER_SIGNATURE, Boolean.TRUE.toString()); + client.getAttributes().put(SamlConfigAttributes.SAML_ASSERTION_SIGNATURE, Boolean.FALSE.toString()); + client.getAttributes().put(SamlConfigAttributes.SAML_SIGNING_CERTIFICATE_ATTRIBUTE, CERTIFICATE); + testClientOperation(client, null, c -> updateClient(realm, c)); + + testAuthenticationPostSignatureSuccess(realm, client, PRIVATE_KEY, CERTIFICATE); + } finally { + removeClientProfileAndPolicyToTest(); + if (clientId != null) { + realm.clients().get(clientId).remove(); + } + } + } + + private Response updateClient(RealmResource realm, ClientRepresentation client) { + try { + realm.clients().get(client.getId()).update(client); + return null; + } catch (ClientErrorException e) { + return e.getResponse(); + } + } + + private void testAuthenticationPostSuccess(ClientRepresentation client) { + new SamlClientBuilder() + .authnRequest(RealmsResource.protocolUrl(UriBuilder.fromUri(getAuthServerRoot())).build(TEST_REALM_NAME, SamlProtocol.LOGIN_PROTOCOL), + client.getClientId(), client.getAdminUrl(), SamlClient.Binding.POST) + .build() + .login().user("test-user@localhost", "password").build() + .execute(hr -> { + try { + SAMLDocumentHolder doc = SamlClient.Binding.POST.extractResponse(hr); + MatcherAssert.assertThat(doc.getSamlObject(), + org.keycloak.testsuite.util.Matchers.isSamlStatusResponse(JBossSAMLURIConstants.STATUS_SUCCESS)); + } catch (IOException ex) { + throw new IllegalStateException(ex); + } + }); + } + + private void testAuthenticationPostSignatureSuccess(RealmResource realm, ClientRepresentation client, String privateKey, String certificate) { + KeysMetadataRepresentation keysMetadata = realm.keys().getKeyMetadata(); + String kid = keysMetadata.getActive().get(Constants.DEFAULT_SIGNATURE_ALGORITHM); + KeyMetadataRepresentation keyMetadata = keysMetadata.getKeys().stream() + .filter(k -> kid.equals(k.getKid())).findAny().orElse(null); + Assert.assertNotNull(keyMetadata); + + new SamlClientBuilder() + .authnRequest(RealmsResource.protocolUrl(UriBuilder.fromUri(getAuthServerRoot())).build(TEST_REALM_NAME, SamlProtocol.LOGIN_PROTOCOL), + client.getClientId(), client.getAdminUrl(), SamlClient.Binding.POST) + .signWith(privateKey, null, certificate) + .build() + .login().user("test-user@localhost", "password").build() + .execute(hr -> { + try { + SAMLDocumentHolder doc = SamlClient.Binding.POST.extractResponse(hr, keyMetadata.getPublicKey()); + MatcherAssert.assertThat(doc.getSamlObject(), + org.keycloak.testsuite.util.Matchers.isSamlStatusResponse(JBossSAMLURIConstants.STATUS_SUCCESS)); + } catch (IOException ex) { + throw new IllegalStateException(ex); + } + }); + } + + private void testAuthenticationPostError(ClientRepresentation client, String assertionUrl) { + new SamlClientBuilder() + .authnRequest(RealmsResource.protocolUrl(UriBuilder.fromUri(getAuthServerRoot())).build(TEST_REALM_NAME, SamlProtocol.LOGIN_PROTOCOL), + client.getClientId(), assertionUrl, SamlClient.Binding.POST) + .build() + .executeAndTransform(response -> { + Assert.assertEquals(Response.Status.BAD_REQUEST.getStatusCode(), response.getStatusLine().getStatusCode()); + MatcherAssert.assertThat(EntityUtils.toString(response.getEntity(), "UTF-8"), + Matchers.containsString("Invalid Request")); + return null; + }); + } + + private void testAuthenticationRedirectPostSuccess(ClientRepresentation client) { + new SamlClientBuilder() + .authnRequest(RealmsResource.protocolUrl(UriBuilder.fromUri(getAuthServerRoot())).build(TEST_REALM_NAME, SamlProtocol.LOGIN_PROTOCOL), + client.getClientId(), client.getAdminUrl(), SamlClient.Binding.REDIRECT) + .build() + .login().user("test-user@localhost", "password").build() + .execute(hr -> { + try { + SAMLDocumentHolder doc = SamlClient.Binding.POST.extractResponse(hr); + MatcherAssert.assertThat(doc.getSamlObject(), + org.keycloak.testsuite.util.Matchers.isSamlStatusResponse(JBossSAMLURIConstants.STATUS_SUCCESS)); + } catch (IOException ex) { + throw new IllegalStateException(ex); + } + }); + } + + private void testAuthenticationRedirectPostError(ClientRepresentation client) { + new SamlClientBuilder() + .authnRequest(RealmsResource.protocolUrl(UriBuilder.fromUri(getAuthServerRoot())).build(TEST_REALM_NAME, SamlProtocol.LOGIN_PROTOCOL), + client.getClientId(), client.getAdminUrl(), SamlClient.Binding.REDIRECT) + .build() + .executeAndTransform(response -> { + Assert.assertEquals(Response.Status.BAD_REQUEST.getStatusCode(), response.getStatusLine().getStatusCode()); + MatcherAssert.assertThat(EntityUtils.toString(response.getEntity(), "UTF-8"), + Matchers.containsString("Invalid Request")); + return null; + }); + } + + private void testClientOperation(ClientRepresentation client, String errorPrefix, Function operation) { + try (Response response = operation.apply(client)) { + if (errorPrefix == null) { + if (response != null) { + // create returns 201, update returns null + Assert.assertEquals(Response.Status.CREATED.getStatusCode(), response.getStatus()); + } + } else { + Assert.assertEquals(Response.Status.BAD_REQUEST.getStatusCode(), response.getStatus()); + OAuth2ErrorRepresentation error = response.readEntity(OAuth2ErrorRepresentation.class); + Assert.assertEquals(OAuthErrorException.INVALID_CLIENT_METADATA, error.getError()); + MatcherAssert.assertThat(error.getErrorDescription(), Matchers.startsWith(errorPrefix)); + } + } + } + + private void testSamlAttributeOperation(ClientRepresentation client, String error, String attr, + String beforeValue, String afterValue, Function operation) { + client.getAttributes().put(attr, beforeValue); + testClientOperation(client, error, operation); + client.getAttributes().put(attr, afterValue); + } + + private void testSamlSecureClientAttribute(ClientRepresentation client, + String attr, Function operation) { + testSamlAttributeOperation(client, "Non secure scheme for ", attr, "http://client.keycloak.org/saml/", + "https://client.keycloak.org/saml/", operation); + } + + private void testRedirectUrisWildcard(ClientRepresentation client, Function operation) + throws Exception { + // wildcards allowed + createClientProfileAndPolicyToTest(SamlSecureClientUrisExecutorFactory.PROVIDER_ID, + new SamlSecureClientUrisExecutor.Configuration(true)); + client.getRedirectUris().add("https://client.keycloak.org/saml/*"); + testClientOperation(client, null, operation); + + // wildcards disallowed + createClientProfileAndPolicyToTest(SamlSecureClientUrisExecutorFactory.PROVIDER_ID, + new SamlSecureClientUrisExecutor.Configuration(false)); + testClientOperation(client, "Unsecure wildcard redirect ", operation); + client.getRedirectUris().remove("https://client.keycloak.org/saml/*"); + testClientOperation(client, null, operation); + } + + private void testSamlSecureClient(ClientRepresentation client, Function operation) { + // rootUrl + client.setRootUrl("http://client.keycloak.org/saml/"); + testClientOperation(client, "Non secure scheme for ", operation); + client.setRootUrl("https://client.keycloak.org/saml/"); + + // baseUrl + client.setBaseUrl("http://client.keycloak.org/saml/"); + testClientOperation(client, "Non secure scheme for ", operation); + client.setBaseUrl("https://client.keycloak.org/saml/"); + + // adminUrl + client.setAdminUrl("http://client.keycloak.org/saml/"); + testClientOperation(client, "Non secure scheme for ", operation); + client.setAdminUrl("https://client.keycloak.org/saml/"); + + // redirect URIs + client.getRedirectUris().add("http://client.keycloak.org/saml/"); + testClientOperation(client, "Non secure scheme for ", operation); + client.getRedirectUris().remove("http://client.keycloak.org/saml/"); + + // test saml specific protocol urls + testSamlSecureClientAttribute(client, SamlProtocol.SAML_ASSERTION_CONSUMER_URL_POST_ATTRIBUTE, operation); + testSamlSecureClientAttribute(client, SamlProtocol.SAML_ASSERTION_CONSUMER_URL_REDIRECT_ATTRIBUTE, operation); + testSamlSecureClientAttribute(client, SamlProtocol.SAML_ASSERTION_CONSUMER_URL_ARTIFACT_ATTRIBUTE, operation); + testSamlSecureClientAttribute(client, SamlProtocol.SAML_SINGLE_LOGOUT_SERVICE_URL_POST_ATTRIBUTE, operation); + testSamlSecureClientAttribute(client, SamlProtocol.SAML_SINGLE_LOGOUT_SERVICE_URL_ARTIFACT_ATTRIBUTE, operation); + testSamlSecureClientAttribute(client, SamlProtocol.SAML_SINGLE_LOGOUT_SERVICE_URL_REDIRECT_ATTRIBUTE, operation); + testSamlSecureClientAttribute(client, SamlProtocol.SAML_SINGLE_LOGOUT_SERVICE_URL_SOAP_ATTRIBUTE, operation); + testSamlSecureClientAttribute(client, SamlProtocol.SAML_ARTIFACT_RESOLUTION_SERVICE_URL_ATTRIBUTE, operation); + } + + private void createClientProfileAndPolicyToTest(String executorId, ClientPolicyExecutorConfigurationRepresentation config) throws Exception { + RealmResource realm = testRealm(); + ClientProfileRepresentation profile = new ClientPoliciesUtil.ClientProfileBuilder() + .createProfile(PROFILE_POLICY_NAME, "The profile to test") + .addExecutor(executorId, config) + .toRepresentation(); + ClientProfilesRepresentation profiles = new ClientPoliciesUtil.ClientProfilesBuilder() + .addProfile(profile) + .toRepresentation(); + realm.clientPoliciesProfilesResource().updateProfiles(profiles); + + ClientPolicyRepresentation policy = new ClientPoliciesUtil.ClientPolicyBuilder() + .createPolicy(PROFILE_POLICY_NAME, "The policy to test.", Boolean.TRUE) + .addCondition(ClientProtocolConditionFactory.PROVIDER_ID, new ClientProtocolCondition.Configuration(SamlProtocol.LOGIN_PROTOCOL)) + .addProfile(PROFILE_POLICY_NAME) + .toRepresentation(); + ClientPoliciesRepresentation policies = new ClientPoliciesUtil.ClientPoliciesBuilder() + .addPolicy(policy) + .toRepresentation(); + realm.clientPoliciesPoliciesResource().updatePolicies(policies); + } + + private void removeClientProfileAndPolicyToTest() { + ClientProfilesRepresentation profiles = new ClientPoliciesUtil.ClientProfilesBuilder().toRepresentation(); + adminClient.realm(TEST_REALM_NAME).clientPoliciesProfilesResource().updateProfiles(profiles); + ClientPoliciesRepresentation policies = new ClientPoliciesUtil.ClientPoliciesBuilder().toRepresentation(); + adminClient.realm(TEST_REALM_NAME).clientPoliciesPoliciesResource().updatePolicies(policies); + } + + private ClientRepresentation createSecureClient(String clientId) { + ClientRepresentation client = new ClientRepresentation(); + client.setName(clientId); + client.setClientId(clientId); + client.setProtocol(SamlProtocol.LOGIN_PROTOCOL); + client.setRootUrl("https://client.keycloak.org/saml/"); + client.setAdminUrl("https://client.keycloak.org/saml/"); + client.setBaseUrl("https://client.keycloak.org/saml/"); + List redirectUris = new ArrayList<>(); + redirectUris.add("https://client.keycloak.org/saml/"); + client.setRedirectUris(redirectUris); + client.setAttributes(new HashMap<>()); + client.getAttributes().put(SamlConfigAttributes.SAML_CLIENT_SIGNATURE_ATTRIBUTE, Boolean.FALSE.toString()); + return client; + } +} \ No newline at end of file diff --git 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 index 1859e8a5f6..24a2012c24 100644 --- 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 @@ -21,7 +21,6 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; -import org.keycloak.admin.client.resource.ClientResource; import org.keycloak.common.util.Base64Url; import org.keycloak.crypto.AsymmetricSignatureSignerContext; import org.keycloak.crypto.ECDSASignatureSignerContext; @@ -34,7 +33,6 @@ import org.keycloak.jose.jwk.ECPublicJWK; import org.keycloak.jose.jwk.JWK; import org.keycloak.jose.jwk.RSAPublicJWK; import org.keycloak.jose.jws.JWSHeader; -import org.keycloak.protocol.oidc.OIDCAdvancedConfigWrapper; import org.keycloak.protocol.oidc.grants.ciba.clientpolicy.executor.SecureCibaAuthenticationRequestSigningAlgorithmExecutor; import org.keycloak.representations.dpop.DPoP; import org.keycloak.representations.idm.ClientPoliciesRepresentation; @@ -45,7 +43,6 @@ import org.keycloak.representations.idm.ClientPolicyExecutorRepresentation; import org.keycloak.representations.idm.ClientPolicyRepresentation; import org.keycloak.representations.idm.ClientProfileRepresentation; import org.keycloak.representations.idm.ClientProfilesRepresentation; -import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.services.clientpolicy.ClientPolicyEvent; import org.keycloak.services.clientpolicy.condition.ClientAccessTypeCondition; import org.keycloak.services.clientpolicy.condition.ClientRolesCondition; @@ -336,6 +333,9 @@ public final class ClientPoliciesUtil { } public ClientPolicyBuilder addCondition(String providerId, ClientPolicyConditionConfigurationRepresentation config) throws Exception { + if (config == null) { + config = new ClientPolicyConditionConfigurationRepresentation(); + } ClientPolicyConditionRepresentation condition = new ClientPolicyConditionRepresentation(); condition.setConditionProviderId(providerId); condition.setConfiguration(JsonSerialization.mapper.readValue(JsonSerialization.mapper.writeValueAsBytes(config), JsonNode.class));