KEYCLOAK-2139

UserCache invalidation does not work proper

model/invalidation-cache/infinispan/src/main/java/org/keycloak/models/cache/infinispan/InfinispanCacheUserProviderFactory.java

@@ -1,11 +1,14 @@
 package org.keycloak.models.cache.infinispan;
 
 import org.infinispan.Cache;
+import org.infinispan.commands.write.RemoveCommand;
+import org.infinispan.container.entries.CacheEntry;
+import org.infinispan.context.InvocationContext;
+import org.infinispan.interceptors.base.CommandInterceptor;
 import org.infinispan.notifications.Listener;
-import org.infinispan.notifications.cachelistener.annotation.CacheEntryCreated;
+import org.infinispan.notifications.cachelistener.annotation.*;
-import org.infinispan.notifications.cachelistener.annotation.CacheEntryRemoved;
+import org.infinispan.notifications.cachelistener.event.*;
-import org.infinispan.notifications.cachelistener.event.CacheEntryCreatedEvent;
+import org.jboss.logging.Logger;
-import org.infinispan.notifications.cachelistener.event.CacheEntryRemovedEvent;
 import org.keycloak.Config;
 import org.keycloak.connections.infinispan.InfinispanConnectionProvider;
 import org.keycloak.models.KeycloakSession;
@@ -21,6 +24,8 @@ import java.util.concurrent.ConcurrentHashMap;
  */
 public class InfinispanCacheUserProviderFactory implements CacheUserProviderFactory {
 
+    private static final Logger log = Logger.getLogger(InfinispanCacheUserProviderFactory.class);
+
     protected InfinispanUserCache userCache;
 
     protected final RealmLookup usernameLookup = new RealmLookup();
@@ -82,36 +87,63 @@ public class InfinispanCacheUserProviderFactory implements CacheUserProviderFact
         @CacheEntryCreated
         public void userCreated(CacheEntryCreatedEvent<String, CachedUser> event) {
             if (!event.isPre()) {
-
+                CachedUser user;
-                CachedUser cachedUser;
 
                 // Try optimized version if available
                 if (isNewInfinispan) {
-                    cachedUser = event.getValue();
+                    user = event.getValue();
                 } else {
                     String userId = event.getKey();
-                    cachedUser = event.getCache().get(userId);
+                    user = event.getCache().get(userId);
                 }
 
-                if (cachedUser != null) {
+                if (user != null) {
-                    String realm = cachedUser.getRealm();
+                    String realm = user.getRealm();
-                    usernameLookup.put(realm, cachedUser.getUsername(), cachedUser.getId());
+
-                    if (cachedUser.getEmail() != null) {
+                    usernameLookup.put(realm, user.getUsername(), user.getId());
-                        emailLookup.put(realm, cachedUser.getEmail(), cachedUser.getId());
+                    if (user.getEmail() != null) {
+                        emailLookup.put(realm, user.getEmail(), user.getId());
                     }
+
+                    log.tracev("User added realm={0}, id={1}, username={2}", realm, event.getValue().getId(), event.getValue().getUsername());
                 }
             }
         }
 
         @CacheEntryRemoved
         public void userRemoved(CacheEntryRemovedEvent<String, CachedUser> event) {
-            if (event.isPre() && event.getValue() != null) {
+            CachedUser user = event.getOldValue();
-                CachedUser cachedUser = event.getValue();
+            if (event.isPre() &&  user != null) {
-                String realm = cachedUser.getRealm();
+                removeUser(user);
-                usernameLookup.remove(realm, cachedUser.getUsername());
+
-                if (cachedUser.getEmail() != null) {
+                log.tracev("User invalidated realm={0}, id={1}, username={2}", user.getRealm(), user.getId(), user.getUsername());
-                    emailLookup.remove(realm, cachedUser.getEmail());
+            }
-                }
+        }
+
+        @CacheEntryInvalidated
+        public void userInvalidated(CacheEntryInvalidatedEvent<String, CachedUser> event) {
+            CachedUser user = event.getValue();
+            if (event.isPre() && user != null) {
+                removeUser(user);
+
+                log.tracev("User invalidated realm={0}, id={1}, username={2}", user.getRealm(), user.getId(), user.getUsername());
+            }
+        }
+
+        @CacheEntriesEvicted
+        public void userEvicted(CacheEntriesEvictedEvent<String, CachedUser> event) {
+            for (CachedUser user : event.getEntries().values()) {
+                removeUser(user);
+
+                log.tracev("User evicted realm={0}, id={1}, username={2}", user.getRealm(), user.getId(), user.getUsername());
+            }
+        }
+
+        private void removeUser(CachedUser cachedUser) {
+            String realm = cachedUser.getRealm();
+            usernameLookup.remove(realm, cachedUser.getUsername());
+            if (cachedUser.getEmail() != null) {
+                emailLookup.remove(realm, cachedUser.getEmail());
             }
         }
 
@@ -119,12 +151,12 @@ public class InfinispanCacheUserProviderFactory implements CacheUserProviderFact
 
     static class RealmLookup {
 
-        protected final ConcurrentHashMap<String, ConcurrentHashMap<String, String>> lookup = new ConcurrentHashMap<String, ConcurrentHashMap<String, String>>();
+        protected final ConcurrentHashMap<String, ConcurrentHashMap<String, String>> lookup = new ConcurrentHashMap<>();
 
         public void put(String realm, String key, String value) {
             ConcurrentHashMap<String, String> map = lookup.get(realm);
             if(map == null) {
-                map = new ConcurrentHashMap<String, String>();
+                map = new ConcurrentHashMap<>();
                 ConcurrentHashMap<String, String> p = lookup.putIfAbsent(realm, map);
                 if (p != null) {
                     map = p;

testsuite/integration/src/test/java/org/keycloak/testsuite/account/AccountTest.java

@@ -28,6 +28,7 @@ import org.junit.ClassRule;
 import org.junit.Rule;
 import org.junit.Test;
 import org.keycloak.events.Details;
+import org.keycloak.events.Errors;
 import org.keycloak.events.Event;
 import org.keycloak.events.EventType;
 import org.keycloak.migration.MigrationModel;
@@ -468,6 +469,104 @@ public class AccountTest {
         }
     }
 
+    @Test
+    public void changeUsernameLoginWithOldUsername() {
+        KeycloakSession session = keycloakRule.startSession();
+        UserModel user = session.users().addUser(session.realms().getRealm("test"), "change-username");
+        user.setEnabled(true);
+        user.setEmail("change-username@localhost");
+        user.setFirstName("first");
+        user.setLastName("last");
+        user.updateCredential(UserCredentialModel.password("password"));
+        keycloakRule.stopSession(session, true);
+
+        keycloakRule.update(new KeycloakRule.KeycloakSetup() {
+            @Override
+            public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) {
+                appRealm.setEditUsernameAllowed(true);
+            }
+        });
+
+        try {
+            profilePage.open();
+            loginPage.login("change-username", "password");
+
+            profilePage.updateUsername("change-username-updated");
+
+            Assert.assertEquals("Your account has been updated.", profilePage.getSuccess());
+
+            profilePage.logout();
+
+            profilePage.open();
+
+            Assert.assertTrue(loginPage.isCurrent());
+
+            loginPage.login("change-username", "password");
+
+            Assert.assertTrue(loginPage.isCurrent());
+            Assert.assertEquals("Invalid username or password.", loginPage.getError());
+
+            loginPage.login("change-username-updated", "password");
+        } finally {
+            events.clear();
+            keycloakRule.update(new KeycloakRule.KeycloakSetup() {
+                @Override
+                public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) {
+                    appRealm.setEditUsernameAllowed(false);
+                }
+            });
+        }
+    }
+
+    @Test
+    public void changeEmailLoginWithOldEmail() {
+        KeycloakSession session = keycloakRule.startSession();
+        UserModel user = session.users().addUser(session.realms().getRealm("test"), "change-email");
+        user.setEnabled(true);
+        user.setEmail("change-username@localhost");
+        user.setFirstName("first");
+        user.setLastName("last");
+        user.updateCredential(UserCredentialModel.password("password"));
+        keycloakRule.stopSession(session, true);
+
+        keycloakRule.update(new KeycloakRule.KeycloakSetup() {
+            @Override
+            public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) {
+                appRealm.setEditUsernameAllowed(true);
+            }
+        });
+
+        try {
+            profilePage.open();
+            loginPage.login("change-username@localhost", "password");
+
+            profilePage.updateEmail("change-username-updated@localhost");
+
+            Assert.assertEquals("Your account has been updated.", profilePage.getSuccess());
+
+            profilePage.logout();
+
+            profilePage.open();
+
+            Assert.assertTrue(loginPage.isCurrent());
+
+            loginPage.login("change-username@localhost", "password");
+
+            Assert.assertTrue(loginPage.isCurrent());
+            Assert.assertEquals("Invalid username or password.", loginPage.getError());
+
+            loginPage.login("change-username-updated@localhost", "password");
+        } finally {
+            events.clear();
+            keycloakRule.update(new KeycloakRule.KeycloakSetup() {
+                @Override
+                public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) {
+                    appRealm.setEditUsernameAllowed(false);
+                }
+            });
+        }
+    }
+
     // KEYCLOAK-1534
     @Test
     public void changeEmailToExisting() {

testsuite/integration/src/test/java/org/keycloak/testsuite/pages/AccountUpdateProfilePage.java

@@ -87,6 +87,18 @@ public class AccountUpdateProfilePage extends AbstractAccountPage {
         submitButton.click();
     }
 
+    public void updateUsername(String username) {
+        usernameInput.clear();
+        usernameInput.sendKeys(username);
+        submitButton.click();
+    }
+
+    public void updateEmail(String email) {
+        emailInput.clear();
+        emailInput.sendKeys(email);
+        submitButton.click();
+    }
+
     public void clickCancel() {
         cancelButton.click();
     }