[KEYCLOAK-9841] use LDAPUser UUID as an identifier instead of username
This commit is contained in:
Pascal Euhus
Marek Posolda
parent
c76ca4ad13
commit
82fc401298
5 changed files with 153 additions and 14 deletions
|
|
@ -486,8 +486,11 @@ public class LDAPStorageProvider implements UserStorageProvider,
|
|||
return existing;
|
||||
}
|
||||
|
||||
LDAPObject ldapUser = loadLDAPUserByUsername(realm, local.getUsername());
|
||||
if (ldapUser == null) {
|
||||
String uuidLdapAttribute = local.getFirstAttribute(LDAPConstants.LDAP_ID);
|
||||
|
||||
LDAPObject ldapUser = loadLDAPUserByUuid(realm, uuidLdapAttribute);
|
||||
|
||||
if(ldapUser == null){
|
||||
return null;
|
||||
}
|
||||
LDAPUtils.checkUuid(ldapUser, ldapIdentityStore.getConfig());
|
||||
|
|
@ -516,7 +519,17 @@ public class LDAPStorageProvider implements UserStorageProvider,
|
|||
|
||||
UserModel imported = null;
|
||||
if (model.isImportEnabled()) {
|
||||
// Search if there is already an existing user, which means the username might have changed in LDAP without Keycloak knowing about it
|
||||
UserModel existingLocalUser = session.userLocalStorage()
|
||||
.searchForUserByUserAttributeStream(realm, LDAPConstants.LDAP_ID, ldapUser.getUuid()).findFirst().orElse(null);
|
||||
if(existingLocalUser != null){
|
||||
imported = existingLocalUser;
|
||||
// Need to evict the existing user from cache
|
||||
session.userCache().evict(realm, existingLocalUser);
|
||||
} else {
|
||||
imported = session.userLocalStorage().addUser(realm, ldapUsername);
|
||||
}
|
||||
|
||||
} else {
|
||||
InMemoryUserAdapter adapter = new InMemoryUserAdapter(session, realm, new StorageId(model.getId(), ldapUsername).getId());
|
||||
adapter.addDefaults();
|
||||
|
|
@ -803,5 +816,19 @@ public class LDAPStorageProvider implements UserStorageProvider,
|
|||
}
|
||||
}
|
||||
|
||||
public LDAPObject loadLDAPUserByUuid(RealmModel realm, String uuid) {
|
||||
if(uuid == null){
|
||||
return null;
|
||||
}
|
||||
try (LDAPQuery ldapQuery = LDAPUtils.createQueryForUserSearch(this, realm)) {
|
||||
LDAPQueryConditionsBuilder conditionsBuilder = new LDAPQueryConditionsBuilder();
|
||||
|
||||
String uuidLDAPAttributeName = this.ldapIdentityStore.getConfig().getUuidLDAPAttributeName();
|
||||
Condition usernameCondition = conditionsBuilder.equal(uuidLDAPAttributeName, uuid, EscapeStrategy.DEFAULT);
|
||||
ldapQuery.addWhereCondition(usernameCondition);
|
||||
|
||||
return ldapQuery.getFirstResult();
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
|||
|
|
@ -64,6 +64,7 @@ import org.keycloak.utils.CredentialHelper;
|
|||
import java.util.Date;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.Optional;
|
||||
import java.util.function.Function;
|
||||
import java.util.stream.Collectors;
|
||||
|
||||
|
|
@ -593,16 +594,18 @@ public class LDAPStorageProviderFactory implements UserStorageProviderFactory<LD
|
|||
String username = LDAPUtils.getUsername(ldapUser, ldapFedProvider.getLdapIdentityStore().getConfig());
|
||||
exists.value = true;
|
||||
LDAPUtils.checkUuid(ldapUser, ldapFedProvider.getLdapIdentityStore().getConfig());
|
||||
UserModel currentUser = session.userLocalStorage().getUserByUsername(currentRealm, username);
|
||||
|
||||
if (currentUser == null) {
|
||||
|
||||
UserModel currentUserLocal = session.userLocalStorage().getUserByUsername(currentRealm, username);
|
||||
Optional<UserModel> userModelOptional = session.userLocalStorage()
|
||||
.searchForUserByUserAttributeStream(currentRealm, LDAPConstants.LDAP_ID, ldapUser.getUuid())
|
||||
.findFirst();
|
||||
if (!userModelOptional.isPresent() && currentUserLocal == null) {
|
||||
// Add new user to Keycloak
|
||||
exists.value = false;
|
||||
ldapFedProvider.importUserFromLDAP(session, currentRealm, ldapUser);
|
||||
syncResult.increaseAdded();
|
||||
|
||||
} else {
|
||||
UserModel currentUser = userModelOptional.isPresent() ? userModelOptional.get() : currentUserLocal;
|
||||
if ((fedModel.getId().equals(currentUser.getFederationLink())) && (ldapUser.getUuid().equals(currentUser.getFirstAttribute(LDAPConstants.LDAP_ID)))) {
|
||||
|
||||
// Update keycloak user
|
||||
|
|
@ -621,7 +624,7 @@ public class LDAPStorageProviderFactory implements UserStorageProviderFactory<LD
|
|||
logger.debugf("Updated user from LDAP: %s", currentUser.getUsername());
|
||||
syncResult.increaseUpdated();
|
||||
} else {
|
||||
logger.warnf("User '%s' is not updated during sync as he already exists in Keycloak database but is not linked to federation provider '%s'", username, fedModel.getName());
|
||||
logger.warnf("User with ID '%s' is not updated during sync as he already exists in Keycloak database but is not linked to federation provider '%s'", ldapUser.getUuid(), fedModel.getName());
|
||||
syncResult.increaseFailed();
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -34,6 +34,7 @@ import org.keycloak.models.RealmModel;
|
|||
import org.keycloak.models.RoleModel;
|
||||
import org.keycloak.models.UserCredentialModel;
|
||||
import org.keycloak.models.UserModel;
|
||||
import org.keycloak.models.KeycloakSessionFactory;
|
||||
import org.keycloak.models.cache.CachedUserModel;
|
||||
import org.keycloak.models.credential.PasswordCredentialModel;
|
||||
import org.keycloak.models.utils.KeycloakModelUtils;
|
||||
|
|
@ -43,6 +44,7 @@ import org.keycloak.representations.idm.CredentialRepresentation;
|
|||
import org.keycloak.representations.idm.RealmRepresentation;
|
||||
import org.keycloak.representations.idm.UserRepresentation;
|
||||
import org.keycloak.services.managers.RealmManager;
|
||||
import org.keycloak.services.managers.UserStorageSyncManager;
|
||||
import org.keycloak.storage.ReadOnlyException;
|
||||
import org.keycloak.storage.StorageId;
|
||||
import org.keycloak.storage.UserStorageProvider;
|
||||
|
|
@ -58,6 +60,7 @@ import org.keycloak.storage.ldap.mappers.HardcodedLDAPRoleStorageMapper;
|
|||
import org.keycloak.storage.ldap.mappers.HardcodedLDAPRoleStorageMapperFactory;
|
||||
import org.keycloak.storage.ldap.mappers.LDAPStorageMapper;
|
||||
import org.keycloak.storage.ldap.mappers.UserAttributeLDAPStorageMapper;
|
||||
import org.keycloak.storage.user.SynchronizationResult;
|
||||
import org.keycloak.testsuite.AbstractAuthTest;
|
||||
import org.keycloak.testsuite.admin.ApiUtil;
|
||||
import org.keycloak.testsuite.arquillian.annotation.DisableFeature;
|
||||
|
|
@ -1176,4 +1179,55 @@ public class LDAPProvidersIntegrationTest extends AbstractLDAPTest {
|
|||
realmRepresentation.setEditUsernameAllowed(allowed);
|
||||
testRealm().update(realmRepresentation);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void updateLDAPUsernameTest() {
|
||||
testingClient.server().run(session -> {
|
||||
LDAPTestContext ctx = LDAPTestContext.init(session);
|
||||
// Add user to LDAP
|
||||
LDAPObject becky = LDAPTestUtils.addLDAPUser(ctx.getLdapProvider(), ctx.getRealm(), "beckybecks", "Becky", "Becks", "becky-becks@email.org", null, "123");
|
||||
LDAPTestUtils.updateLDAPPassword(ctx.getLdapProvider(), becky, "Password1");
|
||||
});
|
||||
|
||||
loginSuccessAndLogout("beckybecks", "Password1");
|
||||
|
||||
String origKeycloakUserId = testingClient.server().fetchString(session -> {
|
||||
LDAPTestContext ctx = LDAPTestContext.init(session);
|
||||
RealmModel testRealm = ctx.getRealm();
|
||||
|
||||
UserModel importedUser = session.userLocalStorage().getUserByUsername(testRealm, "beckybecks");
|
||||
|
||||
// Update user 'beckybecks' in LDAP
|
||||
LDAPObject becky = ctx.getLdapProvider().loadLDAPUserByUsername(testRealm, importedUser.getUsername());
|
||||
// NOTE: Changing LDAP Username directly here
|
||||
String userNameLdapAttributeName = ctx.getLdapProvider().getLdapIdentityStore().getConfig().getUsernameLdapAttribute();
|
||||
becky.setSingleAttribute(userNameLdapAttributeName, "beckyupdated");
|
||||
becky.setSingleAttribute(LDAPConstants.EMAIL, "becky-updated@email.org");
|
||||
ctx.getLdapProvider().getLdapIdentityStore().update(becky);
|
||||
LDAPTestUtils.updateLDAPPassword(ctx.getLdapProvider(), becky, "MyChangedPassword11");
|
||||
return importedUser.getId();
|
||||
});
|
||||
|
||||
loginSuccessAndLogout("beckyupdated", "MyChangedPassword11");
|
||||
|
||||
loginPage.open();
|
||||
loginPage.login("beckybecks", "Password1");
|
||||
Assert.assertEquals("Invalid username or password.", loginPage.getInputError());
|
||||
|
||||
testingClient.server().run(session -> {
|
||||
LDAPTestContext ctx = LDAPTestContext.init(session);
|
||||
|
||||
// The original username is not possible to use as username was changed in LDAP.
|
||||
// However the call to LDAPStorageProvider.loadAndValidateUser shouldn't delete the user just because his username changed in LDAP
|
||||
UserModel user = session.users().getUserByUsername(ctx.getRealm(), "beckybecks");
|
||||
Assert.assertNull(user);
|
||||
|
||||
// Assert user can be found with new username from LDAP. And it is same user as before
|
||||
user = session.users().getUserByUsername(ctx.getRealm(), "beckyupdated");
|
||||
Assert.assertNotNull(user);
|
||||
String newKeycloakUserId = user.getId();
|
||||
// Need to remove double quotes from server response
|
||||
Assert.assertEquals(origKeycloakUserId.replace("\"",""), newKeycloakUserId);
|
||||
});
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -240,10 +240,60 @@ public class LDAPSyncTest extends AbstractLDAPTest {
|
|||
});
|
||||
}
|
||||
|
||||
@Test
|
||||
public void test03LDAPSyncWhenUsernameChanged() {
|
||||
|
||||
testingClient.server().run(session -> {
|
||||
LDAPTestContext ctx = LDAPTestContext.init(session);
|
||||
KeycloakSessionFactory sessionFactory = session.getKeycloakSessionFactory();
|
||||
|
||||
// Add user to LDAP
|
||||
LDAPTestUtils.addLDAPUser(ctx.getLdapProvider(), ctx.getRealm(), "beckybecks", "Becky", "Becks", "becky-becks@email.org", null, "123");
|
||||
SynchronizationResult syncResult = new UserStorageSyncManager().syncAllUsers(sessionFactory, "test", ctx.getLdapModel());
|
||||
Assert.assertEquals(0, syncResult.getFailed());
|
||||
});
|
||||
|
||||
testingClient.server().run(session -> {
|
||||
LDAPTestContext ctx = LDAPTestContext.init(session);
|
||||
RealmModel testRealm = ctx.getRealm();
|
||||
UserStorageSyncManager usersSyncManager = new UserStorageSyncManager();
|
||||
|
||||
// Update user 'beckybecks' in LDAP
|
||||
LDAPObject ldapUser = ctx.getLdapProvider().loadLDAPUserByUsername(testRealm, "beckybecks");
|
||||
// NOTE: Changing LDAP Username directly here
|
||||
String userNameLdapAttributeName = ctx.getLdapProvider().getLdapIdentityStore().getConfig().getUsernameLdapAttribute();
|
||||
ldapUser.setSingleAttribute(userNameLdapAttributeName, "beckyupdated");
|
||||
ldapUser.setSingleAttribute(LDAPConstants.EMAIL, "becky-updated@email.org");
|
||||
ctx.getLdapProvider().getLdapIdentityStore().update(ldapUser);
|
||||
|
||||
// Assert still old users in local provider
|
||||
LDAPTestAsserts.assertUserImported(session.userLocalStorage(), testRealm, "beckybecks", "Becky", "Becks", "becky-becks@email.org", "123");
|
||||
|
||||
// Trigger partial sync
|
||||
KeycloakSessionFactory sessionFactory = session.getKeycloakSessionFactory();
|
||||
SynchronizationResult syncResult = usersSyncManager.syncChangedUsers(sessionFactory, "test", ctx.getLdapModel());
|
||||
Assert.assertEquals(0, syncResult.getFailed());
|
||||
});
|
||||
|
||||
testingClient.server().run(session -> {
|
||||
LDAPTestContext ctx = LDAPTestContext.init(session);
|
||||
RealmModel testRealm = session.realms().getRealm("test");
|
||||
UserProvider userProvider = session.userLocalStorage();
|
||||
// Assert users updated in local provider
|
||||
LDAPTestAsserts.assertUserImported(session.users(), testRealm, "beckyupdated", "Becky", "Becks", "becky-updated@email.org", "123");
|
||||
UserModel updatedLocalUser = userProvider.getUserByUsername(testRealm, "beckyupdated");
|
||||
LDAPObject ldapUser = ctx.getLdapProvider().loadLDAPUserByUsername(testRealm, "beckyupdated");
|
||||
// Assert old user 'beckybecks' does not exists locally
|
||||
Assert.assertNull(userProvider.getUserByUsername(testRealm, "beckybecks"));
|
||||
// Assert UUID didn't change
|
||||
Assert.assertEquals(updatedLocalUser.getAttributeStream(LDAPConstants.LDAP_ID).findFirst().get(),ldapUser.getUuid());
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
// KEYCLOAK-1571
|
||||
@Test
|
||||
public void test03SameUUIDAndUsernameSync() {
|
||||
public void test04SameUUIDAndUsernameSync() {
|
||||
String origUuidAttrName = testingClient.server().fetch(session -> {
|
||||
LDAPTestContext ctx = LDAPTestContext.init(session);
|
||||
|
||||
|
|
@ -290,10 +340,9 @@ public class LDAPSyncTest extends AbstractLDAPTest {
|
|||
testRealm().components().component(ldapModelId).update(ldapRep);
|
||||
}
|
||||
|
||||
|
||||
// KEYCLOAK-1728
|
||||
@Test
|
||||
public void test04MissingLDAPUsernameSync() {
|
||||
public void test05MissingLDAPUsernameSync() {
|
||||
String origUsernameAttrName = testingClient.server().fetch(session -> {
|
||||
LDAPTestContext ctx = LDAPTestContext.init(session);
|
||||
|
||||
|
|
@ -355,7 +404,7 @@ public class LDAPSyncTest extends AbstractLDAPTest {
|
|||
|
||||
// KEYCLOAK-10770 user-storage/{id}/sync should return 400 instead of 404
|
||||
@Test
|
||||
public void test05SyncRestAPIMissingAction() {
|
||||
public void test06SyncRestAPIMissingAction() {
|
||||
ComponentRepresentation ldapRep = testRealm().components().component(ldapModelId).toRepresentation();
|
||||
|
||||
try {
|
||||
|
|
@ -368,7 +417,7 @@ public class LDAPSyncTest extends AbstractLDAPTest {
|
|||
|
||||
// KEYCLOAK-10770 user-storage/{id}/sync should return 400 instead of 404
|
||||
@Test
|
||||
public void test06SyncRestAPIWrongAction() {
|
||||
public void test07SyncRestAPIWrongAction() {
|
||||
ComponentRepresentation ldapRep = testRealm().components().component(ldapModelId).toRepresentation();
|
||||
|
||||
try {
|
||||
|
|
@ -380,7 +429,7 @@ public class LDAPSyncTest extends AbstractLDAPTest {
|
|||
}
|
||||
|
||||
@Test
|
||||
public void test07LDAPGroupSyncAfterGroupRename() {
|
||||
public void test08LDAPGroupSyncAfterGroupRename() {
|
||||
testingClient.server().run(session -> {
|
||||
LDAPTestContext ctx = LDAPTestContext.init(session);
|
||||
RealmModel appRealm = ctx.getRealm();
|
||||
|
|
|
|||
|
|
@ -334,4 +334,10 @@ public class LDAPProvidersIntegrationNoImportTest extends LDAPProvidersIntegrati
|
|||
}
|
||||
}
|
||||
|
||||
// No need to test this in no-import mode. There won't be any users in localStorage.
|
||||
@Test
|
||||
@Ignore
|
||||
@Override
|
||||
public void updateLDAPUsernameTest() {
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue