Fix issue with Get mappings on a Closed index (#4685)

Signed-off-by: Craig Perkins <cwperx@amazon.com>
(cherry picked from commit 99457b1)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

src/integrationTest/java/org/opensearch/security/FlsAndFieldMaskingTests.java

@@ -62,6 +62,7 @@
 import org.opensearch.test.framework.cluster.TestRestClient;
 import org.opensearch.test.framework.log.LogsRule;
 
+import static org.apache.http.HttpStatus.SC_OK;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.allOf;
 import static org.hamcrest.Matchers.containsString;
@@ -237,6 +238,12 @@ public class FlsAndFieldMaskingTests {
         "cluster_composite_ops_ro"
     ).indexPermissions("read").fls(String.format("~%s", FIELD_TITLE)).on(FIRST_INDEX_NAME);
 
+    static final TestSecurityConfig.Role ROLE_NO_FIELD_TITLE_WILDCARD_INDEX_FLS = new TestSecurityConfig.Role("example_exclusive_fls")
+        .clusterPermissions("cluster_composite_ops_ro")
+        .indexPermissions("read", "indices:admin/mappings/get")
+        .fls(String.format("~%s", FIELD_TITLE))
+        .on("*");
+
     static final TestSecurityConfig.Role ROLE_ONLY_FIELD_TITLE_MASKED = new TestSecurityConfig.Role("example_mask").clusterPermissions(
         "cluster_composite_ops_ro"
     ).indexPermissions("read").maskedFields(FIELD_TITLE.concat("::/(?<=.{1})./::").concat(MASK_VALUE)).on(FIRST_INDEX_NAME);
@@ -255,6 +262,12 @@ public class FlsAndFieldMaskingTests {
         ROLE_NO_FIELD_TITLE_FLS
     );
 
+    /**
+     * Example user with fls filter in which the user can see every field but the {@link Song#FIELD_TITLE} field.
+     */
+    static final TestSecurityConfig.User USER_NO_FIELD_TITLE_WILDCARD_INDEX_FLS = new TestSecurityConfig.User("exclusive_wildcard_fls_user")
+        .roles(ROLE_NO_FIELD_TITLE_WILDCARD_INDEX_FLS);
+
     /**
      * Example user in which {@link Song#FIELD_TITLE} field is masked.
      */
@@ -306,6 +319,7 @@ public class FlsAndFieldMaskingTests {
             TWINS_FIRST_ARTIST_READER,
             TWINS_FIRST_ARTIST_READER,
             USER_ONLY_FIELD_TITLE_FLS,
+            USER_NO_FIELD_TITLE_WILDCARD_INDEX_FLS,
             USER_NO_FIELD_TITLE_FLS,
             USER_ONLY_FIELD_TITLE_MASKED,
             USER_BOTH_ONLY_AND_NO_FIELD_TITLE_FLS,
@@ -1778,4 +1792,26 @@ public void testFlsOnAClosedAndReopenedIndex() throws IOException {
         }
     }
 
+    @SuppressWarnings("unchecked")
+    @Test
+    public void testGetMappingsOnAClosedIndexWithFlsRestrictions() throws IOException {
+        String indexName = "fls_excludes_mappings";
+        List<String> docIds = createIndexWithDocs(indexName, SONGS[0], SONGS[1]);
+
+        try (TestRestClient client = cluster.getRestClient(ADMIN_USER)) {
+            TestRestClient.HttpResponse mappingsResponse = client.get(indexName + "/_mapping");
+            mappingsResponse.assertStatusCode(SC_OK);
+            assertThat(mappingsResponse.getBody(), containsString("title"));
+
+            TestRestClient.HttpResponse closeResponse = client.post(indexName + "/_close");
+            closeResponse.assertStatusCode(SC_OK);
+        }
+
+        try (TestRestClient client = cluster.getRestClient(USER_NO_FIELD_TITLE_WILDCARD_INDEX_FLS)) {
+            TestRestClient.HttpResponse mappingsResponse = client.get(indexName + "/_mapping");
+            mappingsResponse.assertStatusCode(SC_OK);
+            assertThat(mappingsResponse.getBody(), not(containsString("title")));
+        }
+    }
+
 }

src/main/java/org/opensearch/security/securityconf/ConfigModelV7.java

@@ -303,7 +303,7 @@ public EvaluatedDlsFlsConfig getDlsFls(
 
             for (SecurityRole role : roles) {
                 for (IndexPattern ip : role.getIpatterns()) {
-                    final Set<String> concreteIndices = ip.concreteIndexNames(user, resolver, cs);
+                    final Set<String> concreteIndices = ip.concreteIndexNames(user, resolver, cs, true);
                     String dls = ip.getDlsQuery(user);
 
                     if (dls != null && dls.length() > 0) {
@@ -757,24 +757,36 @@ public String getUnresolvedIndexPattern(User user) {
             return UserAttributes.replaceProperties(indexPattern, user);
         }
 
+        /** Finds the indices accessible to the user and resolves them to concrete names */
+        public Set<String> concreteIndexNames(
+            final User user,
+            final IndexNameExpressionResolver resolver,
+            final ClusterService cs,
+            final boolean includeClosed
+        ) {
+            return getResolvedIndexPattern(user, resolver, cs, false, includeClosed);
+        }
+
         /** Finds the indices accessible to the user and resolves them to concrete names */
         public Set<String> concreteIndexNames(final User user, final IndexNameExpressionResolver resolver, final ClusterService cs) {
-            return getResolvedIndexPattern(user, resolver, cs, false);
+            return getResolvedIndexPattern(user, resolver, cs, false, false);
         }
 
         /** Finds the indices accessible to the user and attempts to resolve them to names, also includes any unresolved names */
         public Set<String> attemptResolveIndexNames(final User user, final IndexNameExpressionResolver resolver, final ClusterService cs) {
-            return getResolvedIndexPattern(user, resolver, cs, true);
+            return getResolvedIndexPattern(user, resolver, cs, true, false);
         }
 
         public Set<String> getResolvedIndexPattern(
             final User user,
             final IndexNameExpressionResolver resolver,
             final ClusterService cs,
-            final boolean appendUnresolved
+            final boolean appendUnresolved,
+            final boolean includeClosed
         ) {
             final String unresolved = getUnresolvedIndexPattern(user);
             final ImmutableSet.Builder<String> resolvedIndices = new ImmutableSet.Builder<>();
+            final IndicesOptions expansionMode = includeClosed ? IndicesOptions.lenientExpand() : IndicesOptions.lenientExpandOpen();
 
             final WildcardMatcher matcher = WildcardMatcher.from(unresolved);
             boolean includeDataStreams = true;
@@ -791,7 +803,7 @@ public Set<String> getResolvedIndexPattern(
                 if (aliasesAndDataStreamsForPermittedPattern.length > 0) {
                     final String[] resolvedAliasesAndDataStreamIndices = resolver.concreteIndexNames(
                         cs.state(),
-                        IndicesOptions.lenientExpandOpen(),
+                        expansionMode,
                         includeDataStreams,
                         aliasesAndDataStreamsForPermittedPattern
                     );
@@ -802,7 +814,7 @@ public Set<String> getResolvedIndexPattern(
             if (!(unresolved == null || unresolved.isBlank())) {
                 final String[] resolvedIndicesFromPattern = resolver.concreteIndexNames(
                     cs.state(),
-                    IndicesOptions.lenientExpandOpen(),
+                    expansionMode,
                     includeDataStreams,
                     unresolved
                 );

src/test/java/org/opensearch/security/privileges/SystemIndexAccessEvaluatorTest.java

@@ -151,7 +151,7 @@ public void setup(
         when(log.isDebugEnabled()).thenReturn(true);
         when(log.isInfoEnabled()).thenReturn(true);
 
-        doReturn(ImmutableSet.of(index)).when(ip).getResolvedIndexPattern(user, indexNameExpressionResolver, cs, true);
+        doReturn(ImmutableSet.of(index)).when(ip).getResolvedIndexPattern(user, indexNameExpressionResolver, cs, true, false);
     }
 
     @After

src/test/java/org/opensearch/security/securityconf/impl/v7/IndexPatternTests.java

@@ -79,27 +79,27 @@ public void testCtor() {
     /** Ensure that concreteIndexNames sends correct parameters are sent to getResolvedIndexPattern */
     @Test
     public void testConcreteIndexNamesOverload() {
-        doReturn(ImmutableSet.of("darn")).when(ip).getResolvedIndexPattern(user, resolver, clusterService, false);
+        doReturn(ImmutableSet.of("darn")).when(ip).getResolvedIndexPattern(user, resolver, clusterService, false, false);
 
         final Set<String> results = ip.concreteIndexNames(user, resolver, clusterService);
 
         assertThat(results, contains("darn"));
 
-        verify(ip).getResolvedIndexPattern(user, resolver, clusterService, false);
+        verify(ip).getResolvedIndexPattern(user, resolver, clusterService, false, false);
         verify(ip).concreteIndexNames(user, resolver, clusterService);
         verifyNoMoreInteractions(ip);
     }
 
     /** Ensure that attemptResolveIndexNames sends correct parameters are sent to getResolvedIndexPattern */
     @Test
     public void testAttemptResolveIndexNamesOverload() {
-        doReturn(ImmutableSet.of("yarn")).when(ip).getResolvedIndexPattern(user, resolver, clusterService, true);
+        doReturn(ImmutableSet.of("yarn")).when(ip).getResolvedIndexPattern(user, resolver, clusterService, true, false);
 
         final Set<String> results = ip.attemptResolveIndexNames(user, resolver, clusterService);
 
         assertThat(results, contains("yarn"));
 
-        verify(ip).getResolvedIndexPattern(user, resolver, clusterService, true);
+        verify(ip).getResolvedIndexPattern(user, resolver, clusterService, true, false);
         verify(ip).attemptResolveIndexNames(user, resolver, clusterService);
         verifyNoMoreInteractions(ip);
     }
@@ -140,6 +140,24 @@ public void testExactName() {
         verify(resolver).concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq(true), eq("index-17"));
     }
 
+    /** Verify concreteIndexNames on exact name matches */
+    @Test
+    public void testExactNameOnClosedIndex() {
+        doReturn("index-17").when(ip).getUnresolvedIndexPattern(user);
+        when(clusterService.state()).thenReturn(mock(ClusterState.class));
+        when(resolver.concreteIndexNames(any(), eq(IndicesOptions.lenientExpand()), eq(true), eq("index-17"))).thenReturn(
+            new String[] { "resolved-index-17" }
+        );
+
+        final Set<String> results = ip.concreteIndexNames(user, resolver, clusterService, true);
+
+        assertThat(results, contains("resolved-index-17"));
+
+        verify(clusterService).state();
+        verify(ip).getUnresolvedIndexPattern(user);
+        verify(resolver).concreteIndexNames(any(), eq(IndicesOptions.lenientExpand()), eq(true), eq("index-17"));
+    }
+
     /** Verify concreteIndexNames on multiple matches */
     @Test
     public void testMultipleConcreteIndices() {