diff --git a/plugin/trino-ldap-group-provider/pom.xml b/plugin/trino-ldap-group-provider/pom.xml index fa33f57fa007..0eeb45552d55 100644 --- a/plugin/trino-ldap-group-provider/pom.xml +++ b/plugin/trino-ldap-group-provider/pom.xml @@ -40,6 +40,16 @@ log + + io.airlift + units + + + + io.trino + trino-cache + + io.trino trino-plugin-toolkit diff --git a/plugin/trino-ldap-group-provider/src/main/java/io/trino/plugin/ldapgroup/LdapFilteringGroupProvider.java b/plugin/trino-ldap-group-provider/src/main/java/io/trino/plugin/ldapgroup/LdapFilteringGroupProvider.java index e63fda93b2f9..cc46d3b5ea3f 100644 --- a/plugin/trino-ldap-group-provider/src/main/java/io/trino/plugin/ldapgroup/LdapFilteringGroupProvider.java +++ b/plugin/trino-ldap-group-provider/src/main/java/io/trino/plugin/ldapgroup/LdapFilteringGroupProvider.java @@ -13,9 +13,12 @@ */ package io.trino.plugin.ldapgroup; +import com.google.common.cache.CacheLoader; +import com.google.common.cache.LoadingCache; import com.google.common.collect.ImmutableSet; import com.google.inject.Inject; import io.airlift.log.Logger; +import io.trino.cache.EvictableCacheBuilder; import io.trino.plugin.base.ldap.LdapClient; import io.trino.plugin.base.ldap.LdapQuery; import io.trino.spi.security.GroupProvider; @@ -24,6 +27,7 @@ import javax.naming.directory.Attribute; import javax.naming.directory.SearchResult; +import java.time.Duration; import java.util.Optional; import java.util.Set; @@ -42,6 +46,7 @@ public class LdapFilteringGroupProvider private final String groupBaseDN; private final String groupsNameAttribute; private final String combinedGroupSearchFilter; + private final LoadingCache> groupsCache; @Inject public LdapFilteringGroupProvider( @@ -61,18 +66,35 @@ public LdapFilteringGroupProvider( combinedGroupSearchFilter = filteringConfig.getLdapGroupsSearchFilter() .map(filter -> String.format("(&(%s)(%s={0}))", filter, groupsSearchMemberAttribute)) .orElse(String.format("(%s={0})", groupsSearchMemberAttribute)); + + groupsCache = EvictableCacheBuilder.newBuilder() + .expireAfterWrite(Duration.ofMillis(config.getGroupsCacheTtl().toMillis())) + .maximumSize(1000) + .shareResultsAndFailuresEvenIfDisabled() + .build(CacheLoader.from(this::loadGroups)); } /** * Perform an LDAP search for groups, fetching only the names, and returning the name of each group. * Filters groups by user membership AND filter expression {@link LdapFilteringGroupProviderConfig#getLdapGroupsSearchFilter()}. * If {@link LdapGroupProviderConfig#getLdapGroupsNameAttribute()} is missing from group document, fallback on full name. - * Swallows LDAP exceptions. + * Results are cached per user based on the configured cache TTL. * * @return Names of groups that the user is a member of */ @Override public Set getGroups(String user) + { + return groupsCache.getUnchecked(user); + } + + /** + * Loads groups from LDAP for the specified user. + * Swallows LDAP exceptions. + * + * @return Names of groups that the user is a member of + */ + private Set loadGroups(String user) { Optional userDistinguishedName; try { diff --git a/plugin/trino-ldap-group-provider/src/main/java/io/trino/plugin/ldapgroup/LdapGroupProviderConfig.java b/plugin/trino-ldap-group-provider/src/main/java/io/trino/plugin/ldapgroup/LdapGroupProviderConfig.java index 112deba26f06..bb489e0416a6 100644 --- a/plugin/trino-ldap-group-provider/src/main/java/io/trino/plugin/ldapgroup/LdapGroupProviderConfig.java +++ b/plugin/trino-ldap-group-provider/src/main/java/io/trino/plugin/ldapgroup/LdapGroupProviderConfig.java @@ -16,8 +16,12 @@ import io.airlift.configuration.Config; import io.airlift.configuration.ConfigDescription; import io.airlift.configuration.ConfigSecuritySensitive; +import io.airlift.units.Duration; +import io.airlift.units.MinDuration; import jakarta.validation.constraints.NotNull; +import java.util.concurrent.TimeUnit; + public class LdapGroupProviderConfig { private String ldapAdminUser; @@ -26,6 +30,7 @@ public class LdapGroupProviderConfig private String ldapUserSearchFilter = "(uid={0})"; private String ldapGroupsNameAttribute = "cn"; private boolean ldapUseGroupFilter; + private Duration groupsCacheTtl = new Duration(0, TimeUnit.SECONDS); @NotNull public String getLdapAdminUser() @@ -110,4 +115,19 @@ public LdapGroupProviderConfig setLdapUseGroupFilter(boolean ldapUseGroupFilter) this.ldapUseGroupFilter = ldapUseGroupFilter; return this; } + + @NotNull + @MinDuration("0ms") + public Duration getGroupsCacheTtl() + { + return groupsCacheTtl; + } + + @Config("ldap.group-cache-ttl") + @ConfigDescription("Duration to cache groups retrieved from LDAP. Set to 0 to disable caching, default value is 0s (disabled)") + public LdapGroupProviderConfig setGroupsCacheTtl(Duration groupsCacheTtl) + { + this.groupsCacheTtl = groupsCacheTtl; + return this; + } } diff --git a/plugin/trino-ldap-group-provider/src/main/java/io/trino/plugin/ldapgroup/LdapSingleQueryGroupProvider.java b/plugin/trino-ldap-group-provider/src/main/java/io/trino/plugin/ldapgroup/LdapSingleQueryGroupProvider.java index ec5778861648..d90da554b303 100644 --- a/plugin/trino-ldap-group-provider/src/main/java/io/trino/plugin/ldapgroup/LdapSingleQueryGroupProvider.java +++ b/plugin/trino-ldap-group-provider/src/main/java/io/trino/plugin/ldapgroup/LdapSingleQueryGroupProvider.java @@ -13,10 +13,13 @@ */ package io.trino.plugin.ldapgroup; +import com.google.common.cache.CacheLoader; +import com.google.common.cache.LoadingCache; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Streams; import com.google.inject.Inject; import io.airlift.log.Logger; +import io.trino.cache.EvictableCacheBuilder; import io.trino.plugin.base.ldap.LdapClient; import io.trino.plugin.base.ldap.LdapQuery; import io.trino.spi.security.GroupProvider; @@ -27,6 +30,7 @@ import javax.naming.ldap.LdapName; import javax.naming.ldap.Rdn; +import java.time.Duration; import java.util.List; import java.util.Optional; import java.util.Set; @@ -46,6 +50,7 @@ public class LdapSingleQueryGroupProvider private final String groupsNameAttribute; private final String userSearchFilter; private final String userMemberOfAttribute; + private final LoadingCache> groupsCache; @Inject public LdapSingleQueryGroupProvider(LdapClient ldapClient, LdapGroupProviderConfig config, LdapSingleQueryGroupProviderConfig singleQueryGroupProviderConfig) @@ -57,6 +62,12 @@ public LdapSingleQueryGroupProvider(LdapClient ldapClient, LdapGroupProviderConf this.groupsNameAttribute = config.getLdapGroupsNameAttribute(); this.userSearchFilter = config.getLdapUserSearchFilter(); this.userMemberOfAttribute = singleQueryGroupProviderConfig.getLdapUserMemberOfAttribute(); + + groupsCache = EvictableCacheBuilder.newBuilder() + .expireAfterWrite(Duration.ofMillis(config.getGroupsCacheTtl().toMillis())) + .maximumSize(1000) + .shareResultsAndFailuresEvenIfDisabled() + .build(CacheLoader.from(this::loadGroups)); } /** @@ -65,14 +76,26 @@ public LdapSingleQueryGroupProvider(LdapClient ldapClient, LdapGroupProviderConf * If multiple users match the search, the first one is used. * If the requested group attribute is missing from the LDAP document, it will be ignored * (i.e., the function will not error). - * Swallows all LDAP exceptions. + * Results are cached per user based on the configured cache TTL. * * @param user Username of user, used with filter expression {@link LdapGroupProviderConfig#getLdapUserSearchFilter()}. * @return The relative domain name of all the values of attribute - * {@link LdapSingleQueryGroupProviderConfig#getLdapUserMemberOfAttribute()}. + * {@link LdapSingleQueryGroupProviderConfig#getLdapUserMemberOfAttribute()}. */ @Override public Set getGroups(String user) + { + return groupsCache.getUnchecked(user); + } + + /** + * Loads groups from LDAP for the specified user. + * Swallows all LDAP exceptions. + * + * @return The relative domain name of all the values of attribute + * {@link LdapSingleQueryGroupProviderConfig#getLdapUserMemberOfAttribute()}. + */ + private Set loadGroups(String user) { try { return ldapClient.executeLdapQuery(ldapAdminUser, ldapAdminPassword, diff --git a/plugin/trino-ldap-group-provider/src/test/java/io/trino/plugin/ldapgroup/TestLdapGroupProviderConfig.java b/plugin/trino-ldap-group-provider/src/test/java/io/trino/plugin/ldapgroup/TestLdapGroupProviderConfig.java index 8af622db3ff0..8d1361a7d7b1 100644 --- a/plugin/trino-ldap-group-provider/src/test/java/io/trino/plugin/ldapgroup/TestLdapGroupProviderConfig.java +++ b/plugin/trino-ldap-group-provider/src/test/java/io/trino/plugin/ldapgroup/TestLdapGroupProviderConfig.java @@ -13,6 +13,7 @@ */ package io.trino.plugin.ldapgroup; +import io.airlift.units.Duration; import org.junit.jupiter.api.Test; import java.util.Map; @@ -32,7 +33,8 @@ public void testDefaults() .setLdapUserBaseDN(null) .setLdapUserSearchFilter("(uid={0})") .setLdapGroupsNameAttribute("cn") - .setLdapUseGroupFilter(false)); + .setLdapUseGroupFilter(false) + .setGroupsCacheTtl(Duration.valueOf("0m"))); } @Test @@ -44,7 +46,8 @@ public void testExplicitPropertyMappings() "ldap.user-base-dn", "dc=trino,dc=io", "ldap.user-search-filter", "(accountName={0})", "ldap.group-name-attribute", "groupName", - "ldap.use-group-filter", "true"); + "ldap.use-group-filter", "true", + "ldap.group-cache-ttl", "10m"); LdapGroupProviderConfig expected = new LdapGroupProviderConfig() .setLdapAdminUser("cn=admin,dc=trino,dc=io") @@ -52,7 +55,8 @@ public void testExplicitPropertyMappings() .setLdapUserBaseDN("dc=trino,dc=io") .setLdapUserSearchFilter("(accountName={0})") .setLdapGroupsNameAttribute("groupName") - .setLdapUseGroupFilter(true); + .setLdapUseGroupFilter(true) + .setGroupsCacheTtl(Duration.valueOf("10m")); assertFullMapping(properties, expected); } diff --git a/plugin/trino-ldap-group-provider/src/test/java/io/trino/plugin/ldapgroup/TestLdapGroupProviderIntegration.java b/plugin/trino-ldap-group-provider/src/test/java/io/trino/plugin/ldapgroup/TestLdapGroupProviderIntegration.java index 0d9bc1414869..88cb1bbad1b7 100644 --- a/plugin/trino-ldap-group-provider/src/test/java/io/trino/plugin/ldapgroup/TestLdapGroupProviderIntegration.java +++ b/plugin/trino-ldap-group-provider/src/test/java/io/trino/plugin/ldapgroup/TestLdapGroupProviderIntegration.java @@ -36,6 +36,7 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import static java.util.concurrent.TimeUnit.MILLISECONDS; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.TestInstance.Lifecycle.PER_CLASS; import static org.junit.jupiter.api.parallel.ExecutionMode.CONCURRENT; @@ -267,6 +268,60 @@ private void assertGetGroupsConcurrently(ConfigBuilder configBuilder) latch.await(); } + @Test + public void testGroupsCacheWithTtl() + { + for (ConfigBuilder configBuilder : CONFIG_BUILDERS) { + // Test with cache enabled (10 minute TTL) + Map config = configBuilder.apply(new HashMap<>(baseConfig)); + config.put("ldap.group-cache-ttl", "10m"); + GroupProvider groupsProvider = factory.create(config); + + // First call - should query LDAP + long start = System.nanoTime(); + Set groups1 = groupsProvider.getGroups("alicea"); + long firstCallTime = System.nanoTime() - start; + assertThat(groups1).containsAll(ImmutableSet.of("clients", "qualityAssurance", "developers")); + + // Second call - should be cached (much faster) + start = System.nanoTime(); + Set groups2 = groupsProvider.getGroups("alicea"); + long cachedCallTime = System.nanoTime() - start; + assertThat(groups2).isEqualTo(groups1); + + // Cached call should be significantly faster (at least 10x faster as a rough heuristic) + // Note: This is a heuristic test - actual performance may vary + assertThat(cachedCallTime).isLessThan(firstCallTime / 10); + } + } + + @Test + public void testGroupsCacheExpiration() + throws InterruptedException + { + for (ConfigBuilder configBuilder : CONFIG_BUILDERS) { + // Test with very short cache TTL (100ms) + Map config = configBuilder.apply(new HashMap<>(baseConfig)); + config.put("ldap.group-cache-ttl", "100ms"); + GroupProvider groupsProvider = factory.create(config); + + // First call + Set groups1 = groupsProvider.getGroups("alicea"); + assertThat(groups1).containsAll(ImmutableSet.of("clients", "qualityAssurance", "developers")); + + // Immediate second call should be cached + Set groups2 = groupsProvider.getGroups("alicea"); + assertThat(groups2).isEqualTo(groups1); + + // Wait for cache to expire + MILLISECONDS.sleep(150); + + // Call after expiration should query LDAP again (result should still be correct) + Set groups3 = groupsProvider.getGroups("alicea"); + assertThat(groups3).containsAll(ImmutableSet.of("clients", "qualityAssurance", "developers")); + } + } + @FunctionalInterface public interface ConfigBuilder {