Merge branch 'main' into multiple-resource-tests

Author: cwperks

CHANGELOG.md

@@ -27,6 +27,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Fix deprecated SSL transport settings in demo certificates ([#5723](https://github.com/opensearch-project/security/pull/5723))
 - Updates DlsFlsValveImpl condition to return true if request is internal and not a protected resource request ([#5721](https://github.com/opensearch-project/security/pull/5721))
 - [Performance] Call AdminDns.isAdmin once per request ([#5752](https://github.com/opensearch-project/security/pull/5752))
+- Update operations on `.kibana` system index now work correctly with Dashboards multi tenancy enabled. ([#5778](https://github.com/opensearch-project/security/pull/5778))
 
 ### Refactoring
 - [Resource Sharing] Make migrate api require default access level to be supplied and updates documentations + tests ([#5717](https://github.com/opensearch-project/security/pull/5717))

sample-resource-plugin/src/main/java/org/opensearch/sample/SampleResourceGroup.java

@@ -25,7 +25,6 @@
 import static org.opensearch.core.xcontent.ConstructingObjectParser.constructorArg;
 import static org.opensearch.core.xcontent.ConstructingObjectParser.optionalConstructorArg;
 import static org.opensearch.sample.utils.Constants.RESOURCE_GROUP_TYPE;
-import static org.opensearch.sample.utils.Constants.RESOURCE_TYPE;
 
 /**
  * Sample resource group declared by this plugin.
@@ -45,7 +44,7 @@ public SampleResourceGroup(StreamInput in) throws IOException {
     }
 
     private static final ConstructingObjectParser<SampleResourceGroup, Void> PARSER = new ConstructingObjectParser<>(
-        RESOURCE_TYPE,
+        RESOURCE_GROUP_TYPE,
         true,
         a -> {
             SampleResourceGroup s;
@@ -56,13 +55,15 @@ public SampleResourceGroup(StreamInput in) throws IOException {
             }
             s.setName((String) a[0]);
             s.setDescription((String) a[1]);
+            // ignore a[2] as we know the type
             return s;
         }
     );
 
     static {
         PARSER.declareString(constructorArg(), new ParseField("name"));
         PARSER.declareStringOrNull(optionalConstructorArg(), new ParseField("description"));
+        PARSER.declareStringOrNull(optionalConstructorArg(), new ParseField("resource_type"));
     }
 
     public static SampleResourceGroup fromXContent(XContentParser parser) throws IOException {

src/integrationTest/java/org/opensearch/security/privileges/int_tests/DashboardMultiTenancyIntTests.java

@@ -41,7 +41,6 @@
 import static org.opensearch.test.framework.matcher.RestMatchers.isForbidden;
 import static org.opensearch.test.framework.matcher.RestMatchers.isOk;
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
 
 /**
  * An integration test matrix for Dashboards multi-tenancy. Verifies both read and write operations
@@ -660,25 +659,13 @@ public void bulkWithUpdate_withTenantHeader_humanResources() {
             );
 
             if (user.reference(WRITE).covers(dashboards_index_human_resources)) {
-                if (user == GLOBAL_TENANT_READ_WRITE_USER || user == WILDCARD_TENANT_USER) {
-                    assertThat(response, isOk());
-                    assertThat(
-                        response,
-                        containsExactly(dashboards_index_human_resources).at("items[*].update[?(@.result == 'updated')]._index")
-                            .reducedBy(user.reference(WRITE))
-                            .whenEmpty(isOk())
-                    );
-                } else {
-                    // At the moment, we get here a nested error:
-                    // Update is not supported when FLS or DLS or Fieldmasking is activated
-                    // This is a bug; even though it does not seem to have an impact on Dashboards functionality
-                    assertThat(response, isOk());
-                    assertTrue(
-                        response.getBody(),
-                        response.getBody().contains("Update is not supported when FLS or DLS or Fieldmasking is activated")
-                    );
-                }
-
+                assertThat(response, isOk());
+                assertThat(
+                    response,
+                    containsExactly(dashboards_index_human_resources).at("items[*].update[?(@.result == 'updated')]._index")
+                        .reducedBy(user.reference(WRITE))
+                        .whenEmpty(isOk())
+                );
             } else {
                 assertThat(
                     response,

src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java

@@ -189,6 +189,18 @@ public boolean invoke(PrivilegesEvaluationContext context, final ActionListener<
         DlsFlsProcessedConfig config = this.dlsFlsProcessedConfig.get();
         IndexResolverReplacer.Resolved resolved = context.getResolvedRequest();
 
+        DocumentAllowList documentAllowList = DocumentAllowList.get(threadContext);
+
+        if (!resolved.isLocalAll() && resolved.getAllIndices().stream().anyMatch(index -> documentAllowList.isAllowed(index, "*"))) {
+            // The documentAllowList is needed here for Dashboards multi tenancy which can redirect index accesses to indices for which no
+            // normal index privileges are present
+            // If we would not use the documentAllowList here, the index would appear to be protected
+
+            if (resolved.getAllIndices().size() == 1) {
+                return true;
+            }
+        }
+
         try {
             boolean hasDlsRestrictions = !config.getDocumentPrivileges().isUnrestricted(context, resolved);
             boolean hasFlsRestrictions = !config.getFieldPrivileges().isUnrestricted(context, resolved);

src/main/java/org/opensearch/security/resources/ResourceIndexListener.java

@@ -78,6 +78,15 @@ public void postIndex(ShardId shardId, Engine.Index index, Engine.IndexResult re
         log.debug("postIndex called on {}", resourceIndex);
 
         String resourceId = index.id();
+        ResourceProvider provider = resourcePluginInfo.getResourceProvider(resourceType);
+        if (provider == null) {
+            log.warn(
+                "Failed to create a resource sharing entry for resource: {} with type: {}. The type is not declared as a protected type in plugins.security.experimental.resource_sharing.protected_types.",
+                resourceId,
+                resourceType
+            );
+            return;
+        }
 
         // Only proceed if this was a create operation and for primary shard
         if (!index.origin().equals(Engine.Operation.Origin.PRIMARY)) {