libre.sh/keycloak-scim
3
0
Fork 0
Code Issues 15 Pull requests Releases Packages Activity Actions

Fix memory leak in LDAP

Browse source 
The caching in LDAP stores and reuses the session at the time of creating
`LDAPIdentityStore`. On top of that, there is not much cached, since apart
from the session which must not be part of long-lived cache, only config
is cached in the objects which is anyway always recomputed.

The cache for the LDAP still retains the LDAPConfig to keep
the `logLDAPConfig` call upon config change.

Closes: #19396
This commit is contained in:
Hynek Mlnarik 2023-04-05 12:47:47 +02:00 committed by Hynek Mlnařík
parent 615dc89e13
commit 6014070431
1 changed files with 6 additions and 21 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

27
federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPIdentityStoreRegistry.java
View file

@ -35,11 +35,9 @@ public class LDAPIdentityStoreRegistry {
private static final Logger logger = Logger.getLogger(LDAPIdentityStoreRegistry.class); private static final Logger logger = Logger.getLogger(LDAPIdentityStoreRegistry.class);
private Map<String, LDAPIdentityStoreContext> ldapStores = new ConcurrentHashMap<>(); private final Map<String, LDAPConfig> ldapStores = new ConcurrentHashMap<>();
public LDAPIdentityStore getLdapStore(KeycloakSession session, ComponentModel ldapModel, Map<ComponentModel, LDAPConfigDecorator> configDecorators) { public LDAPIdentityStore getLdapStore(KeycloakSession session, ComponentModel ldapModel, Map<ComponentModel, LDAPConfigDecorator> configDecorators) {
LDAPIdentityStoreContext context = ldapStores.get(ldapModel.getId());
// Ldap config might have changed for the realm. In this case, we must re-initialize // Ldap config might have changed for the realm. In this case, we must re-initialize
MultivaluedHashMap<String, String> configModel = ldapModel.getConfig(); MultivaluedHashMap<String, String> configModel = ldapModel.getConfig();
LDAPConfig ldapConfig = new LDAPConfig(configModel); LDAPConfig ldapConfig = new LDAPConfig(configModel);
@ -50,14 +48,13 @@ public class LDAPIdentityStoreRegistry {
decorator.updateLDAPConfig(ldapConfig, mapperModel); decorator.updateLDAPConfig(ldapConfig, mapperModel);
} }
if (context == null || !ldapConfig.equals(context.config)) { LDAPConfig cachedConfig = ldapStores.get(ldapModel.getId());
if (cachedConfig == null || !ldapConfig.equals(cachedConfig)) {
logLDAPConfig(session, ldapModel, ldapConfig); logLDAPConfig(session, ldapModel, ldapConfig);
ldapStores.put(ldapModel.getId(), ldapConfig);
LDAPIdentityStore store = createLdapIdentityStore(session, ldapConfig);
context = new LDAPIdentityStoreContext(ldapConfig, store);
ldapStores.put(ldapModel.getId(), context);
} }
return context.store;
return createLdapIdentityStore(session, ldapConfig);
} }
// Don't log LDAP password // Don't log LDAP password
@ -97,16 +94,4 @@ public class LDAPIdentityStoreRegistry {
} }
System.setProperty(name, value); System.setProperty(name, value);
} }
private static class LDAPIdentityStoreContext {
private LDAPIdentityStoreContext(LDAPConfig config, LDAPIdentityStore store) {
this.config = config;
this.store = store;
}
private LDAPConfig config;
private LDAPIdentityStore store;
}
} }