From 0cfb2f327b62f6683378ecb7ebe6ff6a3ab2fae9 Mon Sep 17 00:00:00 2001 From: Peter Nied Date: Tue, 2 Aug 2022 01:30:59 +0000 Subject: [PATCH] When excluding fields also exclude the term + `.keyword` Signed-off-by: Peter Nied --- .../configuration/DlsFlsFilterLeafReader.java | 1 + .../security/dlic/dlsfls/FlsKeywordTests.java | 86 +++++++++++++++++++ src/test/resources/dlsfls/roles_keyword.yml | 18 ++++ .../dlsfls/roles_mappings_keyword.yml | 22 +++++ 4 files changed, 127 insertions(+) create mode 100644 src/test/java/org/opensearch/security/dlic/dlsfls/FlsKeywordTests.java create mode 100644 src/test/resources/dlsfls/roles_keyword.yml create mode 100644 src/test/resources/dlsfls/roles_mappings_keyword.yml diff --git a/src/main/java/org/opensearch/security/configuration/DlsFlsFilterLeafReader.java b/src/main/java/org/opensearch/security/configuration/DlsFlsFilterLeafReader.java index e94bdcb593..06a89ac099 100644 --- a/src/main/java/org/opensearch/security/configuration/DlsFlsFilterLeafReader.java +++ b/src/main/java/org/opensearch/security/configuration/DlsFlsFilterLeafReader.java @@ -142,6 +142,7 @@ class DlsFlsFilterLeafReader extends SequentialStoredFieldsLeafReader { if (firstChar == '!' || firstChar == '~') { excludesSet.add(incExc.substring(1)); + excludesSet.add(incExc.substring(1) + KEYWORD); } else { includesSet.add(incExc); } diff --git a/src/test/java/org/opensearch/security/dlic/dlsfls/FlsKeywordTests.java b/src/test/java/org/opensearch/security/dlic/dlsfls/FlsKeywordTests.java new file mode 100644 index 0000000000..25069e48ef --- /dev/null +++ b/src/test/java/org/opensearch/security/dlic/dlsfls/FlsKeywordTests.java @@ -0,0 +1,86 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.security.dlic.dlsfls; + +import java.util.Arrays; + +import org.apache.hc.core5.http.Header; +import org.apache.hc.core5.http.HttpStatus; +import org.junit.Test; + +import org.opensearch.action.index.IndexRequest; +import org.opensearch.action.support.WriteRequest.RefreshPolicy; +import org.opensearch.client.Client; +import org.opensearch.common.xcontent.XContentType; +import org.opensearch.security.test.DynamicSecurityConfig; +import org.opensearch.security.test.helper.rest.RestHelper.HttpResponse; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.core.IsEqual.equalTo; +import static org.hamcrest.core.IsNot.not; +import static org.hamcrest.core.StringContains.containsString; + +public class FlsKeywordTests extends AbstractDlsFlsTest { + + protected void populateData(Client tc) { + tc.index(new IndexRequest("movies").id("0").setRefreshPolicy(RefreshPolicy.IMMEDIATE) + .source("{\"year\": 2013, \"title\": \"Rush\", \"actors\": [\"Daniel Br\u00FChl\", \"Chris Hemsworth\", \"Olivia Wilde\"]}", XContentType.JSON)).actionGet(); + } + + private Header movieUser = encodeBasicHeader("user_aaa", "password"); + private Header movieNoActorUser = encodeBasicHeader("user_bbb", "password"); + + private String[] actors = new String[] {"Daniel Br\u00FChl", "Chris Hemsworth", "Olivia Wilde"}; + + @Test + public void testKeywordsAreAutomaticallyFiltered() throws Exception { + setup(new DynamicSecurityConfig() + .setSecurityRoles("roles_keyword.yml") + .setSecurityRolesMapping("roles_mappings_keyword.yml")); + + final String searchQuery = "/movies/_search?filter_path=hits.hits._source"; + final String aggQuery = "/movies/_search?filter_path=aggregations.actors.buckets.key"; + final String aggByActorKeyword = "{\"aggs\":{\"actors\":{\"terms\":{\"field\":\"actors.keyword\",\"size\":10}}}}"; + + // At document level, the user should see actors + final HttpResponse searchMovieUser = rh.executeGetRequest(searchQuery, movieUser); + assertThat(searchMovieUser.getStatusCode(), equalTo(HttpStatus.SC_OK)); + assertActorsPresent(searchMovieUser); + + // In aggregate search, the user should see actors + final HttpResponse searchAggregateMovieUser = rh.executePostRequest(aggQuery, aggByActorKeyword, movieUser); + assertThat(searchAggregateMovieUser.getStatusCode(), equalTo(HttpStatus.SC_OK)); + assertActorsPresent(searchAggregateMovieUser); + + // At document level, the user should see no actors + final HttpResponse searchMovieNoActorUser = rh.executeGetRequest(searchQuery, movieNoActorUser); + assertThat(searchMovieNoActorUser.getStatusCode(), equalTo(HttpStatus.SC_OK)); + assertActorsNotPresent(searchMovieNoActorUser); + + // In aggregate search, the user should see no actors + final HttpResponse searchAggregateMovieNoActorUser = rh.executePostRequest(aggQuery, aggByActorKeyword, movieNoActorUser); + assertThat(searchAggregateMovieNoActorUser.getStatusCode(), equalTo(HttpStatus.SC_OK)); + assertActorsNotPresent(searchAggregateMovieNoActorUser); + } + + private void assertActorsPresent(final HttpResponse response) { + Arrays.stream(actors).forEach(actor -> { + assertThat(response.getBody(), containsString(actor)); + }); + } + + private void assertActorsNotPresent(final HttpResponse response) { + Arrays.stream(actors).forEach(actor -> { + assertThat(response.getBody(), not(containsString(actor))); + }); + } +} diff --git a/src/test/resources/dlsfls/roles_keyword.yml b/src/test/resources/dlsfls/roles_keyword.yml new file mode 100644 index 0000000000..c93209562a --- /dev/null +++ b/src/test/resources/dlsfls/roles_keyword.yml @@ -0,0 +1,18 @@ +--- +_meta: + type: "roles" + config_version: 2 +movies: + index_permissions: + - index_patterns: + - "movies*" + allowed_actions: + - "read" +movies_no_actors: + index_permissions: + - index_patterns: + - "movies*" + fls: + - "~actors" + allowed_actions: + - "read" diff --git a/src/test/resources/dlsfls/roles_mappings_keyword.yml b/src/test/resources/dlsfls/roles_mappings_keyword.yml new file mode 100644 index 0000000000..c3ec6710d6 --- /dev/null +++ b/src/test/resources/dlsfls/roles_mappings_keyword.yml @@ -0,0 +1,22 @@ +--- +_meta: + type: "rolesmapping" + config_version: 2 +movies: + reserved: false + hidden: false + backend_roles: [] + hosts: [] + users: + - "user_aaa" + and_backend_roles: [] + description: "Movies with all fields" +movies_no_actors: + reserved: false + hidden: false + backend_roles: [] + hosts: [] + users: + - "user_bbb" + and_backend_roles: [] + description: "Movies without actors"