From bc81ba7263eac673d97d5ab356657b069ccc29ec Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Sun, 11 May 2025 15:45:31 -0400 Subject: [PATCH 1/5] Register cluster settings listener for plugins.security.cache.ttl_minutes Signed-off-by: Craig Perkins --- .../security/OpenSearchSecurityPlugin.java | 3 +- .../security/auth/BackendRegistry.java | 40 ++++++++++++++++++- .../security/support/SecuritySettings.java | 8 ++++ 3 files changed, 49 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index aa2982f449..e21359071a 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -1139,6 +1139,7 @@ public Collection createComponents( final XFFResolver xffResolver = new XFFResolver(threadPool); backendRegistry = new BackendRegistry(settings, adminDns, xffResolver, auditLog, threadPool); + backendRegistry.registerClusterSettingsChangeListener(clusterService.getClusterSettings()); tokenManager = new SecurityTokenManager(cs, threadPool, userService); final CompatConfig compatConfig = new CompatConfig(environment, transportPassiveAuthSetting); @@ -1484,7 +1485,7 @@ public List> getSettings() { settings.add(Setting.boolSetting(ConfigConstants.SECURITY_DISABLED, false, Property.NodeScope, Property.Filtered)); - settings.add(Setting.intSetting(ConfigConstants.SECURITY_CACHE_TTL_MINUTES, 60, 0, Property.NodeScope, Property.Filtered)); + settings.add(SecuritySettings.CACHE_TTL_SETTING); // Security settings.add( diff --git a/src/main/java/org/opensearch/security/auth/BackendRegistry.java b/src/main/java/org/opensearch/security/auth/BackendRegistry.java index 4f65425a6a..61bc96a503 100644 --- a/src/main/java/org/opensearch/security/auth/BackendRegistry.java +++ b/src/main/java/org/opensearch/security/auth/BackendRegistry.java @@ -30,6 +30,7 @@ import java.net.InetSocketAddress; import java.util.Collection; import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -50,6 +51,7 @@ import org.apache.logging.log4j.Logger; import org.opensearch.OpenSearchSecurityException; +import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.concurrent.ThreadContext; import org.opensearch.core.common.transport.TransportAddress; @@ -66,6 +68,7 @@ import org.opensearch.security.securityconf.DynamicConfigModel; import org.opensearch.security.support.ConfigConstants; import org.opensearch.security.support.HostAndCidrMatcher; +import org.opensearch.security.support.SecuritySettings; import org.opensearch.security.user.AuthCredentials; import org.opensearch.security.user.User; import org.opensearch.threadpool.ThreadPool; @@ -98,12 +101,19 @@ public class BackendRegistry { private final AuditLog auditLog; private final ThreadPool threadPool; private final UserInjector userInjector; - private final int ttlInMin; + private int ttlInMin; private Cache userCache; // rest standard private Cache restImpersonationCache; // used for rest impersonation private Cache> restRoleCache; // private void createCaches() { + Map existingUserCache = new HashMap<>(); + Map existingRestImpersonationCache = new HashMap<>(); + Map> existingRestRoleCache = new HashMap<>(); + if (userCache != null) { + existingUserCache.putAll(userCache.asMap()); + } + userCache = CacheBuilder.newBuilder() .expireAfterWrite(ttlInMin, TimeUnit.MINUTES) .removalListener(new RemovalListener() { @@ -114,6 +124,14 @@ public void onRemoval(RemovalNotification notification) { }) .build(); + if (!existingUserCache.isEmpty()) { + userCache.putAll(existingUserCache); + } + + if (restImpersonationCache != null) { + existingRestImpersonationCache.putAll(restImpersonationCache.asMap()); + } + restImpersonationCache = CacheBuilder.newBuilder() .expireAfterWrite(ttlInMin, TimeUnit.MINUTES) .removalListener(new RemovalListener() { @@ -124,6 +142,14 @@ public void onRemoval(RemovalNotification notification) { }) .build(); + if (!existingRestImpersonationCache.isEmpty()) { + restImpersonationCache.putAll(existingRestImpersonationCache); + } + + if (restRoleCache != null) { + restRoleCache.putAll(existingRestRoleCache); + } + restRoleCache = CacheBuilder.newBuilder() .expireAfterWrite(ttlInMin, TimeUnit.MINUTES) .removalListener(new RemovalListener>() { @@ -134,6 +160,18 @@ public void onRemoval(RemovalNotification> notification) { }) .build(); + if (!existingRestRoleCache.isEmpty()) { + restRoleCache.putAll(existingRestRoleCache); + } + } + + public void registerClusterSettingsChangeListener(final ClusterSettings clusterSettings) { + clusterSettings.addSettingsUpdateConsumer(SecuritySettings.CACHE_TTL_SETTING, newTtlInMin -> { + log.info("Detected change in settings, cluster setting for TTL is {}", newTtlInMin); + + ttlInMin = newTtlInMin; + createCaches(); + }); } public BackendRegistry( diff --git a/src/main/java/org/opensearch/security/support/SecuritySettings.java b/src/main/java/org/opensearch/security/support/SecuritySettings.java index 2a8bb079e8..7c92fb9c6e 100644 --- a/src/main/java/org/opensearch/security/support/SecuritySettings.java +++ b/src/main/java/org/opensearch/security/support/SecuritySettings.java @@ -28,4 +28,12 @@ public class SecuritySettings { Setting.Property.Dynamic ); // Not filtered + public static final Setting CACHE_TTL_SETTING = Setting.intSetting( + ConfigConstants.SECURITY_CACHE_TTL_MINUTES, + 60, + 0, + Setting.Property.NodeScope, + Setting.Property.Filtered + ); // Not filtered + } From 63faa5b7df8d6ae8d1f9c2cb3c709de519c466f4 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Sun, 11 May 2025 16:15:39 -0400 Subject: [PATCH 2/5] Add test for verifying change in setting Signed-off-by: Craig Perkins --- .../security/SecuritySettingsTests.java | 49 +++++++++++++++++++ .../security/auth/BackendRegistry.java | 4 ++ .../security/rest/SecurityHealthAction.java | 5 ++ .../security/support/SecuritySettings.java | 2 +- 4 files changed, 59 insertions(+), 1 deletion(-) create mode 100644 src/integrationTest/java/org/opensearch/security/SecuritySettingsTests.java diff --git a/src/integrationTest/java/org/opensearch/security/SecuritySettingsTests.java b/src/integrationTest/java/org/opensearch/security/SecuritySettingsTests.java new file mode 100644 index 0000000000..9fdb73a091 --- /dev/null +++ b/src/integrationTest/java/org/opensearch/security/SecuritySettingsTests.java @@ -0,0 +1,49 @@ +package org.opensearch.security; + +import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; +import org.awaitility.Awaitility; +import org.junit.ClassRule; +import org.junit.Test; +import org.junit.runner.RunWith; + +import org.opensearch.security.support.ConfigConstants; +import org.opensearch.test.framework.cluster.ClusterManager; +import org.opensearch.test.framework.cluster.LocalCluster; +import org.opensearch.test.framework.cluster.TestRestClient; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL; +import static org.opensearch.test.framework.TestSecurityConfig.User.USER_ADMIN; + +@RunWith(com.carrotsearch.randomizedtesting.RandomizedRunner.class) +@ThreadLeakScope(ThreadLeakScope.Scope.NONE) +public class SecuritySettingsTests { + + @ClassRule + public static LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.SINGLENODE) + .anonymousAuth(false) + .authc(AUTHC_HTTPBASIC_INTERNAL) + .users(USER_ADMIN) + .build(); + + @Test + public void testTtlInMinCanBeUpdatedDynamically() { + try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) { + TestRestClient.HttpResponse updateResponse = client.putJson("_cluster/settings", """ + { + "persistent": { }, + "transient": { + "plugins.security.cache.ttl_minutes": "1440" + } + } + """); + + updateResponse.assertStatusCode(200); + Awaitility.await().untilAsserted(() -> { + TestRestClient.HttpResponse response = client.get("_plugins/_security/health"); + assertThat(response.getBody(), containsString("\"" + ConfigConstants.SECURITY_CACHE_TTL_MINUTES + "\":1440")); + }); + } + } +} diff --git a/src/main/java/org/opensearch/security/auth/BackendRegistry.java b/src/main/java/org/opensearch/security/auth/BackendRegistry.java index 61bc96a503..833d31408c 100644 --- a/src/main/java/org/opensearch/security/auth/BackendRegistry.java +++ b/src/main/java/org/opensearch/security/auth/BackendRegistry.java @@ -203,6 +203,10 @@ public boolean isInitialized() { return initialized; } + public int getTtlInMin() { + return ttlInMin; + } + public void invalidateCache() { userCache.invalidateAll(); restImpersonationCache.invalidateAll(); diff --git a/src/main/java/org/opensearch/security/rest/SecurityHealthAction.java b/src/main/java/org/opensearch/security/rest/SecurityHealthAction.java index 892f48d892..e351bdbee2 100644 --- a/src/main/java/org/opensearch/security/rest/SecurityHealthAction.java +++ b/src/main/java/org/opensearch/security/rest/SecurityHealthAction.java @@ -49,6 +49,7 @@ import static org.opensearch.security.dlic.rest.support.Utils.PLUGIN_ROUTE_PREFIX; import static org.opensearch.security.dlic.rest.support.Utils.addDeprecatedRoutesPrefix; import static org.opensearch.security.dlic.rest.support.Utils.addRoutesPrefix; +import static org.opensearch.security.support.SecuritySettings.CACHE_TTL_SETTING; public class SecurityHealthAction extends BaseRestHandler { private static final List routes = addRoutesPrefix( @@ -108,6 +109,10 @@ public void accept(RestChannel channel) throws Exception { builder.field("message", message); builder.field("mode", mode); builder.field("status", status); + // TODO Consider a separate settings API + builder.startObject("settings"); + builder.field(CACHE_TTL_SETTING.getKey(), registry.getTtlInMin()); + builder.endObject(); builder.endObject(); response = new BytesRestResponse(restStatus, builder); diff --git a/src/main/java/org/opensearch/security/support/SecuritySettings.java b/src/main/java/org/opensearch/security/support/SecuritySettings.java index 7c92fb9c6e..c61145811f 100644 --- a/src/main/java/org/opensearch/security/support/SecuritySettings.java +++ b/src/main/java/org/opensearch/security/support/SecuritySettings.java @@ -33,7 +33,7 @@ public class SecuritySettings { 60, 0, Setting.Property.NodeScope, - Setting.Property.Filtered + Setting.Property.Dynamic ); // Not filtered } From fdfe49f8ef5cea593a6fd88e2ad6fbb358d684e4 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Sun, 11 May 2025 16:18:44 -0400 Subject: [PATCH 3/5] Add License header Signed-off-by: Craig Perkins --- .../org/opensearch/security/SecuritySettingsTests.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/integrationTest/java/org/opensearch/security/SecuritySettingsTests.java b/src/integrationTest/java/org/opensearch/security/SecuritySettingsTests.java index 9fdb73a091..a7d42bc94e 100644 --- a/src/integrationTest/java/org/opensearch/security/SecuritySettingsTests.java +++ b/src/integrationTest/java/org/opensearch/security/SecuritySettingsTests.java @@ -1,3 +1,11 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ package org.opensearch.security; import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; From 120e83dd4f83eccfe885d525b34630c1bb1612f0 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Mon, 12 May 2025 12:14:00 -0400 Subject: [PATCH 4/5] Empty cache after change Signed-off-by: Craig Perkins --- .../security/auth/BackendRegistry.java | 28 ------------------- 1 file changed, 28 deletions(-) diff --git a/src/main/java/org/opensearch/security/auth/BackendRegistry.java b/src/main/java/org/opensearch/security/auth/BackendRegistry.java index 833d31408c..340f7a955d 100644 --- a/src/main/java/org/opensearch/security/auth/BackendRegistry.java +++ b/src/main/java/org/opensearch/security/auth/BackendRegistry.java @@ -30,7 +30,6 @@ import java.net.InetSocketAddress; import java.util.Collection; import java.util.Collections; -import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -107,13 +106,6 @@ public class BackendRegistry { private Cache> restRoleCache; // private void createCaches() { - Map existingUserCache = new HashMap<>(); - Map existingRestImpersonationCache = new HashMap<>(); - Map> existingRestRoleCache = new HashMap<>(); - if (userCache != null) { - existingUserCache.putAll(userCache.asMap()); - } - userCache = CacheBuilder.newBuilder() .expireAfterWrite(ttlInMin, TimeUnit.MINUTES) .removalListener(new RemovalListener() { @@ -124,14 +116,6 @@ public void onRemoval(RemovalNotification notification) { }) .build(); - if (!existingUserCache.isEmpty()) { - userCache.putAll(existingUserCache); - } - - if (restImpersonationCache != null) { - existingRestImpersonationCache.putAll(restImpersonationCache.asMap()); - } - restImpersonationCache = CacheBuilder.newBuilder() .expireAfterWrite(ttlInMin, TimeUnit.MINUTES) .removalListener(new RemovalListener() { @@ -142,14 +126,6 @@ public void onRemoval(RemovalNotification notification) { }) .build(); - if (!existingRestImpersonationCache.isEmpty()) { - restImpersonationCache.putAll(existingRestImpersonationCache); - } - - if (restRoleCache != null) { - restRoleCache.putAll(existingRestRoleCache); - } - restRoleCache = CacheBuilder.newBuilder() .expireAfterWrite(ttlInMin, TimeUnit.MINUTES) .removalListener(new RemovalListener>() { @@ -159,10 +135,6 @@ public void onRemoval(RemovalNotification> notification) { } }) .build(); - - if (!existingRestRoleCache.isEmpty()) { - restRoleCache.putAll(existingRestRoleCache); - } } public void registerClusterSettingsChangeListener(final ClusterSettings clusterSettings) { From a084c1146a4a0b179da4b68e6a43aa07951c4238 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Mon, 12 May 2025 12:26:19 -0400 Subject: [PATCH 5/5] Add CHANGELOG entry Signed-off-by: Craig Perkins --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d63477e6a..3fbac8559d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ### Added - [Resource Permissions] Introduces Centralized Resource Access Control Framework ([#5281](https://github.com/opensearch-project/security/pull/5281)) - Github workflow for changelog verification ([#5318](https://github.com/opensearch-project/security/pull/5318)) +- Register cluster settings listener for `plugins.security.cache.ttl_minutes` ([#5324](https://github.com/opensearch-project/security/pull/5324)) ### Changed