From f80a8fbed0940f28e8a23ebcdc5469c7299c0457 Mon Sep 17 00:00:00 2001 From: danielFesenmeyer Date: Thu, 21 Jul 2022 19:44:27 +0200 Subject: [PATCH] Avoid login failures in case of non-existing group or role references and update references in case of renaming or moving - no longer throw an exception, when a role or group cannot be found, log a warning instead - update mapper references in case of the following events: - moving a group - renaming a group - renaming a role - renaming a client's Client ID (may affect role qualifiers) - in case a role or group is removed, the reference still will not be changed - extend and refactor integration tests in order to check the new behavior Closes #11236 --- .../keycloak/models/jpa/JpaRealmProvider.java | 28 ++ .../models/map/group/MapGroupProvider.java | 30 +++ .../AbstractIdentityProviderMapper.java | 24 ++ .../AbstractConfigPropertySynchronizer.java | 71 +++++ .../mappersync/ConfigSyncEventListener.java | 80 ++++++ .../mappersync/ConfigSynchronizer.java | 35 +++ ...GroupConfigPropertyByPathSynchronizer.java | 67 +++++ ...eConfigPropertyByClientIdSynchronizer.java | 75 ++++++ ...eConfigPropertyByRoleNameSynchronizer.java | 69 +++++ .../models/utils/KeycloakModelUtils.java | 70 ++++- .../models/utils/ModelToRepresentation.java | 12 +- .../models/utils/RepresentationToModel.java | 37 ++- .../models/KeycloakModelUtilsTest.java | 77 ++++++ .../java/org/keycloak/models/ClientModel.java | 7 + .../java/org/keycloak/models/GroupModel.java | 9 +- .../java/org/keycloak/models/RoleModel.java | 13 + .../mappers/AbstractClaimToGroupMapper.java | 36 ++- .../mappers/AbstractClaimToRoleMapper.java | 41 ++- .../mappers/AdvancedClaimToGroupMapper.java | 3 - .../broker/provider/HardcodedRoleMapper.java | 25 +- .../AbstractAttributeToRoleMapper.java | 30 ++- .../AbstractClientRegistrationProvider.java | 2 +- .../resources/admin/ClientResource.java | 2 +- .../resources/admin/GroupResource.java | 43 ++- .../resources/admin/GroupsResource.java | 2 +- .../resources/admin/RoleByIdResource.java | 2 +- .../admin/RoleContainerResource.java | 2 +- .../resources/admin/RoleResource.java | 42 ++- .../AbstractAdvancedGroupMapperTest.java | 111 ++++---- .../AbstractAdvancedRoleMapperTest.java | 135 +++++----- .../broker/AbstractBaseBrokerTest.java | 5 + .../broker/AbstractGroupMapperTest.java | 150 +++++++++-- .../broker/AbstractRoleMapperTest.java | 249 +++++++++++++++--- .../broker/AttributeToRoleMapperTest.java | 64 ++--- .../testsuite/broker/BrokerTestTools.java | 24 +- .../ExternalKeycloakRoleToRoleMapperTest.java | 70 ++--- .../broker/HardcodedRoleMapperTest.java | 58 ++-- ...SamlAdvancedAttributeToRoleMapperTest.java | 39 ++- ...amlMultipleAttributeToRoleMappersTest.java | 47 ++-- .../OidcAdvancedClaimToGroupMapperTest.java | 20 +- .../OidcAdvancedClaimToRoleMapperTest.java | 22 +- .../broker/OidcClaimToRoleMapperTest.java | 99 ++++--- .../OidcMultipleClaimToRoleMappersTest.java | 40 ++- .../OidcUserInfoClaimToRoleMapperTest.java | 93 ++++--- .../testsuite/model/RealmModelTest.java | 49 +++- 45 files changed, 1702 insertions(+), 507 deletions(-) create mode 100644 server-spi-private/src/main/java/org/keycloak/broker/provider/mappersync/AbstractConfigPropertySynchronizer.java create mode 100644 server-spi-private/src/main/java/org/keycloak/broker/provider/mappersync/ConfigSyncEventListener.java create mode 100644 server-spi-private/src/main/java/org/keycloak/broker/provider/mappersync/ConfigSynchronizer.java create mode 100644 server-spi-private/src/main/java/org/keycloak/broker/provider/mappersync/GroupConfigPropertyByPathSynchronizer.java create mode 100644 server-spi-private/src/main/java/org/keycloak/broker/provider/mappersync/RoleConfigPropertyByClientIdSynchronizer.java create mode 100644 server-spi-private/src/main/java/org/keycloak/broker/provider/mappersync/RoleConfigPropertyByRoleNameSynchronizer.java create mode 100644 server-spi-private/src/test/java/org/keycloak/models/KeycloakModelUtilsTest.java diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/JpaRealmProvider.java b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaRealmProvider.java index ec01cdff35..7be533b636 100644 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/JpaRealmProvider.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaRealmProvider.java @@ -444,6 +444,7 @@ public class JpaRealmProvider implements RealmProvider, ClientProvider, ClientSc if (group.getParentId() != null) { group.getParent().removeChild(group); } + GroupModel previousParent = group.getParent(); group.setParent(toParent); if (toParent != null) toParent.addChild(group); else session.groups().addTopLevelGroup(realm, group); @@ -452,6 +453,33 @@ public class JpaRealmProvider implements RealmProvider, ClientProvider, ClientSc // DuplicateModelException {@link PersistenceExceptionConverter} is not called if the // ConstraintViolationException is not thrown in method called directly from EntityManager em.flush(); + + String newPath = KeycloakModelUtils.buildGroupPath(group); + String previousPath = KeycloakModelUtils.buildGroupPath(group, previousParent); + + GroupModel.GroupPathChangeEvent event = + new GroupModel.GroupPathChangeEvent() { + @Override + public RealmModel getRealm() { + return realm; + } + + @Override + public String getNewPath() { + return newPath; + } + + @Override + public String getPreviousPath() { + return previousPath; + } + + @Override + public KeycloakSession getKeycloakSession() { + return session; + } + }; + session.getKeycloakSessionFactory().publish(event); } @Override diff --git a/model/map/src/main/java/org/keycloak/models/map/group/MapGroupProvider.java b/model/map/src/main/java/org/keycloak/models/map/group/MapGroupProvider.java index 1eaa56b766..67615cca25 100644 --- a/model/map/src/main/java/org/keycloak/models/map/group/MapGroupProvider.java +++ b/model/map/src/main/java/org/keycloak/models/map/group/MapGroupProvider.java @@ -33,6 +33,7 @@ import org.keycloak.models.map.storage.ModelCriteriaBuilder.Operator; import org.keycloak.models.map.storage.QueryParameters; import org.keycloak.models.map.storage.criteria.DefaultModelCriteria; +import org.keycloak.models.utils.KeycloakModelUtils; import java.util.Map; import java.util.Objects; @@ -259,6 +260,8 @@ public class MapGroupProvider implements GroupProvider { public void moveGroup(RealmModel realm, GroupModel group, GroupModel toParent) { LOG.tracef("moveGroup(%s, %s, %s)%s", realm, group, toParent, getShortStackTrace()); + GroupModel previousParent = group.getParent(); + if (toParent != null && group.getId().equals(toParent.getId())) { return; } @@ -282,6 +285,33 @@ public class MapGroupProvider implements GroupProvider { } group.setParent(toParent); if (toParent != null) toParent.addChild(group); + + String newPath = KeycloakModelUtils.buildGroupPath(group); + String previousPath = KeycloakModelUtils.buildGroupPath(group, previousParent); + + GroupModel.GroupPathChangeEvent event = + new GroupModel.GroupPathChangeEvent() { + @Override + public RealmModel getRealm() { + return realm; + } + + @Override + public String getNewPath() { + return newPath; + } + + @Override + public String getPreviousPath() { + return previousPath; + } + + @Override + public KeycloakSession getKeycloakSession() { + return session; + } + }; + session.getKeycloakSessionFactory().publish(event); } @Override diff --git a/server-spi-private/src/main/java/org/keycloak/broker/provider/AbstractIdentityProviderMapper.java b/server-spi-private/src/main/java/org/keycloak/broker/provider/AbstractIdentityProviderMapper.java index 5eaef8e10d..9dc289bcbc 100755 --- a/server-spi-private/src/main/java/org/keycloak/broker/provider/AbstractIdentityProviderMapper.java +++ b/server-spi-private/src/main/java/org/keycloak/broker/provider/AbstractIdentityProviderMapper.java @@ -17,6 +17,8 @@ package org.keycloak.broker.provider; +import org.jboss.logging.Logger; +import org.keycloak.broker.provider.mappersync.ConfigSyncEventListener; import org.keycloak.models.IdentityProviderMapperModel; import org.keycloak.models.KeycloakSession; import org.keycloak.models.KeycloakSessionFactory; @@ -28,6 +30,11 @@ import org.keycloak.models.UserModel; * @version $Revision: 1 $ */ public abstract class AbstractIdentityProviderMapper implements IdentityProviderMapper { + + private static final Logger LOG = Logger.getLogger(AbstractIdentityProviderMapper.class); + + private static volatile KeycloakSessionFactory keycloakSessionFactory; + @Override public void close() { @@ -45,7 +52,24 @@ public abstract class AbstractIdentityProviderMapper implements IdentityProvider @Override public void postInit(KeycloakSessionFactory factory) { + registerConfigSyncEventListenerOnce(factory); + } + private void registerConfigSyncEventListenerOnce(KeycloakSessionFactory factory) { + /* + * Make sure that the config sync listener is registered only once for a session factory. It would also be + * possible to register it only once per VM, but that does not work fine in integration tests. + */ + if (keycloakSessionFactory != factory) { + synchronized (AbstractIdentityProviderMapper.class) { + if (keycloakSessionFactory != factory) { + keycloakSessionFactory = factory; + + LOG.infof("Registering %s", ConfigSyncEventListener.class); + factory.register(new ConfigSyncEventListener()); + } + } + } } @Override diff --git a/server-spi-private/src/main/java/org/keycloak/broker/provider/mappersync/AbstractConfigPropertySynchronizer.java b/server-spi-private/src/main/java/org/keycloak/broker/provider/mappersync/AbstractConfigPropertySynchronizer.java new file mode 100644 index 0000000000..bd4fc784a0 --- /dev/null +++ b/server-spi-private/src/main/java/org/keycloak/broker/provider/mappersync/AbstractConfigPropertySynchronizer.java @@ -0,0 +1,71 @@ +/* + * Copyright 2022 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.broker.provider.mappersync; + +import org.jboss.logging.Logger; +import org.keycloak.models.IdentityProviderMapperModel; +import org.keycloak.models.RealmModel; +import org.keycloak.provider.ProviderEvent; +import org.keycloak.utils.StringUtil; + +import java.util.Map; +import java.util.function.Consumer; + +/** + * Abstract base class for updating a single reference (specified via a single config property). + * + * @author Daniel Fesenmeyer + */ +public abstract class AbstractConfigPropertySynchronizer implements ConfigSynchronizer { + private static final Logger LOG = Logger.getLogger(AbstractConfigPropertySynchronizer.class); + + protected abstract String getConfigPropertyName(); + + protected abstract void updateConfigPropertyIfNecessary(T event, String currentPropertyValue, + Consumer propertyUpdater); + + @Override + public final void handleEvent(T event, IdentityProviderMapperModel idpMapper) { + Map config = idpMapper.getConfig(); + if (config == null) { + return; + } + + String configPropertyName = getConfigPropertyName(); + String configuredValue = config.get(configPropertyName); + if (StringUtil.isBlank(configuredValue)) { + return; + } + + Consumer propertyUpdater = value -> config.put(configPropertyName, value); + + updateConfigPropertyIfNecessary(event, configuredValue, propertyUpdater); + + final String newConfiguredValue = config.get(configPropertyName); + if (!configuredValue.equals(newConfiguredValue)) { + RealmModel realm = extractRealm(event); + + LOG.infof( + "Reference of type '%s' changed from '%s' to '%s' in realm '%s'. Adjusting the reference from mapper '%s' of IDP '%s'.", + configPropertyName, configuredValue, newConfiguredValue, realm.getName(), idpMapper.getName(), + idpMapper.getIdentityProviderAlias()); + realm.updateIdentityProviderMapper(idpMapper); + } + } + +} diff --git a/server-spi-private/src/main/java/org/keycloak/broker/provider/mappersync/ConfigSyncEventListener.java b/server-spi-private/src/main/java/org/keycloak/broker/provider/mappersync/ConfigSyncEventListener.java new file mode 100644 index 0000000000..75d7522670 --- /dev/null +++ b/server-spi-private/src/main/java/org/keycloak/broker/provider/mappersync/ConfigSyncEventListener.java @@ -0,0 +1,80 @@ +/* + * Copyright 2022 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.broker.provider.mappersync; + +import org.jboss.logging.Logger; +import org.keycloak.models.IdentityProviderMapperModel; +import org.keycloak.models.RealmModel; +import org.keycloak.provider.ProviderEvent; +import org.keycloak.provider.ProviderEventListener; + +import java.util.Arrays; +import java.util.List; +import java.util.stream.Collectors; + +/** + * Event listener which synchronizes mapper configs, when references change. + * + * @author Daniel Fesenmeyer + */ +public final class ConfigSyncEventListener implements ProviderEventListener { + + private static final Logger LOG = Logger.getLogger(ConfigSyncEventListener.class); + + private static final List> SYNCHRONIZERS = + Arrays.asList(GroupConfigPropertyByPathSynchronizer.INSTANCE, + RoleConfigPropertyByClientIdSynchronizer.INSTANCE, RoleConfigPropertyByRoleNameSynchronizer.INSTANCE); + + @Override + public void onEvent(ProviderEvent event) { + List realmMappers = null; + + for (ConfigSynchronizer s : SYNCHRONIZERS) { + ConfigSynchronizer configSynchronizer = (ConfigSynchronizer) s; + + if (eventMatchesSynchronizer(event, configSynchronizer)) { + LOG.debugf("Synchronizer %s matches event: %s", configSynchronizer, event); + + if (realmMappers == null) { + /* + * an event always refers to just one realm, so we can use an arbitrary synchronizer to extract the + * realm + */ + RealmModel realm = configSynchronizer.extractRealm(event); + realmMappers = realm.getIdentityProviderMappersStream() + .collect(Collectors.toList()); + } + + realmMappers.forEach(idpMapper -> { + LOG.debugf("Apply synchronizer %s to event %s and mapper with name %s", configSynchronizer, event, + idpMapper.getName()); + configSynchronizer.handleEvent(event, idpMapper); + }); + } else { + LOG.debugf("Synchronizer %s does not match event: %s", configSynchronizer, event); + } + } + } + + private static boolean eventMatchesSynchronizer(ProviderEvent event, + ConfigSynchronizer synchronizer) { + Class handledClass = synchronizer.getEventClass(); + return (handledClass.isAssignableFrom(event.getClass())); + } + +} diff --git a/server-spi-private/src/main/java/org/keycloak/broker/provider/mappersync/ConfigSynchronizer.java b/server-spi-private/src/main/java/org/keycloak/broker/provider/mappersync/ConfigSynchronizer.java new file mode 100644 index 0000000000..11d9775797 --- /dev/null +++ b/server-spi-private/src/main/java/org/keycloak/broker/provider/mappersync/ConfigSynchronizer.java @@ -0,0 +1,35 @@ +/* + * Copyright 2022 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.broker.provider.mappersync; + +import org.keycloak.models.IdentityProviderMapperModel; +import org.keycloak.models.RealmModel; +import org.keycloak.provider.ProviderEvent; + +/** + * Interface for updating references in mapper configs, when references (like group path) change. + * + * @author Daniel Fesenmeyer + */ +public interface ConfigSynchronizer { + Class getEventClass(); + + RealmModel extractRealm(T event); + + void handleEvent(T event, IdentityProviderMapperModel idpMapper); +} \ No newline at end of file diff --git a/server-spi-private/src/main/java/org/keycloak/broker/provider/mappersync/GroupConfigPropertyByPathSynchronizer.java b/server-spi-private/src/main/java/org/keycloak/broker/provider/mappersync/GroupConfigPropertyByPathSynchronizer.java new file mode 100644 index 0000000000..dedcc931f9 --- /dev/null +++ b/server-spi-private/src/main/java/org/keycloak/broker/provider/mappersync/GroupConfigPropertyByPathSynchronizer.java @@ -0,0 +1,67 @@ +/* + * Copyright 2022 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.broker.provider.mappersync; + +import org.keycloak.broker.provider.ConfigConstants; +import org.keycloak.models.GroupModel; +import org.keycloak.models.RealmModel; +import org.keycloak.models.utils.KeycloakModelUtils; + +import java.util.function.Consumer; + +/** + * Updates a group reference in a mapper config, when the path of a group changes. + * + * @author Daniel Fesenmeyer + */ +public class GroupConfigPropertyByPathSynchronizer extends AbstractConfigPropertySynchronizer { + + public static final GroupConfigPropertyByPathSynchronizer INSTANCE = new GroupConfigPropertyByPathSynchronizer(); + + private GroupConfigPropertyByPathSynchronizer() { + // noop + } + + @Override + public Class getEventClass() { + return GroupModel.GroupPathChangeEvent.class; + } + + @Override + public RealmModel extractRealm(GroupModel.GroupPathChangeEvent event) { + return event.getRealm(); + } + + @Override + public String getConfigPropertyName() { + return ConfigConstants.GROUP; + } + + @Override + protected void updateConfigPropertyIfNecessary(GroupModel.GroupPathChangeEvent event, + String currentPropertyValue, Consumer propertyUpdater) { + String configuredGroupPath = KeycloakModelUtils.normalizeGroupPath(currentPropertyValue); + + String previousGroupPath = event.getPreviousPath(); + if (previousGroupPath.equals(configuredGroupPath)) { + String newPath = event.getNewPath(); + propertyUpdater.accept(newPath); + } + } + +} diff --git a/server-spi-private/src/main/java/org/keycloak/broker/provider/mappersync/RoleConfigPropertyByClientIdSynchronizer.java b/server-spi-private/src/main/java/org/keycloak/broker/provider/mappersync/RoleConfigPropertyByClientIdSynchronizer.java new file mode 100644 index 0000000000..d18180a15b --- /dev/null +++ b/server-spi-private/src/main/java/org/keycloak/broker/provider/mappersync/RoleConfigPropertyByClientIdSynchronizer.java @@ -0,0 +1,75 @@ +/* + * Copyright 2022 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.broker.provider.mappersync; + +import org.keycloak.broker.provider.ConfigConstants; +import org.keycloak.models.ClientModel; +import org.keycloak.models.RealmModel; +import org.keycloak.models.utils.KeycloakModelUtils; + +import java.util.function.Consumer; + +/** + * Updates a role reference in a mapper config, when a client ID changes. + * + * @author Daniel Fesenmeyer + */ +public class RoleConfigPropertyByClientIdSynchronizer + extends AbstractConfigPropertySynchronizer { + + public static final RoleConfigPropertyByClientIdSynchronizer INSTANCE = + new RoleConfigPropertyByClientIdSynchronizer(); + + private RoleConfigPropertyByClientIdSynchronizer() { + // noop + } + + @Override + public Class getEventClass() { + return ClientModel.ClientIdChangeEvent.class; + } + + @Override + public RealmModel extractRealm(ClientModel.ClientIdChangeEvent event) { + return event.getUpdatedClient().getRealm(); + } + + @Override + public String getConfigPropertyName() { + return ConfigConstants.ROLE; + } + + @Override + protected void updateConfigPropertyIfNecessary(ClientModel.ClientIdChangeEvent event, String currentPropertyValue, + Consumer propertyUpdater) { + String[] parsedConfiguredRoleQualifier = KeycloakModelUtils.parseRole(currentPropertyValue); + String configuredClientId = parsedConfiguredRoleQualifier[0]; + if (configuredClientId == null) { + // a realm role is configured for the mapper, event is not relevant + return; + } + + String configuredRoleName = parsedConfiguredRoleQualifier[1]; + + if (configuredClientId.equals(event.getPreviousClientId())) { + String newRoleQualifier = KeycloakModelUtils.buildRoleQualifier(event.getNewClientId(), configuredRoleName); + propertyUpdater.accept(newRoleQualifier); + } + } + +} diff --git a/server-spi-private/src/main/java/org/keycloak/broker/provider/mappersync/RoleConfigPropertyByRoleNameSynchronizer.java b/server-spi-private/src/main/java/org/keycloak/broker/provider/mappersync/RoleConfigPropertyByRoleNameSynchronizer.java new file mode 100644 index 0000000000..7433f7626c --- /dev/null +++ b/server-spi-private/src/main/java/org/keycloak/broker/provider/mappersync/RoleConfigPropertyByRoleNameSynchronizer.java @@ -0,0 +1,69 @@ +/* + * Copyright 2022 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.broker.provider.mappersync; + +import org.keycloak.broker.provider.ConfigConstants; +import org.keycloak.models.RealmModel; +import org.keycloak.models.RoleModel; +import org.keycloak.models.utils.KeycloakModelUtils; + +import java.util.function.Consumer; + +/** + * Updates a role reference a in mapper config, when a role name changes. + * + * @author Daniel Fesenmeyer + */ +public class RoleConfigPropertyByRoleNameSynchronizer + extends AbstractConfigPropertySynchronizer { + + public static final RoleConfigPropertyByRoleNameSynchronizer INSTANCE = + new RoleConfigPropertyByRoleNameSynchronizer(); + + private RoleConfigPropertyByRoleNameSynchronizer() { + // noop + } + + @Override + public Class getEventClass() { + return RoleModel.RoleNameChangeEvent.class; + } + + @Override + public RealmModel extractRealm(RoleModel.RoleNameChangeEvent event) { + return event.getRealm(); + } + + @Override + public String getConfigPropertyName() { + return ConfigConstants.ROLE; + } + + @Override + protected void updateConfigPropertyIfNecessary(RoleModel.RoleNameChangeEvent event, String currentPropertyValue, + Consumer propertyUpdater) { + + String previousRoleQualifier = + KeycloakModelUtils.buildRoleQualifier(event.getClientId(), event.getPreviousName()); + if (previousRoleQualifier.equals(currentPropertyValue)) { + String newRoleQualifier = KeycloakModelUtils.buildRoleQualifier(event.getClientId(), event.getNewName()); + propertyUpdater.accept(newRoleQualifier); + } + } + +} diff --git a/server-spi-private/src/main/java/org/keycloak/models/utils/KeycloakModelUtils.java b/server-spi-private/src/main/java/org/keycloak/models/utils/KeycloakModelUtils.java index c000c821fb..48c188a42a 100755 --- a/server-spi-private/src/main/java/org/keycloak/models/utils/KeycloakModelUtils.java +++ b/server-spi-private/src/main/java/org/keycloak/models/utils/KeycloakModelUtils.java @@ -79,13 +79,17 @@ import java.util.function.Function; /** * Set of helper methods, which are useful in various model implementations. * - * @author Marek Posolda + * @author Marek Posolda, + * Daniel Fesenmeyer */ public final class KeycloakModelUtils { public static final String AUTH_TYPE_CLIENT_SECRET = "client-secret"; public static final String AUTH_TYPE_CLIENT_SECRET_JWT = "client-secret-jwt"; + public static final String GROUP_PATH_SEPARATOR = "/"; + private static final char CLIENT_ROLE_SEPARATOR = '.'; + private KeycloakModelUtils() { } @@ -541,7 +545,7 @@ public final class KeycloakModelUtils { } /** - * Given the {@code pathParts} of a group with the given {@code groupName}, format the {@pathParts} in order to ignore + * Given the {@code pathParts} of a group with the given {@code groupName}, format the {@code segments} in order to ignore * group names containing a {@code /} character. * * @param segments the path segments @@ -550,7 +554,7 @@ public final class KeycloakModelUtils { * @return a new array of strings with the correct segments in case the group has a name containing slashes */ private static String[] formatPathSegments(String[] segments, int index, String groupName) { - String[] nameSegments = groupName.split("/"); + String[] nameSegments = groupName.split(GROUP_PATH_SEPARATOR); if (nameSegments.length > 1 && segments.length >= nameSegments.length) { for (int i = 0; i < nameSegments.length; i++) { @@ -582,13 +586,13 @@ public final class KeycloakModelUtils { if (path == null) { return null; } - if (path.startsWith("/")) { + if (path.startsWith(GROUP_PATH_SEPARATOR)) { path = path.substring(1); } - if (path.endsWith("/")) { + if (path.endsWith(GROUP_PATH_SEPARATOR)) { path = path.substring(0, path.length() - 1); } - String[] split = path.split("/"); + String[] split = path.split(GROUP_PATH_SEPARATOR); if (split.length == 0) return null; return realm.getTopLevelGroupsStream().map(group -> { @@ -610,6 +614,42 @@ public final class KeycloakModelUtils { }).filter(Objects::nonNull).findFirst().orElse(null); } + private static void buildGroupPath(StringBuilder sb, String groupName, GroupModel parent) { + if (parent != null) { + buildGroupPath(sb, parent.getName(), parent.getParent()); + } + sb.append(GROUP_PATH_SEPARATOR).append(groupName); + } + + public static String buildGroupPath(GroupModel group) { + StringBuilder sb = new StringBuilder(); + buildGroupPath(sb, group.getName(), group.getParent()); + return sb.toString(); + } + + public static String buildGroupPath(GroupModel group, GroupModel otherParentGroup) { + StringBuilder sb = new StringBuilder(); + buildGroupPath(sb, group.getName(), otherParentGroup); + return sb.toString(); + } + + public static String normalizeGroupPath(final String groupPath) { + if (groupPath == null) { + return null; + } + + String normalized = groupPath; + + if (!normalized.startsWith(GROUP_PATH_SEPARATOR)) { + normalized = GROUP_PATH_SEPARATOR + normalized; + } + if (normalized.endsWith(GROUP_PATH_SEPARATOR)) { + normalized = normalized.substring(0, normalized.length() - 1); + } + + return normalized; + } + /** * @param client {@link ClientModel} * @param container {@link ScopeContainerModel} @@ -629,8 +669,12 @@ public final class KeycloakModelUtils { // Used in various role mappers public static RoleModel getRoleFromString(RealmModel realm, String roleName) { + if (roleName == null) { + return null; + } + // Check client roles for all possible splits by dot - int scopeIndex = roleName.lastIndexOf('.'); + int scopeIndex = roleName.lastIndexOf(CLIENT_ROLE_SEPARATOR); while (scopeIndex >= 0) { String appName = roleName.substring(0, scopeIndex); ClientModel client = realm.getClientByClientId(appName); @@ -639,7 +683,7 @@ public final class KeycloakModelUtils { return client.getRole(role); } - scopeIndex = roleName.lastIndexOf('.', scopeIndex - 1); + scopeIndex = roleName.lastIndexOf(CLIENT_ROLE_SEPARATOR, scopeIndex - 1); } // determine if roleName is a realm role @@ -648,7 +692,7 @@ public final class KeycloakModelUtils { // Used for hardcoded role mappers public static String[] parseRole(String role) { - int scopeIndex = role.lastIndexOf('.'); + int scopeIndex = role.lastIndexOf(CLIENT_ROLE_SEPARATOR); if (scopeIndex > -1) { String appName = role.substring(0, scopeIndex); role = role.substring(scopeIndex + 1); @@ -661,6 +705,14 @@ public final class KeycloakModelUtils { } } + public static String buildRoleQualifier(String clientId, String roleName) { + if (clientId == null) { + return roleName; + } + + return clientId + CLIENT_ROLE_SEPARATOR + roleName; + } + /** * Check to see if a flow is currently in use * diff --git a/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java b/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java index 9419830321..b6b7058086 100755 --- a/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java +++ b/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java @@ -114,20 +114,10 @@ public class ModelToRepresentation { private static final Logger LOG = Logger.getLogger(ModelToRepresentation.class); - public static void buildGroupPath(StringBuilder sb, GroupModel group) { - if (group.getParent() != null) { - buildGroupPath(sb, group.getParent()); - } - sb.append('/').append(group.getName()); - } - public static String buildGroupPath(GroupModel group) { - StringBuilder sb = new StringBuilder(); - buildGroupPath(sb, group); - return sb.toString(); + return KeycloakModelUtils.buildGroupPath(group); } - public static GroupRepresentation groupToBriefRepresentation(GroupModel g) { return toRepresentation(g, false); } 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 3d883afb3a..7f62779da9 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 @@ -503,8 +503,10 @@ public class RepresentationToModel { } } - public static void updateClient(ClientRepresentation rep, ClientModel resource) { - if (rep.getClientId() != null) resource.setClientId(rep.getClientId()); + public static void updateClient(ClientRepresentation rep, ClientModel resource, KeycloakSession session) { + String newClientId = rep.getClientId(); + String previousClientId = resource.getClientId(); + if (newClientId != null) resource.setClientId(newClientId); if (rep.getName() != null) resource.setName(rep.getName()); if (rep.getDescription() != null) resource.setDescription(rep.getDescription()); if (rep.isEnabled() != null) resource.setEnabled(rep.isEnabled()); @@ -543,7 +545,7 @@ 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())); + resource.setAttribute("saml.artifact.binding.identifier", computeArtifactBindingIdentifierString(newClientId)); } if (rep.getAuthenticationFlowBindingOverrides() != null) { @@ -566,12 +568,12 @@ public class RepresentationToModel { List redirectUris = rep.getRedirectUris(); if (redirectUris != null) { - resource.setRedirectUris(new HashSet(redirectUris)); + resource.setRedirectUris(new HashSet<>(redirectUris)); } List webOrigins = rep.getWebOrigins(); if (webOrigins != null) { - resource.setWebOrigins(new HashSet(webOrigins)); + resource.setWebOrigins(new HashSet<>(webOrigins)); } if (rep.getRegisteredNodes() != null) { @@ -594,6 +596,31 @@ public class RepresentationToModel { } resource.updateClient(); + + if (!Objects.equals(newClientId, previousClientId)) { + ClientModel.ClientIdChangeEvent event = new ClientModel.ClientIdChangeEvent() { + @Override + public ClientModel getUpdatedClient() { + return resource; + } + + @Override + public String getPreviousClientId() { + return previousClientId; + } + + @Override + public String getNewClientId() { + return newClientId; + } + + @Override + public KeycloakSession getKeycloakSession() { + return session; + } + }; + session.getKeycloakSessionFactory().publish(event); + } } public static void updateClientProtocolMappers(ClientRepresentation rep, ClientModel resource) { diff --git a/server-spi-private/src/test/java/org/keycloak/models/KeycloakModelUtilsTest.java b/server-spi-private/src/test/java/org/keycloak/models/KeycloakModelUtilsTest.java new file mode 100644 index 0000000000..553eb59126 --- /dev/null +++ b/server-spi-private/src/test/java/org/keycloak/models/KeycloakModelUtilsTest.java @@ -0,0 +1,77 @@ +/* + * Copyright 2016 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.models; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.arrayWithSize; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; + +import org.junit.Test; +import org.keycloak.models.utils.KeycloakModelUtils; + +/** + * @author Daniel Fesenmeyer + */ +public class KeycloakModelUtilsTest { + + @Test + public void normalizeGroupPath() { + assertEquals("/test", KeycloakModelUtils.normalizeGroupPath("test")); + assertEquals("/test/x", KeycloakModelUtils.normalizeGroupPath("test/x/")); + assertEquals("", KeycloakModelUtils.normalizeGroupPath("")); + assertNull(KeycloakModelUtils.normalizeGroupPath(null)); + } + + @Test + public void buildRealmRoleQualifier() { + assertEquals("realm-role", KeycloakModelUtils.buildRoleQualifier(null, "realm-role")); + } + + @Test + public void buildClientRoleQualifier() { + assertEquals("my.client.id.role-name", + KeycloakModelUtils.buildRoleQualifier("my.client.id", "role-name")); + } + + @Test + public void parseRealmRoleQualifier() { + String[] clientIdAndRoleName = KeycloakModelUtils.parseRole("realm-role"); + + assertParsedRoleQualifier(clientIdAndRoleName, null, "realm-role"); + } + + @Test + public void parseClientRoleQualifier() { + String[] clientIdAndRoleName = KeycloakModelUtils.parseRole("my.client.id.role-name"); + + assertParsedRoleQualifier(clientIdAndRoleName, "my.client.id", "role-name"); + } + + private static void assertParsedRoleQualifier(String[] clientIdAndRoleName, String expectedClientId, + String expectedRoleName) { + + assertThat(clientIdAndRoleName, arrayWithSize(2)); + + String clientId = clientIdAndRoleName[0]; + assertEquals(expectedClientId, clientId); + String roleName = clientIdAndRoleName[1]; + assertEquals(expectedRoleName, roleName); + } + +} diff --git a/server-spi/src/main/java/org/keycloak/models/ClientModel.java b/server-spi/src/main/java/org/keycloak/models/ClientModel.java index ed39e764d4..da3cb8dbc4 100755 --- a/server-spi/src/main/java/org/keycloak/models/ClientModel.java +++ b/server-spi/src/main/java/org/keycloak/models/ClientModel.java @@ -66,6 +66,13 @@ public interface ClientModel extends ClientScopeModel, RoleContainerModel, Prot KeycloakSession getKeycloakSession(); } + interface ClientIdChangeEvent extends ProviderEvent { + ClientModel getUpdatedClient(); + String getPreviousClientId(); + String getNewClientId(); + KeycloakSession getKeycloakSession(); + } + interface ClientRemovedEvent extends ProviderEvent { ClientModel getClient(); KeycloakSession getKeycloakSession(); diff --git a/server-spi/src/main/java/org/keycloak/models/GroupModel.java b/server-spi/src/main/java/org/keycloak/models/GroupModel.java index bd7ccce1af..4a37306a94 100755 --- a/server-spi/src/main/java/org/keycloak/models/GroupModel.java +++ b/server-spi/src/main/java/org/keycloak/models/GroupModel.java @@ -57,7 +57,14 @@ public interface GroupModel extends RoleMapperModel { GroupModel getGroup(); KeycloakSession getKeycloakSession(); } - + + interface GroupPathChangeEvent extends ProviderEvent { + RealmModel getRealm(); + String getNewPath(); + String getPreviousPath(); + KeycloakSession getKeycloakSession(); + } + Comparator COMPARE_BY_NAME = Comparator.comparing(GroupModel::getName); String getId(); diff --git a/server-spi/src/main/java/org/keycloak/models/RoleModel.java b/server-spi/src/main/java/org/keycloak/models/RoleModel.java index cc36641ea7..9aa7550187 100755 --- a/server-spi/src/main/java/org/keycloak/models/RoleModel.java +++ b/server-spi/src/main/java/org/keycloak/models/RoleModel.java @@ -17,6 +17,7 @@ package org.keycloak.models; +import org.keycloak.provider.ProviderEvent; import org.keycloak.storage.SearchableModelField; import java.util.List; import java.util.Map; @@ -41,6 +42,18 @@ public interface RoleModel { public static final SearchableModelField COMPOSITE_ROLE = new SearchableModelField<>("compositeRoles", Boolean.class); } + interface RoleNameChangeEvent extends ProviderEvent { + RealmModel getRealm(); + String getNewName(); + String getPreviousName(); + + /** + * @return the Client ID of the client, for a client role; {@code null}, for a realm role + */ + String getClientId(); + KeycloakSession getKeycloakSession(); + } + String getName(); String getDescription(); diff --git a/services/src/main/java/org/keycloak/broker/oidc/mappers/AbstractClaimToGroupMapper.java b/services/src/main/java/org/keycloak/broker/oidc/mappers/AbstractClaimToGroupMapper.java index ba8b69260d..560bd26915 100644 --- a/services/src/main/java/org/keycloak/broker/oidc/mappers/AbstractClaimToGroupMapper.java +++ b/services/src/main/java/org/keycloak/broker/oidc/mappers/AbstractClaimToGroupMapper.java @@ -17,9 +17,9 @@ package org.keycloak.broker.oidc.mappers; +import org.jboss.logging.Logger; import org.keycloak.broker.provider.BrokeredIdentityContext; import org.keycloak.broker.provider.ConfigConstants; -import org.keycloak.broker.provider.IdentityBrokerException; import org.keycloak.models.GroupModel; import org.keycloak.models.IdentityProviderMapperModel; import org.keycloak.models.KeycloakSession; @@ -27,13 +27,24 @@ import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; import org.keycloak.models.utils.KeycloakModelUtils; +/** + * @author Artur Baltabayev, + * Daniel Fesenmeyer + */ public abstract class AbstractClaimToGroupMapper extends AbstractClaimMapper { + private static final Logger LOG = Logger.getLogger(AbstractClaimToGroupMapper.class); + + @Override public void importNewUser(KeycloakSession session, RealmModel realm, UserModel user, IdentityProviderMapperModel mapperModel, BrokeredIdentityContext context) { GroupModel group = this.getGroup(realm, mapperModel); + if (group == null) { + return; + } + if (applies(mapperModel, context)) { user.joinGroup(group); } @@ -44,8 +55,11 @@ public abstract class AbstractClaimToGroupMapper extends AbstractClaimMapper { IdentityProviderMapperModel mapperModel, BrokeredIdentityContext context) { GroupModel group = this.getGroup(realm, mapperModel); - String groupId = mapperModel.getConfig().get(ConfigConstants.GROUP); + if (group == null) { + return; + } + String groupId = group.getId(); if (!context.hasMapperAssignedGroup(groupId)) { if (applies(mapperModel, context)) { context.addMapperAssignedGroup(groupId); @@ -67,22 +81,16 @@ public abstract class AbstractClaimToGroupMapper extends AbstractClaimMapper { protected abstract boolean applies(final IdentityProviderMapperModel mapperModel, final BrokeredIdentityContext context); - /** - * Obtains the {@link GroupModel} corresponding the group configured in the specified - * {@link IdentityProviderMapperModel}. - * If the group doesn't exist, this method throws an {@link IdentityBrokerException}. - * - * @param realm a reference to the realm. - * @param mapperModel a reference to the {@link IdentityProviderMapperModel} containing the configured group. - * @return the {@link GroupModel} - * @throws IdentityBrokerException if the group doesn't exist. - */ private GroupModel getGroup(final RealmModel realm, final IdentityProviderMapperModel mapperModel) { - GroupModel group = KeycloakModelUtils.findGroupByPath(realm, mapperModel.getConfig().get(ConfigConstants.GROUP)); + String groupPath = mapperModel.getConfig().get(ConfigConstants.GROUP); + GroupModel group = KeycloakModelUtils.findGroupByPath(realm, groupPath); if (group == null) { - throw new IdentityBrokerException("Unable to find group: " + group.getId()); + LOG.warnf("Unable to find group by path '%s' referenced by mapper '%s' on realm '%s'.", groupPath, + mapperModel.getName(), realm.getName()); } + return group; } + } diff --git a/services/src/main/java/org/keycloak/broker/oidc/mappers/AbstractClaimToRoleMapper.java b/services/src/main/java/org/keycloak/broker/oidc/mappers/AbstractClaimToRoleMapper.java index 2de039e6ac..6f0bc7666e 100644 --- a/services/src/main/java/org/keycloak/broker/oidc/mappers/AbstractClaimToRoleMapper.java +++ b/services/src/main/java/org/keycloak/broker/oidc/mappers/AbstractClaimToRoleMapper.java @@ -16,9 +16,9 @@ */ package org.keycloak.broker.oidc.mappers; +import org.jboss.logging.Logger; import org.keycloak.broker.provider.BrokeredIdentityContext; import org.keycloak.broker.provider.ConfigConstants; -import org.keycloak.broker.provider.IdentityBrokerException; import org.keycloak.models.IdentityProviderMapperModel; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; @@ -30,13 +30,20 @@ import org.keycloak.models.utils.KeycloakModelUtils; * Abstract class that handles the logic for importing and updating brokered users for all mappers that map an OIDC * claim into a {@code Keycloak} role. * - * @author Stefan Guilhen + * @author Stefan Guilhen, + * Daniel Fesenmeyer */ public abstract class AbstractClaimToRoleMapper extends AbstractClaimMapper { + private static final Logger LOG = Logger.getLogger(AbstractClaimToRoleMapper.class); + @Override public void importNewUser(KeycloakSession session, RealmModel realm, UserModel user, IdentityProviderMapperModel mapperModel, BrokeredIdentityContext context) { - RoleModel role = this.getRole(realm, mapperModel); + RoleModel role = getRole(realm, mapperModel); + if (role == null) { + return; + } + if (applies(mapperModel, context)) { user.grantRole(role); } @@ -44,7 +51,11 @@ public abstract class AbstractClaimToRoleMapper extends AbstractClaimMapper { @Override public void updateBrokeredUserLegacy(KeycloakSession session, RealmModel realm, UserModel user, IdentityProviderMapperModel mapperModel, BrokeredIdentityContext context) { - RoleModel role = this.getRole(realm, mapperModel); + RoleModel role = getRole(realm, mapperModel); + if (role == null) { + return; + } + if (!applies(mapperModel, context)) { user.deleteRoleMapping(role); } @@ -52,7 +63,11 @@ public abstract class AbstractClaimToRoleMapper extends AbstractClaimMapper { @Override public void updateBrokeredUser(KeycloakSession session, RealmModel realm, UserModel user, IdentityProviderMapperModel mapperModel, BrokeredIdentityContext context) { - RoleModel role = this.getRole(realm, mapperModel); + RoleModel role = getRole(realm, mapperModel); + if (role == null) { + return; + } + String roleName = mapperModel.getConfig().get(ConfigConstants.ROLE); // KEYCLOAK-8730 if a previous mapper has already granted the same role, skip the checks so we don't accidentally remove a valid role. if (!context.hasMapperGrantedRole(roleName)) { @@ -77,22 +92,24 @@ public abstract class AbstractClaimToRoleMapper extends AbstractClaimMapper { protected abstract boolean applies(final IdentityProviderMapperModel mapperModel, final BrokeredIdentityContext context); /** - * Obtains the {@link RoleModel} corresponding the role configured in the specified {@link IdentityProviderMapperModel}. - * If the role doesn't correspond to one of the realm's client roles or to one of the realm's roles, this method throws - * an {@link IdentityBrokerException} to convey that an invalid role was configured. + * Obtains the {@link RoleModel} corresponding the role configured in the specified + * {@link IdentityProviderMapperModel}. + * If the role doesn't correspond to one of the realm's client roles or to one of the realm's roles, this method + * returns {@code null}. * * @param realm a reference to the realm. * @param mapperModel a reference to the {@link IdentityProviderMapperModel} containing the configured role. - * @return the {@link RoleModel} that corresponds to the mapper model role. - * @throws IdentityBrokerException if the role name doesn't correspond to one of the realm's client roles or to one - * of the realm's roles. + * @return the {@link RoleModel} that corresponds to the mapper model role; {@code null}, when role was not found */ private RoleModel getRole(final RealmModel realm, final IdentityProviderMapperModel mapperModel) { String roleName = mapperModel.getConfig().get(ConfigConstants.ROLE); RoleModel role = KeycloakModelUtils.getRoleFromString(realm, roleName); + if (role == null) { - throw new IdentityBrokerException("Unable to find role: " + roleName); + LOG.warnf("Unable to find role '%s' referenced by mapper '%s' on realm '%s'.", roleName, + mapperModel.getName(), realm.getName()); } + return role; } } diff --git a/services/src/main/java/org/keycloak/broker/oidc/mappers/AdvancedClaimToGroupMapper.java b/services/src/main/java/org/keycloak/broker/oidc/mappers/AdvancedClaimToGroupMapper.java index 7241c0b223..b90f9dfa4a 100644 --- a/services/src/main/java/org/keycloak/broker/oidc/mappers/AdvancedClaimToGroupMapper.java +++ b/services/src/main/java/org/keycloak/broker/oidc/mappers/AdvancedClaimToGroupMapper.java @@ -26,11 +26,8 @@ import org.keycloak.models.IdentityProviderSyncMode; import org.keycloak.provider.ProviderConfigProperty; import java.util.ArrayList; -import java.util.Arrays; -import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Set; import static org.keycloak.utils.RegexUtils.valueMatchesRegex; diff --git a/services/src/main/java/org/keycloak/broker/provider/HardcodedRoleMapper.java b/services/src/main/java/org/keycloak/broker/provider/HardcodedRoleMapper.java index e613ad0e46..24278ffd82 100755 --- a/services/src/main/java/org/keycloak/broker/provider/HardcodedRoleMapper.java +++ b/services/src/main/java/org/keycloak/broker/provider/HardcodedRoleMapper.java @@ -17,6 +17,7 @@ package org.keycloak.broker.provider; +import org.jboss.logging.Logger; import org.keycloak.models.IdentityProviderMapperModel; import org.keycloak.models.IdentityProviderSyncMode; import org.keycloak.models.KeycloakSession; @@ -37,8 +38,12 @@ import java.util.Set; * @version $Revision: 1 $ */ public class HardcodedRoleMapper extends AbstractIdentityProviderMapper { - protected static final List configProperties = new ArrayList(); - private static final Set IDENTITY_PROVIDER_SYNC_MODES = new HashSet<>(Arrays.asList(IdentityProviderSyncMode.values())); + protected static final List configProperties = new ArrayList<>(); + + private static final Logger LOG = Logger.getLogger(HardcodedRoleMapper.class); + + private static final Set IDENTITY_PROVIDER_SYNC_MODES = + new HashSet<>(Arrays.asList(IdentityProviderSyncMode.values())); static { ProviderConfigProperty property; @@ -91,10 +96,22 @@ public class HardcodedRoleMapper extends AbstractIdentityProviderMapper { } private void grantUserRole(RealmModel realm, UserModel user, IdentityProviderMapperModel mapperModel) { + RoleModel role = getRole(realm, mapperModel); + if (role != null) { + user.grantRole(role); + } + } + + private RoleModel getRole(final RealmModel realm, final IdentityProviderMapperModel mapperModel) { String roleName = mapperModel.getConfig().get(ConfigConstants.ROLE); RoleModel role = KeycloakModelUtils.getRoleFromString(realm, roleName); - if (role == null) throw new IdentityBrokerException("Unable to find role: " + roleName); - user.grantRole(role); + + if (role == null) { + LOG.warnf("Unable to find role '%s' referenced by mapper '%s' on realm '%s'.", roleName, + mapperModel.getName(), realm.getName()); + } + + return role; } @Override diff --git a/services/src/main/java/org/keycloak/broker/saml/mappers/AbstractAttributeToRoleMapper.java b/services/src/main/java/org/keycloak/broker/saml/mappers/AbstractAttributeToRoleMapper.java index 3981b6e26f..28e28adc3f 100644 --- a/services/src/main/java/org/keycloak/broker/saml/mappers/AbstractAttributeToRoleMapper.java +++ b/services/src/main/java/org/keycloak/broker/saml/mappers/AbstractAttributeToRoleMapper.java @@ -16,10 +16,10 @@ */ package org.keycloak.broker.saml.mappers; +import org.jboss.logging.Logger; import org.keycloak.broker.provider.AbstractIdentityProviderMapper; import org.keycloak.broker.provider.BrokeredIdentityContext; import org.keycloak.broker.provider.ConfigConstants; -import org.keycloak.broker.provider.IdentityBrokerException; import org.keycloak.models.IdentityProviderMapperModel; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; @@ -31,13 +31,20 @@ import org.keycloak.models.utils.KeycloakModelUtils; * Abstract class that handles the logic for importing and updating brokered users for all mappers that map a SAML * attribute into a {@code Keycloak} role. * - * @author Stefan Guilhen + * @author Stefan Guilhen, + * Daniel Fesenmeyer */ public abstract class AbstractAttributeToRoleMapper extends AbstractIdentityProviderMapper { + private static final Logger LOG = Logger.getLogger(AbstractAttributeToRoleMapper.class); + @Override public void importNewUser(KeycloakSession session, RealmModel realm, UserModel user, IdentityProviderMapperModel mapperModel, BrokeredIdentityContext context) { RoleModel role = this.getRole(realm, mapperModel); + if (role == null) { + return; + } + if (this.applies(mapperModel, context)) { user.grantRole(role); } @@ -46,6 +53,10 @@ public abstract class AbstractAttributeToRoleMapper extends AbstractIdentityProv @Override public void updateBrokeredUser(KeycloakSession session, RealmModel realm, UserModel user, IdentityProviderMapperModel mapperModel, BrokeredIdentityContext context) { RoleModel role = this.getRole(realm, mapperModel); + if (role == null) { + return; + } + String roleName = mapperModel.getConfig().get(ConfigConstants.ROLE); // KEYCLOAK-8730 if a previous mapper has already granted the same role, skip the checks so we don't accidentally remove a valid role. if (!context.hasMapperGrantedRole(roleName)) { @@ -69,21 +80,22 @@ public abstract class AbstractAttributeToRoleMapper extends AbstractIdentityProv protected abstract boolean applies(final IdentityProviderMapperModel mapperModel, final BrokeredIdentityContext context); /** - * Obtains the {@link RoleModel} corresponding the role configured in the specified {@link IdentityProviderMapperModel}. - * If the role doesn't correspond to one of the realm's client roles or to one of the realm's roles, this method throws - * an {@link IdentityBrokerException} to convey that an invalid role was configured. + * Obtains the {@link RoleModel} corresponding the role configured in the specified + * {@link IdentityProviderMapperModel}. + * If the role doesn't correspond to one of the realm's client roles or to one of the realm's roles, this method + * returns {@code null}. * * @param realm a reference to the realm. * @param mapperModel a reference to the {@link IdentityProviderMapperModel} containing the configured role. - * @return the {@link RoleModel} that corresponds to the mapper model role. - * @throws IdentityBrokerException if the role name doesn't correspond to one of the realm's client roles or to one - * of the realm's roles. + * @return the {@link RoleModel} that corresponds to the mapper model role or {@code null}, if the role could not be + * found */ private RoleModel getRole(final RealmModel realm, final IdentityProviderMapperModel mapperModel) { String roleName = mapperModel.getConfig().get(ConfigConstants.ROLE); RoleModel role = KeycloakModelUtils.getRoleFromString(realm, roleName); if (role == null) { - throw new IdentityBrokerException("Unable to find role: " + roleName); + LOG.warnf("Unable to find role '%s' for mapper '%s' on realm '%s'.", roleName, mapperModel.getName(), + realm.getName()); } return role; } diff --git a/services/src/main/java/org/keycloak/services/clientregistration/AbstractClientRegistrationProvider.java b/services/src/main/java/org/keycloak/services/clientregistration/AbstractClientRegistrationProvider.java index 5dfb35d775..523e4a16c0 100755 --- a/services/src/main/java/org/keycloak/services/clientregistration/AbstractClientRegistrationProvider.java +++ b/services/src/main/java/org/keycloak/services/clientregistration/AbstractClientRegistrationProvider.java @@ -148,7 +148,7 @@ public abstract class AbstractClientRegistrationProvider implements ClientRegist throw new ErrorResponseException(ErrorCodes.INVALID_CLIENT_METADATA, "Client Identifier modified", Response.Status.BAD_REQUEST); } - RepresentationToModel.updateClient(rep, client); + RepresentationToModel.updateClient(rep, client, session); RepresentationToModel.updateClientProtocolMappers(rep, client); if (rep.getDefaultRoles() != null) { 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 dda1cee7f2..054032c4c9 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 @@ -777,7 +777,7 @@ public class ClientResource { rep.setAuthorizationServicesEnabled(false); } - RepresentationToModel.updateClient(rep, client); + RepresentationToModel.updateClient(rep, client, session); RepresentationToModel.updateClientProtocolMappers(rep, client); updateAuthorizationSettings(rep); } diff --git a/services/src/main/java/org/keycloak/services/resources/admin/GroupResource.java b/services/src/main/java/org/keycloak/services/resources/admin/GroupResource.java index e71aa403e1..684db87eef 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/GroupResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/GroupResource.java @@ -26,6 +26,7 @@ import org.keycloak.models.Constants; import org.keycloak.models.GroupModel; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; +import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.models.utils.ModelToRepresentation; import org.keycloak.representations.idm.GroupRepresentation; import org.keycloak.representations.idm.ManagementPermissionReference; @@ -114,7 +115,7 @@ public class GroupResource { } } - updateGroup(rep, group); + updateGroup(rep, group, realm, session); adminEvent.operation(OperationType.UPDATE).resourcePath(session.getContext().getUri()).representation(rep).success(); return Response.noContent().build(); @@ -167,7 +168,7 @@ public class GroupResource { adminEvent.operation(OperationType.UPDATE); } else { child = realm.createGroup(groupName, group); - updateGroup(rep, child); + updateGroup(rep, child, realm, session); URI uri = session.getContext().getUri().getBaseUriBuilder() .path(session.getContext().getUri().getMatchedURIs().get(2)) .path(child.getId()).build(); @@ -182,8 +183,42 @@ public class GroupResource { return builder.type(MediaType.APPLICATION_JSON_TYPE).entity(childRep).build(); } - public static void updateGroup(GroupRepresentation rep, GroupModel model) { - if (rep.getName() != null) model.setName(rep.getName()); + public static void updateGroup(GroupRepresentation rep, GroupModel model, RealmModel realm, KeycloakSession session) { + String newName = rep.getName(); + if (newName != null) { + String existingName = model.getName(); + if (!newName.equals(existingName)) { + String previousPath = KeycloakModelUtils.buildGroupPath(model); + + model.setName(newName); + + String newPath = KeycloakModelUtils.buildGroupPath(model); + + GroupModel.GroupPathChangeEvent event = + new GroupModel.GroupPathChangeEvent() { + @Override + public RealmModel getRealm() { + return realm; + } + + @Override + public String getNewPath() { + return newPath; + } + + @Override + public String getPreviousPath() { + return previousPath; + } + + @Override + public KeycloakSession getKeycloakSession() { + return session; + } + }; + session.getKeycloakSessionFactory().publish(event); + } + } if (rep.getAttributes() != null) { Set attrsToRemove = new HashSet<>(model.getAttributes().keySet()); diff --git a/services/src/main/java/org/keycloak/services/resources/admin/GroupsResource.java b/services/src/main/java/org/keycloak/services/resources/admin/GroupsResource.java index a34fdb181c..e10d04f98f 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/GroupsResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/GroupsResource.java @@ -163,7 +163,7 @@ public class GroupsResource { adminEvent.operation(OperationType.UPDATE).resourcePath(session.getContext().getUri()); } else { child = realm.createGroup(groupName); - GroupResource.updateGroup(rep, child); + GroupResource.updateGroup(rep, child, realm, session); URI uri = session.getContext().getUri().getAbsolutePathBuilder() .path(child.getId()).build(); builder.status(201).location(uri); diff --git a/services/src/main/java/org/keycloak/services/resources/admin/RoleByIdResource.java b/services/src/main/java/org/keycloak/services/resources/admin/RoleByIdResource.java index 2749b79f61..cf83e29fc7 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/RoleByIdResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/RoleByIdResource.java @@ -139,7 +139,7 @@ public class RoleByIdResource extends RoleResource { public void updateRole(final @PathParam("role-id") String id, final RoleRepresentation rep) { RoleModel role = getRoleModel(id); auth.roles().requireManage(role); - updateRole(rep, role); + updateRole(rep, role, realm, session); if (role.isClientRole()) { adminEvent.resource(ResourceType.CLIENT_ROLE); diff --git a/services/src/main/java/org/keycloak/services/resources/admin/RoleContainerResource.java b/services/src/main/java/org/keycloak/services/resources/admin/RoleContainerResource.java index 23a40b2a9a..bb8bd03db6 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/RoleContainerResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/RoleContainerResource.java @@ -269,7 +269,7 @@ public class RoleContainerResource extends RoleResource { throw new NotFoundException("Could not find role"); } try { - updateRole(rep, role); + updateRole(rep, role, realm, session); if (role.isClientRole()) { adminEvent.resource(ResourceType.CLIENT_ROLE); diff --git a/services/src/main/java/org/keycloak/services/resources/admin/RoleResource.java b/services/src/main/java/org/keycloak/services/resources/admin/RoleResource.java index b66850db69..ed84e2e343 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/RoleResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/RoleResource.java @@ -20,6 +20,7 @@ package org.keycloak.services.resources.admin; import org.keycloak.events.admin.OperationType; import org.keycloak.events.admin.ResourceType; import org.keycloak.models.ClientModel; +import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.models.RoleModel; import org.keycloak.models.utils.ModelToRepresentation; @@ -57,8 +58,45 @@ public abstract class RoleResource { } } - protected void updateRole(RoleRepresentation rep, RoleModel role) { - role.setName(rep.getName()); + protected void updateRole(RoleRepresentation rep, RoleModel role, RealmModel realm, + KeycloakSession session) { + String newName = rep.getName(); + String previousName = role.getName(); + if (!Objects.equals(previousName, newName)) { + role.setName(newName); + + session.getKeycloakSessionFactory().publish(new RoleModel.RoleNameChangeEvent() { + @Override + public RealmModel getRealm() { + return realm; + } + + @Override + public String getNewName() { + return newName; + } + + @Override + public String getPreviousName() { + return previousName; + } + + @Override + public String getClientId() { + if (!role.isClientRole()) { + return null; + } + + return ((ClientModel) role.getContainer()).getClientId(); + } + + @Override + public KeycloakSession getKeycloakSession() { + return session; + } + }); + } + role.setDescription(rep.getDescription()); if (rep.getAttributes() != null) { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractAdvancedGroupMapperTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractAdvancedGroupMapperTest.java index cda5d21817..35e82fcd26 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractAdvancedGroupMapperTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractAdvancedGroupMapperTest.java @@ -3,10 +3,8 @@ package org.keycloak.testsuite.broker; import static org.keycloak.models.IdentityProviderMapperSyncMode.FORCE; import static org.keycloak.models.IdentityProviderMapperSyncMode.IMPORT; -import org.junit.Before; import org.junit.Test; import org.keycloak.models.IdentityProviderMapperSyncMode; -import org.keycloak.representations.idm.GroupRepresentation; import org.keycloak.representations.idm.IdentityProviderRepresentation; import org.keycloak.representations.idm.UserRepresentation; @@ -14,7 +12,12 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import java.util.List; +import java.util.Map; +/** + * @author Artur Baltabayev, + * Daniel Fesenmeyer + */ public abstract class AbstractAdvancedGroupMapperTest extends AbstractGroupMapperTest { private static final String CLAIMS_OR_ATTRIBUTES = "[\n" + @@ -41,24 +44,12 @@ public abstract class AbstractAdvancedGroupMapperTest extends AbstractGroupMappe private String newValueForAttribute2 = ""; - @Before - public void addMapperTestGroupToConsumerRealm() { - GroupRepresentation mapperTestGroup = new GroupRepresentation(); - mapperTestGroup.setName(MAPPER_TEST_GROUP_NAME); - mapperTestGroup.setPath(MAPPER_TEST_GROUP_PATH); - - adminClient.realm(bc.consumerRealmName()).groups().add(mapperTestGroup); - } - @Test public void allValuesMatch() { - createAdvancedGroupMapper(CLAIMS_OR_ATTRIBUTES, false); - createUserInProviderRealm(ImmutableMap.>builder() - .put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME, ImmutableList.builder().add("value 1").build()) - .put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME_2, ImmutableList.builder().add("value 2").build()) - .build()); + createAdvancedGroupMapper(CLAIMS_OR_ATTRIBUTES, false, MAPPER_TEST_GROUP_PATH); + createUserInProviderRealm(createMatchingAttributes()); - logInAsUserInIDPForFirstTime(); + logInAsUserInIDPForFirstTimeAndAssertSuccess(); UserRepresentation user = findUser(bc.consumerRealmName(), bc.getUserLogin(), bc.getUserEmail()); assertThatUserHasBeenAssignedToGroup(user); @@ -66,13 +57,13 @@ public abstract class AbstractAdvancedGroupMapperTest extends AbstractGroupMappe @Test public void valuesMismatch() { - createAdvancedGroupMapper(CLAIMS_OR_ATTRIBUTES, false); + createAdvancedGroupMapper(CLAIMS_OR_ATTRIBUTES, false, MAPPER_TEST_GROUP_PATH); createUserInProviderRealm(ImmutableMap.>builder() .put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME, ImmutableList.builder().add("value 1").build()) .put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME_2, ImmutableList.builder().add("value mismatch").build()) .build()); - logInAsUserInIDPForFirstTime(); + logInAsUserInIDPForFirstTimeAndAssertSuccess(); UserRepresentation user = findUser(bc.consumerRealmName(), bc.getUserLogin(), bc.getUserEmail()); assertThatUserHasNotBeenAssignedToGroup(user); @@ -80,13 +71,13 @@ public abstract class AbstractAdvancedGroupMapperTest extends AbstractGroupMappe @Test public void valuesMatchIfNoClaimsSpecified() { - createAdvancedGroupMapper("[]", false); + createAdvancedGroupMapper("[]", false, MAPPER_TEST_GROUP_PATH); createUserInProviderRealm(ImmutableMap.>builder() .put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME, ImmutableList.builder().add("some value").build()) .put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME_2, ImmutableList.builder().add("some value").build()) .build()); - logInAsUserInIDPForFirstTime(); + logInAsUserInIDPForFirstTimeAndAssertSuccess(); UserRepresentation user = findUser(bc.consumerRealmName(), bc.getUserLogin(), bc.getUserEmail()); assertThatUserHasBeenAssignedToGroup(user); @@ -94,13 +85,10 @@ public abstract class AbstractAdvancedGroupMapperTest extends AbstractGroupMappe @Test public void allValuesMatchRegex() { - createAdvancedGroupMapper(CLAIMS_OR_ATTRIBUTES_REGEX, true); - createUserInProviderRealm(ImmutableMap.>builder() - .put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME, ImmutableList.builder().add("value 1").build()) - .put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME_2, ImmutableList.builder().add("value 2").build()) - .build()); + createAdvancedGroupMapper(CLAIMS_OR_ATTRIBUTES_REGEX, true, MAPPER_TEST_GROUP_PATH); + createUserInProviderRealm(createMatchingAttributes()); - logInAsUserInIDPForFirstTime(); + logInAsUserInIDPForFirstTimeAndAssertSuccess(); UserRepresentation user = findUser(bc.consumerRealmName(), bc.getUserLogin(), bc.getUserEmail()); assertThatUserHasBeenAssignedToGroup(user); @@ -108,13 +96,13 @@ public abstract class AbstractAdvancedGroupMapperTest extends AbstractGroupMappe @Test public void valuesMismatchRegex() { - createAdvancedGroupMapper(CLAIMS_OR_ATTRIBUTES_REGEX, true); + createAdvancedGroupMapper(CLAIMS_OR_ATTRIBUTES_REGEX, true, MAPPER_TEST_GROUP_PATH); createUserInProviderRealm(ImmutableMap.>builder() .put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME, ImmutableList.builder().add("mismatch").build()) .put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME_2, ImmutableList.builder().add("value 2").build()) .build()); - logInAsUserInIDPForFirstTime(); + logInAsUserInIDPForFirstTimeAndAssertSuccess(); UserRepresentation user = findUser(bc.consumerRealmName(), bc.getUserLogin(), bc.getUserEmail()); assertThatUserHasNotBeenAssignedToGroup(user); @@ -123,7 +111,7 @@ public abstract class AbstractAdvancedGroupMapperTest extends AbstractGroupMappe @Test public void updateBrokeredUserMismatchLeavesGroup() { newValueForAttribute2 = "value mismatch"; - UserRepresentation user = createMapperAndLoginAsUserTwiceWithMapper(FORCE, false); + UserRepresentation user = createMapperAndLoginAsUserTwiceWithMapper(FORCE, false, MAPPER_TEST_GROUP_PATH); assertThatUserHasNotBeenAssignedToGroup(user); } @@ -131,7 +119,7 @@ public abstract class AbstractAdvancedGroupMapperTest extends AbstractGroupMappe @Test public void updateBrokeredUserMismatchDoesNotLeaveGroupInImportMode() { newValueForAttribute2 = "value mismatch"; - UserRepresentation user = createMapperAndLoginAsUserTwiceWithMapper(IMPORT, false); + UserRepresentation user = createMapperAndLoginAsUserTwiceWithMapper(IMPORT, false, MAPPER_TEST_GROUP_PATH); assertThatUserHasBeenAssignedToGroup(user); } @@ -139,24 +127,31 @@ public abstract class AbstractAdvancedGroupMapperTest extends AbstractGroupMappe @Test public void updateBrokeredUserMatchDoesntLeaveGroup() { newValueForAttribute2 = "value 2"; - UserRepresentation user = createMapperAndLoginAsUserTwiceWithMapper(FORCE, false); + UserRepresentation user = createMapperAndLoginAsUserTwiceWithMapper(FORCE, false, MAPPER_TEST_GROUP_PATH); assertThatUserHasBeenAssignedToGroup(user); } + @Test + public void tryToUpdateBrokeredUserWithMissingGroupDoesNotBreakLogin() { + newValueForAttribute2 = "value 2"; + UserRepresentation user = + createMapperAndLoginAsUserTwiceWithMapper(FORCE, true, MAPPER_TEST_NOT_EXISTING_GROUP_PATH); + + assertThatUserDoesNotHaveGroups(user); + } + @Test public void updateBrokeredUserIsAssignedToGroupInForceModeWhenCreatingTheMapperAfterFirstLogin() { newValueForAttribute2 = "value 2"; - UserRepresentation user = createMapperAndLoginAsUserTwiceWithMapper(FORCE, true); + UserRepresentation user = createMapperAndLoginAsUserTwiceWithMapper(FORCE, true, MAPPER_TEST_GROUP_PATH); assertThatUserHasBeenAssignedToGroup(user); } - public UserRepresentation createMapperAndLoginAsUserTwiceWithMapper(IdentityProviderMapperSyncMode syncMode, boolean createAfterFirstLogin) { - return loginAsUserTwiceWithMapper(syncMode, createAfterFirstLogin, ImmutableMap.>builder() - .put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME, ImmutableList.builder().add("value 1").build()) - .put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME_2, ImmutableList.builder().add("value 2").build()) - .build()); + public UserRepresentation createMapperAndLoginAsUserTwiceWithMapper(IdentityProviderMapperSyncMode syncMode, + boolean createAfterFirstLogin, String groupPath) { + return loginAsUserTwiceWithMapper(syncMode, createAfterFirstLogin, createMatchingAttributes(), groupPath); } @Override @@ -171,16 +166,44 @@ public abstract class AbstractAdvancedGroupMapperTest extends AbstractGroupMappe adminClient.realm(bc.providerRealmName()).users().get(user.getId()).update(user); } + + @Override - protected void createMapperInIdp(IdentityProviderRepresentation idp, IdentityProviderMapperSyncMode syncMode) { - createMapperInIdp(idp, CLAIMS_OR_ATTRIBUTES, false, syncMode); + protected String createMapperInIdp(IdentityProviderRepresentation idp, IdentityProviderMapperSyncMode syncMode, + String groupPath) { + return createMapperInIdp(idp, CLAIMS_OR_ATTRIBUTES, false, syncMode, groupPath); } - protected void createAdvancedGroupMapper(String claimsOrAttributeRepresentation, boolean areClaimsOrAttributeValuesRegexes) { + @Override + protected String setupScenarioWithMatchingGroup() { + String mapperId = createAdvancedGroupMapper(CLAIMS_OR_ATTRIBUTES, false, MAPPER_TEST_GROUP_PATH); + createUserInProviderRealm(createMatchingAttributes()); + return mapperId; + } + + @Override + protected void setupScenarioWithNonExistingGroup() { + createAdvancedGroupMapper(CLAIMS_OR_ATTRIBUTES, false, MAPPER_TEST_NOT_EXISTING_GROUP_PATH); + createUserInProviderRealm(createMatchingAttributes()); + } + + protected String createAdvancedGroupMapper(String claimsOrAttributeRepresentation, + boolean areClaimsOrAttributeValuesRegexes, String groupPath) { IdentityProviderRepresentation idp = setupIdentityProvider(); - createMapperInIdp(idp, claimsOrAttributeRepresentation, areClaimsOrAttributeValuesRegexes, IMPORT); + return createMapperInIdp(idp, claimsOrAttributeRepresentation, areClaimsOrAttributeValuesRegexes, IMPORT, + groupPath); } - abstract protected void createMapperInIdp( - IdentityProviderRepresentation idp, String claimsOrAttributeRepresentation, boolean areClaimsOrAttributeValuesRegexes, IdentityProviderMapperSyncMode syncMode); + abstract protected String createMapperInIdp( + IdentityProviderRepresentation idp, String claimsOrAttributeRepresentation, + boolean areClaimsOrAttributeValuesRegexes, IdentityProviderMapperSyncMode syncMode, String groupPath); + + private static Map> createMatchingAttributes() { + return ImmutableMap.> builder() + .put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME, + ImmutableList. builder().add("value 1").build()) + .put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME_2, + ImmutableList. builder().add("value 2").build()) + .build(); + } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractAdvancedRoleMapperTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractAdvancedRoleMapperTest.java index b7c70e512b..879a31319e 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractAdvancedRoleMapperTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractAdvancedRoleMapperTest.java @@ -1,21 +1,23 @@ package org.keycloak.testsuite.broker; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; -import org.junit.Test; -import org.keycloak.models.IdentityProviderMapperSyncMode; -import org.keycloak.representations.idm.IdentityProviderRepresentation; -import org.keycloak.representations.idm.UserRepresentation; - -import java.util.List; - import static org.keycloak.models.IdentityProviderMapperSyncMode.FORCE; import static org.keycloak.models.IdentityProviderMapperSyncMode.IMPORT; +import org.junit.Test; +import org.keycloak.models.IdentityProviderMapperSyncMode; +import org.keycloak.representations.idm.UserRepresentation; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; + +import java.util.List; +import java.util.Map; + /** * @author hmlnarik, * Benjamin Weimer, - * Martin Idel + * Martin Idel, + * Daniel Fesenmeyer */ public abstract class AbstractAdvancedRoleMapperTest extends AbstractRoleMapperTest { @@ -46,134 +48,145 @@ public abstract class AbstractAdvancedRoleMapperTest extends AbstractRoleMapperT @Test public void allValuesMatch() { createAdvancedRoleMapper(CLAIMS_OR_ATTRIBUTES, false); - createUserInProviderRealm(ImmutableMap.>builder() - .put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME, ImmutableList.builder().add("value 1").build()) - .put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME_2, ImmutableList.builder().add("value 2").build()) - .build()); + createUserInProviderRealm(createMatchingUserConfig()); logInAsUserInIDPForFirstTime(); - UserRepresentation user = findUser(bc.consumerRealmName(), bc.getUserLogin(), bc.getUserEmail()); - assertThatRoleHasBeenAssignedInConsumerRealmTo(user); + assertThatRoleHasBeenAssignedInConsumerRealm(); } @Test public void valuesMismatch() { createAdvancedRoleMapper(CLAIMS_OR_ATTRIBUTES, false); - createUserInProviderRealm(ImmutableMap.>builder() - .put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME, ImmutableList.builder().add("value 1").build()) - .put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME_2, ImmutableList.builder().add("value mismatch").build()) + createUserInProviderRealm(ImmutableMap.> builder() + .put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME, + ImmutableList. builder().add("value 1").build()) + .put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME_2, + ImmutableList. builder().add("value mismatch").build()) .build()); logInAsUserInIDPForFirstTime(); - UserRepresentation user = findUser(bc.consumerRealmName(), bc.getUserLogin(), bc.getUserEmail()); - assertThatRoleHasNotBeenAssignedInConsumerRealmTo(user); + assertThatRoleHasNotBeenAssignedInConsumerRealm(); } @Test public void valuesMatchIfNoClaimsSpecified() { createAdvancedRoleMapper("[]", false); - createUserInProviderRealm(ImmutableMap.>builder() - .put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME, ImmutableList.builder().add("some value").build()) - .put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME_2, ImmutableList.builder().add("some value").build()) + createUserInProviderRealm(ImmutableMap.> builder() + .put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME, + ImmutableList. builder().add("some value").build()) + .put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME_2, + ImmutableList. builder().add("some value").build()) .build()); logInAsUserInIDPForFirstTime(); - UserRepresentation user = findUser(bc.consumerRealmName(), bc.getUserLogin(), bc.getUserEmail()); - assertThatRoleHasBeenAssignedInConsumerRealmTo(user); + assertThatRoleHasBeenAssignedInConsumerRealm(); } @Test public void allValuesMatchRegex() { createAdvancedRoleMapper(CLAIMS_OR_ATTRIBUTES_REGEX, true); - createUserInProviderRealm(ImmutableMap.>builder() - .put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME, ImmutableList.builder().add("value 1").build()) - .put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME_2, ImmutableList.builder().add("value 2").build()) - .build()); + createUserInProviderRealm(createMatchingUserConfig()); logInAsUserInIDPForFirstTime(); - UserRepresentation user = findUser(bc.consumerRealmName(), bc.getUserLogin(), bc.getUserEmail()); - assertThatRoleHasBeenAssignedInConsumerRealmTo(user); + assertThatRoleHasBeenAssignedInConsumerRealm(); } @Test public void valuesMismatchRegex() { createAdvancedRoleMapper(CLAIMS_OR_ATTRIBUTES_REGEX, true); - createUserInProviderRealm(ImmutableMap.>builder() - .put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME, ImmutableList.builder().add("mismatch").build()) - .put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME_2, ImmutableList.builder().add("value 2").build()) + createUserInProviderRealm(ImmutableMap.> builder() + .put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME, + ImmutableList. builder().add("mismatch").build()) + .put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME_2, + ImmutableList. builder().add("value 2").build()) .build()); logInAsUserInIDPForFirstTime(); - UserRepresentation user = findUser(bc.consumerRealmName(), bc.getUserLogin(), bc.getUserEmail()); - assertThatRoleHasNotBeenAssignedInConsumerRealmTo(user); + assertThatRoleHasNotBeenAssignedInConsumerRealm(); } @Test public void updateBrokeredUserMismatchDeletesRole() { newValueForAttribute2 = "value mismatch"; - UserRepresentation user = createMapperAndLoginAsUserTwiceWithMapper(FORCE, false); + createMapperAndLoginAsUserTwiceWithMapper(FORCE, false); - assertThatRoleHasNotBeenAssignedInConsumerRealmTo(user); + assertThatRoleHasNotBeenAssignedInConsumerRealm(); } @Test public void updateBrokeredUserMismatchDoesNotDeleteRoleInImportMode() { newValueForAttribute2 = "value mismatch"; - UserRepresentation user = createMapperAndLoginAsUserTwiceWithMapper(IMPORT, false); + createMapperAndLoginAsUserTwiceWithMapper(IMPORT, false); - assertThatRoleHasBeenAssignedInConsumerRealmTo(user); + assertThatRoleHasBeenAssignedInConsumerRealm(); } @Test public void updateBrokeredUserMatchDoesntDeleteRole() { newValueForAttribute2 = "value 2"; - UserRepresentation user = createMapperAndLoginAsUserTwiceWithMapper(FORCE, false); + createMapperAndLoginAsUserTwiceWithMapper(FORCE, false); - assertThatRoleHasBeenAssignedInConsumerRealmTo(user); + assertThatRoleHasBeenAssignedInConsumerRealm(); } @Test public void updateBrokeredUserAssignsRoleInForceModeWhenCreatingTheMapperAfterFirstLogin() { newValueForAttribute2 = "value 2"; - UserRepresentation user = createMapperAndLoginAsUserTwiceWithMapper(FORCE, true); + createMapperAndLoginAsUserTwiceWithMapper(FORCE, true); - assertThatRoleHasBeenAssignedInConsumerRealmTo(user); + assertThatRoleHasBeenAssignedInConsumerRealm(); } - public UserRepresentation createMapperAndLoginAsUserTwiceWithMapper(IdentityProviderMapperSyncMode syncMode, boolean createAfterFirstLogin) { - return loginAsUserTwiceWithMapper(syncMode, createAfterFirstLogin, ImmutableMap.>builder() - .put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME, ImmutableList.builder().add("value 1").build()) - .put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME_2, ImmutableList.builder().add("value 2").build()) - .build()); + public void createMapperAndLoginAsUserTwiceWithMapper(IdentityProviderMapperSyncMode syncMode, + boolean createAfterFirstLogin) { + loginAsUserTwiceWithMapper(syncMode, createAfterFirstLogin, createMatchingUserConfig()); } @Override protected void updateUser() { UserRepresentation user = findUser(bc.providerRealmName(), bc.getUserLogin(), bc.getUserEmail()); - ImmutableMap> matchingAttributes = ImmutableMap.>builder() - .put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME, ImmutableList.builder().add("value 1").build()) - .put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME_2, ImmutableList.builder().add(newValueForAttribute2).build()) - .put("some.other.attribute", ImmutableList.builder().add("some value").build()) + ImmutableMap> matchingAttributes = ImmutableMap.> builder() + .put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME, + ImmutableList. builder().add("value 1").build()) + .put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME_2, + ImmutableList. builder().add(newValueForAttribute2).build()) + .put("some.other.attribute", ImmutableList. builder().add("some value").build()) .build(); user.setAttributes(matchingAttributes); adminClient.realm(bc.providerRealmName()).users().get(user.getId()).update(user); } @Override - protected void createMapperInIdp(IdentityProviderRepresentation idp, IdentityProviderMapperSyncMode syncMode) { - createMapperInIdp(idp, CLAIMS_OR_ATTRIBUTES, false, syncMode); + protected void createMapperInIdp(IdentityProviderMapperSyncMode syncMode, String roleValue) { + createMapperInIdp(CLAIMS_OR_ATTRIBUTES, false, syncMode, roleValue); } - protected void createAdvancedRoleMapper(String claimsOrAttributeRepresentation, boolean areClaimsOrAttributeValuesRegexes) { - IdentityProviderRepresentation idp = setupIdentityProvider(); - createMapperInIdp(idp, claimsOrAttributeRepresentation, areClaimsOrAttributeValuesRegexes, IMPORT); + @Override + protected Map> createUserConfigForRole(String roleValue) { + return createMatchingUserConfig(); } - abstract protected void createMapperInIdp( - IdentityProviderRepresentation idp, String claimsOrAttributeRepresentation, boolean areClaimsOrAttributeValuesRegexes, IdentityProviderMapperSyncMode syncMode); + private static Map> createMatchingUserConfig() { + return ImmutableMap.> builder() + .put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME, + ImmutableList. builder().add("value 1").build()) + .put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME_2, + ImmutableList. builder().add("value 2").build()) + .build(); + } + + protected void createAdvancedRoleMapper(String claimsOrAttributeRepresentation, + boolean areClaimsOrAttributeValuesRegexes) { + setupIdentityProvider(); + createMapperInIdp(claimsOrAttributeRepresentation, areClaimsOrAttributeValuesRegexes, IMPORT, + CLIENT_ROLE_MAPPER_REPRESENTATION); + } + + abstract protected void createMapperInIdp(String claimsOrAttributeRepresentation, + boolean areClaimsOrAttributeValuesRegexes, IdentityProviderMapperSyncMode syncMode, String roleValue); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractBaseBrokerTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractBaseBrokerTest.java index 4c4dadcba9..6693829fc8 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractBaseBrokerTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractBaseBrokerTest.java @@ -265,6 +265,11 @@ public abstract class AbstractBaseBrokerTest extends AbstractKeycloakTest { updateAccountInformation(); } + protected void logInAsUserInIDPForFirstTimeAndAssertSuccess() { + logInAsUserInIDPForFirstTime(); + assertLoggedInAccountManagement(); + } + protected void updateAccountInformation() { waitForPage(driver, "update account information", false); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractGroupMapperTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractGroupMapperTest.java index 1d22f9f738..7a9e8ebd37 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractGroupMapperTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractGroupMapperTest.java @@ -1,38 +1,134 @@ package org.keycloak.testsuite.broker; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.not; import static org.keycloak.testsuite.broker.BrokerTestTools.getConsumerRoot; +import org.junit.Before; +import org.junit.Test; +import org.keycloak.admin.client.CreatedResponseUtil; +import org.keycloak.broker.provider.ConfigConstants; import org.keycloak.models.IdentityProviderMapperSyncMode; +import org.keycloak.models.utils.KeycloakModelUtils; +import org.keycloak.representations.idm.GroupRepresentation; +import org.keycloak.representations.idm.IdentityProviderMapperRepresentation; import org.keycloak.representations.idm.IdentityProviderRepresentation; import org.keycloak.representations.idm.UserRepresentation; -import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.stream.Collectors; +import javax.ws.rs.core.Response; + +/** + * @author Artur Baltabayev, + * Daniel Fesenmeyer + */ public abstract class AbstractGroupMapperTest extends AbstractIdentityProviderMapperTest { public static final String MAPPER_TEST_GROUP_NAME = "mapper-test"; - public static final String MAPPER_TEST_GROUP_PATH = "/" + MAPPER_TEST_GROUP_NAME; + public static final String MAPPER_TEST_GROUP_PATH = buildGroupPath(MAPPER_TEST_GROUP_NAME); - protected abstract void createMapperInIdp( - IdentityProviderRepresentation idp, IdentityProviderMapperSyncMode syncMode); + public static final String MAPPER_TEST_NOT_EXISTING_GROUP_PATH = buildGroupPath("mapper-test-not-existing"); + + protected String mapperGroupId; + + protected abstract String createMapperInIdp( + IdentityProviderRepresentation idp, IdentityProviderMapperSyncMode syncMode, String groupPath); + + /** + * Sets up a scenario with a matching group. + * @return the ID of the mapper + */ + protected abstract String setupScenarioWithMatchingGroup(); + + protected abstract void setupScenarioWithNonExistingGroup(); protected void updateUser() { } + @Before + public void addMapperTestGroupToConsumerRealm() { + GroupRepresentation mapperTestGroup = new GroupRepresentation(); + mapperTestGroup.setName(MAPPER_TEST_GROUP_NAME); + + Response response = adminClient.realm(bc.consumerRealmName()).groups().add(mapperTestGroup); + mapperGroupId = CreatedResponseUtil.getCreatedId(response); + } + + @Test + public void tryToCreateBrokeredUserWithNonExistingGroupDoesNotBreakLogin() { + setupScenarioWithNonExistingGroup(); + + logInAsUserInIDPForFirstTimeAndAssertSuccess(); + + UserRepresentation user = findUser(bc.consumerRealmName(), bc.getUserLogin(), bc.getUserEmail()); + assertThatUserDoesNotHaveGroups(user); + } + + @Test + public void mapperStillWorksWhenGroupIsMoved() { + final String mapperId = setupScenarioWithMatchingGroup(); + + String newParentGroupName = "new-parent"; + GroupRepresentation newParentGroup = new GroupRepresentation(); + newParentGroup.setName(newParentGroupName); + String newParentGroupId = CreatedResponseUtil.getCreatedId(realm.groups().add(newParentGroup)); + + GroupRepresentation mappedGroup = realm.groups().group(mapperGroupId).toRepresentation(); + realm.groups().group(newParentGroupId).subGroup(mappedGroup).close(); + + String expectedNewGroupPath = buildGroupPath(newParentGroupName, MAPPER_TEST_GROUP_NAME); + + // mapper should have been updated to the new path of the group + IdentityProviderMapperRepresentation mapper = + realm.identityProviders().get(bc.getIDPAlias()).getMapperById(mapperId); + Map config = mapper.getConfig(); + assertThat(config.get(ConfigConstants.GROUP), equalTo(expectedNewGroupPath)); + + logInAsUserInIDPForFirstTimeAndAssertSuccess(); + + UserRepresentation user = findUser(bc.consumerRealmName(), bc.getUserLogin(), bc.getUserEmail()); + assertThatUserHasBeenAssignedToGroup(user, expectedNewGroupPath); + } + + @Test + public void mapperStillWorksWhenGroupIsRenamed() { + final String mapperId = setupScenarioWithMatchingGroup(); + + String newGroupName = "new-name-" + MAPPER_TEST_GROUP_NAME; + GroupRepresentation mappedGroup = realm.groups().group(mapperGroupId).toRepresentation(); + mappedGroup.setName(newGroupName); + realm.groups().group(mapperGroupId).update(mappedGroup); + + String expectedNewGroupPath = buildGroupPath(newGroupName); + + // mapper should have been updated to the new path of the group + IdentityProviderMapperRepresentation mapper = + realm.identityProviders().get(bc.getIDPAlias()).getMapperById(mapperId); + Map config = mapper.getConfig(); + assertThat(config.get(ConfigConstants.GROUP), equalTo(expectedNewGroupPath)); + + logInAsUserInIDPForFirstTimeAndAssertSuccess(); + + UserRepresentation user = findUser(bc.consumerRealmName(), bc.getUserLogin(), bc.getUserEmail()); + assertThatUserHasBeenAssignedToGroup(user, expectedNewGroupPath); + } + protected UserRepresentation loginAsUserTwiceWithMapper( IdentityProviderMapperSyncMode syncMode, boolean createAfterFirstLogin, - Map> userConfig) { + Map> userConfig, String groupPath) { final IdentityProviderRepresentation idp = setupIdentityProvider(); if (!createAfterFirstLogin) { - createMapperInIdp(idp, syncMode); + createMapperInIdp(idp, syncMode, groupPath); } createUserInProviderRealm(userConfig); - logInAsUserInIDPForFirstTime(); + logInAsUserInIDPForFirstTimeAndAssertSuccess(); UserRepresentation user = findUser(bc.consumerRealmName(), bc.getUserLogin(), bc.getUserEmail()); if (!createAfterFirstLogin) { @@ -42,34 +138,46 @@ public abstract class AbstractGroupMapperTest extends AbstractIdentityProviderMa } if (createAfterFirstLogin) { - createMapperInIdp(idp, syncMode); + createMapperInIdp(idp, syncMode, groupPath); } logoutFromRealm(getConsumerRoot(), bc.consumerRealmName()); updateUser(); logInAsUserInIDP(); + assertLoggedInAccountManagement(); + user = findUser(bc.consumerRealmName(), bc.getUserLogin(), bc.getUserEmail()); return user; } protected void assertThatUserHasBeenAssignedToGroup(UserRepresentation user) { - List groupNames = new ArrayList<>(); + assertThatUserHasBeenAssignedToGroup(user, MAPPER_TEST_GROUP_PATH); + } - realm.users().get(user.getId()).groups().forEach(group -> { - groupNames.add(group.getName()); - }); - - assertTrue(groupNames.contains(MAPPER_TEST_GROUP_NAME)); + protected void assertThatUserHasBeenAssignedToGroup(UserRepresentation user, String groupPath) { + assertThat(getUserGroupPaths(user), contains(groupPath)); } protected void assertThatUserHasNotBeenAssignedToGroup(UserRepresentation user) { - List groupNames = new ArrayList<>(); + assertThat(getUserGroupPaths(user), not(contains(MAPPER_TEST_GROUP_PATH))); + } - realm.users().get(user.getId()).groups().forEach(group -> { - groupNames.add(group.getName()); - }); + protected void assertThatUserDoesNotHaveGroups(UserRepresentation user) { + assertThat(getUserGroupPaths(user), empty()); + } - assertFalse(groupNames.contains(MAPPER_TEST_GROUP_NAME)); + protected static String buildGroupPath(String firstSegment, String... furtherSegments) { + String separator = KeycloakModelUtils.GROUP_PATH_SEPARATOR; + StringBuilder sb = new StringBuilder(separator).append(firstSegment); + for (String furtherSegment : furtherSegments) { + sb.append(separator).append(furtherSegment); + } + return sb.toString(); + } + + private List getUserGroupPaths(UserRepresentation user) { + return realm.users().get(user.getId()).groups().stream().map(GroupRepresentation::getPath) + .collect(Collectors.toList()); } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractRoleMapperTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractRoleMapperTest.java index a5c1437806..3eb6292ce8 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractRoleMapperTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractRoleMapperTest.java @@ -1,82 +1,269 @@ package org.keycloak.testsuite.broker; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.aMapWithSize; import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasEntry; +import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.in; +import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; -import static org.junit.Assert.assertThat; import static org.keycloak.testsuite.broker.BrokerTestTools.getConsumerRoot; -import static org.keycloak.testsuite.util.WaitUtils.pause; + +import org.hamcrest.Matchers; +import org.junit.Before; +import org.junit.Test; +import org.keycloak.admin.client.CreatedResponseUtil; +import org.keycloak.admin.client.resource.IdentityProviderResource; +import org.keycloak.admin.client.resource.RealmResource; +import org.keycloak.admin.client.resource.UserResource; +import org.keycloak.broker.provider.ConfigConstants; +import org.keycloak.models.Constants; +import org.keycloak.models.IdentityProviderMapperSyncMode; +import org.keycloak.representations.idm.ClientRepresentation; +import org.keycloak.representations.idm.IdentityProviderMapperRepresentation; +import org.keycloak.representations.idm.RoleRepresentation; +import org.keycloak.representations.idm.UserRepresentation; +import org.keycloak.testsuite.util.ClientBuilder; +import org.keycloak.testsuite.util.RoleBuilder; import java.util.Collections; import java.util.List; import java.util.Map; - -import org.keycloak.admin.client.resource.UserResource; -import org.keycloak.models.IdentityProviderMapperSyncMode; -import org.keycloak.representations.idm.IdentityProviderRepresentation; -import org.keycloak.representations.idm.RoleRepresentation; -import org.keycloak.representations.idm.UserRepresentation; -import org.openqa.selenium.firefox.FirefoxDriver; +import java.util.concurrent.ConcurrentHashMap; /** * @author hmlnarik, * Benjamin Weimer, * Martin Idel, + * Daniel Fesenmeyer */ public abstract class AbstractRoleMapperTest extends AbstractIdentityProviderMapperTest { - private static final String CLIENT = "realm-management"; - private static final String CLIENT_ROLE = "view-realm"; - public static final String ROLE_USER = "user"; - public static final String CLIENT_ROLE_MAPPER_REPRESENTATION = CLIENT + "." + CLIENT_ROLE; + protected static final String CLIENT_ID = "mapper-test-client"; + protected static final String CLIENT_ROLE = "test-role"; + protected static final String CLIENT_ROLE_MAPPER_REPRESENTATION = createClientRoleString(CLIENT_ID, CLIENT_ROLE); + protected static final String ROLE_USER = "user"; - protected abstract void createMapperInIdp(IdentityProviderRepresentation idp, IdentityProviderMapperSyncMode syncMode); + private static final String REALM_ROLE = "test-realm-role"; + + protected String clientUuid; + + private Map mapperIdsWithName; + + protected static String createClientRoleString(final String clientId, final String roleName) { + return clientId + "." + roleName; + } + + protected abstract void createMapperInIdp(IdentityProviderMapperSyncMode syncMode, String roleValue); + + protected abstract Map> createUserConfigForRole(String roleValue); protected void updateUser() { } - protected UserRepresentation loginAsUserTwiceWithMapper( - IdentityProviderMapperSyncMode syncMode, boolean createAfterFirstLogin, Map> userConfig) { - final IdentityProviderRepresentation idp = setupIdentityProvider(); + @Before + public void init() { + mapperIdsWithName = new ConcurrentHashMap<>(); + + RealmResource consumerRealmResource = adminClient.realm(bc.consumerRealmName()); + + clientUuid = CreatedResponseUtil.getCreatedId( + consumerRealmResource.clients().create(ClientBuilder.create().clientId(CLIENT_ID).build())); + consumerRealmResource.clients().get(clientUuid).roles().create(RoleBuilder.create().name(CLIENT_ROLE).build()); + + consumerRealmResource.roles().create(RoleBuilder.create().name(REALM_ROLE).build()); + } + + @Test + public void tryToCreateBrokeredUserWithNonExistingClientRoleDoesNotBreakLogin() { + String clientRoleStringWithMissingRole = createClientRoleString(CLIENT_ID, "does-not-exist"); + setup(clientRoleStringWithMissingRole); + + assertLoginSucceedsWithoutRoleAssignment(); + } + + /** + * This test checks that the mapper can also be applied to realm roles (other tests mostly use client roles). + */ + @Test + public void mapperCanBeAppliedToRealmRoles() { + setup(REALM_ROLE); + + logInAsUserInIDPForFirstTimeAndAssertSuccess(); + + assertThatRoleHasBeenAssignedInConsumerRealm(REALM_ROLE); + } + + @Test + public void mapperStillWorksWhenClientRoleIsRenamed() { + setup(CLIENT_ROLE_MAPPER_REPRESENTATION); + + String newRoleName = "new-name-" + CLIENT_ROLE; + RoleRepresentation mappedRole = realm.clients().get(clientUuid).roles().get(CLIENT_ROLE).toRepresentation(); + mappedRole.setName(newRoleName); + realm.clients().get(clientUuid).roles().get(CLIENT_ROLE).update(mappedRole); + + String expectedNewClientRoleName = createClientRoleString(CLIENT_ID, newRoleName); + + // mapper(s) should have been updated to the new client role name + assertMappersAreConfiguredWithRole(expectedNewClientRoleName); + + logInAsUserInIDPForFirstTimeAndAssertSuccess(); + + assertThatRoleHasBeenAssignedInConsumerRealm(CLIENT_ID, newRoleName); + } + + @Test + public void mapperStillWorksWhenClientIdIsChanged() { + setup(CLIENT_ROLE_MAPPER_REPRESENTATION); + + String newClientId = "new-name-" + CLIENT_ID; + ClientRepresentation mappedClient = realm.clients().get(clientUuid).toRepresentation(); + mappedClient.setClientId(newClientId); + realm.clients().get(clientUuid).update(mappedClient); + + String expectedNewClientRoleName = createClientRoleString(newClientId, CLIENT_ROLE); + + // mapper(s) should have been updated to the new client role name + assertMappersAreConfiguredWithRole(expectedNewClientRoleName); + + logInAsUserInIDPForFirstTimeAndAssertSuccess(); + + assertThatRoleHasBeenAssignedInConsumerRealm(newClientId, CLIENT_ROLE); + } + + @Test + public void mapperStillWorksWhenRealmRoleIsRenamed() { + setup(REALM_ROLE); + + String newRoleName = "new-name-" + REALM_ROLE; + RoleRepresentation mappedRole = realm.roles().get(REALM_ROLE).toRepresentation(); + mappedRole.setName(newRoleName); + realm.roles().get(REALM_ROLE).update(mappedRole); + + // mapper(s) should have been updated to the new realm role name + assertMappersAreConfiguredWithRole(newRoleName); + + logInAsUserInIDPForFirstTimeAndAssertSuccess(); + + assertThatRoleHasBeenAssignedInConsumerRealm(newRoleName); + } + + private void assertMappersAreConfiguredWithRole(String expectedRoleQualifier) { + mapperIdsWithName.forEach((mapperId, mapperName) -> { + try { + IdentityProviderMapperRepresentation mapper = + realm.identityProviders().get(bc.getIDPAlias()).getMapperById(mapperId); + Map config = mapper.getConfig(); + assertThat(config.get(ConfigConstants.ROLE), equalTo(expectedRoleQualifier)); + } catch (final AssertionError | RuntimeException t) { + throw new AssertionError( + String.format( + "Failed assertion for mapper with ID '%s' and name '%s'. See the cause for details.", + mapperId, mapperName), + t); + } + }); + } + + protected final void persistMapper(IdentityProviderMapperRepresentation idpMapper) { + String idpAlias = bc.getIDPAlias(); + IdentityProviderResource idpResource = realm.identityProviders().get(idpAlias); + idpMapper.setIdentityProviderAlias(idpAlias); + + String mapperId = CreatedResponseUtil.getCreatedId(idpResource.addMapper(idpMapper)); + mapperIdsWithName.put(mapperId, idpMapper.getName()); + } + + protected void loginAsUserTwiceWithMapper(IdentityProviderMapperSyncMode syncMode, boolean createAfterFirstLogin, + Map> userConfig) { + setupIdentityProvider(); if (!createAfterFirstLogin) { - createMapperInIdp(idp, syncMode); + createMapperInIdp(syncMode, CLIENT_ROLE_MAPPER_REPRESENTATION); } - createUserInProviderRealm(userConfig); - createUserRoleAndGrantToUserInProviderRealm(); + setupUser(userConfig); logInAsUserInIDPForFirstTime(); - UserRepresentation user = findUser(bc.consumerRealmName(), bc.getUserLogin(), bc.getUserEmail()); if (!createAfterFirstLogin) { - assertThatRoleHasBeenAssignedInConsumerRealmTo(user); + assertThatRoleHasBeenAssignedInConsumerRealm(); } else { - assertThatRoleHasNotBeenAssignedInConsumerRealmTo(user); + assertThatRoleHasNotBeenAssignedInConsumerRealm(); } if (createAfterFirstLogin) { - createMapperInIdp(idp, syncMode); + createMapperInIdp(syncMode, CLIENT_ROLE_MAPPER_REPRESENTATION); } logoutFromRealm(getConsumerRoot(), bc.consumerRealmName()); updateUser(); logInAsUserInIDP(); - user = findUser(bc.consumerRealmName(), bc.getUserLogin(), bc.getUserEmail()); - return user; + } + + private void setup(String roleValue) { + setupIdentityProvider(); + createMapperInIdp(IdentityProviderMapperSyncMode.IMPORT, roleValue); + setupUser(createUserConfigForRole(roleValue)); + } + + private void setupUser(Map> userConfig) { + createUserInProviderRealm(userConfig); + createUserRoleAndGrantToUserInProviderRealm(); } protected void createUserRoleAndGrantToUserInProviderRealm() { - RoleRepresentation userRole = new RoleRepresentation(ROLE_USER,null, false); + RoleRepresentation userRole = new RoleRepresentation(ROLE_USER, null, false); adminClient.realm(bc.providerRealmName()).roles().create(userRole); RoleRepresentation role = adminClient.realm(bc.providerRealmName()).roles().get(ROLE_USER).toRepresentation(); UserResource userResource = adminClient.realm(bc.providerRealmName()).users().get(userId); userResource.roles().realmLevel().add(Collections.singletonList(role)); } - protected void assertThatRoleHasBeenAssignedInConsumerRealmTo(UserRepresentation user) { - assertThat(user.getClientRoles().get(CLIENT), contains(CLIENT_ROLE)); + private void assertLoginSucceedsWithoutRoleAssignment() { + logInAsUserInIDPForFirstTimeAndAssertSuccess(); + + assertThatNoRolesHaveBeenAssignedInConsumerRealm(); } - protected void assertThatRoleHasNotBeenAssignedInConsumerRealmTo(UserRepresentation user) { - assertThat(user.getClientRoles().get(CLIENT), not(contains(CLIENT_ROLE))); + protected void assertThatRoleHasBeenAssignedInConsumerRealm() { + assertThatRoleHasBeenAssignedInConsumerRealm(CLIENT_ID, CLIENT_ROLE); } + + protected void assertThatRoleHasNotBeenAssignedInConsumerRealm() { + UserRepresentation user = getConsumerUser(); + assertThat(user.getClientRoles().get(CLIENT_ID), not(contains(CLIENT_ROLE))); + } + + protected void assertThatRoleHasBeenAssignedInConsumerRealm(String clientId, String roleName) { + UserRepresentation user = getConsumerUser(); + assertThat(user.getClientRoles().get(clientId), contains(roleName)); + } + + protected void assertThatRoleHasBeenAssignedInConsumerRealm(String roleName) { + UserRepresentation user = getConsumerUser(); + assertThat(roleName, is(in(user.getRealmRoles()))); + } + + /** + * Check that just initial (default) roles are assigned to the user. + */ + protected void assertThatNoRolesHaveBeenAssignedInConsumerRealm() { + UserRepresentation user = getConsumerUser(); + + Map> clientRoles = user.getClientRoles(); + assertThat(clientRoles, Matchers.anyOf(aMapWithSize(0), + hasEntry(Constants.BROKER_SERVICE_CLIENT_ID, Collections.singletonList(Constants.READ_TOKEN_ROLE)))); + + List realmRoles = user.getRealmRoles(); + assertThat(realmRoles, hasSize(1)); + assertThat(realmRoles, contains(Constants.DEFAULT_ROLES_ROLE_PREFIX + "-" + bc.consumerRealmName())); + } + + private UserRepresentation getConsumerUser() { + return findUser(bc.consumerRealmName(), bc.getUserLogin(), bc.getUserEmail()); + } + } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AttributeToRoleMapperTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AttributeToRoleMapperTest.java index f757c30633..f28a1c6c49 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AttributeToRoleMapperTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AttributeToRoleMapperTest.java @@ -3,27 +3,27 @@ package org.keycloak.testsuite.broker; import static org.keycloak.models.IdentityProviderMapperSyncMode.FORCE; import static org.keycloak.models.IdentityProviderMapperSyncMode.LEGACY; -import java.util.List; - import org.junit.Test; -import org.keycloak.admin.client.resource.IdentityProviderResource; +import org.keycloak.broker.provider.ConfigConstants; import org.keycloak.broker.saml.mappers.AttributeToRoleMapper; import org.keycloak.broker.saml.mappers.UserAttributeMapper; import org.keycloak.models.IdentityProviderMapperModel; import org.keycloak.models.IdentityProviderMapperSyncMode; -import org.keycloak.models.IdentityProviderSyncMode; import org.keycloak.representations.idm.IdentityProviderMapperRepresentation; -import org.keycloak.representations.idm.IdentityProviderRepresentation; -import org.keycloak.representations.idm.UserRepresentation; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import java.util.List; +import java.util.Map; + /** * @author Martin Idel */ public class AttributeToRoleMapperTest extends AbstractRoleMapperTest { + private static final String ROLE_ATTR_NAME = "Role"; + @Override protected BrokerConfiguration getBrokerConfiguration() { return new KcSamlBrokerConfiguration(); @@ -31,53 +31,53 @@ public class AttributeToRoleMapperTest extends AbstractRoleMapperTest { @Test public void mapperGrantsRoleOnFirstLogin() { - UserRepresentation user = createMapperThenLoginAsUserTwiceWithAttributeToRoleMapper(FORCE); + createMapperThenLoginAsUserTwiceWithAttributeToRoleMapper(); - assertThatRoleHasBeenAssignedInConsumerRealmTo(user); + assertThatRoleHasBeenAssignedInConsumerRealm(); } @Test public void updateBrokeredUserGrantsRoleInLegacyMode() { - UserRepresentation user = loginAsUserThenCreateMapperAndLoginAgainWithAttributeToRoleMapper(LEGACY); + loginAsUserThenCreateMapperAndLoginAgainWithAttributeToRoleMapper(LEGACY); - assertThatRoleHasBeenAssignedInConsumerRealmTo(user); + assertThatRoleHasBeenAssignedInConsumerRealm(); } @Test public void updateBrokeredUserGrantsRoleInForceMode() { - UserRepresentation user = loginAsUserThenCreateMapperAndLoginAgainWithAttributeToRoleMapper(FORCE); + loginAsUserThenCreateMapperAndLoginAgainWithAttributeToRoleMapper(FORCE); - assertThatRoleHasBeenAssignedInConsumerRealmTo(user); + assertThatRoleHasBeenAssignedInConsumerRealm(); } - private UserRepresentation createMapperThenLoginAsUserTwiceWithAttributeToRoleMapper(IdentityProviderMapperSyncMode syncMode) { - return loginAsUserTwiceWithMapper(syncMode, false, - ImmutableMap.>builder() - .put("Role", ImmutableList.builder().add(CLIENT_ROLE_MAPPER_REPRESENTATION).build()) - .build()); + private void createMapperThenLoginAsUserTwiceWithAttributeToRoleMapper() { + loginAsUserTwiceWithMapper(IdentityProviderMapperSyncMode.FORCE, false, + createUserConfigForRole(CLIENT_ROLE_MAPPER_REPRESENTATION)); } - private UserRepresentation loginAsUserThenCreateMapperAndLoginAgainWithAttributeToRoleMapper(IdentityProviderMapperSyncMode syncMode) { - return loginAsUserTwiceWithMapper(syncMode, true, - ImmutableMap.>builder() - .put("Role", ImmutableList.builder().add(CLIENT_ROLE_MAPPER_REPRESENTATION).build()) - .build()); + private void loginAsUserThenCreateMapperAndLoginAgainWithAttributeToRoleMapper( + IdentityProviderMapperSyncMode syncMode) { + loginAsUserTwiceWithMapper(syncMode, true, createUserConfigForRole(CLIENT_ROLE_MAPPER_REPRESENTATION)); } @Override - protected void createMapperInIdp(IdentityProviderRepresentation idp, IdentityProviderMapperSyncMode syncMode) { + protected void createMapperInIdp(IdentityProviderMapperSyncMode syncMode, String roleValue) { IdentityProviderMapperRepresentation samlAttributeToRoleMapper = new IdentityProviderMapperRepresentation(); samlAttributeToRoleMapper.setName("user-role-mapper"); samlAttributeToRoleMapper.setIdentityProviderMapper(AttributeToRoleMapper.PROVIDER_ID); - samlAttributeToRoleMapper.setConfig(ImmutableMap.builder() - .put(IdentityProviderMapperModel.SYNC_MODE, syncMode.toString()) - .put(UserAttributeMapper.ATTRIBUTE_NAME, "Role") - .put(ATTRIBUTE_VALUE, ROLE_USER) - .put("role", CLIENT_ROLE_MAPPER_REPRESENTATION) - .build()); + samlAttributeToRoleMapper.setConfig(ImmutableMap. builder() + .put(IdentityProviderMapperModel.SYNC_MODE, syncMode.toString()) + .put(UserAttributeMapper.ATTRIBUTE_NAME, ROLE_ATTR_NAME) + .put(ATTRIBUTE_VALUE, ROLE_USER) + .put(ConfigConstants.ROLE, roleValue) + .build()); - IdentityProviderResource idpResource = realm.identityProviders().get(idp.getAlias()); - samlAttributeToRoleMapper.setIdentityProviderAlias(bc.getIDPAlias()); - idpResource.addMapper(samlAttributeToRoleMapper).close(); + persistMapper(samlAttributeToRoleMapper); + } + + protected Map> createUserConfigForRole(String roleValue) { + return ImmutableMap.> builder() + .put(ROLE_ATTR_NAME, ImmutableList. builder().add(roleValue).build()) + .build(); } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/BrokerTestTools.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/BrokerTestTools.java index fbd0a1d2cb..0c622ef787 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/BrokerTestTools.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/BrokerTestTools.java @@ -59,11 +59,31 @@ public class BrokerTestTools { return identityProviderRepresentation; } - public static void waitForPage(WebDriver driver, final String title, final boolean isHtmlTitle) { + public static void waitForPage(final WebDriver driver, final String title, final boolean isHtmlTitle) { waitForPageToLoad(); WebDriverWait wait = new WebDriverWait(driver, 5); - ExpectedCondition condition = (WebDriver input) -> isHtmlTitle ? input.getTitle().toLowerCase().contains(title) : PageUtils.getPageTitle(input).toLowerCase().contains(title); + ExpectedCondition condition = new ExpectedCondition() { + private String actualTitle = null; + + public Boolean apply(final WebDriver input) { + if (input == null) { + return false; + } + + actualTitle = isHtmlTitle ? input.getTitle() : PageUtils.getPageTitle(input); + if (actualTitle == null) { + return false; + } + + return actualTitle.toLowerCase().contains(title.toLowerCase()); + } + + public String toString() { + return String.format("value to contain (ignoring case) \"%s\". Current value: \"%s\"", title, + this.actualTitle); + } + }; wait.until(condition); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/ExternalKeycloakRoleToRoleMapperTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/ExternalKeycloakRoleToRoleMapperTest.java index 3140e4de71..ec8d6c1fc0 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/ExternalKeycloakRoleToRoleMapperTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/ExternalKeycloakRoleToRoleMapperTest.java @@ -1,30 +1,30 @@ package org.keycloak.testsuite.broker; -import java.util.Collections; -import java.util.List; +import static org.keycloak.models.IdentityProviderMapperSyncMode.FORCE; +import static org.keycloak.models.IdentityProviderMapperSyncMode.IMPORT; +import static org.keycloak.models.IdentityProviderMapperSyncMode.LEGACY; import org.junit.Before; import org.junit.Test; -import org.keycloak.admin.client.resource.IdentityProviderResource; -import org.keycloak.admin.client.resource.RealmResource; import org.keycloak.admin.client.resource.UserResource; import org.keycloak.broker.oidc.mappers.ExternalKeycloakRoleToRoleMapper; +import org.keycloak.broker.provider.ConfigConstants; import org.keycloak.models.IdentityProviderMapperModel; import org.keycloak.models.IdentityProviderMapperSyncMode; import org.keycloak.representations.idm.IdentityProviderMapperRepresentation; -import org.keycloak.representations.idm.IdentityProviderRepresentation; import org.keycloak.representations.idm.RoleRepresentation; -import org.keycloak.representations.idm.UserRepresentation; import com.google.common.collect.ImmutableMap; -import static org.keycloak.models.IdentityProviderMapperSyncMode.*; +import java.util.Collections; +import java.util.List; +import java.util.Map; /** - * @author Martin Idel + * @author Martin Idel, + * Daniel Fesenmeyer */ public class ExternalKeycloakRoleToRoleMapperTest extends AbstractRoleMapperTest { - private RealmResource realm; private boolean deleteRoleFromUser = true; @Override @@ -35,67 +35,66 @@ public class ExternalKeycloakRoleToRoleMapperTest extends AbstractRoleMapperTest @Before public void setupRealm() { super.addClients(); - realm = adminClient.realm(bc.consumerRealmName()); } @Test public void mapperGrantsRoleOnFirstLogin() { - UserRepresentation user = createMapperThenLoginAsUserTwiceWithExternalKeycloakRoleToRoleMapper(IMPORT); + createMapperThenLoginAsUserTwiceWithExternalKeycloakRoleToRoleMapper(IMPORT); - assertThatRoleHasBeenAssignedInConsumerRealmTo(user); + assertThatRoleHasBeenAssignedInConsumerRealm(); } @Test public void updateBrokeredUserDoesNotGrantRoleInLegacyMode() { - UserRepresentation user = loginAsUserThenCreateMapperAndLoginAgainWithExternalKeycloakRoleToRoleMapper(LEGACY); + loginAsUserThenCreateMapperAndLoginAgainWithExternalKeycloakRoleToRoleMapper(LEGACY); - assertThatRoleHasNotBeenAssignedInConsumerRealmTo(user); + assertThatRoleHasNotBeenAssignedInConsumerRealm(); } @Test public void updateBrokeredUserGrantsRoleInForceMode() { - UserRepresentation user = loginAsUserThenCreateMapperAndLoginAgainWithExternalKeycloakRoleToRoleMapper(FORCE); + loginAsUserThenCreateMapperAndLoginAgainWithExternalKeycloakRoleToRoleMapper(FORCE); - assertThatRoleHasBeenAssignedInConsumerRealmTo(user); + assertThatRoleHasBeenAssignedInConsumerRealm(); } @Test public void updateBrokeredUserMatchDeletesRoleInForceMode() { - UserRepresentation user = createMapperThenLoginAsUserTwiceWithExternalKeycloakRoleToRoleMapper(FORCE); + createMapperThenLoginAsUserTwiceWithExternalKeycloakRoleToRoleMapper(FORCE); - assertThatRoleHasNotBeenAssignedInConsumerRealmTo(user); + assertThatRoleHasNotBeenAssignedInConsumerRealm(); } @Test public void updateBrokeredUserMatchDoesNotDeleteRoleInLegacyMode() { - UserRepresentation user = createMapperThenLoginAsUserTwiceWithExternalKeycloakRoleToRoleMapper(LEGACY); + createMapperThenLoginAsUserTwiceWithExternalKeycloakRoleToRoleMapper(LEGACY); - assertThatRoleHasBeenAssignedInConsumerRealmTo(user); + assertThatRoleHasBeenAssignedInConsumerRealm(); } - private UserRepresentation createMapperThenLoginAsUserTwiceWithExternalKeycloakRoleToRoleMapper(IdentityProviderMapperSyncMode syncMode) { - return loginAsUserTwiceWithMapper(syncMode, false, ImmutableMap.>builder().build()); + private void createMapperThenLoginAsUserTwiceWithExternalKeycloakRoleToRoleMapper( + IdentityProviderMapperSyncMode syncMode) { + loginAsUserTwiceWithMapper(syncMode, false, Collections.emptyMap()); } - private UserRepresentation loginAsUserThenCreateMapperAndLoginAgainWithExternalKeycloakRoleToRoleMapper(IdentityProviderMapperSyncMode syncMode) { + private void loginAsUserThenCreateMapperAndLoginAgainWithExternalKeycloakRoleToRoleMapper( + IdentityProviderMapperSyncMode syncMode) { deleteRoleFromUser = false; - return loginAsUserTwiceWithMapper(syncMode, true, ImmutableMap.>builder().build()); + loginAsUserTwiceWithMapper(syncMode, true, Collections.emptyMap()); } @Override - protected void createMapperInIdp(IdentityProviderRepresentation idp, IdentityProviderMapperSyncMode syncMode) { + protected void createMapperInIdp(IdentityProviderMapperSyncMode syncMode, String roleValue) { IdentityProviderMapperRepresentation externalRoleToRoleMapper = new IdentityProviderMapperRepresentation(); externalRoleToRoleMapper.setName("external-keycloak-role-mapper"); externalRoleToRoleMapper.setIdentityProviderMapper(ExternalKeycloakRoleToRoleMapper.PROVIDER_ID); - externalRoleToRoleMapper.setConfig(ImmutableMap.builder() - .put(IdentityProviderMapperModel.SYNC_MODE, syncMode.toString()) - .put("external.role", ROLE_USER) - .put("role", CLIENT_ROLE_MAPPER_REPRESENTATION) - .build()); + externalRoleToRoleMapper.setConfig(ImmutableMap. builder() + .put(IdentityProviderMapperModel.SYNC_MODE, syncMode.toString()) + .put("external.role", ROLE_USER) + .put(ConfigConstants.ROLE, roleValue) + .build()); - IdentityProviderResource idpResource = realm.identityProviders().get(idp.getAlias()); - externalRoleToRoleMapper.setIdentityProviderAlias(bc.getIDPAlias()); - idpResource.addMapper(externalRoleToRoleMapper).close(); + persistMapper(externalRoleToRoleMapper); } @Override @@ -106,4 +105,9 @@ public class ExternalKeycloakRoleToRoleMapperTest extends AbstractRoleMapperTest userResource.roles().realmLevel().remove(Collections.singletonList(role)); } } + + @Override + protected Map> createUserConfigForRole(String roleValue) { + return Collections.emptyMap(); + } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/HardcodedRoleMapperTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/HardcodedRoleMapperTest.java index 70f46142f1..ddf893777e 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/HardcodedRoleMapperTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/HardcodedRoleMapperTest.java @@ -4,27 +4,25 @@ import static org.keycloak.models.IdentityProviderMapperSyncMode.FORCE; import static org.keycloak.models.IdentityProviderMapperSyncMode.IMPORT; import static org.keycloak.models.IdentityProviderMapperSyncMode.LEGACY; -import java.util.HashMap; - import org.junit.Before; import org.junit.Test; -import org.keycloak.admin.client.resource.IdentityProviderResource; -import org.keycloak.admin.client.resource.RealmResource; import org.keycloak.broker.provider.ConfigConstants; import org.keycloak.broker.provider.HardcodedRoleMapper; import org.keycloak.models.IdentityProviderMapperModel; import org.keycloak.models.IdentityProviderMapperSyncMode; import org.keycloak.representations.idm.IdentityProviderMapperRepresentation; -import org.keycloak.representations.idm.IdentityProviderRepresentation; -import org.keycloak.representations.idm.UserRepresentation; import com.google.common.collect.ImmutableMap; +import java.util.Collections; +import java.util.List; +import java.util.Map; + /** - * @author Martin Idel + * @author Martin Idel, + * Daniel Fesenmeyer */ public class HardcodedRoleMapperTest extends AbstractRoleMapperTest { - private RealmResource realm; @Override protected BrokerConfiguration getBrokerConfiguration() { @@ -34,64 +32,66 @@ public class HardcodedRoleMapperTest extends AbstractRoleMapperTest { @Before public void setupRealm() { super.addClients(); - realm = adminClient.realm(bc.consumerRealmName()); } @Test public void mapperGrantsRoleOnFirstLogin() { - UserRepresentation user = createMapperThenLoginAsUserTwiceWithHardcodedRoleMapper(IMPORT); + createMapperThenLoginAsUserTwiceWithHardcodedRoleMapper(IMPORT); - assertThatRoleHasBeenAssignedInConsumerRealmTo(user); + assertThatRoleHasBeenAssignedInConsumerRealm(); } @Test public void mapperDoesNotGrantRoleInModeImportIfMapperIsAddedLater() { - UserRepresentation user = loginAsUserThenCreateMapperAndLoginAgainWithHardcodedRoleMapper(IMPORT); + loginAsUserThenCreateMapperAndLoginAgainWithHardcodedRoleMapper(IMPORT); - assertThatRoleHasNotBeenAssignedInConsumerRealmTo(user); + assertThatRoleHasNotBeenAssignedInConsumerRealm(); } @Test public void updateBrokeredUserDoesNotGrantRoleInLegacyMode() { - UserRepresentation user = loginAsUserThenCreateMapperAndLoginAgainWithHardcodedRoleMapper(LEGACY); + loginAsUserThenCreateMapperAndLoginAgainWithHardcodedRoleMapper(LEGACY); - assertThatRoleHasNotBeenAssignedInConsumerRealmTo(user); + assertThatRoleHasNotBeenAssignedInConsumerRealm(); } @Test public void updateBrokeredUserGrantsRoleInForceMode() { - UserRepresentation user = loginAsUserThenCreateMapperAndLoginAgainWithHardcodedRoleMapper(FORCE); + loginAsUserThenCreateMapperAndLoginAgainWithHardcodedRoleMapper(FORCE); - assertThatRoleHasBeenAssignedInConsumerRealmTo(user); + assertThatRoleHasBeenAssignedInConsumerRealm(); } @Test public void updateBrokeredUserMatchDoesntDeleteRole() { - UserRepresentation user = createMapperThenLoginAsUserTwiceWithHardcodedRoleMapper(FORCE); + createMapperThenLoginAsUserTwiceWithHardcodedRoleMapper(FORCE); - assertThatRoleHasBeenAssignedInConsumerRealmTo(user); + assertThatRoleHasBeenAssignedInConsumerRealm(); } - private UserRepresentation createMapperThenLoginAsUserTwiceWithHardcodedRoleMapper(IdentityProviderMapperSyncMode syncMode) { - return loginAsUserTwiceWithMapper(syncMode, false, new HashMap<>()); + private void createMapperThenLoginAsUserTwiceWithHardcodedRoleMapper(IdentityProviderMapperSyncMode syncMode) { + loginAsUserTwiceWithMapper(syncMode, false, Collections.emptyMap()); } - private UserRepresentation loginAsUserThenCreateMapperAndLoginAgainWithHardcodedRoleMapper(IdentityProviderMapperSyncMode syncMode) { - return loginAsUserTwiceWithMapper(syncMode, true, new HashMap<>()); + private void loginAsUserThenCreateMapperAndLoginAgainWithHardcodedRoleMapper(IdentityProviderMapperSyncMode syncMode) { + loginAsUserTwiceWithMapper(syncMode, true, Collections.emptyMap()); } @Override - protected void createMapperInIdp(IdentityProviderRepresentation idp, IdentityProviderMapperSyncMode syncMode) { + protected void createMapperInIdp(IdentityProviderMapperSyncMode syncMode, String roleValue) { IdentityProviderMapperRepresentation advancedClaimToRoleMapper = new IdentityProviderMapperRepresentation(); advancedClaimToRoleMapper.setName("oidc-hardcoded-role-mapper"); advancedClaimToRoleMapper.setIdentityProviderMapper(HardcodedRoleMapper.PROVIDER_ID); - advancedClaimToRoleMapper.setConfig(ImmutableMap.builder() + advancedClaimToRoleMapper.setConfig(ImmutableMap. builder() .put(IdentityProviderMapperModel.SYNC_MODE, syncMode.toString()) - .put(ConfigConstants.ROLE, CLIENT_ROLE_MAPPER_REPRESENTATION) + .put(ConfigConstants.ROLE, roleValue) .build()); - IdentityProviderResource idpResource = realm.identityProviders().get(idp.getAlias()); - advancedClaimToRoleMapper.setIdentityProviderAlias(bc.getIDPAlias()); - idpResource.addMapper(advancedClaimToRoleMapper).close(); + persistMapper(advancedClaimToRoleMapper); + } + + @Override + protected Map> createUserConfigForRole(String roleValue) { + return Collections.emptyMap(); } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlAdvancedAttributeToRoleMapperTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlAdvancedAttributeToRoleMapperTest.java index 67b642b887..d0ca3e5254 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlAdvancedAttributeToRoleMapperTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlAdvancedAttributeToRoleMapperTest.java @@ -1,23 +1,22 @@ package org.keycloak.testsuite.broker; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; +import static org.keycloak.testsuite.broker.KcSamlBrokerConfiguration.ATTRIBUTE_TO_MAP_FRIENDLY_NAME; + import org.junit.Test; -import org.keycloak.admin.client.resource.IdentityProviderResource; import org.keycloak.broker.provider.ConfigConstants; import org.keycloak.broker.saml.mappers.AdvancedAttributeToRoleMapper; import org.keycloak.models.IdentityProviderMapperModel; import org.keycloak.models.IdentityProviderMapperSyncMode; import org.keycloak.representations.idm.IdentityProviderMapperRepresentation; -import org.keycloak.representations.idm.IdentityProviderRepresentation; -import org.keycloak.representations.idm.UserRepresentation; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import java.util.List; -import static org.keycloak.testsuite.broker.KcSamlBrokerConfiguration.ATTRIBUTE_TO_MAP_FRIENDLY_NAME; - /** - * Martin Idel + * @author Martin Idel, + * Daniel Fesenmeyer */ public class KcSamlAdvancedAttributeToRoleMapperTest extends AbstractAdvancedRoleMapperTest { @@ -39,34 +38,34 @@ public class KcSamlAdvancedAttributeToRoleMapperTest extends AbstractAdvancedRol } @Override - protected void createMapperInIdp(IdentityProviderRepresentation idp, String claimsOrAttributeRepresentation, boolean areClaimsOrAttributeValuesRegexes, IdentityProviderMapperSyncMode syncMode) { + protected void createMapperInIdp(String claimsOrAttributeRepresentation, + boolean areClaimsOrAttributeValuesRegexes, IdentityProviderMapperSyncMode syncMode, String roleValue) { IdentityProviderMapperRepresentation advancedAttributeToRoleMapper = new IdentityProviderMapperRepresentation(); advancedAttributeToRoleMapper.setName("advanced-attribute-to-role-mapper"); advancedAttributeToRoleMapper.setIdentityProviderMapper(AdvancedAttributeToRoleMapper.PROVIDER_ID); - advancedAttributeToRoleMapper.setConfig(ImmutableMap.builder() + advancedAttributeToRoleMapper.setConfig(ImmutableMap. builder() .put(IdentityProviderMapperModel.SYNC_MODE, syncMode.toString()) .put(AdvancedAttributeToRoleMapper.ATTRIBUTE_PROPERTY_NAME, claimsOrAttributeRepresentation) - .put(AdvancedAttributeToRoleMapper.ARE_ATTRIBUTE_VALUES_REGEX_PROPERTY_NAME, areClaimsOrAttributeValuesRegexes ? "true" : "false") - .put(ConfigConstants.ROLE, CLIENT_ROLE_MAPPER_REPRESENTATION) + .put(AdvancedAttributeToRoleMapper.ARE_ATTRIBUTE_VALUES_REGEX_PROPERTY_NAME, + Boolean.valueOf(areClaimsOrAttributeValuesRegexes).toString()) + .put(ConfigConstants.ROLE, roleValue) .build()); - IdentityProviderResource idpResource = realm.identityProviders().get(idp.getAlias()); - advancedAttributeToRoleMapper.setIdentityProviderAlias(bc.getIDPAlias()); - idpResource.addMapper(advancedAttributeToRoleMapper).close(); + persistMapper(advancedAttributeToRoleMapper); } @Test public void attributeFriendlyNameGetsConsideredAndMatchedToRole() { createAdvancedRoleMapper(ATTRIBUTES, false); - createUserInProviderRealm(ImmutableMap.>builder() - .put(ATTRIBUTE_TO_MAP_FRIENDLY_NAME, ImmutableList.builder().add("value 1").build()) - .put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME_2, ImmutableList.builder().add("value 2").build()) + createUserInProviderRealm(ImmutableMap.> builder() + .put(ATTRIBUTE_TO_MAP_FRIENDLY_NAME, ImmutableList. builder().add("value 1").build()) + .put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME_2, + ImmutableList. builder().add("value 2").build()) .build()); logInAsUserInIDPForFirstTime(); - UserRepresentation user = findUser(bc.consumerRealmName(), bc.getUserLogin(), bc.getUserEmail()); - assertThatRoleHasBeenAssignedInConsumerRealmTo(user); + assertThatRoleHasBeenAssignedInConsumerRealm(); } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlMultipleAttributeToRoleMappersTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlMultipleAttributeToRoleMappersTest.java index 77f7da6d4b..46304c7dba 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlMultipleAttributeToRoleMappersTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlMultipleAttributeToRoleMappersTest.java @@ -16,8 +16,6 @@ */ package org.keycloak.testsuite.broker; -import com.google.common.collect.ImmutableMap; -import org.keycloak.admin.client.resource.IdentityProviderResource; import org.keycloak.broker.provider.ConfigConstants; import org.keycloak.broker.saml.mappers.AdvancedAttributeToRoleMapper; import org.keycloak.broker.saml.mappers.AttributeToRoleMapper; @@ -25,7 +23,8 @@ import org.keycloak.broker.saml.mappers.UserAttributeMapper; import org.keycloak.models.IdentityProviderMapperModel; import org.keycloak.models.IdentityProviderMapperSyncMode; import org.keycloak.representations.idm.IdentityProviderMapperRepresentation; -import org.keycloak.representations.idm.IdentityProviderRepresentation; + +import com.google.common.collect.ImmutableMap; /** * Runs the same tests as {@link AttributeToRoleMapperTest} but using multiple SAML mappers that map different IDP attributes @@ -45,7 +44,8 @@ import org.keycloak.representations.idm.IdentityProviderRepresentation; * mapper actually succeeds in applying the mapping, the other two do nothing as the test user doesn't have the necessary * role/attribute(s). The test then verifies that the user still contains the mapped role after all mappers run. * - * @author Stefan Guilhen + * @author Stefan Guilhen, + * Daniel Fesenmeyer */ public class KcSamlMultipleAttributeToRoleMappersTest extends AttributeToRoleMapperTest { @@ -57,49 +57,48 @@ public class KcSamlMultipleAttributeToRoleMappersTest extends AttributeToRoleMap "]"; @Override - protected void createMapperInIdp(IdentityProviderRepresentation idp, IdentityProviderMapperSyncMode syncMode) { + protected void createMapperInIdp(IdentityProviderMapperSyncMode syncMode, String roleValue) { // first mapper that maps a role the test user has - it should perform the mapping. - IdentityProviderMapperRepresentation firstSamlAttributeToRoleMapper = new IdentityProviderMapperRepresentation(); + IdentityProviderMapperRepresentation firstSamlAttributeToRoleMapper = + new IdentityProviderMapperRepresentation(); firstSamlAttributeToRoleMapper.setName("first-role-mapper"); firstSamlAttributeToRoleMapper.setIdentityProviderMapper(AttributeToRoleMapper.PROVIDER_ID); - firstSamlAttributeToRoleMapper.setConfig(ImmutableMap.builder() + firstSamlAttributeToRoleMapper.setConfig(ImmutableMap. builder() .put(IdentityProviderMapperModel.SYNC_MODE, syncMode.toString()) .put(UserAttributeMapper.ATTRIBUTE_NAME, "Role") .put(ATTRIBUTE_VALUE, ROLE_USER) - .put(ConfigConstants.ROLE, CLIENT_ROLE_MAPPER_REPRESENTATION) + .put(ConfigConstants.ROLE, roleValue) .build()); - IdentityProviderResource idpResource = realm.identityProviders().get(idp.getAlias()); - firstSamlAttributeToRoleMapper.setIdentityProviderAlias(bc.getIDPAlias()); - idpResource.addMapper(firstSamlAttributeToRoleMapper).close(); + persistMapper(firstSamlAttributeToRoleMapper); - // second mapper that maps a role the test user doesn't have - it would normally end up removing the mapped role but - // it should now check if a previous mapper has already granted the same mapped role. - IdentityProviderMapperRepresentation secondSamlAttributeToRoleMapper = new IdentityProviderMapperRepresentation(); + // second mapper that maps a role the test user doesn't have - it would normally end up removing the mapped + // role, but it should now check if a previous mapper has already granted the same mapped role. + IdentityProviderMapperRepresentation secondSamlAttributeToRoleMapper = + new IdentityProviderMapperRepresentation(); secondSamlAttributeToRoleMapper.setName("second-role-mapper"); secondSamlAttributeToRoleMapper.setIdentityProviderMapper(AttributeToRoleMapper.PROVIDER_ID); - secondSamlAttributeToRoleMapper.setConfig(ImmutableMap.builder() + secondSamlAttributeToRoleMapper.setConfig(ImmutableMap. builder() .put(IdentityProviderMapperModel.SYNC_MODE, syncMode.toString()) .put(UserAttributeMapper.ATTRIBUTE_NAME, "Role") .put(ATTRIBUTE_VALUE, "missing-role") - .put(ConfigConstants.ROLE, CLIENT_ROLE_MAPPER_REPRESENTATION) + .put(ConfigConstants.ROLE, roleValue) .build()); - secondSamlAttributeToRoleMapper.setIdentityProviderAlias(bc.getIDPAlias()); - idpResource.addMapper(secondSamlAttributeToRoleMapper).close(); + persistMapper(secondSamlAttributeToRoleMapper); - // third mapper (advanced) that maps an attribute the test user doesn't have - it would normally end up removing the - // mapped role but it should now check if a previous mapper has already granted the same role. + // third mapper (advanced) that maps an attribute the test user doesn't have - it would normally end up removing + // the mapped role, but it should now check if a previous mapper has already granted the same role. IdentityProviderMapperRepresentation thirdSamlAttributeToRoleMapper = new IdentityProviderMapperRepresentation(); thirdSamlAttributeToRoleMapper.setName("advanced-role-mapper"); thirdSamlAttributeToRoleMapper.setIdentityProviderMapper(AdvancedAttributeToRoleMapper.PROVIDER_ID); - thirdSamlAttributeToRoleMapper.setConfig(ImmutableMap.builder() + thirdSamlAttributeToRoleMapper.setConfig(ImmutableMap. builder() .put(IdentityProviderMapperModel.SYNC_MODE, syncMode.toString()) .put(AdvancedAttributeToRoleMapper.ATTRIBUTE_PROPERTY_NAME, ATTRIBUTES_TO_MATCH) .put(AdvancedAttributeToRoleMapper.ARE_ATTRIBUTE_VALUES_REGEX_PROPERTY_NAME, Boolean.FALSE.toString()) - .put(ConfigConstants.ROLE, CLIENT_ROLE_MAPPER_REPRESENTATION) + .put(ConfigConstants.ROLE, roleValue) .build()); - thirdSamlAttributeToRoleMapper.setIdentityProviderAlias(bc.getIDPAlias()); - idpResource.addMapper(thirdSamlAttributeToRoleMapper).close(); + + persistMapper(thirdSamlAttributeToRoleMapper); } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/OidcAdvancedClaimToGroupMapperTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/OidcAdvancedClaimToGroupMapperTest.java index 1b21993312..81368598fd 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/OidcAdvancedClaimToGroupMapperTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/OidcAdvancedClaimToGroupMapperTest.java @@ -1,5 +1,6 @@ package org.keycloak.testsuite.broker; +import org.keycloak.admin.client.CreatedResponseUtil; import org.keycloak.admin.client.resource.IdentityProviderResource; import org.keycloak.broker.oidc.mappers.AdvancedClaimToGroupMapper; import org.keycloak.broker.provider.ConfigConstants; @@ -10,15 +11,22 @@ import org.keycloak.representations.idm.IdentityProviderRepresentation; import com.google.common.collect.ImmutableMap; +import javax.ws.rs.core.Response; + +/** + * @author Artur Baltabayev, + * Daniel Fesenmeyer + */ public class OidcAdvancedClaimToGroupMapperTest extends AbstractAdvancedGroupMapperTest { + @Override protected BrokerConfiguration getBrokerConfiguration() { return new KcOidcBrokerConfiguration(); } @Override - protected void createMapperInIdp(IdentityProviderRepresentation idp, String claimsOrAttributeRepresentation, - boolean areClaimsOrAttributeValuesRegexes, IdentityProviderMapperSyncMode syncMode) { + protected String createMapperInIdp(IdentityProviderRepresentation idp, String claimsOrAttributeRepresentation, + boolean areClaimsOrAttributeValuesRegexes, IdentityProviderMapperSyncMode syncMode, String groupPath) { IdentityProviderMapperRepresentation advancedClaimToGroupMapper = new IdentityProviderMapperRepresentation(); advancedClaimToGroupMapper.setName("advanced-claim-to-group-mapper"); advancedClaimToGroupMapper.setIdentityProviderMapper(AdvancedClaimToGroupMapper.PROVIDER_ID); @@ -26,12 +34,14 @@ public class OidcAdvancedClaimToGroupMapperTest extends AbstractAdvancedGroupMap .put(IdentityProviderMapperModel.SYNC_MODE, syncMode.toString()) .put(AdvancedClaimToGroupMapper.CLAIM_PROPERTY_NAME, claimsOrAttributeRepresentation) .put(AdvancedClaimToGroupMapper.ARE_CLAIM_VALUES_REGEX_PROPERTY_NAME, - areClaimsOrAttributeValuesRegexes ? "true" : "false") - .put(ConfigConstants.GROUP, MAPPER_TEST_GROUP_PATH) + Boolean.valueOf(areClaimsOrAttributeValuesRegexes).toString()) + .put(ConfigConstants.GROUP, groupPath) .build()); IdentityProviderResource idpResource = realm.identityProviders().get(idp.getAlias()); advancedClaimToGroupMapper.setIdentityProviderAlias(bc.getIDPAlias()); - idpResource.addMapper(advancedClaimToGroupMapper).close(); + Response response = idpResource.addMapper(advancedClaimToGroupMapper); + return CreatedResponseUtil.getCreatedId(response); } + } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/OidcAdvancedClaimToRoleMapperTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/OidcAdvancedClaimToRoleMapperTest.java index 7058877ce5..f57026495b 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/OidcAdvancedClaimToRoleMapperTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/OidcAdvancedClaimToRoleMapperTest.java @@ -1,17 +1,17 @@ package org.keycloak.testsuite.broker; -import com.google.common.collect.ImmutableMap; -import org.keycloak.admin.client.resource.IdentityProviderResource; import org.keycloak.broker.oidc.mappers.AdvancedClaimToRoleMapper; import org.keycloak.broker.provider.ConfigConstants; import org.keycloak.models.IdentityProviderMapperModel; import org.keycloak.models.IdentityProviderMapperSyncMode; import org.keycloak.representations.idm.IdentityProviderMapperRepresentation; -import org.keycloak.representations.idm.IdentityProviderRepresentation; + +import com.google.common.collect.ImmutableMap; /** * Benjamin Weimer, - * Martin Idel + * Martin Idel, + * Daniel Fesenmeyer */ public class OidcAdvancedClaimToRoleMapperTest extends AbstractAdvancedRoleMapperTest { @Override @@ -20,19 +20,19 @@ public class OidcAdvancedClaimToRoleMapperTest extends AbstractAdvancedRoleMappe } @Override - protected void createMapperInIdp(IdentityProviderRepresentation idp, String claimsOrAttributeRepresentation, boolean areClaimsOrAttributeValuesRegexes, IdentityProviderMapperSyncMode syncMode) { + protected void createMapperInIdp(String claimsOrAttributeRepresentation, + boolean areClaimsOrAttributeValuesRegexes, IdentityProviderMapperSyncMode syncMode, String roleValue) { IdentityProviderMapperRepresentation advancedClaimToRoleMapper = new IdentityProviderMapperRepresentation(); advancedClaimToRoleMapper.setName("advanced-claim-to-role-mapper"); advancedClaimToRoleMapper.setIdentityProviderMapper(AdvancedClaimToRoleMapper.PROVIDER_ID); - advancedClaimToRoleMapper.setConfig(ImmutableMap.builder() + advancedClaimToRoleMapper.setConfig(ImmutableMap. builder() .put(IdentityProviderMapperModel.SYNC_MODE, syncMode.toString()) .put(AdvancedClaimToRoleMapper.CLAIM_PROPERTY_NAME, claimsOrAttributeRepresentation) - .put(AdvancedClaimToRoleMapper.ARE_CLAIM_VALUES_REGEX_PROPERTY_NAME, areClaimsOrAttributeValuesRegexes ? "true" : "false") - .put(ConfigConstants.ROLE, CLIENT_ROLE_MAPPER_REPRESENTATION) + .put(AdvancedClaimToRoleMapper.ARE_CLAIM_VALUES_REGEX_PROPERTY_NAME, + Boolean.valueOf(areClaimsOrAttributeValuesRegexes).toString()) + .put(ConfigConstants.ROLE, roleValue) .build()); - IdentityProviderResource idpResource = realm.identityProviders().get(idp.getAlias()); - advancedClaimToRoleMapper.setIdentityProviderAlias(bc.getIDPAlias()); - idpResource.addMapper(advancedClaimToRoleMapper).close(); + persistMapper(advancedClaimToRoleMapper); } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/OidcClaimToRoleMapperTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/OidcClaimToRoleMapperTest.java index d694aa3aa1..3881bdd5d7 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/OidcClaimToRoleMapperTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/OidcClaimToRoleMapperTest.java @@ -3,24 +3,23 @@ package org.keycloak.testsuite.broker; import static org.keycloak.models.IdentityProviderMapperSyncMode.FORCE; import static org.keycloak.models.IdentityProviderMapperSyncMode.LEGACY; -import java.util.List; - -import org.jetbrains.annotations.NotNull; import org.junit.Test; -import org.keycloak.admin.client.resource.IdentityProviderResource; import org.keycloak.broker.oidc.mappers.ClaimToRoleMapper; import org.keycloak.broker.provider.ConfigConstants; import org.keycloak.models.IdentityProviderMapperModel; import org.keycloak.models.IdentityProviderMapperSyncMode; import org.keycloak.representations.idm.IdentityProviderMapperRepresentation; -import org.keycloak.representations.idm.IdentityProviderRepresentation; import org.keycloak.representations.idm.UserRepresentation; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import java.util.List; +import java.util.Map; + /** - * @author Martin Idel + * @author Martin Idel, + * Daniel Fesenmeyer */ public class OidcClaimToRoleMapperTest extends AbstractRoleMapperTest { @@ -36,89 +35,79 @@ public class OidcClaimToRoleMapperTest extends AbstractRoleMapperTest { @Test public void allClaimValuesMatch() { createClaimToRoleMapper(CLAIM_VALUE); - createUserInProviderRealm(ImmutableMap.>builder() - .put(CLAIM, ImmutableList.builder().add(CLAIM_VALUE).build()) - .build()); + createUserInProviderRealm(createUserConfig()); logInAsUserInIDPForFirstTime(); - UserRepresentation user = findUser(bc.consumerRealmName(), bc.getUserLogin(), bc.getUserEmail()); - assertThatRoleHasBeenAssignedInConsumerRealmTo(user); + assertThatRoleHasBeenAssignedInConsumerRealm(); } @Test public void claimValuesMismatch() { createClaimToRoleMapper("other value"); - createUserInProviderRealm(ImmutableMap.>builder() - .put(CLAIM, ImmutableList.builder().add(CLAIM_VALUE).build()) - .build()); + createUserInProviderRealm(createUserConfig()); logInAsUserInIDPForFirstTime(); - UserRepresentation user = findUser(bc.consumerRealmName(), bc.getUserLogin(), bc.getUserEmail()); - assertThatRoleHasNotBeenAssignedInConsumerRealmTo(user); + assertThatRoleHasNotBeenAssignedInConsumerRealm(); } @Test public void updateBrokeredUserMismatchDeletesRoleInForceMode() { - UserRepresentation user = loginWithClaimThenChangeClaimToValue("value mismatch", FORCE, false); + loginWithClaimThenChangeClaimToValue("value mismatch", FORCE, false); - assertThatRoleHasNotBeenAssignedInConsumerRealmTo(user); + assertThatRoleHasNotBeenAssignedInConsumerRealm(); } @Test public void updateBrokeredUserMismatchDeletesRoleInLegacyMode() { - UserRepresentation user = createMapperThenLoginWithStandardClaimThenChangeClaimToValue("value mismatch", LEGACY); + createMapperThenLoginWithStandardClaimThenChangeClaimToValue("value mismatch", LEGACY); - assertThatRoleHasNotBeenAssignedInConsumerRealmTo(user); + assertThatRoleHasNotBeenAssignedInConsumerRealm(); } @Test public void updateBrokeredUserNewMatchGrantsRoleAfterFirstLoginInForceMode() { - UserRepresentation user = loginWithStandardClaimThenAddMapperAndLoginAgain(FORCE); + loginWithStandardClaimThenAddMapperAndLoginAgain(FORCE); - assertThatRoleHasBeenAssignedInConsumerRealmTo(user); + assertThatRoleHasBeenAssignedInConsumerRealm(); } @Test public void updateBrokeredUserNewMatchDoesNotGrantRoleAfterFirstLoginInLegacyMode() { - UserRepresentation user = loginWithStandardClaimThenAddMapperAndLoginAgain(LEGACY); + loginWithStandardClaimThenAddMapperAndLoginAgain(LEGACY); - assertThatRoleHasNotBeenAssignedInConsumerRealmTo(user); + assertThatRoleHasNotBeenAssignedInConsumerRealm(); } @Test public void updateBrokeredUserDoesNotDeleteRoleIfClaimStillMatches() { - UserRepresentation user = createMapperThenLoginWithStandardClaimThenChangeClaimToValue(CLAIM_VALUE, FORCE); + createMapperThenLoginWithStandardClaimThenChangeClaimToValue(CLAIM_VALUE, FORCE); - assertThatRoleHasBeenAssignedInConsumerRealmTo(user); + assertThatRoleHasBeenAssignedInConsumerRealm(); } - private UserRepresentation loginWithStandardClaimThenAddMapperAndLoginAgain(IdentityProviderMapperSyncMode syncMode) { - return loginWithClaimThenChangeClaimToValue(OidcClaimToRoleMapperTest.CLAIM_VALUE, syncMode, true); + private void loginWithStandardClaimThenAddMapperAndLoginAgain(IdentityProviderMapperSyncMode syncMode) { + loginWithClaimThenChangeClaimToValue(OidcClaimToRoleMapperTest.CLAIM_VALUE, syncMode, true); } - private UserRepresentation createMapperThenLoginWithStandardClaimThenChangeClaimToValue(String claimOnSecondLogin, IdentityProviderMapperSyncMode syncMode) { - return loginWithClaimThenChangeClaimToValue(claimOnSecondLogin, syncMode, false); + private void createMapperThenLoginWithStandardClaimThenChangeClaimToValue(String claimOnSecondLogin, IdentityProviderMapperSyncMode syncMode) { + loginWithClaimThenChangeClaimToValue(claimOnSecondLogin, syncMode, false); } - @NotNull - private UserRepresentation loginWithClaimThenChangeClaimToValue(String claimOnSecondLogin, IdentityProviderMapperSyncMode syncMode, boolean createAfterFirstLogin) { + private void loginWithClaimThenChangeClaimToValue(String claimOnSecondLogin, IdentityProviderMapperSyncMode syncMode, boolean createAfterFirstLogin) { this.claimOnSecondLogin = claimOnSecondLogin; - return loginAsUserTwiceWithMapper(syncMode, createAfterFirstLogin, - ImmutableMap.>builder() - .put(CLAIM, ImmutableList.builder().add(CLAIM_VALUE).build()) - .build()); + loginAsUserTwiceWithMapper(syncMode, createAfterFirstLogin, createUserConfig()); } private void createClaimToRoleMapper(String claimValue) { - IdentityProviderRepresentation idp = setupIdentityProvider(); - createClaimToRoleMapper(idp, claimValue, IdentityProviderMapperSyncMode.IMPORT); + setupIdentityProvider(); + createClaimToRoleMapper(claimValue, IdentityProviderMapperSyncMode.IMPORT, CLIENT_ROLE_MAPPER_REPRESENTATION); } @Override - protected void createMapperInIdp(IdentityProviderRepresentation idp, IdentityProviderMapperSyncMode syncMode) { - createClaimToRoleMapper(idp, CLAIM_VALUE, syncMode); + protected void createMapperInIdp(IdentityProviderMapperSyncMode syncMode, String roleValue) { + createClaimToRoleMapper(CLAIM_VALUE, syncMode, roleValue); } @Override @@ -131,19 +120,29 @@ public class OidcClaimToRoleMapperTest extends AbstractRoleMapperTest { adminClient.realm(bc.providerRealmName()).users().get(user.getId()).update(user); } - protected void createClaimToRoleMapper(IdentityProviderRepresentation idp, String claimValue, IdentityProviderMapperSyncMode syncMode) { + protected void createClaimToRoleMapper(String claimValue, IdentityProviderMapperSyncMode syncMode, + String roleValue) { IdentityProviderMapperRepresentation claimToRoleMapper = new IdentityProviderMapperRepresentation(); claimToRoleMapper.setName("claim-to-role-mapper"); claimToRoleMapper.setIdentityProviderMapper(ClaimToRoleMapper.PROVIDER_ID); - claimToRoleMapper.setConfig(ImmutableMap.builder() - .put(IdentityProviderMapperModel.SYNC_MODE, syncMode.toString()) - .put(ClaimToRoleMapper.CLAIM, OidcClaimToRoleMapperTest.CLAIM) - .put(ClaimToRoleMapper.CLAIM_VALUE, claimValue) - .put(ConfigConstants.ROLE, CLIENT_ROLE_MAPPER_REPRESENTATION) - .build()); + claimToRoleMapper.setConfig(ImmutableMap. builder() + .put(IdentityProviderMapperModel.SYNC_MODE, syncMode.toString()) + .put(ClaimToRoleMapper.CLAIM, OidcClaimToRoleMapperTest.CLAIM) + .put(ClaimToRoleMapper.CLAIM_VALUE, claimValue) + .put(ConfigConstants.ROLE, roleValue) + .build()); - IdentityProviderResource idpResource = realm.identityProviders().get(idp.getAlias()); - claimToRoleMapper.setIdentityProviderAlias(bc.getIDPAlias()); - idpResource.addMapper(claimToRoleMapper).close(); + persistMapper(claimToRoleMapper); + } + + @Override + protected Map> createUserConfigForRole(String roleValue) { + return createUserConfig(); + } + + private static ImmutableMap> createUserConfig() { + return ImmutableMap.>builder() + .put(CLAIM, ImmutableList.builder().add(CLAIM_VALUE).build()) + .build(); } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/OidcMultipleClaimToRoleMappersTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/OidcMultipleClaimToRoleMappersTest.java index 9156a78032..18ed272a49 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/OidcMultipleClaimToRoleMappersTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/OidcMultipleClaimToRoleMappersTest.java @@ -16,8 +16,6 @@ */ package org.keycloak.testsuite.broker; -import com.google.common.collect.ImmutableMap; -import org.keycloak.admin.client.resource.IdentityProviderResource; import org.keycloak.broker.oidc.mappers.AdvancedClaimToRoleMapper; import org.keycloak.broker.oidc.mappers.ClaimToRoleMapper; import org.keycloak.broker.oidc.mappers.ExternalKeycloakRoleToRoleMapper; @@ -25,7 +23,8 @@ import org.keycloak.broker.provider.ConfigConstants; import org.keycloak.models.IdentityProviderMapperModel; import org.keycloak.models.IdentityProviderMapperSyncMode; import org.keycloak.representations.idm.IdentityProviderMapperRepresentation; -import org.keycloak.representations.idm.IdentityProviderRepresentation; + +import com.google.common.collect.ImmutableMap; /** * Runs the same tests as {@link OidcClaimToRoleMapperTest} but using multiple OIDC mappers that map different IDP claims @@ -45,7 +44,8 @@ import org.keycloak.representations.idm.IdentityProviderRepresentation; * mapper actually succeeds in applying the mapping, the other two do nothing as the test user doesn't have the necessary * role/attribute(s). The test then verifies that the user still contains the mapped role after all mappers run. * - * @author Stefan Guilhen + * @author Stefan Guilhen, + * Daniel Fesenmeyer */ public class OidcMultipleClaimToRoleMappersTest extends OidcClaimToRoleMapperTest { @@ -57,48 +57,46 @@ public class OidcMultipleClaimToRoleMappersTest extends OidcClaimToRoleMapperTes "]"; @Override - protected void createClaimToRoleMapper(IdentityProviderRepresentation idp, String claimValue, IdentityProviderMapperSyncMode syncMode) { + protected void createClaimToRoleMapper(String claimValue, IdentityProviderMapperSyncMode syncMode, + String roleValue) { // first mapper that maps attributes the user has - it should perform the mapping to the expected role. IdentityProviderMapperRepresentation firstOidcClaimToRoleMapper = new IdentityProviderMapperRepresentation(); firstOidcClaimToRoleMapper.setName("claim-to-role-mapper"); firstOidcClaimToRoleMapper.setIdentityProviderMapper(ClaimToRoleMapper.PROVIDER_ID); - firstOidcClaimToRoleMapper.setConfig(ImmutableMap.builder() + firstOidcClaimToRoleMapper.setConfig(ImmutableMap. builder() .put(IdentityProviderMapperModel.SYNC_MODE, syncMode.toString()) .put(ClaimToRoleMapper.CLAIM, OidcClaimToRoleMapperTest.CLAIM) .put(ClaimToRoleMapper.CLAIM_VALUE, claimValue) - .put(ConfigConstants.ROLE, CLIENT_ROLE_MAPPER_REPRESENTATION) + .put(ConfigConstants.ROLE, roleValue) .build()); - IdentityProviderResource idpResource = realm.identityProviders().get(idp.getAlias()); - firstOidcClaimToRoleMapper.setIdentityProviderAlias(bc.getIDPAlias()); - idpResource.addMapper(firstOidcClaimToRoleMapper).close(); + persistMapper(firstOidcClaimToRoleMapper); - // second mapper that maps an external role claim the test user doesn't have - it would normally end up removing the - // mapped role but it should now check if a previous mapper has already granted the same role. + // second mapper that maps an external role claim the test user doesn't have - it would normally end up removing + // the mapped role, but it should now check if a previous mapper has already granted the same role. IdentityProviderMapperRepresentation secondOidcClaimToRoleMapper = new IdentityProviderMapperRepresentation(); secondOidcClaimToRoleMapper.setName("external-keycloak-role-mapper"); secondOidcClaimToRoleMapper.setIdentityProviderMapper(ExternalKeycloakRoleToRoleMapper.PROVIDER_ID); - secondOidcClaimToRoleMapper.setConfig(ImmutableMap.builder() + secondOidcClaimToRoleMapper.setConfig(ImmutableMap. builder() .put(IdentityProviderMapperModel.SYNC_MODE, syncMode.toString()) .put("external.role", "missing-role") - .put("role", CLIENT_ROLE_MAPPER_REPRESENTATION) + .put(ConfigConstants.ROLE, roleValue) .build()); - secondOidcClaimToRoleMapper.setIdentityProviderAlias(bc.getIDPAlias()); - idpResource.addMapper(secondOidcClaimToRoleMapper).close(); + + persistMapper(secondOidcClaimToRoleMapper); // third mapper (advanced) that maps a claim the test user doesn't have - it would normally end up removing the - // mapped role but it should now check if a previous mapper has already granted the same role. + // mapped role, but it should now check if a previous mapper has already granted the same role. IdentityProviderMapperRepresentation thirdOidcClaimToRoleMapper = new IdentityProviderMapperRepresentation(); thirdOidcClaimToRoleMapper.setName("advanced-claim-to-role-mapper"); thirdOidcClaimToRoleMapper.setIdentityProviderMapper(AdvancedClaimToRoleMapper.PROVIDER_ID); - thirdOidcClaimToRoleMapper.setConfig(ImmutableMap.builder() + thirdOidcClaimToRoleMapper.setConfig(ImmutableMap. builder() .put(IdentityProviderMapperModel.SYNC_MODE, syncMode.toString()) .put(AdvancedClaimToRoleMapper.CLAIM_PROPERTY_NAME, CLAIMS_OR_ATTRIBUTES) .put(AdvancedClaimToRoleMapper.ARE_CLAIM_VALUES_REGEX_PROPERTY_NAME, Boolean.TRUE.toString()) - .put(ConfigConstants.ROLE, CLIENT_ROLE_MAPPER_REPRESENTATION) + .put(ConfigConstants.ROLE, roleValue) .build()); - thirdOidcClaimToRoleMapper.setIdentityProviderAlias(bc.getIDPAlias()); - idpResource.addMapper(thirdOidcClaimToRoleMapper).close(); + persistMapper(thirdOidcClaimToRoleMapper); } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/OidcUserInfoClaimToRoleMapperTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/OidcUserInfoClaimToRoleMapperTest.java index a145534314..8dc4feb875 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/OidcUserInfoClaimToRoleMapperTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/OidcUserInfoClaimToRoleMapperTest.java @@ -1,9 +1,6 @@ package org.keycloak.testsuite.broker; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; import org.junit.Test; -import org.keycloak.admin.client.resource.IdentityProviderResource; import org.keycloak.broker.oidc.mappers.ClaimToRoleMapper; import org.keycloak.broker.provider.ConfigConstants; import org.keycloak.models.IdentityProviderMapperModel; @@ -14,18 +11,28 @@ import org.keycloak.protocol.oidc.OIDCLoginProtocol; import org.keycloak.protocol.oidc.mappers.OIDCAttributeMapperHelper; import org.keycloak.protocol.oidc.mappers.UserAttributeMapper; import org.keycloak.provider.ProviderConfigProperty; -import org.keycloak.representations.idm.*; +import org.keycloak.representations.idm.ClientRepresentation; +import org.keycloak.representations.idm.IdentityProviderMapperRepresentation; +import org.keycloak.representations.idm.ProtocolMapperRepresentation; +import org.keycloak.representations.idm.UserRepresentation; -import java.util.Arrays; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; + +import java.util.Collections; import java.util.List; import java.util.Map; +/** + * @author Dashaylan Naidoo, + * Daniel Fesenmeyer + */ public class OidcUserInfoClaimToRoleMapperTest extends AbstractRoleMapperTest { protected static final String ATTRIBUTE_TO_MAP_USER_INFO = "user-attribute-info"; private static final String USER_INFO_CLAIM = ATTRIBUTE_TO_MAP_USER_INFO; private static final String USER_INFO_CLAIM_VALUE = "value 1"; - private String claimOnSecondLogin = ""; + private static final String CLAIM_ON_SECOND_LOGIN = ""; @Override @@ -35,69 +42,63 @@ public class OidcUserInfoClaimToRoleMapperTest extends AbstractRoleMapperTest { @Test public void singleClaimValueInUserInfoMatches() { - createClaimToRoleMapper(USER_INFO_CLAIM_VALUE); - createUserInProviderRealm(ImmutableMap.>builder() - .put(USER_INFO_CLAIM, ImmutableList.builder().add(USER_INFO_CLAIM_VALUE).build()) - .build()); + createClaimToRoleMapper(); + createUserInProviderRealm(createUserConfig()); logInAsUserInIDPForFirstTime(); - UserRepresentation user = findUser(bc.consumerRealmName(), bc.getUserLogin(), bc.getUserEmail()); - assertThatRoleHasBeenAssignedInConsumerRealmTo(user); + assertThatRoleHasBeenAssignedInConsumerRealm(); } @Test public void noRoleAddedIfUserInfoDisabledAndOnlyClaimIsInUserInfo() { - createClaimToRoleMapperWithUserInfoDisabledInIdP(USER_INFO_CLAIM_VALUE); - createUserInProviderRealm(ImmutableMap.>builder() - .put(USER_INFO_CLAIM, ImmutableList.builder().add(USER_INFO_CLAIM_VALUE).build()) - .build()); + createClaimToRoleMapperWithUserInfoDisabledInIdP(); + createUserInProviderRealm(createUserConfig()); logInAsUserInIDPForFirstTime(); - UserRepresentation user = findUser(bc.consumerRealmName(), bc.getUserLogin(), bc.getUserEmail()); - assertThatRoleHasNotBeenAssignedInConsumerRealmTo(user); + assertThatRoleHasNotBeenAssignedInConsumerRealm(); } - private void createClaimToRoleMapper(String claimValue) { - IdentityProviderRepresentation idp = setupIdentityProvider(); - createClaimToRoleMapper(idp, claimValue, IdentityProviderMapperSyncMode.IMPORT); + private void createClaimToRoleMapper() { + setupIdentityProvider(); + createClaimToRoleMapper(OidcUserInfoClaimToRoleMapperTest.USER_INFO_CLAIM_VALUE, + IdentityProviderMapperSyncMode.IMPORT, CLIENT_ROLE_MAPPER_REPRESENTATION); } - private void createClaimToRoleMapperWithUserInfoDisabledInIdP(String claimValue) { - IdentityProviderRepresentation idp = setupIdentityProviderDisableUserInfo(); - createClaimToRoleMapper(idp, claimValue, IdentityProviderMapperSyncMode.IMPORT); + private void createClaimToRoleMapperWithUserInfoDisabledInIdP() { + setupIdentityProviderDisableUserInfo(); + createClaimToRoleMapper(OidcUserInfoClaimToRoleMapperTest.USER_INFO_CLAIM_VALUE, + IdentityProviderMapperSyncMode.IMPORT, CLIENT_ROLE_MAPPER_REPRESENTATION); } @Override - protected void createMapperInIdp(IdentityProviderRepresentation idp, IdentityProviderMapperSyncMode syncMode) { - createClaimToRoleMapper(idp, USER_INFO_CLAIM_VALUE, syncMode); + protected void createMapperInIdp(IdentityProviderMapperSyncMode syncMode, String roleValue) { + createClaimToRoleMapper(USER_INFO_CLAIM_VALUE, syncMode, roleValue); } @Override protected void updateUser() { UserRepresentation user = findUser(bc.providerRealmName(), bc.getUserLogin(), bc.getUserEmail()); - ImmutableMap> mismatchingAttributes = ImmutableMap.>builder() - .put(USER_INFO_CLAIM, ImmutableList.builder().add(claimOnSecondLogin).build()) - .build(); + ImmutableMap> mismatchingAttributes = ImmutableMap.> builder() + .put(USER_INFO_CLAIM, ImmutableList. builder().add(CLAIM_ON_SECOND_LOGIN).build()) + .build(); user.setAttributes(mismatchingAttributes); adminClient.realm(bc.providerRealmName()).users().get(user.getId()).update(user); } - private void createClaimToRoleMapper(IdentityProviderRepresentation idp, String claimValue, IdentityProviderMapperSyncMode syncMode) { + private void createClaimToRoleMapper(String claimValue, IdentityProviderMapperSyncMode syncMode, String roleValue) { IdentityProviderMapperRepresentation claimToRoleMapper = new IdentityProviderMapperRepresentation(); claimToRoleMapper.setName("userinfo-claim-to-role-mapper"); claimToRoleMapper.setIdentityProviderMapper(ClaimToRoleMapper.PROVIDER_ID); - claimToRoleMapper.setConfig(ImmutableMap.builder() - .put(IdentityProviderMapperModel.SYNC_MODE, syncMode.toString()) - .put(ClaimToRoleMapper.CLAIM, OidcUserInfoClaimToRoleMapperTest.USER_INFO_CLAIM) - .put(ClaimToRoleMapper.CLAIM_VALUE, claimValue) - .put(ConfigConstants.ROLE, CLIENT_ROLE_MAPPER_REPRESENTATION) - .build()); + claimToRoleMapper.setConfig(ImmutableMap. builder() + .put(IdentityProviderMapperModel.SYNC_MODE, syncMode.toString()) + .put(ClaimToRoleMapper.CLAIM, OidcUserInfoClaimToRoleMapperTest.USER_INFO_CLAIM) + .put(ClaimToRoleMapper.CLAIM_VALUE, claimValue) + .put(ConfigConstants.ROLE, roleValue) + .build()); - IdentityProviderResource idpResource = realm.identityProviders().get(idp.getAlias()); - claimToRoleMapper.setIdentityProviderAlias(bc.getIDPAlias()); - idpResource.addMapper(claimToRoleMapper).close(); + persistMapper(claimToRoleMapper); } private class KcOidcBrokerConfigurationUserInfoOnlyMappers extends KcOidcBrokerConfiguration { @@ -120,8 +121,8 @@ public class OidcUserInfoClaimToRoleMapperTest extends AbstractRoleMapperTest { userAttrMapperConfig.put(OIDCAttributeMapperHelper.INCLUDE_IN_ID_TOKEN, "false"); userAttrMapperConfig.put(OIDCAttributeMapperHelper.INCLUDE_IN_USERINFO, "true"); - for (ClientRepresentation client: clientsRepList) { - client.setProtocolMappers(Arrays.asList(userAttrMapper)); + for (ClientRepresentation client : clientsRepList) { + client.setProtocolMappers(Collections.singletonList(userAttrMapper)); } return clientsRepList; @@ -135,4 +136,14 @@ public class OidcUserInfoClaimToRoleMapperTest extends AbstractRoleMapperTest { } } + @Override + protected Map> createUserConfigForRole(String roleValue) { + return createUserConfig(); + } + + private static Map> createUserConfig() { + return ImmutableMap.> builder() + .put(USER_INFO_CLAIM, ImmutableList. builder().add(USER_INFO_CLAIM_VALUE).build()) + .build(); + } } diff --git a/testsuite/model/src/test/java/org/keycloak/testsuite/model/RealmModelTest.java b/testsuite/model/src/test/java/org/keycloak/testsuite/model/RealmModelTest.java index 7133e841dc..ebfc779671 100644 --- a/testsuite/model/src/test/java/org/keycloak/testsuite/model/RealmModelTest.java +++ b/testsuite/model/src/test/java/org/keycloak/testsuite/model/RealmModelTest.java @@ -21,12 +21,16 @@ import org.keycloak.authorization.AuthorizationProvider; import org.keycloak.authorization.model.ResourceServer; import org.keycloak.models.ClientModel; import org.keycloak.models.Constants; +import org.keycloak.models.GroupModel; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.models.RealmProvider; +import org.keycloak.models.utils.KeycloakModelUtils; +import org.keycloak.provider.ProviderEventListener; +import java.util.ArrayList; +import java.util.List; import java.util.function.Consumer; -import java.util.function.Function; import static org.hamcrest.CoreMatchers.allOf; import static org.hamcrest.MatcherAssert.assertThat; @@ -34,6 +38,7 @@ import static org.hamcrest.Matchers.aMapWithSize; import static org.hamcrest.Matchers.anEmptyMap; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasEntry; +import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.notNullValue; @RequireProvider(RealmProvider.class) @@ -108,12 +113,12 @@ public class RealmModelTest extends KeycloakModelTest { @Test public void testRealmPreRemoveDoesntRemoveEntitiesFromOtherRealms() { - realm1Id = inComittedTransaction((Function) session -> { + realm1Id = inComittedTransaction(session -> { RealmModel realm = session.realms().createRealm("realm1"); realm.setDefaultRole(session.roles().addRealmRole(realm, Constants.DEFAULT_ROLES_ROLE_PREFIX + "-" + realm.getName())); return realm.getId(); }); - realm2Id = inComittedTransaction((Function) session -> { + realm2Id = inComittedTransaction(session -> { RealmModel realm = session.realms().createRealm("realm2"); realm.setDefaultRole(session.roles().addRealmRole(realm, Constants.DEFAULT_ROLES_ROLE_PREFIX + "-" + realm.getName())); return realm.getId(); @@ -140,4 +145,42 @@ public class RealmModelTest extends KeycloakModelTest { assertThat(resourceServer, notNullValue()); } + + @Test + public void testMoveGroup() { + ProviderEventListener providerEventListener = null; + try { + List groupPathChangeEvents = new ArrayList<>(); + providerEventListener = event -> { + if (event instanceof GroupModel.GroupPathChangeEvent) { + groupPathChangeEvents.add((GroupModel.GroupPathChangeEvent) event); + } + }; + getFactory().register(providerEventListener); + + withRealm(realmId, (session, realm) -> { + GroupModel groupA = realm.createGroup("a"); + GroupModel groupB = realm.createGroup("b"); + + final String previousPath = "/a"; + assertThat(KeycloakModelUtils.buildGroupPath(groupA), equalTo(previousPath)); + + realm.moveGroup(groupA, groupB); + + final String expectedNewPath = "/b/a"; + assertThat(KeycloakModelUtils.buildGroupPath(groupA), equalTo(expectedNewPath)); + + assertThat(groupPathChangeEvents, hasSize(1)); + GroupModel.GroupPathChangeEvent groupPathChangeEvent = groupPathChangeEvents.get(0); + assertThat(groupPathChangeEvent.getPreviousPath(), equalTo(previousPath)); + assertThat(groupPathChangeEvent.getNewPath(), equalTo(expectedNewPath)); + + return null; + }); + } finally { + if (providerEventListener != null) { + getFactory().unregister(providerEventListener); + } + } + } }