This is an automated email from the ASF dual-hosted git repository.
jbonofre pushed a commit to branch karaf-4.4.x
in repository https://gitbox.apache.org/repos/asf/karaf.git
The following commit(s) were added to refs/heads/karaf-4.4.x by this push:
new d2983139fd [KARAF-7671] Fix LDAP cache handling so it does not reset
on every login attempt (#2071)
d2983139fd is described below
commit d2983139fd9868e25fb210a18a2bfb81c6370b39
Author: JB Onofré <[email protected]>
AuthorDate: Sun Oct 5 08:54:44 2025 +0200
[KARAF-7671] Fix LDAP cache handling so it does not reset on every login
attempt (#2071)
- Add metrics to the LDAPCache object
- Fix unit test to _not_ disable cache
- Update unit test to verify metrics counts and caching are working
Co-authored-by: Matt Pavlovich <[email protected]>
---
.../apache/karaf/jaas/modules/impl/Activator.java | 1 +
.../karaf/jaas/modules/ldap/LDAPBackingEngine.java | 2 -
.../modules/ldap/LDAPBackingEngineFactory.java | 1 -
.../apache/karaf/jaas/modules/ldap/LDAPCache.java | 86 ++++++++++++++++++++++
.../karaf/jaas/modules/ldap/LDAPLoginModule.java | 4 +-
.../jaas/modules/ldap/LDAPPubkeyLoginModule.java | 1 -
.../karaf/jaas/modules/ldap/LdapCacheTest.java | 62 ++++++++++++----
.../apache/karaf/jaas/modules/ldap/ldap.properties | 1 +
.../karaf/jaas/modules/ldap/ldap_badref.properties | 1 +
9 files changed, 139 insertions(+), 20 deletions(-)
diff --git
a/jaas/modules/src/main/java/org/apache/karaf/jaas/modules/impl/Activator.java
b/jaas/modules/src/main/java/org/apache/karaf/jaas/modules/impl/Activator.java
index b91f5f6fc0..9abc16dcd4 100644
---
a/jaas/modules/src/main/java/org/apache/karaf/jaas/modules/impl/Activator.java
+++
b/jaas/modules/src/main/java/org/apache/karaf/jaas/modules/impl/Activator.java
@@ -56,6 +56,7 @@ public class Activator extends BaseActivator implements
ManagedService {
@Override
protected void doOpen() throws Exception {
super.doOpen();
+ LDAPCache.clear();
register(BackingEngineFactory.class, new
PropertiesBackingEngineFactory());
register(BackingEngineFactory.class, new
PublickeyBackingEngineFactory());
diff --git
a/jaas/modules/src/main/java/org/apache/karaf/jaas/modules/ldap/LDAPBackingEngine.java
b/jaas/modules/src/main/java/org/apache/karaf/jaas/modules/ldap/LDAPBackingEngine.java
index 90daf5dc2c..1039a60ee8 100644
---
a/jaas/modules/src/main/java/org/apache/karaf/jaas/modules/ldap/LDAPBackingEngine.java
+++
b/jaas/modules/src/main/java/org/apache/karaf/jaas/modules/ldap/LDAPBackingEngine.java
@@ -18,8 +18,6 @@ import org.apache.karaf.jaas.boot.principal.GroupPrincipal;
import org.apache.karaf.jaas.boot.principal.RolePrincipal;
import org.apache.karaf.jaas.boot.principal.UserPrincipal;
import org.apache.karaf.jaas.modules.BackingEngine;
-import org.apache.karaf.jaas.modules.ldap.LDAPCache;
-import org.apache.karaf.jaas.modules.ldap.LDAPOptions;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
diff --git
a/jaas/modules/src/main/java/org/apache/karaf/jaas/modules/ldap/LDAPBackingEngineFactory.java
b/jaas/modules/src/main/java/org/apache/karaf/jaas/modules/ldap/LDAPBackingEngineFactory.java
index 16d1a78025..35768c263c 100644
---
a/jaas/modules/src/main/java/org/apache/karaf/jaas/modules/ldap/LDAPBackingEngineFactory.java
+++
b/jaas/modules/src/main/java/org/apache/karaf/jaas/modules/ldap/LDAPBackingEngineFactory.java
@@ -16,7 +16,6 @@ package org.apache.karaf.jaas.modules.ldap;
import org.apache.karaf.jaas.modules.BackingEngine;
import org.apache.karaf.jaas.modules.BackingEngineFactory;
-import org.apache.karaf.jaas.modules.ldap.LDAPLoginModule;
import java.util.Map;
/**
diff --git
a/jaas/modules/src/main/java/org/apache/karaf/jaas/modules/ldap/LDAPCache.java
b/jaas/modules/src/main/java/org/apache/karaf/jaas/modules/ldap/LDAPCache.java
index 07efe25587..20759e8ee5 100644
---
a/jaas/modules/src/main/java/org/apache/karaf/jaas/modules/ldap/LDAPCache.java
+++
b/jaas/modules/src/main/java/org/apache/karaf/jaas/modules/ldap/LDAPCache.java
@@ -30,6 +30,7 @@ import javax.naming.event.NamingExceptionEvent;
import javax.naming.event.ObjectChangeListener;
import java.io.Closeable;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
@@ -37,6 +38,7 @@ import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.atomic.AtomicLong;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
@@ -45,6 +47,7 @@ import org.slf4j.LoggerFactory;
public class LDAPCache implements Closeable, NamespaceChangeListener,
ObjectChangeListener {
+ private static final AtomicLong idGenerator = new AtomicLong(0l);
private static final ConcurrentMap<LDAPOptions, LDAPCache> CACHES = new
ConcurrentHashMap<>();
private static Logger LOGGER =
LoggerFactory.getLogger(LDAPLoginModule.class);
@@ -68,14 +71,43 @@ public class LDAPCache implements Closeable,
NamespaceChangeListener, ObjectChan
return cache;
}
+ public static LDAPCache removeCache(long id) {
+
+ LDAPOptions match = null;
+ for(Map.Entry<LDAPOptions, LDAPCache> entry : CACHES.entrySet()) {
+ if(entry.getValue().getId() == id) {
+ match = entry.getKey();
+ }
+ }
+
+ if(match != null) {
+ return CACHES.remove(match);
+ }
+
+ return null;
+ }
+
+ public static Map<LDAPOptions, LDAPCache> getCacheCopy() {
+ return Collections.unmodifiableMap(CACHES);
+ }
+
+ private final long id;
private final Map<String, String[]> userDnAndNamespace;
private final Map<String, String[]> userRoles;
private final Map<String, String[]> userPubkeys;
private final LDAPOptions options;
private DirContext context;
+ private final AtomicLong userDNCacheHitCount = new AtomicLong(0l);
+ private final AtomicLong userDNCacheMissCount = new AtomicLong(0l);
+ private final AtomicLong userRolesCacheHitCount = new AtomicLong(0l);
+ private final AtomicLong userRolesCacheMissCount = new AtomicLong(0l);
+ private final AtomicLong userPubkeysCacheHitCount = new AtomicLong(0l);
+ private final AtomicLong userPubkeysCacheMissCount = new AtomicLong(0l);
+ private final AtomicLong clearCacheCount = new AtomicLong(0l);
public LDAPCache(LDAPOptions options) {
this.options = options;
+ this.id = idGenerator.getAndIncrement();
userDnAndNamespace = new HashMap<>();
userRoles = new HashMap<>();
userPubkeys = new HashMap<>();
@@ -142,10 +174,13 @@ public class LDAPCache implements Closeable,
NamespaceChangeListener, ObjectChan
public synchronized String[] getUserDnAndNamespace(String user) throws
Exception {
String[] result = userDnAndNamespace.get(user);
if (result == null) {
+ this.userDNCacheMissCount.incrementAndGet();
result = doGetUserDnAndNamespace(user);
if (result != null && !options.getDisableCache()) {
userDnAndNamespace.put(user, result);
}
+ } else {
+ this.userDNCacheHitCount.incrementAndGet();
}
return result;
}
@@ -207,10 +242,13 @@ public class LDAPCache implements Closeable,
NamespaceChangeListener, ObjectChan
public synchronized String[] getUserRoles(String user, String userDn,
String userDnNamespace) throws Exception {
String[] result = userRoles.get(userDn);
if (result == null) {
+ this.userRolesCacheMissCount.incrementAndGet();
result = doGetUserRoles(user, userDn, userDnNamespace);
if (!options.getDisableCache()) {
userRoles.put(userDn, result);
}
+ } else {
+ this.userRolesCacheHitCount.incrementAndGet();
}
return result;
}
@@ -218,10 +256,13 @@ public class LDAPCache implements Closeable,
NamespaceChangeListener, ObjectChan
public synchronized String[] getUserPubkeys(String userDn) throws
NamingException {
String[] result = userPubkeys.get(userDn);
if (result == null) {
+ this.userPubkeysCacheMissCount.incrementAndGet();
result = doGetUserPubkeys(userDn);
if (!options.getDisableCache()) {
userPubkeys.put(userDn, result);
}
+ } else {
+ this.userPubkeysCacheHitCount.incrementAndGet();
}
return result;
}
@@ -366,8 +407,53 @@ public class LDAPCache implements Closeable,
NamespaceChangeListener, ObjectChan
}
protected synchronized void clearCache() {
+ this.clearCacheCount.incrementAndGet();
userDnAndNamespace.clear();
userRoles.clear();
userPubkeys.clear();
}
+
+ public Map<String, String[]> listCachedUserDNAndNamespace() {
+ return Collections.unmodifiableMap(userDnAndNamespace);
+ }
+
+ public Map<String, String[]> listCachedUserPubkeys() {
+ return Collections.unmodifiableMap(userPubkeys);
+ }
+
+ public Map<String, String[]> listCachedUserRoles() {
+ return Collections.unmodifiableMap(userRoles);
+ }
+
+ public long getUserDNCacheHitCount() {
+ return this.userDNCacheHitCount.get();
+ }
+
+ public long getUserDNCacheMissCount() {
+ return this.userDNCacheMissCount.get();
+ }
+
+ public long getUserRolesCacheHitCount() {
+ return this.userRolesCacheHitCount.get();
+ }
+
+ public long getUserRolesCacheMissCount() {
+ return this.userRolesCacheMissCount.get();
+ }
+
+ public long getUserPubkeysCacheHitCount() {
+ return this.userPubkeysCacheHitCount.get();
+ }
+
+ public long getUserPubkeysCacheMissCount() {
+ return this.userPubkeysCacheMissCount.get();
+ }
+
+ public long getClearCacheCount() {
+ return this.clearCacheCount.get();
+ }
+
+ public long getId() {
+ return this.id;
+ }
}
diff --git
a/jaas/modules/src/main/java/org/apache/karaf/jaas/modules/ldap/LDAPLoginModule.java
b/jaas/modules/src/main/java/org/apache/karaf/jaas/modules/ldap/LDAPLoginModule.java
index 770e15e1a5..e64e36e3f9 100644
---
a/jaas/modules/src/main/java/org/apache/karaf/jaas/modules/ldap/LDAPLoginModule.java
+++
b/jaas/modules/src/main/java/org/apache/karaf/jaas/modules/ldap/LDAPLoginModule.java
@@ -42,11 +42,9 @@ import org.slf4j.LoggerFactory;
public class LDAPLoginModule extends AbstractKarafLoginModule {
private static Logger logger =
LoggerFactory.getLogger(LDAPLoginModule.class);
-
-
+
public void initialize(Subject subject, CallbackHandler callbackHandler,
Map<String, ?> sharedState, Map<String, ?> options) {
super.initialize(subject, callbackHandler, options);
- LDAPCache.clear();
}
public boolean login() throws LoginException {
diff --git
a/jaas/modules/src/main/java/org/apache/karaf/jaas/modules/ldap/LDAPPubkeyLoginModule.java
b/jaas/modules/src/main/java/org/apache/karaf/jaas/modules/ldap/LDAPPubkeyLoginModule.java
index b3772bfbd7..308763d8bc 100644
---
a/jaas/modules/src/main/java/org/apache/karaf/jaas/modules/ldap/LDAPPubkeyLoginModule.java
+++
b/jaas/modules/src/main/java/org/apache/karaf/jaas/modules/ldap/LDAPPubkeyLoginModule.java
@@ -44,7 +44,6 @@ public class LDAPPubkeyLoginModule extends
AbstractKarafLoginModule {
public void initialize(Subject subject, CallbackHandler callbackHandler,
Map<String, ?> sharedState, Map<String, ?> options) {
super.initialize(subject, callbackHandler, options);
- LDAPCache.clear();
}
public boolean login() throws LoginException {
diff --git
a/jaas/modules/src/test/java/org/apache/karaf/jaas/modules/ldap/LdapCacheTest.java
b/jaas/modules/src/test/java/org/apache/karaf/jaas/modules/ldap/LdapCacheTest.java
index b0185456d4..1568ac532d 100644
---
a/jaas/modules/src/test/java/org/apache/karaf/jaas/modules/ldap/LdapCacheTest.java
+++
b/jaas/modules/src/test/java/org/apache/karaf/jaas/modules/ldap/LdapCacheTest.java
@@ -73,6 +73,8 @@ public class LdapCacheTest extends AbstractLdapTestUnit {
@Test
public void testAdminLogin() throws Exception {
Properties options = ldapLoginModuleOptions();
+ validateMetrics(LDAPCache.getCache(new LDAPOptions(options)), 0, 0, 0,
0, 0, 0, 0);
+
LDAPLoginModule module = new LDAPLoginModule();
CallbackHandler cb = new NamePasswordCallbackHandler("admin",
"admin123");
Subject subject = new Subject();
@@ -90,20 +92,45 @@ public class LdapCacheTest extends AbstractLdapTestUnit {
assertTrue(module.logout());
assertEquals("Principals should be gone as the user has logged out",
0, subject.getPrincipals().size());
- LDAPCache ldapCache = new LDAPCache(new LDAPOptions(options));
- DirContext context = ldapCache.open();
- addUserToGroup(context, "cn=admin,ou=people,dc=example,dc=com",
"another");
- ldapCache.close();
-
- Thread.sleep(100);
+ try(LDAPCache ldapCache = LDAPCache.getCache(new
LDAPOptions(options))) {
+ // Cache clear is called on dirContext open (correctly)
+ validateMetrics(ldapCache, 0, 1, 0, 1, 0, 0, 1);
+
+ DirContext context = ldapCache.open();
+ addUserToGroup(context, "cn=admin,ou=people,dc=example,dc=com",
"another");
+ Thread.sleep(100);
+
+ for(int i=0; i < 10; i++) {
+ module = new LDAPLoginModule();
+ subject = new Subject();
+ module.initialize(subject, cb, null, options);
+ assertEquals("Precondition", 0,
subject.getPrincipals().size());
+ assertTrue(module.login());
+ assertTrue(module.commit());
+ assertEquals("Postcondition", 3,
subject.getPrincipals().size());
+
+ if(i == 0) {
+ // Cache is cleared when naming event detects change to
the tree
+ // due to group add above
+ validateMetrics(ldapCache, 0, 2, 0, 2, 0, 0, 2);
+ } else {
+ validateMetrics(ldapCache, i, 2, i, 2, 0, 0, 2);
+ }
+ }
+ }
+ }
- module = new LDAPLoginModule();
- subject = new Subject();
- module.initialize(subject, cb, null, options);
- assertEquals("Precondition", 0, subject.getPrincipals().size());
- assertTrue(module.login());
- assertTrue(module.commit());
- assertEquals("Postcondition", 3, subject.getPrincipals().size());
+ @Test
+ public void testLDAPCacheHashCode() throws Exception {
+ Properties options = ldapLoginModuleOptions();
+ LDAPCache cache1 = LDAPCache.getCache(new LDAPOptions(options));
+ LDAPCache cache2 = LDAPCache.getCache(new LDAPOptions(options));
+ assertEquals(cache1, cache2);
+ assertEquals(Integer.valueOf(cache1.hashCode()),
Integer.valueOf(cache2.hashCode()));
+
+ // Validate instance handling is correct
+ cache1.clearCache();
+ assertEquals(Long.valueOf(1),
Long.valueOf(cache2.getClearCacheCount()));
}
private void addUserToGroup(DirContext context, String userCn, String
group) throws NamingException {
@@ -128,4 +155,13 @@ public class LdapCacheTest extends AbstractLdapTestUnit {
return new Properties(file);
}
+ protected void validateMetrics(LDAPCache ldapCache, long userDNCacheHit,
long userDNCacheMiss, long userRoleCacheHit, long userRoleCacheMiss, long
userPubkeyCacheHit, long userPubkeyCacheMiss, long clearCache) {
+ assertEquals(Long.valueOf(userDNCacheHit),
Long.valueOf(ldapCache.getUserDNCacheHitCount()));
+ assertEquals(Long.valueOf(userDNCacheMiss),
Long.valueOf(ldapCache.getUserDNCacheMissCount()));
+ assertEquals(Long.valueOf(userRoleCacheHit),
Long.valueOf(ldapCache.getUserRolesCacheHitCount()));
+ assertEquals(Long.valueOf(userRoleCacheMiss),
Long.valueOf(ldapCache.getUserRolesCacheMissCount()));
+ assertEquals(Long.valueOf(userPubkeyCacheHit),
Long.valueOf(ldapCache.getUserPubkeysCacheHitCount()));
+ assertEquals(Long.valueOf(userPubkeyCacheMiss),
Long.valueOf(ldapCache.getUserPubkeysCacheMissCount()));
+ assertEquals(Long.valueOf(clearCache),
Long.valueOf(ldapCache.getClearCacheCount()));
+ }
}
diff --git
a/jaas/modules/src/test/resources/org/apache/karaf/jaas/modules/ldap/ldap.properties
b/jaas/modules/src/test/resources/org/apache/karaf/jaas/modules/ldap/ldap.properties
index 7916b1ea44..8c9f7b920e 100644
---
a/jaas/modules/src/test/resources/org/apache/karaf/jaas/modules/ldap/ldap.properties
+++
b/jaas/modules/src/test/resources/org/apache/karaf/jaas/modules/ldap/ldap.properties
@@ -18,6 +18,7 @@
################################################################################
debug=true
+disableCache=false
connection.url=ldap://127.0.0.1:portno
connection.username=uid=admin,ou=system
connection.password=secret
diff --git
a/jaas/modules/src/test/resources/org/apache/karaf/jaas/modules/ldap/ldap_badref.properties
b/jaas/modules/src/test/resources/org/apache/karaf/jaas/modules/ldap/ldap_badref.properties
index 07221d8bcc..f3e9afd767 100644
---
a/jaas/modules/src/test/resources/org/apache/karaf/jaas/modules/ldap/ldap_badref.properties
+++
b/jaas/modules/src/test/resources/org/apache/karaf/jaas/modules/ldap/ldap_badref.properties
@@ -18,6 +18,7 @@
################################################################################
debug=true
+disableCache=false
connection.url=ldap://127.0.0.1:portno
connection.username=uid=admin,ou=system
connection.password=secret
