From ad255cbdd6d63271a77bae5646672cec4d46e588 Mon Sep 17 00:00:00 2001 From: David Lin Date: Fri, 8 Dec 2023 21:38:57 -0600 Subject: [PATCH 1/5] add flush cache for individual user Signed-off-by: David Lin --- .../security/auth/BackendRegistry.java | 23 ++++++ .../dlic/rest/api/FlushCacheApiAction.java | 55 +++++++++++-- .../rest/api/SecurityApiDependencies.java | 10 ++- .../dlic/rest/api/SecurityRestApiActions.java | 4 +- .../api/AbstractApiActionValidationTest.java | 3 +- .../dlic/rest/api/FlushCacheApiTest.java | 80 +++++++++++++++++++ ...SecurityConfigApiActionValidationTest.java | 8 +- 7 files changed, 170 insertions(+), 13 deletions(-) create mode 100644 src/test/java/org/opensearch/security/dlic/rest/api/FlushCacheApiTest.java diff --git a/src/main/java/org/opensearch/security/auth/BackendRegistry.java b/src/main/java/org/opensearch/security/auth/BackendRegistry.java index 97c060be35..f2840241ba 100644 --- a/src/main/java/org/opensearch/security/auth/BackendRegistry.java +++ b/src/main/java/org/opensearch/security/auth/BackendRegistry.java @@ -168,6 +168,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 2f579ecbd9..2e648e07f5 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,11 +66,33 @@ 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<>() { - + (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 onResponse(ConfigUpdateResponse configUpdateResponse) { if (configUpdateResponse.hasFailures()) { @@ -83,17 +107,34 @@ public void onResponse(ConfigUpdateResponse configUpdateResponse) { 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() + "."); + } @Override public void onFailure(final Exception e) { LOGGER.error("Cannot flush cache due to", e); internalServerError(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 8ccf494d3d..bd3744da8f 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.hasher.PasswordHasher; @@ -63,7 +64,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, passwordHasher), 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 b91374e725..1baf66be13 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 @@ -77,7 +77,8 @@ public void setup() { null, restApiAdminPrivilegesEvaluator, null, - Settings.EMPTY + Settings.EMPTY, + null ); passwordHasher = PasswordHasherFactory.createPasswordHasher( 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 new file mode 100644 index 0000000000..04d94b179f --- /dev/null +++ b/src/test/java/org/opensearch/security/dlic/rest/api/FlushCacheApiTest.java @@ -0,0 +1,80 @@ +/* + * 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. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.security.dlic.rest.api; + +import org.apache.hc.core5.http.Header; +import org.apache.http.HttpStatus; +import org.junit.Assert; +import org.junit.Test; + +import org.opensearch.common.settings.Settings; +import org.opensearch.common.xcontent.XContentType; +import org.opensearch.security.test.helper.rest.RestHelper.HttpResponse; + +import static org.opensearch.security.OpenSearchSecurityPlugin.PLUGINS_PREFIX; + +public class FlushCacheApiTest extends AbstractRestApiUnitTest { + private final String ENDPOINT; + + protected String getEndpointPrefix() { + return PLUGINS_PREFIX; + } + + public FlushCacheApiTest() { + ENDPOINT = getEndpointPrefix() + "/api/cache"; + } + + @Test + public void testFlushCache() throws Exception { + + setup(); + + // Only DELETE is allowed for flush cache + 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()); + Settings settings = Settings.builder().loadFromSource(response.getBody(), XContentType.JSON).build(); + Assert.assertEquals(settings.get("message"), "Method GET not supported for this action."); + + // PUT + response = rh.executePutRequest(ENDPOINT, "{}", new Header[0]); + Assert.assertEquals(HttpStatus.SC_NOT_IMPLEMENTED, response.getStatusCode()); + settings = Settings.builder().loadFromSource(response.getBody(), XContentType.JSON).build(); + Assert.assertEquals(settings.get("message"), "Method PUT not supported for this action."); + + // POST + response = rh.executePostRequest(ENDPOINT, "{}", new Header[0]); + Assert.assertEquals(HttpStatus.SC_NOT_IMPLEMENTED, response.getStatusCode()); + settings = Settings.builder().loadFromSource(response.getBody(), XContentType.JSON).build(); + Assert.assertEquals(settings.get("message"), "Method POST not supported for this action."); + + // DELETE + response = rh.executeDeleteRequest(ENDPOINT, 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 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())); From 7cbd113d68213513fcb42b1210281a1e25d0a3fb Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Mon, 18 Dec 2023 11:51:17 -0500 Subject: [PATCH 2/5] Add test that demonstrates flush cache for particular user Signed-off-by: Craig Perkins --- .../http/DirectoryInformationTrees.java | 84 ++++++++ .../http/LdapAuthenticationCacheTest.java | 193 ++++++++++++++++++ .../framework/ldap/EmbeddedLDAPServer.java | 8 + .../test/framework/ldap/LdapServer.java | 3 +- .../configupdate/ConfigUpdateRequest.java | 20 ++ .../TransportConfigUpdateAction.java | 11 +- .../security/auth/BackendRegistry.java | 31 +-- .../dlic/rest/api/FlushCacheApiAction.java | 53 ++--- ...SecurityConfigApiActionValidationTest.java | 11 +- 9 files changed, 363 insertions(+), 51 deletions(-) create mode 100644 src/integrationTest/java/org/opensearch/security/http/LdapAuthenticationCacheTest.java diff --git a/src/integrationTest/java/org/opensearch/security/http/DirectoryInformationTrees.java b/src/integrationTest/java/org/opensearch/security/http/DirectoryInformationTrees.java index 3f9c220923..bf8144d0bd 100644 --- a/src/integrationTest/java/org/opensearch/security/http/DirectoryInformationTrees.java +++ b/src/integrationTest/java/org/opensearch/security/http/DirectoryInformationTrees.java @@ -38,6 +38,7 @@ class DirectoryInformationTrees { public static final String CN_GROUP_ADMIN = "admin"; public static final String CN_GROUP_CREW = "crew"; + public static final String CN_GROUP_ENTERPRISE = "enterprise"; public static final String CN_GROUP_BRIDGE = "bridge"; public static final String USER_SEARCH = "(uid={0})"; @@ -120,4 +121,87 @@ class DirectoryInformationTrees { .classes("groupofuniquenames", "top") .buildRecord() .buildLdif(); + + static final LdifData LDIF_DATA_UPDATED_BACKEND_ROLES = new LdifBuilder().root("o=test.org") + .dc("TEST") + .classes("top", "domain") + .newRecord(DN_PEOPLE_TEST_ORG) + .ou("people") + .classes("organizationalUnit", "top") + .newRecord(DN_OPEN_SEARCH_PEOPLE_TEST_ORG) + .classes("inetOrgPerson") + .cn("Open Search") + .sn("Search") + .uid(USER_OPENS) + .userPassword(PASSWORD_OPEN_SEARCH) + .mail("open.search@example.com") + .ou("Human Resources") + .newRecord(DN_CAPTAIN_SPOCK_PEOPLE_TEST_ORG) + .classes("inetOrgPerson") + .cn("Captain Spock") + .sn(USER_SPOCK) + .uid(USER_SPOCK) + .userPassword(PASSWORD_SPOCK) + .mail("spock@example.com") + .ou("Human Resources") + .newRecord(DN_KIRK_PEOPLE_TEST_ORG) + .classes("inetOrgPerson") + .cn("Kirk") + .sn("Kirk") + .uid(USER_KIRK) + .userPassword(PASSWORD_KIRK) + .mail("spock@example.com") + .ou("Human Resources") + .newRecord(DN_CHRISTPHER_PEOPLE_TEST_ORG) + .classes("inetOrgPerson") + .cn("Christpher") + .sn("Christpher") + .uid("christpher") + .userPassword(PASSWORD_CHRISTPHER) + .mail("christpher@example.com") + .ou("Human Resources") + .newRecord(DN_LEONARD_PEOPLE_TEST_ORG) + .classes("inetOrgPerson") + .cn("Leonard") + .sn("Leonard") + .uid(USER_LEONARD) + .userPassword(PASSWORD_LEONARD) + .mail("leonard@example.com") + .ou("Human Resources") + .newRecord(DN_JEAN_PEOPLE_TEST_ORG) + .classes("inetOrgPerson") + .cn("Jean") + .sn("Jean") + .uid(USER_JEAN) + .userPassword(PASSWORD_JEAN) + .mail("jean@example.com") + .ou("Human Resources") + .newRecord(DN_GROUPS_TEST_ORG) + .ou("groups") + .cn("groupsRoot") + .classes("groupofuniquenames", "top") + .newRecord("cn=admin,ou=groups,o=test.org") + .ou("groups") + .cn(CN_GROUP_ADMIN) + .uniqueMember(DN_KIRK_PEOPLE_TEST_ORG) + .classes("groupofuniquenames", "top") + .newRecord("cn=crew,ou=groups,o=test.org") + .ou("groups") + .cn(CN_GROUP_CREW) + .uniqueMember(DN_CAPTAIN_SPOCK_PEOPLE_TEST_ORG) + .uniqueMember(DN_CHRISTPHER_PEOPLE_TEST_ORG) + .uniqueMember(DN_BRIDGE_GROUPS_TEST_ORG) + .classes("groupofuniquenames", "top") + .newRecord("cn=enterprise,ou=groups,o=test.org") + .cn(CN_GROUP_ENTERPRISE) + .uniqueMember(DN_KIRK_PEOPLE_TEST_ORG) + .uniqueMember(DN_CAPTAIN_SPOCK_PEOPLE_TEST_ORG) + .classes("groupofuniquenames", "top") + .newRecord(DN_BRIDGE_GROUPS_TEST_ORG) + .ou("groups") + .cn(CN_GROUP_BRIDGE) + .uniqueMember(DN_JEAN_PEOPLE_TEST_ORG) + .classes("groupofuniquenames", "top") + .buildRecord() + .buildLdif(); } diff --git a/src/integrationTest/java/org/opensearch/security/http/LdapAuthenticationCacheTest.java b/src/integrationTest/java/org/opensearch/security/http/LdapAuthenticationCacheTest.java new file mode 100644 index 0000000000..a0376e93ac --- /dev/null +++ b/src/integrationTest/java/org/opensearch/security/http/LdapAuthenticationCacheTest.java @@ -0,0 +1,193 @@ +/* +* 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.http; + +import java.util.List; +import java.util.Map; + +import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.junit.ClassRule; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.RuleChain; +import org.junit.runner.RunWith; + +import org.opensearch.security.support.ConfigConstants; +import org.opensearch.test.framework.AuthorizationBackend; +import org.opensearch.test.framework.AuthzDomain; +import org.opensearch.test.framework.LdapAuthenticationConfigBuilder; +import org.opensearch.test.framework.LdapAuthorizationConfigBuilder; +import org.opensearch.test.framework.RolesMapping; +import org.opensearch.test.framework.TestSecurityConfig; +import org.opensearch.test.framework.TestSecurityConfig.AuthcDomain; +import org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AuthenticationBackend; +import org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.HttpAuthenticator; +import org.opensearch.test.framework.certificate.TestCertificates; +import org.opensearch.test.framework.cluster.ClusterManager; +import org.opensearch.test.framework.cluster.LocalCluster; +import org.opensearch.test.framework.cluster.TestRestClient; +import org.opensearch.test.framework.ldap.EmbeddedLDAPServer; +import org.opensearch.test.framework.log.LogsRule; + +import static org.hamcrest.CoreMatchers.not; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.contains; +import static org.opensearch.security.http.DirectoryInformationTrees.CN_GROUP_ADMIN; +import static org.opensearch.security.http.DirectoryInformationTrees.DN_GROUPS_TEST_ORG; +import static org.opensearch.security.http.DirectoryInformationTrees.DN_OPEN_SEARCH_PEOPLE_TEST_ORG; +import static org.opensearch.security.http.DirectoryInformationTrees.DN_PEOPLE_TEST_ORG; +import static org.opensearch.security.http.DirectoryInformationTrees.LDIF_DATA; +import static org.opensearch.security.http.DirectoryInformationTrees.LDIF_DATA_UPDATED_BACKEND_ROLES; +import static org.opensearch.security.http.DirectoryInformationTrees.PASSWORD_KIRK; +import static org.opensearch.security.http.DirectoryInformationTrees.PASSWORD_OPEN_SEARCH; +import static org.opensearch.security.http.DirectoryInformationTrees.PASSWORD_SPOCK; +import static org.opensearch.security.http.DirectoryInformationTrees.USERNAME_ATTRIBUTE; +import static org.opensearch.security.http.DirectoryInformationTrees.USER_KIRK; +import static org.opensearch.security.http.DirectoryInformationTrees.USER_SEARCH; +import static org.opensearch.security.http.DirectoryInformationTrees.USER_SPOCK; +import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_ADMIN_ENABLED; +import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_ROLES_ENABLED; +import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL; +import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.BASIC_AUTH_DOMAIN_ORDER; +import static org.opensearch.test.framework.TestSecurityConfig.Role.ALL_ACCESS; + +/** +* Test uses plain (non TLS) connection between OpenSearch and LDAP server. +*/ +@RunWith(com.carrotsearch.randomizedtesting.RandomizedRunner.class) +@ThreadLeakScope(ThreadLeakScope.Scope.NONE) +public class LdapAuthenticationCacheTest { + + private static final Logger log = LogManager.getLogger(LdapAuthenticationCacheTest.class); + + private static final TestSecurityConfig.User ADMIN_USER = new TestSecurityConfig.User("admin").roles(ALL_ACCESS); + + private static final TestCertificates TEST_CERTIFICATES = new TestCertificates(); + + public static final EmbeddedLDAPServer embeddedLDAPServer = new EmbeddedLDAPServer( + TEST_CERTIFICATES.getRootCertificateData(), + TEST_CERTIFICATES.getLdapCertificateData(), + LDIF_DATA + ); + + public static LocalCluster cluster = new LocalCluster.Builder().testCertificates(TEST_CERTIFICATES) + .clusterManager(ClusterManager.SINGLENODE) + .anonymousAuth(false) + .nodeSettings( + Map.of( + ConfigConstants.SECURITY_AUTHCZ_REST_IMPERSONATION_USERS + "." + ADMIN_USER.getName(), + List.of(USER_KIRK), + SECURITY_RESTAPI_ROLES_ENABLED, + List.of("user_" + ADMIN_USER.getName() + "__" + ALL_ACCESS.getName()), + SECURITY_RESTAPI_ADMIN_ENABLED, + true + ) + ) + .authc( + new AuthcDomain("ldap", BASIC_AUTH_DOMAIN_ORDER + 1, true).httpAuthenticator(new HttpAuthenticator("basic").challenge(false)) + .backend( + new AuthenticationBackend("ldap").config( + () -> LdapAuthenticationConfigBuilder.config() + // this port is available when embeddedLDAPServer is already started, therefore Supplier interface is used to + // postpone + // execution of the code in this block. + .enableSsl(false) + .enableStartTls(false) + .hosts(List.of("localhost:" + embeddedLDAPServer.getLdapNonTlsPort())) + .bindDn(DN_OPEN_SEARCH_PEOPLE_TEST_ORG) + .password(PASSWORD_OPEN_SEARCH) + .userBase(DN_PEOPLE_TEST_ORG) + .userSearch(USER_SEARCH) + .usernameAttribute(USERNAME_ATTRIBUTE) + .build() + ) + ) + ) + .authc(AUTHC_HTTPBASIC_INTERNAL) + .users(ADMIN_USER) + .rolesMapping(new RolesMapping(ALL_ACCESS).backendRoles(CN_GROUP_ADMIN)) + .authz( + new AuthzDomain("ldap_roles").httpEnabled(true) + .transportEnabled(true) + .authorizationBackend( + new AuthorizationBackend("ldap").config( + () -> new LdapAuthorizationConfigBuilder().hosts(List.of("localhost:" + embeddedLDAPServer.getLdapNonTlsPort())) + .enableSsl(false) + .bindDn(DN_OPEN_SEARCH_PEOPLE_TEST_ORG) + .password(PASSWORD_OPEN_SEARCH) + .userBase(DN_PEOPLE_TEST_ORG) + .userSearch(USER_SEARCH) + .usernameAttribute(USERNAME_ATTRIBUTE) + .roleBase(DN_GROUPS_TEST_ORG) + .roleSearch("(uniqueMember={0})") + .userRoleAttribute(null) + .userRoleName("disabled") + .roleName("cn") + .resolveNestedRoles(true) + .build() + ) + ) + ) + .build(); + + @ClassRule + public static RuleChain ruleChain = RuleChain.outerRule(embeddedLDAPServer).around(cluster); + + @Rule + public LogsRule logsRule = new LogsRule("com.amazon.dlic.auth.ldap.backend.LDAPAuthenticationBackend"); + + @Test + public void shouldAuthenticateUserWithLdap_positive() { + try (TestRestClient client = cluster.getRestClient(USER_SPOCK, PASSWORD_SPOCK)) { + TestRestClient.HttpResponse response = client.getAuthInfo(); + + response.assertStatusCode(200); + + assertThat(response.getTextArrayFromJsonBody("/backend_roles"), contains("crew")); + assertThat(response.getTextArrayFromJsonBody("/backend_roles"), not(contains("enterprise"))); + } + + try (TestRestClient client = cluster.getRestClient(USER_KIRK, PASSWORD_KIRK)) { + TestRestClient.HttpResponse response = client.getAuthInfo(); + + response.assertStatusCode(200); + + assertThat(response.getTextArrayFromJsonBody("/backend_roles"), contains("admin")); + assertThat(response.getTextArrayFromJsonBody("/backend_roles"), not(contains("enterprise"))); + } + + embeddedLDAPServer.loadLdifData(LDIF_DATA_UPDATED_BACKEND_ROLES); + + try (TestRestClient client = cluster.getRestClient(ADMIN_USER)) { + TestRestClient.HttpResponse response = client.delete("_plugins/_security/api/cache/user/spock"); + + response.assertStatusCode(200); + } + + try (TestRestClient client = cluster.getRestClient(USER_SPOCK, PASSWORD_SPOCK)) { + TestRestClient.HttpResponse response = client.getAuthInfo(); + + response.assertStatusCode(200); + + assertThat(response.getTextArrayFromJsonBody("/backend_roles"), contains("enterprise", "crew")); + } + + try (TestRestClient client = cluster.getRestClient(USER_KIRK, PASSWORD_KIRK)) { + TestRestClient.HttpResponse response = client.getAuthInfo(); + + response.assertStatusCode(200); + + assertThat(response.getTextArrayFromJsonBody("/backend_roles"), contains("admin")); + assertThat(response.getTextArrayFromJsonBody("/backend_roles"), not(contains("enterprise"))); + } + } +} diff --git a/src/integrationTest/java/org/opensearch/test/framework/ldap/EmbeddedLDAPServer.java b/src/integrationTest/java/org/opensearch/test/framework/ldap/EmbeddedLDAPServer.java index 583a0cdaeb..0a1d6a658a 100755 --- a/src/integrationTest/java/org/opensearch/test/framework/ldap/EmbeddedLDAPServer.java +++ b/src/integrationTest/java/org/opensearch/test/framework/ldap/EmbeddedLDAPServer.java @@ -46,6 +46,14 @@ protected void after() { } } + public void loadLdifData(LdifData ldifData) { + try { + server.loadLdifData(ldifData); + } catch (Exception e) { + throw new RuntimeException("Cannot reload LDIF data.", e); + } + } + public int getLdapNonTlsPort() { return server.getLdapNonTlsPort(); } diff --git a/src/integrationTest/java/org/opensearch/test/framework/ldap/LdapServer.java b/src/integrationTest/java/org/opensearch/test/framework/ldap/LdapServer.java index dece74f1e5..7cd7ebecd9 100644 --- a/src/integrationTest/java/org/opensearch/test/framework/ldap/LdapServer.java +++ b/src/integrationTest/java/org/opensearch/test/framework/ldap/LdapServer.java @@ -212,7 +212,8 @@ public void stop() throws InterruptedException { } } - private void loadLdifData(LdifData ldifData) throws Exception { + public void loadLdifData(LdifData ldifData) throws Exception { + server.clear(); try (LDIFReader r = new LDIFReader(new BufferedReader(new StringReader(ldifData.getContent())))) { Entry entry; while ((entry = r.readEntry()) != null) { diff --git a/src/main/java/org/opensearch/security/action/configupdate/ConfigUpdateRequest.java b/src/main/java/org/opensearch/security/action/configupdate/ConfigUpdateRequest.java index d4e860569d..3a83fb551a 100644 --- a/src/main/java/org/opensearch/security/action/configupdate/ConfigUpdateRequest.java +++ b/src/main/java/org/opensearch/security/action/configupdate/ConfigUpdateRequest.java @@ -37,9 +37,12 @@ public class ConfigUpdateRequest extends BaseNodesRequest { private String[] configTypes; + private String[] entityNames; + public ConfigUpdateRequest(StreamInput in) throws IOException { super(in); this.configTypes = in.readStringArray(); + this.entityNames = in.readOptionalStringArray(); } public ConfigUpdateRequest() { @@ -51,10 +54,17 @@ public ConfigUpdateRequest(String[] configTypes) { setConfigTypes(configTypes); } + public ConfigUpdateRequest(String configType, String[] entityNames) { + this(); + setConfigTypes(new String[] { configType }); + setEntityNames(entityNames); + } + @Override public void writeTo(final StreamOutput out) throws IOException { super.writeTo(out); out.writeStringArray(configTypes); + out.writeOptionalStringArray(entityNames); } public String[] getConfigTypes() { @@ -65,10 +75,20 @@ public void setConfigTypes(final String[] configTypes) { this.configTypes = configTypes; } + public String[] getEntityNames() { + return entityNames; + } + + public void setEntityNames(final String[] entityNames) { + this.entityNames = entityNames; + } + @Override public ActionRequestValidationException validate() { if (configTypes == null || configTypes.length == 0) { return new ActionRequestValidationException(); + } else if (configTypes.length > 1 && (entityNames != null && entityNames.length > 1)) { + return new ActionRequestValidationException(); } return null; } diff --git a/src/main/java/org/opensearch/security/action/configupdate/TransportConfigUpdateAction.java b/src/main/java/org/opensearch/security/action/configupdate/TransportConfigUpdateAction.java index 6f1f99a434..7fb3f13298 100644 --- a/src/main/java/org/opensearch/security/action/configupdate/TransportConfigUpdateAction.java +++ b/src/main/java/org/opensearch/security/action/configupdate/TransportConfigUpdateAction.java @@ -27,6 +27,7 @@ package org.opensearch.security.action.configupdate; import java.io.IOException; +import java.util.Arrays; import java.util.List; import org.apache.logging.log4j.LogManager; @@ -125,8 +126,14 @@ protected ConfigUpdateResponse newResponse( @Override protected ConfigUpdateNodeResponse nodeOperation(final NodeConfigUpdateRequest request) { - boolean didReload = configurationRepository.reloadConfiguration(CType.fromStringValues((request.request.getConfigTypes()))); - if (didReload) { + if (request.request.getConfigTypes() != null + && request.request.getEntityNames() != null + && request.request.getConfigTypes().length == 1 + && Arrays.asList(request.request.getConfigTypes()).contains(CType.INTERNALUSERS.toLCString()) + && request.request.getEntityNames().length > 0) { + backendRegistry.get().invalidateUserCache(request.request.getEntityNames()); + } else { + configurationRepository.reloadConfiguration(CType.fromStringValues((request.request.getConfigTypes()))); backendRegistry.get().invalidateCache(); } return new ConfigUpdateNodeResponse(clusterService.localNode(), request.request.getConfigTypes(), null); diff --git a/src/main/java/org/opensearch/security/auth/BackendRegistry.java b/src/main/java/org/opensearch/security/auth/BackendRegistry.java index f2840241ba..2fd9a40209 100644 --- a/src/main/java/org/opensearch/security/auth/BackendRegistry.java +++ b/src/main/java/org/opensearch/security/auth/BackendRegistry.java @@ -28,6 +28,7 @@ import java.net.InetAddress; import java.net.InetSocketAddress; +import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.HashSet; @@ -168,27 +169,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."); + public void invalidateUserCache(String[] usernames) { + if (usernames == null || usernames.length == 0) { + log.debug("No usernames given, not invalidating user cache."); return; } - + + List usernamesAsList = Arrays.asList(usernames); + // Invalidate entries in the userCache by iterating over the keys and matching the username. - userCache.asMap().keySet().stream() - .filter(authCreds -> username.equals(authCreds.getUsername())) + userCache.asMap() + .keySet() + .stream() + .filter(authCreds -> usernamesAsList.contains(authCreds.getUsername())) .forEach(userCache::invalidate); - + // Invalidate entries in the restImpersonationCache directly since it uses the username as the key. - restImpersonationCache.invalidate(username); - + restImpersonationCache.invalidateAll(usernamesAsList); + // 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); - + restRoleCache.asMap().keySet().stream().filter(user -> usernamesAsList.contains(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); + log.debug("Invalidated cache for users {}", String.join(", ", usernames)); } @Subscribe 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 2e648e07f5..aeba7c69d1 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,7 +20,6 @@ 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; @@ -63,22 +62,21 @@ public List routes() { } private void flushCacheApiRequestHandlers(RequestHandler.RequestHandlersBuilder requestHandlersBuilder) { - requestHandlersBuilder.allMethodsNotImplemented() - .override( - Method.DELETE, - (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<>() { + requestHandlersBuilder.allMethodsNotImplemented().override(Method.DELETE, (channel, request, client) -> { + final ConfigUpdateRequest configUpdateRequest; + if (request.path().contains("/user/")) { + // Extract the username from the request + final String username = request.param("username"); + if (username == null || username.isEmpty()) { + internalSeverError(channel, "No username provided for cache invalidation."); + return; + } + // Validate and handle user-specific cache invalidation + configUpdateRequest = new ConfigUpdateRequest(CType.INTERNALUSERS.toLCString(), new String[] { username }); + } else { + configUpdateRequest = new ConfigUpdateRequest(CType.lcStringValues().toArray(new String[0])); + } + client.execute(ConfigUpdateAction.INSTANCE, configUpdateRequest, new ActionListener<>() { @Override public void onResponse(ConfigUpdateResponse configUpdateResponse) { @@ -118,21 +116,8 @@ public void onFailure(final Exception e) { internalServerError(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 @@ -142,6 +127,8 @@ protected CType getConfigType() { @Override protected void consumeParameters(final RestRequest request) { - // not needed + if (request.path().contains("/user/")) { + request.param("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 94a812d014..573c027482 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,16 @@ public void accessHandlerForDefaultSettings() { final var securityConfigApiAction = new SecurityConfigApiAction( clusterService, threadPool, - new SecurityApiDependencies(null, configurationRepository, null, null, restApiAdminPrivilegesEvaluator, null, Settings.EMPTY, null) + 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())); From 5a158e49aec0a64b0ad3b4fef46b45d64be9c1ce Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Mon, 18 Dec 2023 11:53:08 -0500 Subject: [PATCH 3/5] Remove backendRegistry as api dependency Signed-off-by: Craig Perkins --- .../dlic/rest/api/SecurityApiDependencies.java | 10 +--------- .../security/dlic/rest/api/SecurityRestApiActions.java | 4 +--- 2 files changed, 2 insertions(+), 12 deletions(-) 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 ead030afcc..498230423f 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,7 +13,6 @@ 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; @@ -24,7 +23,6 @@ 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; @@ -37,8 +35,7 @@ public SecurityApiDependencies( final RestApiPrivilegesEvaluator restApiPrivilegesEvaluator, final RestApiAdminPrivilegesEvaluator restApiAdminPrivilegesEvaluator, final AuditLog auditLog, - final Settings settings, - final BackendRegistry backendRegistry + final Settings settings ) { this.adminDNs = adminDNs; this.configurationRepository = configurationRepository; @@ -47,7 +44,6 @@ public SecurityApiDependencies( this.restApiAdminPrivilegesEvaluator = restApiAdminPrivilegesEvaluator; this.auditLog = auditLog; this.settings = settings; - this.backendRegistry = backendRegistry; } public AdminDNs adminDNs() { @@ -78,10 +74,6 @@ 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 bd3744da8f..8ccf494d3d 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,7 +21,6 @@ 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.hasher.PasswordHasher; @@ -64,8 +63,7 @@ public static Collection getHandler( settings.getAsBoolean(SECURITY_RESTAPI_ADMIN_ENABLED, false) ), auditLog, - settings, - backendRegistry + settings ); return List.of( new InternalUsersApiAction(clusterService, threadPool, userService, securityApiDependencies, passwordHasher), From 8f5dc6fb679ec7ec74069e8558a6b5e08dc8f268 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Mon, 18 Dec 2023 11:57:34 -0500 Subject: [PATCH 4/5] Update test Signed-off-by: Craig Perkins --- .../api/AbstractApiActionValidationTest.java | 3 +-- .../SecurityConfigApiActionValidationTest.java | 17 +++-------------- 2 files changed, 4 insertions(+), 16 deletions(-) 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 1baf66be13..b91374e725 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 @@ -77,8 +77,7 @@ public void setup() { null, restApiAdminPrivilegesEvaluator, null, - Settings.EMPTY, - null + Settings.EMPTY ); passwordHasher = PasswordHasherFactory.createPasswordHasher( 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 573c027482..a6832457b3 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,16 +30,7 @@ public void accessHandlerForDefaultSettings() { final var securityConfigApiAction = new SecurityConfigApiAction( clusterService, threadPool, - new SecurityApiDependencies( - null, - configurationRepository, - null, - null, - restApiAdminPrivilegesEvaluator, - null, - Settings.EMPTY, - null - ) + new SecurityApiDependencies(null, configurationRepository, null, null, restApiAdminPrivilegesEvaluator, null, Settings.EMPTY) ); assertTrue(securityConfigApiAction.accessHandler(FakeRestRequest.builder().withMethod(RestRequest.Method.GET).build())); assertFalse(securityConfigApiAction.accessHandler(FakeRestRequest.builder().withMethod(RestRequest.Method.PUT).build())); @@ -58,8 +49,7 @@ public void accessHandlerForUnsupportedSetting() { null, restApiAdminPrivilegesEvaluator, null, - Settings.builder().put(SECURITY_UNSUPPORTED_RESTAPI_ALLOW_SECURITYCONFIG_MODIFICATION, true).build(), - null + Settings.builder().put(SECURITY_UNSUPPORTED_RESTAPI_ALLOW_SECURITYCONFIG_MODIFICATION, true).build() ) ); assertTrue(securityConfigApiAction.accessHandler(FakeRestRequest.builder().withMethod(RestRequest.Method.GET).build())); @@ -80,8 +70,7 @@ public void accessHandlerForRestAdmin() { null, restApiAdminPrivilegesEvaluator, null, - Settings.builder().put(SECURITY_RESTAPI_ADMIN_ENABLED, true).build(), - null + Settings.builder().put(SECURITY_RESTAPI_ADMIN_ENABLED, true).build() ) ); assertTrue(securityConfigApiAction.accessHandler(FakeRestRequest.builder().withMethod(RestRequest.Method.GET).build())); From 18cd88320451df62ed1afb550363e82e6b86220a Mon Sep 17 00:00:00 2001 From: Prabhas Kurapati Date: Tue, 16 Jul 2024 07:47:13 -0700 Subject: [PATCH 5/5] test changes Signed-off-by: Prabhas Kurapati --- ...DefaultApiAvailabilityIntegrationTest.java | 18 ------ .../api/FlushCacheApiIntegrationTest.java | 57 +++++++++++++++++++ .../http/LdapAuthenticationCacheTest.java | 8 +-- .../dlic/rest/api/FlushCacheApiAction.java | 57 +++++++------------ 4 files changed, 77 insertions(+), 63 deletions(-) create mode 100644 src/integrationTest/java/org/opensearch/security/api/FlushCacheApiIntegrationTest.java diff --git a/src/integrationTest/java/org/opensearch/security/api/DefaultApiAvailabilityIntegrationTest.java b/src/integrationTest/java/org/opensearch/security/api/DefaultApiAvailabilityIntegrationTest.java index f3b701dc38..7ac2262899 100644 --- a/src/integrationTest/java/org/opensearch/security/api/DefaultApiAvailabilityIntegrationTest.java +++ b/src/integrationTest/java/org/opensearch/security/api/DefaultApiAvailabilityIntegrationTest.java @@ -16,7 +16,6 @@ import org.opensearch.test.framework.cluster.TestRestClient; import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.core.Is.is; import static org.opensearch.security.api.PatchPayloadHelper.patch; import static org.opensearch.security.api.PatchPayloadHelper.replaceOp; @@ -93,23 +92,6 @@ private void verifyAuthInfoApi(final TestRestClient client) throws Exception { } - @Test - public void flushCache() throws Exception { - withUser(NEW_USER, client -> { - forbidden(() -> client.get(apiPath("cache"))); - forbidden(() -> client.postJson(apiPath("cache"), EMPTY_BODY)); - forbidden(() -> client.putJson(apiPath("cache"), EMPTY_BODY)); - forbidden(() -> client.delete(apiPath("cache"))); - }); - withUser(ADMIN_USER_NAME, localCluster.getAdminCertificate(), client -> { - notImplemented(() -> client.get(apiPath("cache"))); - notImplemented(() -> client.postJson(apiPath("cache"), EMPTY_BODY)); - notImplemented(() -> client.putJson(apiPath("cache"), EMPTY_BODY)); - final var response = ok(() -> client.delete(apiPath("cache"))); - assertThat(response.getBody(), response.getTextFromJsonBody("/message"), is("Cache flushed successfully.")); - }); - } - @Test public void reloadSSLCertsNotAvailable() throws Exception { withUser(NEW_USER, client -> { diff --git a/src/integrationTest/java/org/opensearch/security/api/FlushCacheApiIntegrationTest.java b/src/integrationTest/java/org/opensearch/security/api/FlushCacheApiIntegrationTest.java new file mode 100644 index 0000000000..048078badb --- /dev/null +++ b/src/integrationTest/java/org/opensearch/security/api/FlushCacheApiIntegrationTest.java @@ -0,0 +1,57 @@ +/* + * 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. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.security.api; + +import org.junit.Test; + +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.MatcherAssert.assertThat; + +public class FlushCacheApiIntegrationTest extends AbstractApiIntegrationTest { + private final static String TEST_USER = "testuser"; + + private String cachePath() { + return super.apiPath("cache"); + } + + private String cachePath(String user) { + return super.apiPath("cache", "user", user); + } + + @Test + public void testFlushCache() throws Exception { + withUser(NEW_USER, client -> { + forbidden(() -> client.get(cachePath())); + forbidden(() -> client.postJson(cachePath(), EMPTY_BODY)); + forbidden(() -> client.putJson(cachePath(), EMPTY_BODY)); + forbidden(() -> client.delete(cachePath())); + forbidden(() -> client.delete(cachePath(TEST_USER))); + }); + withUser(ADMIN_USER_NAME, localCluster.getAdminCertificate(), client -> { + notImplemented(() -> client.get(cachePath())); + notImplemented(() -> client.postJson(cachePath(), EMPTY_BODY)); + notImplemented(() -> client.putJson(cachePath(), EMPTY_BODY)); + final var deleteAllCacheResponse = ok(() -> client.delete(cachePath())); + assertThat( + deleteAllCacheResponse.getBody(), + deleteAllCacheResponse.getTextFromJsonBody("/message"), + is("Cache flushed successfully.") + ); + final var deleteUserCacheResponse = ok(() -> client.delete(cachePath(TEST_USER))); + assertThat( + deleteUserCacheResponse.getBody(), + deleteAllCacheResponse.getTextFromJsonBody("/message"), + is("Cache invalidated for user: " + TEST_USER) + ); + }); + } +} diff --git a/src/integrationTest/java/org/opensearch/security/http/LdapAuthenticationCacheTest.java b/src/integrationTest/java/org/opensearch/security/http/LdapAuthenticationCacheTest.java index a0376e93ac..2cc94b13e9 100644 --- a/src/integrationTest/java/org/opensearch/security/http/LdapAuthenticationCacheTest.java +++ b/src/integrationTest/java/org/opensearch/security/http/LdapAuthenticationCacheTest.java @@ -13,8 +13,6 @@ import java.util.Map; import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; import org.junit.ClassRule; import org.junit.Rule; import org.junit.Test; @@ -26,7 +24,6 @@ import org.opensearch.test.framework.AuthzDomain; import org.opensearch.test.framework.LdapAuthenticationConfigBuilder; import org.opensearch.test.framework.LdapAuthorizationConfigBuilder; -import org.opensearch.test.framework.RolesMapping; import org.opensearch.test.framework.TestSecurityConfig; import org.opensearch.test.framework.TestSecurityConfig.AuthcDomain; import org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AuthenticationBackend; @@ -67,8 +64,6 @@ @ThreadLeakScope(ThreadLeakScope.Scope.NONE) public class LdapAuthenticationCacheTest { - private static final Logger log = LogManager.getLogger(LdapAuthenticationCacheTest.class); - private static final TestSecurityConfig.User ADMIN_USER = new TestSecurityConfig.User("admin").roles(ALL_ACCESS); private static final TestCertificates TEST_CERTIFICATES = new TestCertificates(); @@ -114,10 +109,9 @@ public class LdapAuthenticationCacheTest { ) .authc(AUTHC_HTTPBASIC_INTERNAL) .users(ADMIN_USER) - .rolesMapping(new RolesMapping(ALL_ACCESS).backendRoles(CN_GROUP_ADMIN)) + .rolesMapping(new TestSecurityConfig.RoleMapping(ALL_ACCESS.getName()).backendRoles(CN_GROUP_ADMIN)) .authz( new AuthzDomain("ldap_roles").httpEnabled(true) - .transportEnabled(true) .authorizationBackend( new AuthorizationBackend("ldap").config( () -> new LdapAuthorizationConfigBuilder().hosts(List.of("localhost:" + embeddedLDAPServer.getLdapNonTlsPort())) 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 aeba7c69d1..df673353f8 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 @@ -68,7 +68,7 @@ private void flushCacheApiRequestHandlers(RequestHandler.RequestHandlersBuilder // Extract the username from the request final String username = request.param("username"); if (username == null || username.isEmpty()) { - internalSeverError(channel, "No username provided for cache invalidation."); + internalServerError(channel, "No username provided for cache invalidation."); return; } // Validate and handle user-specific cache invalidation @@ -77,44 +77,25 @@ private void flushCacheApiRequestHandlers(RequestHandler.RequestHandlersBuilder configUpdateRequest = new ConfigUpdateRequest(CType.lcStringValues().toArray(new String[0])); } client.execute(ConfigUpdateAction.INSTANCE, configUpdateRequest, new ActionListener<>() { + @Override + public void onResponse(ConfigUpdateResponse configUpdateResponse) { + if (configUpdateResponse.hasFailures()) { + LOGGER.error("Cannot flush cache due to", configUpdateResponse.failures().get(0)); + internalServerError( + 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 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 onResponse(ConfigUpdateResponse configUpdateResponse) { - if (configUpdateResponse.hasFailures()) { - LOGGER.error("Cannot flush cache due to", configUpdateResponse.failures().get(0)); - internalServerError( - 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() + "."); - } - @Override - public void onFailure(final Exception e) { - LOGGER.error("Cannot flush cache due to", e); - internalServerError(channel, "Cannot flush cache due to " + e.getMessage() + "."); - } + @Override + public void onFailure(final Exception e) { + LOGGER.error("Cannot flush cache due to", e); + internalServerError(channel, "Cannot flush cache due to " + e.getMessage() + "."); + } }); });