keycloak-memoizeit

KEYCLOAK-9002 StackOverflowError when reading LDAP-backed

12/6/2018 2:17:40 PM

Details

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 f130f25..f95050a 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 bac2dc1..953b89b 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 -> {