From 7474a31d95922b39aaa43a4effd21895946db148 Mon Sep 17 00:00:00 2001 From: mposolda Date: Tue, 4 Aug 2015 13:17:06 +0200 Subject: [PATCH] KEYCLOAK-1562 better error reporting when missing UUID on ldap user record --- .../federation/ldap/LDAPFederationProvider.java | 12 ++++++++---- .../ldap/LDAPFederationProviderFactory.java | 2 ++ .../java/org/keycloak/federation/ldap/LDAPUtils.java | 8 ++++++++ .../admin/resources/partials/federated-ldap.html | 12 +++++++++--- 4 files changed, 27 insertions(+), 7 deletions(-) diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProvider.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProvider.java index 102393c8da..6fdb33c30c 100755 --- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProvider.java +++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProvider.java @@ -147,11 +147,12 @@ public class LDAPFederationProvider implements UserFederationProvider { if (editMode == EditMode.READ_ONLY || editMode == EditMode.UNSYNCED) throw new IllegalStateException("Registration is not supported by this ldap server"); if (!synchronizeRegistrations()) throw new IllegalStateException("Registration is not supported by this ldap server"); - LDAPObject ldapObject = LDAPUtils.addUserToLDAP(this, realm, user); - user.setSingleAttribute(LDAPConstants.LDAP_ID, ldapObject.getUuid()); - user.setSingleAttribute(LDAPConstants.LDAP_ENTRY_DN, ldapObject.getDn().toString()); + LDAPObject ldapUser = LDAPUtils.addUserToLDAP(this, realm, user); + LDAPUtils.checkUuid(ldapUser, ldapIdentityStore.getConfig()); + user.setSingleAttribute(LDAPConstants.LDAP_ID, ldapUser.getUuid()); + user.setSingleAttribute(LDAPConstants.LDAP_ENTRY_DN, ldapUser.getDn().toString()); - return proxy(realm, user, ldapObject); + return proxy(realm, user, ldapUser); } @Override @@ -232,6 +233,8 @@ public class LDAPFederationProvider implements UserFederationProvider { if (ldapUser == null) { return null; } + LDAPUtils.checkUuid(ldapUser, ldapIdentityStore.getConfig()); + if (ldapUser.getUuid().equals(local.getFirstAttribute(LDAPConstants.LDAP_ID))) { return ldapUser; } else { @@ -257,6 +260,7 @@ public class LDAPFederationProvider implements UserFederationProvider { protected UserModel importUserFromLDAP(KeycloakSession session, RealmModel realm, LDAPObject ldapUser) { String ldapUsername = LDAPUtils.getUsername(ldapUser, ldapIdentityStore.getConfig()); + LDAPUtils.checkUuid(ldapUser, ldapIdentityStore.getConfig()); UserModel imported = session.userStorage().addUser(realm, ldapUsername); imported.setEnabled(true); diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProviderFactory.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProviderFactory.java index 256af7ee4c..915e5bbf73 100755 --- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProviderFactory.java +++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProviderFactory.java @@ -291,6 +291,8 @@ public class LDAPFederationProviderFactory extends UserFederationEventAwareProvi RealmModel currentRealm = session.realms().getRealm(realmId); String username = LDAPUtils.getUsername(ldapUser, ldapFedProvider.getLdapIdentityStore().getConfig()); + exists.value = true; + LDAPUtils.checkUuid(ldapUser, ldapFedProvider.getLdapIdentityStore().getConfig()); UserModel currentUser = session.userStorage().getUserByUsername(username, currentRealm); if (currentUser == null) { diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPUtils.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPUtils.java index 4c4e65bb2a..91f0000c6a 100755 --- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPUtils.java +++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPUtils.java @@ -81,4 +81,12 @@ public class LDAPUtils { return ldapUsername; } + + public static void checkUuid(LDAPObject ldapUser, LDAPConfig config) { + if (ldapUser.getUuid() == null) { + throw new ModelException("User returned from LDAP has null uuid! Check configuration of your LDAP settings. UUID Attribute must be unique among your LDAP records and available on all the LDAP user records. " + + "If your LDAP server really doesn't support the notion of UUID, you can use any other attribute, which is supposed to be unique among LDAP users in tree. For example 'uid' or 'entryDN' . " + + "Mapped UUID LDAP attribute: " + config.getUuidLDAPAttributeName() + ", user DN: " + ldapUser.getDn()); + } + } } diff --git a/forms/common-themes/src/main/resources/theme/base/admin/resources/partials/federated-ldap.html b/forms/common-themes/src/main/resources/theme/base/admin/resources/partials/federated-ldap.html index eb75db9445..e405f7218b 100755 --- a/forms/common-themes/src/main/resources/theme/base/admin/resources/partials/federated-ldap.html +++ b/forms/common-themes/src/main/resources/theme/base/admin/resources/partials/federated-ldap.html @@ -70,21 +70,27 @@
- Name of LDAP attribute, which is mapped as Keycloak username. For many LDAP server vendors it's 'uid'. For Active directory it's usually 'sAMAccountName' or 'cn' + Name of LDAP attribute, which is mapped as Keycloak username. For many LDAP server vendors it can be 'uid'. For Active directory it can be 'sAMAccountName' or 'cn' . + The attribute should be filled for all LDAP user records you want to import from LDAP to Keycloak. +
- Name of LDAP attribute, which is used as RDN (top attribute) of typical user DN. Usually it's the same as Username LDAP attribute, however for Active directory it could be 'cn' when username attribute might be 'sAMAccountName' + Name of LDAP attribute, which is used as RDN (top attribute) of typical user DN. Usually it's the same as Username LDAP attribute, however it's not required. + For example for Active directory it's common to use 'cn' as RDN attribute when username attribute might be 'sAMAccountName' . +
- Name of LDAP attribute, which is used as unique object identifier (UUID) for objects in LDAP. For many LDAP server vendors it's 'entryUUID' however some are different. For example for Active directory it should be 'objectGUID' + Name of LDAP attribute, which is used as unique object identifier (UUID) for objects in LDAP. For many LDAP server vendors it's 'entryUUID' however some are different. For example for Active directory it should be 'objectGUID' . + If your LDAP server really doesn't support the notion of UUID, you can use any other attribute, which is supposed to be unique among LDAP users in tree. For example 'uid' or 'entryDN' . +