KEYCLOAK-1490 Possibility to always read user attribute values from LDAP

federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProvider.java

@@ -232,7 +232,7 @@ public class LDAPFederationProvider implements UserFederationProvider {
         if (ldapUser.getUuid().equals(local.getAttribute(LDAPConstants.LDAP_ID))) {
             return ldapUser;
         } else {
-            logger.warnf("LDAP User invalid. ID doesn't match. ID from LDAP [%s], ID from local DB: [%s]", ldapUser.getUuid(), local.getAttribute(LDAPConstants.LDAP_ID));
+            logger.warnf("LDAP User invalid. ID doesn't match. ID from LDAP [%s], LDAP ID from local DB: [%s]", ldapUser.getUuid(), local.getAttribute(LDAPConstants.LDAP_ID));
             return null;
         }
     }

federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProviderFactory.java

@@ -84,14 +84,17 @@ public class LDAPFederationProviderFactory extends UserFederationEventAwareProvi
 
         boolean activeDirectory = ldapConfig.isActiveDirectory();
         UserFederationProvider.EditMode editMode = ldapConfig.getEditMode();
-        String readOnly = String.valueOf(editMode==UserFederationProvider.EditMode.READ_ONLY || editMode== UserFederationProvider.EditMode.UNSYNCED);
+        String readOnly = String.valueOf(editMode == UserFederationProvider.EditMode.READ_ONLY || editMode == UserFederationProvider.EditMode.UNSYNCED);
         String usernameLdapAttribute = ldapConfig.getUsernameLdapAttribute();
 
+        String alwaysReadValueFromLDAP = String.valueOf(editMode==UserFederationProvider.EditMode.READ_ONLY || editMode== UserFederationProvider.EditMode.WRITABLE);
+
         UserFederationMapperModel mapperModel;
         mapperModel = KeycloakModelUtils.createUserFederationMapperModel("username", newProviderModel.getId(), UserAttributeLDAPFederationMapperFactory.PROVIDER_ID,
                 UserAttributeLDAPFederationMapper.USER_MODEL_ATTRIBUTE, UserModel.USERNAME,
                 UserAttributeLDAPFederationMapper.LDAP_ATTRIBUTE, usernameLdapAttribute,
-                UserAttributeLDAPFederationMapper.READ_ONLY, readOnly);
+                UserAttributeLDAPFederationMapper.READ_ONLY, readOnly,
+                UserAttributeLDAPFederationMapper.ALWAYS_READ_VALUE_FROM_LDAP, "false");
         realm.addUserFederationMapper(mapperModel);
 
         // CN is typically used as RDN for Active Directory deployments
@@ -103,7 +106,8 @@ public class LDAPFederationProviderFactory extends UserFederationEventAwareProvi
                 mapperModel = KeycloakModelUtils.createUserFederationMapperModel("first name", newProviderModel.getId(), UserAttributeLDAPFederationMapperFactory.PROVIDER_ID,
                         UserAttributeLDAPFederationMapper.USER_MODEL_ATTRIBUTE, UserModel.FIRST_NAME,
                         UserAttributeLDAPFederationMapper.LDAP_ATTRIBUTE, LDAPConstants.GIVENNAME,
-                        UserAttributeLDAPFederationMapper.READ_ONLY, readOnly);
+                        UserAttributeLDAPFederationMapper.READ_ONLY, readOnly,
+                        UserAttributeLDAPFederationMapper.ALWAYS_READ_VALUE_FROM_LDAP, alwaysReadValueFromLDAP);
                 realm.addUserFederationMapper(mapperModel);
 
             } else {
@@ -113,13 +117,15 @@ public class LDAPFederationProviderFactory extends UserFederationEventAwareProvi
                     mapperModel = KeycloakModelUtils.createUserFederationMapperModel("first name", newProviderModel.getId(), UserAttributeLDAPFederationMapperFactory.PROVIDER_ID,
                             UserAttributeLDAPFederationMapper.USER_MODEL_ATTRIBUTE, UserModel.FIRST_NAME,
                             UserAttributeLDAPFederationMapper.LDAP_ATTRIBUTE, LDAPConstants.GIVENNAME,
-                            UserAttributeLDAPFederationMapper.READ_ONLY, readOnly);
+                            UserAttributeLDAPFederationMapper.READ_ONLY, readOnly,
+                            UserAttributeLDAPFederationMapper.ALWAYS_READ_VALUE_FROM_LDAP, alwaysReadValueFromLDAP);
                     realm.addUserFederationMapper(mapperModel);
 
                     mapperModel = KeycloakModelUtils.createUserFederationMapperModel("username-cn", newProviderModel.getId(), UserAttributeLDAPFederationMapperFactory.PROVIDER_ID,
                             UserAttributeLDAPFederationMapper.USER_MODEL_ATTRIBUTE, UserModel.USERNAME,
                             UserAttributeLDAPFederationMapper.LDAP_ATTRIBUTE, LDAPConstants.CN,
-                            UserAttributeLDAPFederationMapper.READ_ONLY, readOnly);
+                            UserAttributeLDAPFederationMapper.READ_ONLY, readOnly,
+                            UserAttributeLDAPFederationMapper.ALWAYS_READ_VALUE_FROM_LDAP, "false");
                     realm.addUserFederationMapper(mapperModel);
                 } else {
 
@@ -134,20 +140,23 @@ public class LDAPFederationProviderFactory extends UserFederationEventAwareProvi
             mapperModel = KeycloakModelUtils.createUserFederationMapperModel("first name", newProviderModel.getId(), UserAttributeLDAPFederationMapperFactory.PROVIDER_ID,
                     UserAttributeLDAPFederationMapper.USER_MODEL_ATTRIBUTE, UserModel.FIRST_NAME,
                     UserAttributeLDAPFederationMapper.LDAP_ATTRIBUTE, LDAPConstants.CN,
-                    UserAttributeLDAPFederationMapper.READ_ONLY, readOnly);
+                    UserAttributeLDAPFederationMapper.READ_ONLY, readOnly,
+                    UserAttributeLDAPFederationMapper.ALWAYS_READ_VALUE_FROM_LDAP, alwaysReadValueFromLDAP);
             realm.addUserFederationMapper(mapperModel);
         }
 
         mapperModel = KeycloakModelUtils.createUserFederationMapperModel("last name", newProviderModel.getId(), UserAttributeLDAPFederationMapperFactory.PROVIDER_ID,
                 UserAttributeLDAPFederationMapper.USER_MODEL_ATTRIBUTE, UserModel.LAST_NAME,
                 UserAttributeLDAPFederationMapper.LDAP_ATTRIBUTE, LDAPConstants.SN,
-                UserAttributeLDAPFederationMapper.READ_ONLY, readOnly);
+                UserAttributeLDAPFederationMapper.READ_ONLY, readOnly,
+                UserAttributeLDAPFederationMapper.ALWAYS_READ_VALUE_FROM_LDAP, alwaysReadValueFromLDAP);
         realm.addUserFederationMapper(mapperModel);
 
         mapperModel = KeycloakModelUtils.createUserFederationMapperModel("email", newProviderModel.getId(), UserAttributeLDAPFederationMapperFactory.PROVIDER_ID,
                 UserAttributeLDAPFederationMapper.USER_MODEL_ATTRIBUTE, UserModel.EMAIL,
                 UserAttributeLDAPFederationMapper.LDAP_ATTRIBUTE, LDAPConstants.EMAIL,
-                UserAttributeLDAPFederationMapper.READ_ONLY, readOnly);
+                UserAttributeLDAPFederationMapper.READ_ONLY, readOnly,
+                UserAttributeLDAPFederationMapper.ALWAYS_READ_VALUE_FROM_LDAP, alwaysReadValueFromLDAP);
         realm.addUserFederationMapper(mapperModel);
 
         String createTimestampLdapAttrName = activeDirectory ? "whenCreated" : LDAPConstants.CREATE_TIMESTAMP;
@@ -157,14 +166,16 @@ public class LDAPFederationProviderFactory extends UserFederationEventAwareProvi
         mapperModel = KeycloakModelUtils.createUserFederationMapperModel("creation date", newProviderModel.getId(), UserAttributeLDAPFederationMapperFactory.PROVIDER_ID,
                 UserAttributeLDAPFederationMapper.USER_MODEL_ATTRIBUTE, LDAPConstants.CREATE_TIMESTAMP,
                 UserAttributeLDAPFederationMapper.LDAP_ATTRIBUTE, createTimestampLdapAttrName,
-                UserAttributeLDAPFederationMapper.READ_ONLY, "true");
+                UserAttributeLDAPFederationMapper.READ_ONLY, "true",
+                UserAttributeLDAPFederationMapper.ALWAYS_READ_VALUE_FROM_LDAP, alwaysReadValueFromLDAP);
         realm.addUserFederationMapper(mapperModel);
 
         // map modifyTimeStamp as read-only
         mapperModel = KeycloakModelUtils.createUserFederationMapperModel("modify date", newProviderModel.getId(), UserAttributeLDAPFederationMapperFactory.PROVIDER_ID,
                 UserAttributeLDAPFederationMapper.USER_MODEL_ATTRIBUTE, LDAPConstants.MODIFY_TIMESTAMP,
                 UserAttributeLDAPFederationMapper.LDAP_ATTRIBUTE, modifyTimestampLdapAttrName,
-                UserAttributeLDAPFederationMapper.READ_ONLY, "true");
+                UserAttributeLDAPFederationMapper.READ_ONLY, "true",
+                UserAttributeLDAPFederationMapper.ALWAYS_READ_VALUE_FROM_LDAP, alwaysReadValueFromLDAP);
         realm.addUserFederationMapper(mapperModel);
     }
 

federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/UserAttributeLDAPFederationMapper.java

@@ -1,6 +1,7 @@
 package org.keycloak.federation.ldap.mappers;
 
 import java.lang.reflect.Method;
+import java.util.HashMap;
 import java.util.Map;
 
 import org.keycloak.federation.ldap.LDAPFederationProvider;
@@ -12,6 +13,7 @@ import org.keycloak.models.RealmModel;
 import org.keycloak.models.UserFederationMapperModel;
 import org.keycloak.models.UserFederationProvider;
 import org.keycloak.models.UserModel;
+import org.keycloak.models.utils.UserModelDelegate;
 import org.keycloak.models.utils.reflection.Property;
 import org.keycloak.models.utils.reflection.PropertyCriteria;
 import org.keycloak.models.utils.reflection.PropertyQueries;
@@ -41,6 +43,7 @@ public class UserAttributeLDAPFederationMapper extends AbstractLDAPFederationMap
     public static final String USER_MODEL_ATTRIBUTE = "user.model.attribute";
     public static final String LDAP_ATTRIBUTE = "ldap.attribute";
     public static final String READ_ONLY = "read.only";
+    public static final String ALWAYS_READ_VALUE_FROM_LDAP = "always.read.value.from.ldap";
 
 
     @Override
@@ -85,13 +88,15 @@ public class UserAttributeLDAPFederationMapper extends AbstractLDAPFederationMap
     }
 
     @Override
-    public UserModel proxy(UserFederationMapperModel mapperModel, LDAPFederationProvider ldapProvider, LDAPObject ldapUser, UserModel delegate, RealmModel realm) {
+    public UserModel proxy(UserFederationMapperModel mapperModel, LDAPFederationProvider ldapProvider, final LDAPObject ldapUser, UserModel delegate, RealmModel realm) {
-        if (ldapProvider.getEditMode() == UserFederationProvider.EditMode.WRITABLE && !isReadOnly(mapperModel)) {
-
         final String userModelAttrName = mapperModel.getConfig().get(USER_MODEL_ATTRIBUTE);
         final String ldapAttrName = mapperModel.getConfig().get(LDAP_ATTRIBUTE);
+        boolean isAlwaysReadValueFromLDAP = parseBooleanParameter(mapperModel, ALWAYS_READ_VALUE_FROM_LDAP);
 
-            TxAwareLDAPUserModelDelegate txDelegate = new TxAwareLDAPUserModelDelegate(delegate, ldapProvider, ldapUser) {
+        // For writable mode, we want to propagate writing of attribute to LDAP as well
+        if (ldapProvider.getEditMode() == UserFederationProvider.EditMode.WRITABLE && !isReadOnly(mapperModel)) {
+
+            delegate = new TxAwareLDAPUserModelDelegate(delegate, ldapProvider, ldapUser) {
 
                 @Override
                 public void setAttribute(String name, String value) {
@@ -131,10 +136,67 @@ public class UserAttributeLDAPFederationMapper extends AbstractLDAPFederationMap
 
             };
 
-            return txDelegate;
-        } else {
-            return delegate;
         }
+
+        // We prefer to read attribute value from LDAP instead of from local Keycloak DB
+        if (isAlwaysReadValueFromLDAP) {
+
+            delegate = new UserModelDelegate(delegate) {
+
+                @Override
+                public String getAttribute(String name) {
+                    if (name.equalsIgnoreCase(userModelAttrName)) {
+                        // TODO: Support different types than strings as well...
+                        return ldapUser.getAttributeAsStringCaseInsensitive(ldapAttrName);
+                    } else {
+                        return super.getAttribute(name);
+                    }
+                }
+
+                @Override
+                public Map<String, String> getAttributes() {
+                    Map<String, String> attrs = new HashMap<>(super.getAttributes());
+
+                    // Ignore properties
+                    if (UserModel.EMAIL.equalsIgnoreCase(userModelAttrName) || UserModel.FIRST_NAME.equalsIgnoreCase(userModelAttrName) || UserModel.LAST_NAME.equalsIgnoreCase(userModelAttrName)) {
+                        return attrs;
+                    }
+
+                    attrs.put(userModelAttrName, ldapUser.getAttributeAsStringCaseInsensitive(ldapAttrName));
+                    return attrs;
+                }
+
+                @Override
+                public String getEmail() {
+                    if (UserModel.EMAIL.equalsIgnoreCase(userModelAttrName)) {
+                        return ldapUser.getAttributeAsStringCaseInsensitive(ldapAttrName);
+                    } else {
+                        return super.getEmail();
+                    }
+                }
+
+                @Override
+                public String getLastName() {
+                    if (UserModel.LAST_NAME.equalsIgnoreCase(userModelAttrName)) {
+                        return ldapUser.getAttributeAsStringCaseInsensitive(ldapAttrName);
+                    } else {
+                        return super.getLastName();
+                    }
+                }
+
+                @Override
+                public String getFirstName() {
+                    if (UserModel.FIRST_NAME.equalsIgnoreCase(userModelAttrName)) {
+                        return ldapUser.getAttributeAsStringCaseInsensitive(ldapAttrName);
+                    } else {
+                        return super.getFirstName();
+                    }
+                }
+
+            };
+        }
+
+        return delegate;
     }
 
     @Override

federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/UserAttributeLDAPFederationMapperFactory.java

@@ -30,6 +30,10 @@ public class UserAttributeLDAPFederationMapperFactory extends AbstractLDAPFedera
         ProviderConfigProperty readOnly = createConfigProperty(UserAttributeLDAPFederationMapper.READ_ONLY, "Read Only",
                 "Read-only attribute is imported from LDAP to Keycloak DB, but it's not saved back to LDAP when user is updated in Keycloak.", ProviderConfigProperty.BOOLEAN_TYPE, "false");
         configProperties.add(readOnly);
+
+        ProviderConfigProperty alwaysReadValueFromLDAP = createConfigProperty(UserAttributeLDAPFederationMapper.ALWAYS_READ_VALUE_FROM_LDAP, "Always read value from LDAP",
+                "If on, then during reading of the user will be value of attribute from LDAP always used instead of the value from Keycloak DB", ProviderConfigProperty.BOOLEAN_TYPE, "false");
+        configProperties.add(alwaysReadValueFromLDAP);
     }
 
     @Override

testsuite/integration/src/test/java/org/keycloak/testsuite/federation/FederationProvidersIntegrationTest.java

@@ -265,7 +265,6 @@ public class FederationProvidersIntegrationTest {
     @Test
     public void testCaseSensitiveAttributeName() {
         KeycloakSession session = keycloakRule.startSession();
-        UserFederationMapperModel zipCodeMapper = null;
 
         try {
             RealmModel appRealm = new RealmManager(session).getRealmByName("test");
@@ -273,11 +272,9 @@ public class FederationProvidersIntegrationTest {
             LDAPFederationProvider ldapFedProvider = FederationTestUtils.getLdapProvider(session, ldapModel);
             LDAPObject johnZip = FederationTestUtils.addLDAPUser(ldapFedProvider, appRealm, "johnzip", "John", "Zip", "johnzip@email.org", "12398");
 
-            // Remove default zipcode mapper
+            // Remove default zipcode mapper and add the mapper for "POstalCode" to test case sensitivity
             UserFederationMapperModel currentZipMapper = appRealm.getUserFederationMapperByName(ldapModel.getId(), "zipCodeMapper");
             appRealm.removeUserFederationMapper(currentZipMapper);
-
-            // Add zipcode mapper for "POstalCode"
             FederationTestUtils.addUserAttributeMapper(appRealm, ldapModel, "zipCodeMapper-cs", "postal_code", "POstalCode");
 
             // Fetch user from LDAP and check that postalCode is filled
@@ -290,6 +287,76 @@ public class FederationProvidersIntegrationTest {
         }
     }
 
+    @Test
+    public void testDirectLDAPUpdate() {
+        KeycloakSession session = keycloakRule.startSession();
+
+        try {
+            RealmModel appRealm = new RealmManager(session).getRealmByName("test");
+
+            LDAPFederationProvider ldapFedProvider = FederationTestUtils.getLdapProvider(session, ldapModel);
+            LDAPObject johnDirect = FederationTestUtils.addLDAPUser(ldapFedProvider, appRealm, "johndirect", "John", "Direct", "johndirect@email.org", "12399");
+
+            // Fetch user from LDAP and check that postalCode is filled
+            UserModel user = session.users().getUserByUsername("johndirect", appRealm);
+            String postalCode = user.getAttribute("postal_code");
+            Assert.assertEquals("12399", postalCode);
+
+            // Directly update user in LDAP
+            johnDirect.setAttribute(LDAPConstants.POSTAL_CODE, "12400");
+            johnDirect.setAttribute(LDAPConstants.SN, "DirectLDAPUpdated");
+            ldapFedProvider.getLdapIdentityStore().update(johnDirect);
+
+            // Verify that postalCode is still the same as we read it's value from Keycloak DB
+            user = session.users().getUserByUsername("johndirect", appRealm);
+            postalCode = user.getAttribute("postal_code");
+            Assert.assertEquals("12399", postalCode);
+
+            // Check user.getAttributes()
+            postalCode = user.getAttributes().get("postal_code");
+            Assert.assertEquals("12399", postalCode);
+
+            // LastName is new as lastName mapper will read the value from LDAP
+            String lastName = user.getLastName();
+            Assert.assertEquals("DirectLDAPUpdated", lastName);
+        } finally {
+            keycloakRule.stopSession(session, true);
+        }
+
+        session = keycloakRule.startSession();
+        try {
+            RealmModel appRealm = new RealmManager(session).getRealmByName("test");
+
+            // Update postalCode mapper to always read the value from LDAP
+            UserFederationMapperModel zipMapper = appRealm.getUserFederationMapperByName(ldapModel.getId(), "zipCodeMapper");
+            zipMapper.getConfig().put(UserAttributeLDAPFederationMapper.ALWAYS_READ_VALUE_FROM_LDAP, "true");
+            appRealm.updateUserFederationMapper(zipMapper);
+
+            // Update lastName mapper to read the value from Keycloak DB
+            UserFederationMapperModel lastNameMapper = appRealm.getUserFederationMapperByName(ldapModel.getId(), "last name");
+            lastNameMapper.getConfig().put(UserAttributeLDAPFederationMapper.ALWAYS_READ_VALUE_FROM_LDAP, "false");
+            appRealm.updateUserFederationMapper(lastNameMapper);
+
+            // Verify that postalCode is read from LDAP now
+            UserModel user = session.users().getUserByUsername("johndirect", appRealm);
+            String postalCode = user.getAttribute("postal_code");
+            Assert.assertEquals("12400", postalCode);
+
+            // Check user.getAttributes()
+            postalCode = user.getAttributes().get("postal_code");
+            Assert.assertEquals("12400", postalCode);
+
+            Assert.assertFalse(user.getAttributes().containsKey(UserModel.LAST_NAME));
+
+            // lastName is read from Keycloak DB now
+            String lastName = user.getLastName();
+            Assert.assertEquals("Direct", lastName);
+
+        } finally {
+            keycloakRule.stopSession(session, false);
+        }
+    }
+
     @Test
     public void testFullNameMapper() {
         KeycloakSession session = keycloakRule.startSession();

testsuite/integration/src/test/java/org/keycloak/testsuite/federation/FederationTestUtils.java

@@ -102,7 +102,8 @@ class FederationTestUtils {
         UserFederationMapperModel mapperModel = KeycloakModelUtils.createUserFederationMapperModel(mapperName, providerModel.getId(), UserAttributeLDAPFederationMapperFactory.PROVIDER_ID,
                 UserAttributeLDAPFederationMapper.USER_MODEL_ATTRIBUTE, userModelAttributeName,
                 UserAttributeLDAPFederationMapper.LDAP_ATTRIBUTE, ldapAttributeName,
-                UserAttributeLDAPFederationMapper.READ_ONLY, "false");
+                UserAttributeLDAPFederationMapper.READ_ONLY, "false",
+                UserAttributeLDAPFederationMapper.ALWAYS_READ_VALUE_FROM_LDAP, "false");
         realm.addUserFederationMapper(mapperModel);
     }
 

testsuite/integration/src/test/resources/ldap/users.ldif

@@ -41,5 +41,3 @@ sn: Wilson
 mail: bwilson@keycloak.org
 postalCode: 88441
 postalCode: 77332
-
-