From 4dcb69596b651c3302229977f16dfb61c84b8609 Mon Sep 17 00:00:00 2001 From: Michal Hajas Date: Wed, 26 May 2021 15:29:15 +0200 Subject: [PATCH] KEYCLOAK-18146 Search for clients by client attribute when doing saml artifact resolution --- ...te14_0_0_MigrateSamlArtifactAttribute.java | 84 +++++++++++++++++++ .../models/jpa/JpaClientProviderFactory.java | 4 +- .../META-INF/jpa-changelog-14.0.0.xml | 4 + .../models/utils/RepresentationToModel.java | 13 +++ .../protocol/saml/ArtifactResolver.java | 7 +- .../saml/util/ArtifactBindingUtils.java | 51 +++++++++++ .../saml/DefaultSamlArtifactResolver.java | 28 +++---- .../keycloak/protocol/saml/SamlClient.java | 9 ++ .../protocol/saml/SamlConfigAttributes.java | 1 + .../protocol/saml/SamlProtocolFactory.java | 2 + .../keycloak/protocol/saml/SamlService.java | 4 +- .../protocol/util/ArtifactBindingUtils.java | 15 ---- .../services/managers/ClientManager.java | 13 ++- .../resources/admin/ClientResource.java | 2 +- .../CustomTestingSamlArtifactResolver.java | 4 +- ...tomTestingSamlArtifactResolverFactory.java | 2 +- .../client/SAMLClientRegistrationTest.java | 5 +- .../migration/AbstractMigrationTest.java | 22 ++++- .../testsuite/saml/ArtifactBindingTest.java | 36 ++++++++ .../migration-realm-1.9.8.Final.json | 17 +++- .../migration-realm-2.5.5.Final.json | 17 +++- .../migration-realm-3.4.3.Final.json | 17 +++- .../migration-realm-4.8.3.Final.json | 17 +++- .../migration-test/migration-realm-9.0.3.json | 17 +++- .../console/clients/AbstractClientTest.java | 5 +- .../console/clients/ClientSettingsTest.java | 20 +++++ 26 files changed, 364 insertions(+), 52 deletions(-) create mode 100644 model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/custom/JpaUpdate14_0_0_MigrateSamlArtifactAttribute.java create mode 100644 server-spi-private/src/main/java/org/keycloak/protocol/saml/util/ArtifactBindingUtils.java delete mode 100644 services/src/main/java/org/keycloak/protocol/util/ArtifactBindingUtils.java diff --git a/model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/custom/JpaUpdate14_0_0_MigrateSamlArtifactAttribute.java b/model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/custom/JpaUpdate14_0_0_MigrateSamlArtifactAttribute.java new file mode 100644 index 0000000000..9b5afbc349 --- /dev/null +++ b/model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/custom/JpaUpdate14_0_0_MigrateSamlArtifactAttribute.java @@ -0,0 +1,84 @@ +/* + * Copyright 2020 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.connections.jpa.updater.liquibase.custom; + +import liquibase.exception.CustomChangeException; +import liquibase.statement.core.InsertStatement; +import liquibase.structure.core.Table; + +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.util.HashMap; +import java.util.Map; + +import static org.keycloak.protocol.saml.util.ArtifactBindingUtils.computeArtifactBindingIdentifierString; + +public class JpaUpdate14_0_0_MigrateSamlArtifactAttribute extends CustomKeycloakTask { + + private static final String SAML_ARTIFACT_BINDING_IDENTIFIER = "saml.artifact.binding.identifier"; + + private final Map clientIds = new HashMap<>(); + + @Override + protected void generateStatementsImpl() throws CustomChangeException { + extractClientsData("SELECT C.ID, C.CLIENT_ID FROM " + getTableName("CLIENT") + " C " + + "LEFT JOIN " + getTableName("CLIENT_ATTRIBUTES") + " CA " + + "ON C.ID = CA.CLIENT_ID AND CA.NAME='" + SAML_ARTIFACT_BINDING_IDENTIFIER + "' " + + "WHERE C.PROTOCOL='saml' AND CA.NAME IS NULL"); + + for (Map.Entry clientPair : clientIds.entrySet()) { + String id = clientPair.getKey(); + + String clientId = clientPair.getValue(); + String samlIdentifier = computeArtifactBindingIdentifierString(clientId); + + statements.add( + new InsertStatement(null, null, database.correctObjectName("CLIENT_ATTRIBUTES", Table.class)) + .addColumnValue("CLIENT_ID", id) + .addColumnValue("NAME", SAML_ARTIFACT_BINDING_IDENTIFIER) + .addColumnValue("VALUE", samlIdentifier) + ); + } + } + + private void extractClientsData(String sql) throws CustomChangeException { + try (PreparedStatement statement = jdbcConnection.prepareStatement(sql); + ResultSet rs = statement.executeQuery()) { + + while (rs.next()) { + String id = rs.getString(1); + String clientId = rs.getString(2); + + if (id == null || id.trim().isEmpty() + || clientId == null || clientId.trim().isEmpty()) { + continue; + } + + clientIds.put(id, clientId); + } + + } catch (Exception e) { + throw new CustomChangeException(getTaskId() + ": Exception when extracting data from previous version", e); + } + } + + @Override + protected String getTaskId() { + return "Migrate Saml attributes (14.0.0)"; + } + +} diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/JpaClientProviderFactory.java b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaClientProviderFactory.java index cf2cd44f9d..b6b43206f0 100644 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/JpaClientProviderFactory.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaClientProviderFactory.java @@ -23,6 +23,7 @@ import org.keycloak.models.ClientProvider; import org.keycloak.models.ClientProviderFactory; import org.keycloak.models.KeycloakSession; import org.keycloak.models.KeycloakSessionFactory; +import org.keycloak.protocol.saml.SamlConfigAttributes; import javax.persistence.EntityManager; import java.util.Arrays; @@ -38,7 +39,8 @@ public class JpaClientProviderFactory implements ClientProviderFactory { private Set clientSearchableAttributes = null; private static final List REQUIRED_SEARCHABLE_ATTRIBUTES = Arrays.asList( - "saml_idp_initiated_sso_url_name" + "saml_idp_initiated_sso_url_name", + SamlConfigAttributes.SAML_ARTIFACT_BINDING_IDENTIFIER ); @Override diff --git a/model/jpa/src/main/resources/META-INF/jpa-changelog-14.0.0.xml b/model/jpa/src/main/resources/META-INF/jpa-changelog-14.0.0.xml index 0ae5b2356c..e2b8795f6d 100644 --- a/model/jpa/src/main/resources/META-INF/jpa-changelog-14.0.0.xml +++ b/model/jpa/src/main/resources/META-INF/jpa-changelog-14.0.0.xml @@ -71,4 +71,8 @@ + + + + diff --git a/server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java b/server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java index cbb2b48cdb..8faaf69e66 100755 --- a/server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java +++ b/server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java @@ -141,6 +141,8 @@ import org.keycloak.storage.federated.UserFederatedStorageProvider; import org.keycloak.util.JsonSerialization; import org.keycloak.validation.ValidationUtil; +import static org.keycloak.protocol.saml.util.ArtifactBindingUtils.computeArtifactBindingIdentifierString; + public class RepresentationToModel { private static Logger logger = Logger.getLogger(RepresentationToModel.class); @@ -1407,6 +1409,11 @@ public class RepresentationToModel { } } + if ("saml".equals(resourceRep.getProtocol()) + && (resourceRep.getAttributes() == null + || !resourceRep.getAttributes().containsKey("saml.artifact.binding.identifier"))) { + client.setAttribute("saml.artifact.binding.identifier", computeArtifactBindingIdentifierString(resourceRep.getClientId())); + } if (resourceRep.getAuthenticationFlowBindingOverrides() != null) { for (Map.Entry entry : resourceRep.getAuthenticationFlowBindingOverrides().entrySet()) { @@ -1559,6 +1566,12 @@ public class RepresentationToModel { } } + if ("saml".equals(rep.getProtocol()) + && (rep.getAttributes() == null + || !rep.getAttributes().containsKey("saml.artifact.binding.identifier"))) { + resource.setAttribute("saml.artifact.binding.identifier", computeArtifactBindingIdentifierString(rep.getClientId())); + } + if (rep.getAuthenticationFlowBindingOverrides() != null) { for (Map.Entry entry : rep.getAuthenticationFlowBindingOverrides().entrySet()) { if (entry.getValue() == null || entry.getValue().trim().equals("")) { diff --git a/server-spi-private/src/main/java/org/keycloak/protocol/saml/ArtifactResolver.java b/server-spi-private/src/main/java/org/keycloak/protocol/saml/ArtifactResolver.java index b71f93845a..a38cfbf3a7 100644 --- a/server-spi-private/src/main/java/org/keycloak/protocol/saml/ArtifactResolver.java +++ b/server-spi-private/src/main/java/org/keycloak/protocol/saml/ArtifactResolver.java @@ -2,10 +2,9 @@ package org.keycloak.protocol.saml; import org.keycloak.models.AuthenticatedClientSessionModel; import org.keycloak.models.ClientModel; +import org.keycloak.models.KeycloakSession; import org.keycloak.provider.Provider; -import java.util.stream.Stream; - /** * Provides a way to create and resolve artifacts for SAML Artifact binding @@ -15,12 +14,12 @@ public interface ArtifactResolver extends Provider { /** * Returns client model that issued artifact * + * @param session KeycloakSession for searching for client corresponding client * @param artifact the artifact - * @param clients stream of clients, the stream will be searched for a client that issued the artifact * @return the client model that issued the artifact * @throws ArtifactResolverProcessingException When an error occurs during client search */ - ClientModel selectSourceClient(String artifact, Stream clients) throws ArtifactResolverProcessingException; + ClientModel selectSourceClient(KeycloakSession session, String artifact) throws ArtifactResolverProcessingException; /** * Creates and stores an artifact diff --git a/server-spi-private/src/main/java/org/keycloak/protocol/saml/util/ArtifactBindingUtils.java b/server-spi-private/src/main/java/org/keycloak/protocol/saml/util/ArtifactBindingUtils.java new file mode 100644 index 0000000000..11a4f994b0 --- /dev/null +++ b/server-spi-private/src/main/java/org/keycloak/protocol/saml/util/ArtifactBindingUtils.java @@ -0,0 +1,51 @@ +package org.keycloak.protocol.saml.util; + +import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.util.Base64; + +public class ArtifactBindingUtils { + public static String artifactToResolverProviderId(String artifact) { + return byteArrayToResolverProviderId(Base64.getDecoder().decode(artifact)); + } + + public static String byteArrayToResolverProviderId(byte[] ar) { + return String.format("%02X%02X", ar[0], ar[1]); + } + + /** + * Computes identifier from the given String, for example, from entityId + * + * @param identifierFrom String that will be turned into an identifier + * @return Base64 of SHA-1 hash of the identifierFrom + */ + public static String computeArtifactBindingIdentifierString(String identifierFrom) { + return Base64.getEncoder().encodeToString(computeArtifactBindingIdentifier(identifierFrom)); + } + + /** + * Turns byte representation of the identifier into readable String + * + * @param identifier byte representation of the identifier + * @return Base64 of the identifier + */ + public static String getArtifactBindingIdentifierString(byte[] identifier) { + return Base64.getEncoder().encodeToString(identifier); + } + + /** + * Computes 20 bytes long byte identifier of the given string, for example, from entityId + * + * @param identifierFrom String that will be turned into an identifier + * @return SHA-1 hash of the given identifierFrom + */ + public static byte[] computeArtifactBindingIdentifier(String identifierFrom) { + try { + MessageDigest sha1Digester = MessageDigest.getInstance("SHA-1"); + return sha1Digester.digest(identifierFrom.getBytes(StandardCharsets.UTF_8)); + } catch (NoSuchAlgorithmException e) { + throw new RuntimeException("JVM does not support required cryptography algorithms: SHA-1/SHA1PRNG.", e); + } + } +} diff --git a/services/src/main/java/org/keycloak/protocol/saml/DefaultSamlArtifactResolver.java b/services/src/main/java/org/keycloak/protocol/saml/DefaultSamlArtifactResolver.java index 6889d06522..63a8c522b7 100644 --- a/services/src/main/java/org/keycloak/protocol/saml/DefaultSamlArtifactResolver.java +++ b/services/src/main/java/org/keycloak/protocol/saml/DefaultSamlArtifactResolver.java @@ -1,22 +1,22 @@ package org.keycloak.protocol.saml; -import com.google.common.base.Charsets; import com.google.common.base.Strings; import org.jboss.logging.Logger; import org.keycloak.models.AuthenticatedClientSessionModel; import org.keycloak.models.ClientModel; +import org.keycloak.models.KeycloakSession; +import org.keycloak.protocol.saml.util.ArtifactBindingUtils; import org.keycloak.saml.common.constants.GeneralConstants; import java.io.ByteArrayOutputStream; import java.io.IOException; -import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.security.SecureRandom; -import java.util.Arrays; import java.util.Base64; -import java.util.stream.Stream; +import java.util.Collections; import static org.keycloak.protocol.saml.DefaultSamlArtifactResolverFactory.TYPE_CODE; +import static org.keycloak.protocol.saml.SamlConfigAttributes.SAML_ARTIFACT_BINDING_IDENTIFIER; /** * ArtifactResolver for artifact-04 format. @@ -43,18 +43,13 @@ public class DefaultSamlArtifactResolver implements ArtifactResolver { } @Override - public ClientModel selectSourceClient(String artifact, Stream clients) throws ArtifactResolverProcessingException { - try { - byte[] source = extractSourceFromArtifact(artifact); + public ClientModel selectSourceClient(KeycloakSession session, String artifact) throws ArtifactResolverProcessingException { + byte[] source = extractSourceFromArtifact(artifact); + String identifier = ArtifactBindingUtils.getArtifactBindingIdentifierString(source); - MessageDigest sha1Digester = MessageDigest.getInstance("SHA-1"); - return clients.filter(clientModel -> Arrays.equals(source, - sha1Digester.digest(clientModel.getClientId().getBytes(Charsets.UTF_8)))) - .findFirst() - .orElseThrow(() -> new ArtifactResolverProcessingException("No client matching the artifact source found")); - } catch (NoSuchAlgorithmException e) { - throw new ArtifactResolverProcessingException(e); - } + return session.clients().searchClientsByAttributes(session.getContext().getRealm(), + Collections.singletonMap(SAML_ARTIFACT_BINDING_IDENTIFIER, identifier), 0, 1) + .findFirst().orElseThrow(() -> new ArtifactResolverProcessingException("No client matching the artifact source found")); } @Override @@ -109,8 +104,7 @@ public class DefaultSamlArtifactResolver implements ArtifactResolver { SecureRandom handleGenerator = SecureRandom.getInstance("SHA1PRNG"); byte[] trimmedIndex = new byte[2]; - MessageDigest sha1Digester = MessageDigest.getInstance("SHA-1"); - byte[] source = sha1Digester.digest(entityId.getBytes(Charsets.UTF_8)); + byte[] source = ArtifactBindingUtils.computeArtifactBindingIdentifier(entityId); byte[] assertionHandle = new byte[20]; handleGenerator.nextBytes(assertionHandle); diff --git a/services/src/main/java/org/keycloak/protocol/saml/SamlClient.java b/services/src/main/java/org/keycloak/protocol/saml/SamlClient.java index c312966ac8..1b8f23495e 100755 --- a/services/src/main/java/org/keycloak/protocol/saml/SamlClient.java +++ b/services/src/main/java/org/keycloak/protocol/saml/SamlClient.java @@ -20,6 +20,7 @@ package org.keycloak.protocol.saml; import org.jboss.logging.Logger; import org.keycloak.models.ClientConfigResolver; import org.keycloak.models.ClientModel; +import org.keycloak.protocol.saml.util.ArtifactBindingUtils; import org.keycloak.saml.SignatureAlgorithm; import org.keycloak.saml.common.constants.JBossSAMLURIConstants; import org.keycloak.saml.common.util.XmlKeyInfoKeyNameTransformer; @@ -258,4 +259,12 @@ public class SamlClient extends ClientConfigResolver { return -1; } } + + public void setArtifactBindingIdentifierFrom(String identifierFrom) { + client.setAttribute(SamlConfigAttributes.SAML_ARTIFACT_BINDING_IDENTIFIER, ArtifactBindingUtils.computeArtifactBindingIdentifierString(identifierFrom)); + } + + public String getArtifactBindingIdentifier() { + return client.getAttribute(SamlConfigAttributes.SAML_ARTIFACT_BINDING_IDENTIFIER); + } } diff --git a/services/src/main/java/org/keycloak/protocol/saml/SamlConfigAttributes.java b/services/src/main/java/org/keycloak/protocol/saml/SamlConfigAttributes.java index 93c4ae7f97..59f27f50ab 100755 --- a/services/src/main/java/org/keycloak/protocol/saml/SamlConfigAttributes.java +++ b/services/src/main/java/org/keycloak/protocol/saml/SamlConfigAttributes.java @@ -43,4 +43,5 @@ public interface SamlConfigAttributes { String SAML_ENCRYPTION_CERTIFICATE_ATTRIBUTE = "saml.encryption." + CertificateInfoHelper.X509CERTIFICATE; String SAML_ENCRYPTION_PRIVATE_KEY_ATTRIBUTE = "saml.encryption." + CertificateInfoHelper.PRIVATE_KEY; String SAML_ASSERTION_LIFESPAN = "saml.assertion.lifespan"; + String SAML_ARTIFACT_BINDING_IDENTIFIER = "saml.artifact.binding.identifier"; } diff --git a/services/src/main/java/org/keycloak/protocol/saml/SamlProtocolFactory.java b/services/src/main/java/org/keycloak/protocol/saml/SamlProtocolFactory.java index 19514042f9..fae3e4de09 100755 --- a/services/src/main/java/org/keycloak/protocol/saml/SamlProtocolFactory.java +++ b/services/src/main/java/org/keycloak/protocol/saml/SamlProtocolFactory.java @@ -175,6 +175,8 @@ public class SamlProtocolFactory extends AbstractLoginProtocolFactory { if (clientRep.isFrontchannelLogout() == null) { newClient.setFrontchannelLogout(true); } + + client.setArtifactBindingIdentifierFrom(clientRep.getClientId()); } } 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 3f02993041..39b7c6f380 100755 --- a/services/src/main/java/org/keycloak/protocol/saml/SamlService.java +++ b/services/src/main/java/org/keycloak/protocol/saml/SamlService.java @@ -71,7 +71,7 @@ import org.keycloak.protocol.oidc.utils.RedirectUtils; import org.keycloak.protocol.saml.preprocessor.SamlAuthenticationPreprocessor; import org.keycloak.protocol.saml.profile.ecp.SamlEcpProfileService; import org.keycloak.protocol.saml.profile.util.Soap; -import org.keycloak.protocol.util.ArtifactBindingUtils; +import org.keycloak.protocol.saml.util.ArtifactBindingUtils; import org.keycloak.rotation.HardcodedKeyLocator; import org.keycloak.rotation.KeyLocator; import org.keycloak.saml.BaseSAML2BindingBuilder; @@ -341,7 +341,7 @@ public class SamlService extends AuthorizationEndpointBase { //Find client ClientModel client; try { - client = getArtifactResolver(artifact).selectSourceClient(artifact, realm.getClientsStream()); + client = getArtifactResolver(artifact).selectSourceClient(session, artifact); Response error = checkClientValidity(client); if (error != null) { diff --git a/services/src/main/java/org/keycloak/protocol/util/ArtifactBindingUtils.java b/services/src/main/java/org/keycloak/protocol/util/ArtifactBindingUtils.java deleted file mode 100644 index b4532b46a0..0000000000 --- a/services/src/main/java/org/keycloak/protocol/util/ArtifactBindingUtils.java +++ /dev/null @@ -1,15 +0,0 @@ -package org.keycloak.protocol.util; - -import org.keycloak.protocol.saml.DefaultSamlArtifactResolverFactory; - -import java.util.Base64; - -public class ArtifactBindingUtils { - public static String artifactToResolverProviderId(String artifact) { - return byteArrayToResolverProviderId(Base64.getDecoder().decode(artifact)); - } - - public static String byteArrayToResolverProviderId(byte[] ar) { - return String.format("%02X%02X", ar[0], ar[1]); - } -} diff --git a/services/src/main/java/org/keycloak/services/managers/ClientManager.java b/services/src/main/java/org/keycloak/services/managers/ClientManager.java index dea2176837..524b31e258 100644 --- a/services/src/main/java/org/keycloak/services/managers/ClientManager.java +++ b/services/src/main/java/org/keycloak/services/managers/ClientManager.java @@ -35,6 +35,9 @@ import org.keycloak.protocol.LoginProtocol; import org.keycloak.protocol.LoginProtocolFactory; import org.keycloak.protocol.oidc.OIDCLoginProtocol; import org.keycloak.protocol.oidc.mappers.UserSessionNoteMapper; +import org.keycloak.protocol.saml.SamlClient; +import org.keycloak.protocol.saml.SamlConfigAttributes; +import org.keycloak.protocol.saml.SamlProtocol; import org.keycloak.representations.adapters.config.BaseRealmConfig; import org.keycloak.representations.adapters.config.PolicyEnforcerConfig; import org.keycloak.representations.idm.ClientRepresentation; @@ -191,7 +194,8 @@ public class ClientManager { } } - public void clientIdChanged(ClientModel client, String newClientId) { + public void clientIdChanged(ClientModel client, ClientRepresentation newClientRepresentation) { + String newClientId = newClientRepresentation.getClientId(); logger.debugf("Updating clientId from '%s' to '%s'", client.getClientId(), newClientId); UserModel serviceAccountUser = realmManager.getSession().users().getServiceAccount(client); @@ -199,6 +203,13 @@ public class ClientManager { String username = ServiceAccountConstants.SERVICE_ACCOUNT_USER_PREFIX + newClientId; serviceAccountUser.setUsername(username); } + + if (SamlProtocol.LOGIN_PROTOCOL.equals(client.getProtocol())) { + SamlClient samlClient = new SamlClient(client); + samlClient.setArtifactBindingIdentifierFrom(newClientId); + + newClientRepresentation.getAttributes().put(SamlConfigAttributes.SAML_ARTIFACT_BINDING_IDENTIFIER, samlClient.getArtifactBindingIdentifier()); + } } @JsonPropertyOrder({"realm", "realm-public-key", "bearer-only", "auth-server-url", "ssl-required", diff --git a/services/src/main/java/org/keycloak/services/resources/admin/ClientResource.java b/services/src/main/java/org/keycloak/services/resources/admin/ClientResource.java index cc4ecde123..429ae44210 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/ClientResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/ClientResource.java @@ -667,7 +667,7 @@ public class ClientResource { } if (rep.getClientId() != null && !rep.getClientId().equals(client.getClientId())) { - new ClientManager(new RealmManager(session)).clientIdChanged(client, rep.getClientId()); + new ClientManager(new RealmManager(session)).clientIdChanged(client, rep); } if (rep.isFullScopeAllowed() != null && rep.isFullScopeAllowed() != client.isFullScopeAllowed()) { diff --git a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/authentication/CustomTestingSamlArtifactResolver.java b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/authentication/CustomTestingSamlArtifactResolver.java index e80183f5f1..ea3196f5d3 100644 --- a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/authentication/CustomTestingSamlArtifactResolver.java +++ b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/authentication/CustomTestingSamlArtifactResolver.java @@ -2,6 +2,7 @@ package org.keycloak.testsuite.authentication; import org.keycloak.models.AuthenticatedClientSessionModel; import org.keycloak.models.ClientModel; +import org.keycloak.models.KeycloakSession; import org.keycloak.protocol.saml.ArtifactResolver; import java.io.ByteArrayInputStream; @@ -10,7 +11,6 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Base64; import java.util.List; -import java.util.stream.Stream; import static org.keycloak.testsuite.authentication.CustomTestingSamlArtifactResolverFactory.TYPE_CODE; @@ -23,7 +23,7 @@ public class CustomTestingSamlArtifactResolver implements ArtifactResolver { public static List list = new ArrayList<>(); @Override - public ClientModel selectSourceClient(String artifact, Stream clients) { + public ClientModel selectSourceClient(KeycloakSession session, String artifact) { return null; } diff --git a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/authentication/CustomTestingSamlArtifactResolverFactory.java b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/authentication/CustomTestingSamlArtifactResolverFactory.java index 95e298b24b..1f3bdfe27d 100644 --- a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/authentication/CustomTestingSamlArtifactResolverFactory.java +++ b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/authentication/CustomTestingSamlArtifactResolverFactory.java @@ -5,7 +5,7 @@ import org.keycloak.models.KeycloakSession; import org.keycloak.models.KeycloakSessionFactory; import org.keycloak.protocol.saml.ArtifactResolver; import org.keycloak.protocol.saml.ArtifactResolverFactory; -import org.keycloak.protocol.util.ArtifactBindingUtils; +import org.keycloak.protocol.saml.util.ArtifactBindingUtils; /** * This ArtifactResolver should be used only for testing purposes. diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/SAMLClientRegistrationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/SAMLClientRegistrationTest.java index f0800e0ca0..449cea823a 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/SAMLClientRegistrationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/SAMLClientRegistrationTest.java @@ -25,7 +25,9 @@ import org.keycloak.client.registration.Auth; import org.keycloak.client.registration.ClientRegistrationException; import org.keycloak.client.registration.HttpErrorException; import org.keycloak.events.Errors; +import org.keycloak.protocol.saml.SamlConfigAttributes; import org.keycloak.protocol.saml.mappers.AttributeStatementHelper; +import org.keycloak.protocol.saml.util.ArtifactBindingUtils; import org.keycloak.representations.idm.ClientInitialAccessCreatePresentation; import org.keycloak.representations.idm.ClientInitialAccessPresentation; import org.keycloak.representations.idm.ClientRepresentation; @@ -85,7 +87,8 @@ public class SAMLClientRegistrationTest extends AbstractClientRegistrationTest { )); assertThat(response.getAttributes().get("saml_single_logout_service_url_redirect"), is("https://LoadBalancer-9.siroe.com:3443/federation/SPSloRedirect/metaAlias/sp")); - + assertThat(response.getAttributes().get(SamlConfigAttributes.SAML_ARTIFACT_BINDING_IDENTIFIER), is(ArtifactBindingUtils.computeArtifactBindingIdentifierString("loadbalancer-9.siroe.com"))); + Assert.assertNotNull(response.getProtocolMappers()); Assert.assertEquals(1,response.getProtocolMappers().size()); ProtocolMapperRepresentation mapper = response.getProtocolMappers().get(0); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/migration/AbstractMigrationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/migration/AbstractMigrationTest.java index 9ee774af4d..82e10cd88e 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/migration/AbstractMigrationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/migration/AbstractMigrationTest.java @@ -45,6 +45,8 @@ import org.keycloak.models.utils.DefaultAuthenticationFlows; import org.keycloak.models.utils.TimeBasedOTP; import org.keycloak.protocol.oidc.OIDCConfigAttributes; import org.keycloak.protocol.oidc.OIDCLoginProtocolFactory; +import org.keycloak.protocol.saml.SamlConfigAttributes; +import org.keycloak.protocol.saml.util.ArtifactBindingUtils; import org.keycloak.representations.AccessToken; import org.keycloak.representations.RefreshToken; import org.keycloak.representations.idm.AuthenticationExecutionExportRepresentation; @@ -78,6 +80,7 @@ import java.util.Arrays; import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -86,6 +89,7 @@ import java.util.stream.Collectors; import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasEntry; import static org.hamcrest.Matchers.hasItem; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -125,7 +129,7 @@ public abstract class AbstractMigrationTest extends AbstractKeycloakTest { protected void testMigratedMigrationData(boolean supportsAuthzService) { assertNames(migrationRealm.roles().list(), "offline_access", "uma_authorization", "default-roles-migration", "migration-test-realm-role"); - List expectedClientIds = new ArrayList<>(Arrays.asList("account", "account-console", "admin-cli", "broker", "migration-test-client", "realm-management", "security-admin-console")); + List expectedClientIds = new ArrayList<>(Arrays.asList("account", "account-console", "admin-cli", "broker", "migration-test-client", "migration-saml-client", "realm-management", "security-admin-console")); if (supportsAuthzService) { expectedClientIds.add("authz-servlet"); @@ -304,6 +308,10 @@ public abstract class AbstractMigrationTest extends AbstractKeycloakTest { } } + protected void testMigrationTo14_0_0() { + testSamlAttributes(migrationRealm); + } + protected void testDeleteAccount(RealmResource realm) { ClientRepresentation accountClient = realm.clients().findByClientId(ACCOUNT_MANAGEMENT_CLIENT_ID).get(0); ClientResource accountResource = realm.clients().get(accountClient.getId()); @@ -924,6 +932,7 @@ public abstract class AbstractMigrationTest extends AbstractKeycloakTest { protected void testMigrationTo12_x(boolean testRealmAttributesMigration) { testMigrationTo12_0_0(); testMigrationTo13_0_0(testRealmAttributesMigration); + testMigrationTo14_0_0(); } protected void testMigrationTo7_x(boolean supportedAuthzServices) { @@ -965,6 +974,17 @@ public abstract class AbstractMigrationTest extends AbstractKeycloakTest { assertThat(migrationRealm2.toRepresentation().getDefaultRole().getName(), equalTo("default-roles-migration2-1")); } + protected void testSamlAttributes(RealmResource realm) { + log.info("Testing SAML ARTIFACT BINDING IDENTIFIER"); + + realm.clients().findAll().stream() + .filter(clientRepresentation -> Objects.equals("saml", clientRepresentation.getProtocol())) + .forEach(clientRepresentation -> { + String clientId = clientRepresentation.getClientId(); + assertThat(clientRepresentation.getAttributes(), hasEntry(SamlConfigAttributes.SAML_ARTIFACT_BINDING_IDENTIFIER, ArtifactBindingUtils.computeArtifactBindingIdentifierString(clientId))); + }); + } + protected void testRealmAttributesMigration() { log.info("testing realm attributes migration"); Map realmAttributes = migrationRealm.toRepresentation().getAttributes(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/ArtifactBindingTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/ArtifactBindingTest.java index c9d6b8227c..5943d6cb7d 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/ArtifactBindingTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/ArtifactBindingTest.java @@ -19,6 +19,7 @@ import org.keycloak.protocol.saml.SamlConfigAttributes; import org.keycloak.protocol.saml.SamlProtocol; import org.keycloak.protocol.saml.SamlProtocolUtils; import org.keycloak.protocol.saml.profile.util.Soap; +import org.keycloak.protocol.saml.util.ArtifactBindingUtils; import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.saml.SAML2LogoutResponseBuilder; import org.keycloak.saml.common.constants.GeneralConstants; @@ -984,4 +985,39 @@ public class ArtifactBindingTest extends AbstractSamlTest { assertThat(spDescriptor.getSingleLogoutService().get(0).getLocation(), is(equalTo(new URI("http://url.artifact.test")))); } + @Test + public void testArtifactBindingIdentifierChangedWhenClientIdChanged() throws IOException { + ClientRepresentation clientRepresentation = adminClient.realm(REALM_NAME) + .clients() + .findByClientId(SAML_CLIENT_ID_SALES_POST) + .get(0); + + String oldIdentifier = clientRepresentation.getAttributes().get(SamlConfigAttributes.SAML_ARTIFACT_BINDING_IDENTIFIER); + assertThat(oldIdentifier, notNullValue()); + + final String newClientId = "new_client_id"; + + try (ClientAttributeUpdater cau = ClientAttributeUpdater.forClient(adminClient, REALM_NAME, SAML_CLIENT_ID_SALES_POST) + .setClientId(newClientId) + .update() + ) { + clientRepresentation = adminClient.realm(REALM_NAME) + .clients() + .findByClientId(newClientId) + .get(0); + + String identifier = clientRepresentation.getAttributes().get(SamlConfigAttributes.SAML_ARTIFACT_BINDING_IDENTIFIER); + + assertThat(identifier, not(equalTo(oldIdentifier))); + assertThat(identifier, equalTo(ArtifactBindingUtils.computeArtifactBindingIdentifierString(newClientId))); + } + + clientRepresentation = adminClient.realm(REALM_NAME) + .clients() + .findByClientId(SAML_CLIENT_ID_SALES_POST) + .get(0); + + assertThat(clientRepresentation.getAttributes().get(SamlConfigAttributes.SAML_ARTIFACT_BINDING_IDENTIFIER), equalTo(oldIdentifier)); + } + } diff --git a/testsuite/integration-arquillian/tests/base/src/test/resources/migration-test/migration-realm-1.9.8.Final.json b/testsuite/integration-arquillian/tests/base/src/test/resources/migration-test/migration-realm-1.9.8.Final.json index eb5b21b511..811636987f 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/resources/migration-test/migration-realm-1.9.8.Final.json +++ b/testsuite/integration-arquillian/tests/base/src/test/resources/migration-test/migration-realm-1.9.8.Final.json @@ -2225,6 +2225,21 @@ "useTemplateConfig" : false, "useTemplateScope" : false, "useTemplateMappers" : false + }, { + "clientId": "migration-saml-client", + "enabled": true, + "fullScopeAllowed": true, + "protocol": "saml", + "baseUrl": "http://localhost:8080/sales-post", + "redirectUris": [ + "http://localhost:8080/sales-post/*" + ], + "attributes": { + "saml_assertion_consumer_url_post": "http://localhost:8080/sales-post/saml", + "saml_single_logout_service_url_post": "http://localhost:8080/sales-post/saml", + "saml.authnstatement": "true", + "saml_idp_initiated_sso_url_name": "sales-post" + } }, { "id" : "e6856a02-8f24-48d3-bb06-fae5dddae83e", "clientId" : "realm-management", @@ -3943,4 +3958,4 @@ "resetCredentialsFlow" : "reset credentials", "clientAuthenticationFlow" : "clients", "keycloakVersion" : "7.0.0.GA" -} ] \ No newline at end of file +} ] diff --git a/testsuite/integration-arquillian/tests/base/src/test/resources/migration-test/migration-realm-2.5.5.Final.json b/testsuite/integration-arquillian/tests/base/src/test/resources/migration-test/migration-realm-2.5.5.Final.json index c2fe1207c8..b41107b3ca 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/resources/migration-test/migration-realm-2.5.5.Final.json +++ b/testsuite/integration-arquillian/tests/base/src/test/resources/migration-test/migration-realm-2.5.5.Final.json @@ -2602,6 +2602,21 @@ "useTemplateConfig" : false, "useTemplateScope" : false, "useTemplateMappers" : false + }, { + "clientId": "migration-saml-client", + "enabled": true, + "fullScopeAllowed": true, + "protocol": "saml", + "baseUrl": "http://localhost:8080/sales-post", + "redirectUris": [ + "http://localhost:8080/sales-post/*" + ], + "attributes": { + "saml_assertion_consumer_url_post": "http://localhost:8080/sales-post/saml", + "saml_single_logout_service_url_post": "http://localhost:8080/sales-post/saml", + "saml.authnstatement": "true", + "saml_idp_initiated_sso_url_name": "sales-post" + } }, { "id" : "c8204f6f-f8c2-4af8-9bac-c45c95b4673b", "clientId" : "realm-management", @@ -4616,4 +4631,4 @@ "waitIncrementSeconds" : "60" }, "keycloakVersion" : "2.5.5.Final" -} ] \ No newline at end of file +} ] diff --git a/testsuite/integration-arquillian/tests/base/src/test/resources/migration-test/migration-realm-3.4.3.Final.json b/testsuite/integration-arquillian/tests/base/src/test/resources/migration-test/migration-realm-3.4.3.Final.json index f757a3b2b1..e82ecbbde9 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/resources/migration-test/migration-realm-3.4.3.Final.json +++ b/testsuite/integration-arquillian/tests/base/src/test/resources/migration-test/migration-realm-3.4.3.Final.json @@ -810,6 +810,21 @@ "useTemplateConfig" : false, "useTemplateScope" : false, "useTemplateMappers" : false + }, { + "clientId": "migration-saml-client", + "enabled": true, + "fullScopeAllowed": true, + "protocol": "saml", + "baseUrl": "http://localhost:8080/sales-post", + "redirectUris": [ + "http://localhost:8080/sales-post/*" + ], + "attributes": { + "saml_assertion_consumer_url_post": "http://localhost:8080/sales-post/saml", + "saml_single_logout_service_url_post": "http://localhost:8080/sales-post/saml", + "saml.authnstatement": "true", + "saml_idp_initiated_sso_url_name": "sales-post" + } }, { "id" : "9a37d2c5-6a36-4a2c-b837-f2ea846fb0d5", "clientId" : "realm-management", @@ -4974,4 +4989,4 @@ "waitIncrementSeconds" : "60" }, "keycloakVersion" : "3.4.3.Final" -} ] \ No newline at end of file +} ] diff --git a/testsuite/integration-arquillian/tests/base/src/test/resources/migration-test/migration-realm-4.8.3.Final.json b/testsuite/integration-arquillian/tests/base/src/test/resources/migration-test/migration-realm-4.8.3.Final.json index 185a95a750..82e18a6643 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/resources/migration-test/migration-realm-4.8.3.Final.json +++ b/testsuite/integration-arquillian/tests/base/src/test/resources/migration-test/migration-realm-4.8.3.Final.json @@ -528,6 +528,21 @@ "nodeReRegistrationTimeout" : -1, "defaultClientScopes" : [ "web-origins", "role_list", "profile", "roles", "email" ], "optionalClientScopes" : [ "address", "phone", "offline_access" ] + }, { + "clientId": "migration-saml-client", + "enabled": true, + "fullScopeAllowed": true, + "protocol": "saml", + "baseUrl": "http://localhost:8080/sales-post", + "redirectUris": [ + "http://localhost:8080/sales-post/*" + ], + "attributes": { + "saml_assertion_consumer_url_post": "http://localhost:8080/sales-post/saml", + "saml_single_logout_service_url_post": "http://localhost:8080/sales-post/saml", + "saml.authnstatement": "true", + "saml_idp_initiated_sso_url_name": "sales-post" + } }, { "id" : "99a28a93-e2e3-4b1d-a377-8a30f6b4930a", "clientId" : "realm-management", @@ -4730,4 +4745,4 @@ }, "keycloakVersion" : "4.8.3.Final", "userManagedAccessAllowed" : false -} ] \ No newline at end of file +} ] diff --git a/testsuite/integration-arquillian/tests/base/src/test/resources/migration-test/migration-realm-9.0.3.json b/testsuite/integration-arquillian/tests/base/src/test/resources/migration-test/migration-realm-9.0.3.json index bafb31c45b..f2fe67a1c4 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/resources/migration-test/migration-realm-9.0.3.json +++ b/testsuite/integration-arquillian/tests/base/src/test/resources/migration-test/migration-realm-9.0.3.json @@ -554,6 +554,21 @@ "defaultClientScopes" : [ "web-origins", "role_list", "profile", "roles", "email" ], "optionalClientScopes" : [ "address", "phone", "offline_access", "microprofile-jwt" ] }, { + "clientId": "migration-saml-client", + "enabled": true, + "fullScopeAllowed": true, + "protocol": "saml", + "baseUrl": "http://localhost:8080/sales-post", + "redirectUris": [ + "http://localhost:8080/sales-post/*" + ], + "attributes": { + "saml_assertion_consumer_url_post": "http://localhost:8080/sales-post/saml", + "saml_single_logout_service_url_post": "http://localhost:8080/sales-post/saml", + "saml.authnstatement": "true", + "saml_idp_initiated_sso_url_name": "sales-post" + } + },{ "id" : "cb1a7042-228c-4f8e-b0c9-654f1855d1b8", "clientId" : "realm-management", "name" : "${client_realm-management}", @@ -5411,4 +5426,4 @@ "attributes" : { }, "keycloakVersion" : "9.0.3", "userManagedAccessAllowed" : false -} ] \ No newline at end of file +} ] diff --git a/testsuite/integration-arquillian/tests/other/console/src/test/java/org/keycloak/testsuite/console/clients/AbstractClientTest.java b/testsuite/integration-arquillian/tests/other/console/src/test/java/org/keycloak/testsuite/console/clients/AbstractClientTest.java index 5243003b3d..20c2f2aefd 100644 --- a/testsuite/integration-arquillian/tests/other/console/src/test/java/org/keycloak/testsuite/console/clients/AbstractClientTest.java +++ b/testsuite/integration-arquillian/tests/other/console/src/test/java/org/keycloak/testsuite/console/clients/AbstractClientTest.java @@ -4,6 +4,8 @@ import org.jboss.arquillian.graphene.page.Page; import org.junit.Before; import org.keycloak.admin.client.resource.ClientResource; import org.keycloak.admin.client.resource.ClientsResource; +import org.keycloak.protocol.saml.SamlConfigAttributes; +import org.keycloak.protocol.saml.util.ArtifactBindingUtils; import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.representations.idm.ProtocolMapperRepresentation; import org.keycloak.testsuite.console.AbstractConsoleTest; @@ -87,6 +89,7 @@ public abstract class AbstractClientTest extends AbstractConsoleTest { attributes.put(SAML_SIGNATURE_ALGORITHM, "RSA_SHA256"); attributes.put(SAML_FORCE_NAME_ID_FORMAT, "false"); attributes.put(SAML_NAME_ID_FORMAT, "username"); + attributes.put(SamlConfigAttributes.SAML_ARTIFACT_BINDING_IDENTIFIER, ArtifactBindingUtils.computeArtifactBindingIdentifierString("saml")); return attributes; } @@ -147,4 +150,4 @@ public abstract class AbstractClientTest extends AbstractConsoleTest { return clientsResource().get(id); } -} \ No newline at end of file +} diff --git a/testsuite/integration-arquillian/tests/other/console/src/test/java/org/keycloak/testsuite/console/clients/ClientSettingsTest.java b/testsuite/integration-arquillian/tests/other/console/src/test/java/org/keycloak/testsuite/console/clients/ClientSettingsTest.java index 2ae679d969..bef6333b3a 100644 --- a/testsuite/integration-arquillian/tests/other/console/src/test/java/org/keycloak/testsuite/console/clients/ClientSettingsTest.java +++ b/testsuite/integration-arquillian/tests/other/console/src/test/java/org/keycloak/testsuite/console/clients/ClientSettingsTest.java @@ -20,6 +20,8 @@ package org.keycloak.testsuite.console.clients; import org.jboss.arquillian.graphene.page.Page; import org.junit.Test; import org.keycloak.common.Profile; +import org.keycloak.protocol.saml.SamlConfigAttributes; +import org.keycloak.protocol.saml.util.ArtifactBindingUtils; import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.testsuite.arquillian.annotation.EnableFeature; import org.keycloak.testsuite.console.page.clients.settings.ClientSettings; @@ -29,6 +31,7 @@ import org.openqa.selenium.By; import javax.ws.rs.core.Response; import java.util.ArrayList; import java.util.List; +import java.util.Map; import static org.junit.Assert.*; import static org.keycloak.testsuite.auth.page.login.Login.OIDC; @@ -202,6 +205,23 @@ public class ClientSettingsTest extends AbstractClientTest { assertClientSamlAttributes(getSAMLAttributes(), found.getAttributes()); } + @Test + public void updateSAML() { + createSAML(); + + final String newClientId = "new_client_id"; + + clientSettingsPage.form().setClientId(newClientId); + clientSettingsPage.form().save(); + + ClientRepresentation found = findClientByClientId(newClientId); + + Map samlAttributes = getSAMLAttributes(); + samlAttributes.put(SamlConfigAttributes.SAML_ARTIFACT_BINDING_IDENTIFIER, ArtifactBindingUtils.computeArtifactBindingIdentifierString(newClientId)); + + assertClientSamlAttributes(samlAttributes, found.getAttributes()); + } + @Test public void invalidSettings() { clientsPage.table().createClient();