From 88141320acc82a842aa6c17fde9c249aab854b08 Mon Sep 17 00:00:00 2001
From: mposolda <mposolda@gmail.com>
Date: Thu, 6 Dec 2018 17:17:40 +0100
Subject: [PATCH] KEYCLOAK-9002 StackOverflowError when reading LDAP-backed
 users via REST API

---
 .../storage/ldap/LDAPStorageProvider.java     | 11 ++++++++
 .../ldap/LDAPProvidersIntegrationTest.java    | 28 +++++++++++++++++++
 2 files changed, 39 insertions(+)

diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java
index f130f25a6d..f95050aa4e 100755
--- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java
+++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java
@@ -39,6 +39,7 @@ import org.keycloak.credential.CredentialModel;
 import org.keycloak.federation.kerberos.impl.KerberosUsernamePasswordAuthenticator;
 import org.keycloak.federation.kerberos.impl.SPNEGOAuthenticator;
 import org.keycloak.models.*;
+import org.keycloak.models.cache.CachedUserModel;
 import org.keycloak.models.utils.DefaultRoles;
 import org.keycloak.models.utils.ReadOnlyUserModelDelegate;
 import org.keycloak.policy.PasswordPolicyManagerProvider;
@@ -160,6 +161,16 @@ public class LDAPStorageProvider implements UserStorageProvider,
             return existing;
         }
 
+        // We need to avoid having CachedUserModel as cache is upper-layer then LDAP. Hence having CachedUserModel here may cause StackOverflowError
+        if (local instanceof CachedUserModel) {
+            local = session.userStorageManager().getUserById(local.getId(), realm);
+
+            existing = userManager.getManagedProxiedUser(local.getId());
+            if (existing != null) {
+                return existing;
+            }
+        }
+
         UserModel proxied = local;
 
         checkDNChanged(realm, local, ldapObject);
diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPProvidersIntegrationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPProvidersIntegrationTest.java
index bac2dc16d3..953b89bade 100755
--- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPProvidersIntegrationTest.java
+++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPProvidersIntegrationTest.java
@@ -985,6 +985,34 @@ public class LDAPProvidersIntegrationTest extends AbstractLDAPTest {
         });
     }
 
+
+    // KEYCLOAK-9002
+    @Test
+    public void testSearchWithPartiallyCachedUser() {
+        testingClient.server().run(session -> {
+            session.userCache().clear();
+        });
+
+
+        // This will load user from LDAP and partially cache him (including attributes)
+        testingClient.server().run(session -> {
+            LDAPTestContext ctx = LDAPTestContext.init(session);
+            RealmModel appRealm = ctx.getRealm();
+            UserModel user = session.users().getUserByUsername("johnkeycloak", appRealm);
+            Assert.assertNotNull(user);
+
+            user.getAttributes();
+        });
+
+
+        // Assert search without arguments won't blow up with StackOverflowError
+        adminClient.realm("test").users().search(null, 0, 10, false);
+
+        List<UserRepresentation> users = adminClient.realm("test").users().search("johnkeycloak", 0, 10, false);
+        Assert.assertTrue(users.stream().anyMatch(userRepresentation -> "johnkeycloak".equals(userRepresentation.getUsername())));
+    }
+
+
     @Test
     public void testLDAPUserRefreshCache() {
         testingClient.server().run(session -> {