Skip to content

Commit

Permalink
code changes
Browse files Browse the repository at this point in the history
add resolve to dnfof logic

unify operation flows where possible

add dnfof pattern

add test cases for resolve
  • Loading branch information
hsiang9431-amzn committed Aug 6, 2021
1 parent 960d420 commit eb97a93
Show file tree
Hide file tree
Showing 6 changed files with 204 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,13 @@
import java.util.Map;
import java.util.Set;
import java.util.StringJoiner;
import java.util.regex.Pattern;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.opensearch.OpenSearchSecurityException;
import org.opensearch.action.ActionRequest;
import org.opensearch.action.IndicesRequest;
import org.opensearch.action.admin.cluster.shards.ClusterSearchShardsRequest;
import org.opensearch.action.admin.cluster.snapshots.restore.RestoreSnapshotRequest;
import org.opensearch.action.admin.indices.alias.IndicesAliasesAction;
Expand Down Expand Up @@ -128,6 +130,12 @@ public class PrivilegesEvaluator {
private final boolean dlsFlsEnabled;
private DynamicConfigModel dcm;

private static final Pattern dnfofPatterns = Pattern.compile(
"indices:(data/read/.*|(admin/(mappings/fields/get.*|shards/search_shards|resolve/index)))"
);

private static final IndicesOptions idxOptAllowEmpty = IndicesOptions.fromOptions(true, true, false, false);

public PrivilegesEvaluator(final ClusterService clusterService, final ThreadPool threadPool,
final ConfigurationRepository configurationRepository, final IndexNameExpressionResolver resolver,
AuditLog auditLog, final Settings settings, final PrivilegesInterceptor privilegesInterceptor, final ClusterInfoHolder clusterInfoHolder,
Expand Down Expand Up @@ -386,45 +394,32 @@ public PrivilegesEvaluatorResponse evaluate(final User user, String action0, fin
}
}

if (dnfofEnabled
&& (action0.startsWith("indices:data/read/")
|| action0.startsWith("indices:admin/mappings/fields/get")
|| action0.equals("indices:admin/shards/search_shards"))) {
if (dnfofEnabled && dnfofPatterns.matcher(action0).matches()) {

if(requestedResolved.getAllIndices().isEmpty()) {
presponse.missingPrivileges.clear();
presponse.allowed = true;
return presponse;
}


Set<String> reduced = securityRoles.reduce(requestedResolved, user, allIndexPermsRequiredA, resolver, clusterService);

if(reduced.isEmpty()) {
if(dcm.isDnfofForEmptyResultsEnabled()) {
if(request instanceof SearchRequest) {
((SearchRequest) request).indices(new String[0]);
((SearchRequest) request).indicesOptions(IndicesOptions.fromOptions(true, true, false, false));
presponse.missingPrivileges.clear();
presponse.allowed = true;
return presponse;
}
if(dcm.isDnfofForEmptyResultsEnabled() && request instanceof IndicesRequest.Replaceable) {

if(request instanceof ClusterSearchShardsRequest) {
((ClusterSearchShardsRequest) request).indices(new String[0]);
((ClusterSearchShardsRequest) request).indicesOptions(IndicesOptions.fromOptions(true, true, false, false));
presponse.missingPrivileges.clear();
presponse.allowed = true;
return presponse;
}
((IndicesRequest.Replaceable) request).indices(new String[0]);
presponse.missingPrivileges.clear();
presponse.allowed = true;

if(request instanceof GetFieldMappingsRequest) {
((GetFieldMappingsRequest) request).indices(new String[0]);
((GetFieldMappingsRequest) request).indicesOptions(IndicesOptions.fromOptions(true, true, false, false));
presponse.missingPrivileges.clear();
presponse.allowed = true;
return presponse;
if(request instanceof SearchRequest) {
((SearchRequest) request).indicesOptions(idxOptAllowEmpty);
} else if(request instanceof ClusterSearchShardsRequest) {
((ClusterSearchShardsRequest) request).indicesOptions(idxOptAllowEmpty);
} else if(request instanceof GetFieldMappingsRequest) {
((GetFieldMappingsRequest) request).indicesOptions(idxOptAllowEmpty);
}

return presponse;
}
presponse.allowed = false;
return presponse;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import java.util.stream.Collectors;

import org.opensearch.action.admin.indices.datastream.CreateDataStreamAction;
import org.opensearch.action.admin.indices.resolve.ResolveIndexAction;
import org.opensearch.security.OpenSearchSecurityPlugin;
import org.apache.commons.collections.keyvalue.MultiKey;
import org.apache.logging.log4j.LogManager;
Expand Down Expand Up @@ -87,6 +88,7 @@
import org.opensearch.index.reindex.ReindexRequest;
import org.opensearch.security.configuration.ClusterInfoHolder;
import org.opensearch.security.securityconf.DynamicConfigModel;
import org.opensearch.security.support.ConfigConstants;
import org.opensearch.snapshots.SnapshotInfo;
import org.opensearch.snapshots.SnapshotUtils;
import org.opensearch.transport.RemoteClusterService;
Expand Down Expand Up @@ -294,7 +296,9 @@ private void resolveTo(Iterable<String> matchingAliases, Iterable<String> matchi
@Override
public String[] provide(String[] original, Object localRequest, boolean supportsReplace) {
final IndicesOptions indicesOptions = indicesOptionsFrom(localRequest);
final boolean enableCrossClusterResolution = localRequest instanceof FieldCapabilitiesRequest || localRequest instanceof SearchRequest;
final boolean enableCrossClusterResolution = localRequest instanceof FieldCapabilitiesRequest
|| localRequest instanceof SearchRequest
|| localRequest instanceof ResolveIndexAction.Request;
// skip the whole thing if we have seen this exact resolveIndexPatterns request
if (alreadyResolved.add(new MultiKey(indicesOptions, enableCrossClusterResolution,
(original != null) ? new MultiKey(original, false) : null))) {
Expand Down
3 changes: 3 additions & 0 deletions src/main/resources/static_config/static_action_groups.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ search:
allowed_actions:
- "indices:data/read/search*"
- "indices:data/read/msearch*"
- "indices:admin/resolve/index"
- "suggest"
type: "index"
description: "Allow searching"
Expand Down Expand Up @@ -87,6 +88,7 @@ read:
allowed_actions:
- "indices:data/read*"
- "indices:admin/mappings/fields/get*"
- "indices:admin/resolve/index"
type: "index"
description: "Allow all read operations"
indices_all:
Expand Down Expand Up @@ -127,6 +129,7 @@ cluster_composite_ops_ro:
- "indices:admin/aliases/exists*"
- "indices:admin/aliases/get*"
- "indices:data/read/scroll"
- "indices:admin/resolve/index"
type: "cluster"
description: "Allow readonly bulk and m* operations"
get:
Expand Down
172 changes: 172 additions & 0 deletions src/test/java/org/opensearch/security/ResolveAPITests.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,172 @@
/*
* Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License").
* You may not use this file except in compliance with the License.
* A copy of the License is located at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* or in the "license" file accompanying this file. This file is distributed
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
* express or implied. See the License for the specific language governing
* permissions and limitations under the License.
*/

package org.opensearch.security;

import org.apache.http.HttpStatus;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.junit.Assert;
import org.junit.Test;
import org.opensearch.action.admin.indices.alias.IndicesAliasesRequest;
import org.opensearch.action.admin.indices.create.CreateIndexRequest;
import org.opensearch.action.index.IndexRequest;
import org.opensearch.action.support.WriteRequest;
import org.opensearch.client.transport.TransportClient;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.xcontent.XContentType;
import org.opensearch.security.support.ConfigConstants;
import org.opensearch.security.test.DynamicSecurityConfig;
import org.opensearch.security.test.SingleClusterTest;
import org.opensearch.security.test.helper.rest.RestHelper;

import java.util.Arrays;
import java.util.Collections;

public class ResolveAPITests extends SingleClusterTest {

protected final Logger log = LogManager.getLogger(this.getClass());

@Test
public void testResolveDnfofFalse() throws Exception {

Settings settings = Settings.builder().build();

setup(settings);
setupIndices();

final RestHelper rh = nonSslRestHelper();
RestHelper.HttpResponse res;

Assert.assertEquals(HttpStatus.SC_OK, (res = rh.executeGetRequest("_resolve/index/*?pretty", encodeBasicHeader("nagilum", "nagilum"))).getStatusCode());
log.debug(res.getBody());
assertNotContains(res, "*xception*");
assertNotContains(res, "*erial*");
assertNotContains(res, "*mpty*");
assertContains(res, "*vulcangov*");
assertContains(res, "*starfleet*");
assertContains(res, "*klingonempire*");
assertContains(res, "*xyz*");
assertContains(res, "*role01_role02*");

Assert.assertEquals(HttpStatus.SC_OK, (res = rh.executeGetRequest("_resolve/index/starfleet*?pretty", encodeBasicHeader("nagilum", "nagilum"))).getStatusCode());
log.debug(res.getBody());
assertNotContains(res, "*xception*");
assertNotContains(res, "*erial*");
assertNotContains(res, "*mpty*");
assertNotContains(res, "*vulcangov*");
assertNotContains(res, "*klingonempire*");
assertNotContains(res, "*xyz*");
assertNotContains(res, "*role01_role02*");
assertContains(res, "*starfleet*");
assertContains(res, "*starfleet_academy*");
assertContains(res, "*starfleet_library*");

Assert.assertEquals(HttpStatus.SC_FORBIDDEN, (res = rh.executeGetRequest("_resolve/index/*?pretty", encodeBasicHeader("worf", "worf"))).getStatusCode());
log.debug(res.getBody());

Assert.assertEquals(HttpStatus.SC_OK, (res = rh.executeGetRequest("_resolve/index/starfleet*?pretty", encodeBasicHeader("worf", "worf"))).getStatusCode());
log.debug(res.getBody());
assertContains(res, "*starfleet*");
assertContains(res, "*starfleet_academy*");
assertContains(res, "*starfleet_library*");
}

@Test
public void testResolveDnfofTrue() throws Exception {
final Settings settings = Settings.builder().build();

setup(Settings.EMPTY, new DynamicSecurityConfig().setConfig("config_dnfof.yml"), settings);
setupIndices();

final RestHelper rh = nonSslRestHelper();
RestHelper.HttpResponse res;

Assert.assertEquals(HttpStatus.SC_OK, (res = rh.executeGetRequest("_resolve/index/*?pretty", encodeBasicHeader("nagilum", "nagilum"))).getStatusCode());
log.debug(res.getBody());
assertNotContains(res, "*xception*");
assertNotContains(res, "*erial*");
assertNotContains(res, "*mpty*");
assertContains(res, "*vulcangov*");
assertContains(res, "*starfleet*");
assertContains(res, "*klingonempire*");
assertContains(res, "*xyz*");
assertContains(res, "*role01_role02*");

Assert.assertEquals(HttpStatus.SC_OK, (res = rh.executeGetRequest("_resolve/index/starfleet*?pretty", encodeBasicHeader("nagilum", "nagilum"))).getStatusCode());
log.debug(res.getBody());
assertNotContains(res, "*xception*");
assertNotContains(res, "*erial*");
assertNotContains(res, "*mpty*");
assertNotContains(res, "*vulcangov*");
assertNotContains(res, "*klingonempire*");
assertNotContains(res, "*xyz*");
assertNotContains(res, "*role01_role02*");
assertContains(res, "*starfleet*");
assertContains(res, "*starfleet_academy*");
assertContains(res, "*starfleet_library*");

Assert.assertEquals(HttpStatus.SC_OK, (res = rh.executeGetRequest("_resolve/index/*?pretty", encodeBasicHeader("worf", "worf"))).getStatusCode());
log.debug(res.getBody());
assertNotContains(res, "*xception*");
assertNotContains(res, "*erial*");
assertNotContains(res, "*mpty*");
assertNotContains(res, "*vulcangov*");
assertNotContains(res, "*kirk*");
assertContains(res, "*starfleet*");
assertContains(res, "*public*");
assertContains(res, "*xyz*");

Assert.assertEquals(HttpStatus.SC_OK, (res = rh.executeGetRequest("_resolve/index/starfleet*?pretty", encodeBasicHeader("worf", "worf"))).getStatusCode());
log.debug(res.getBody());
assertNotContains(res, "*xception*");
assertNotContains(res, "*erial*");
assertNotContains(res, "*mpty*");
assertNotContains(res, "*vulcangov*");
assertNotContains(res, "*kirk*");
assertNotContains(res, "*public*");
assertNotContains(res, "*xyz*");
assertContains(res, "*starfleet*");
assertContains(res, "*starfleet_academy*");
assertContains(res, "*starfleet_library*");

Assert.assertEquals(HttpStatus.SC_FORBIDDEN, (res = rh.executeGetRequest("_resolve/index/vulcangov*?pretty", encodeBasicHeader("worf", "worf"))).getStatusCode());
log.debug(res.getBody());
assertNotContains(res, "*vulcangov*");
}

private void setupIndices() {
try (TransportClient tc = getInternalTransportClient()) {
tc.admin().indices().create(new CreateIndexRequest("copysf")).actionGet();
tc.index(new IndexRequest("vulcangov").type("kolinahr").setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE).source("{\"content\":1}", XContentType.JSON)).actionGet();
tc.index(new IndexRequest("starfleet").type("ships").setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE).source("{\"content\":1}", XContentType.JSON)).actionGet();
tc.index(new IndexRequest("starfleet_academy").type("students").setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE).source("{\"content\":1}", XContentType.JSON)).actionGet();
tc.index(new IndexRequest("starfleet_library").type("public").setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE).source("{\"content\":1}", XContentType.JSON)).actionGet();
tc.index(new IndexRequest("klingonempire").type("ships").setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE).source("{\"content\":1}", XContentType.JSON)).actionGet();
tc.index(new IndexRequest("public").type("legends").setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE).source("{\"content\":1}", XContentType.JSON)).actionGet();

tc.index(new IndexRequest("spock").type("type01").setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE).source("{\"content\":1}", XContentType.JSON)).actionGet();
tc.index(new IndexRequest("kirk").type("type01").setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE).source("{\"content\":1}", XContentType.JSON)).actionGet();
tc.index(new IndexRequest("role01_role02").type("type01").setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE).source("{\"content\":1}", XContentType.JSON)).actionGet();

tc.index(new IndexRequest("xyz").type("doc").setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE).source("{\"content\":1}", XContentType.JSON)).actionGet();

tc.admin().indices().aliases(new IndicesAliasesRequest().addAliasAction(IndicesAliasesRequest.AliasActions.add().indices("starfleet","starfleet_academy","starfleet_library").alias("sf"))).actionGet();
tc.admin().indices().aliases(new IndicesAliasesRequest().addAliasAction(IndicesAliasesRequest.AliasActions.add().indices("klingonempire","vulcangov").alias("nonsf"))).actionGet();
tc.admin().indices().aliases(new IndicesAliasesRequest().addAliasAction(IndicesAliasesRequest.AliasActions.add().indices("public").alias("unrestricted"))).actionGet();
tc.admin().indices().aliases(new IndicesAliasesRequest().addAliasAction(IndicesAliasesRequest.AliasActions.add().indices("xyz").alias("alias1"))).actionGet();
}
}
}
2 changes: 2 additions & 0 deletions src/test/resources/action_groups.yml
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ OPENDISTRO_SECURITY_READ:
hidden: false
allowed_actions:
- "indices:data/read*"
- "indices:admin/resolve/index"
type: "index"
description: "Migrated from v6"
OPENDISTRO_SECURITY_DELETE:
Expand Down Expand Up @@ -113,6 +114,7 @@ OPENDISTRO_SECURITY_GET:
allowed_actions:
- "indices:data/read/get*"
- "indices:data/read/mget*"
- "indices:admin/resolve/index"
type: "index"
description: "Migrated from v6"
OPENDISTRO_SECURITY_MANAGE:
Expand Down
1 change: 1 addition & 0 deletions src/test/resources/action_groups_packaged.yml
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ OPENDISTRO_SECURITY_READ:
allowed_actions:
- "indices:data/read*"
- "indices:admin/mappings/fields/get*"
- "indices:admin/resolve/index"
type: "index"
description: "Migrated from v6"
OPENDISTRO_SECURITY_INDICES_ALL:
Expand Down

0 comments on commit eb97a93

Please sign in to comment.