Fix attribute deleted from LDAP is not immediately reflected even if it is "Always Read Value From LDAP"

federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/UserAttributeLDAPStorageMapper.java

@@ -391,6 +391,8 @@ public Map<String, List<String>> getAttributes() {
                     Set<String> allLdapAttrValues = ldapUser.getAttributeAsSet(ldapAttrName);
                     if (allLdapAttrValues != null) {
                         attrs.put(userModelAttrName, new ArrayList<>(allLdapAttrValues));
+                    } else {
+                        attrs.remove(userModelAttrName);
                     }
                     return attrs;
                 }

testsuite/model/src/test/java/org/keycloak/testsuite/model/user/UserSyncTest.java

@@ -23,9 +23,18 @@
 import org.keycloak.component.ComponentModel;
 import org.keycloak.models.Constants;
 import org.keycloak.models.KeycloakSession;
+import org.keycloak.models.LDAPConstants;
 import org.keycloak.models.RealmModel;
 import org.keycloak.models.RealmProvider;
+import org.keycloak.models.UserModel;
 import org.keycloak.models.UserProvider;
+import org.keycloak.models.utils.KeycloakModelUtils;
+import org.keycloak.storage.CacheableStorageProviderModel;
+import org.keycloak.storage.ldap.idm.model.LDAPObject;
+import org.keycloak.storage.ldap.idm.store.ldap.LDAPOperationManager;
+import org.keycloak.storage.ldap.mappers.LDAPStorageMapper;
+import org.keycloak.storage.ldap.mappers.UserAttributeLDAPStorageMapper;
+import org.keycloak.storage.ldap.mappers.UserAttributeLDAPStorageMapperFactory;
 import org.keycloak.storage.managers.UserStorageSyncManager;
 import org.keycloak.storage.UserStoragePrivateUtil;
 import org.keycloak.storage.UserStorageProvider;
@@ -39,10 +48,14 @@
 import org.keycloak.testsuite.model.RequireProvider;
 import org.keycloak.testsuite.util.LDAPTestUtils;
 
+import javax.naming.directory.BasicAttribute;
+import java.util.Collections;
 import java.util.stream.IntStream;
 
+import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.CoreMatchers.notNullValue;
+import static org.hamcrest.CoreMatchers.nullValue;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.Assume.assumeThat;
 
@@ -117,10 +130,89 @@ public void testManyUsersImport() {
         long timeNeeded = end - start;
 
         // The sync shouldn't take more than 18 second per user
-        assertThat(String.format("User sync took %f seconds per user, but it should take less than 18 seconds", 
+        assertThat(String.format("User sync took %f seconds per user, but it should take less than 18 seconds",
                 (float)(timeNeeded) / NUMBER_OF_USERS), timeNeeded, Matchers.lessThan((long) (18 * NUMBER_OF_USERS)));
         assertThat(res.getAdded(), is(NUMBER_OF_USERS));
         assertThat(withRealm(realmId, (session, realm) -> UserStoragePrivateUtil.userLocalStorage(session).getUsersCount(realm)), is(NUMBER_OF_USERS));
     }
+
+    @Test
+    public void testAlwaysReadValueFromLDAPWorksWithNoCachePolicy() {
+        // Create mapper from sn to a new user attribute
+        withRealm(realmId, (session, realm) -> {
+            UserStorageProviderModel providerModel = new UserStorageProviderModel(realm.getComponent(userFederationId));
+            // disable cache
+            providerModel.setCachePolicy(CacheableStorageProviderModel.CachePolicy.NO_CACHE);
+            realm.updateComponent(providerModel);
+
+            // Create mapper
+            ComponentModel mapperModel = KeycloakModelUtils.createComponentModel("My-street-mapper", providerModel.getId(), UserAttributeLDAPStorageMapperFactory.PROVIDER_ID, LDAPStorageMapper.class.getName(),
+                    UserAttributeLDAPStorageMapper.USER_MODEL_ATTRIBUTE, LDAPConstants.STREET,
+                    UserAttributeLDAPStorageMapper.LDAP_ATTRIBUTE, LDAPConstants.STREET,
+                    UserAttributeLDAPStorageMapper.READ_ONLY, "false",
+                    UserAttributeLDAPStorageMapper.ALWAYS_READ_VALUE_FROM_LDAP, "true",
+                    UserAttributeLDAPStorageMapper.IS_MANDATORY_IN_LDAP, "false");
+            realm.addComponentModel(mapperModel);
+
+            return null;
+        });
+
+        final String MY_STREET_NAME = "my-street 9";
+
+        // create 1 user in LDAP
+        withRealm(realmId, (session, realm) -> {
+            ComponentModel ldapModel = LDAPTestUtils.getLdapProviderModel(realm);
+            LDAPStorageProvider ldapFedProvider = LDAPTestUtils.getLdapProvider(session, ldapModel);
+            int i = 1;
+            LDAPTestUtils.addLDAPUser(ldapFedProvider, realm, "user" + i, "User" + i + "FN", "User" + i + "LN", "user" + i + "@email.org", MY_STREET_NAME, "12" + i);
+            return null;
+        });
+
+
+        // Read attribute that should be mapped by created mapper
+        String id = withRealm(realmId, (session, realm) -> {
+            UserModel user1 = session.users().getUserByUsername(realm, "user1");
+            assertThat(user1.getAttributes().get(LDAPConstants.STREET).get(0), is(equalTo(MY_STREET_NAME)));
+            return user1.getId();
+        });
+
+        // Remove attribute from the LDAP for given user
+        withRealm(realmId, (session, realm) -> {
+            UserStorageProviderModel providerModel = new UserStorageProviderModel(realm.getComponent(userFederationId));
+            LDAPStorageProvider ldapFedProvider = LDAPTestUtils.getLdapProvider(session, providerModel);
+
+            LDAPObject user1LdapObject = ldapFedProvider.loadLDAPUserByUsername(realm, "user1");
+            LDAPOperationManager ldapOperationManager = new LDAPOperationManager(session, ldapFedProvider.getLdapIdentityStore().getConfig());
+
+            ldapOperationManager.removeAttribute(user1LdapObject.getDn().toString(), new BasicAttribute(LDAPConstants.STREET));
+            return null;
+        });
+
+        // Check local storage contains the old value
+        withRealm(realmId, (session, realm) -> {
+            UserModel user1 = UserStoragePrivateUtil.userLocalStorage(session).getUserById(realm, id);
+            assertThat(user1.getAttributes().get(LDAPConstants.STREET).get(0), is(equalTo(MY_STREET_NAME)));
+            return user1.getId();
+        });
+
+        // Read from Keycloak by id
+        withRealm(realmId, (session, realm) -> {
+            UserModel user1 = session.users().getUserById(realm, id);
+            assertThat(user1.getAttributes().get(LDAPConstants.STREET), is(nullValue()));
+            assertThat(user1.getFirstAttribute(LDAPConstants.STREET), is(nullValue()));
+            assertThat(user1.getAttributeStream(LDAPConstants.STREET).findFirst().isPresent(), is(false));
+            return null;
+        });
+
+        // Read from Keycloak by query
+        withRealm(realmId, (session, realm) -> {
+            UserModel user1 = session.users().searchForUserStream(realm, Collections.emptyMap()).findFirst().orElse(null);
+            assertThat(user1.getAttributes().get(LDAPConstants.STREET), is(nullValue()));
+            assertThat(user1.getFirstAttribute(LDAPConstants.STREET), is(nullValue()));
+            assertThat(user1.getAttributeStream(LDAPConstants.STREET).findFirst().isPresent(), is(false));
+            return null;
+        });
+    }
+
 }