KEYCLOAK-5245 Restart failures when deleting a client with existing sessions/offline_tokens

model/jpa/src/main/java/org/keycloak/models/jpa/session/JpaUserSessionPersisterProvider.java

@@ -36,8 +36,10 @@ import javax.persistence.Query;
 import javax.persistence.TypedQuery;
 import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 /**
  * @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
@@ -162,7 +164,11 @@ public class JpaUserSessionPersisterProvider implements UserSessionPersisterProv
 
     @Override
     public void onClientRemoved(RealmModel realm, ClientModel client) {
-        int num = em.createNamedQuery("deleteClientSessionsByClient").setParameter("clientId", client.getId()).executeUpdate();
+        onClientRemoved(client.getId());
+    }
+
+    private void onClientRemoved(String clientUUID) {
+        int num = em.createNamedQuery("deleteClientSessionsByClient").setParameter("clientId", clientUUID).executeUpdate();
         num = em.createNamedQuery("deleteDetachedUserSessions").executeUpdate();
     }
 
@@ -223,6 +229,8 @@ public class JpaUserSessionPersisterProvider implements UserSessionPersisterProv
             userSessionIds.add(entity.getUserSessionId());
         }
 
+        Set<String> removedClientUUIDs = new HashSet<>();
+
         if (!userSessionIds.isEmpty()) {
             TypedQuery<PersistentClientSessionEntity> query2 = em.createNamedQuery("findClientSessionsByUserSessions", PersistentClientSessionEntity.class);
             query2.setParameter("userSessionIds", userSessionIds);
@@ -240,7 +248,13 @@ public class JpaUserSessionPersisterProvider implements UserSessionPersisterProv
                     PersistentClientSessionEntity clientSession = clientSessions.get(j);
                     if (clientSession.getUserSessionId().equals(userSession.getId())) {
                         PersistentAuthenticatedClientSessionAdapter clientSessAdapter = toAdapter(userSession.getRealm(), userSession, clientSession);
+
+                        // Case when client was removed in the meantime
+                        if (clientSessAdapter.getClient() == null) {
+                            removedClientUUIDs.add(clientSession.getClientId());
+                        } else {
                             currentClientSessions.put(clientSession.getClientId(), clientSessAdapter);
+                        }
                         j++;
                     } else {
                         next = false;
@@ -249,6 +263,9 @@ public class JpaUserSessionPersisterProvider implements UserSessionPersisterProv
             }
         }
 
+        for (String clientUUID : removedClientUUIDs) {
+            onClientRemoved(clientUUID);
+        }
 
         return result;
     }

testsuite/integration-deprecated/src/test/java/org/keycloak/testsuite/model/UserSessionInitializerTest.java

@@ -33,7 +33,6 @@ import org.keycloak.models.UserModel;
 import org.keycloak.models.UserSessionModel;
 import org.keycloak.models.UserSessionProvider;
 import org.keycloak.models.UserSessionProviderFactory;
-import org.keycloak.models.utils.KeycloakModelUtils;
 import org.keycloak.protocol.oidc.OIDCLoginProtocol;
 import org.keycloak.models.UserManager;
 import org.keycloak.services.managers.UserSessionManager;
@@ -80,14 +79,72 @@ public class UserSessionInitializerTest {
 
     @Test
     public void testUserSessionInitializer() {
-        UserSessionModel[] origSessions = createSessions();
+        int started = Time.currentTime();
+        int serverStartTime = session.getProvider(ClusterProvider.class).getClusterStartupTime();
+
+        UserSessionModel[] origSessions = createSessionsInPersisterOnly();
+
+        // Load sessions from persister into infinispan/memory
+        UserSessionProviderFactory userSessionFactory = (UserSessionProviderFactory) session.getKeycloakSessionFactory().getProviderFactory(UserSessionProvider.class);
+        userSessionFactory.loadPersistentSessions(session.getKeycloakSessionFactory(), 1, 2);
 
         resetSession();
 
-        // Create and persist offline sessions
+        // Assert sessions are in
+        ClientModel testApp = realm.getClientByClientId("test-app");
+        ClientModel thirdparty = realm.getClientByClientId("third-party");
+        Assert.assertEquals(3, session.sessions().getOfflineSessionsCount(realm, testApp));
+        Assert.assertEquals(1, session.sessions().getOfflineSessionsCount(realm, thirdparty));
+
+        List<UserSessionModel> loadedSessions = session.sessions().getOfflineUserSessions(realm, testApp, 0, 10);
+        UserSessionProviderTest.assertSessions(loadedSessions, origSessions);
+
+        UserSessionPersisterProviderTest.assertSessionLoaded(loadedSessions, origSessions[0].getId(), session.users().getUserByUsername("user1", realm), "127.0.0.1", started, serverStartTime, "test-app", "third-party");
+        UserSessionPersisterProviderTest.assertSessionLoaded(loadedSessions, origSessions[1].getId(), session.users().getUserByUsername("user1", realm), "127.0.0.2", started, serverStartTime, "test-app");
+        UserSessionPersisterProviderTest.assertSessionLoaded(loadedSessions, origSessions[2].getId(), session.users().getUserByUsername("user2", realm), "127.0.0.3", started, serverStartTime, "test-app");
+    }
+
+
+    // KEYCLOAK-5245
+    @Test
+    public void testUserSessionInitializerWithDeletingClient() {
         int started = Time.currentTime();
         int serverStartTime = session.getProvider(ClusterProvider.class).getClusterStartupTime();
 
+        UserSessionModel[] origSessions = createSessionsInPersisterOnly();
+
+        // Delete one of the clients now. Delete it directly in DB just for the purpose of simulating the issue (normally clients should be removed through ClientManager)
+        ClientModel testApp = realm.getClientByClientId("test-app");
+        realm.removeClient(testApp.getId());
+
+        resetSession();
+
+        // Load sessions from persister into infinispan/memory
+        UserSessionProviderFactory userSessionFactory = (UserSessionProviderFactory) session.getKeycloakSessionFactory().getProviderFactory(UserSessionProvider.class);
+        userSessionFactory.loadPersistentSessions(session.getKeycloakSessionFactory(), 1, 2);
+
+        resetSession();
+
+        // Assert sessions are in
+        ClientModel thirdparty = realm.getClientByClientId("third-party");
+        Assert.assertEquals(1, session.sessions().getOfflineSessionsCount(realm, thirdparty));
+
+        List<UserSessionModel> loadedSessions = session.sessions().getOfflineUserSessions(realm, thirdparty, 0, 10);
+        Assert.assertEquals(1, loadedSessions.size());
+
+        UserSessionPersisterProviderTest.assertSessionLoaded(loadedSessions, origSessions[0].getId(), session.users().getUserByUsername("user1", realm), "127.0.0.1", started, serverStartTime, "third-party");
+
+        // Revert client
+        realm.addClient("test-app");
+    }
+
+
+    // Create sessions in persister+infinispan, but then delete them from infinispan cache. This is to allow later testing of initializer. Return the list of "origSessions"
+    private UserSessionModel[] createSessionsInPersisterOnly() {
+        UserSessionModel[] origSessions = createSessions();
+
+        resetSession();
+
         for (UserSessionModel origSession : origSessions) {
             UserSessionModel userSession = session.sessions().getUserSession(realm, origSession.getId());
             for (AuthenticatedClientSessionModel clientSession : userSession.getAuthenticatedClientSessions().values()) {
@@ -111,25 +168,10 @@ public class UserSessionInitializerTest {
         Assert.assertEquals(0, session.sessions().getOfflineSessionsCount(realm, testApp));
         Assert.assertEquals(0, session.sessions().getOfflineSessionsCount(realm, thirdparty));
 
-        // Load sessions from persister into infinispan/memory
+        return origSessions;
-        UserSessionProviderFactory userSessionFactory = (UserSessionProviderFactory) session.getKeycloakSessionFactory().getProviderFactory(UserSessionProvider.class);
-        userSessionFactory.loadPersistentSessions(session.getKeycloakSessionFactory(), 1, 2);
-
-        resetSession();
-
-        // Assert sessions are in
-        testApp = realm.getClientByClientId("test-app");
-        Assert.assertEquals(3, session.sessions().getOfflineSessionsCount(realm, testApp));
-        Assert.assertEquals(1, session.sessions().getOfflineSessionsCount(realm, thirdparty));
-
-        List<UserSessionModel> loadedSessions = session.sessions().getOfflineUserSessions(realm, testApp, 0, 10);
-        UserSessionProviderTest.assertSessions(loadedSessions, origSessions);
-
-        UserSessionPersisterProviderTest.assertSessionLoaded(loadedSessions, origSessions[0].getId(), session.users().getUserByUsername("user1", realm), "127.0.0.1", started, serverStartTime, "test-app", "third-party");
-        UserSessionPersisterProviderTest.assertSessionLoaded(loadedSessions, origSessions[1].getId(), session.users().getUserByUsername("user1", realm), "127.0.0.2", started, serverStartTime, "test-app");
-        UserSessionPersisterProviderTest.assertSessionLoaded(loadedSessions, origSessions[2].getId(), session.users().getUserByUsername("user2", realm), "127.0.0.3", started, serverStartTime, "test-app");
     }
 
+
     private AuthenticatedClientSessionModel createClientSession(ClientModel client, UserSessionModel userSession, String redirect, String state, Set<String> roles, Set<String> protocolMappers) {
         AuthenticatedClientSessionModel clientSession = session.sessions().createClientSession(realm, client, userSession);
         clientSession.setRedirectUri(redirect);