From b996e88dbd4110290a9841fa091be549a9587ae3 Mon Sep 17 00:00:00 2001 From: Stian Thorgersen Date: Thu, 26 Nov 2015 16:23:17 +0100 Subject: [PATCH] KEYCLOAK-2139 UserCache invalidation does not work proper --- .../InfinispanCacheUserProviderFactory.java | 76 +++++++++----- .../testsuite/account/AccountTest.java | 99 +++++++++++++++++++ .../pages/AccountUpdateProfilePage.java | 12 +++ 3 files changed, 165 insertions(+), 22 deletions(-) diff --git a/model/invalidation-cache/infinispan/src/main/java/org/keycloak/models/cache/infinispan/InfinispanCacheUserProviderFactory.java b/model/invalidation-cache/infinispan/src/main/java/org/keycloak/models/cache/infinispan/InfinispanCacheUserProviderFactory.java index b046efc9c1..60c089b486 100755 --- a/model/invalidation-cache/infinispan/src/main/java/org/keycloak/models/cache/infinispan/InfinispanCacheUserProviderFactory.java +++ b/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.CacheEntryRemoved; -import org.infinispan.notifications.cachelistener.event.CacheEntryCreatedEvent; -import org.infinispan.notifications.cachelistener.event.CacheEntryRemovedEvent; +import org.infinispan.notifications.cachelistener.annotation.*; +import org.infinispan.notifications.cachelistener.event.*; +import org.jboss.logging.Logger; 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 event) { if (!event.isPre()) { - - CachedUser cachedUser; + CachedUser user; // 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) { - String realm = cachedUser.getRealm(); - usernameLookup.put(realm, cachedUser.getUsername(), cachedUser.getId()); - if (cachedUser.getEmail() != null) { - emailLookup.put(realm, cachedUser.getEmail(), cachedUser.getId()); + if (user != null) { + String realm = user.getRealm(); + + usernameLookup.put(realm, user.getUsername(), user.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 event) { - if (event.isPre() && event.getValue() != null) { - CachedUser cachedUser = event.getValue(); - String realm = cachedUser.getRealm(); - usernameLookup.remove(realm, cachedUser.getUsername()); - if (cachedUser.getEmail() != null) { - emailLookup.remove(realm, cachedUser.getEmail()); - } + CachedUser user = event.getOldValue(); + if (event.isPre() && user != null) { + removeUser(user); + + log.tracev("User invalidated realm={0}, id={1}, username={2}", user.getRealm(), user.getId(), user.getUsername()); + } + } + + @CacheEntryInvalidated + public void userInvalidated(CacheEntryInvalidatedEvent 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 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> lookup = new ConcurrentHashMap>(); + protected final ConcurrentHashMap> lookup = new ConcurrentHashMap<>(); public void put(String realm, String key, String value) { ConcurrentHashMap map = lookup.get(realm); if(map == null) { - map = new ConcurrentHashMap(); + map = new ConcurrentHashMap<>(); ConcurrentHashMap p = lookup.putIfAbsent(realm, map); if (p != null) { map = p; diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/account/AccountTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/account/AccountTest.java index 9458998b19..d02c2559cc 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/account/AccountTest.java +++ b/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() { diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/pages/AccountUpdateProfilePage.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/pages/AccountUpdateProfilePage.java index c013345048..1fe620396a 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/pages/AccountUpdateProfilePage.java +++ b/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(); }