From e34ff6cd9c18f1761501cafe920c27a40aff5ce9 Mon Sep 17 00:00:00 2001 From: Martin Bartos Date: Wed, 2 Sep 2020 14:52:30 +0200 Subject: [PATCH] [KEYCLOAK-14326] Identity Provider force sync is not working --- .../resources/IdentityBrokerService.java | 21 ++++ .../testsuite/broker/KcOidcBrokerTest.java | 99 +++++++++++++++++++ 2 files changed, 120 insertions(+) diff --git a/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java b/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java index 71807e9a58..b54c4bb8e8 100755 --- a/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java +++ b/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java @@ -26,6 +26,8 @@ import org.keycloak.authentication.AuthenticationFlowException; import org.keycloak.authentication.authenticators.broker.AbstractIdpAuthenticator; import org.keycloak.authentication.authenticators.broker.util.PostBrokerLoginConstants; import org.keycloak.authentication.authenticators.broker.util.SerializedBrokeredIdentityContext; +import org.keycloak.broker.oidc.KeycloakOIDCIdentityProviderFactory; +import org.keycloak.broker.oidc.mappers.UserAttributeMapper; import org.keycloak.broker.provider.AuthenticationRequest; import org.keycloak.broker.provider.BrokeredIdentityContext; import org.keycloak.broker.provider.IdentityBrokerException; @@ -54,6 +56,7 @@ import org.keycloak.models.Constants; import org.keycloak.models.FederatedIdentityModel; import org.keycloak.models.IdentityProviderMapperModel; import org.keycloak.models.IdentityProviderModel; +import org.keycloak.models.IdentityProviderSyncMode; import org.keycloak.models.KeycloakSession; import org.keycloak.models.KeycloakSessionFactory; import org.keycloak.models.RealmModel; @@ -120,6 +123,7 @@ import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.UUID; +import java.util.function.Consumer; /** *

@@ -1007,6 +1011,10 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal private void updateFederatedIdentity(BrokeredIdentityContext context, UserModel federatedUser) { FederatedIdentityModel federatedIdentityModel = this.session.users().getFederatedIdentity(federatedUser, context.getIdpConfig().getAlias(), this.realmModel); + if (context.getIdpConfig().getSyncMode() == IdentityProviderSyncMode.FORCE) { + setBasicUserAttributes(context, federatedUser); + } + // Skip DB write if tokens are null or equal updateToken(context, federatedUser, federatedIdentityModel); context.getIdp().updateBrokeredUser(session, realmModel, federatedUser, context); @@ -1021,6 +1029,19 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal } + private void setBasicUserAttributes(BrokeredIdentityContext context, UserModel federatedUser) { + setDiffAttrToConsumer(federatedUser.getEmail(), context.getEmail(), federatedUser::setEmail); + setDiffAttrToConsumer(federatedUser.getFirstName(), context.getFirstName(), federatedUser::setFirstName); + setDiffAttrToConsumer(federatedUser.getLastName(), context.getLastName(), federatedUser::setLastName); + } + + private void setDiffAttrToConsumer(String actualValue, String newValue, Consumer consumer) { + String actualValueNotNull = Optional.ofNullable(actualValue).orElse(""); + if (newValue != null && !newValue.equals(actualValueNotNull)) { + consumer.accept(newValue); + } + } + private void migrateFederatedIdentityId(BrokeredIdentityContext context, UserModel federatedUser) { FederatedIdentityModel identityModel = this.session.users().getFederatedIdentity(federatedUser, context.getIdpConfig().getAlias(), this.realmModel); FederatedIdentityModel migratedIdentityModel = new FederatedIdentityModel(identityModel, context.getId()); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerTest.java index f66fd9ef95..a62ca2573f 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerTest.java @@ -16,6 +16,8 @@ import org.keycloak.broker.oidc.mappers.UserAttributeMapper; import org.keycloak.crypto.Algorithm; import org.keycloak.models.IdentityProviderMapperModel; import org.keycloak.models.IdentityProviderMapperSyncMode; +import org.keycloak.models.IdentityProviderModel; +import org.keycloak.models.IdentityProviderSyncMode; import org.keycloak.protocol.oidc.OIDCConfigAttributes; import org.keycloak.provider.ProviderConfigProperty; import org.keycloak.representations.idm.ClientRepresentation; @@ -26,6 +28,7 @@ import org.keycloak.representations.idm.RoleRepresentation; import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.testsuite.Assert; import org.keycloak.testsuite.admin.ApiUtil; +import org.keycloak.testsuite.util.WaitUtils; import javax.ws.rs.core.Response; import java.util.Collections; @@ -445,6 +448,102 @@ public final class KcOidcBrokerTest extends AbstractAdvancedBrokerTest { } } + @Test + public void testIdPForceSyncUserAttributes() { + checkUpdatedUserAttributesIdP(true); + } + + @Test + public void testIdPNotForceSyncUserAttributes() { + checkUpdatedUserAttributesIdP(false); + } + + private void checkUpdatedUserAttributesIdP(boolean isForceSync) { + final String IDP_NAME = getBrokerConfiguration().getIDPAlias(); + final String USERNAME = "demoUser"; + + final String FIRST_NAME = "John"; + final String LAST_NAME = "Doe"; + final String EMAIL = "mail@example.com"; + + final String NEW_FIRST_NAME = "Jack"; + final String NEW_LAST_NAME = "Doee"; + final String NEW_EMAIL = "mail123@example.com"; + + UsersResource providerUserResource = Optional.ofNullable(realmsResouce().realm(bc.providerRealmName()).users()).orElse(null); + assertThat("Cannot get User Resource from Provider realm", providerUserResource, Matchers.notNullValue()); + + String userID = createUser(bc.providerRealmName(), USERNAME, USERNAME, FIRST_NAME, LAST_NAME, EMAIL); + assertThat("Cannot create user : " + USERNAME, userID, Matchers.notNullValue()); + + try { + UserRepresentation user = Optional.ofNullable(providerUserResource.get(userID).toRepresentation()).orElse(null); + assertThat("Cannot get user from provider", user, Matchers.notNullValue()); + + IdentityProviderResource consumerIdentityResource = Optional.ofNullable(getIdentityProviderResource()).orElse(null); + assertThat("Cannot get Identity Provider resource", consumerIdentityResource, Matchers.notNullValue()); + + IdentityProviderRepresentation idProvider = Optional.ofNullable(consumerIdentityResource.toRepresentation()).orElse(null); + assertThat("Cannot get Identity Provider", idProvider, Matchers.notNullValue()); + + updateIdPSyncMode(idProvider, consumerIdentityResource, isForceSync ? IdentityProviderSyncMode.FORCE : IdentityProviderSyncMode.IMPORT); + + driver.navigate().to(getAccountUrl(getConsumerRoot(), bc.consumerRealmName())); + WaitUtils.waitForPageToLoad(); + + assertThat(driver.getTitle(), Matchers.containsString("Log in to " + bc.consumerRealmName())); + logInWithIdp(IDP_NAME, USERNAME, USERNAME); + accountUpdateProfilePage.assertCurrent(); + + logoutFromRealm(getProviderRoot(), bc.providerRealmName()); + logoutFromRealm(getConsumerRoot(), bc.consumerRealmName()); + + driver.navigate().to(getAccountUrl(getProviderRoot(), bc.providerRealmName())); + WaitUtils.waitForPageToLoad(); + + assertThat(driver.getTitle(), Matchers.containsString("Log in to " + bc.providerRealmName())); + + loginPage.login(USERNAME, USERNAME); + WaitUtils.waitForPageToLoad(); + + accountUpdateProfilePage.assertCurrent(); + accountUpdateProfilePage.updateProfile(NEW_FIRST_NAME, NEW_LAST_NAME, NEW_EMAIL); + logoutFromRealm(getProviderRoot(), bc.providerRealmName()); + + driver.navigate().to(getAccountUrl(getConsumerRoot(), bc.consumerRealmName())); + WaitUtils.waitForPageToLoad(); + + assertThat(driver.getTitle(), Matchers.containsString("Log in to " + bc.consumerRealmName())); + logInWithIdp(IDP_NAME, USERNAME, USERNAME); + + accountUpdateProfilePage.assertCurrent(); + + assertThat(accountUpdateProfilePage.getEmail(), Matchers.equalTo(isForceSync ? NEW_EMAIL : EMAIL)); + assertThat(accountUpdateProfilePage.getFirstName(), Matchers.equalTo(isForceSync ? NEW_FIRST_NAME : FIRST_NAME)); + assertThat(accountUpdateProfilePage.getLastName(), Matchers.equalTo(isForceSync ? NEW_LAST_NAME : LAST_NAME)); + } finally { + providerUserResource.delete(userID); + assertThat("User wasn't deleted", providerUserResource.search(USERNAME).size(), Matchers.is(0)); + } + } + + private void updateIdPSyncMode(IdentityProviderRepresentation idProvider, IdentityProviderResource idProviderResource, IdentityProviderSyncMode syncMode) { + assertThat(idProvider, Matchers.notNullValue()); + assertThat(idProviderResource, Matchers.notNullValue()); + assertThat(syncMode, Matchers.notNullValue()); + + if (idProvider.getConfig().get(IdentityProviderModel.SYNC_MODE).equals(syncMode.name())) { + return; + } + + idProvider.getConfig().put(IdentityProviderModel.SYNC_MODE, syncMode.name()); + idProviderResource.update(idProvider); + + idProvider = Optional.ofNullable(idProviderResource.toRepresentation()).orElse(null); + assertThat("Cannot get Identity Provider", idProvider, Matchers.notNullValue()); + assertThat("Sync mode didn't change", idProvider.getConfig().get(IdentityProviderModel.SYNC_MODE), Matchers.equalTo(syncMode.name())); + } + private UserRepresentation getFederatedIdentity() { List users = realmsResouce().realm(bc.consumerRealmName()).users().search(bc.getUserLogin());