From d9643a2f285b53ddd68494fb3aa8e1f1732b391a Mon Sep 17 00:00:00 2001 From: Sean Li Date: Thu, 19 Oct 2023 14:03:27 -0700 Subject: [PATCH] Add RenameFieldResponseProcessor FLS/DLS tests (#3562) ### Description Adds FLS/DLS tests for RenameFieldResponseProcessor * Category Enhancement * Why these changes are required? Testing to ensure the RenameFieldResponseProcessor complies with FLS/DLS security * What is the old behavior before changes and new behavior after changes? No difference ### Issues Resolved #2890 Is this a backport? If so, please add backport PR # and/or commits #: No, but it does need to be backported. ### Testing N/A ### Check List - [x] New functionality includes testing - [ ] New functionality has been documented - [x] Commits are signed per the DCO using --signoff By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. For more information on following Developer Certificate of Origin and signing off your commits, please check [here](https://github.com/opensearch-project/OpenSearch/blob/main/CONTRIBUTING.md#developer-certificate-of-origin). --------- Signed-off-by: Sean Li --- build.gradle | 1 + .../dlic/dlsfls/AbstractDlsFlsTest.java | 16 +- .../RenameFieldResponseProcessorTest.java | 245 ++++++++++++++++++ .../helper/cluster/ClusterConfiguration.java | 10 +- .../dlsfls/roles_flsdls_rename_processor.yml | 25 ++ .../roles_mapping_flsdls_rename_processor.yml | 22 ++ 6 files changed, 315 insertions(+), 4 deletions(-) create mode 100644 src/test/java/org/opensearch/security/dlic/dlsfls/RenameFieldResponseProcessorTest.java create mode 100644 src/test/resources/dlsfls/roles_flsdls_rename_processor.yml create mode 100644 src/test/resources/dlsfls/roles_mapping_flsdls_rename_processor.yml diff --git a/build.gradle b/build.gradle index 448780554c..2267039cef 100644 --- a/build.gradle +++ b/build.gradle @@ -650,6 +650,7 @@ dependencies { testImplementation "org.opensearch.plugin:lang-mustache-client:${opensearch_version}" testImplementation "org.opensearch.plugin:parent-join-client:${opensearch_version}" testImplementation "org.opensearch.plugin:aggs-matrix-stats-client:${opensearch_version}" + testImplementation "org.opensearch.plugin:search-pipeline-common:${opensearch_version}" testImplementation "org.apache.logging.log4j:log4j-core:${versions.log4j}" testImplementation 'javax.servlet:servlet-api:2.5' testImplementation 'com.unboundid:unboundid-ldapsdk:4.0.14' diff --git a/src/test/java/org/opensearch/security/dlic/dlsfls/AbstractDlsFlsTest.java b/src/test/java/org/opensearch/security/dlic/dlsfls/AbstractDlsFlsTest.java index dfe96bb55d..0edb14ce73 100644 --- a/src/test/java/org/opensearch/security/dlic/dlsfls/AbstractDlsFlsTest.java +++ b/src/test/java/org/opensearch/security/dlic/dlsfls/AbstractDlsFlsTest.java @@ -30,6 +30,7 @@ import org.opensearch.security.support.ConfigConstants; import org.opensearch.security.test.DynamicSecurityConfig; import org.opensearch.security.test.SingleClusterTest; +import org.opensearch.security.test.helper.cluster.ClusterConfiguration; import org.opensearch.security.test.helper.rest.RestHelper; import org.opensearch.security.test.helper.rest.RestHelper.HttpResponse; @@ -47,16 +48,25 @@ protected final void setup() throws Exception { } protected final void setup(Settings override) throws Exception { - setup(override, new DynamicSecurityConfig()); + setup(override, new DynamicSecurityConfig(), ClusterConfiguration.DEFAULT); } protected final void setup(DynamicSecurityConfig dynamicSecurityConfig) throws Exception { - setup(Settings.EMPTY, dynamicSecurityConfig); + setup(Settings.EMPTY, dynamicSecurityConfig, ClusterConfiguration.DEFAULT); } protected final void setup(Settings override, DynamicSecurityConfig dynamicSecurityConfig) throws Exception { + setup(override, dynamicSecurityConfig, ClusterConfiguration.DEFAULT); + } + + protected final void setup(DynamicSecurityConfig dynamicSecurityConfig, ClusterConfiguration clusterConfiguration) throws Exception { + setup(Settings.EMPTY, dynamicSecurityConfig, clusterConfiguration); + } + + protected final void setup(Settings override, DynamicSecurityConfig dynamicSecurityConfig, ClusterConfiguration clusterConfiguration) + throws Exception { Settings settings = Settings.builder().put(ConfigConstants.SECURITY_AUDIT_TYPE_DEFAULT, "debug").put(override).build(); - setup(Settings.EMPTY, dynamicSecurityConfig, settings, true); + setup(Settings.EMPTY, dynamicSecurityConfig, settings, true, clusterConfiguration); try (Client tc = getClient()) { populateData(tc); diff --git a/src/test/java/org/opensearch/security/dlic/dlsfls/RenameFieldResponseProcessorTest.java b/src/test/java/org/opensearch/security/dlic/dlsfls/RenameFieldResponseProcessorTest.java new file mode 100644 index 0000000000..28d9ed27ab --- /dev/null +++ b/src/test/java/org/opensearch/security/dlic/dlsfls/RenameFieldResponseProcessorTest.java @@ -0,0 +1,245 @@ +/* + * 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 org.apache.hc.core5.http.Header; +import org.opensearch.client.Client; + +import org.apache.hc.core5.http.HttpStatus; +import org.junit.Test; + +import org.opensearch.action.index.IndexRequest; +import org.opensearch.security.test.DynamicSecurityConfig; +import org.opensearch.action.support.WriteRequest.RefreshPolicy; +import org.opensearch.common.xcontent.XContentType; +import org.opensearch.security.test.helper.cluster.ClusterConfiguration; +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; + +public class RenameFieldResponseProcessorTest extends AbstractDlsFlsTest { + private final String ROLES_FILE = "roles_flsdls_rename_processor.yml"; + private final String ROLES_MAPPINGS_FILE = "roles_mapping_flsdls_rename_processor.yml"; + private final Header asUserA = encodeBasicHeader("user_aaa", "password"); + private final Header asAdmin = encodeBasicHeader("admin", "admin"); + private final String emptyQuery = "{ \"query\": { \"match_all\": {} } }"; + + protected void populateData(final Client tc) { + // Insert in some dummy flight data + tc.index( + new IndexRequest("flights").id("0") + .setRefreshPolicy(RefreshPolicy.IMMEDIATE) + .source( + "{" + + "\"FlightNum\": \"9HY9SWR\"," + + "\"DestAirportID\": \"SYD\"," + + "\"Dest\": \"Sydney Kingsford Smith International Airport\"," + + "\"DestCountry\": \"AU\"," + + "\"OriginAirportID\": \"FRA\"," + + "\"Origin\": \"Frankfurt am Main Airport\"," + + "\"OriginCountry\": \"DE\"," + + "\"FlightDelay\" : true," + + "\"Canceled\": true" + + "}", + XContentType.JSON + ) + ).actionGet(); + tc.index( + new IndexRequest("flights").id("1") + .setRefreshPolicy(RefreshPolicy.IMMEDIATE) + .source( + "{" + + "\"FlightNum\": \"X98CCZO\"," + + "\"DestAirportID\": \"VE05\"," + + "\"Dest\": \"Venice Marco Polo Airport\"," + + "\"DestCountry\": \"IT\"," + + "\"OriginAirportID\": \"CPT\"," + + "\"Origin\": \"Cape Town International Airport\"," + + "\"OriginCountry\": \"ZA\"," + + "\"FlightDelay\" : false," + + "\"Canceled\": false" + + "}", + XContentType.JSON + ) + ).actionGet(); + tc.index( + new IndexRequest("flights").id("2") + .setRefreshPolicy(RefreshPolicy.IMMEDIATE) + .source( + "{" + + "\"FlightNum\": \"UFK2WIZ\"," + + "\"DestAirportID\": \"SYD\"," + + "\"Dest\": \"Venice Marco Polo Airport\"," + + "\"DestCountry\": \"IT\"," + + "\"OriginAirportID\": \"FRA\"," + + "\"Origin\": \"Venice Marco Polo Airport\"," + + "\"OriginCountry\": \"IT\"," + + "\"FlightDelay\" : false," + + "\"Canceled\": true" + + "}", + XContentType.JSON + ) + ).actionGet(); + } + + @Test + public void testMaskedField() throws Exception { + setup( + new DynamicSecurityConfig().setSecurityRoles(ROLES_FILE).setSecurityRolesMapping(ROLES_MAPPINGS_FILE), + ClusterConfiguration.SEARCH_PIPELINE + ); + + HttpResponse res; + res = rh.executePostRequest("/flights/_search", emptyQuery, asUserA); + assertThat(res.findValueInJson("hits.hits[0]._source.FlightNum"), not(equalTo("9HY9SWR"))); + String testRenameMaskedFieldPipeline = "{" + + "\"description\": \"A pipeline to rename masked field 'FlightNum' to 'FlightNumNew'\"," + + "\"response_processors\": [" + + "{" + + "\"rename_field\": {" + + "\"target_field\": \"FlightNumNew\"," + + "\"field\": \"FlightNum\"" + + "}" + + "}" + + "]" + + "}"; + res = rh.executePutRequest("/_search/pipeline/test-rename-masked-field", testRenameMaskedFieldPipeline, asAdmin); + assertThat(res.getStatusCode(), equalTo(HttpStatus.SC_OK)); + + // Search with this pipeline should succeed and the value of the new field "FlightNumNew" should also be masked + res = rh.executePostRequest("/flights/_search?search_pipeline=test-rename-masked-field&size=1", emptyQuery, asUserA); + assertThat(res.getStatusCode(), equalTo(HttpStatus.SC_OK)); + assertThat(res.findValueInJson("hits.hits[0]._source.FlightNumNew"), not(equalTo("9HY9SWR"))); + } + + @Test + public void testFieldLevelSecurity() throws Exception { + setup( + new DynamicSecurityConfig().setSecurityRoles(ROLES_FILE).setSecurityRolesMapping(ROLES_MAPPINGS_FILE), + ClusterConfiguration.SEARCH_PIPELINE + ); + + HttpResponse res; + String testFieldLevelSecurityPipeline = "{" + + "\"description\": \"A pipeline to rename 'DestCountry' to 'DestCountryNew'\"," + + "\"response_processors\": [" + + "{" + + "\"rename_field\": {" + + "\"target_field\": \"DestCountryNew\"," + + "\"field\": \"DestCountry\"" + + "}" + + "}" + + "]" + + "}"; + res = rh.executePutRequest("/_search/pipeline/test-field-level-security", testFieldLevelSecurityPipeline, asAdmin); + assertThat(res.getStatusCode(), equalTo(HttpStatus.SC_OK)); + + // Search with this pipeline should fail because "DestCountry" is restricted under FLS + res = rh.executePostRequest("/flights/_search?search_pipeline=test-field-level-security&size=1", emptyQuery, asUserA); + assertThat(res.getStatusCode(), equalTo(HttpStatus.SC_BAD_REQUEST)); + } + + @Test + public void testFieldLevelSecurityReverse() throws Exception { + setup( + new DynamicSecurityConfig().setSecurityRoles(ROLES_FILE).setSecurityRolesMapping(ROLES_MAPPINGS_FILE), + ClusterConfiguration.SEARCH_PIPELINE + ); + + HttpResponse res; + String testFieldLevelSecurityReversePipeline = "{" + + "\"description\": \"A pipeline to rename an accessible field name to an inaccessible field name\"," + + "\"response_processors\": [" + + "{" + + "\"rename_field\": {" + + "\"target_field\": \"DestCountry\"," + + "\"field\": \"Dest\"" + + "}" + + "}" + + "]" + + "}"; + res = rh.executePutRequest("/_search/pipeline/test-field-level-security-reverse", testFieldLevelSecurityReversePipeline, asAdmin); + assertThat(res.getStatusCode(), equalTo(HttpStatus.SC_OK)); + + // Search with this pipeline should succeed and the value of the new "DestCountry" field should be the previous value of "Dest" + res = rh.executePostRequest("/flights/_search?search_pipeline=test-field-level-security-reverse&size=1", emptyQuery, asUserA); + assertThat(res.getStatusCode(), equalTo(HttpStatus.SC_OK)); + assertThat(res.findValueInJson("hits.hits[0]._source.DestCountry"), equalTo("Sydney Kingsford Smith International Airport")); + } + + @Test + public void testDocumentLevelSecurity() throws Exception { + setup( + new DynamicSecurityConfig().setSecurityRoles(ROLES_FILE).setSecurityRolesMapping(ROLES_MAPPINGS_FILE), + ClusterConfiguration.SEARCH_PIPELINE + ); + + HttpResponse res; + String testDocumentLevelSecurityPipeline = "{" + + "\"description\": \"A pipeline to rename a DLS restricted field name to a new field name\"," + + "\"response_processors\": [" + + "{" + + "\"rename_field\": {" + + "\"target_field\": \"FlightDelayNew\"," + + "\"field\": \"FlightDelay\"" + + "}" + + "}" + + "]" + + "}"; + res = rh.executePutRequest("/_search/pipeline/test-document-level-security", testDocumentLevelSecurityPipeline, asAdmin); + assertThat(res.getStatusCode(), equalTo(HttpStatus.SC_OK)); + + // Search with this pipeline should succeed and should only return documents where "FlightDelay" is true and the new + // "FlightDelayNew" will also be true + res = rh.executePostRequest("/flights/_search?search_pipeline=test-document-level-security&size=3", emptyQuery, asUserA); + assertThat(res.getStatusCode(), equalTo(HttpStatus.SC_OK)); + assertThat(res.findValueInJson("hits.total.value"), equalTo("1")); + assertThat(res.findValueInJson("hits.hits[0]._source.FlightDelayNew"), equalTo("true")); + } + + @Test + public void testDocumentLevelSecurityReverse() throws Exception { + setup( + new DynamicSecurityConfig().setSecurityRoles(ROLES_FILE).setSecurityRolesMapping(ROLES_MAPPINGS_FILE), + ClusterConfiguration.SEARCH_PIPELINE + ); + + HttpResponse res; + String testDocumentLevelSecurityReversePipeline = "{" + + "\"description\": \"A pipeline to rename an accessible field name to a DLS restricted field name\"," + + "\"response_processors\": [" + + "{" + + "\"rename_field\": {" + + "\"target_field\": \"FlightDelay\"," + + "\"field\": \"Canceled\"" + + "}" + + "}" + + "]" + + "}"; + res = rh.executePutRequest( + "/_search/pipeline/test-document-level-security-reverse", + testDocumentLevelSecurityReversePipeline, + asAdmin + ); + assertThat(res.getStatusCode(), equalTo(HttpStatus.SC_OK)); + + // Search with this pipeline should succeed and should only return documents where "FlightDelay" is true in the original document + // and the new "FlightDelay" + // will also be true + res = rh.executePostRequest("/flights/_search?search_pipeline=test-document-level-security-reverse&size=3", emptyQuery, asUserA); + assertThat(res.getStatusCode(), equalTo(HttpStatus.SC_OK)); + assertThat(res.findValueInJson("hits.total.value"), equalTo("1")); + assertThat(res.findValueInJson("hits.hits[0]._source.FlightDelay"), equalTo("true")); + } +} diff --git a/src/test/java/org/opensearch/security/test/helper/cluster/ClusterConfiguration.java b/src/test/java/org/opensearch/security/test/helper/cluster/ClusterConfiguration.java index 714ab5e42e..1454422c22 100644 --- a/src/test/java/org/opensearch/security/test/helper/cluster/ClusterConfiguration.java +++ b/src/test/java/org/opensearch/security/test/helper/cluster/ClusterConfiguration.java @@ -41,6 +41,7 @@ import org.opensearch.plugins.Plugin; import org.opensearch.script.mustache.MustacheModulePlugin; import org.opensearch.search.aggregations.matrix.MatrixAggregationModulePlugin; +import org.opensearch.search.pipeline.common.SearchPipelineCommonModulePlugin; import org.opensearch.security.OpenSearchSecurityPlugin; import org.opensearch.transport.Netty4ModulePlugin; @@ -79,7 +80,14 @@ public enum ClusterConfiguration { CLIENTNODE(new NodeSettings(true, false), new NodeSettings(false, true), new NodeSettings(false, true), new NodeSettings(false, false)), // 3 nodes (1m, 2d) plus additional UserInjectorPlugin - USERINJECTOR(new NodeSettings(true, false), new NodeSettings(false, true), new NodeSettings(false, true)); + USERINJECTOR(new NodeSettings(true, false), new NodeSettings(false, true), new NodeSettings(false, true)), + + // 3 nodes with search pipelines module installed + SEARCH_PIPELINE( + new NodeSettings(true, false, List.of(SearchPipelineCommonModulePlugin.class)), + new NodeSettings(false, true, List.of(SearchPipelineCommonModulePlugin.class)), + new NodeSettings(false, true, List.of(SearchPipelineCommonModulePlugin.class)) + ); private List nodeSettings = new LinkedList<>(); diff --git a/src/test/resources/dlsfls/roles_flsdls_rename_processor.yml b/src/test/resources/dlsfls/roles_flsdls_rename_processor.yml new file mode 100644 index 0000000000..a0c4c50320 --- /dev/null +++ b/src/test/resources/dlsfls/roles_flsdls_rename_processor.yml @@ -0,0 +1,25 @@ +--- +_meta: + type: "roles" + config_version: 2 +flsdls_test: + cluster_permissions: + - "data/read/search" + - "read" + index_permissions: + - index_patterns: + - "flights" + dls: "{ \"match\": { \"FlightDelay\": true }}" + fls: + - "~DestCountry" + masked_fields: + - "FlightNum" + allowed_actions: + - "data/read/search" + - "read" +search_pipelines: + cluster_permissions: + - "cluster:admin/search/pipeline/put" + - "cluster:monitor/nodes/info" + - "cluster:monitor/nodes/stats" + - "cluster:monitor/state" diff --git a/src/test/resources/dlsfls/roles_mapping_flsdls_rename_processor.yml b/src/test/resources/dlsfls/roles_mapping_flsdls_rename_processor.yml new file mode 100644 index 0000000000..4f41b98b52 --- /dev/null +++ b/src/test/resources/dlsfls/roles_mapping_flsdls_rename_processor.yml @@ -0,0 +1,22 @@ +--- +_meta: + type: "rolesmapping" + config_version: 2 +flsdls_test: + reserved: false + hidden: false + backend_roles: [] + hosts: [] + and_backend_roles: [] + description: "Role for testing RenameFieldResponseProcessor" + users: + - "user_aaa" +search_pipelines: + reserved: false + hidden: false + backend_roles: [] + hosts: [] + and_backend_roles: [] + description: "Adding permissions for search pipelines CRUD operations" + users: + - "admin"