diff --git a/CHANGELOG.md b/CHANGELOG.md index b645ad32e1..a317acc598 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - [SecurityPlugin Health Check] Add AuthZ initialization completion check in health check API [(#5626)](https://github.com/opensearch-project/security/pull/5626) - [Resource Sharing] Adds API to provide dashboards support for resource access management ([#5597](https://github.com/opensearch-project/security/pull/5597)) - Direct JWKS (JSON Web Key Set) support in the JWT authentication backend ([#5578](https://github.com/opensearch-project/security/pull/5578)) +- Adds a list setting to explicitly specify resources to be protected ([#5671](https://github.com/opensearch-project/security/pull/5671)) - Make configuration setting for user custom attribute serialization dynamic ([#5657](https://github.com/opensearch-project/security/pull/5657)) ### Bug Fixes diff --git a/RESOURCE_SHARING_AND_ACCESS_CONTROL.md b/RESOURCE_SHARING_AND_ACCESS_CONTROL.md index 42fc7216cc..0eb99b14d2 100644 --- a/RESOURCE_SHARING_AND_ACCESS_CONTROL.md +++ b/RESOURCE_SHARING_AND_ACCESS_CONTROL.md @@ -506,7 +506,9 @@ Since no entities are listed, the resource is accessible **only by its creator a ## Part 2: Cluster-admin and User guide -## **1. Feature Flag** +## **1. Setup** + +### **Feature Flag** This feature is controlled by the following flag: - **Feature flag:** `plugins.security.experimental.resource_sharing.enabled` @@ -515,6 +517,17 @@ This feature is controlled by the following flag: ```yaml plugins.security.experimental.resource_sharing.enabled: true ``` +### **List protected types** + +The list of protected types are controlled through following opensearch setting + +- **Setting:** `plugins.security.experimental.resource_sharing.protected_types` +- **Default value:** `[]` +- **How to specify a type?** Add entries of existing types in the list: + ```yaml + plugins.security.experimental.resource_sharing.protected_types: [sample-resource] + ``` +NOTE: These types will be available on documentation website. ## **2. User Setup** diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/TestUtils.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/TestUtils.java index dfcd9d0d16..6bd3b40bbc 100644 --- a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/TestUtils.java +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/TestUtils.java @@ -39,9 +39,11 @@ import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.hamcrest.Matchers.is; import static org.opensearch.sample.utils.Constants.RESOURCE_INDEX_NAME; +import static org.opensearch.sample.utils.Constants.RESOURCE_TYPE; import static org.opensearch.sample.utils.Constants.SAMPLE_RESOURCE_PLUGIN_PREFIX; import static org.opensearch.security.resources.ResourceSharingIndexHandler.getSharingIndex; import static org.opensearch.security.support.ConfigConstants.OPENSEARCH_RESOURCE_SHARING_ENABLED; +import static org.opensearch.security.support.ConfigConstants.OPENSEARCH_RESOURCE_SHARING_PROTECTED_TYPES; import static org.opensearch.security.support.ConfigConstants.SECURITY_SYSTEM_INDICES_ENABLED_KEY; import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL; import static org.opensearch.test.framework.TestSecurityConfig.User.USER_ADMIN; @@ -90,6 +92,10 @@ public final class TestUtils { public static final String SECURITY_LIST_ENDPOINT = "_plugins/_security/api/resource/list"; public static LocalCluster newCluster(boolean featureEnabled, boolean systemIndexEnabled) { + return newCluster(featureEnabled, systemIndexEnabled, List.of(RESOURCE_TYPE)); + } + + public static LocalCluster newCluster(boolean featureEnabled, boolean systemIndexEnabled, List protectedResourceTypes) { return new LocalCluster.Builder().clusterManager(ClusterManager.THREE_CLUSTER_MANAGERS_COORDINATOR) .plugin( new PluginInfo( @@ -108,7 +114,14 @@ public static LocalCluster newCluster(boolean featureEnabled, boolean systemInde .authc(AUTHC_HTTPBASIC_INTERNAL) .users(USER_ADMIN, FULL_ACCESS_USER, LIMITED_ACCESS_USER, NO_ACCESS_USER) .nodeSettings( - Map.of(OPENSEARCH_RESOURCE_SHARING_ENABLED, featureEnabled, SECURITY_SYSTEM_INDICES_ENABLED_KEY, systemIndexEnabled) + Map.of( + OPENSEARCH_RESOURCE_SHARING_ENABLED, + featureEnabled, + SECURITY_SYSTEM_INDICES_ENABLED_KEY, + systemIndexEnabled, + OPENSEARCH_RESOURCE_SHARING_PROTECTED_TYPES, + protectedResourceTypes + ) ) .build(); } diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/DirectIndexAccessTests.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/DirectIndexAccessTests.java index 6e209d4efe..11169e2cca 100644 --- a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/DirectIndexAccessTests.java +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/DirectIndexAccessTests.java @@ -284,15 +284,11 @@ public void testRawAccess_allAccessUser() { api.assertDirectShare(userResId, FULL_ACCESS_USER, USER_ADMIN, SAMPLE_READ_ONLY_RESOURCE_AG, HttpStatus.SC_OK); api.assertDirectGet(userResId, USER_ADMIN, HttpStatus.SC_OK, "sample"); - // can only access own resource since api.assertDirectGetSearch(FULL_ACCESS_USER, HttpStatus.SC_OK, 2, "sample"); api.assertDirectPostSearch(searchAllPayload(), FULL_ACCESS_USER, HttpStatus.SC_OK, 2, "sample"); api.assertDirectPostSearch(searchByNamePayload("sample"), FULL_ACCESS_USER, HttpStatus.SC_OK, 1, "sample"); api.assertDirectPostSearch(searchByNamePayload("sampleUser"), FULL_ACCESS_USER, HttpStatus.SC_OK, 1, "sampleUser"); - // cannot update or delete resource - api.assertDirectUpdate(adminResId, FULL_ACCESS_USER, "sampleUpdateAdmin", HttpStatus.SC_FORBIDDEN); - api.assertDirectDelete(adminResId, FULL_ACCESS_USER, HttpStatus.SC_FORBIDDEN); // can update and delete own resource api.assertDirectUpdate(userResId, FULL_ACCESS_USER, "sampleUpdateUser", HttpStatus.SC_OK); api.assertDirectDelete(userResId, FULL_ACCESS_USER, HttpStatus.SC_OK); @@ -302,6 +298,10 @@ public void testRawAccess_allAccessUser() { api.assertDirectShare(adminResId, FULL_ACCESS_USER, NO_ACCESS_USER, SAMPLE_FULL_ACCESS_RESOURCE_AG, HttpStatus.SC_OK); api.assertDirectRevoke(adminResId, FULL_ACCESS_USER, NO_ACCESS_USER, SAMPLE_FULL_ACCESS_RESOURCE_AG, HttpStatus.SC_OK); api.assertDirectDeleteResourceSharingRecord(adminResId, FULL_ACCESS_USER, HttpStatus.SC_OK); + + // can update or delete admin resource, since system index protection is disabled and user has direct index access. + api.assertDirectUpdate(adminResId, FULL_ACCESS_USER, "sampleUpdateAdmin", HttpStatus.SC_OK); + api.assertDirectDelete(adminResId, FULL_ACCESS_USER, HttpStatus.SC_OK); } @Test diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/ExcludedResourceTypeTests.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/ExcludedResourceTypeTests.java new file mode 100644 index 0000000000..a62a0d3762 --- /dev/null +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/ExcludedResourceTypeTests.java @@ -0,0 +1,89 @@ +/* + * 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.sample.resource.feature.enabled; + +import java.util.List; + +import com.carrotsearch.randomizedtesting.RandomizedRunner; +import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; +import org.apache.http.HttpStatus; +import org.junit.After; +import org.junit.Before; +import org.junit.ClassRule; +import org.junit.Test; +import org.junit.runner.RunWith; + +import org.opensearch.sample.resource.TestUtils; +import org.opensearch.test.framework.cluster.LocalCluster; +import org.opensearch.test.framework.cluster.TestRestClient; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.not; +import static org.opensearch.sample.resource.TestUtils.FULL_ACCESS_USER; +import static org.opensearch.sample.resource.TestUtils.LIMITED_ACCESS_USER; +import static org.opensearch.sample.resource.TestUtils.NO_ACCESS_USER; +import static org.opensearch.sample.resource.TestUtils.RESOURCE_SHARING_INDEX; +import static org.opensearch.sample.resource.TestUtils.newCluster; +import static org.opensearch.test.framework.TestSecurityConfig.User.USER_ADMIN; + +/** + * Test resource access when sample-resource is not marked as protected resource, even-though resource sharing protection is enabled. + */ +@RunWith(RandomizedRunner.class) +@ThreadLeakScope(ThreadLeakScope.Scope.NONE) +public class ExcludedResourceTypeTests { + + // do not include sample resource as protected resource, should behave as if feature was disable for that resource + @ClassRule + public static LocalCluster cluster = newCluster(true, true, List.of()); + + private final TestUtils.ApiHelper api = new TestUtils.ApiHelper(cluster); + private String resourceId; + + @Before + public void setup() { + resourceId = api.createSampleResourceAs(USER_ADMIN); + } + + @After + public void cleanup() { + api.wipeOutResourceEntries(); + } + + @Test + public void testSampleResourceSharingIndexDoesNotExist() { + try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) { + TestRestClient.HttpResponse response = client.get("_cat/indices?expand_wildcards=all"); + response.assertStatusCode(HttpStatus.SC_OK); + assertThat(response.getBody(), not(containsString(RESOURCE_SHARING_INDEX))); + } + } + + @Test + public void fullAccessUser_canCRUD() { + api.assertApiGet(resourceId, FULL_ACCESS_USER, HttpStatus.SC_OK, "sample"); + api.assertApiUpdate(resourceId, FULL_ACCESS_USER, "sampleUpdateAdmin", HttpStatus.SC_OK); + api.assertApiDelete(resourceId, FULL_ACCESS_USER, HttpStatus.SC_OK); + } + + @Test + public void limitedAccessUser_canCRUD() { + api.assertApiGet(resourceId, LIMITED_ACCESS_USER, HttpStatus.SC_OK, "sample"); + api.assertApiUpdate(resourceId, LIMITED_ACCESS_USER, "sampleUpdateAdmin", HttpStatus.SC_FORBIDDEN); + api.assertApiDelete(resourceId, LIMITED_ACCESS_USER, HttpStatus.SC_FORBIDDEN); + } + + @Test + public void noAccessUser_canCRUD() { + api.assertApiGet(resourceId, NO_ACCESS_USER, HttpStatus.SC_FORBIDDEN, ""); + api.assertApiUpdate(resourceId, NO_ACCESS_USER, "sampleUpdateAdmin", HttpStatus.SC_FORBIDDEN); + api.assertApiDelete(resourceId, NO_ACCESS_USER, HttpStatus.SC_FORBIDDEN); + } +} diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/securityapis/MigrateApiTests.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/securityapis/MigrateApiTests.java index 0395a85644..cd4e8e6427 100644 --- a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/securityapis/MigrateApiTests.java +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/securityapis/MigrateApiTests.java @@ -46,8 +46,10 @@ import static org.opensearch.sample.resource.TestUtils.migrationPayload_valid; import static org.opensearch.sample.resource.TestUtils.migrationPayload_valid_withSpecifiedAccessLevel; import static org.opensearch.sample.utils.Constants.RESOURCE_INDEX_NAME; +import static org.opensearch.sample.utils.Constants.RESOURCE_TYPE; import static org.opensearch.security.resources.ResourceSharingIndexHandler.getSharingIndex; import static org.opensearch.security.support.ConfigConstants.OPENSEARCH_RESOURCE_SHARING_ENABLED; +import static org.opensearch.security.support.ConfigConstants.OPENSEARCH_RESOURCE_SHARING_PROTECTED_TYPES; import static org.opensearch.security.support.ConfigConstants.SECURITY_SYSTEM_INDICES_ENABLED_KEY; import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL; @@ -79,7 +81,16 @@ public class MigrateApiTests { .anonymousAuth(false) .authc(AUTHC_HTTPBASIC_INTERNAL) .users(MIGRATION_USER) - .nodeSettings(Map.of(SECURITY_SYSTEM_INDICES_ENABLED_KEY, true, OPENSEARCH_RESOURCE_SHARING_ENABLED, true)) + .nodeSettings( + Map.of( + SECURITY_SYSTEM_INDICES_ENABLED_KEY, + true, + OPENSEARCH_RESOURCE_SHARING_ENABLED, + true, + OPENSEARCH_RESOURCE_SHARING_PROTECTED_TYPES, + List.of(RESOURCE_TYPE) + ) + ) .build(); @After diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/create/CreateResourceRequest.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/create/CreateResourceRequest.java index 3079f4c28f..5345667fb0 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/create/CreateResourceRequest.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/create/CreateResourceRequest.java @@ -18,6 +18,7 @@ import org.opensearch.sample.SampleResource; import static org.opensearch.sample.utils.Constants.RESOURCE_INDEX_NAME; +import static org.opensearch.sample.utils.Constants.RESOURCE_TYPE; /** * Request object for CreateSampleResource transport action @@ -59,6 +60,11 @@ public boolean shouldStoreUser() { return this.shouldStoreUser; } + @Override + public String type() { + return RESOURCE_TYPE; + } + @Override public String index() { return RESOURCE_INDEX_NAME; diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/create/UpdateResourceRequest.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/create/UpdateResourceRequest.java index 52f6b76d24..ef8e5cf077 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/create/UpdateResourceRequest.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/create/UpdateResourceRequest.java @@ -18,6 +18,7 @@ import org.opensearch.sample.SampleResource; import static org.opensearch.sample.utils.Constants.RESOURCE_INDEX_NAME; +import static org.opensearch.sample.utils.Constants.RESOURCE_TYPE; /** * Request object for UpdateResource transport action @@ -59,6 +60,11 @@ public String getResourceId() { return this.resourceId; } + @Override + public String type() { + return RESOURCE_TYPE; + } + @Override public String index() { return RESOURCE_INDEX_NAME; diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/delete/DeleteResourceRequest.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/delete/DeleteResourceRequest.java index e0009cface..7ab9606f74 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/delete/DeleteResourceRequest.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/delete/DeleteResourceRequest.java @@ -17,6 +17,7 @@ import org.opensearch.core.common.io.stream.StreamOutput; import static org.opensearch.sample.utils.Constants.RESOURCE_INDEX_NAME; +import static org.opensearch.sample.utils.Constants.RESOURCE_TYPE; /** * Request object for DeleteSampleResource transport action @@ -50,6 +51,11 @@ public String getResourceId() { return this.resourceId; } + @Override + public String type() { + return RESOURCE_TYPE; + } + @Override public String index() { return RESOURCE_INDEX_NAME; diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/get/GetResourceRequest.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/get/GetResourceRequest.java index ebf3122c52..132114d011 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/get/GetResourceRequest.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/get/GetResourceRequest.java @@ -17,6 +17,7 @@ import org.opensearch.core.common.io.stream.StreamOutput; import static org.opensearch.sample.utils.Constants.RESOURCE_INDEX_NAME; +import static org.opensearch.sample.utils.Constants.RESOURCE_TYPE; /** * Request object for GetSampleResource transport action @@ -50,6 +51,11 @@ public String getResourceId() { return this.resourceId; } + @Override + public String type() { + return RESOURCE_TYPE; + } + @Override public String index() { return RESOURCE_INDEX_NAME; 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 ee5e3bf597..f47f9b1c63 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 @@ -18,6 +18,7 @@ import org.opensearch.security.spi.resources.sharing.ShareWith; import static org.opensearch.sample.utils.Constants.RESOURCE_INDEX_NAME; +import static org.opensearch.sample.utils.Constants.RESOURCE_TYPE; /** * Request object for revoking access to a sample resource @@ -56,6 +57,11 @@ public ShareWith getEntitiesToRevoke() { return revokeAccess; } + @Override + public String type() { + return RESOURCE_TYPE; + } + @Override public String index() { return RESOURCE_INDEX_NAME; diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/share/ShareResourceRequest.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/share/ShareResourceRequest.java index 0688012ae7..9ad7efd6b8 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/share/ShareResourceRequest.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/share/ShareResourceRequest.java @@ -18,6 +18,7 @@ import org.opensearch.security.spi.resources.sharing.ShareWith; import static org.opensearch.sample.utils.Constants.RESOURCE_INDEX_NAME; +import static org.opensearch.sample.utils.Constants.RESOURCE_TYPE; /** * Request object for sharing sample resource transport action @@ -57,6 +58,11 @@ public ShareWith getShareWith() { return shareWithRecipients; } + @Override + public String type() { + return RESOURCE_TYPE; + } + @Override public String index() { return RESOURCE_INDEX_NAME; diff --git a/spi/README.md b/spi/README.md index 5fb6f15896..b428834e8b 100644 --- a/spi/README.md +++ b/spi/README.md @@ -175,6 +175,11 @@ public class ShareResourceRequest extends ActionRequest implements DocRequest { return shareWithRecipients; } + @Override + public String type() { + return RESOURCE_TYPE; + } + @Override public String index() { return RESOURCE_INDEX_NAME; diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index 2ca0e7d4fd..726ae43507 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -1225,7 +1225,10 @@ public Collection createComponents( // CS-SUPPRESS-SINGLE: RegexpSingleline get Resource Sharing Extensions // Assign resource sharing client to each extension // Using the non-gated client (i.e. no additional permissions required) - ResourceSharingClient resourceAccessControlClient = new ResourceAccessControlClient(resourceAccessHandler); + ResourceSharingClient resourceAccessControlClient = new ResourceAccessControlClient( + resourceAccessHandler, + resourcePluginInfo.getResourceIndices() + ); resourcePluginInfo.getResourceSharingExtensions().forEach(extension -> { extension.assignResourceSharingClient(resourceAccessControlClient); }); @@ -2236,6 +2239,18 @@ public List> getSettings() { ) ); + // resource marked here will be protected, other resources will not be protected with resource sharing model + // Defaults to no resources as protected + settings.add( + Setting.listSetting( + ConfigConstants.OPENSEARCH_RESOURCE_SHARING_PROTECTED_TYPES, + ConfigConstants.OPENSEARCH_RESOURCE_SHARING_PROTECTED_TYPES_DEFAULT, + Function.identity(), + Property.NodeScope, + Property.Filtered + ) + ); + settings.add(UserFactory.Caching.MAX_SIZE); settings.add(UserFactory.Caching.EXPIRE_AFTER_ACCESS); @@ -2460,7 +2475,10 @@ public void loadExtensions(ExtensionLoader loader) { // discover & register extensions and their types Set exts = new HashSet<>(loader.loadExtensions(ResourceSharingExtension.class)); - resourcePluginInfo.setResourceSharingExtensions(exts); + resourcePluginInfo.setResourceSharingExtensions( + exts, + settings.getAsList(ConfigConstants.OPENSEARCH_RESOURCE_SHARING_PROTECTED_TYPES) + ); // load action-groups in memory ResourceActionGroupsHelper.loadActionGroupsConfig(resourcePluginInfo); diff --git a/src/main/java/org/opensearch/security/privileges/ResourceAccessEvaluator.java b/src/main/java/org/opensearch/security/privileges/ResourceAccessEvaluator.java index 1984028b77..3758b3a5cf 100644 --- a/src/main/java/org/opensearch/security/privileges/ResourceAccessEvaluator.java +++ b/src/main/java/org/opensearch/security/privileges/ResourceAccessEvaluator.java @@ -10,6 +10,7 @@ package org.opensearch.security.privileges; +import java.util.List; import java.util.Set; import org.apache.logging.log4j.LogManager; @@ -98,6 +99,10 @@ public boolean shouldEvaluate(ActionRequest request) { ConfigConstants.OPENSEARCH_RESOURCE_SHARING_ENABLED, ConfigConstants.OPENSEARCH_RESOURCE_SHARING_ENABLED_DEFAULT ); + List protectedTypes = settings.getAsList( + ConfigConstants.OPENSEARCH_RESOURCE_SHARING_PROTECTED_TYPES, + ConfigConstants.OPENSEARCH_RESOURCE_SHARING_PROTECTED_TYPES_DEFAULT + ); if (!isResourceSharingFeatureEnabled) return false; if (!(request instanceof DocRequest docRequest)) return false; if (request instanceof GetRequest) return false; @@ -110,7 +115,9 @@ public boolean shouldEvaluate(ActionRequest request) { log.debug("Request index {} is not a protected resource index", docRequest.index()); return false; } - return true; + + // if a resource is not included in protected resource list, we do not perform resource-level authorization + return protectedTypes.contains(docRequest.type()); } } diff --git a/src/main/java/org/opensearch/security/resources/ResourceAccessControlClient.java b/src/main/java/org/opensearch/security/resources/ResourceAccessControlClient.java index c8d08e42fa..ad2be8da0d 100644 --- a/src/main/java/org/opensearch/security/resources/ResourceAccessControlClient.java +++ b/src/main/java/org/opensearch/security/resources/ResourceAccessControlClient.java @@ -10,6 +10,9 @@ import java.util.Set; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + import org.opensearch.core.action.ActionListener; import org.opensearch.security.spi.resources.client.ResourceSharingClient; import org.opensearch.security.spi.resources.sharing.ResourceSharing; @@ -22,15 +25,18 @@ * @opensearch.experimental */ public final class ResourceAccessControlClient implements ResourceSharingClient { + private static final Logger LOGGER = LogManager.getLogger(ResourceAccessControlClient.class); private final ResourceAccessHandler resourceAccessHandler; + private final Set resourceIndices; /** * Constructs a new ResourceAccessControlClient. * */ - public ResourceAccessControlClient(ResourceAccessHandler resourceAccessHandler) { + public ResourceAccessControlClient(ResourceAccessHandler resourceAccessHandler, Set resourceIndices) { this.resourceAccessHandler = resourceAccessHandler; + this.resourceIndices = resourceIndices; } /** @@ -43,6 +49,16 @@ public ResourceAccessControlClient(ResourceAccessHandler resourceAccessHandler) */ @Override public void verifyAccess(String resourceId, String resourceIndex, String action, ActionListener listener) { + // following situation will arise when resource is onboarded to framework but not marked as protected + if (!resourceIndices.contains(resourceIndex)) { + LOGGER.warn( + "Resource '{}' is onboarded to sharing framework but is not marked as protected. Action {} is allowed.", + resourceId, + action + ); + listener.onResponse(true); + return; + } resourceAccessHandler.hasPermission(resourceId, resourceIndex, action, null, listener); } diff --git a/src/main/java/org/opensearch/security/resources/ResourcePluginInfo.java b/src/main/java/org/opensearch/security/resources/ResourcePluginInfo.java index 8080f9e561..23c22f0032 100644 --- a/src/main/java/org/opensearch/security/resources/ResourcePluginInfo.java +++ b/src/main/java/org/opensearch/security/resources/ResourcePluginInfo.java @@ -15,6 +15,7 @@ import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashSet; +import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; @@ -48,28 +49,35 @@ public class ResourcePluginInfo { // AuthZ: resolved (flattened) groups per type private final Map typeToFlattened = new HashMap<>(); - public void setResourceSharingExtensions(Set extensions) { + public void setResourceSharingExtensions(Set extensions, List protectedTypes) { resourceSharingExtensions.clear(); typeToIndex.clear(); indexToType.clear(); - // Enforce resource-type unique-ness - Set resourceTypes = new HashSet<>(); - for (ResourceSharingExtension extension : extensions) { - for (var rp : extension.getResourceProviders()) { - if (!resourceTypes.contains(rp.resourceType())) { - // add name seen so far to the resource-types set - resourceTypes.add(rp.resourceType()); - // also cache type->index and index->type mapping - typeToIndex.put(rp.resourceType(), rp.resourceIndexName()); - indexToType.put(rp.resourceIndexName(), rp.resourceType()); - } else { - throw new OpenSearchSecurityException( - String.format( - "Resource type [%s] is already registered. Please provide a different unique-name for the resource declared by %s.", - rp.resourceType(), - extension.getClass().getName() - ) - ); + // only assign types if the list setting is non-empty + if (!protectedTypes.isEmpty()) { + // Enforce resource-type unique-ness + Set resourceTypes = new HashSet<>(); + for (ResourceSharingExtension extension : extensions) { + for (var rp : extension.getResourceProviders()) { + // exclude resource types not mentioned in the explicit list. defaults to no resource marked as protected resources + if (!protectedTypes.contains(rp.resourceType())) { + continue; + } + if (!resourceTypes.contains(rp.resourceType())) { + // add name seen so far to the resource-types set + resourceTypes.add(rp.resourceType()); + // also cache type->index and index->type mapping + typeToIndex.put(rp.resourceType(), rp.resourceIndexName()); + indexToType.put(rp.resourceIndexName(), rp.resourceType()); + } else { + throw new OpenSearchSecurityException( + String.format( + "Resource type [%s] is already registered. Please provide a different unique-name for the resource declared by %s.", + rp.resourceType(), + extension.getClass().getName() + ) + ); + } } } } @@ -117,14 +125,9 @@ public String indexByType(String type) { } public Set getResourceTypes() { - return typeToIndex.entrySet() + return typeToIndex.keySet() .stream() - .map( - e -> new ResourceDashboardInfo( - e.getKey(), - Collections.unmodifiableSet(typeToGroupNames.getOrDefault(e.getKey(), new LinkedHashSet<>())) - ) - ) + .map(s -> new ResourceDashboardInfo(s, Collections.unmodifiableSet(typeToGroupNames.getOrDefault(s, new LinkedHashSet<>())))) .collect(Collectors.toCollection(LinkedHashSet::new)); } diff --git a/src/main/java/org/opensearch/security/resources/api/share/ShareRequest.java b/src/main/java/org/opensearch/security/resources/api/share/ShareRequest.java index fd9aa8a608..5ab658595b 100644 --- a/src/main/java/org/opensearch/security/resources/api/share/ShareRequest.java +++ b/src/main/java/org/opensearch/security/resources/api/share/ShareRequest.java @@ -41,6 +41,8 @@ public class ShareRequest extends ActionRequest implements DocRequest { @JsonProperty("revoke") private final ShareWith revoke; + private final String resourceType; + private final RestRequest.Method method; /** @@ -53,6 +55,7 @@ private ShareRequest(Builder builder) { this.add = builder.add; this.revoke = builder.revoke; this.method = builder.method; + this.resourceType = builder.resourceType; } public ShareRequest(StreamInput in) throws IOException { @@ -60,6 +63,7 @@ public ShareRequest(StreamInput in) throws IOException { this.method = in.readEnum(RestRequest.Method.class); this.resourceId = in.readString(); this.resourceIndex = in.readString(); + this.resourceType = in.readString(); this.shareWith = in.readOptionalWriteable(ShareWith::new); this.add = in.readOptionalWriteable(ShareWith::new); this.revoke = in.readOptionalWriteable(ShareWith::new); @@ -70,6 +74,7 @@ public void writeTo(StreamOutput out) throws IOException { out.writeEnum(method); out.writeString(resourceId); out.writeString(resourceIndex); + out.writeString(resourceType); out.writeOptionalWriteable(shareWith); out.writeOptionalWriteable(add); out.writeOptionalWriteable(revoke); @@ -115,6 +120,11 @@ public RestRequest.Method getMethod() { return method; } + @Override + public String type() { + return resourceType; + } + /** * Get the index that this request operates on * @@ -141,6 +151,7 @@ public String id() { public static class Builder { private String resourceId; private String resourceIndex; + private String resourceType; private ShareWith shareWith; private ShareWith add; private ShareWith revoke; @@ -154,6 +165,10 @@ public void resourceIndex(String resourceIndex) { this.resourceIndex = resourceIndex; } + public void resourceType(String resourceType) { + this.resourceType = resourceType; + } + public void shareWith(ShareWith shareWith) { this.shareWith = shareWith; } @@ -186,6 +201,7 @@ public void parseContent(XContentParser xContentParser, ResourcePluginInfo resou this.resourceId(parser.text()); break; case "resource_type": + this.resourceType(parser.text()); this.resourceIndex(resourcePluginInfo.indexByType(parser.text())); break; case "share_with": diff --git a/src/main/java/org/opensearch/security/resources/api/share/ShareRestAction.java b/src/main/java/org/opensearch/security/resources/api/share/ShareRestAction.java index cce1a0aae4..8cc9d87032 100644 --- a/src/main/java/org/opensearch/security/resources/api/share/ShareRestAction.java +++ b/src/main/java/org/opensearch/security/resources/api/share/ShareRestAction.java @@ -73,6 +73,7 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli if (resourceIndex != null) { builder.resourceIndex(resourceIndex); + builder.resourceType(resourceType); } if (resourceId != null) { builder.resourceId(resourceId); diff --git a/src/main/java/org/opensearch/security/support/ConfigConstants.java b/src/main/java/org/opensearch/security/support/ConfigConstants.java index 81202e47fe..8549b0a004 100644 --- a/src/main/java/org/opensearch/security/support/ConfigConstants.java +++ b/src/main/java/org/opensearch/security/support/ConfigConstants.java @@ -426,6 +426,13 @@ public enum RolesMappingResolution { public static final String OPENSEARCH_RESOURCE_SHARING_ENABLED = "plugins.security.experimental.resource_sharing.enabled"; public static final boolean OPENSEARCH_RESOURCE_SHARING_ENABLED_DEFAULT = false; + // Protected resource types + // Resource sharing will only apply to these types + public static final String OPENSEARCH_RESOURCE_SHARING_PROTECTED_TYPES = + "plugins.security.experimental.resource_sharing.protected_types"; + public static final List OPENSEARCH_RESOURCE_SHARING_PROTECTED_TYPES_DEFAULT = List.of(); // defaults to no registered types as + // protected + public static Set getSettingAsSet( final Settings settings, final String key,