From b26c9e1ed1828d3edbd0a11d28c2173e3c31ce28 Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Tue, 3 Jul 2018 21:17:52 -0700 Subject: [PATCH 1/3] Add basic unit tests for field level security with field aliases. --- .../integration/FieldLevelSecurityTests.java | 223 +++++++++++++++--- 1 file changed, 191 insertions(+), 32 deletions(-) diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/integration/FieldLevelSecurityTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/integration/FieldLevelSecurityTests.java index d7d8c2ceb1be2..050d3094fb8ca 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/integration/FieldLevelSecurityTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/integration/FieldLevelSecurityTests.java @@ -30,8 +30,10 @@ import org.elasticsearch.join.ParentJoinPlugin; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.rest.RestStatus; +import org.elasticsearch.search.SearchHit; import org.elasticsearch.search.aggregations.AggregationBuilders; import org.elasticsearch.search.aggregations.bucket.terms.Terms; +import org.elasticsearch.search.fetch.subphase.highlight.HighlightBuilder; import org.elasticsearch.search.sort.SortOrder; import org.elasticsearch.test.ESIntegTestCase; import org.elasticsearch.test.InternalSettingsPlugin; @@ -161,10 +163,12 @@ public Settings nodeSettings(int nodeOrdinal) { .build(); } - public void testQuery() throws Exception { - assertAcked(client().admin().indices().prepareCreate("test") - .addMapping("type1", "field1", "type=text", "field2", "type=text", "field3", "type=text") - ); + public void testQuery() { + assertAcked(client().admin().indices().prepareCreate("test").addMapping("type1", + "field1", "type=text", + "field2", "type=text", + "field3", "type=text", + "alias", "type=alias,path=field1")); client().prepareIndex("test", "type1", "1").setSource("field1", "value1", "field2", "value2", "field3", "value3") .setRefreshPolicy(IMMEDIATE) .get(); @@ -299,6 +303,20 @@ public void testQuery() throws Exception { .setQuery(matchQuery("field3", "value3")) .get(); assertHitCount(response, 0); + + // user1 has access to field1, so a query on its field alias should match with the document: + response = client() + .filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user1", USERS_PASSWD))) + .prepareSearch("test") + .setQuery(matchQuery("alias", "value1")) + .get(); + assertHitCount(response, 1); + // user2 has no access to field1, so a query on its field alias should match with the document: + response = client().filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user2", USERS_PASSWD))) + .prepareSearch("test") + .setQuery(matchQuery("alias", "value1")) + .get(); + assertHitCount(response, 0); } public void testGetApi() throws Exception { @@ -793,10 +811,11 @@ public void testRequestCache() throws Exception { } public void testFields() throws Exception { - assertAcked(client().admin().indices().prepareCreate("test") - .addMapping("type1", "field1", "type=text,store=true", "field2", "type=text,store=true", - "field3", "type=text,store=true") - ); + assertAcked(client().admin().indices().prepareCreate("test").addMapping("type1", + "field1", "type=text,store=true", + "field2", "type=text,store=true", + "field3", "type=text,store=true", + "alias", "type=alias,path=field1")); client().prepareIndex("test", "type1", "1").setSource("field1", "value1", "field2", "value2", "field3", "value3") .setRefreshPolicy(IMMEDIATE) .get(); @@ -888,6 +907,22 @@ public void testFields() throws Exception { assertThat(response.getHits().getAt(0).getFields().size(), equalTo(2)); assertThat(response.getHits().getAt(0).getFields().get("field1").getValue(), equalTo("value1")); assertThat(response.getHits().getAt(0).getFields().get("field2").getValue(), equalTo("value2")); + + // user1 is granted access to field1 only, and so should be able to load it by alias: + response = client() + .filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user1", USERS_PASSWD))) + .prepareSearch("test") + .addStoredField("alias") + .get(); + assertThat(response.getHits().getAt(0).getFields().size(), equalTo(1)); + assertThat(response.getHits().getAt(0).getFields().get("alias").getValue(), equalTo("value1")); + + // user2 is not granted access to field1, and so should not be able to load it by alias: + response = client().filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user2", USERS_PASSWD))) + .prepareSearch("test") + .addStoredField("alias") + .get(); + assertThat(response.getHits().getAt(0).getFields().size(), equalTo(0)); } public void testSource() throws Exception { @@ -963,11 +998,11 @@ public void testSource() throws Exception { assertThat(response.getHits().getAt(0).getSourceAsMap().get("field2").toString(), equalTo("value2")); } - public void testSort() throws Exception { - assertAcked(client().admin().indices().prepareCreate("test") - .addMapping("type1", "field1", "type=long", "field2", "type=long") - ); - + public void testSort() { + assertAcked(client().admin().indices().prepareCreate("test").addMapping("type1", + "field1", "type=long", + "field2", "type=long", + "alias", "type=alias,path=field1")); client().prepareIndex("test", "type1", "1").setSource("field1", 1d, "field2", 2d) .setRefreshPolicy(IMMEDIATE) .get(); @@ -1000,12 +1035,81 @@ public void testSort() throws Exception { .addSort("field2", SortOrder.ASC) .get(); assertThat(response.getHits().getAt(0).getSortValues()[0], equalTo(2L)); + + // user1 is granted to use field1, so it is included in the sort_values when using its alias: + response = client() + .filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user1", USERS_PASSWD))) + .prepareSearch("test") + .addSort("alias", SortOrder.ASC) + .get(); + assertThat(response.getHits().getAt(0).getSortValues()[0], equalTo(1L)); + + // user2 is not granted to use field1, so the default missing sort value is included when using its alias: + response = client().filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user2", USERS_PASSWD))) + .prepareSearch("test") + .addSort("alias", SortOrder.ASC) + .get(); + assertThat(response.getHits().getAt(0).getSortValues()[0], equalTo(Long.MAX_VALUE)); } - public void testAggs() throws Exception { - assertAcked(client().admin().indices().prepareCreate("test") - .addMapping("type1", "field1", "type=text,fielddata=true", "field2", "type=text,fielddata=true") - ); + public void testHighlighting() { + assertAcked(client().admin().indices().prepareCreate("test").addMapping("type1", + "field1", "type=text", + "field2", "type=text", + "field3", "type=text", + "alias", "type=alias,path=field1")); + client().prepareIndex("test", "type1", "1").setSource("field1", "value1", "field2", "value2", "field3", "value3") + .setRefreshPolicy(IMMEDIATE) + .get(); + + // user1 has access to field1, so the highlight should be visible: + SearchResponse response = client() + .filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user1", USERS_PASSWD))) + .prepareSearch("test") + .setQuery(matchQuery("field1", "value1")) + .highlighter(new HighlightBuilder().field("field1")) + .get(); + assertHitCount(response, 1); + SearchHit hit = response.getHits().iterator().next(); + assertEquals(hit.getHighlightFields().size(), 1); + + // user2 has no access to field1, so the highlight should not be visible: + response = client().filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user2", USERS_PASSWD))) + .prepareSearch("test") + .setQuery(matchQuery("field2", "value2")) + .highlighter(new HighlightBuilder().field("field1")) + .get(); + assertHitCount(response, 1); + hit = response.getHits().iterator().next(); + assertEquals(hit.getHighlightFields().size(), 0); + + // user1 has access to field1, so the highlight on its alias should be visible: + response = client() + .filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user1", USERS_PASSWD))) + .prepareSearch("test") + .setQuery(matchQuery("field1", "value1")) + .highlighter(new HighlightBuilder().field("alias")) + .get(); + assertHitCount(response, 1); + hit = response.getHits().iterator().next(); + assertEquals(hit.getHighlightFields().size(), 1); + + // user2 has no access to field1, so the highlight on its alias should not be visible: + response = client().filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user2", USERS_PASSWD))) + .prepareSearch("test") + .setQuery(matchQuery("field2", "value2")) + .highlighter(new HighlightBuilder().field("alias")) + .get(); + assertHitCount(response, 1); + hit = response.getHits().iterator().next(); + assertEquals(hit.getHighlightFields().size(), 0); + } + + public void testAggs() { + assertAcked(client().admin().indices().prepareCreate("test").addMapping("type1", + "field1", "type=text,fielddata=true", + "field2", "type=text,fielddata=true", + "alias", "type=alias,path=field1")); client().prepareIndex("test", "type1", "1").setSource("field1", "value1", "field2", "value2") .setRefreshPolicy(IMMEDIATE) .get(); @@ -1038,6 +1142,21 @@ public void testAggs() throws Exception { .addAggregation(AggregationBuilders.terms("_name").field("field2")) .get(); assertThat(((Terms) response.getAggregations().get("_name")).getBucketByKey("value2").getDocCount(), equalTo(1L)); + + // user1 is authorized to use field1, so buckets are include for a term agg on its alias: + response = client() + .filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user1", USERS_PASSWD))) + .prepareSearch("test") + .addAggregation(AggregationBuilders.terms("_name").field("alias")) + .get(); + assertThat(((Terms) response.getAggregations().get("_name")).getBucketByKey("value1").getDocCount(), equalTo(1L)); + + // user2 is not authorized to use field1, so no buckets are include for a term agg on its alias: + response = client().filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user2", USERS_PASSWD))) + .prepareSearch("test") + .addAggregation(AggregationBuilders.terms("_name").field("alias")) + .get(); + assertThat(((Terms) response.getAggregations().get("_name")).getBucketByKey("value1"), nullValue()); } public void testTVApi() throws Exception { @@ -1218,15 +1337,22 @@ public void testMTVApi() throws Exception { public void testParentChild() throws Exception { XContentBuilder mapping = XContentFactory.jsonBuilder().startObject() - .startObject("properties") - .startObject("join_field") - .field("type", "join") - .startObject("relations") - .field("parent", "child") - .endObject() - .endObject() - .endObject() - .endObject(); + .startObject("properties") + .startObject("field1") + .field("type", "keyword") + .endObject() + .startObject("alias") + .field("type", "alias") + .field("path", "field1") + .endObject() + .startObject("join_field") + .field("type", "join") + .startObject("relations") + .field("parent", "child") + .endObject() + .endObject() + .endObject() + .endObject(); assertAcked(prepareCreate("test") .addMapping("doc", mapping)); ensureGreen(); @@ -1264,6 +1390,23 @@ private void verifyParentChild() { .setQuery(hasChildQuery("child", termQuery("field1", "yellow"), ScoreMode.None)) .get(); assertHitCount(searchResponse, 0L); + + // Perform the same checks, but using an alias for field1. + searchResponse = client() + .filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user1", USERS_PASSWD))) + .prepareSearch("test") + .setQuery(hasChildQuery("child", termQuery("alias", "yellow"), ScoreMode.None)) + .get(); + assertHitCount(searchResponse, 1L); + assertThat(searchResponse.getHits().getTotalHits(), equalTo(1L)); + assertThat(searchResponse.getHits().getAt(0).getId(), equalTo("p1")); + + searchResponse = client() + .filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user2", USERS_PASSWD))) + .prepareSearch("test") + .setQuery(hasChildQuery("child", termQuery("alias", "yellow"), ScoreMode.None)) + .get(); + assertHitCount(searchResponse, 0L); } public void testUpdateApiIsBlocked() throws Exception { @@ -1315,10 +1458,9 @@ public void testUpdateApiIsBlocked() throws Exception { assertThat(client().prepareGet("test", "type", "1").get().getSource().get("field2").toString(), equalTo("value3")); } - public void testQuery_withRoleWithFieldWildcards() throws Exception { + public void testQuery_withRoleWithFieldWildcards() { assertAcked(client().admin().indices().prepareCreate("test") - .addMapping("type1", "field1", "type=text", "field2", "type=text") - ); + .addMapping("type1", "field1", "type=text", "field2", "type=text")); client().prepareIndex("test", "type1", "1").setSource("field1", "value1", "field2", "value2") .setRefreshPolicy(IMMEDIATE) .get(); @@ -1345,9 +1487,12 @@ public void testQuery_withRoleWithFieldWildcards() throws Exception { } public void testExistQuery() { - assertAcked(client().admin().indices().prepareCreate("test") - .addMapping("type1", "field1", "type=text", "field2", "type=text", "field3", "type=text") - ); + assertAcked(client().admin().indices().prepareCreate("test").addMapping("type1", + "field1", "type=text", + "field2", "type=text", + "field3", "type=text", + "alias", "type=alias,path=field1")); + client().prepareIndex("test", "type1", "1").setSource("field1", "value1", "field2", "value2", "field3", "value3") .setRefreshPolicy(IMMEDIATE) .get(); @@ -1402,6 +1547,20 @@ public void testExistQuery() { .setQuery(existsQuery("field2")) .get(); assertHitCount(response, 0); + + // user1 has access to field1, so a query on its alias should match with the document: + response = client() + .filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user1", USERS_PASSWD))) + .prepareSearch("test") + .setQuery(existsQuery("alias")) + .get(); + assertHitCount(response, 1); + // user2 has no access to field1, so the query should not match with the document: + response = client().filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user2", USERS_PASSWD))) + .prepareSearch("test") + .setQuery(existsQuery("alias")) + .get(); + assertHitCount(response, 0); } } From 86d11b97c531d798ef74f544f4aefaeb210eb36f Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Tue, 3 Jul 2018 23:28:06 -0700 Subject: [PATCH 2/3] Ensure that field caps information is filtered when a field alias is provided. --- ...TransportFieldCapabilitiesIndexAction.java | 4 +- .../search/fieldcaps/FieldCapabilitiesIT.java | 43 ++++++++++++++++++- 2 files changed, 44 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesIndexAction.java b/server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesIndexAction.java index f1a1dc451406e..18f33ab397f73 100644 --- a/server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesIndexAction.java +++ b/server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesIndexAction.java @@ -83,8 +83,8 @@ protected FieldCapabilitiesIndexResponse shardOperation(final FieldCapabilitiesI for (String field : fieldNames) { MappedFieldType ft = mapperService.fullName(field); if (ft != null) { - FieldCapabilities fieldCap = new FieldCapabilities(field, ft.typeName(), ft.isSearchable(), ft.isAggregatable()); - if (indicesService.isMetaDataField(field) || fieldPredicate.test(field)) { + if (indicesService.isMetaDataField(field) || fieldPredicate.test(ft.name())) { + FieldCapabilities fieldCap = new FieldCapabilities(field, ft.typeName(), ft.isSearchable(), ft.isAggregatable()); responseMap.put(field, fieldCap); } } diff --git a/server/src/test/java/org/elasticsearch/search/fieldcaps/FieldCapabilitiesIT.java b/server/src/test/java/org/elasticsearch/search/fieldcaps/FieldCapabilitiesIT.java index 90b1437c93c06..8440357758ea8 100644 --- a/server/src/test/java/org/elasticsearch/search/fieldcaps/FieldCapabilitiesIT.java +++ b/server/src/test/java/org/elasticsearch/search/fieldcaps/FieldCapabilitiesIT.java @@ -23,10 +23,16 @@ import org.elasticsearch.action.fieldcaps.FieldCapabilitiesResponse; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.plugins.MapperPlugin; +import org.elasticsearch.plugins.Plugin; import org.elasticsearch.test.ESIntegTestCase; import org.junit.Before; +import java.util.Collection; +import java.util.Collections; import java.util.Map; +import java.util.function.Function; +import java.util.function.Predicate; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; @@ -47,6 +53,13 @@ public void setUp() throws Exception { .field("type", "alias") .field("path", "distance") .endObject() + .startObject("playlist") + .field("type", "text") + .endObject() + .startObject("secret_soundtrack") + .field("type", "alias") + .field("path", "playlist") + .endObject() .endObject() .endObject() .endObject(); @@ -68,6 +81,18 @@ public void setUp() throws Exception { assertAcked(prepareCreate("new_index").addMapping("_doc", newIndexMapping)); } + public static class FieldFilterPlugin extends Plugin implements MapperPlugin { + @Override + public Function> getFieldFilter() { + return index -> field -> !field.equals("playlist"); + } + } + + @Override + protected Collection> nodePlugins() { + return Collections.singleton(FieldFilterPlugin.class); + } + public void testFieldAlias() { FieldCapabilitiesResponse response = client().prepareFieldCaps().setFields("distance", "route_length_miles") .execute().actionGet(); @@ -100,11 +125,27 @@ public void testFieldAlias() { routeLength.get("double")); } - public void testFieldAliasWithWildcardField() { + public void testFieldAliasWithWildcard() { FieldCapabilitiesResponse response = client().prepareFieldCaps().setFields("route*") .execute().actionGet(); assertEquals(1, response.get().size()); assertTrue(response.get().containsKey("route_length_miles")); } + + public void testFieldAliasFiltering() { + FieldCapabilitiesResponse response = client().prepareFieldCaps().setFields( + "secret-soundtrack", "route_length_miles") + .execute().actionGet(); + assertEquals(1, response.get().size()); + assertTrue(response.get().containsKey("route_length_miles")); + } + + public void testFieldAliasFilteringWithWildcard() { + FieldCapabilitiesResponse response = client().prepareFieldCaps() + .setFields("distance", "secret*") + .execute().actionGet(); + assertEquals(1, response.get().size()); + assertTrue(response.get().containsKey("distance")); + } } From 89e136dac283cc58b0a75f383d53cbdc0a8ab71b Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Mon, 16 Jul 2018 10:00:45 -0700 Subject: [PATCH 3/3] Correct a comment in FieldLevelSecurityTests. --- .../org/elasticsearch/integration/FieldLevelSecurityTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/integration/FieldLevelSecurityTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/integration/FieldLevelSecurityTests.java index 050d3094fb8ca..dd41bc5a7fdb7 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/integration/FieldLevelSecurityTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/integration/FieldLevelSecurityTests.java @@ -311,7 +311,7 @@ public void testQuery() { .setQuery(matchQuery("alias", "value1")) .get(); assertHitCount(response, 1); - // user2 has no access to field1, so a query on its field alias should match with the document: + // user2 has no access to field1, so a query on its field alias should not match with the document: response = client().filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user2", USERS_PASSWD))) .prepareSearch("test") .setQuery(matchQuery("alias", "value1"))