[Backport 2.4] Fix issues with datastream backing indexes (#2242)

* Resolving backing indices of data streams when resolving for aliases * Fixing resolution of indices for non-wild card scenarios / exact matches * Adding tests for DLS/FLS/Field-Masking on Data Streams * Datastream test fixes * Add pit-3 mapping Co-authored-by: Sandesh Kumar <kusandes@amazon.com> Signed-off-by: Craig Perkins <cwperx@amazon.com> Co-authored-by: Dave Lago <davelago@amazon.com> Co-authored-by: Sandesh Kumar <kusandes@amazon.com>
opensearch-project · Nov 8, 2022 · bca4612 · bca4612
1 parent 982f281
commit bca4612
Show file tree

Hide file tree

Showing 8 changed files with 530 additions and 29 deletions.
diff --git a/src/main/java/org/opensearch/security/securityconf/ConfigModelV7.java b/src/main/java/org/opensearch/security/securityconf/ConfigModelV7.java
@@ -68,6 +68,7 @@
 import org.opensearch.security.user.User;
 
 import static org.opensearch.cluster.metadata.IndexAbstraction.Type.ALIAS;
+import static org.opensearch.cluster.metadata.IndexAbstraction.Type.DATA_STREAM;
 
 public class ConfigModelV7 extends ConfigModel {
 
@@ -768,20 +769,22 @@ public Set<String> getResolvedIndexPattern(final User user, final IndexNameExpre
             final ImmutableSet.Builder<String> resolvedIndices = new ImmutableSet.Builder<>();
 
             final WildcardMatcher matcher = WildcardMatcher.from(unresolved);
+            boolean includeDataStreams = true;
             if (!(matcher instanceof WildcardMatcher.Exact)) {
-                final String[] aliasesForPermittedPattern = cs.state().getMetadata().getIndicesLookup().entrySet().stream()
-                        .filter(e -> e.getValue().getType() == ALIAS)
+                final String[] aliasesAndDataStreamsForPermittedPattern = cs.state().getMetadata().getIndicesLookup().entrySet().stream()
+                        .filter(e -> (e.getValue().getType() == ALIAS) || (e.getValue().getType() == DATA_STREAM))
                         .filter(e -> matcher.test(e.getKey()))
                         .map(e -> e.getKey())
                         .toArray(String[]::new);
-                if (aliasesForPermittedPattern.length > 0) {
-                    final String[] resolvedAliases = resolver.concreteIndexNames(cs.state(), IndicesOptions.lenientExpandOpen(), aliasesForPermittedPattern);
-                    resolvedIndices.addAll(Arrays.asList(resolvedAliases));
+                if (aliasesAndDataStreamsForPermittedPattern.length > 0) {
+                    final String[] resolvedAliasesAndDataStreamIndices = resolver.concreteIndexNames(cs.state(),
+                            IndicesOptions.lenientExpandOpen(), includeDataStreams, aliasesAndDataStreamsForPermittedPattern);
+                    resolvedIndices.addAll(Arrays.asList(resolvedAliasesAndDataStreamIndices));
                 }
             }
 
             if (Strings.isNotBlank(unresolved)) {
-                final String[] resolvedIndicesFromPattern = resolver.concreteIndexNames(cs.state(), IndicesOptions.lenientExpandOpen(), unresolved);
+                final String[] resolvedIndicesFromPattern = resolver.concreteIndexNames(cs.state(), IndicesOptions.lenientExpandOpen(), includeDataStreams, unresolved);
                 resolvedIndices.addAll(Arrays.asList(resolvedIndicesFromPattern));
             }
 

diff --git a/src/test/java/org/opensearch/security/DataStreamIntegrationTests.java b/src/test/java/org/opensearch/security/DataStreamIntegrationTests.java
diff --git a/src/test/java/org/opensearch/security/IndexTemplateClusterPermissionsCheckTest.java b/src/test/java/org/opensearch/security/IndexTemplateClusterPermissionsCheckTest.java
@@ -36,10 +36,10 @@ public void setupRestHelper() throws Exception{
         }
         @Test
         public void testPutIndexTemplateByNonPrivilegedUser() throws Exception {
-            String expectedFailureResponse = getFailureResponseReason("ds3");
+            String expectedFailureResponse = getFailureResponseReason("ds4");
 
             // should fail, as user `ds3` doesn't have correct permissions
-            HttpResponse response = rh.executePutRequest("/_index_template/sem1234", indexTemplateBody, encodeBasicHeader("ds3", "nagilum"));
+            HttpResponse response = rh.executePutRequest("/_index_template/sem1234", indexTemplateBody, encodeBasicHeader("ds4", "nagilum"));
             Assert.assertEquals(HttpStatus.SC_FORBIDDEN, response.getStatusCode());
             Assert.assertEquals(expectedFailureResponse, response.findValueInJson("error.root_cause[0].reason"));
         }

diff --git a/src/test/java/org/opensearch/security/PitIntegrationTests.java b/src/test/java/org/opensearch/security/PitIntegrationTests.java
@@ -229,10 +229,10 @@ public void testDataStreamWithPits() throws Exception {
         Assert.assertEquals(HttpStatus.SC_OK, resc.getStatusCode());
         String pitId2 =  resc.findValueInJson("pit_id");
 
-        // since pit-2 doesn't have permission to backing data stream indices, throw security error
+        // since pit-3 doesn't have permission to backing data stream indices, throw security error
         resc = rh.executeGetRequest("/_cat/pit_segments",
                 "{\"pit_id\":\"" + pitId2 +"\"}",
-                encodeBasicHeader("pit-2", "nagilum"));
+                encodeBasicHeader("pit-3", "nagilum"));
         Assert.assertEquals(HttpStatus.SC_FORBIDDEN, resc.getStatusCode());
 
         // Delete all PITs should work for user with all index access

diff --git a/src/test/java/org/opensearch/security/securityconf/impl/v7/IndexPatternTests.java b/src/test/java/org/opensearch/security/securityconf/impl/v7/IndexPatternTests.java
@@ -108,47 +108,47 @@ public void testAttemptResolveIndexNamesOverload() {
     public void testExactNameWithNoMatches() {
         doReturn("index-17").when(ip).getUnresolvedIndexPattern(user);
         when(clusterService.state()).thenReturn(mock(ClusterState.class));
-        when(resolver.concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq("index-17"))).thenReturn(new String[]{});
+        when(resolver.concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq(true), eq("index-17"))).thenReturn(new String[]{});
 
         final Set<String> results = ip.concreteIndexNames(user, resolver, clusterService);
 
         assertThat(results, contains("index-17"));
 
         verify(clusterService).state();
         verify(ip).getUnresolvedIndexPattern(user);
-        verify(resolver).concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq("index-17"));
+        verify(resolver).concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq(true), eq("index-17"));
     }
 
     /** Verify concreteIndexNames on exact name matches */
     @Test
     public void testExactName() {
         doReturn("index-17").when(ip).getUnresolvedIndexPattern(user);
         when(clusterService.state()).thenReturn(mock(ClusterState.class));
-        when(resolver.concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq("index-17"))).thenReturn(new String[]{"resolved-index-17"});
+        when(resolver.concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq(true), eq("index-17"))).thenReturn(new String[]{"resolved-index-17"});
 
         final Set<String> results = ip.concreteIndexNames(user, resolver, clusterService);
 
         assertThat(results, contains("resolved-index-17"));
 
         verify(clusterService).state();
         verify(ip).getUnresolvedIndexPattern(user);
-        verify(resolver).concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq("index-17"));
+        verify(resolver).concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq(true), eq("index-17"));
     }
 
     /** Verify concreteIndexNames on multiple matches */
     @Test
     public void testMultipleConcreteIndices() {
         doReturn("index-1*").when(ip).getUnresolvedIndexPattern(user);
         doReturn(createClusterState()).when(clusterService).state();
-        when(resolver.concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq("index-1*"))).thenReturn(new String[]{"resolved-index-17", "resolved-index-18"});
+        when(resolver.concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq(true), eq("index-1*"))).thenReturn(new String[]{"resolved-index-17", "resolved-index-18"});
 
         final Set<String> results = ip.concreteIndexNames(user, resolver, clusterService);
 
         assertThat(results, contains("resolved-index-17", "resolved-index-18"));
 
         verify(clusterService, times(2)).state();
         verify(ip).getUnresolvedIndexPattern(user);
-        verify(resolver).concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq("index-1*"));
+        verify(resolver).concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq(true), eq("index-1*"));
     }
 
     /** Verify concreteIndexNames when there is an alias */
@@ -157,44 +157,42 @@ public void testMultipleConcreteIndicesWithOneAlias() {
         doReturn("index-1*").when(ip).getUnresolvedIndexPattern(user);
 
         doReturn(createClusterState(
-            new IndexShorthand("index-111", Type.DATA_STREAM), // Name matches/wrong type
             new IndexShorthand("index-100", Type.ALIAS), // Name and type match
             new IndexShorthand("19", Type.ALIAS) // Type matches/wrong name
             )).when(clusterService).state();
-        when(resolver.concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq("index-100"))).thenReturn(new String[]{"resolved-index-100"});
-        when(resolver.concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq("index-1*"))).thenReturn(new String[]{"resolved-index-17", "resolved-index-18"});
+        when(resolver.concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq(true), eq("index-100"))).thenReturn(new String[]{"resolved-index-100"});
+        when(resolver.concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq(true), eq("index-1*"))).thenReturn(new String[]{"resolved-index-17", "resolved-index-18"});
 
         final Set<String> results = ip.concreteIndexNames(user, resolver, clusterService);
 
         assertThat(results, contains("resolved-index-100", "resolved-index-17", "resolved-index-18"));
 
         verify(clusterService, times(3)).state();
         verify(ip).getUnresolvedIndexPattern(user);
-        verify(resolver).concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq("index-100"));
-        verify(resolver).concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq("index-1*"));
+        verify(resolver).concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq(true), eq("index-100"));
+        verify(resolver).concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq(true), eq("index-1*"));
     }
 
     /** Verify attemptResolveIndexNames with multiple aliases */
     @Test
     public void testMultipleConcreteAliasedAndUnresolved() {
         doReturn("index-1*").when(ip).getUnresolvedIndexPattern(user);
         doReturn(createClusterState(
-            new IndexShorthand("index-111", Type.DATA_STREAM), // Name matches/wrong type
             new IndexShorthand("index-100", Type.ALIAS), // Name and type match
             new IndexShorthand("index-101", Type.ALIAS), // Name and type match
             new IndexShorthand("19", Type.ALIAS) // Type matches/wrong name
             )).when(clusterService).state();
-        when(resolver.concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq("index-100"), eq("index-101"))).thenReturn(new String[]{"resolved-index-100", "resolved-index-101"});
-        when(resolver.concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq("index-1*"))).thenReturn(new String[]{"resolved-index-17", "resolved-index-18"});
+        when(resolver.concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq(true), eq("index-100"), eq("index-101"))).thenReturn(new String[]{"resolved-index-100", "resolved-index-101"});
+        when(resolver.concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq(true), eq("index-1*"))).thenReturn(new String[]{"resolved-index-17", "resolved-index-18"});
 
         final Set<String> results = ip.attemptResolveIndexNames(user, resolver, clusterService);
 
         assertThat(results, contains("resolved-index-100", "resolved-index-101", "resolved-index-17", "resolved-index-18", "index-1*"));
 
         verify(clusterService, times(3)).state();
         verify(ip).getUnresolvedIndexPattern(user);
-        verify(resolver).concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq("index-100"), eq("index-101"));
-        verify(resolver).concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq("index-1*"));
+        verify(resolver).concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq(true), eq("index-100"), eq("index-101"));
+        verify(resolver).concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq(true), eq("index-1*"));
     }
 
     private ClusterState createClusterState(final IndexShorthand... indices) {

diff --git a/src/test/resources/internal_users.yml b/src/test/resources/internal_users.yml
@@ -346,15 +346,51 @@ ds2:
 ds3:
   hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
   #password is: nagilum
+ds4:
+  hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
+  #password is: nagilum
 pit-1:
   hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
   #password is: nagilum
 pit-2:
   hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
   #password is: nagilum
+pit-3:
+  hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
+  #password is: nagilum
 all-pit:
   hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
   #password is: nagilum
+ds_admin:
+  hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
+  #password is: nagilum
+ds_dls1:
+  hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
+  #password is: nagilum
+ds_dls2:
+  hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
+  #password is: nagilum
+ds_dls3:
+  hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
+  #password is: nagilum
+ds_fls1:
+  hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
+  #password is: nagilum
+ds_fls2:
+  hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
+  #password is: nagilum
+ds_fls3:
+  hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
+  #password is: nagilum
+ds_fm1:
+  hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
+  #password is: nagilum
+ds_fm2:
+  hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
+  #password is: nagilum
+ds_fm3:
+  hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
+  #password is: nagilum
 hidden_test:
   hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
   opendistro_security_roles:

diff --git a/src/test/resources/roles.yml b/src/test/resources/roles.yml
@@ -1119,6 +1119,20 @@ data_stream_2:
         - "indices:admin/get"
 
 data_stream_3:
+  reserved: true
+  hidden: false
+  description: "Migrated from v6 (all types mapped)"
+  cluster_permissions:
+    - "*"
+  index_permissions:
+    - index_patterns:
+        - "*"
+      allowed_actions:
+        - "DATASTREAM_ALL"
+        - "indices:data/write/index"
+        - "indices:data/write/bulk*"
+
+data_stream_4:
   reserved: true
   hidden: false
   description: "Migrated from v6 (all types mapped)"
@@ -1129,6 +1143,135 @@ data_stream_3:
       allowed_actions:
         - "DATASTREAM_ALL"
 
+data_stream_admin:
+  reserved: true
+  hidden: false
+  description: "Migrated from v6 (all types mapped)"
+  cluster_permissions:
+    - "*"
+  index_permissions:
+    - index_patterns:
+        - "my-data-stream*"
+      allowed_actions:
+        - "*"
+
+data_stream_dls_1:
+  reserved: true
+  hidden: false
+  description: "Migrated from v6 (all types mapped)"
+  cluster_permissions: []
+  index_permissions:
+    - index_patterns:
+        - "my-data-stream11"
+      dls: "{\n  \"bool\": {\n \"must\": {\n \"match\": {\n \"user.id\": \"8a4f500d\"\n }\n }\n }\n}"
+      allowed_actions:
+        - "read"
+
+data_stream_dls_2:
+  reserved: true
+  hidden: false
+  description: "Migrated from v6 (all types mapped)"
+  cluster_permissions: []
+  index_permissions:
+    - index_patterns:
+        - "my-data-stream2*"
+      dls: "{\n  \"bool\": {\n \"must\": {\n \"match\": {\n \"user.id\": \"8a4f500d\"\n }\n }\n }\n}"
+      allowed_actions:
+        - "read"
+
+data_stream_dls_3:
+  reserved: true
+  hidden: false
+  description: "Migrated from v6 (all types mapped)"
+  cluster_permissions: []
+  index_permissions:
+    - index_patterns:
+        - "my-data-stream*"
+      dls: "{\n  \"bool\": {\n \"must\": {\n \"match\": {\n \"user.id\": \"8a4f500d\"\n }\n }\n }\n}"
+      allowed_actions:
+        - "read"
+
+data_stream_fls_1:
+  reserved: true
+  hidden: false
+  description: "Migrated from v6 (all types mapped)"
+  cluster_permissions: []
+  index_permissions:
+    - index_patterns:
+        - "my-data-stream11"
+      fls:
+        - "user.id"
+        - "message"
+      allowed_actions:
+        - "read"
+
+data_stream_fls_2:
+  reserved: true
+  hidden: false
+  description: "Migrated from v6 (all types mapped)"
+  cluster_permissions: []
+  index_permissions:
+    - index_patterns:
+        - "my-data-stream2*"
+      fls:
+        - "user.id"
+        - "user.name"
+      allowed_actions:
+        - "read"
+
+data_stream_fls_3:
+  reserved: true
+  hidden: false
+  description: "Migrated from v6 (all types mapped)"
+  cluster_permissions: []
+  index_permissions:
+    - index_patterns:
+        - "my-data-stream*"
+      fls:
+        - "~message"
+      allowed_actions:
+        - "read"
+
+data_stream_fm_1:
+  reserved: true
+  hidden: false
+  description: "Migrated from v6 (all types mapped)"
+  cluster_permissions: []
+  index_permissions:
+    - index_patterns:
+        - "my-data-stream11"
+      masked_fields:
+        - "message"
+      allowed_actions:
+        - "read"
+
+data_stream_fm_2:
+  reserved: true
+  hidden: false
+  description: "Migrated from v6 (all types mapped)"
+  cluster_permissions: []
+  index_permissions:
+    - index_patterns:
+        - "my-data-stream2*"
+      masked_fields:
+        - "message"
+      allowed_actions:
+        - "read"
+
+data_stream_fm_3:
+  reserved: true
+  hidden: false
+  description: "Migrated from v6 (all types mapped)"
+  cluster_permissions: []
+  index_permissions:
+    - index_patterns:
+        - "my-data-stream*"
+      masked_fields:
+        - "user.name"
+        - "message"
+      allowed_actions:
+        - "read"
+
 point_in_time_1:
   reserved: true
   hidden: false
@@ -1157,6 +1300,20 @@ point_in_time_2:
       allowed_actions:
         - "manage_point_in_time"
 
+point_in_time_3:
+  reserved: true
+  hidden: false
+  description: "Migrated from v6 (all types mapped)"
+  index_permissions:
+    - index_patterns:
+        - "my-data-stream31"
+        - "pit_3"
+      dls: null
+      fls: null
+      masked_fields: null
+      allowed_actions:
+        - "manage_point_in_time"
+
 point_in_time_all:
   reserved: true
   hidden: false