Preserve aliases in index permissions (elastic/x-pack-elasticsearch#1501)

Aliases might be contained in requests that check index permissions to disable caches etc. This commit preserves permissions for aliases as well. Original commit: elastic/x-pack-elasticsearch@233195aeba
2017-05-20 21:34:38 +02:00 · 2017-05-20 21:34:38 +02:00 · 392e67851e
parent 883f5d8a7a
commit 392e67851e
5 changed files with 252 additions and 1 deletions
--- a/plugin/src/main/java/org/elasticsearch/xpack/security/action/interceptor/FieldAndDocumentLevelSecurityRequestInterceptor.java
+++ b/plugin/src/main/java/org/elasticsearch/xpack/security/action/interceptor/FieldAndDocumentLevelSecurityRequestInterceptor.java
@ -35,7 +35,7 @@ abstract class FieldAndDocumentLevelSecurityRequestInterceptor<Request extends I
        if (licenseState.isDocumentAndFieldLevelSecurityAllowed() == false) {
            return;
        }
-        IndicesAccessControl indicesAccessControl = threadContext.getTransient(AuthorizationService.INDICES_PERMISSIONS_KEY);
+        final IndicesAccessControl indicesAccessControl = threadContext.getTransient(AuthorizationService.INDICES_PERMISSIONS_KEY);
        for (String index : request.indices()) {
            IndicesAccessControl.IndexAccessControl indexAccessControl = indicesAccessControl.getIndexPermissions(index);
            if (indexAccessControl != null) {
--- a/plugin/src/main/java/org/elasticsearch/xpack/security/authz/accesscontrol/IndicesAccessControl.java
+++ b/plugin/src/main/java/org/elasticsearch/xpack/security/authz/accesscontrol/IndicesAccessControl.java
@ -85,5 +85,22 @@ public class IndicesAccessControl {
        public Set<BytesReference> getQueries() {
            return queries;
        }
+
+        @Override
+        public String toString() {
+            return "IndexAccessControl{" +
+                    "granted=" + granted +
+                    ", fieldPermissions=" + fieldPermissions +
+                    ", queries=" + queries +
+                    '}';
+        }
+    }
+
+    @Override
+    public String toString() {
+        return "IndicesAccessControl{" +
+                "granted=" + granted +
+                ", indexPermissions=" + indexPermissions +
+                '}';
    }
 }
--- a/plugin/src/main/java/org/elasticsearch/xpack/security/authz/permission/IndicesPermission.java
+++ b/plugin/src/main/java/org/elasticsearch/xpack/security/authz/permission/IndicesPermission.java
@ -118,9 +118,11 @@ public final class IndicesPermission implements Iterable<IndicesPermission.Group
                    granted = true;
                    for (String index : concreteIndices) {
                        Set<FieldPermissions> fieldPermissions = fieldPermissionsByIndex.computeIfAbsent(index, (k) -> new HashSet<>());
+                        fieldPermissionsByIndex.put(indexOrAlias, fieldPermissions);
                        fieldPermissions.add(group.getFieldPermissions());
                        DocumentLevelPermissions permissions =
                                roleQueriesByIndex.computeIfAbsent(index, (k) -> new DocumentLevelPermissions());
+                        roleQueriesByIndex.putIfAbsent(indexOrAlias, permissions);
                        if (group.hasQuery()) {
                            permissions.addAll(group.getQuery());
                        } else {
@ -136,6 +138,7 @@ public final class IndicesPermission implements Iterable<IndicesPermission.Group
            if (concreteIndices.isEmpty()) {
                grantedBuilder.put(indexOrAlias, granted);
            } else {
+                grantedBuilder.put(indexOrAlias, granted);
                for (String concreteIndex : concreteIndices) {
                    grantedBuilder.put(concreteIndex, granted);
                }
--- a/plugin/src/test/java/org/elasticsearch/xpack/security/authz/accesscontrol/IndicesPermissionTests.java
+++ b/plugin/src/test/java/org/elasticsearch/xpack/security/authz/accesscontrol/IndicesPermissionTests.java
@ -16,6 +16,8 @@ import org.elasticsearch.common.io.stream.BytesStreamOutput;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.util.set.Sets;
+import org.elasticsearch.index.query.QueryBuilder;
+import org.elasticsearch.index.query.QueryBuilders;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.xpack.security.authz.RoleDescriptor;
 import org.elasticsearch.xpack.security.authz.permission.FieldPermissions;
@ -26,7 +28,9 @@ import org.elasticsearch.xpack.security.authz.permission.Role;
 import org.elasticsearch.xpack.security.authz.privilege.IndexPrivilege;

 import java.io.IOException;
+import java.util.Arrays;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;

@ -87,6 +91,12 @@ public class IndicesPermissionTests extends ESTestCase {
        assertThat(permissions.getIndexPermissions("_index").getQueries().size(), equalTo(1));
        assertThat(permissions.getIndexPermissions("_index").getQueries(), equalTo(query));

+        assertThat(permissions.getIndexPermissions("_alias"), notNullValue());
+        assertTrue(permissions.getIndexPermissions("_alias").getFieldPermissions().grantsAccessTo("_field"));
+        assertTrue(permissions.getIndexPermissions("_alias").getFieldPermissions().hasFieldLevelSecurity());
+        assertThat(permissions.getIndexPermissions("_alias").getQueries().size(), equalTo(1));
+        assertThat(permissions.getIndexPermissions("_alias").getQueries(), equalTo(query));
+
        // match all fields
        String[] allFields = randomFrom(new String[]{"*"}, new String[]{"foo", "*"},
        new String[]{randomAlphaOfLengthBetween(1, 10), "*"});
@ -97,6 +107,46 @@ public class IndicesPermissionTests extends ESTestCase {
        assertFalse(permissions.getIndexPermissions("_index").getFieldPermissions().hasFieldLevelSecurity());
        assertThat(permissions.getIndexPermissions("_index").getQueries().size(), equalTo(1));
        assertThat(permissions.getIndexPermissions("_index").getQueries(), equalTo(query));
+
+        assertThat(permissions.getIndexPermissions("_alias"), notNullValue());
+        assertFalse(permissions.getIndexPermissions("_alias").getFieldPermissions().hasFieldLevelSecurity());
+        assertThat(permissions.getIndexPermissions("_alias").getQueries().size(), equalTo(1));
+        assertThat(permissions.getIndexPermissions("_alias").getQueries(), equalTo(query));
+
+        IndexMetaData.Builder imbBuilder1 = IndexMetaData.builder("_index_1")
+                .settings(Settings.builder()
+                        .put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, 1)
+                        .put(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, 1)
+                        .put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)
+                )
+                .putAlias(AliasMetaData.builder("_alias"));
+        md = MetaData.builder(md).put(imbBuilder1).build();
+
+
+        // match all fields with more than one permission
+        Set<BytesReference> fooQuery = Collections.singleton(new BytesArray("{foo}"));
+        allFields = randomFrom(new String[]{"*"}, new String[]{"foo", "*"},
+                new String[]{randomAlphaOfLengthBetween(1, 10), "*"});
+        role = Role.builder("_role")
+                .add(new FieldPermissions(fieldPermissionDef(allFields, null)), fooQuery, IndexPrivilege.ALL, "_alias")
+                .add(new FieldPermissions(fieldPermissionDef(allFields, null)), query, IndexPrivilege.ALL, "_alias").build();
+        permissions = role.authorize(SearchAction.NAME, Sets.newHashSet("_alias"), md, fieldPermissionsCache);
+        Set<BytesReference> bothQueries = Sets.union(fooQuery, query);
+        assertThat(permissions.getIndexPermissions("_index"), notNullValue());
+        assertFalse(permissions.getIndexPermissions("_index").getFieldPermissions().hasFieldLevelSecurity());
+        assertThat(permissions.getIndexPermissions("_index").getQueries().size(), equalTo(2));
+        assertThat(permissions.getIndexPermissions("_index").getQueries(), equalTo(bothQueries));
+
+        assertThat(permissions.getIndexPermissions("_index_1"), notNullValue());
+        assertFalse(permissions.getIndexPermissions("_index_1").getFieldPermissions().hasFieldLevelSecurity());
+        assertThat(permissions.getIndexPermissions("_index_1").getQueries().size(), equalTo(2));
+        assertThat(permissions.getIndexPermissions("_index_1").getQueries(), equalTo(bothQueries));
+
+        assertThat(permissions.getIndexPermissions("_alias"), notNullValue());
+        assertFalse(permissions.getIndexPermissions("_alias").getFieldPermissions().hasFieldLevelSecurity());
+        assertThat(permissions.getIndexPermissions("_alias").getQueries().size(), equalTo(2));
+        assertThat(permissions.getIndexPermissions("_alias").getQueries(), equalTo(bothQueries));
+
    }

    public void testAuthorizeMultipleGroupsMixedDls() {
--- a/plugin/src/test/resources/rest-api-spec/test/authenticate/10_field_level_security.yml
+++ b/plugin/src/test/resources/rest-api-spec/test/authenticate/10_field_level_security.yml
@ -0,0 +1,181 @@
+---
+setup:
+  - skip:
+      features: headers
+
+  - do:
+      cluster.health:
+          wait_for_status: yellow
+
+  - do:
+      xpack.security.put_role:
+        name: "readall"
+        body:  >
+            {
+              "indices": [
+                {
+                  "names": ["*"],
+                  "privileges": ["read"]
+                }
+              ]
+            }
+
+  - do:
+      xpack.security.put_role:
+        name: "limitread"
+        body:  >
+            {
+              "indices": [
+                {
+                  "names": ["*"],
+                  "privileges": ["read"],
+                  "query": {"match": {"marker": "test_1"}}
+                }
+              ]
+            }
+
+  - do:
+      xpack.security.put_user:
+        username: "full"
+        body:  >
+          {
+            "password" : "changeme",
+            "roles" : [ "readall" ],
+            "full_name" : "user who can read all data"
+          }
+
+  - do:
+      xpack.security.put_user:
+        username: "limited"
+        body:  >
+          {
+            "password" : "changeme",
+            "roles" : [ "limitread" ],
+            "full_name" : "user who can read some data"
+          }
+---
+teardown:
+  - do:
+      xpack.security.delete_user:
+        username: "full"
+        ignore: 404
+
+  - do:
+      xpack.security.delete_user:
+        username: "limited"
+        ignore: 404
+
+  - do:
+      xpack.security.delete_role:
+        name: "readall"
+        ignore: 404
+
+  - do:
+      xpack.security.delete_role:
+        name: "limitread"
+        ignore: 404
+
+---
+"Test doc level security against alias with different users":
+
+  - do:
+      indices.create:
+        index: test_index
+        body:
+          aliases:
+            the_alias : {}
+          mappings:
+            doc:
+              properties:
+                location:
+                  properties:
+                    city:
+                      type: "keyword"
+          settings:
+            index:
+              number_of_shards: 1
+              number_of_replicas: 0
+
+  - do:
+      bulk:
+        refresh: true
+        body:
+            - '{"index": {"_index": "test_index", "_type": "doc"}}'
+            - '{"marker": "test_1", "location.city": "bos"}'
+            - '{"index": {"_index": "test_index", "_type": "doc"}}'
+            - '{"marker": "test_2", "location.city": "ams"}'
+
+  - do:
+      headers: { Authorization: "Basic ZnVsbDpjaGFuZ2VtZQ==" } # full - user
+      search:
+        index: the_alias
+        size: 0
+        from: 0
+        body:
+          aggs:
+            cities:
+              terms:
+                field: location.city
+
+  - match: { _shards.total: 1 }
+  - match: { hits.total: 2 }
+  - length: { aggregations.cities.buckets: 2 }
+  - match: { aggregations.cities.buckets.0.key: "ams" }
+  - match: { aggregations.cities.buckets.0.doc_count: 1 }
+  - match: { aggregations.cities.buckets.1.key: "bos" }
+  - match: { aggregations.cities.buckets.1.doc_count: 1 }
+
+  - do:
+      headers: { Authorization: "Basic bGltaXRlZDpjaGFuZ2VtZQ==" } # limited - user
+      search:
+        index: the_alias
+        size: 0
+        from: 0
+        body:
+          aggs:
+            cities:
+              terms:
+                field: location.city
+
+  - match: { _shards.total: 1 }
+  - match: { hits.total: 1 }
+  - length: { aggregations.cities.buckets: 1 }
+  - match: { aggregations.cities.buckets.0.key: "bos" }
+  - match: { aggregations.cities.buckets.0.doc_count: 1 }
+
+  - do:
+      headers: { Authorization: "Basic bGltaXRlZDpjaGFuZ2VtZQ==" } # limited - user
+      search:
+        index: the_*
+        size: 0
+        from: 0
+        body:
+          aggs:
+            cities:
+              terms:
+                field: location.city
+
+  - match: { _shards.total: 1 }
+  - match: { hits.total: 1 }
+  - length: { aggregations.cities.buckets: 1 }
+  - match: { aggregations.cities.buckets.0.key: "bos" }
+  - match: { aggregations.cities.buckets.0.doc_count: 1 }
+
+  - do:
+      headers: { Authorization: "Basic bGltaXRlZDpjaGFuZ2VtZQ==" } # limited - user
+      search:
+        index: test_*
+        size: 0
+        from: 0
+        body:
+          aggs:
+            cities:
+              terms:
+                field: location.city
+
+  - match: { _shards.total: 1 }
+  - match: { hits.total: 1 }
+  - length: { aggregations.cities.buckets: 1 }
+  - match: { aggregations.cities.buckets.0.key: "bos" }
+  - match: { aggregations.cities.buckets.0.doc_count: 1 }
+