From 9bb7efc9f71f525e47d06a60ee21ce12a9b9cc47 Mon Sep 17 00:00:00 2001 From: David Lin Date: Fri, 8 Dec 2023 21:38:57 -0600 Subject: [PATCH] add flush cache for individual user --- .../security/OpenSearchSecurityPlugin.java | 3 +- .../security/auth/BackendRegistry.java | 23 ++++++ .../dlic/rest/api/FlushCacheApiAction.java | 75 ++++++++++++------- .../rest/api/SecurityApiDependencies.java | 10 ++- .../dlic/rest/api/SecurityRestApiActions.java | 7 +- .../api/AbstractApiActionValidationTest.java | 3 +- .../dlic/rest/api/FlushCacheApiTest.java | 10 +++ ...SecurityConfigApiActionValidationTest.java | 8 +- 8 files changed, 105 insertions(+), 34 deletions(-) diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index 3c04816c32..2f52294831 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -584,7 +584,8 @@ public List getRestHandlers( Objects.requireNonNull(auditLog), sks, Objects.requireNonNull(userService), - sslCertReloadEnabled + sslCertReloadEnabled, + backendRegistry ) ); log.debug("Added {} rest handler(s)", handlers.size()); diff --git a/src/main/java/org/opensearch/security/auth/BackendRegistry.java b/src/main/java/org/opensearch/security/auth/BackendRegistry.java index 3ab9a2afc9..d452b3612e 100644 --- a/src/main/java/org/opensearch/security/auth/BackendRegistry.java +++ b/src/main/java/org/opensearch/security/auth/BackendRegistry.java @@ -166,6 +166,29 @@ public void invalidateCache() { restRoleCache.invalidateAll(); } + public void invalidateUserCache(String username) { + if (username == null || username.isEmpty()) { + log.debug("No username given, not invalidating user cache."); + return; + } + + // Invalidate entries in the userCache by iterating over the keys and matching the username. + userCache.asMap().keySet().stream() + .filter(authCreds -> username.equals(authCreds.getUsername())) + .forEach(userCache::invalidate); + + // Invalidate entries in the restImpersonationCache directly since it uses the username as the key. + restImpersonationCache.invalidate(username); + + // Invalidate entries in the restRoleCache by iterating over the keys and matching the username. + restRoleCache.asMap().keySet().stream() + .filter(user -> username.equals(user.getName())) + .forEach(restRoleCache::invalidate); + + // If the user isn't found it still says this, which could be bad + log.debug("Invalidated cache for user {}", username); + } + @Subscribe public void onDynamicConfigModelChanged(DynamicConfigModel dcm) { diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/FlushCacheApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/FlushCacheApiAction.java index d6f5e24d7d..d015dfc412 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/FlushCacheApiAction.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/FlushCacheApiAction.java @@ -20,6 +20,7 @@ import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.inject.Inject; import org.opensearch.core.action.ActionListener; +import org.opensearch.rest.RestChannel; import org.opensearch.rest.RestRequest; import org.opensearch.rest.RestRequest.Method; import org.opensearch.security.action.configupdate.ConfigUpdateAction; @@ -41,7 +42,8 @@ public class FlushCacheApiAction extends AbstractApiAction { new Route(Method.DELETE, "/cache"), new Route(Method.GET, "/cache"), new Route(Method.PUT, "/cache"), - new Route(Method.POST, "/cache") + new Route(Method.POST, "/cache"), + new Route(Method.DELETE, "/cache/user/{username}") ) ); @@ -64,36 +66,57 @@ private void flushCacheApiRequestHandlers(RequestHandler.RequestHandlersBuilder requestHandlersBuilder.allMethodsNotImplemented() .override( Method.DELETE, - (channel, request, client) -> client.execute( - ConfigUpdateAction.INSTANCE, - new ConfigUpdateRequest(CType.lcStringValues().toArray(new String[0])), - new ActionListener<>() { - - @Override - public void onResponse(ConfigUpdateResponse configUpdateResponse) { - if (configUpdateResponse.hasFailures()) { - LOGGER.error("Cannot flush cache due to", configUpdateResponse.failures().get(0)); - internalSeverError( - channel, - "Cannot flush cache due to " + configUpdateResponse.failures().get(0).getMessage() + "." - ); - return; - } - LOGGER.debug("cache flushed successfully"); - ok(channel, "Cache flushed successfully."); - } - - @Override - public void onFailure(final Exception e) { - LOGGER.error("Cannot flush cache due to", e); - internalSeverError(channel, "Cannot flush cache due to " + e.getMessage() + "."); - } + (channel, request, client) -> { + if (request.path().contains("/user/")) { + // Extract the username from the request + final String username = request.param("username"); + // Validate and handle user-specific cache invalidation + handleUserCacheInvalidation(channel, username); + } + else + { + client.execute( + ConfigUpdateAction.INSTANCE, + new ConfigUpdateRequest(CType.lcStringValues().toArray(new String[0])), + new ActionListener<>() { + + @Override + public void onResponse(ConfigUpdateResponse configUpdateResponse) { + if (configUpdateResponse.hasFailures()) { + LOGGER.error("Cannot flush cache due to", configUpdateResponse.failures().get(0)); + internalSeverError( + channel, + "Cannot flush cache due to " + configUpdateResponse.failures().get(0).getMessage() + "." + ); + return; + } + LOGGER.debug("cache flushed successfully"); + ok(channel, "Cache flushed successfully."); + } + + @Override + public void onFailure(final Exception e) { + LOGGER.error("Cannot flush cache due to", e); + internalSeverError(channel, "Cannot flush cache due to " + e.getMessage() + "."); + } + } + ); } - ) + } ); } + private void handleUserCacheInvalidation(RestChannel channel, String username) { + if (username == null || username.isEmpty()) { + internalSeverError(channel, "No username provided for cache invalidation."); + return; + } + // Use BackendRegistry's method to invalidate cache for the specific user + securityApiDependencies.backendRegistry().invalidateUserCache(username); + ok(channel, "Cache invalidated for user: " + username); + } + @Override protected CType getConfigType() { return null; diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/SecurityApiDependencies.java b/src/main/java/org/opensearch/security/dlic/rest/api/SecurityApiDependencies.java index 498230423f..ead030afcc 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/SecurityApiDependencies.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/SecurityApiDependencies.java @@ -13,6 +13,7 @@ import org.opensearch.common.settings.Settings; import org.opensearch.security.auditlog.AuditLog; +import org.opensearch.security.auth.BackendRegistry; import org.opensearch.security.configuration.AdminDNs; import org.opensearch.security.configuration.ConfigurationRepository; import org.opensearch.security.privileges.PrivilegesEvaluator; @@ -23,6 +24,7 @@ public class SecurityApiDependencies { private final ConfigurationRepository configurationRepository; private final RestApiPrivilegesEvaluator restApiPrivilegesEvaluator; private final RestApiAdminPrivilegesEvaluator restApiAdminPrivilegesEvaluator; + private final BackendRegistry backendRegistry; private final AuditLog auditLog; private final Settings settings; @@ -35,7 +37,8 @@ public SecurityApiDependencies( final RestApiPrivilegesEvaluator restApiPrivilegesEvaluator, final RestApiAdminPrivilegesEvaluator restApiAdminPrivilegesEvaluator, final AuditLog auditLog, - final Settings settings + final Settings settings, + final BackendRegistry backendRegistry ) { this.adminDNs = adminDNs; this.configurationRepository = configurationRepository; @@ -44,6 +47,7 @@ public SecurityApiDependencies( this.restApiAdminPrivilegesEvaluator = restApiAdminPrivilegesEvaluator; this.auditLog = auditLog; this.settings = settings; + this.backendRegistry = backendRegistry; } public AdminDNs adminDNs() { @@ -74,6 +78,10 @@ public Settings settings() { return settings; } + public BackendRegistry backendRegistry() { + return backendRegistry; + } + public String securityIndexName() { return settings().get(ConfigConstants.SECURITY_CONFIG_INDEX_NAME, ConfigConstants.OPENDISTRO_SECURITY_DEFAULT_CONFIG_INDEX); } diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/SecurityRestApiActions.java b/src/main/java/org/opensearch/security/dlic/rest/api/SecurityRestApiActions.java index b0d46f8774..b358ad83ae 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/SecurityRestApiActions.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/SecurityRestApiActions.java @@ -21,6 +21,7 @@ import org.opensearch.rest.RestController; import org.opensearch.rest.RestHandler; import org.opensearch.security.auditlog.AuditLog; +import org.opensearch.security.auth.BackendRegistry; import org.opensearch.security.configuration.AdminDNs; import org.opensearch.security.configuration.ConfigurationRepository; import org.opensearch.security.privileges.PrivilegesEvaluator; @@ -47,7 +48,8 @@ public static Collection getHandler( final AuditLog auditLog, final SecurityKeyStore securityKeyStore, final UserService userService, - final boolean certificatesReloadEnabled + final boolean certificatesReloadEnabled, + final BackendRegistry backendRegistry ) { final var securityApiDependencies = new SecurityApiDependencies( adminDns, @@ -61,7 +63,8 @@ public static Collection getHandler( settings.getAsBoolean(SECURITY_RESTAPI_ADMIN_ENABLED, false) ), auditLog, - settings + settings, + backendRegistry ); return List.of( new InternalUsersApiAction(clusterService, threadPool, userService, securityApiDependencies), diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/AbstractApiActionValidationTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/AbstractApiActionValidationTest.java index f2df09549f..2a943a41ce 100644 --- a/src/test/java/org/opensearch/security/dlic/rest/api/AbstractApiActionValidationTest.java +++ b/src/test/java/org/opensearch/security/dlic/rest/api/AbstractApiActionValidationTest.java @@ -71,7 +71,8 @@ public void setup() { null, restApiAdminPrivilegesEvaluator, null, - Settings.EMPTY + Settings.EMPTY, + null ); } diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/FlushCacheApiTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/FlushCacheApiTest.java index ee75ccc984..04d94b179f 100644 --- a/src/test/java/org/opensearch/security/dlic/rest/api/FlushCacheApiTest.java +++ b/src/test/java/org/opensearch/security/dlic/rest/api/FlushCacheApiTest.java @@ -42,6 +42,9 @@ public void testFlushCache() throws Exception { rh.keystore = "restapi/kirk-keystore.jks"; rh.sendAdminCertificate = true; + // Username to test cache invalidation + String username = "testuser"; + // GET HttpResponse response = rh.executeGetRequest(ENDPOINT); Assert.assertEquals(HttpStatus.SC_NOT_IMPLEMENTED, response.getStatusCode()); @@ -66,5 +69,12 @@ public void testFlushCache() throws Exception { settings = Settings.builder().loadFromSource(response.getBody(), XContentType.JSON).build(); Assert.assertEquals(settings.get("message"), "Cache flushed successfully."); + // DELETE request for a specific user's cache + String userEndpoint = ENDPOINT + "/user/" + username; + response = rh.executeDeleteRequest(userEndpoint, new Header[0]); + Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); + settings = Settings.builder().loadFromSource(response.getBody(), XContentType.JSON).build(); + Assert.assertEquals(settings.get("message"), "Cache invalidated for user: " + username); + } } diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiActionValidationTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiActionValidationTest.java index a6832457b3..94a812d014 100644 --- a/src/test/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiActionValidationTest.java +++ b/src/test/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiActionValidationTest.java @@ -30,7 +30,7 @@ public void accessHandlerForDefaultSettings() { final var securityConfigApiAction = new SecurityConfigApiAction( clusterService, threadPool, - new SecurityApiDependencies(null, configurationRepository, null, null, restApiAdminPrivilegesEvaluator, null, Settings.EMPTY) + new SecurityApiDependencies(null, configurationRepository, null, null, restApiAdminPrivilegesEvaluator, null, Settings.EMPTY, null) ); assertTrue(securityConfigApiAction.accessHandler(FakeRestRequest.builder().withMethod(RestRequest.Method.GET).build())); assertFalse(securityConfigApiAction.accessHandler(FakeRestRequest.builder().withMethod(RestRequest.Method.PUT).build())); @@ -49,7 +49,8 @@ public void accessHandlerForUnsupportedSetting() { null, restApiAdminPrivilegesEvaluator, null, - Settings.builder().put(SECURITY_UNSUPPORTED_RESTAPI_ALLOW_SECURITYCONFIG_MODIFICATION, true).build() + Settings.builder().put(SECURITY_UNSUPPORTED_RESTAPI_ALLOW_SECURITYCONFIG_MODIFICATION, true).build(), + null ) ); assertTrue(securityConfigApiAction.accessHandler(FakeRestRequest.builder().withMethod(RestRequest.Method.GET).build())); @@ -70,7 +71,8 @@ public void accessHandlerForRestAdmin() { null, restApiAdminPrivilegesEvaluator, null, - Settings.builder().put(SECURITY_RESTAPI_ADMIN_ENABLED, true).build() + Settings.builder().put(SECURITY_RESTAPI_ADMIN_ENABLED, true).build(), + null ) ); assertTrue(securityConfigApiAction.accessHandler(FakeRestRequest.builder().withMethod(RestRequest.Method.GET).build()));