diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a031169b7..e7a7b5d45e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ### Changed - Use extendedPlugins in integrationTest framework for sample resource plugin testing ([#5322](https://github.com/opensearch-project/security/pull/5322)) +- Refactor ResourcePermissions to refer to action groups as access levels ([#5335](https://github.com/opensearch-project/security/pull/5335)) ### Dependencies - Bump `guava_version` from 33.4.6-jre to 33.4.8-jre ([#5284](https://github.com/opensearch-project/security/pull/5284)) diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/SampleResourcePluginLimitedPermissionsTests.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/SampleResourcePluginLimitedPermissionsTests.java index 1f49b21983..e6bb555846 100644 --- a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/SampleResourcePluginLimitedPermissionsTests.java +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/SampleResourcePluginLimitedPermissionsTests.java @@ -8,22 +8,22 @@ package org.opensearch.sample; +import java.util.List; import java.util.Map; import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; import org.apache.http.HttpStatus; import org.awaitility.Awaitility; import org.junit.After; -import org.junit.Before; import org.junit.ClassRule; import org.junit.Test; import org.junit.runner.RunWith; +import org.opensearch.Version; import org.opensearch.painless.PainlessModulePlugin; -import org.opensearch.sample.resource.client.ResourceSharingClientAccessor; -import org.opensearch.security.resources.ResourcePluginInfo; -import org.opensearch.security.spi.resources.ResourceAccessActionGroups; -import org.opensearch.security.spi.resources.ResourceSharingExtension; +import org.opensearch.plugins.PluginInfo; +import org.opensearch.security.OpenSearchSecurityPlugin; +import org.opensearch.security.spi.resources.ResourceAccessLevels; import org.opensearch.test.framework.cluster.ClusterManager; import org.opensearch.test.framework.cluster.LocalCluster; import org.opensearch.test.framework.cluster.TestRestClient; @@ -32,6 +32,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.not; import static org.opensearch.sample.SampleResourcePluginTestHelper.SAMPLE_RESOURCE_CREATE_ENDPOINT; import static org.opensearch.sample.SampleResourcePluginTestHelper.SAMPLE_RESOURCE_DELETE_ENDPOINT; import static org.opensearch.sample.SampleResourcePluginTestHelper.SAMPLE_RESOURCE_GET_ENDPOINT; @@ -39,7 +40,6 @@ import static org.opensearch.sample.SampleResourcePluginTestHelper.SAMPLE_RESOURCE_SHARE_ENDPOINT; import static org.opensearch.sample.SampleResourcePluginTestHelper.SAMPLE_RESOURCE_UPDATE_ENDPOINT; import static org.opensearch.sample.SampleResourcePluginTestHelper.SHARED_WITH_USER_LIMITED_PERMISSIONS; -import static org.opensearch.sample.SampleResourcePluginTestHelper.createResourceAccessControlClient; import static org.opensearch.sample.SampleResourcePluginTestHelper.revokeAccessPayload; import static org.opensearch.sample.SampleResourcePluginTestHelper.shareWithPayload; import static org.opensearch.sample.utils.Constants.RESOURCE_INDEX_NAME; @@ -56,29 +56,33 @@ @ThreadLeakScope(ThreadLeakScope.Scope.NONE) public class SampleResourcePluginLimitedPermissionsTests { - ResourcePluginInfo resourcePluginInfo; - final ResourceSharingExtension resourceSharingExtension = new SampleResourceExtension(); - @ClassRule public static LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.SINGLENODE) - .plugin(SampleResourcePlugin.class, PainlessModulePlugin.class) + .plugin(PainlessModulePlugin.class) + .plugin( + new PluginInfo( + SampleResourcePlugin.class.getName(), + "classpath plugin", + "NA", + Version.CURRENT, + "1.8", + SampleResourcePlugin.class.getName(), + null, + List.of(OpenSearchSecurityPlugin.class.getName()), + false + ) + ) .anonymousAuth(true) .authc(AUTHC_HTTPBASIC_INTERNAL) .users(USER_ADMIN, SHARED_WITH_USER_LIMITED_PERMISSIONS) .nodeSettings(Map.of(SECURITY_SYSTEM_INDICES_ENABLED_KEY, true, OPENSEARCH_RESOURCE_SHARING_ENABLED, true)) .build(); - @Before - public void setup() { - resourcePluginInfo = cluster.nodes().getFirst().getInjectable(ResourcePluginInfo.class); - } - @After public void clearIndices() { try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) { client.delete(RESOURCE_INDEX_NAME); client.delete(OPENSEARCH_RESOURCE_SHARING_INDEX); - resourcePluginInfo.getResourceSharingExtensionsMutable().remove(resourceSharingExtension); } } @@ -94,7 +98,6 @@ public void testPluginInstalledCorrectly() { @Test public void testCreateUpdateDeleteSampleResource() throws Exception { String resourceId; - String resourceSharingDocId; // create sample resource try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) { String sampleResource = """ @@ -109,31 +112,6 @@ public void testCreateUpdateDeleteSampleResource() throws Exception { // Create an entry in resource-sharing index try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) { - // Since test framework doesn't yet allow loading ex tensions we need to create a resource sharing entry manually - String json = """ - { - "source_idx": ".sample_resource_sharing_plugin", - "resource_id": "%s", - "created_by": { - "user": "admin" - } - } - """.formatted(resourceId); - - TestRestClient.HttpResponse response = client.postJson(OPENSEARCH_RESOURCE_SHARING_INDEX + "/_doc", json); - assertThat(response.getStatusReason(), containsString("Created")); - resourceSharingDocId = response.bodyAsJsonNode().get("_id").asText(); - // update extension list - resourcePluginInfo.getResourceSharingExtensionsMutable().add(resourceSharingExtension); - - ResourceSharingClientAccessor.getInstance().setResourceSharingClient(createResourceAccessControlClient(cluster)); - - Awaitility.await() - .alias("Wait until resource data is populated") - .until(() -> client.get(SAMPLE_RESOURCE_GET_ENDPOINT + "/" + resourceId).getStatusCode(), equalTo(200)); - response = client.get(SAMPLE_RESOURCE_GET_ENDPOINT + "/" + resourceId); - response.assertStatusCode(HttpStatus.SC_OK); - assertThat(response.getBody(), containsString("sample")); // Wait until resource-sharing entry is successfully created Awaitility.await() .alias("Wait until resource-sharing data is populated") @@ -211,7 +189,7 @@ public void testCreateUpdateDeleteSampleResource() throws Exception { ); response.assertStatusCode(HttpStatus.SC_OK); assertThat( - response.bodyAsJsonNode().get("share_with").get(ResourceAccessActionGroups.PLACE_HOLDER).get("users").get(0).asText(), + response.bodyAsJsonNode().get("share_with").get(ResourceAccessLevels.PLACE_HOLDER).get("users").get(0).asText(), containsString(SHARED_WITH_USER_LIMITED_PERMISSIONS.getName()) ); } @@ -241,7 +219,7 @@ public void testCreateUpdateDeleteSampleResource() throws Exception { revokeAccessPayload(SHARED_WITH_USER_LIMITED_PERMISSIONS.getName()) ); response.assertStatusCode(HttpStatus.SC_OK); - assertThat(response.bodyAsJsonNode().get("share_with").size(), equalTo(0)); + assertThat(response.getBody(), not(containsString("resource_sharing_test_user_limited_perms"))); } // get sample resource with SHARED_WITH_USER_LIMITED_PERMISSIONS, user no longer has access to resource @@ -268,10 +246,6 @@ public void testCreateUpdateDeleteSampleResource() throws Exception { // corresponding entry should be removed from resource-sharing index try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) { - // Since test framework doesn't yet allow loading ex tensions we need to delete the resource sharing entry manually - TestRestClient.HttpResponse response = client.delete(OPENSEARCH_RESOURCE_SHARING_INDEX + "/_doc/" + resourceSharingDocId); - response.assertStatusCode(HttpStatus.SC_OK); - Awaitility.await() .alias("Wait until resource-sharing data is updated") .until( @@ -310,35 +284,12 @@ public void testAccessWithLimitedIP() throws Exception { // Create an entry in resource-sharing index try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) { - // Since test framework doesn't yet allow loading ex tensions we need to create a resource sharing entry manually - - String json = """ - { - "source_idx": "%s", - "resource_id": "%s", - "created_by": { - "user": "%s" - } - } - """.formatted(RESOURCE_INDEX_NAME, resourceId, SHARED_WITH_USER_LIMITED_PERMISSIONS.getName()); - - HttpResponse response = client.postJson(OPENSEARCH_RESOURCE_SHARING_INDEX + "/_doc", json); - assertThat(response.getStatusReason(), containsString("Created")); - - resourcePluginInfo.getResourceSharingExtensionsMutable().add(resourceSharingExtension); - - ResourceSharingClientAccessor.getInstance().setResourceSharingClient(createResourceAccessControlClient(cluster)); - Awaitility.await() .alias("Wait until resource-sharing data is populated") .until( () -> client.get(OPENSEARCH_RESOURCE_SHARING_INDEX + "/_search").bodyAsJsonNode().get("hits").get("hits").size(), equalTo(1) ); - response = client.get(SAMPLE_RESOURCE_GET_ENDPOINT); - response.assertStatusCode(HttpStatus.SC_OK); - assertThat(response.bodyAsJsonNode().get("resources").size(), equalTo(1)); - assertThat(response.getBody(), containsString("sample")); } // user should be able to get its own resource as it has get API access diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/SampleResourcePluginSystemIndexDisabledTests.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/SampleResourcePluginSystemIndexDisabledTests.java index af99893a96..cae15e708a 100644 --- a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/SampleResourcePluginSystemIndexDisabledTests.java +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/SampleResourcePluginSystemIndexDisabledTests.java @@ -8,22 +8,22 @@ package org.opensearch.sample; +import java.util.List; import java.util.Map; import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; import org.apache.http.HttpStatus; import org.awaitility.Awaitility; import org.junit.After; -import org.junit.Before; import org.junit.ClassRule; import org.junit.Test; import org.junit.runner.RunWith; +import org.opensearch.Version; import org.opensearch.painless.PainlessModulePlugin; -import org.opensearch.sample.resource.client.ResourceSharingClientAccessor; -import org.opensearch.security.resources.ResourcePluginInfo; -import org.opensearch.security.spi.resources.ResourceAccessActionGroups; -import org.opensearch.security.spi.resources.ResourceSharingExtension; +import org.opensearch.plugins.PluginInfo; +import org.opensearch.security.OpenSearchSecurityPlugin; +import org.opensearch.security.spi.resources.ResourceAccessLevels; import org.opensearch.test.framework.cluster.ClusterManager; import org.opensearch.test.framework.cluster.LocalCluster; import org.opensearch.test.framework.cluster.TestRestClient; @@ -32,6 +32,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.not; import static org.opensearch.sample.SampleResourcePluginTestHelper.SAMPLE_RESOURCE_CREATE_ENDPOINT; import static org.opensearch.sample.SampleResourcePluginTestHelper.SAMPLE_RESOURCE_DELETE_ENDPOINT; import static org.opensearch.sample.SampleResourcePluginTestHelper.SAMPLE_RESOURCE_GET_ENDPOINT; @@ -39,7 +40,6 @@ import static org.opensearch.sample.SampleResourcePluginTestHelper.SAMPLE_RESOURCE_SHARE_ENDPOINT; import static org.opensearch.sample.SampleResourcePluginTestHelper.SAMPLE_RESOURCE_UPDATE_ENDPOINT; import static org.opensearch.sample.SampleResourcePluginTestHelper.SHARED_WITH_USER; -import static org.opensearch.sample.SampleResourcePluginTestHelper.createResourceAccessControlClient; import static org.opensearch.sample.SampleResourcePluginTestHelper.revokeAccessPayload; import static org.opensearch.sample.SampleResourcePluginTestHelper.shareWithPayload; import static org.opensearch.sample.utils.Constants.RESOURCE_INDEX_NAME; @@ -55,30 +55,33 @@ @ThreadLeakScope(ThreadLeakScope.Scope.NONE) public class SampleResourcePluginSystemIndexDisabledTests { - ResourcePluginInfo resourcePluginInfo; - ResourceSharingExtension resourceSharingExtension; - @ClassRule public static LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.SINGLENODE) - .plugin(SampleResourcePlugin.class, PainlessModulePlugin.class) + .plugin(PainlessModulePlugin.class) + .plugin( + new PluginInfo( + SampleResourcePlugin.class.getName(), + "classpath plugin", + "NA", + Version.CURRENT, + "1.8", + SampleResourcePlugin.class.getName(), + null, + List.of(OpenSearchSecurityPlugin.class.getName()), + false + ) + ) .anonymousAuth(true) .authc(AUTHC_HTTPBASIC_INTERNAL) .users(USER_ADMIN, SHARED_WITH_USER) .nodeSettings(Map.of(OPENSEARCH_RESOURCE_SHARING_ENABLED, true)) .build(); - @Before - public void setup() { - resourcePluginInfo = cluster.nodes().getFirst().getInjectable(ResourcePluginInfo.class); - resourceSharingExtension = new SampleResourceExtension(); - } - @After public void clearIndices() { try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) { client.delete(RESOURCE_INDEX_NAME); client.delete(OPENSEARCH_RESOURCE_SHARING_INDEX); - resourcePluginInfo.getResourceSharingExtensionsMutable().remove(resourceSharingExtension); } } @@ -94,7 +97,6 @@ public void testPluginInstalledCorrectly() { @Test public void testCreateUpdateDeleteSampleResource() throws Exception { String resourceId; - String resourceSharingDocId; // create sample resource try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) { String sampleResource = """ @@ -109,30 +111,6 @@ public void testCreateUpdateDeleteSampleResource() throws Exception { // Create an entry in resource-sharing index try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) { - // Since test framework doesn't yet allow loading ex tensions we need to create a resource sharing entry manually - String json = """ - { - "source_idx": ".sample_resource_sharing_plugin", - "resource_id": "%s", - "created_by": { - "user": "admin" - } - } - """.formatted(resourceId); - - TestRestClient.HttpResponse response = client.postJson(OPENSEARCH_RESOURCE_SHARING_INDEX + "/_doc", json); - assertThat(response.getStatusReason(), containsString("Created")); - resourceSharingDocId = response.bodyAsJsonNode().get("_id").asText(); - resourcePluginInfo.getResourceSharingExtensionsMutable().add(resourceSharingExtension); - - ResourceSharingClientAccessor.getInstance().setResourceSharingClient(createResourceAccessControlClient(cluster)); - - Awaitility.await() - .alias("Wait until resource data is populated") - .until(() -> client.get(SAMPLE_RESOURCE_GET_ENDPOINT + "/" + resourceId).getStatusCode(), equalTo(200)); - response = client.get(SAMPLE_RESOURCE_GET_ENDPOINT + "/" + resourceId); - response.assertStatusCode(HttpStatus.SC_OK); - assertThat(response.getBody(), containsString("sample")); // Wait until resource-sharing entry is successfully created Awaitility.await() .alias("Wait until resource-sharing data is populated") @@ -210,7 +188,7 @@ public void testCreateUpdateDeleteSampleResource() throws Exception { ); response.assertStatusCode(HttpStatus.SC_OK); assertThat( - response.bodyAsJsonNode().get("share_with").get(ResourceAccessActionGroups.PLACE_HOLDER).get("users").get(0).asText(), + response.bodyAsJsonNode().get("share_with").get(ResourceAccessLevels.PLACE_HOLDER).get("users").get(0).asText(), containsString(SHARED_WITH_USER.getName()) ); } @@ -240,7 +218,7 @@ public void testCreateUpdateDeleteSampleResource() throws Exception { revokeAccessPayload(SHARED_WITH_USER.getName()) ); response.assertStatusCode(HttpStatus.SC_OK); - assertThat(response.bodyAsJsonNode().get("share_with").size(), equalTo(0)); + assertThat(response.getBody(), not(containsString("resource_sharing_test_user"))); } // get sample resource with SHARED_WITH_USER, user no longer has access to resource @@ -267,10 +245,6 @@ public void testCreateUpdateDeleteSampleResource() throws Exception { // corresponding entry should be removed from resource-sharing index try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) { - // Since test framework doesn't yet allow loading ex tensions we need to delete the resource sharing entry manually - TestRestClient.HttpResponse response = client.delete(OPENSEARCH_RESOURCE_SHARING_INDEX + "/_doc/" + resourceSharingDocId); - response.assertStatusCode(HttpStatus.SC_OK); - Awaitility.await() .alias("Wait until resource-sharing data is updated") .until( @@ -309,33 +283,12 @@ public void testDirectAccess() throws Exception { // Create an entry in resource-sharing index try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) { - // Since test framework doesn't yet allow loading ex tensions we need to create a resource sharing entry manually - String json = """ - { - "source_idx": "%s", - "resource_id": "%s", - "created_by": { - "user": "admin" - } - } - """.formatted(RESOURCE_INDEX_NAME, resourceId); - - HttpResponse response = client.postJson(OPENSEARCH_RESOURCE_SHARING_INDEX + "/_doc", json); - assertThat(response.getStatusReason(), containsString("Created")); - resourcePluginInfo.getResourceSharingExtensionsMutable().add(resourceSharingExtension); - - ResourceSharingClientAccessor.getInstance().setResourceSharingClient(createResourceAccessControlClient(cluster)); - Awaitility.await() .alias("Wait until resource-sharing data is populated") .until( () -> client.get(OPENSEARCH_RESOURCE_SHARING_INDEX + "/_search").bodyAsJsonNode().get("hits").get("hits").size(), equalTo(1) ); - response = client.get(SAMPLE_RESOURCE_GET_ENDPOINT); - response.assertStatusCode(HttpStatus.SC_OK); - assertThat(response.bodyAsJsonNode().get("resources").size(), equalTo(1)); - assertThat(response.getBody(), containsString("sample")); } // admin will be able to access resource directly since system index protection is disabled, and also via sample plugin @@ -385,7 +338,7 @@ public void testDirectAccess() throws Exception { ); response.assertStatusCode(HttpStatus.SC_OK); assertThat( - response.bodyAsJsonNode().get("share_with").get(ResourceAccessActionGroups.PLACE_HOLDER).get("users").get(0).asText(), + response.bodyAsJsonNode().get("share_with").get(ResourceAccessLevels.PLACE_HOLDER).get("users").get(0).asText(), containsString(SHARED_WITH_USER.getName()) ); } @@ -413,7 +366,7 @@ public void testDirectAccess() throws Exception { revokeAccessPayload(SHARED_WITH_USER.getName()) ); response.assertStatusCode(HttpStatus.SC_OK); - assertThat(response.bodyAsJsonNode().get("share_with").size(), equalTo(0)); + assertThat(response.getBody(), not(containsString("resource_sharing_test_user"))); } // shared_with_user will still be able to access the resource directly but not via sample plugin since access is revoked diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/SampleResourcePluginTestHelper.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/SampleResourcePluginTestHelper.java index 2badb694d6..fddaa94940 100644 --- a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/SampleResourcePluginTestHelper.java +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/SampleResourcePluginTestHelper.java @@ -8,12 +8,7 @@ package org.opensearch.sample; -import org.opensearch.common.settings.Settings; -import org.opensearch.security.resources.ResourceAccessControlClient; -import org.opensearch.security.resources.ResourceAccessHandler; -import org.opensearch.security.spi.resources.client.ResourceSharingClient; import org.opensearch.test.framework.TestSecurityConfig; -import org.opensearch.test.framework.cluster.LocalCluster; import static org.opensearch.sample.utils.Constants.SAMPLE_RESOURCE_PLUGIN_PREFIX; @@ -47,12 +42,6 @@ public abstract class SampleResourcePluginTestHelper { protected static final String SAMPLE_RESOURCE_SHARE_ENDPOINT = SAMPLE_RESOURCE_PLUGIN_PREFIX + "/share"; protected static final String SAMPLE_RESOURCE_REVOKE_ENDPOINT = SAMPLE_RESOURCE_PLUGIN_PREFIX + "/revoke"; - protected static ResourceSharingClient createResourceAccessControlClient(LocalCluster cluster) { - ResourceAccessHandler rAH = cluster.nodes().getFirst().getInjectable(ResourceAccessHandler.class); - Settings settings = cluster.node().settings(); - return new ResourceAccessControlClient(rAH, settings); - } - protected static String shareWithPayload(String user) { return """ { diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/SampleResourcePluginTests.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/SampleResourcePluginTests.java index e3135e60a7..5c14dab825 100644 --- a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/SampleResourcePluginTests.java +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/SampleResourcePluginTests.java @@ -15,7 +15,6 @@ import org.apache.http.HttpStatus; import org.awaitility.Awaitility; import org.junit.After; -import org.junit.Before; import org.junit.ClassRule; import org.junit.Test; import org.junit.runner.RunWith; @@ -24,9 +23,7 @@ import org.opensearch.painless.PainlessModulePlugin; import org.opensearch.plugins.PluginInfo; import org.opensearch.security.OpenSearchSecurityPlugin; -import org.opensearch.security.resources.ResourcePluginInfo; -import org.opensearch.security.spi.resources.ResourceAccessActionGroups; -import org.opensearch.security.spi.resources.ResourceSharingExtension; +import org.opensearch.security.spi.resources.ResourceAccessLevels; import org.opensearch.test.framework.cluster.ClusterManager; import org.opensearch.test.framework.cluster.LocalCluster; import org.opensearch.test.framework.cluster.TestRestClient; @@ -35,6 +32,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.not; import static org.opensearch.sample.SampleResourcePluginTestHelper.SAMPLE_RESOURCE_CREATE_ENDPOINT; import static org.opensearch.sample.SampleResourcePluginTestHelper.SAMPLE_RESOURCE_DELETE_ENDPOINT; import static org.opensearch.sample.SampleResourcePluginTestHelper.SAMPLE_RESOURCE_GET_ENDPOINT; @@ -58,9 +56,6 @@ @ThreadLeakScope(ThreadLeakScope.Scope.NONE) public class SampleResourcePluginTests { - ResourcePluginInfo resourcePluginInfo; - ResourceSharingExtension resourceSharingExtension; - @ClassRule public static LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.SINGLENODE) .plugin(PainlessModulePlugin.class) @@ -83,18 +78,11 @@ public class SampleResourcePluginTests { .nodeSettings(Map.of(SECURITY_SYSTEM_INDICES_ENABLED_KEY, true, OPENSEARCH_RESOURCE_SHARING_ENABLED, true)) .build(); - @Before - public void setup() { - resourcePluginInfo = cluster.nodes().getFirst().getInjectable(ResourcePluginInfo.class); - resourceSharingExtension = new SampleResourceExtension(); - } - @After public void clearIndices() { try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) { client.delete(RESOURCE_INDEX_NAME); client.delete(OPENSEARCH_RESOURCE_SHARING_INDEX); - resourcePluginInfo.getResourceSharingExtensionsMutable().remove(resourceSharingExtension); } } @@ -185,7 +173,7 @@ public void testCreateUpdateDeleteSampleResource() throws Exception { ); response.assertStatusCode(HttpStatus.SC_OK); assertThat( - response.bodyAsJsonNode().get("share_with").get(ResourceAccessActionGroups.PLACE_HOLDER).get("users").get(0).asText(), + response.bodyAsJsonNode().get("share_with").get(ResourceAccessLevels.PLACE_HOLDER).get("users").get(0).asText(), containsString(SHARED_WITH_USER.getName()) ); } @@ -208,7 +196,7 @@ public void testCreateUpdateDeleteSampleResource() throws Exception { revokeAccessPayload(SHARED_WITH_USER.getName()) ); response.assertStatusCode(HttpStatus.SC_OK); - assertThat(response.bodyAsJsonNode().get("share_with").size(), equalTo(0)); + assertThat(response.getBody(), not(containsString("resource_sharing_test_user"))); } // get sample resource with SHARED_WITH_USER, user no longer has access to resource @@ -250,8 +238,6 @@ public void testCreateUpdateDeleteSampleResource() throws Exception { @Test public void testDirectAccess() throws Exception { - ResourcePluginInfo resourcePluginInfo = cluster.nodes().getFirst().getInjectable(ResourcePluginInfo.class); - String resourceId; // create sample resource try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) { @@ -329,7 +315,7 @@ public void testDirectAccess() throws Exception { ); response.assertStatusCode(HttpStatus.SC_OK); assertThat( - response.bodyAsJsonNode().get("share_with").get(ResourceAccessActionGroups.PLACE_HOLDER).get("users").get(0).asText(), + response.bodyAsJsonNode().get("share_with").get(ResourceAccessLevels.PLACE_HOLDER).get("users").get(0).asText(), containsString(SHARED_WITH_USER.getName()) ); } @@ -359,7 +345,7 @@ public void testDirectAccess() throws Exception { revokeAccessPayload(SHARED_WITH_USER.getName()) ); response.assertStatusCode(HttpStatus.SC_OK); - assertThat(response.bodyAsJsonNode().get("share_with").size(), equalTo(0)); + assertThat(response.getBody(), not(containsString("resource_sharing_test_user"))); } // shared_with_user should not be able to access the resource directly nor via sample plugin since access is revoked diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/revoke/RevokeResourceAccessRequest.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/revoke/RevokeResourceAccessRequest.java index a066aefd6e..5768969e04 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/revoke/RevokeResourceAccessRequest.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/revoke/RevokeResourceAccessRequest.java @@ -9,12 +9,14 @@ package org.opensearch.sample.resource.actions.rest.revoke; import java.io.IOException; +import java.util.Map; +import java.util.Set; import org.opensearch.action.ActionRequest; import org.opensearch.action.ActionRequestValidationException; import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.StreamOutput; -import org.opensearch.security.spi.resources.sharing.SharedWithActionGroup; +import org.opensearch.security.spi.resources.sharing.Recipient; /** * Request object for revoking access to a sample resource @@ -22,22 +24,26 @@ public class RevokeResourceAccessRequest extends ActionRequest { String resourceId; - SharedWithActionGroup.ActionGroupRecipients entitiesToRevoke; + Map> entitiesToRevoke; - public RevokeResourceAccessRequest(String resourceId, SharedWithActionGroup.ActionGroupRecipients entitiesToRevoke) { + public RevokeResourceAccessRequest(String resourceId, Map> entitiesToRevoke) { this.resourceId = resourceId; this.entitiesToRevoke = entitiesToRevoke; } public RevokeResourceAccessRequest(StreamInput in) throws IOException { resourceId = in.readString(); - entitiesToRevoke = in.readNamedWriteable(SharedWithActionGroup.ActionGroupRecipients.class); + entitiesToRevoke = in.readMap(key -> key.readEnum(Recipient.class), input -> input.readSet(StreamInput::readString)); } @Override public void writeTo(final StreamOutput out) throws IOException { out.writeString(resourceId); - out.writeNamedWriteable(entitiesToRevoke); + out.writeMap( + entitiesToRevoke, + StreamOutput::writeEnum, + (streamOutput, strings) -> streamOutput.writeCollection(strings, StreamOutput::writeString) + ); } @Override @@ -49,7 +55,7 @@ public String getResourceId() { return resourceId; } - public SharedWithActionGroup.ActionGroupRecipients getEntitiesToRevoke() { + public Map> getEntitiesToRevoke() { return entitiesToRevoke; } } diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/revoke/RevokeResourceAccessRestAction.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/revoke/RevokeResourceAccessRestAction.java index 3f98ce9fa4..a25914548d 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/revoke/RevokeResourceAccessRestAction.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/revoke/RevokeResourceAccessRestAction.java @@ -11,6 +11,7 @@ import java.io.IOException; import java.util.Collection; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Set; import java.util.stream.Collectors; @@ -21,7 +22,6 @@ import org.opensearch.rest.RestRequest; import org.opensearch.rest.action.RestToXContentListener; import org.opensearch.security.spi.resources.sharing.Recipient; -import org.opensearch.security.spi.resources.sharing.SharedWithActionGroup; import org.opensearch.transport.client.node.NodeClient; import static java.util.Collections.singletonList; @@ -67,7 +67,7 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli ); } - private SharedWithActionGroup.ActionGroupRecipients parseRevokedEntities(Map source) { + private Map> parseRevokedEntities(Map source) { if (source == null || source.isEmpty()) { throw new IllegalArgumentException("entities_to_revoke is required and cannot be empty"); } @@ -77,7 +77,7 @@ private SharedWithActionGroup.ActionGroupRecipients parseRevokedEntities(Map entry.getValue() instanceof Collection) .collect( Collectors.toMap( - entry -> Recipient.fromValue(entry.getKey()), + entry -> Recipient.valueOf(entry.getKey().toUpperCase(Locale.ROOT)), entry -> ((Collection) entry.getValue()).stream() .filter(String.class::isInstance) .map(String.class::cast) @@ -85,6 +85,6 @@ private SharedWithActionGroup.ActionGroupRecipients parseRevokedEntities(Map> recipients; - public ShareResourceRequest(String resourceId, SharedWithActionGroup.ActionGroupRecipients shareWith) { + public ShareResourceRequest(String resourceId, Map> recipients) { this.resourceId = resourceId; - this.shareWith = shareWith; + this.recipients = recipients; } public ShareResourceRequest(StreamInput in) throws IOException { this.resourceId = in.readString(); - this.shareWith = in.readNamedWriteable(SharedWithActionGroup.ActionGroupRecipients.class); + this.recipients = in.readMap(key -> key.readEnum(Recipient.class), input -> input.readSet(StreamInput::readString)); } @Override public void writeTo(final StreamOutput out) throws IOException { out.writeString(this.resourceId); - out.writeNamedWriteable(shareWith); + out.writeMap( + recipients, + StreamOutput::writeEnum, + (streamOutput, strings) -> streamOutput.writeCollection(strings, StreamOutput::writeString) + ); } @Override @@ -50,7 +56,7 @@ public String getResourceId() { return this.resourceId; } - public SharedWithActionGroup.ActionGroupRecipients getShareWith() { - return shareWith; + public Map> getRecipients() { + return recipients; } } diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/share/ShareResourceRestAction.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/share/ShareResourceRestAction.java index 05e6660fc1..6793ff6801 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/share/ShareResourceRestAction.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/share/ShareResourceRestAction.java @@ -9,19 +9,20 @@ package org.opensearch.sample.resource.actions.rest.share; import java.io.IOException; +import java.util.Collection; +import java.util.HashMap; +import java.util.HashSet; import java.util.List; +import java.util.Locale; import java.util.Map; +import java.util.Set; -import org.opensearch.common.xcontent.LoggingDeprecationHandler; -import org.opensearch.common.xcontent.XContentFactory; -import org.opensearch.common.xcontent.XContentType; import org.opensearch.core.common.Strings; -import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.core.xcontent.XContentParser; import org.opensearch.rest.BaseRestHandler; import org.opensearch.rest.RestRequest; import org.opensearch.rest.action.RestToXContentListener; -import org.opensearch.security.spi.resources.sharing.SharedWithActionGroup; +import org.opensearch.security.spi.resources.sharing.Recipient; import org.opensearch.transport.client.node.NodeClient; import static java.util.Collections.singletonList; @@ -60,21 +61,16 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli } Map shareWith = (Map) source.get("share_with"); + Map> recipients = new HashMap<>(); + if (shareWith != null) { + for (Map.Entry entry : shareWith.entrySet()) { + Recipient recipient = Recipient.valueOf(entry.getKey().toUpperCase(Locale.ROOT)); + Set targets = new HashSet<>((Collection) entry.getValue()); + recipients.put(recipient, targets); + } + } - final ShareResourceRequest shareResourceRequest = new ShareResourceRequest(resourceId, parseShareWith(shareWith)); + final ShareResourceRequest shareResourceRequest = new ShareResourceRequest(resourceId, recipients); return channel -> client.executeLocally(ShareResourceAction.INSTANCE, shareResourceRequest, new RestToXContentListener<>(channel)); } - - private SharedWithActionGroup.ActionGroupRecipients parseShareWith(Map source) throws IOException { - String jsonString = XContentFactory.jsonBuilder().map(source).toString(); - - try ( - XContentParser parser = XContentType.JSON.xContent() - .createParser(NamedXContentRegistry.EMPTY, LoggingDeprecationHandler.INSTANCE, jsonString) - ) { - return SharedWithActionGroup.ActionGroupRecipients.fromXContent(parser); - } catch (IllegalArgumentException e) { - throw new IllegalArgumentException("Invalid share_with structure: " + e.getMessage(), e); - } - } } diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/DeleteResourceTransportAction.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/DeleteResourceTransportAction.java index 3d7f5fd5d2..6a1c8b1751 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/DeleteResourceTransportAction.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/DeleteResourceTransportAction.java @@ -74,7 +74,7 @@ protected void doExecute(Task task, DeleteResourceRequest request, ActionListene deleteResource(resourceId, deleteResponseListener); return; } - resourceSharingClient.verifyResourceAccess(resourceId, RESOURCE_INDEX_NAME, ActionListener.wrap(isAuthorized -> { + resourceSharingClient.verifyAccess(resourceId, RESOURCE_INDEX_NAME, ActionListener.wrap(isAuthorized -> { if (!isAuthorized) { listener.onFailure( new OpenSearchStatusException("Current user is not authorized to delete resource: " + resourceId, RestStatus.FORBIDDEN) diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/GetResourceTransportAction.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/GetResourceTransportAction.java index b945414775..fbb7f0260c 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/GetResourceTransportAction.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/GetResourceTransportAction.java @@ -105,7 +105,7 @@ private void verifyAndFetchSingle(String resourceId, ActionListener { + client.verifyAccess(resourceId, RESOURCE_INDEX_NAME, ActionListener.wrap(authorized -> { if (!authorized) { listener.onFailure(new OpenSearchStatusException("Not authorized to access resource: " + resourceId, RestStatus.FORBIDDEN)); } else { diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/RevokeResourceAccessTransportAction.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/RevokeResourceAccessTransportAction.java index 7f79d6f806..7509af2e81 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/RevokeResourceAccessTransportAction.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/RevokeResourceAccessTransportAction.java @@ -8,6 +8,8 @@ package org.opensearch.sample.resource.actions.transport; +import java.util.Map; + import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -20,10 +22,13 @@ import org.opensearch.sample.resource.actions.rest.revoke.RevokeResourceAccessResponse; import org.opensearch.sample.resource.client.ResourceSharingClientAccessor; import org.opensearch.security.spi.resources.client.ResourceSharingClient; +import org.opensearch.security.spi.resources.sharing.Recipients; +import org.opensearch.security.spi.resources.sharing.ShareWith; import org.opensearch.tasks.Task; import org.opensearch.transport.TransportService; import static org.opensearch.sample.utils.Constants.RESOURCE_INDEX_NAME; +import static org.opensearch.security.spi.resources.ResourceAccessLevels.PLACE_HOLDER; /** * Transport action for revoking resource access. @@ -39,16 +44,12 @@ public RevokeResourceAccessTransportAction(TransportService transportService, Ac @Override protected void doExecute(Task task, RevokeResourceAccessRequest request, ActionListener listener) { ResourceSharingClient resourceSharingClient = ResourceSharingClientAccessor.getInstance().getResourceSharingClient(); - resourceSharingClient.revoke( - request.getResourceId(), - RESOURCE_INDEX_NAME, - request.getEntitiesToRevoke(), - ActionListener.wrap(success -> { - RevokeResourceAccessResponse response = new RevokeResourceAccessResponse(success.getShareWith()); - log.debug("Revoked resource access: {}", response.toString()); - listener.onResponse(response); - }, listener::onFailure) - ); + ShareWith target = new ShareWith(Map.of(PLACE_HOLDER, new Recipients(request.getEntitiesToRevoke()))); + resourceSharingClient.revoke(request.getResourceId(), RESOURCE_INDEX_NAME, target, ActionListener.wrap(success -> { + RevokeResourceAccessResponse response = new RevokeResourceAccessResponse(success.getShareWith()); + log.debug("Revoked resource access: {}", response.toString()); + listener.onResponse(response); + }, listener::onFailure)); } } diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/ShareResourceTransportAction.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/ShareResourceTransportAction.java index fde8fb8c9e..86f68cf497 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/ShareResourceTransportAction.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/ShareResourceTransportAction.java @@ -8,6 +8,8 @@ package org.opensearch.sample.resource.actions.transport; +import java.util.Map; + import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -20,10 +22,13 @@ import org.opensearch.sample.resource.actions.rest.share.ShareResourceResponse; import org.opensearch.sample.resource.client.ResourceSharingClientAccessor; import org.opensearch.security.spi.resources.client.ResourceSharingClient; +import org.opensearch.security.spi.resources.sharing.Recipients; +import org.opensearch.security.spi.resources.sharing.ShareWith; import org.opensearch.tasks.Task; import org.opensearch.transport.TransportService; import static org.opensearch.sample.utils.Constants.RESOURCE_INDEX_NAME; +import static org.opensearch.security.spi.resources.ResourceAccessLevels.PLACE_HOLDER; /** * Transport action implementation for sharing a resource. @@ -44,7 +49,8 @@ protected void doExecute(Task task, ShareResourceRequest request, ActionListener } ResourceSharingClient resourceSharingClient = ResourceSharingClientAccessor.getInstance().getResourceSharingClient(); - resourceSharingClient.share(request.getResourceId(), RESOURCE_INDEX_NAME, request.getShareWith(), ActionListener.wrap(sharing -> { + ShareWith shareWith = new ShareWith(Map.of(PLACE_HOLDER, new Recipients(request.getRecipients()))); + resourceSharingClient.share(request.getResourceId(), RESOURCE_INDEX_NAME, shareWith, ActionListener.wrap(sharing -> { ShareResourceResponse response = new ShareResourceResponse(sharing.getShareWith()); log.debug("Shared resource: {}", response.toString()); listener.onResponse(response); diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/UpdateResourceTransportAction.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/UpdateResourceTransportAction.java index 7d7ed42fa0..7bbffdd4d1 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/UpdateResourceTransportAction.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/UpdateResourceTransportAction.java @@ -65,7 +65,7 @@ protected void doExecute(Task task, UpdateResourceRequest request, ActionListene updateResource(request, listener); return; } - resourceSharingClient.verifyResourceAccess(request.getResourceId(), RESOURCE_INDEX_NAME, ActionListener.wrap(isAuthorized -> { + resourceSharingClient.verifyAccess(request.getResourceId(), RESOURCE_INDEX_NAME, ActionListener.wrap(isAuthorized -> { if (!isAuthorized) { listener.onFailure( new OpenSearchStatusException( diff --git a/spi/src/main/java/org/opensearch/security/spi/resources/ResourceAccessActionGroups.java b/spi/src/main/java/org/opensearch/security/spi/resources/ResourceAccessActionGroups.java deleted file mode 100644 index 2289ce5676..0000000000 --- a/spi/src/main/java/org/opensearch/security/spi/resources/ResourceAccessActionGroups.java +++ /dev/null @@ -1,37 +0,0 @@ -/* - * 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.spi.resources; - -import java.util.Arrays; - -/** - * This class represents action-groups to be utilized to share resources. - * - * @opensearch.experimental - */ -public interface ResourceAccessActionGroups> { - // TODO update following comment once ResourceAuthz is implemented as a standalone framework - // At present, we define this place-holder value represents the default action group this resource is shared with. - String PLACE_HOLDER = "default"; - - static & ResourceAccessActionGroups> E fromValue(Class enumClass, String value) { - for (E enumConstant : enumClass.getEnumConstants()) { - if (enumConstant.value().equalsIgnoreCase(value)) { - return enumConstant; - } - } - throw new IllegalArgumentException("Unknown value: " + value); - } - - String value(); - - static & ResourceAccessActionGroups> String[] values(Class enumClass) { - return Arrays.stream(enumClass.getEnumConstants()).map(ResourceAccessActionGroups::value).toArray(String[]::new); - } -} diff --git a/spi/src/main/java/org/opensearch/security/spi/resources/ResourceAccessLevels.java b/spi/src/main/java/org/opensearch/security/spi/resources/ResourceAccessLevels.java new file mode 100644 index 0000000000..11b314ede3 --- /dev/null +++ b/spi/src/main/java/org/opensearch/security/spi/resources/ResourceAccessLevels.java @@ -0,0 +1,20 @@ +/* + * 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.spi.resources; + +/** + * This class represents action-groups to be utilized to share resources. + * + * @opensearch.experimental + */ +public interface ResourceAccessLevels { + // TODO update following comment once ResourceAuthz is implemented as a standalone framework + // At present, we define this place-holder value represents the default action group this resource is shared with. + String PLACE_HOLDER = "default"; +} diff --git a/spi/src/main/java/org/opensearch/security/spi/resources/client/ResourceSharingClient.java b/spi/src/main/java/org/opensearch/security/spi/resources/client/ResourceSharingClient.java index 429a220208..07ad609536 100644 --- a/spi/src/main/java/org/opensearch/security/spi/resources/client/ResourceSharingClient.java +++ b/spi/src/main/java/org/opensearch/security/spi/resources/client/ResourceSharingClient.java @@ -12,7 +12,7 @@ import org.opensearch.core.action.ActionListener; import org.opensearch.security.spi.resources.sharing.ResourceSharing; -import org.opensearch.security.spi.resources.sharing.SharedWithActionGroup; +import org.opensearch.security.spi.resources.sharing.ShareWith; /** * Interface for resource sharing client operations. @@ -27,35 +27,25 @@ public interface ResourceSharingClient { * @param resourceIndex The index containing the resource. * @param listener The listener to be notified with the access verification result. */ - void verifyResourceAccess(String resourceId, String resourceIndex, ActionListener listener); + void verifyAccess(String resourceId, String resourceIndex, ActionListener listener); /** * Shares a resource with the specified users, roles, and backend roles. * @param resourceId The ID of the resource to share. * @param resourceIndex The index containing the resource. - * @param recipients The users, roles, and backend roles to share the resource with. + * @param target The users, roles, and backend roles to share the resource with and respective access levels. * @param listener The listener to be notified with the updated ResourceSharing document. */ - void share( - String resourceId, - String resourceIndex, - SharedWithActionGroup.ActionGroupRecipients recipients, - ActionListener listener - ); + void share(String resourceId, String resourceIndex, ShareWith target, ActionListener listener); /** * Revokes access to a resource for the specified entities. * @param resourceId The ID of the resource to revoke access for. * @param resourceIndex The index containing the resource. - * @param entitiesToRevoke The entities to revoke access for. + * @param target The entities to revoke access for. * @param listener The listener to be notified with the updated ResourceSharing document. */ - void revoke( - String resourceId, - String resourceIndex, - SharedWithActionGroup.ActionGroupRecipients entitiesToRevoke, - ActionListener listener - ); + void revoke(String resourceId, String resourceIndex, ShareWith target, ActionListener listener); /** * Lists resourceIds of all shareable resources accessible by the current user. diff --git a/spi/src/main/java/org/opensearch/security/spi/resources/sharing/CreatedBy.java b/spi/src/main/java/org/opensearch/security/spi/resources/sharing/CreatedBy.java index 50bdd1aea7..3f19d52cdf 100644 --- a/spi/src/main/java/org/opensearch/security/spi/resources/sharing/CreatedBy.java +++ b/spi/src/main/java/org/opensearch/security/spi/resources/sharing/CreatedBy.java @@ -24,30 +24,23 @@ */ public class CreatedBy implements ToXContentFragment, NamedWriteable { - private final Creator creatorType; - private final String creator; + private final String username; - public CreatedBy(Creator creatorType, String creator) { - this.creatorType = creatorType; - this.creator = creator; + public CreatedBy(String username) { + this.username = username; } public CreatedBy(StreamInput in) throws IOException { - this.creatorType = in.readEnum(Creator.class); - this.creator = in.readString(); + this.username = in.readString(); } - public String getCreator() { - return creator; - } - - public Creator getCreatorType() { - return creatorType; + public String getUsername() { + return username; } @Override public String toString() { - return "CreatedBy {" + this.creatorType.getName() + "='" + this.creator + '\'' + '}'; + return "CreatedBy {user='" + this.username + '\'' + '}'; } @Override @@ -57,32 +50,32 @@ public String getWriteableName() { @Override public void writeTo(StreamOutput out) throws IOException { - out.writeEnum(Creator.valueOf(creatorType.name())); - out.writeString(creator); + out.writeString(username); } @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - return builder.startObject().field(creatorType.getName(), creator).endObject(); + return builder.startObject().field("user", username).endObject(); } public static CreatedBy fromXContent(XContentParser parser) throws IOException { - String creator = null; - Creator creatorType = null; + String username = null; XContentParser.Token token; while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { if (token == XContentParser.Token.FIELD_NAME) { - creatorType = Creator.fromName(parser.currentName()); + if (!"user".equals(parser.currentName())) { + throw new IllegalArgumentException("created_by must only contain a single field with user"); + } } else if (token == XContentParser.Token.VALUE_STRING) { - creator = parser.text(); + username = parser.text(); } } - if (creator == null) { - throw new IllegalArgumentException(creatorType + " is required"); + if (username == null) { + throw new IllegalArgumentException("created_by cannot be empty"); } - return new CreatedBy(creatorType, creator); + return new CreatedBy(username); } } diff --git a/spi/src/main/java/org/opensearch/security/spi/resources/sharing/Creator.java b/spi/src/main/java/org/opensearch/security/spi/resources/sharing/Creator.java deleted file mode 100644 index 75e2415b93..0000000000 --- a/spi/src/main/java/org/opensearch/security/spi/resources/sharing/Creator.java +++ /dev/null @@ -1,37 +0,0 @@ -/* - * 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.spi.resources.sharing; - -/** - * This enum is used to store information about the creator of a resource. - * - * @opensearch.experimental - */ -public enum Creator { - USER("user"); - - private final String name; - - Creator(String name) { - this.name = name; - } - - public String getName() { - return name; - } - - public static Creator fromName(String name) { - for (Creator creator : values()) { - if (creator.name.equalsIgnoreCase(name)) { // Case-insensitive comparison - return creator; - } - } - throw new IllegalArgumentException("No enum constant for name: " + name); - } -} diff --git a/spi/src/main/java/org/opensearch/security/spi/resources/sharing/Recipient.java b/spi/src/main/java/org/opensearch/security/spi/resources/sharing/Recipient.java index 95d10107cf..ee637c6427 100644 --- a/spi/src/main/java/org/opensearch/security/spi/resources/sharing/Recipient.java +++ b/spi/src/main/java/org/opensearch/security/spi/resources/sharing/Recipient.java @@ -29,15 +29,6 @@ public String getName() { return name; } - public static Recipient fromValue(String name) { - for (Recipient recipient : Recipient.values()) { - if (recipient.name.equals(name)) { - return recipient; - } - } - throw new IllegalArgumentException("No Recipient with value: " + name); - } - @Override public String toString() { return name; diff --git a/spi/src/main/java/org/opensearch/security/spi/resources/sharing/Recipients.java b/spi/src/main/java/org/opensearch/security/spi/resources/sharing/Recipients.java new file mode 100644 index 0000000000..0558d91715 --- /dev/null +++ b/spi/src/main/java/org/opensearch/security/spi/resources/sharing/Recipients.java @@ -0,0 +1,136 @@ +/* + * 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.spi.resources.sharing; + +import java.io.IOException; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Locale; +import java.util.Map; +import java.util.Set; + +import org.opensearch.core.common.io.stream.NamedWriteable; +import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.core.common.io.stream.StreamOutput; +import org.opensearch.core.xcontent.ToXContentFragment; +import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.core.xcontent.XContentParser; + +/** + * This class represents the entities with which a resource is shared for a particular action-group. + * Example: + * "default": { + * "users": [], + * "roles": [], + * "backend_roles": [] + * } + * where "users", "roles" and "backend_roles" are the recipient entities, and "default" is the action-group + * + * @opensearch.experimental + */ +public class Recipients implements ToXContentFragment, NamedWriteable { + + /* + * accessLevel is an actionGroup that is pertinent to sharable resources + * + * i.e. With Google Docs I can share a doc with another user of Google Docs and specify the access level + * when sharing + */ + private final Map> recipients; + + public Recipients(Map> recipients) { + this.recipients = recipients; + } + + public Recipients(StreamInput in) throws IOException { + this.recipients = in.readMap(key -> key.readEnum(Recipient.class), input -> input.readSet(StreamInput::readString)); + } + + public Map> getRecipients() { + return recipients; + } + + public void share(Recipients target) { + Map> targetRecipients = target.getRecipients(); + for (Recipient recipientType : targetRecipients.keySet()) { + Set updatedRecipients = recipients.get(recipientType); + updatedRecipients.addAll(targetRecipients.get(recipientType)); + } + } + + public void revoke(Recipients target) { + Map> targetRecipients = target.getRecipients(); + for (Recipient recipientType : targetRecipients.keySet()) { + Set updatedRecipients = recipients.get(recipientType); + updatedRecipients.removeAll(targetRecipients.get(recipientType)); + } + } + + public boolean isPublic() { + return recipients.values().stream().anyMatch(recipients -> recipients.contains("*")); + } + + public boolean isSharedWithAny(Recipient recipientType, Set targets) { + return !Collections.disjoint(recipients.getOrDefault(recipientType, Collections.emptySet()), targets); + } + + public Set getRecipientsByType(Recipient recipientType) { + return recipients.get(recipientType); + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + builder.startObject(); + for (Map.Entry> entry : recipients.entrySet()) { + builder.array(entry.getKey().getName(), entry.getValue().toArray()); + } + return builder.endObject(); + } + + public static Recipients fromXContent(XContentParser parser) throws IOException { + Map> recipients = new HashMap<>(); + + XContentParser.Token token; + while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { + if (token == XContentParser.Token.FIELD_NAME) { + String fieldName = parser.currentName(); + Recipient recipient = Recipient.valueOf(fieldName.toUpperCase(Locale.ROOT)); + + parser.nextToken(); + Set values = new HashSet<>(); + while (parser.nextToken() != XContentParser.Token.END_ARRAY) { + values.add(parser.text()); + } + recipients.put(recipient, values); + } + } + + return new Recipients(recipients); + } + + @Override + public String toString() { + return "{" + recipients + '}'; + } + + @Override + public String getWriteableName() { + return "access_level_recipients"; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeMap( + recipients, + StreamOutput::writeEnum, + (streamOutput, strings) -> streamOutput.writeCollection(strings, StreamOutput::writeString) + ); + } +} diff --git a/spi/src/main/java/org/opensearch/security/spi/resources/sharing/ResourceSharing.java b/spi/src/main/java/org/opensearch/security/spi/resources/sharing/ResourceSharing.java index 007087f9ef..27a9684bd9 100644 --- a/spi/src/main/java/org/opensearch/security/spi/resources/sharing/ResourceSharing.java +++ b/spi/src/main/java/org/opensearch/security/spi/resources/sharing/ResourceSharing.java @@ -9,7 +9,7 @@ package org.opensearch.security.spi.resources.sharing; import java.io.IOException; -import java.util.HashSet; +import java.util.Map; import java.util.Objects; import java.util.Set; @@ -46,7 +46,7 @@ public class ResourceSharing implements ToXContentFragment, NamedWriteable { /** * The index where the resource is defined */ - private String sourceIdx; + private String resourceIndex; /** * The unique identifier of the resource @@ -63,8 +63,8 @@ public class ResourceSharing implements ToXContentFragment, NamedWriteable { */ private ShareWith shareWith; - public ResourceSharing(String sourceIdx, String resourceId, CreatedBy createdBy, ShareWith shareWith) { - this.sourceIdx = sourceIdx; + public ResourceSharing(String resourceIndex, String resourceId, CreatedBy createdBy, ShareWith shareWith) { + this.resourceIndex = resourceIndex; this.resourceId = resourceId; this.createdBy = createdBy; this.shareWith = shareWith; @@ -78,12 +78,8 @@ public void setDocId(String docId) { this.docId = docId; } - public String getSourceIdx() { - return sourceIdx; - } - - public void setSourceIdx(String sourceIdx) { - this.sourceIdx = sourceIdx; + public String getResourceIndex() { + return resourceIndex; } public String getResourceId() { @@ -98,24 +94,26 @@ public CreatedBy getCreatedBy() { return createdBy; } - public void setCreatedBy(CreatedBy createdBy) { - this.createdBy = createdBy; - } - public ShareWith getShareWith() { return shareWith; } - public void setShareWith(ShareWith shareWith) { - this.shareWith = shareWith; + public void share(String accessLevel, Recipients target) { + if (shareWith == null) { + shareWith = new ShareWith(Map.of(accessLevel, target)); + } else { + Recipients sharedWith = shareWith.atAccessLevel(accessLevel); + sharedWith.share(target); + } } - public void share(String accessLevel, SharedWithActionGroup target) { + public void revoke(String accessLevel, Recipients target) { if (shareWith == null) { - shareWith = new ShareWith(Set.of(target)); + // TODO log a warning that this is a noop + return; } else { - SharedWithActionGroup sharedWith = shareWith.atAccessLevel(accessLevel); - sharedWith.share(target); + Recipients sharedWith = shareWith.atAccessLevel(accessLevel); + sharedWith.revoke(target); } } @@ -124,7 +122,7 @@ public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; ResourceSharing resourceSharing = (ResourceSharing) o; - return Objects.equals(getSourceIdx(), resourceSharing.getSourceIdx()) + return Objects.equals(getResourceIndex(), resourceSharing.getResourceIndex()) && Objects.equals(getResourceId(), resourceSharing.getResourceId()) && Objects.equals(getCreatedBy(), resourceSharing.getCreatedBy()) && Objects.equals(getShareWith(), resourceSharing.getShareWith()); @@ -132,14 +130,14 @@ public boolean equals(Object o) { @Override public int hashCode() { - return Objects.hash(getSourceIdx(), getResourceId(), getCreatedBy(), getShareWith()); + return Objects.hash(getResourceIndex(), getResourceId(), getCreatedBy(), getShareWith()); } @Override public String toString() { return "ResourceSharing {" - + "sourceIdx='" - + sourceIdx + + "resourceIndex='" + + resourceIndex + '\'' + ", resourceId='" + resourceId @@ -158,7 +156,7 @@ public String getWriteableName() { @Override public void writeTo(StreamOutput out) throws IOException { - out.writeString(sourceIdx); + out.writeString(resourceIndex); out.writeString(resourceId); createdBy.writeTo(out); if (shareWith != null) { @@ -171,9 +169,9 @@ public void writeTo(StreamOutput out) throws IOException { @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - builder.startObject().field("source_idx", sourceIdx).field("resource_id", resourceId).field("created_by"); + builder.startObject().field("source_idx", resourceIndex).field("resource_id", resourceId).field("created_by"); createdBy.toXContent(builder, params); - if (shareWith != null && !shareWith.getSharedWithActionGroups().isEmpty()) { + if (shareWith != null) { builder.field("share_with"); shareWith.toXContent(builder, params); } @@ -181,7 +179,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws } public static ResourceSharing fromXContent(XContentParser parser) throws IOException { - String sourceIdx = null; + String resourceIndex = null; String resourceId = null; CreatedBy createdBy = null; ShareWith shareWith = null; @@ -195,7 +193,7 @@ public static ResourceSharing fromXContent(XContentParser parser) throws IOExcep } else { switch (Objects.requireNonNull(currentFieldName)) { case "source_idx": - sourceIdx = parser.text(); + resourceIndex = parser.text(); break; case "resource_id": resourceId = parser.text(); @@ -213,11 +211,11 @@ public static ResourceSharing fromXContent(XContentParser parser) throws IOExcep } } - validateRequiredField("source_idx", sourceIdx); + validateRequiredField("resourceIndex", resourceIndex); validateRequiredField("resource_id", resourceId); validateRequiredField("created_by", createdBy); - return new ResourceSharing(sourceIdx, resourceId, createdBy, shareWith); + return new ResourceSharing(resourceIndex, resourceId, createdBy, shareWith); } private static void validateRequiredField(String field, T value) { @@ -233,7 +231,7 @@ private static void validateRequiredField(String field, T value) { * @return True if the resource is owned by the user, false otherwise. */ public boolean isCreatedBy(String userName) { - return this.createdBy != null && this.createdBy.getCreator().equals(userName); + return this.createdBy != null && this.createdBy.getUsername().equals(userName); } /** @@ -242,37 +240,23 @@ public boolean isCreatedBy(String userName) { * @return True if the resource is shared with everyone, false otherwise. */ public boolean isSharedWithEveryone() { - return this.shareWith != null - && this.shareWith.getSharedWithActionGroups() - .stream() - .anyMatch(sharedWithActionGroup -> sharedWithActionGroup.getActionGroup().equals("*")); + return this.shareWith != null && this.shareWith.isPublic(); } /** * Checks if the given resource is shared with the specified entities. * - * @param recipient The recipient entity - * @param entities The set of entities to check for sharing. - * @param actionGroups The set of action groups to check for sharing. + * @param recipientType The recipient type + * @param targets The set of targets to check for sharing. + * @param accessLevel The access level to check for sharing. * * @return True if the resource is shared with the entities, false otherwise. */ - public boolean isSharedWithEntity(Recipient recipient, Set entities, Set actionGroups) { - if (shareWith == null) { + public boolean isSharedWithEntity(Recipient recipientType, Set targets, String accessLevel) { + if (shareWith == null || shareWith.atAccessLevel(accessLevel) == null) { return false; } - return shareWith.getSharedWithActionGroups() - .stream() - // only keep the action-groups we care about - .filter(sWAG -> actionGroups.contains(sWAG.getActionGroup())) - // for each matching action-group, grab the recipients’ entities for YOUR recipient - .map(sWAG -> sWAG.getSharedWithPerActionGroup().getRecipients().getOrDefault(recipient, Set.of())) - // check intersection with input entities - .anyMatch(sharedEntities -> { - Set intersection = new HashSet<>(sharedEntities); - intersection.retainAll(entities); - return !intersection.isEmpty(); - }); + return shareWith.atAccessLevel(accessLevel).isSharedWithAny(recipientType, targets); } } diff --git a/spi/src/main/java/org/opensearch/security/spi/resources/sharing/ShareWith.java b/spi/src/main/java/org/opensearch/security/spi/resources/sharing/ShareWith.java index 3de22827a0..ba42ac1ffa 100644 --- a/spi/src/main/java/org/opensearch/security/spi/resources/sharing/ShareWith.java +++ b/spi/src/main/java/org/opensearch/security/spi/resources/sharing/ShareWith.java @@ -9,9 +9,9 @@ package org.opensearch.security.spi.resources.sharing; import java.io.IOException; -import java.util.HashSet; +import java.util.HashMap; +import java.util.Map; import java.util.Set; -import java.util.stream.Collectors; import org.opensearch.core.common.io.stream.NamedWriteable; import org.opensearch.core.common.io.stream.StreamInput; @@ -19,6 +19,7 @@ import org.opensearch.core.xcontent.ToXContentFragment; import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.core.xcontent.XContentParser; +import org.opensearch.security.spi.resources.ResourceAccessLevels; /** * This class contains information about whom a resource is shared with and what is the action-group associated with it. @@ -34,7 +35,7 @@ * } * * - * "default" is a place-holder {@link org.opensearch.security.spi.resources.ResourceAccessActionGroups#PLACE_HOLDER } that must be replaced with action-group names once Resource Authorization framework is implemented. + * "default" is a place-holder {@link ResourceAccessLevels#PLACE_HOLDER } that must be replaced with action-group names once Resource Authorization framework is implemented. * * @opensearch.experimental */ @@ -44,41 +45,48 @@ public class ShareWith implements ToXContentFragment, NamedWriteable { /** * A set of objects representing the action-groups and their associated users, roles, and backend roles. */ - private final Set sharedWithActionGroups; + private final Map sharingInfo; - public ShareWith(Set sharedWithActionGroups) { - this.sharedWithActionGroups = sharedWithActionGroups; + public ShareWith(Map sharingInfo) { + this.sharingInfo = sharingInfo; } public ShareWith(StreamInput in) throws IOException { - this.sharedWithActionGroups = in.readSet(SharedWithActionGroup::new); + this.sharingInfo = in.readMap(StreamInput::readString, Recipients::new); } - public Set getSharedWithActionGroups() { - return sharedWithActionGroups; + public boolean isPublic() { + // TODO Contemplate following google doc model of link sharing which has single access level when link sharing is enabled + return sharingInfo.values().stream().anyMatch(Recipients::isPublic); + } + + public boolean isPrivate() { + return sharingInfo == null || sharingInfo.isEmpty(); } public Set accessLevels() { - return sharedWithActionGroups.stream().map(SharedWithActionGroup::getActionGroup).collect(Collectors.toSet()); + return sharingInfo.keySet(); } - public SharedWithActionGroup atAccessLevel(String accessLevel) { - return sharedWithActionGroups.stream().filter(g -> accessLevel.equals(g.getActionGroup())).findFirst().orElse(null); + public Recipients atAccessLevel(String accessLevel) { + return sharingInfo.get(accessLevel); } @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(); - for (SharedWithActionGroup actionGroup : sharedWithActionGroups) { - actionGroup.toXContent(builder, params); + for (String accessLevel : sharingInfo.keySet()) { + builder.field(accessLevel); + Recipients recipients = sharingInfo.get(accessLevel); + recipients.toXContent(builder, params); } return builder.endObject(); } public static ShareWith fromXContent(XContentParser parser) throws IOException { - Set sharedWithActionGroups = new HashSet<>(); + Map sharingInfo = new HashMap<>(); if (parser.currentToken() != XContentParser.Token.START_OBJECT) { parser.nextToken(); @@ -88,12 +96,16 @@ public static ShareWith fromXContent(XContentParser parser) throws IOException { while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { // Each field in the object represents a SharedWithActionGroup if (token == XContentParser.Token.FIELD_NAME) { - SharedWithActionGroup actionGroup = SharedWithActionGroup.fromXContent(parser); - sharedWithActionGroups.add(actionGroup); + String accessLevel = parser.currentName(); + + parser.nextToken(); + + Recipients recipients = Recipients.fromXContent(parser); + sharingInfo.put(accessLevel, recipients); } } - return new ShareWith(sharedWithActionGroups); + return new ShareWith(sharingInfo); } @Override @@ -103,11 +115,11 @@ public String getWriteableName() { @Override public void writeTo(StreamOutput out) throws IOException { - out.writeCollection(sharedWithActionGroups); + out.writeMap(sharingInfo, StreamOutput::writeString, (o, sw) -> sw.writeTo(o)); } @Override public String toString() { - return "ShareWith " + sharedWithActionGroups; + return "ShareWith " + sharingInfo; } } diff --git a/spi/src/main/java/org/opensearch/security/spi/resources/sharing/SharedWithActionGroup.java b/spi/src/main/java/org/opensearch/security/spi/resources/sharing/SharedWithActionGroup.java deleted file mode 100644 index 737f2ba3cf..0000000000 --- a/spi/src/main/java/org/opensearch/security/spi/resources/sharing/SharedWithActionGroup.java +++ /dev/null @@ -1,178 +0,0 @@ -/* - * 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.spi.resources.sharing; - -import java.io.IOException; -import java.util.HashMap; -import java.util.HashSet; -import java.util.Map; -import java.util.Set; - -import org.opensearch.core.common.io.stream.NamedWriteable; -import org.opensearch.core.common.io.stream.StreamInput; -import org.opensearch.core.common.io.stream.StreamOutput; -import org.opensearch.core.xcontent.ToXContentFragment; -import org.opensearch.core.xcontent.XContentBuilder; -import org.opensearch.core.xcontent.XContentParser; - -/** - * This class represents the entities with which a resource is shared for a particular action-group. - * Example: - * "default": { - * "users": [], - * "roles": [], - * "backend_roles": [] - * } - * where "users", "roles" and "backend_roles" are the recipient entities, and "default" is the action-group - * - * @opensearch.experimental - */ -public class SharedWithActionGroup implements ToXContentFragment, NamedWriteable { - - private final String actionGroup; - - private final ActionGroupRecipients actionGroupRecipients; - - public SharedWithActionGroup(String actionGroup, ActionGroupRecipients actionGroupRecipients) { - this.actionGroup = actionGroup; - this.actionGroupRecipients = actionGroupRecipients; - } - - public SharedWithActionGroup(StreamInput in) throws IOException { - this.actionGroup = in.readString(); - this.actionGroupRecipients = new ActionGroupRecipients(in); - } - - public String getActionGroup() { - return actionGroup; - } - - public ActionGroupRecipients getSharedWithPerActionGroup() { - return actionGroupRecipients; - } - - public void share(SharedWithActionGroup target) { - Map> targetRecipients = target.actionGroupRecipients.getRecipients(); - for (Recipient recipientType : targetRecipients.keySet()) { - Set recipients = actionGroupRecipients.getRecipientsByType(recipientType); - recipients.addAll(targetRecipients.get(recipientType)); - } - } - - @Override - public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - builder.field(actionGroup); - builder.startObject(); - - actionGroupRecipients.toXContent(builder, params); - - return builder.endObject(); - } - - public static SharedWithActionGroup fromXContent(XContentParser parser) throws IOException { - String actionGroup = parser.currentName(); - - parser.nextToken(); - - ActionGroupRecipients actionGroupRecipients = ActionGroupRecipients.fromXContent(parser); - - return new SharedWithActionGroup(actionGroup, actionGroupRecipients); - } - - @Override - public String toString() { - return "{" + actionGroup + ": " + actionGroupRecipients + '}'; - } - - @Override - public String getWriteableName() { - return "shared_with_action_group"; - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - out.writeString(actionGroup); - out.writeNamedWriteable(actionGroupRecipients); - } - - /** - * This class represents the entities with whom a resource is shared with for a given action-group. - * - * @opensearch.experimental - */ - public static class ActionGroupRecipients implements ToXContentFragment, NamedWriteable { - - private final Map> recipients; - - public ActionGroupRecipients(Map> recipients) { - if (recipients == null) { - throw new IllegalArgumentException("Recipients map cannot be null"); - } - this.recipients = recipients; - } - - public ActionGroupRecipients(StreamInput in) throws IOException { - this.recipients = in.readMap(key -> key.readEnum(Recipient.class), input -> input.readSet(StreamInput::readString)); - } - - public Map> getRecipients() { - return recipients; - } - - public Set getRecipientsByType(Recipient recipientType) { - return recipients.computeIfAbsent(recipientType, key -> new HashSet<>()); - } - - @Override - public String getWriteableName() { - return "action_group_recipients"; - } - - public static ActionGroupRecipients fromXContent(XContentParser parser) throws IOException { - Map> recipients = new HashMap<>(); - - XContentParser.Token token; - while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { - if (token == XContentParser.Token.FIELD_NAME) { - String fieldName = parser.currentName(); - Recipient recipient = Recipient.fromValue(fieldName); - - parser.nextToken(); - Set values = new HashSet<>(); - while (parser.nextToken() != XContentParser.Token.END_ARRAY) { - values.add(parser.text()); - } - recipients.put(recipient, values); - } - } - - return new ActionGroupRecipients(recipients); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - out.writeMap( - recipients, - StreamOutput::writeEnum, - (streamOutput, strings) -> streamOutput.writeCollection(strings, StreamOutput::writeString) - ); - } - - @Override - public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - if (recipients.isEmpty()) { - return builder; - } - for (Map.Entry> entry : recipients.entrySet()) { - builder.array(entry.getKey().getName(), entry.getValue().toArray()); - } - return builder; - } - } -} diff --git a/spi/src/test/java/org/opensearch/security/spi/resources/CreatedByTests.java b/spi/src/test/java/org/opensearch/security/spi/resources/CreatedByTests.java index 7d6eb5c61a..93f5892ce1 100644 --- a/spi/src/test/java/org/opensearch/security/spi/resources/CreatedByTests.java +++ b/spi/src/test/java/org/opensearch/security/spi/resources/CreatedByTests.java @@ -21,13 +21,11 @@ import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.core.xcontent.XContentParser; import org.opensearch.security.spi.resources.sharing.CreatedBy; -import org.opensearch.security.spi.resources.sharing.Creator; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; -import static org.hamcrest.Matchers.nullValue; import static org.junit.Assert.assertThrows; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -39,14 +37,12 @@ */ public class CreatedByTests { - private static final Creator CREATOR_TYPE = Creator.USER; - @Test public void testCreatedByConstructorWithValidUser() { String expectedUser = "testUser"; - CreatedBy createdBy = new CreatedBy(CREATOR_TYPE, expectedUser); + CreatedBy createdBy = new CreatedBy(expectedUser); - MatcherAssert.assertThat(expectedUser, is(equalTo(createdBy.getCreator()))); + MatcherAssert.assertThat(expectedUser, is(equalTo(createdBy.getUsername()))); } @Test @@ -54,14 +50,13 @@ public void testCreatedByFromStreamInput() throws IOException { String expectedUser = "testUser"; try (BytesStreamOutput out = new BytesStreamOutput()) { - out.writeEnum(Creator.valueOf(CREATOR_TYPE.name())); out.writeString(expectedUser); StreamInput in = out.bytes().streamInput(); CreatedBy createdBy = new CreatedBy(in); - MatcherAssert.assertThat(expectedUser, is(equalTo(createdBy.getCreator()))); + MatcherAssert.assertThat(expectedUser, is(equalTo(createdBy.getUsername()))); } } @@ -78,8 +73,8 @@ public void testCreatedByWithEmptyStreamInput() throws IOException { @Test public void testCreatedByWithEmptyUser() { - CreatedBy createdBy = new CreatedBy(CREATOR_TYPE, ""); - MatcherAssert.assertThat("", equalTo(createdBy.getCreator())); + CreatedBy createdBy = new CreatedBy(""); + MatcherAssert.assertThat("", equalTo(createdBy.getUsername())); } @Test @@ -95,15 +90,15 @@ public void testCreatedByWithIOException() throws IOException { @Test public void testCreatedByWithLongUsername() { String longUsername = "a".repeat(10000); - CreatedBy createdBy = new CreatedBy(CREATOR_TYPE, longUsername); - MatcherAssert.assertThat(longUsername, equalTo(createdBy.getCreator())); + CreatedBy createdBy = new CreatedBy(longUsername); + MatcherAssert.assertThat(longUsername, equalTo(createdBy.getUsername())); } @Test public void testCreatedByWithUnicodeCharacters() { String unicodeUsername = "用户こんにちは"; - CreatedBy createdBy = new CreatedBy(CREATOR_TYPE, unicodeUsername); - MatcherAssert.assertThat(unicodeUsername, equalTo(createdBy.getCreator())); + CreatedBy createdBy = new CreatedBy(unicodeUsername); + MatcherAssert.assertThat(unicodeUsername, equalTo(createdBy.getUsername())); } @Test @@ -115,7 +110,7 @@ public void testFromXContentThrowsExceptionWhenUserFieldIsMissing() throws IOExc exception = assertThrows(IllegalArgumentException.class, () -> CreatedBy.fromXContent(parser)); } - MatcherAssert.assertThat("null is required", equalTo(exception.getMessage())); + MatcherAssert.assertThat("created_by cannot be empty", equalTo(exception.getMessage())); } @Test @@ -146,7 +141,7 @@ public void testFromXContentWithIncorrectFieldType() throws IOException { @Test public void testFromXContentWithEmptyUser() throws IOException { - String emptyJson = "{\"" + CREATOR_TYPE + "\": \"\" }"; + String emptyJson = "{\"user\": \"\" }"; CreatedBy createdBy; try (XContentParser parser = JsonXContent.jsonXContent.createParser(null, null, emptyJson)) { parser.nextToken(); @@ -154,8 +149,7 @@ public void testFromXContentWithEmptyUser() throws IOException { createdBy = CreatedBy.fromXContent(parser); } - MatcherAssert.assertThat(CREATOR_TYPE, equalTo(createdBy.getCreatorType())); - MatcherAssert.assertThat("", equalTo(createdBy.getCreator())); + MatcherAssert.assertThat("", equalTo(createdBy.getUsername())); } @Test @@ -175,42 +169,35 @@ public void testFromXContentWithValidUser() throws IOException { CreatedBy createdBy = CreatedBy.fromXContent(parser); MatcherAssert.assertThat(createdBy, notNullValue()); - MatcherAssert.assertThat("testUser", equalTo(createdBy.getCreator())); + MatcherAssert.assertThat("testUser", equalTo(createdBy.getUsername())); } @Test public void testGetCreatorReturnsCorrectValue() { String expectedUser = "testUser"; - CreatedBy createdBy = new CreatedBy(CREATOR_TYPE, expectedUser); + CreatedBy createdBy = new CreatedBy(expectedUser); - String actualUser = createdBy.getCreator(); + String actualUser = createdBy.getUsername(); MatcherAssert.assertThat(expectedUser, equalTo(actualUser)); } - @Test - public void testGetCreatorWithNullString() { - - CreatedBy createdBy = new CreatedBy(CREATOR_TYPE, null); - MatcherAssert.assertThat(createdBy.getCreator(), nullValue()); - } - @Test public void testGetWriteableNameReturnsCorrectString() { - CreatedBy createdBy = new CreatedBy(CREATOR_TYPE, "testUser"); + CreatedBy createdBy = new CreatedBy("testUser"); MatcherAssert.assertThat("created_by", equalTo(createdBy.getWriteableName())); } @Test public void testToStringWithEmptyUser() { - CreatedBy createdBy = new CreatedBy(CREATOR_TYPE, ""); + CreatedBy createdBy = new CreatedBy(""); String result = createdBy.toString(); MatcherAssert.assertThat("CreatedBy {user=''}", equalTo(result)); } @Test public void testToStringWithNullUser() { - CreatedBy createdBy = new CreatedBy(CREATOR_TYPE, (String) null); + CreatedBy createdBy = new CreatedBy((String) null); String result = createdBy.toString(); MatcherAssert.assertThat("CreatedBy {user='null'}", equalTo(result)); } @@ -219,7 +206,7 @@ public void testToStringWithNullUser() { public void testToStringWithLongUserName() { String longUserName = "a".repeat(1000); - CreatedBy createdBy = new CreatedBy(CREATOR_TYPE, longUserName); + CreatedBy createdBy = new CreatedBy(longUserName); String result = createdBy.toString(); MatcherAssert.assertThat(result.startsWith("CreatedBy {user='"), is(true)); MatcherAssert.assertThat(result.endsWith("'}"), is(true)); @@ -228,7 +215,7 @@ public void testToStringWithLongUserName() { @Test public void testToXContentWithEmptyUser() throws IOException { - CreatedBy createdBy = new CreatedBy(CREATOR_TYPE, ""); + CreatedBy createdBy = new CreatedBy(""); XContentBuilder builder = JsonXContent.contentBuilder(); createdBy.toXContent(builder, null); @@ -238,7 +225,7 @@ public void testToXContentWithEmptyUser() throws IOException { @Test public void testWriteToWithExceptionInStreamOutput() throws IOException { - CreatedBy createdBy = new CreatedBy(CREATOR_TYPE, "user1"); + CreatedBy createdBy = new CreatedBy("user1"); try (StreamOutput failingOutput = new StreamOutput() { @Override public void writeByte(byte b) throws IOException { @@ -273,7 +260,7 @@ public void reset() throws IOException { @Test public void testWriteToWithLongUserName() throws IOException { String longUserName = "a".repeat(65536); - CreatedBy createdBy = new CreatedBy(CREATOR_TYPE, longUserName); + CreatedBy createdBy = new CreatedBy(longUserName); BytesStreamOutput out = new BytesStreamOutput(); createdBy.writeTo(out); MatcherAssert.assertThat(out.size(), greaterThan(65536)); @@ -282,7 +269,7 @@ public void testWriteToWithLongUserName() throws IOException { @Test public void test_createdByToStringReturnsCorrectFormat() { String testUser = "testUser"; - CreatedBy createdBy = new CreatedBy(CREATOR_TYPE, testUser); + CreatedBy createdBy = new CreatedBy(testUser); String expected = "CreatedBy {user='" + testUser + "'}"; String actual = createdBy.toString(); @@ -293,7 +280,7 @@ public void test_createdByToStringReturnsCorrectFormat() { @Test public void test_toXContent_serializesCorrectly() throws IOException { String expectedUser = "testUser"; - CreatedBy createdBy = new CreatedBy(CREATOR_TYPE, expectedUser); + CreatedBy createdBy = new CreatedBy(expectedUser); XContentBuilder builder = XContentFactory.jsonBuilder(); createdBy.toXContent(builder, null); @@ -305,13 +292,12 @@ public void test_toXContent_serializesCorrectly() throws IOException { @Test public void test_writeTo_writesUserCorrectly() throws IOException { String expectedUser = "testUser"; - CreatedBy createdBy = new CreatedBy(CREATOR_TYPE, expectedUser); + CreatedBy createdBy = new CreatedBy(expectedUser); BytesStreamOutput out = new BytesStreamOutput(); createdBy.writeTo(out); StreamInput in = out.bytes().streamInput(); - in.readString(); String actualUser = in.readString(); MatcherAssert.assertThat(expectedUser, equalTo(actualUser)); diff --git a/spi/src/test/java/org/opensearch/security/spi/resources/ShareWithTests.java b/spi/src/test/java/org/opensearch/security/spi/resources/ShareWithTests.java index 034bff2175..fabfb24ddc 100644 --- a/spi/src/test/java/org/opensearch/security/spi/resources/ShareWithTests.java +++ b/spi/src/test/java/org/opensearch/security/spi/resources/ShareWithTests.java @@ -10,7 +10,7 @@ import java.io.IOException; import java.util.Collections; -import java.util.HashSet; +import java.util.HashMap; import java.util.Map; import java.util.Set; @@ -26,19 +26,19 @@ import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.core.xcontent.XContentParser; import org.opensearch.security.spi.resources.sharing.Recipient; +import org.opensearch.security.spi.resources.sharing.Recipients; import org.opensearch.security.spi.resources.sharing.ShareWith; -import org.opensearch.security.spi.resources.sharing.SharedWithActionGroup; import org.mockito.Mockito; import static org.hamcrest.Matchers.contains; -import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; import static org.junit.Assert.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doThrow; -import static org.mockito.Mockito.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -61,16 +61,11 @@ public void testFromXContentWhenCurrentTokenIsNotStartObject() throws IOExceptio ShareWith shareWith = ShareWith.fromXContent(parser); MatcherAssert.assertThat(shareWith, notNullValue()); - Set sharedWithActionGroups = shareWith.getSharedWithActionGroups(); - MatcherAssert.assertThat(sharedWithActionGroups, notNullValue()); - MatcherAssert.assertThat(1, equalTo(sharedWithActionGroups.size())); + Recipients readOnly = shareWith.atAccessLevel("read_only"); + MatcherAssert.assertThat(readOnly, notNullValue()); - SharedWithActionGroup actionGroup = sharedWithActionGroups.iterator().next(); - MatcherAssert.assertThat("read_only", equalTo(actionGroup.getActionGroup())); - - SharedWithActionGroup.ActionGroupRecipients actionGroupRecipients = actionGroup.getSharedWithPerActionGroup(); - MatcherAssert.assertThat(actionGroupRecipients, notNullValue()); - Map> recipients = actionGroupRecipients.getRecipients(); + Map> recipients = readOnly.getRecipients(); + MatcherAssert.assertThat(recipients, notNullValue()); MatcherAssert.assertThat(recipients.get(Recipient.USERS).size(), is(1)); MatcherAssert.assertThat(recipients.get(Recipient.USERS), contains("user1")); MatcherAssert.assertThat(recipients.get(Recipient.ROLES).size(), is(0)); @@ -85,7 +80,8 @@ public void testFromXContentWithEmptyInput() throws IOException { ShareWith result = ShareWith.fromXContent(parser); MatcherAssert.assertThat(result, notNullValue()); - MatcherAssert.assertThat(result.getSharedWithActionGroups(), is(empty())); + MatcherAssert.assertThat(result.isPrivate(), is(true)); + MatcherAssert.assertThat(result.isPublic(), is(false)); } @Test @@ -93,12 +89,12 @@ public void testFromXContentWithStartObject() throws IOException { XContentParser parser; try (XContentBuilder builder = XContentFactory.jsonBuilder()) { builder.startObject() - .startObject(ResourceAccessActionGroups.PLACE_HOLDER) + .startObject(ResourceAccessLevels.PLACE_HOLDER) .array("users", "user1", "user2") .array("roles", "role1") .array("backend_roles", "backend_role1") .endObject() - .startObject("random-action-group") + .startObject("read-only") .array("users", "*") .array("roles", "*") .array("backend_roles", "*") @@ -113,22 +109,21 @@ public void testFromXContentWithStartObject() throws IOException { ShareWith shareWith = ShareWith.fromXContent(parser); MatcherAssert.assertThat(shareWith, notNullValue()); - Set actionGroups = shareWith.getSharedWithActionGroups(); - MatcherAssert.assertThat(actionGroups.size(), equalTo(2)); - - for (SharedWithActionGroup actionGroup : actionGroups) { - SharedWithActionGroup.ActionGroupRecipients perScope = actionGroup.getSharedWithPerActionGroup(); - Map> recipients = perScope.getRecipients(); - if (actionGroup.getActionGroup().equals(ResourceAccessActionGroups.PLACE_HOLDER)) { - MatcherAssert.assertThat(recipients.get(Recipient.USERS).size(), is(2)); - MatcherAssert.assertThat(recipients.get(Recipient.ROLES).size(), is(1)); - MatcherAssert.assertThat(recipients.get(Recipient.BACKEND_ROLES).size(), is(1)); - } else if (actionGroup.getActionGroup().equals("random-action-group")) { - MatcherAssert.assertThat(recipients.get(Recipient.USERS).size(), is(1)); - MatcherAssert.assertThat(recipients.get(Recipient.ROLES).size(), is(1)); - MatcherAssert.assertThat(recipients.get(Recipient.BACKEND_ROLES).size(), is(1)); - } - } + + Recipients defaultAccessLevel = shareWith.atAccessLevel(ResourceAccessLevels.PLACE_HOLDER); + + Recipients readOnly = shareWith.atAccessLevel("read-only"); + + MatcherAssert.assertThat(defaultAccessLevel, notNullValue()); + MatcherAssert.assertThat(readOnly, notNullValue()); + + MatcherAssert.assertThat(defaultAccessLevel.getRecipientsByType(Recipient.USERS).size(), is(2)); + MatcherAssert.assertThat(defaultAccessLevel.getRecipientsByType(Recipient.ROLES).size(), is(1)); + MatcherAssert.assertThat(defaultAccessLevel.getRecipientsByType(Recipient.BACKEND_ROLES).size(), is(1)); + + MatcherAssert.assertThat(readOnly.getRecipientsByType(Recipient.USERS).size(), is(1)); + MatcherAssert.assertThat(readOnly.getRecipientsByType(Recipient.ROLES).size(), is(1)); + MatcherAssert.assertThat(readOnly.getRecipientsByType(Recipient.BACKEND_ROLES).size(), is(1)); } @Test @@ -140,20 +135,15 @@ public void testFromXContentWithUnexpectedEndOfInput() throws IOException { ShareWith result = ShareWith.fromXContent(mockParser); MatcherAssert.assertThat(result, notNullValue()); - MatcherAssert.assertThat(result.getSharedWithActionGroups(), is(empty())); + MatcherAssert.assertThat(result.isPrivate(), is(true)); + MatcherAssert.assertThat(result.isPublic(), is(false)); } @Test public void testToXContentBuildsCorrectly() throws IOException { - SharedWithActionGroup actionGroup = new SharedWithActionGroup( - "actionGroup1", - new SharedWithActionGroup.ActionGroupRecipients(Map.of(Recipient.USERS, Set.of("bleh"))) - ); - - Set actionGroups = new HashSet<>(); - actionGroups.add(actionGroup); + Recipients actionGroup = new Recipients(Map.of(Recipient.USERS, Set.of("bleh"))); - ShareWith shareWith = new ShareWith(actionGroups); + ShareWith shareWith = new ShareWith(Map.of("actionGroup1", actionGroup)); XContentBuilder builder = JsonXContent.contentBuilder(); @@ -169,39 +159,39 @@ public void testToXContentBuildsCorrectly() throws IOException { @Test public void testWriteToWithEmptySet() throws IOException { - Set emptySet = Collections.emptySet(); - ShareWith shareWith = new ShareWith(emptySet); + Map emptyMap = Collections.emptyMap(); + ShareWith shareWith = new ShareWith(emptyMap); StreamOutput mockOutput = Mockito.mock(StreamOutput.class); shareWith.writeTo(mockOutput); - verify(mockOutput).writeCollection(emptySet); + verify(mockOutput).writeMap(eq(emptyMap), any(), any()); } @Test public void testWriteToWithIOException() throws IOException { - Set set = new HashSet<>(); - set.add(new SharedWithActionGroup("test", new SharedWithActionGroup.ActionGroupRecipients(Map.of()))); - ShareWith shareWith = new ShareWith(set); + Recipients recipients = new Recipients(Map.of()); + Map map = Map.of("test", recipients); + ShareWith shareWith = new ShareWith(map); StreamOutput mockOutput = Mockito.mock(StreamOutput.class); - doThrow(new IOException("Simulated IO exception")).when(mockOutput).writeCollection(set); + doThrow(new IOException("Simulated IO exception")).when(mockOutput).writeMap(eq(map), any(), any()); assertThrows(IOException.class, () -> shareWith.writeTo(mockOutput)); } @Test public void testWriteToWithLargeSet() throws IOException { - Set largeSet = new HashSet<>(); - for (int i = 0; i < 10000; i++) { - largeSet.add(new SharedWithActionGroup("actionGroup" + i, new SharedWithActionGroup.ActionGroupRecipients(Map.of()))); + Map largeMap = new HashMap<>(); + for (int i = 0; i < 10; i++) { + largeMap.put("actionGroup" + i, new Recipients(Map.of())); } - ShareWith shareWith = new ShareWith(largeSet); + ShareWith shareWith = new ShareWith(largeMap); StreamOutput mockOutput = Mockito.mock(StreamOutput.class); shareWith.writeTo(mockOutput); - verify(mockOutput).writeCollection(largeSet); + verify(mockOutput).writeMap(eq(largeMap), any(), any()); } @Test @@ -214,38 +204,20 @@ public void test_fromXContent_emptyObject() throws IOException { ShareWith shareWith = ShareWith.fromXContent(parser); - MatcherAssert.assertThat(shareWith.getSharedWithActionGroups(), is(empty())); + MatcherAssert.assertThat(shareWith.isPrivate(), is(true)); + MatcherAssert.assertThat(shareWith.isPublic(), is(false)); } @Test public void test_writeSharedWithScopesToStream() throws IOException { StreamOutput mockStreamOutput = Mockito.mock(StreamOutput.class); - Set sharedWithActionGroups = new HashSet<>(); - sharedWithActionGroups.add( - new SharedWithActionGroup(ResourceAccessActionGroups.PLACE_HOLDER, new SharedWithActionGroup.ActionGroupRecipients(Map.of())) - ); + Map map = Map.of(ResourceAccessLevels.PLACE_HOLDER, new Recipients(Map.of())); - ShareWith shareWith = new ShareWith(sharedWithActionGroups); + ShareWith shareWith = new ShareWith(map); shareWith.writeTo(mockStreamOutput); - verify(mockStreamOutput, times(1)).writeCollection(eq(sharedWithActionGroups)); - } -} - -enum DefaultRecipient { - USERS("users"), - ROLES("roles"), - BACKEND_ROLES("backend_roles"); - - private final String name; - - DefaultRecipient(String name) { - this.name = name; - } - - public String getName() { - return name; + verify(mockStreamOutput, times(1)).writeMap(eq(map), any(), any()); } } diff --git a/src/main/java/org/opensearch/security/resources/ResourceAccessControlClient.java b/src/main/java/org/opensearch/security/resources/ResourceAccessControlClient.java index a0aadef079..9446faafd1 100644 --- a/src/main/java/org/opensearch/security/resources/ResourceAccessControlClient.java +++ b/src/main/java/org/opensearch/security/resources/ResourceAccessControlClient.java @@ -15,11 +15,10 @@ import org.opensearch.common.settings.Settings; import org.opensearch.core.action.ActionListener; -import org.opensearch.security.spi.resources.ResourceAccessActionGroups; +import org.opensearch.security.spi.resources.ResourceAccessLevels; import org.opensearch.security.spi.resources.client.ResourceSharingClient; import org.opensearch.security.spi.resources.sharing.ResourceSharing; import org.opensearch.security.spi.resources.sharing.ShareWith; -import org.opensearch.security.spi.resources.sharing.SharedWithActionGroup; /** * Access control client responsible for handling resource sharing operations such as verifying, @@ -32,7 +31,6 @@ public final class ResourceAccessControlClient implements ResourceSharingClient private static final Logger log = LogManager.getLogger(ResourceAccessControlClient.class); private final ResourceAccessHandler resourceAccessHandler; - private final Settings settings; /** * Constructs a new ResourceAccessControlClient. @@ -40,7 +38,6 @@ public final class ResourceAccessControlClient implements ResourceSharingClient */ public ResourceAccessControlClient(ResourceAccessHandler resourceAccessHandler, Settings settings) { this.resourceAccessHandler = resourceAccessHandler; - this.settings = settings; } /** @@ -51,8 +48,8 @@ public ResourceAccessControlClient(ResourceAccessHandler resourceAccessHandler, * @param listener Callback that receives {@code true} if access is granted, {@code false} otherwise. */ @Override - public void verifyResourceAccess(String resourceId, String resourceIndex, ActionListener listener) { - resourceAccessHandler.hasPermission(resourceId, resourceIndex, Set.of(ResourceAccessActionGroups.PLACE_HOLDER), listener); + public void verifyAccess(String resourceId, String resourceIndex, ActionListener listener) { + resourceAccessHandler.hasPermission(resourceId, resourceIndex, ResourceAccessLevels.PLACE_HOLDER, listener); } /** @@ -60,20 +57,12 @@ public void verifyResourceAccess(String resourceId, String resourceIndex, Action * * @param resourceId The ID of the resource to share. * @param resourceIndex The index containing the resource. - * @param recipients The recipients of the resource, including users, roles, and backend roles. + * @param target The recipients of the resource, including users, roles, and backend roles and respective access levels. * @param listener Callback receiving the updated {@link ResourceSharing} document. */ @Override - public void share( - String resourceId, - String resourceIndex, - SharedWithActionGroup.ActionGroupRecipients recipients, - ActionListener listener - ) { - SharedWithActionGroup sharedWithActionGroup = new SharedWithActionGroup(ResourceAccessActionGroups.PLACE_HOLDER, recipients); - ShareWith shareWith = new ShareWith(Set.of(sharedWithActionGroup)); - - resourceAccessHandler.shareWith(resourceId, resourceIndex, shareWith, listener); + public void share(String resourceId, String resourceIndex, ShareWith target, ActionListener listener) { + resourceAccessHandler.share(resourceId, resourceIndex, target, listener); } /** @@ -81,23 +70,13 @@ public void share( * * @param resourceId The ID of the resource. * @param resourceIndex The index containing the resource. - * @param entitiesToRevoke A map of entities whose access is to be revoked. + * @param target A map of entities whose access is to be revoked. * @param listener Callback receiving the updated {@link ResourceSharing} document. */ @Override - public void revoke( - String resourceId, - String resourceIndex, - SharedWithActionGroup.ActionGroupRecipients entitiesToRevoke, - ActionListener listener - ) { - resourceAccessHandler.revokeAccess( - resourceId, - resourceIndex, - entitiesToRevoke, - Set.of(ResourceAccessActionGroups.PLACE_HOLDER), - listener - ); + public void revoke(String resourceId, String resourceIndex, ShareWith target, ActionListener listener) { + // TODO access level may be unnecessary in this API if a specific user or role can only be provisioned at a single access level + resourceAccessHandler.revoke(resourceId, resourceIndex, target, listener); } /** diff --git a/src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java b/src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java index 5f2b478278..b214221df6 100644 --- a/src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java +++ b/src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java @@ -31,7 +31,6 @@ import org.opensearch.security.spi.resources.sharing.Recipient; import org.opensearch.security.spi.resources.sharing.ResourceSharing; import org.opensearch.security.spi.resources.sharing.ShareWith; -import org.opensearch.security.spi.resources.sharing.SharedWithActionGroup; import org.opensearch.security.support.ConfigConstants; import org.opensearch.security.user.User; import org.opensearch.threadpool.ThreadPool; @@ -120,13 +119,13 @@ public void getAccessibleResourceIdsForCurrentUser(@NonNull String resourceIndex * * @param resourceId The resource ID to check access for. * @param resourceIndex The resource index containing the resource. - * @param actionGroups The set of action groups to check permission for. + * @param accessLevel The access level to check permission for. * @param listener The listener to be notified with the permission check result. */ public void hasPermission( @NonNull String resourceId, @NonNull String resourceIndex, - @NonNull Set actionGroups, + @NonNull String accessLevel, ActionListener listener ) { final UserSubjectImpl userSubject = (UserSubjectImpl) threadContext.getPersistent( @@ -151,7 +150,7 @@ public void hasPermission( Set userRoles = new HashSet<>(user.getSecurityRoles()); Set userBackendRoles = new HashSet<>(user.getRoles()); - this.resourceSharingIndexHandler.fetchResourceSharingDocument(resourceIndex, resourceId, ActionListener.wrap(document -> { + this.resourceSharingIndexHandler.fetchSharingInfo(resourceIndex, resourceId, ActionListener.wrap(document -> { if (document == null) { LOGGER.warn( "ResourceSharing entry not found for '{}' and index '{}'. Access to this resource will be allowed.", @@ -168,9 +167,9 @@ public void hasPermission( userBackendRoles.add("*"); if (document.isCreatedBy(user.getName()) || document.isSharedWithEveryone() - || document.isSharedWithEntity(Recipient.USERS, Set.of(user.getName(), "*"), actionGroups) - || document.isSharedWithEntity(Recipient.ROLES, userRoles, actionGroups) - || document.isSharedWithEntity(Recipient.BACKEND_ROLES, userBackendRoles, actionGroups)) { + || document.isSharedWithEntity(Recipient.USERS, Set.of(user.getName(), "*"), accessLevel) + || document.isSharedWithEntity(Recipient.ROLES, userRoles, accessLevel) + || document.isSharedWithEntity(Recipient.BACKEND_ROLES, userBackendRoles, accessLevel)) { LOGGER.debug("User '{}' has permission to resource '{}'", user.getName(), resourceId); listener.onResponse(true); @@ -194,13 +193,13 @@ public void hasPermission( * * @param resourceId The resource ID to share. * @param resourceIndex The index where resource is store - * @param shareWith The users, roles, and backend roles as well as the action group to share the resource with. + * @param target The users, roles, and backend roles as well as the action group to share the resource with. * @param listener The listener to be notified with the updated ResourceSharing document. */ - public void shareWith( + public void share( @NonNull String resourceId, @NonNull String resourceIndex, - @NonNull ShareWith shareWith, + @NonNull ShareWith target, ActionListener listener ) { final UserSubjectImpl userSubject = (UserSubjectImpl) threadContext.getPersistent( @@ -219,21 +218,21 @@ public void shareWith( return; } - LOGGER.debug("Sharing resource {} created by {} with {}", resourceId, user.getName(), shareWith.toString()); + LOGGER.debug("Sharing resource {} created by {} with {}", resourceId, user.getName(), target.toString()); boolean isAdmin = adminDNs.isAdmin(user); - this.resourceSharingIndexHandler.updateResourceSharingInfo( + this.resourceSharingIndexHandler.updateSharingInfo( resourceId, resourceIndex, user.getName(), - shareWith, + target, isAdmin, - ActionListener.wrap(updatedResourceSharing -> { - LOGGER.debug("Successfully shared resource {} with {}", resourceId, shareWith.toString()); - listener.onResponse(updatedResourceSharing); + ActionListener.wrap(sharingInfo -> { + LOGGER.debug("Successfully shared resource {} with {}", resourceId, target.toString()); + listener.onResponse(sharingInfo); }, e -> { - LOGGER.error("Failed to share resource {} with {}: {}", resourceId, shareWith.toString(), e.getMessage()); + LOGGER.error("Failed to share resource {} with {}: {}", resourceId, target.toString(), e.getMessage()); listener.onFailure(e); }) ); @@ -244,15 +243,13 @@ public void shareWith( * * @param resourceId The resource ID to revoke access from. * @param resourceIndex The index where resource is store - * @param revokeAccess The users, roles, and backend roles to revoke access for. - * @param actionGroups The action groups to revoke access for. + * @param target The access levels, users, roles, and backend roles to revoke access for. * @param listener The listener to be notified with the updated ResourceSharing document. */ - public void revokeAccess( + public void revoke( @NonNull String resourceId, @NonNull String resourceIndex, - @NonNull SharedWithActionGroup.ActionGroupRecipients revokeAccess, - @NonNull Set actionGroups, + @NonNull ShareWith target, ActionListener listener ) { final UserSubjectImpl userSubject = (UserSubjectImpl) threadContext.getPersistent( @@ -271,15 +268,14 @@ public void revokeAccess( return; } - LOGGER.debug("User {} revoking access to resource {} for {}.", user.getName(), resourceId, revokeAccess); + LOGGER.debug("User {} revoking access to resource {} for {}.", user.getName(), resourceId, target); boolean isAdmin = adminDNs.isAdmin(user); - this.resourceSharingIndexHandler.revokeAccess( + this.resourceSharingIndexHandler.revoke( resourceId, resourceIndex, - revokeAccess, - actionGroups, + target, user.getName(), isAdmin, ActionListener.wrap(listener::onResponse, exception -> { @@ -289,21 +285,6 @@ public void revokeAccess( ); } - /** - * Deletes all resource sharing records for the current user. - * - * @param listener The listener to be notified with the deletion result. - */ - public void deleteAllResourceSharingRecordsForUser(String name, ActionListener listener) { - - LOGGER.debug("Deleting all resource sharing records for user {}", name); - - resourceSharingIndexHandler.deleteAllResourceSharingRecordsForUser(name, ActionListener.wrap(listener::onResponse, exception -> { - LOGGER.error("Failed to delete all resource sharing records for user {}: {}", name, exception.getMessage(), exception); - listener.onFailure(exception); - })); - } - /** * Loads all resources within the specified resource index. * diff --git a/src/main/java/org/opensearch/security/resources/ResourceIndexListener.java b/src/main/java/org/opensearch/security/resources/ResourceIndexListener.java index ba656fe470..811c998e3f 100644 --- a/src/main/java/org/opensearch/security/resources/ResourceIndexListener.java +++ b/src/main/java/org/opensearch/security/resources/ResourceIndexListener.java @@ -20,7 +20,6 @@ import org.opensearch.index.shard.IndexingOperationListener; import org.opensearch.security.auth.UserSubjectImpl; import org.opensearch.security.spi.resources.sharing.CreatedBy; -import org.opensearch.security.spi.resources.sharing.Creator; import org.opensearch.security.spi.resources.sharing.ResourceSharing; import org.opensearch.security.support.ConfigConstants; import org.opensearch.security.user.User; @@ -73,7 +72,7 @@ public void postIndex(ShardId shardId, Engine.Index index, Engine.IndexResult re ResourceSharing sharing = this.resourceSharingIndexHandler.indexResourceSharing( resourceId, resourceIndex, - new CreatedBy(Creator.USER, user.getName()), + new CreatedBy(user.getName()), null ); log.debug( diff --git a/src/main/java/org/opensearch/security/resources/ResourcePluginInfo.java b/src/main/java/org/opensearch/security/resources/ResourcePluginInfo.java index 2526b00a77..6470d3c26b 100644 --- a/src/main/java/org/opensearch/security/resources/ResourcePluginInfo.java +++ b/src/main/java/org/opensearch/security/resources/ResourcePluginInfo.java @@ -33,11 +33,5 @@ public void setResourceSharingExtensions(Set extension public Set getResourceSharingExtensions() { return ImmutableSet.copyOf(resourceSharingExtensions); } - - // TODO following should be removed once core test framework allows loading extended classes - public Set getResourceSharingExtensionsMutable() { - return resourceSharingExtensions; - } - } // CS-ENFORCE-SINGLE diff --git a/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java b/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java index 741b06098a..1f0463ca1a 100644 --- a/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java +++ b/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java @@ -10,14 +10,10 @@ package org.opensearch.security.resources; import java.io.IOException; -import java.util.ArrayList; -import java.util.HashMap; import java.util.HashSet; -import java.util.List; import java.util.Map; import java.util.Set; -import com.fasterxml.jackson.core.type.TypeReference; import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -37,13 +33,11 @@ import org.opensearch.common.unit.TimeValue; import org.opensearch.common.util.concurrent.ThreadContext; import org.opensearch.common.xcontent.LoggingDeprecationHandler; -import org.opensearch.common.xcontent.XContentFactory; import org.opensearch.common.xcontent.XContentType; import org.opensearch.core.action.ActionListener; import org.opensearch.core.rest.RestStatus; import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.core.xcontent.ToXContent; -import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.core.xcontent.XContentParser; import org.opensearch.index.query.AbstractQueryBuilder; import org.opensearch.index.query.BoolQueryBuilder; @@ -52,19 +46,16 @@ import org.opensearch.index.reindex.BulkByScrollResponse; import org.opensearch.index.reindex.DeleteByQueryAction; import org.opensearch.index.reindex.DeleteByQueryRequest; -import org.opensearch.index.reindex.UpdateByQueryAction; -import org.opensearch.index.reindex.UpdateByQueryRequest; import org.opensearch.script.Script; import org.opensearch.script.ScriptType; import org.opensearch.search.Scroll; import org.opensearch.search.SearchHit; import org.opensearch.search.builder.SearchSourceBuilder; -import org.opensearch.security.DefaultObjectMapper; import org.opensearch.security.spi.resources.sharing.CreatedBy; import org.opensearch.security.spi.resources.sharing.Recipient; +import org.opensearch.security.spi.resources.sharing.Recipients; import org.opensearch.security.spi.resources.sharing.ResourceSharing; import org.opensearch.security.spi.resources.sharing.ShareWith; -import org.opensearch.security.spi.resources.sharing.SharedWithActionGroup; import org.opensearch.threadpool.ThreadPool; import org.opensearch.transport.client.Client; @@ -211,7 +202,7 @@ public ResourceSharing indexResourceSharing(String resourceId, String resourceIn * } * * - * @param pluginIndex The source index to match against the source_idx field + * @param resourceIndex The source index to match against the source_idx field * @param listener The listener to be notified when the operation completes. * The listener receives a set of resource IDs as a result. * @apiNote This method: @@ -221,22 +212,22 @@ public ResourceSharing indexResourceSharing(String resourceId, String resourceIn *
  • Returns an empty get instead of throwing exceptions
    • * */ - public void fetchAllResourceIds(String pluginIndex, ActionListener> listener) { - LOGGER.debug("Fetching all documents asynchronously from {} where source_idx = {}", resourceSharingIndex, pluginIndex); + public void fetchAllResourceIds(String resourceIndex, ActionListener> listener) { + LOGGER.debug("Fetching all documents asynchronously from {} where source_idx = {}", resourceSharingIndex, resourceIndex); Scroll scroll = new Scroll(TimeValue.timeValueMinutes(1L)); try (ThreadContext.StoredContext ctx = threadPool.getThreadContext().stashContext()) { final SearchRequest searchRequest = new SearchRequest(resourceSharingIndex); searchRequest.scroll(scroll); - TermQueryBuilder query = QueryBuilders.termQuery("source_idx.keyword", pluginIndex); + TermQueryBuilder query = QueryBuilders.termQuery("source_idx.keyword", resourceIndex); executeSearchRequest(scroll, searchRequest, query, ActionListener.wrap(resourceIds -> { LOGGER.debug("Found {} documents in {}", resourceIds.size(), resourceSharingIndex); listener.onResponse(resourceIds); }, exception -> { - LOGGER.error("Search failed while locating all records inside pluginIndex={} ", pluginIndex, exception); + LOGGER.error("Search failed while locating all records inside resourceIndex={} ", resourceIndex, exception); listener.onFailure(exception); })); @@ -250,7 +241,7 @@ public void fetchAllResourceIds(String pluginIndex, ActionListener> * This method uses scroll API to handle large result sets efficiently. * * - * @param pluginIndex The source index to match against the source_idx field + * @param resourceIndex The source index to match against the source_idx field * @param entities Set of values to match in the specified Recipient field. Used for logging. ActionGroupQuery is already updated with these values. * @param actionGroupQuery The query to match against the action-group field * @param listener The listener to be notified when the operation completes. @@ -266,7 +257,7 @@ public void fetchAllResourceIds(String pluginIndex, ActionListener> * */ public void fetchSharedDocuments( - String pluginIndex, + String resourceIndex, Set entities, BoolQueryBuilder actionGroupQuery, ActionListener> listener @@ -277,7 +268,7 @@ public void fetchSharedDocuments( SearchRequest searchRequest = new SearchRequest(resourceSharingIndex); searchRequest.scroll(scroll); - BoolQueryBuilder boolQuery = QueryBuilders.boolQuery().must(QueryBuilders.termQuery("source_idx.keyword", pluginIndex)); + BoolQueryBuilder boolQuery = QueryBuilders.boolQuery().must(QueryBuilders.termQuery("source_idx.keyword", resourceIndex)); boolQuery.must(QueryBuilders.existsQuery("share_with")).must(actionGroupQuery); @@ -286,15 +277,15 @@ public void fetchSharedDocuments( listener.onResponse(resourceIds); }, exception -> { - LOGGER.error("Search failed for pluginIndex={}, entities={}", pluginIndex, entities, exception); + LOGGER.error("Search failed for resourceIndex={}, entities={}", resourceIndex, entities, exception); listener.onFailure(exception); })); } catch (Exception e) { LOGGER.error( - "Failed to initiate fetch from {} for criteria - pluginIndex: {}, entities: {}", + "Failed to initiate fetch from {} for criteria - resourceIndex: {}, entities: {}", resourceSharingIndex, - pluginIndex, + resourceIndex, entities, e ); @@ -334,7 +325,7 @@ public void fetchSharedDocuments( * @param resourceId The resource ID to fetch. Must exactly match the resource_id field * @param listener The listener to be notified when the operation completes. * The listener receives the parsed ResourceSharing object or null if not found - * @throws IllegalArgumentException if pluginIndexName or resourceId is null or empty + * @throws IllegalArgumentException if resourceIndex or resourceId is null or empty * @throws RuntimeException if the search operation fails or parsing errors occur, * wrapping the underlying exception * @apiNote This method: @@ -354,7 +345,7 @@ public void fetchSharedDocuments( * } * */ - public void fetchResourceSharingDocument(String resourceIndex, String resourceId, ActionListener listener) { + public void fetchSharingInfo(String resourceIndex, String resourceId, ActionListener listener) { if (StringUtils.isBlank(resourceIndex) || StringUtils.isBlank(resourceId)) { listener.onFailure(new IllegalArgumentException("resourceIndex and resourceId must not be null or empty")); return; @@ -478,12 +469,12 @@ public void onFailure(Exception e) { /** * Updates the sharing configuration for an existing resource in the resource sharing index. - * NOTE: This method only grants new access. To remove access use {@link #revokeAccess(String, String, org.opensearch.security.spi.resources.sharing.SharedWithActionGroup.ActionGroupRecipients, Set, String, boolean, ActionListener)} + * NOTE: This method only grants new access. To remove access use {@link #revoke(String, String, ShareWith, String, boolean, ActionListener)} * This method modifies the sharing permissions for a specific resource identified by its * resource ID and source index. * * @param resourceId The unique identifier of the resource whose sharing configuration needs to be updated - * @param sourceIdx The source index where the original resource is stored + * @param resourceIndex The source index where the original resource is stored * @param requestUserName The user requesting to revoke the resource * @param shareWith Updated sharing configuration object containing access control settings: * { @@ -498,37 +489,23 @@ public void onFailure(Exception e) { * @throws RuntimeException if there's an error during the update operation */ @SuppressWarnings("unchecked") - public void updateResourceSharingInfo( + public void updateSharingInfo( String resourceId, - String sourceIdx, + String resourceIndex, String requestUserName, ShareWith shareWith, boolean isAdmin, ActionListener listener ) { - XContentBuilder builder; - Map shareWithMap; - try { - builder = XContentFactory.jsonBuilder(); - shareWith.toXContent(builder, ToXContent.EMPTY_PARAMS); - String json = builder.toString(); - shareWithMap = DefaultObjectMapper.readValue(json, new TypeReference<>() { - }); - } catch (IOException e) { - LOGGER.error("Failed to build json content", e); - listener.onFailure(new OpenSearchStatusException("Failed to build json content", RestStatus.INTERNAL_SERVER_ERROR)); - return; - } - - StepListener fetchDocListener = new StepListener<>(); + StepListener sharingInfoListener = new StepListener<>(); // Fetch resource sharing doc - fetchResourceSharingDocument(sourceIdx, resourceId, fetchDocListener); + fetchSharingInfo(resourceIndex, resourceId, sharingInfoListener); // build update script - fetchDocListener.whenComplete(sharingInfo -> { + sharingInfoListener.whenComplete(sharingInfo -> { // Check if user can share. At present only the resource creator and admin is allowed to share the resource - if (!isAdmin && sharingInfo != null && !sharingInfo.getCreatedBy().getCreator().equals(requestUserName)) { + if (!isAdmin && sharingInfo != null && !sharingInfo.getCreatedBy().getUsername().equals(requestUserName)) { LOGGER.error("User {} is not authorized to share resource {}", requestUserName, resourceId); listener.onFailure( @@ -541,8 +518,7 @@ public void updateResourceSharingInfo( } for (String accessLevel : shareWith.accessLevels()) { - SharedWithActionGroup target = shareWith.atAccessLevel(accessLevel); - assert sharingInfo != null; + Recipients target = shareWith.atAccessLevel(accessLevel); sharingInfo.share(accessLevel, target); } @@ -555,7 +531,12 @@ public void updateResourceSharingInfo( .request(); ActionListener irListener = ActionListener.wrap(idxResponse -> { - LOGGER.info("Successfully updated {} entry for resource {} in index {}.", resourceSharingIndex, resourceId, sourceIdx); + LOGGER.info( + "Successfully updated {} entry for resource {} in index {}.", + resourceSharingIndex, + resourceId, + resourceIndex + ); listener.onResponse(sharingInfo); }, (failResponse) -> { LOGGER.error(failResponse.getMessage()); }); client.index(ir, irListener); @@ -592,55 +573,43 @@ public void updateResourceSharingInfo( * * * @param resourceId The ID of the resource from which to revoke access - * @param sourceIdx The name of the system index where the resource exists + * @param resourceIndex The name of the system index where the resource exists * @param revokeAccess A map containing entity types (USER, ROLE, BACKEND_ROLE) and their corresponding * values to be removed from the sharing configuration - * @param actionGroups A set of action-groups to revoke access from. If null or empty, access is revoked from all action-groups * @param requestUserName The user trying to revoke the accesses * @param isAdmin Boolean indicating whether the user is an admin or not * @param listener Listener to be notified when the operation completes - * @throws IllegalArgumentException if resourceId, sourceIdx is null/empty, or if revokeAccess is null/empty + * @throws IllegalArgumentException if resourceId, resourceIndex is null/empty, or if revokeAccess is null/empty * @throws RuntimeException if the update operation fails or encounters an error * @apiNote This method modifies the existing document. If no modifications are needed (i.e., specified * entities don't exist in the current configuration), the original document is returned unchanged. * @see Recipient * @see ResourceSharing */ - public void revokeAccess( + public void revoke( String resourceId, - String sourceIdx, - SharedWithActionGroup.ActionGroupRecipients revokeAccess, - Set actionGroups, + String resourceIndex, + ShareWith revokeAccess, String requestUserName, boolean isAdmin, ActionListener listener ) { - if (StringUtils.isBlank(resourceId) || StringUtils.isBlank(sourceIdx) || revokeAccess == null) { - listener.onFailure(new IllegalArgumentException("resourceId, sourceIdx, and revokeAccess must not be null or empty")); + if (StringUtils.isBlank(resourceId) || StringUtils.isBlank(resourceIndex) || revokeAccess == null) { + listener.onFailure(new IllegalArgumentException("resourceId, resourceIndex, and revokeAccess must not be null or empty")); return; } try (ThreadContext.StoredContext ctx = this.threadPool.getThreadContext().stashContext()) { - LOGGER.debug( - "Revoking access for resource {} in {} for entities: {} and actionGroups: {}", - resourceId, - sourceIdx, - revokeAccess, - actionGroups - ); - - StepListener currentSharingListener = new StepListener<>(); - StepListener revokeUpdateListener = new StepListener<>(); - StepListener updatedSharingListener = new StepListener<>(); + StepListener sharingInfoListener = new StepListener<>(); // Fetch the current ResourceSharing document - fetchResourceSharingDocument(sourceIdx, resourceId, currentSharingListener); + fetchSharingInfo(resourceIndex, resourceId, sharingInfoListener); // Check permissions & build revoke script - currentSharingListener.whenComplete(currentSharingInfo -> { + sharingInfoListener.whenComplete(sharingInfo -> { // Only admin or the creator of the resource is currently allowed to revoke access - if (!isAdmin && currentSharingInfo != null && !currentSharingInfo.getCreatedBy().getCreator().equals(requestUserName)) { + if (!isAdmin && sharingInfo != null && !sharingInfo.getCreatedBy().getUsername().equals(requestUserName)) { listener.onFailure( new OpenSearchStatusException( "User " + requestUserName + " is not authorized to revoke access to resource " + resourceId, @@ -649,66 +618,36 @@ public void revokeAccess( ); } - Map revoke = new HashMap<>(); - for (Map.Entry> entry : revokeAccess.getRecipients().entrySet()) { - revoke.put(entry.getKey().getName(), new ArrayList<>(entry.getValue())); - } - List actionGroupsToUse = (actionGroups != null) ? new ArrayList<>(actionGroups) : new ArrayList<>(); - - Script revokeScript = new Script( - ScriptType.INLINE, - "painless", - """ - if (ctx._source.share_with != null) { - Set actionGroupsToProcess = new HashSet(params.actionGroups.isEmpty() ? ctx._source.share_with.keySet() : params.actionGroups); - - for (def actionGroupName : actionGroupsToProcess) { - if (ctx._source.share_with.containsKey(actionGroupName)) { - def existingActionGroup = ctx._source.share_with.get(actionGroupName); - - for (def entry : params.revokeAccess.entrySet()) { - def recipient = entry.getKey(); - def entitiesToRemove = entry.getValue(); - - if (existingActionGroup.containsKey(recipient) && existingActionGroup[recipient] != null) { - if (!(existingActionGroup[recipient] instanceof HashSet)) { - existingActionGroup[recipient] = new HashSet(existingActionGroup[recipient]); - } - - existingActionGroup[recipient].removeAll(entitiesToRemove); - - if (existingActionGroup[recipient].isEmpty()) { - existingActionGroup.remove(recipient); - } - } - } - - if (existingActionGroup.isEmpty()) { - ctx._source.share_with.remove(actionGroupName); - } - } - } - } - """, - Map.of("revokeAccess", revoke, "actionGroups", actionGroupsToUse) - ); - updateByQueryResourceSharing(sourceIdx, resourceId, revokeScript, revokeUpdateListener); - - }, listener::onFailure); + for (String accessLevel : revokeAccess.accessLevels()) { + Recipients target = revokeAccess.atAccessLevel(accessLevel); + LOGGER.debug( + "Revoking access for resource {} in {} for entities: {} and accessLevel: {}", + resourceId, + resourceIndex, + target, + accessLevel + ); - // Return doc or null based on successful result, fail otherwise - revokeUpdateListener.whenComplete(success -> { - if (!success) { - LOGGER.error("Failed to revoke access for resource {} in index {} (no docs updated).", resourceId, sourceIdx); - listener.onResponse(null); - return; + sharingInfo.revoke(accessLevel, target); } - // TODO check if this should be replaced by Java in-memory computation (current intuition is that it will be more memory - // intensive to do it in java) - fetchResourceSharingDocument(sourceIdx, resourceId, updatedSharingListener); - }, listener::onFailure); + IndexRequest ir = client.prepareIndex(resourceSharingIndex) + .setId(sharingInfo.getDocId()) + .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE) + .setSource(sharingInfo.toXContent(jsonBuilder(), ToXContent.EMPTY_PARAMS)) + .setOpType(DocWriteRequest.OpType.INDEX) + .request(); - updatedSharingListener.whenComplete(listener::onResponse, listener::onFailure); + ActionListener irListener = ActionListener.wrap(idxResponse -> { + LOGGER.info( + "Successfully updated {} entry for resource {} in index {}.", + resourceSharingIndex, + resourceId, + resourceIndex + ); + listener.onResponse(sharingInfo); + }, (failResponse) -> { LOGGER.error(failResponse.getMessage()); }); + client.index(ir, irListener); + }, listener::onFailure); } } @@ -735,10 +674,10 @@ public void revokeAccess( * } * * - * @param sourceIdx The source index to match in the query (exact match) + * @param resourceIndex The source index to match in the query (exact match) * @param resourceId The resource ID to match in the query (exact match) * @param listener The listener to be notified when the operation completes - * @throws IllegalArgumentException if sourceIdx or resourceId is null/empty + * @throws IllegalArgumentException if resourceIndex or resourceId is null/empty * @throws RuntimeException if the delete operation fails or encounters an error * @implNote The delete operation uses a bool query with two must clauses to ensure exact matching: * 
    @@ -746,7 +685,7 @@ public void revokeAccess(
      *   "query": {
      *     "bool": {
      *       "must": [
-     *         { "term": { "source_idx": sourceIdx } },
+     *         { "term": { "source_idx": resourceIndex } },
      *         { "term": { "resource_id": resourceId } }
      *       ]
      *     }
@@ -754,18 +693,18 @@ public void revokeAccess(
      * }
      *
    */ - public void deleteResourceSharingRecord(String resourceId, String sourceIdx, ActionListener listener) { + public void deleteResourceSharingRecord(String resourceId, String resourceIndex, ActionListener listener) { LOGGER.debug( "Deleting documents asynchronously from {} where source_idx = {} and resource_id = {}", resourceSharingIndex, - sourceIdx, + resourceIndex, resourceId ); try (ThreadContext.StoredContext ctx = this.threadPool.getThreadContext().stashContext()) { DeleteByQueryRequest dbq = new DeleteByQueryRequest(resourceSharingIndex).setQuery( QueryBuilders.boolQuery() - .must(QueryBuilders.termQuery("source_idx.keyword", sourceIdx)) + .must(QueryBuilders.termQuery("source_idx.keyword", resourceIndex)) .must(QueryBuilders.termQuery("resource_id.keyword", resourceId)) ).setRefresh(true); @@ -781,7 +720,7 @@ public void onResponse(BulkByScrollResponse response) { LOGGER.debug( "No documents found to delete in {} for source_idx: {} and resource_id: {}", resourceSharingIndex, - sourceIdx, + resourceIndex, resourceId ); // No documents were deleted @@ -802,181 +741,6 @@ public void onFailure(Exception e) { } } - /** - * Deletes all resource sharing records that were created by a specific user. - * This method performs a delete-by-query operation to remove all documents where - * the created_by.user field matches the specified username. - * - *

    The method executes the following steps: - *

      - *
    1. Validates the input username parameter
      2. - *
    2. Creates a delete-by-query request with term query matching
      3. - *
    3. Executes the delete operation with immediate refresh
      4. - *
    4. Returns the operation status based on number of deleted documents
      5. - *
    - * - *

    Example query structure: - * 

    -     * {
-     *   "query": {
-     *     "term": {
-     *       "created_by.user": "username"
-     *     }
-     *   }
-     * }
-     *
    - * - * @param name The username to match against the created_by.user field - * @param listener The listener to be notified when the operation completes - * @throws IllegalArgumentException if name is null or empty - * @implNote Implementation details: - *
      - *
    • Uses DeleteByQueryRequest for efficient bulk deletion
      • - *
    • Sets refresh=true for immediate consistency
      • - *
    • Uses term query for exact username matching
      • - *
    • Implements comprehensive error handling and logging
      • - *
    - *

    - * Example usage: - * 

    -     * boolean success = deleteAllRecordsForUser("john.doe");
-     * if (success) {
-     *     // Records were successfully deleted
-     * } else {
-     *     // No matching records found or operation failed
-     * }
-     *
    - */ - public void deleteAllResourceSharingRecordsForUser(String name, ActionListener listener) { - if (StringUtils.isBlank(name)) { - listener.onFailure(new IllegalArgumentException("Username must not be null or empty")); - return; - } - - LOGGER.debug("Deleting all records for user {} asynchronously", name); - - try (ThreadContext.StoredContext ctx = this.threadPool.getThreadContext().stashContext()) { - DeleteByQueryRequest deleteRequest = new DeleteByQueryRequest(resourceSharingIndex).setQuery( - QueryBuilders.termQuery("created_by.user", name) - ).setRefresh(true); - - client.execute(DeleteByQueryAction.INSTANCE, deleteRequest, new ActionListener<>() { - @Override - public void onResponse(BulkByScrollResponse response) { - long deletedDocs = response.getDeleted(); - if (deletedDocs > 0) { - LOGGER.debug("Successfully deleted {} documents created by user {}", deletedDocs, name); - listener.onResponse(true); - } else { - LOGGER.warn("No documents found for user {}", name); - // No documents matched => success = false - listener.onResponse(false); - } - } - - @Override - public void onFailure(Exception e) { - LOGGER.error("Failed to delete documents for user {}", name, e); - listener.onFailure(e); - } - }); - } catch (Exception e) { - LOGGER.error("Failed to delete documents for user {} before request submission", name, e); - listener.onFailure(e); - } - } - - /** - * Updates resource sharing entries that match the specified source index and resource ID - * using the provided update script. This method performs an update-by-query operation - * in the resource sharing index. - * - *

    The method executes the following steps: - *

      - *
    1. Creates a bool query to match exact source index and resource ID
      2. - *
    2. Constructs an update-by-query request with the query and update script
      3. - *
    3. Executes the update operation
      4. - *
    4. Returns success/failure status based on update results
      5. - *
    - * - *

    Example document matching structure: - * 

    -     * {
-     *   "source_idx": "source_index_name",
-     *   "resource_id": "resource_id_value",
-     *   "share_with": {
-     *     // sharing configuration to be updated
-     *   }
-     * }
-     *
    - * - * @param sourceIdx The source index to match in the query (exact match) - * @param resourceId The resource ID to match in the query (exact match) - * @param updateScript The script containing the update operations to be performed. - * This script defines how the matching documents should be modified - * @param listener Listener to be notified when the operation completes - * @apiNote This method: - *
      - *
    • Uses term queries for exact matching of source_idx and resource_id
      • - *
    • Returns false for both "no matching documents" and "operation failure" cases
      • - *
    • Logs the complete update request for debugging purposes
      • - *
    • Provides detailed logging for success and failure scenarios
      • - *
    - * @implNote The update operation uses a bool query with two must clauses: - * 
    -     * {
-     *   "query": {
-     *     "bool": {
-     *       "must": [
-     *         { "term": { "source_idx.keyword": sourceIdx } },
-     *         { "term": { "resource_id.keyword": resourceId } }
-     *       ]
-     *     }
-     *   }
-     * }
-     *
    - */ - private void updateByQueryResourceSharing(String sourceIdx, String resourceId, Script updateScript, ActionListener listener) { - try (ThreadContext.StoredContext ctx = this.threadPool.getThreadContext().stashContext()) { - BoolQueryBuilder query = QueryBuilders.boolQuery() - .must(QueryBuilders.termQuery("source_idx.keyword", sourceIdx)) - .must(QueryBuilders.termQuery("resource_id.keyword", resourceId)); - - UpdateByQueryRequest ubq = new UpdateByQueryRequest(resourceSharingIndex).setQuery(query) - .setScript(updateScript) - .setRefresh(true); - - client.execute(UpdateByQueryAction.INSTANCE, ubq, new ActionListener<>() { - @Override - public void onResponse(BulkByScrollResponse response) { - long updated = response.getUpdated(); - if (updated > 0) { - LOGGER.debug("Successfully updated {} documents in {}.", updated, resourceSharingIndex); - listener.onResponse(true); - } else { - LOGGER.debug( - "No documents found to update in {} for source_idx: {} and resource_id: {}", - resourceSharingIndex, - sourceIdx, - resourceId - ); - listener.onResponse(false); - } - } - - @Override - public void onFailure(Exception e) { - LOGGER.error("Failed to update documents in {}.", resourceSharingIndex, e); - listener.onFailure(e); - - } - }); - } catch (Exception e) { - LOGGER.error("Failed to update documents in {} before request submission.", resourceSharingIndex, e); - listener.onFailure(e); - } - } - /** * Executes a search request and returns a set of collected resource IDs using scroll. *