Skip to content

Commit

Permalink
Continue LDAP search if a duplicated user (ModelDuplicateException) i…
Browse files Browse the repository at this point in the history 
…s found

Closes keycloak#25778

Signed-off-by: rmartinc <rmartinc@redhat.com>
  • Loading branch information
@rmartinc @pedroigor
rmartinc authored and pedroigor committed Mar 13, 2024
1 parent 1f772d2 commit d679c13
Show file tree
Hide file tree
Showing 3 changed files with 126 additions and 39 deletions.
95 changes: 56 additions & 39 deletions federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
import org.keycloak.models.UserCredentialModel;
import org.keycloak.models.UserManager;
import org.keycloak.models.UserModel;
import org.keycloak.models.UserProvider;
import org.keycloak.models.cache.CachedUserModel;
import org.keycloak.models.credential.PasswordCredentialModel;
import org.keycloak.models.utils.ReadOnlyUserModelDelegate;
Expand Down Expand Up @@ -648,65 +649,81 @@ protected UserModel importUserFromLDAP(KeycloakSession session, RealmModel realm
return importUserFromLDAP(session, realm, ldapUser, true);
}

protected UserModel importUserFromLDAP(KeycloakSession session, RealmModel realm, LDAPObject ldapUser, boolean duplicates) {
String ldapUsername = LDAPUtils.getUsername(ldapUser, ldapIdentityStore.getConfig());
LDAPUtils.checkUuid(ldapUser, ldapIdentityStore.getConfig());

UserModel imported;
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 = UserStoragePrivateUtil.userLocalStorage(session)
.searchForUserByUserAttributeStream(realm, LDAPConstants.LDAP_ID, ldapUser.getUuid()).findFirst().orElse(null);
if(existingLocalUser != null){
imported = existingLocalUser;
// Need to evict the existing user from cache
if (UserStorageUtil.userCache(session) != null) {
UserStorageUtil.userCache(session).evict(realm, existingLocalUser);
}
if (!duplicates) {
// if duplicates are not wanted return null
return null;
}
} else {
imported = UserStoragePrivateUtil.userLocalStorage(session).addUser(realm, ldapUsername);
}

} else {
InMemoryUserAdapter adapter = new InMemoryUserAdapter(session, realm, new StorageId(model.getId(), ldapUsername).getId());
adapter.addDefaults();
imported = adapter;
}
imported.setEnabled(true);

UserModel finalImported = imported;
private void doImportUser(final RealmModel realm, final UserModel user, final LDAPObject ldapUser) {
user.setEnabled(true);
realm.getComponentsStream(model.getId(), LDAPStorageMapper.class.getName())
.sorted(ldapMappersComparator.sortDesc())
.forEachOrdered(mapperModel -> {
if (logger.isTraceEnabled()) {
logger.tracef("Using mapper %s during import user from LDAP", mapperModel);
}
LDAPStorageMapper ldapMapper = mapperManager.getMapper(mapperModel);
ldapMapper.onImportUserFromLDAP(ldapUser, finalImported, realm, true);
ldapMapper.onImportUserFromLDAP(ldapUser, user, realm, true);
});

String userDN = ldapUser.getDn().toString();
if (model.isImportEnabled()) imported.setFederationLink(model.getId());
imported.setSingleAttribute(LDAPConstants.LDAP_ID, ldapUser.getUuid());
imported.setSingleAttribute(LDAPConstants.LDAP_ENTRY_DN, userDN);
if (model.isImportEnabled()) user.setFederationLink(model.getId());
user.setSingleAttribute(LDAPConstants.LDAP_ID, ldapUser.getUuid());
user.setSingleAttribute(LDAPConstants.LDAP_ENTRY_DN, userDN);
if(getLdapIdentityStore().getConfig().isTrustEmail()){
imported.setEmailVerified(true);
user.setEmailVerified(true);
}
if (kerberosConfig.isAllowKerberosAuthentication() && kerberosConfig.getKerberosPrincipalAttribute() != null) {
String kerberosPrincipal = ldapUser.getAttributeAsString(kerberosConfig.getKerberosPrincipalAttribute());
if (kerberosPrincipal == null) {
logger.warnf("Kerberos principal attribute not found on LDAP user [%s]. Configured kerberos principal attribute name is [%s]", ldapUser.getDn(), kerberosConfig.getKerberosPrincipalAttribute());
} else {
KerberosPrincipal kerberosPrinc = new KerberosPrincipal(kerberosPrincipal);
imported.setSingleAttribute(KerberosConstants.KERBEROS_PRINCIPAL, kerberosPrinc.toString());
user.setSingleAttribute(KerberosConstants.KERBEROS_PRINCIPAL, kerberosPrinc.toString());
}
}
logger.debugf("Imported new user from LDAP to Keycloak DB. Username: [%s], Email: [%s], LDAP_ID: [%s], LDAP Entry DN: [%s]",
user.getUsername(), user.getEmail(), ldapUser.getUuid(), userDN);
}

protected UserModel importUserFromLDAP(KeycloakSession session, RealmModel realm, LDAPObject ldapUser, boolean forcedImport) {
String ldapUsername = LDAPUtils.getUsername(ldapUser, ldapIdentityStore.getConfig());
LDAPUtils.checkUuid(ldapUser, ldapIdentityStore.getConfig());

UserModel imported = null;
UserModel existingLocalUser = null;
final UserProvider userProvider = UserStoragePrivateUtil.userLocalStorage(session);
try {
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
existingLocalUser = userProvider.searchForUserByUserAttributeStream(realm, LDAPConstants.LDAP_ID, ldapUser.getUuid())
.findFirst().orElse(null);
if (existingLocalUser != null) {
imported = existingLocalUser;
// Need to evict the existing user from cache
if (UserStorageUtil.userCache(session) != null) {
UserStorageUtil.userCache(session).evict(realm, existingLocalUser);
}
if (!forcedImport) {
// if import is not forced return null as it was already imported
return null;
}
} else {
imported = userProvider.addUser(realm, ldapUsername);
}
} else {
InMemoryUserAdapter adapter = new InMemoryUserAdapter(session, realm, new StorageId(model.getId(), ldapUsername).getId());
adapter.addDefaults();
imported = adapter;
}
doImportUser(realm, imported, ldapUser);
} catch (ModelDuplicateException e) {
logger.warnf(e, "Duplicated user importing from LDAP. LDAP Entry DN: [%s], LDAP_ID: [%s]", ldapUser.getDn(), ldapUser.getUuid());
if (!forcedImport && existingLocalUser == null) {
// try to continue if import was not forced, delete created db user if necessary
if (model.isImportEnabled() && imported != null) {
userProvider.removeUser(realm, imported);
}
return null;
}
throw e;
}
logger.debugf("Imported new user from LDAP to Keycloak DB. Username: [%s], Email: [%s], LDAP_ID: [%s], LDAP Entry DN: [%s]", imported.getUsername(), imported.getEmail(),
ldapUser.getUuid(), userDN);

UserModel proxy = proxy(realm, imported, ldapUser, false);
return proxy;
}
Expand Down
35 changes: 35 additions & 0 deletions ...rc/test/java/org/keycloak/testsuite/federation/ldap/LDAPSearchForUsersPaginationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,13 @@
import org.junit.Test;
import org.junit.runners.MethodSorters;
import org.keycloak.models.LDAPConstants;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.RealmModel;
import org.keycloak.representations.idm.UserRepresentation;
import org.keycloak.testsuite.admin.ApiUtil;
import org.keycloak.testsuite.util.LDAPRule;
import org.keycloak.testsuite.util.LDAPTestUtils;
import org.keycloak.testsuite.util.UserBuilder;

@FixMethodOrder(MethodSorters.NAME_ASCENDING)
public class LDAPSearchForUsersPaginationTest extends AbstractLDAPTest {
Expand Down Expand Up @@ -180,6 +183,38 @@ public void testSearchLDAPLdapEntryDn() {
Assert.assertEquals(Set.of("john"), usernames);
}

public void testDuplicateEmailInDatabase() {
setLDAPEnabled(false);
try {
// create a local db user with the same email than an a ldap user
String userId = ApiUtil.getCreatedId(testRealm().users().create(UserBuilder.create()
.username("jdoe").firstName("John").lastName("Doe")
.email("john14@email.org")
.build()));
Assert.assertNotNull("User not created", userId);
getCleanup().addUserId(userId);
} finally {
setLDAPEnabled(true);
}

List<UserRepresentation> search = adminClient.realm(TEST_REALM_NAME).users()
.search("john14@email.org", null, null)
.stream().collect(Collectors.toList());
Assert.assertEquals("Incorrect users found", 1, search.size());
Assert.assertEquals("Incorrect User", "jdoe", search.get(0).getUsername());
Assert.assertTrue("Duplicated user created", adminClient.realm(TEST_REALM_NAME).users().search("john", true).isEmpty());
}

private void setLDAPEnabled(final boolean enabled) {
testingClient.server().run((KeycloakSession session) -> {
LDAPTestContext ctx = LDAPTestContext.init(session);
RealmModel appRealm = ctx.getRealm();

ctx.getLdapModel().getConfig().putSingle("enabled", Boolean.toString(enabled));
appRealm.updateComponent(ctx.getLdapModel());
});
}

private void assertLDAPSearchMatchesLocalDB(String searchString) {
//this call should import some users into local database
List<String> importedUsers = adminClient.realm(TEST_REALM_NAME).users().search(searchString, null, null).stream().map(UserRepresentation::getUsername).collect(Collectors.toList());
Expand Down
35 changes: 35 additions & 0 deletions ...keycloak/testsuite/federation/ldap/noimport/LDAPSearchForUsersPaginationNoImportTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,15 @@
import org.junit.Test;
import org.junit.runners.MethodSorters;
import org.keycloak.models.LDAPConstants;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.RealmModel;
import org.keycloak.representations.idm.UserRepresentation;
import org.keycloak.testsuite.admin.ApiUtil;
import org.keycloak.testsuite.federation.ldap.AbstractLDAPTest;
import org.keycloak.testsuite.federation.ldap.LDAPTestContext;
import org.keycloak.testsuite.util.LDAPRule;
import org.keycloak.testsuite.util.LDAPTestUtils;
import org.keycloak.testsuite.util.UserBuilder;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.hasItem;
import static org.hamcrest.Matchers.hasSize;
Expand Down Expand Up @@ -192,4 +195,36 @@ public void testSearchLDAPLdapEntryDn() {
.collect(Collectors.toSet());
Assert.assertEquals(Set.of("john"), usernames);
}

public void testDuplicateEmailInDatabase() {
setLDAPEnabled(false);
try {
// create a local db user with the same email than an a ldap user
String userId = ApiUtil.getCreatedId(testRealm().users().create(UserBuilder.create()
.username("jdoe").firstName("John").lastName("Doe")
.email("john14@email.org")
.build()));
Assert.assertNotNull("User not created", userId);
getCleanup().addUserId(userId);
} finally {
setLDAPEnabled(true);
}

List<UserRepresentation> search = adminClient.realm(TEST_REALM_NAME).users()
.search("john14@email.org", null, null)
.stream().collect(Collectors.toList());
Assert.assertEquals("User not found", 1, search.size());
Assert.assertEquals("Incorrect User", "jdoe", search.get(0).getUsername());
Assert.assertTrue("Duplicated user created", adminClient.realm(TEST_REALM_NAME).users().search("john", true).isEmpty());
}

private void setLDAPEnabled(final boolean enabled) {
testingClient.server().run((KeycloakSession session) -> {
LDAPTestContext ctx = LDAPTestContext.init(session);
RealmModel appRealm = ctx.getRealm();

ctx.getLdapModel().getConfig().putSingle("enabled", Boolean.toString(enabled));
appRealm.updateComponent(ctx.getLdapModel());
});
}
}

0 comments on commit d679c13

Please sign in to comment.