keycloak-memoizeit
Changes
connections/jpa-liquibase/src/main/java/org/keycloak/connections/jpa/updater/liquibase/custom/JpaUpdate1_2_0_Beta1.java 2(+1 -1)
federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/store/ldap/LDAPIdentityStore.java 14(+7 -7)
federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProviderFactory.java 17(+13 -4)
forms/common-themes/src/main/resources/theme/base/admin/resources/partials/federated-ldap.html 12(+9 -3)
Details
diff --git a/connections/jpa-liquibase/src/main/java/org/keycloak/connections/jpa/updater/liquibase/custom/JpaUpdate1_2_0_Beta1.java b/connections/jpa-liquibase/src/main/java/org/keycloak/connections/jpa/updater/liquibase/custom/JpaUpdate1_2_0_Beta1.java
index 89e7885..895a785 100644
--- a/connections/jpa-liquibase/src/main/java/org/keycloak/connections/jpa/updater/liquibase/custom/JpaUpdate1_2_0_Beta1.java
+++ b/connections/jpa-liquibase/src/main/java/org/keycloak/connections/jpa/updater/liquibase/custom/JpaUpdate1_2_0_Beta1.java
@@ -293,7 +293,7 @@ public class JpaUpdate1_2_0_Beta1 extends CustomKeycloakTask {
}
Object acmObj = resultSet.getObject("ALLOWED_CLAIMS_MASK");
- long mask = (acmObj != null) ? (Long) acmObj : ClaimMask.ALL;
+ long mask = (acmObj != null) ? ((Number) acmObj).longValue() : ClaimMask.ALL;
MigrationProvider migrationProvider = this.kcSession.getProvider(MigrationProvider.class);
List<ProtocolMapperRepresentation> protocolMappers = migrationProvider.getMappersForClaimMask(mask);
diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/model/LDAPObject.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/model/LDAPObject.java
index 81f058d..cbb28f9 100644
--- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/model/LDAPObject.java
+++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/model/LDAPObject.java
@@ -129,4 +129,9 @@ public class LDAPObject {
result = 31 * result + (getUuid() != null ? getUuid().hashCode() : 0);
return result;
}
+
+ @Override
+ public String toString() {
+ return "LDAP Object [ dn: " + dn + " , uuid: " + uuid + ", attributes: " + attributes + ", readOnly attribute names: " + readOnlyAttributeNames + " ]";
+ }
}
diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/store/ldap/LDAPIdentityStore.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/store/ldap/LDAPIdentityStore.java
index 43057d1..dc93742 100644
--- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/store/ldap/LDAPIdentityStore.java
+++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/store/ldap/LDAPIdentityStore.java
@@ -81,8 +81,8 @@ public class LDAPIdentityStore implements IdentityStore {
this.operationManager.createSubContext(entryDN, ldapAttributes);
ldapObject.setUuid(getEntryIdentifier(ldapObject));
- if (logger.isTraceEnabled()) {
- logger.tracef("Type with identifier [%s] and dn [%s] successfully added to LDAP store.", ldapObject.getUuid(), entryDN);
+ if (logger.isDebugEnabled()) {
+ logger.debugf("Type with identifier [%s] and dn [%s] successfully added to LDAP store.", ldapObject.getUuid(), entryDN);
}
}
@@ -94,8 +94,8 @@ public class LDAPIdentityStore implements IdentityStore {
String entryDn = ldapObject.getDn().toString();
this.operationManager.modifyAttributes(entryDn, attributes);
- if (logger.isTraceEnabled()) {
- logger.tracef("Type with identifier [%s] and DN [%s] successfully updated to LDAP store.", ldapObject.getUuid(), entryDn);
+ if (logger.isDebugEnabled()) {
+ logger.debugf("Type with identifier [%s] and DN [%s] successfully updated to LDAP store.", ldapObject.getUuid(), entryDn);
}
}
@@ -103,8 +103,8 @@ public class LDAPIdentityStore implements IdentityStore {
public void remove(LDAPObject ldapObject) {
this.operationManager.removeEntry(ldapObject.getDn().toString());
- if (logger.isTraceEnabled()) {
- logger.tracef("Type with identifier [%s] and DN [%s] successfully removed from LDAP store.", ldapObject.getUuid(), ldapObject.getDn().toString());
+ if (logger.isDebugEnabled()) {
+ logger.debugf("Type with identifier [%s] and DN [%s] successfully removed from LDAP store.", ldapObject.getUuid(), ldapObject.getDn().toString());
}
}
@@ -429,7 +429,7 @@ public class LDAPIdentityStore implements IdentityStore {
}
if (logger.isTraceEnabled()) {
- logger.tracef("Found ldap object [%s] and populated with the attributes [%s]. Read-only attributes are [%s]", ldapObject.getDn().toString(), ldapObject.getAttributes(), ldapObject.getReadOnlyAttributeNames());
+ logger.tracef("Found ldap object and populated with the attributes. LDAP Object: %s", ldapObject.toString());
}
return ldapObject;
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 4edbc83..6fdb33c 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,17 +260,16 @@ public class LDAPFederationProvider implements UserFederationProvider {
protected UserModel importUserFromLDAP(KeycloakSession session, RealmModel realm, LDAPObject ldapUser) {
String ldapUsername = LDAPUtils.getUsername(ldapUser, ldapIdentityStore.getConfig());
-
- if (ldapUsername == null) {
- throw new ModelException("User returned from LDAP has null username! Check configuration of your LDAP mappings. Mapped username LDAP attribute: " +
- ldapIdentityStore.getConfig().getUsernameLdapAttribute() + ", attributes from LDAP: " + ldapUser.getAttributes());
- }
+ LDAPUtils.checkUuid(ldapUser, ldapIdentityStore.getConfig());
UserModel imported = session.userStorage().addUser(realm, ldapUsername);
imported.setEnabled(true);
Set<UserFederationMapperModel> federationMappers = realm.getUserFederationMappersByFederationProvider(getModel().getId());
for (UserFederationMapperModel mapperModel : federationMappers) {
+ if (logger.isTraceEnabled()) {
+ logger.tracef("Using mapper %s during import user from LDAP", mapperModel);
+ }
LDAPFederationMapper ldapMapper = getMapper(mapperModel);
ldapMapper.onImportUserFromLDAP(mapperModel, this, ldapUser, imported, realm, 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 3198255..915e5bb 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) {
@@ -332,10 +334,17 @@ public class LDAPFederationProviderFactory extends UserFederationEventAwareProvi
public void run(KeycloakSession session) {
LDAPFederationProvider ldapFedProvider = getInstance(session, fedModel);
RealmModel currentRealm = session.realms().getRealm(realmId);
- String username = LDAPUtils.getUsername(ldapUser, ldapFedProvider.getLdapIdentityStore().getConfig());
- UserModel existing = session.userStorage().getUserByUsername(username, currentRealm);
- if (existing != null) {
- session.userStorage().removeUser(currentRealm, existing);
+ String username = null;
+ try {
+ username = LDAPUtils.getUsername(ldapUser, ldapFedProvider.getLdapIdentityStore().getConfig());
+ } catch (ModelException ignore) {
+ }
+
+ if (username != null) {
+ UserModel existing = session.userStorage().getUserByUsername(username, currentRealm);
+ if (existing != null) {
+ session.userStorage().removeUser(currentRealm, existing);
+ }
}
}
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 c731330..91f0000 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
@@ -72,6 +72,21 @@ public class LDAPUtils {
public static String getUsername(LDAPObject ldapUser, LDAPConfig config) {
String usernameAttr = config.getUsernameLdapAttribute();
- return ldapUser.getAttributeAsString(usernameAttr);
+ String ldapUsername = ldapUser.getAttributeAsString(usernameAttr);
+
+ if (ldapUsername == null) {
+ throw new ModelException("User returned from LDAP has null username! Check configuration of your LDAP mappings. Mapped username LDAP attribute: " +
+ config.getUsernameLdapAttribute() + ", user DN: " + ldapUser.getDn() + ", attributes from LDAP: " + ldapUser.getAttributes());
+ }
+
+ 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 eb75db9..e405f72 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 @@
<div class="col-md-6">
<input class="form-control" id="usernameLDAPAttribute" type="text" ng-model="instance.config.usernameLDAPAttribute" placeholder="LDAP attribute name for username" required>
</div>
- <kc-tooltip>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'</kc-tooltip>
+ <kc-tooltip>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.
+ </kc-tooltip>
</div>
<div class="form-group clearfix">
<label class="col-md-2 control-label" for="rdnLDAPAttribute"><span class="required">*</span> RDN LDAP attribute</label>
<div class="col-md-6">
<input class="form-control" id="rdnLDAPAttribute" type="text" ng-model="instance.config.rdnLDAPAttribute" placeholder="LDAP attribute name for user RDN" required>
</div>
- <kc-tooltip>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' </kc-tooltip>
+ <kc-tooltip>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' .
+ </kc-tooltip>
</div>
<div class="form-group clearfix">
<label class="col-md-2 control-label" for="uuidLDAPAttribute"><span class="required">*</span> UUID LDAP attribute</label>
<div class="col-md-6">
<input class="form-control" id="uuidLDAPAttribute" type="text" ng-model="instance.config.uuidLDAPAttribute" placeholder="LDAP attribute name for UUID" required>
</div>
- <kc-tooltip>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' </kc-tooltip>
+ <kc-tooltip>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' .
+ </kc-tooltip>
</div>
<div class="form-group clearfix">
<label class="col-md-2 control-label" for="userObjectClasses"><span class="required">*</span> User Object Classes</label>
diff --git a/model/api/src/main/java/org/keycloak/models/UserFederationMapperModel.java b/model/api/src/main/java/org/keycloak/models/UserFederationMapperModel.java
index 80f22be..ea08a58 100644
--- a/model/api/src/main/java/org/keycloak/models/UserFederationMapperModel.java
+++ b/model/api/src/main/java/org/keycloak/models/UserFederationMapperModel.java
@@ -76,4 +76,12 @@ public class UserFederationMapperModel implements Serializable {
public int hashCode() {
return id.hashCode();
}
+
+ @Override
+ public String toString() {
+ return new StringBuilder(" { name=" + name)
+ .append(", federationMapperType=" + federationMapperType)
+ .append(", config=" + config)
+ .append(" } ").toString();
+ }
}
diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/FederationTestUtils.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/FederationTestUtils.java
index 939fe24..63a27e6 100644
--- a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/FederationTestUtils.java
+++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/FederationTestUtils.java
@@ -102,14 +102,14 @@ class FederationTestUtils {
addUserAttributeMapper(realm, providerModel, "zipCodeMapper", "postal_code", LDAPConstants.POSTAL_CODE);
}
- public static void addUserAttributeMapper(RealmModel realm, UserFederationProviderModel providerModel, String mapperName, String userModelAttributeName, String ldapAttributeName) {
+ public static UserFederationMapperModel addUserAttributeMapper(RealmModel realm, UserFederationProviderModel providerModel, String mapperName, String userModelAttributeName, String ldapAttributeName) {
UserFederationMapperModel mapperModel = KeycloakModelUtils.createUserFederationMapperModel(mapperName, providerModel.getId(), UserAttributeLDAPFederationMapperFactory.PROVIDER_ID,
UserAttributeLDAPFederationMapper.USER_MODEL_ATTRIBUTE, userModelAttributeName,
UserAttributeLDAPFederationMapper.LDAP_ATTRIBUTE, ldapAttributeName,
UserAttributeLDAPFederationMapper.READ_ONLY, "false",
UserAttributeLDAPFederationMapper.ALWAYS_READ_VALUE_FROM_LDAP, "false",
UserAttributeLDAPFederationMapper.IS_MANDATORY_IN_LDAP, "false");
- realm.addUserFederationMapper(mapperModel);
+ return realm.addUserFederationMapper(mapperModel);
}
public static void addOrUpdateRoleLDAPMappers(RealmModel realm, UserFederationProviderModel providerModel, RoleLDAPFederationMapper.Mode mode) {
diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/SyncProvidersTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/SyncProvidersTest.java
index e5e893a..f946825 100755
--- a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/SyncProvidersTest.java
+++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/SyncProvidersTest.java
@@ -11,10 +11,12 @@ import org.keycloak.federation.ldap.LDAPConfig;
import org.keycloak.federation.ldap.LDAPFederationProvider;
import org.keycloak.federation.ldap.LDAPFederationProviderFactory;
import org.keycloak.federation.ldap.idm.model.LDAPObject;
+import org.keycloak.federation.ldap.mappers.UserAttributeLDAPFederationMapper;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.KeycloakSessionFactory;
import org.keycloak.models.LDAPConstants;
import org.keycloak.models.RealmModel;
+import org.keycloak.models.UserFederationMapperModel;
import org.keycloak.models.UserFederationProvider;
import org.keycloak.models.UserFederationProviderModel;
import org.keycloak.models.UserFederationSyncResult;
@@ -287,6 +289,68 @@ public class SyncProvidersTest {
}
}
+ // KEYCLOAK-1728
+ @Test
+ public void test04MissingLDAPUsernameSync() {
+ KeycloakSession session = keycloakRule.startSession();
+ String origUsernameAttrName;
+
+ try {
+ RealmModel testRealm = session.realms().getRealm("test");
+
+ // Remove all users from model
+ for (UserModel user : session.userStorage().getUsers(testRealm, true)) {
+ session.userStorage().removeUser(testRealm, user);
+ }
+
+ UserFederationProviderModel providerModel = KeycloakModelUtils.findUserFederationProviderByDisplayName(ldapModel.getDisplayName(), testRealm);
+
+ // Add street mapper and add some user including street
+ UserFederationMapperModel streetMapper = FederationTestUtils.addUserAttributeMapper(testRealm, ldapModel, "streetMapper", "street", LDAPConstants.STREET);
+ LDAPFederationProvider ldapFedProvider = FederationTestUtils.getLdapProvider(session, ldapModel);
+ LDAPObject streetUser = FederationTestUtils.addLDAPUser(ldapFedProvider, testRealm, "user8", "User8FN", "User8LN", "user8@email.org", "user8street", "126");
+
+ // Change name of username attribute name to street
+ origUsernameAttrName = providerModel.getConfig().get(LDAPConstants.USERNAME_LDAP_ATTRIBUTE);
+ providerModel.getConfig().put(LDAPConstants.USERNAME_LDAP_ATTRIBUTE, "street");
+
+ // Need to change this due to ApacheDS pagination bug (For other LDAP servers, pagination works fine) TODO: Remove once ApacheDS upgraded and pagination is fixed
+ providerModel.getConfig().put(LDAPConstants.BATCH_SIZE_FOR_SYNC, "10");
+ testRealm.updateUserFederationProvider(providerModel);
+
+ } finally {
+ keycloakRule.stopSession(session, true);
+ }
+
+ // Just user8 synced. All others failed to sync
+ session = keycloakRule.startSession();
+ try {
+ RealmModel testRealm = session.realms().getRealm("test");
+ UserFederationProviderModel providerModel = KeycloakModelUtils.findUserFederationProviderByDisplayName(ldapModel.getDisplayName(), testRealm);
+
+ KeycloakSessionFactory sessionFactory = session.getKeycloakSessionFactory();
+ UserFederationSyncResult syncResult = new UsersSyncManager().syncAllUsers(sessionFactory, "test", providerModel);
+ Assert.assertEquals(1, syncResult.getAdded());
+ Assert.assertTrue(syncResult.getFailed() > 0);
+ } finally {
+ keycloakRule.stopSession(session, false);
+ }
+
+ session = keycloakRule.startSession();
+ try {
+ RealmModel testRealm = session.realms().getRealm("test");
+
+ // Revert config changes
+ UserFederationProviderModel providerModel = KeycloakModelUtils.findUserFederationProviderByDisplayName(ldapModel.getDisplayName(), testRealm);
+ providerModel.getConfig().put(LDAPConstants.USERNAME_LDAP_ATTRIBUTE, origUsernameAttrName);
+ testRealm.updateUserFederationProvider(providerModel);
+ UserFederationMapperModel streetMapper = testRealm.getUserFederationMapperByName(providerModel.getId(), "streetMapper");
+ testRealm.removeUserFederationMapper(streetMapper);
+ } finally {
+ keycloakRule.stopSession(session, true);
+ }
+ }
+
@Test
public void testPeriodicSync() {
KeycloakSession session = keycloakRule.startSession();