keycloak-uncached
Changes
federation/kerberos/src/main/java/org/keycloak/federation/kerberos/KerberosFederationProvider.java 3(+2 -1)
model/jpa/src/main/java/org/keycloak/models/jpa/session/JpaUserSessionPersisterProvider.java 24(+17 -7)
Details
diff --git a/federation/kerberos/src/main/java/org/keycloak/federation/kerberos/KerberosFederationProvider.java b/federation/kerberos/src/main/java/org/keycloak/federation/kerberos/KerberosFederationProvider.java
index e45debd..a3f30ba 100755
--- a/federation/kerberos/src/main/java/org/keycloak/federation/kerberos/KerberosFederationProvider.java
+++ b/federation/kerberos/src/main/java/org/keycloak/federation/kerberos/KerberosFederationProvider.java
@@ -22,6 +22,7 @@ import org.keycloak.models.UserFederationProvider;
import org.keycloak.models.UserFederationProviderModel;
import org.keycloak.models.UserModel;
import org.keycloak.common.constants.KerberosConstants;
+import org.keycloak.services.managers.UserManager;
/**
* @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
@@ -242,7 +243,7 @@ public class KerberosFederationProvider implements UserFederationProvider {
logger.warn("User with username " + username + " already exists and is linked to provider [" + model.getDisplayName() +
"] but kerberos principal is not correct. Kerberos principal on user is: " + user.getFirstAttribute(KERBEROS_PRINCIPAL));
logger.warn("Will re-create user");
- session.userStorage().removeUser(realm, user);
+ new UserManager(session).removeUser(realm, user, session.userStorage());
}
}
}
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 61786cc..07eb752 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
@@ -26,6 +26,7 @@ import org.keycloak.models.UserFederationProvider;
import org.keycloak.models.UserFederationProviderModel;
import org.keycloak.models.UserModel;
import org.keycloak.common.constants.KerberosConstants;
+import org.keycloak.services.managers.UserManager;
import java.util.ArrayList;
import java.util.Arrays;
@@ -465,7 +466,7 @@ public class LDAPFederationProvider implements UserFederationProvider {
logger.warnf("User with username [%s] aready exists and is linked to provider [%s] but is not valid. Stale LDAP_ID on local user is: %s",
username, model.getDisplayName(), user.getFirstAttribute(LDAPConstants.LDAP_ID));
logger.warn("Will re-create user");
- session.userStorage().removeUser(realm, user);
+ new UserManager(session).removeUser(realm, user, session.userStorage());
}
}
}
diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/session/JpaUserSessionPersisterProvider.java b/model/jpa/src/main/java/org/keycloak/models/jpa/session/JpaUserSessionPersisterProvider.java
index 5fa648e..56c3964 100644
--- a/model/jpa/src/main/java/org/keycloak/models/jpa/session/JpaUserSessionPersisterProvider.java
+++ b/model/jpa/src/main/java/org/keycloak/models/jpa/session/JpaUserSessionPersisterProvider.java
@@ -150,8 +150,12 @@ public class JpaUserSessionPersisterProvider implements UserSessionPersisterProv
@Override
public void onUserRemoved(RealmModel realm, UserModel user) {
- int num = em.createNamedQuery("deleteClientSessionsByUser").setParameter("userId", user.getId()).executeUpdate();
- num = em.createNamedQuery("deleteUserSessionsByUser").setParameter("userId", user.getId()).executeUpdate();
+ onUserRemoved(realm, user.getId());
+ }
+
+ private void onUserRemoved(RealmModel realm, String userId) {
+ int num = em.createNamedQuery("deleteClientSessionsByUser").setParameter("userId", userId).executeUpdate();
+ num = em.createNamedQuery("deleteUserSessionsByUser").setParameter("userId", userId).executeUpdate();
}
@Override
@@ -184,7 +188,16 @@ public class JpaUserSessionPersisterProvider implements UserSessionPersisterProv
List<UserSessionModel> result = new ArrayList<>();
List<String> userSessionIds = new ArrayList<>();
for (PersistentUserSessionEntity entity : results) {
- result.add(toAdapter(entity));
+ RealmModel realm = session.realms().getRealm(entity.getRealmId());
+ UserModel user = session.users().getUserById(entity.getUserId(), realm);
+
+ // Case when user was deleted in the meantime
+ if (user == null) {
+ onUserRemoved(realm, entity.getUserId());
+ return loadUserSessions(firstResult, maxResults, offline);
+ }
+
+ result.add(toAdapter(realm, user, entity));
userSessionIds.add(entity.getUserSessionId());
}
@@ -217,10 +230,7 @@ public class JpaUserSessionPersisterProvider implements UserSessionPersisterProv
return result;
}
- private PersistentUserSessionAdapter toAdapter(PersistentUserSessionEntity entity) {
- RealmModel realm = session.realms().getRealm(entity.getRealmId());
- UserModel user = session.users().getUserById(entity.getUserId(), realm);
-
+ private PersistentUserSessionAdapter toAdapter(RealmModel realm, UserModel user, PersistentUserSessionEntity entity) {
PersistentUserSessionModel model = new PersistentUserSessionModel();
model.setUserSessionId(entity.getUserSessionId());
model.setLastSessionRefresh(entity.getLastSessionRefresh());
diff --git a/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/MongoUserSessionPersisterProvider.java b/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/MongoUserSessionPersisterProvider.java
index f23e7fb..47b719f 100644
--- a/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/MongoUserSessionPersisterProvider.java
+++ b/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/MongoUserSessionPersisterProvider.java
@@ -191,8 +191,12 @@ public class MongoUserSessionPersisterProvider implements UserSessionPersisterPr
@Override
public void onUserRemoved(RealmModel realm, UserModel user) {
+ onUserRemoved(realm, user.getId());
+ }
+
+ private void onUserRemoved(RealmModel realm, String userId) {
DBObject query = new QueryBuilder()
- .and("userId").is(user.getId())
+ .and("userId").is(userId)
.get();
getMongoStore().removeEntities(MongoOnlineUserSessionEntity.class, query, false, invocationContext);
getMongoStore().removeEntities(MongoOfflineUserSessionEntity.class, query, false, invocationContext);
@@ -263,16 +267,22 @@ public class MongoUserSessionPersisterProvider implements UserSessionPersisterPr
List<UserSessionModel> results = new LinkedList<>();
for (MongoUserSessionEntity entity : entities) {
- PersistentUserSessionAdapter userSession = toAdapter(entity);
+ RealmModel realm = session.realms().getRealm(entity.getRealmId());
+ UserModel user = session.users().getUserById(entity.getUserId(), realm);
+
+ // Case when user was deleted in the meantime
+ if (user == null) {
+ onUserRemoved(realm, entity.getUserId());
+ return loadUserSessions(firstResult, maxResults, offline);
+ }
+
+ PersistentUserSessionAdapter userSession = toAdapter(realm, user, entity);
results.add(userSession);
}
return results;
}
- private PersistentUserSessionAdapter toAdapter(PersistentUserSessionEntity entity) {
- RealmModel realm = session.realms().getRealm(entity.getRealmId());
- UserModel user = session.users().getUserById(entity.getUserId(), realm);
-
+ private PersistentUserSessionAdapter toAdapter(RealmModel realm, UserModel user, PersistentUserSessionEntity entity) {
PersistentUserSessionModel model = new PersistentUserSessionModel();
model.setUserSessionId(entity.getId());
model.setLastSessionRefresh(entity.getLastSessionRefresh());
diff --git a/server-spi/src/main/java/org/keycloak/models/UserFederationManager.java b/server-spi/src/main/java/org/keycloak/models/UserFederationManager.java
index c58278b..581fc3b 100755
--- a/server-spi/src/main/java/org/keycloak/models/UserFederationManager.java
+++ b/server-spi/src/main/java/org/keycloak/models/UserFederationManager.java
@@ -2,6 +2,7 @@ package org.keycloak.models;
import org.jboss.logging.Logger;
import org.keycloak.models.utils.KeycloakModelUtils;
+import org.keycloak.services.managers.UserManager;
import java.util.ArrayList;
import java.util.Arrays;
@@ -113,7 +114,7 @@ public class UserFederationManager implements UserProvider {
RealmModel realmModel = session.realms().getRealm(realm.getId());
if (realmModel == null) return;
UserModel deletedUser = session.userStorage().getUserById(user.getId(), realmModel);
- session.userStorage().removeUser(realmModel, deletedUser);
+ new UserManager(session).removeUser(realmModel, deletedUser, session.userStorage());
logger.debugf("Removed invalid user '%s'", user.getUsername());
}
diff --git a/services/src/main/java/org/keycloak/services/managers/ClientManager.java b/services/src/main/java/org/keycloak/services/managers/ClientManager.java
index db68a3c..585e063 100644
--- a/services/src/main/java/org/keycloak/services/managers/ClientManager.java
+++ b/services/src/main/java/org/keycloak/services/managers/ClientManager.java
@@ -104,7 +104,7 @@ public class ClientManager {
UserModel serviceAccountUser = realmManager.getSession().users().getUserByServiceAccountClient(client);
if (serviceAccountUser != null) {
- realmManager.getSession().users().removeUser(realm, serviceAccountUser);
+ new UserManager(realmManager.getSession()).removeUser(realm, serviceAccountUser);
}
return true;
diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/model/UserSessionPersisterProviderTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/model/UserSessionPersisterProviderTest.java
index cbc8284..88e13e8 100644
--- a/testsuite/integration/src/test/java/org/keycloak/testsuite/model/UserSessionPersisterProviderTest.java
+++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/model/UserSessionPersisterProviderTest.java
@@ -53,8 +53,12 @@ public class UserSessionPersisterProviderTest {
UserModel user2 = session.users().getUserByUsername("user2", realm);
UserManager um = new UserManager(session);
- um.removeUser(realm, user1);
- um.removeUser(realm, user2);
+ if (user1 != null) {
+ um.removeUser(realm, user1);
+ }
+ if (user2 != null) {
+ um.removeUser(realm, user2);
+ }
kc.stopSession(session, true);
}
@@ -290,6 +294,43 @@ public class UserSessionPersisterProviderTest {
realmMgr.removeRealm(realmMgr.getRealm("foo"));
}
+ @Test
+ public void testOnUserRemoved() {
+ // Create some sessions in infinispan
+ int started = Time.currentTime();
+ UserSessionModel[] origSessions = createSessions();
+
+ resetSession();
+
+ // Persist 2 offline sessions of 2 users
+ UserSessionModel userSession1 = session.sessions().getUserSession(realm, origSessions[1].getId());
+ UserSessionModel userSession2 = session.sessions().getUserSession(realm, origSessions[2].getId());
+ persistUserSession(userSession1, true);
+ persistUserSession(userSession2, true);
+
+ resetSession();
+
+ // Load offline sessions
+ List<UserSessionModel> loadedSessions = loadPersistedSessionsPaginated(true, 10, 1, 2);
+
+ // Properly delete user and assert his offlineSession removed
+ UserModel user1 = session.users().getUserByUsername("user1", realm);
+ new UserManager(session).removeUser(realm, user1);
+
+ resetSession();
+
+ loadedSessions = loadPersistedSessionsPaginated(true, 10, 1, 1);
+ UserSessionModel persistedSession = loadedSessions.get(0);
+ UserSessionProviderTest.assertSession(persistedSession, session.users().getUserByUsername("user2", realm), "127.0.0.3", started, started, "test-app");
+
+ // KEYCLOAK-2431 Assert that userSessionPersister is resistent even to situation, when users are deleted "directly"
+ UserModel user2 = session.users().getUserByUsername("user2", realm);
+ session.users().removeUser(realm, user2);
+
+ loadedSessions = loadPersistedSessionsPaginated(true, 10, 0, 0);
+
+ }
+
// KEYCLOAK-1999
@Test
public void testNoSessions() {
@@ -376,8 +417,7 @@ public class UserSessionPersisterProviderTest {
}
Assert.assertEquals(pageCount, expectedPageCount);
- Assert.assertEquals(count, expectedSessionsCount);
- Assert.assertEquals(count, result.size());
+ Assert.assertEquals(result.size(), expectedSessionsCount);
return result;
}
}