Skip to content

Commit

Permalink
Add RenameFieldResponseProcessor FLS/DLS tests (opensearch-project#3562)
Browse files Browse the repository at this point in the history
### 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
opensearch-project#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 <[email protected]>
  • Loading branch information
sejli authored Oct 19, 2023
1 parent 40588e6 commit d9643a2
Show file tree
Hide file tree
Showing 6 changed files with 315 additions and 4 deletions.
1 change: 1 addition & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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> nodeSettings = new LinkedList<>();

Expand Down
25 changes: 25 additions & 0 deletions src/test/resources/dlsfls/roles_flsdls_rename_processor.yml
Original file line number Diff line number Diff line change
@@ -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"
Original file line number Diff line number Diff line change
@@ -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"

0 comments on commit d9643a2

Please sign in to comment.