From cb81cc6fb87268fd9f9975074e68a47c66161193 Mon Sep 17 00:00:00 2001 From: "opensearch-trigger-bot[bot]" <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Date: Fri, 6 Jan 2023 14:24:51 -0600 Subject: [PATCH] [Backport 1.x] When excluding fields also exclude the term + `.keyword` (#2378) * [Backport 2.x] When excluding fields also exclude the term + `.keyword` (#2377) (cherry picked from commit b9652fecc3401f584cd9211a8d9aa33fee820a74) * Switch to transport client for 1.x Signed-off-by: Peter Nied Signed-off-by: Peter Nied Co-authored-by: opensearch-trigger-bot[bot] <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Co-authored-by: Peter Nied --- .../configuration/DlsFlsFilterLeafReader.java | 1 + .../security/dlic/dlsfls/FlsKeywordTests.java | 87 +++++++++++++++++++ src/test/resources/dlsfls/roles_keyword.yml | 18 ++++ .../dlsfls/roles_mappings_keyword.yml | 22 +++++ 4 files changed, 128 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 3d78dd9ac3..b15ac2f054 100644 --- a/src/main/java/org/opensearch/security/configuration/DlsFlsFilterLeafReader.java +++ b/src/main/java/org/opensearch/security/configuration/DlsFlsFilterLeafReader.java @@ -146,6 +146,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..f5cceb8beb --- /dev/null +++ b/src/test/java/org/opensearch/security/dlic/dlsfls/FlsKeywordTests.java @@ -0,0 +1,87 @@ +/* + * 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.http.Header; +import org.apache.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.client.transport.TransportClient; +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(TransportClient 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"