Improve permission granting for index referred by multiple names (ela…

…stic#78902) (elastic#79220) A concrete index can be referred explicitly or implicitly by multiple names including its own name, data stream name, aliases. This PR ensures the permission granting is done correctly in an edge case where the same concrete index is referred by multiple names and the user have different (disagreeing) privileges over these names.
DaveCTurner · Oct 15, 2021 · 259fec8 · 259fec8
1 parent c5b2ce6
commit 259fec8
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 3 deletions.
diff --git a/...c/main/java/org/elasticsearch/xpack/core/security/authz/permission/IndicesPermission.java b/...c/main/java/org/elasticsearch/xpack/core/security/authz/permission/IndicesPermission.java
@@ -265,6 +265,10 @@ public Collection<String> resolveConcreteIndices() {
                 return concreteIndices;
             }
         }
+
+        public boolean canHaveBackingIndices() {
+            return indexAbstraction != null && indexAbstraction.getType() != IndexAbstraction.Type.CONCRETE_INDEX;
+        }
     }
 
     /**
@@ -391,8 +395,13 @@ public Map<String, IndicesAccessControl.IndexAccessControl> authorize(
             }
 
             grantedBuilder.put(resource.name, granted);
-            for (String concreteIndex : concreteIndices) {
-                grantedBuilder.put(concreteIndex, granted);
+            if (resource.canHaveBackingIndices()) {
+                for (String concreteIndex : concreteIndices) {
+                    // If the name appear directly as part of the requested indices, it takes precedence over implicit access
+                    if (false == requestedIndicesOrAliases.contains(concreteIndex)) {
+                        grantedBuilder.merge(concreteIndex, granted, Boolean::logicalOr);
+                    }
+                }
             }
         }
 

diff --git a/...ternalClusterTest/java/org/elasticsearch/integration/MultipleIndicesPermissionsTests.java b/...ternalClusterTest/java/org/elasticsearch/integration/MultipleIndicesPermissionsTests.java
@@ -8,6 +8,7 @@
 
 import org.elasticsearch.ElasticsearchSecurityException;
 import org.elasticsearch.action.DocWriteResponse;
+import org.elasticsearch.action.admin.indices.alias.Alias;
 import org.elasticsearch.action.admin.indices.recovery.RecoveryResponse;
 import org.elasticsearch.action.admin.indices.segments.IndicesSegmentResponse;
 import org.elasticsearch.action.admin.indices.settings.get.GetSettingsResponse;
@@ -22,6 +23,7 @@
 import org.elasticsearch.client.RequestOptions;
 import org.elasticsearch.client.Response;
 import org.elasticsearch.common.settings.SecureString;
+import org.elasticsearch.index.query.QueryBuilders;
 import org.elasticsearch.rest.RestStatus;
 import org.elasticsearch.test.SecurityIntegTestCase;
 import org.elasticsearch.test.SecuritySettingsSource;
@@ -33,14 +35,18 @@
 
 import java.util.Collections;
 
+import static org.elasticsearch.action.support.WriteRequest.RefreshPolicy.IMMEDIATE;
+import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
 import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder;
 import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery;
 import static org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken.BASIC_AUTH_HEADER;
 import static org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken.basicAuthHeaderValue;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailures;
-import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.containsInAnyOrder;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.is;
 
 public class MultipleIndicesPermissionsTests extends SecurityIntegTestCase {
 
@@ -81,6 +87,8 @@ protected String configRoles() {
                 "  indices:\n" +
                 "    - names: 'a'\n" +
                 "      privileges: [all]\n" +
+                "    - names: 'alias1'\n" +
+                "      privileges: [read]\n" +
                 "\n" +
                 "role_monitor_all_unrestricted_indices:\n" +
                 "  cluster: [monitor]\n" +
@@ -304,4 +312,29 @@ public void testMultipleRoles() throws Exception {
         assertNoFailures(response);
         assertHitCount(response, 2);
     }
+
+    public void testMultiNamesWorkCorrectly() {
+        assertAcked(client().admin().indices().prepareCreate("index1")
+            .addMapping("_doc","field1", "type=text")
+            .addAlias(new Alias("alias1").filter(QueryBuilders.termQuery("field1", "public")))
+        );
+
+        client().prepareIndex("index1", "_doc").setId("1").setSource("field1", "private").setRefreshPolicy(IMMEDIATE).get();
+
+        final Client userAClient = client()
+            .filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user_a", USERS_PASSWD)));
+
+        final SearchResponse searchResponse = userAClient
+            .prepareSearch("alias1").setSize(0).get();
+        assertThat(searchResponse.getHits().getTotalHits().value, equalTo(0L));
+
+        final ElasticsearchSecurityException e1 =
+            expectThrows(ElasticsearchSecurityException.class, () -> userAClient.prepareSearch("index1").get());
+        assertThat(e1.getMessage(), containsString("is unauthorized for user [user_a]"));
+
+        // The request should fail because index1 is directly requested and is not authorized for user_a
+        final ElasticsearchSecurityException e2 =
+            expectThrows(ElasticsearchSecurityException.class, () -> userAClient.prepareSearch("index1", "alias1").get());
+        assertThat(e2.getMessage(), containsString("is unauthorized for user [user_a]"));
+    }
 }