Skip to content

Commit

Permalink
[Backport 2.x] Command cat/indices will filter results per the do n…
Browse files Browse the repository at this point in the history 
…ot fail on forbidden setting (opensearch-project#3258)

### Description
Backport 4c095d2 of opensearch-project#3236 

### Check List
- [ ] 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: Derek Ho <[email protected]>
  • Loading branch information
@derek-ho
derek-ho authored Aug 29, 2023
1 parent 8c3c9c5 commit 8197431
Show file tree
Hide file tree
Showing 3 changed files with 115 additions and 6 deletions.
19 changes: 13 additions & 6 deletions src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@
import java.util.Map;
import java.util.Set;
import java.util.StringJoiner;
import java.util.regex.Pattern;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
Expand Down Expand Up @@ -101,12 +101,19 @@

public class PrivilegesEvaluator {

private static final WildcardMatcher ACTION_MATCHER = WildcardMatcher.from("indices:data/read/*search*");

private static final Pattern DNFOF_PATTERNS = Pattern.compile(
"indices:(data/read/.*|(admin/(mappings/fields/get.*|shards/search_shards|resolve/index)))"
static final WildcardMatcher DNFOF_MATCHER = WildcardMatcher.from(
ImmutableList.of(
"indices:data/read/*",
"indices:admin/mappings/fields/get*",
"indices:admin/shards/search_shards",
"indices:admin/resolve/index",
"indices:monitor/settings/get",
"indices:monitor/stats"
)
);

private static final WildcardMatcher ACTION_MATCHER = WildcardMatcher.from("indices:data/read/*search*");

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

protected final Logger log = LogManager.getLogger(this.getClass());
Expand Down Expand Up @@ -474,7 +481,7 @@ public PrivilegesEvaluatorResponse evaluate(
}
}

if (dnfofEnabled && DNFOF_PATTERNS.matcher(action0).matches()) {
if (dnfofEnabled && DNFOF_MATCHER.test(action0)) {

if (requestedResolved.getAllIndices().isEmpty()) {
presponse.missingPrivileges.clear();
Expand Down
7 changes: 7 additions & 0 deletions src/test/java/org/opensearch/security/IntegrationTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -768,6 +768,13 @@ public void testDnfof() throws Exception {
HttpStatus.SC_OK,
rh.executeGetRequest("_all/_mapping/field/*", encodeBasicHeader("nagilum", "nagilum")).getStatusCode()
);

Assert.assertEquals(
HttpStatus.SC_OK,
(resc = rh.executeGetRequest("_cat/indices", encodeBasicHeader("user_a", "user_a"))).getStatusCode()
);
Assert.assertTrue(resc.getBody(), resc.getBody().contains("indexa"));
Assert.assertFalse(resc.getBody(), resc.getBody().contains("indexb"));
}

@Test
Expand Down
95 changes: 95 additions & 0 deletions src/test/java/org/opensearch/security/privileges/PrivilegesEvaluatorUnitTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,95 @@

package org.opensearch.security.privileges;

import com.google.common.collect.ImmutableList;
import org.junit.Test;

import java.util.List;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.equalTo;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.opensearch.security.privileges.PrivilegesEvaluator.DNFOF_MATCHER;
import static org.opensearch.security.privileges.PrivilegesEvaluator.isClusterPerm;

public class PrivilegesEvaluatorUnitTest {

private static final List<String> allowedDnfof = ImmutableList.of(
"indices:admin/mappings/fields/get",
"indices:admin/resolve/index",
"indices:admin/shards/search_shards",
"indices:data/read/explain",
"indices:data/read/field_caps",
"indices:data/read/get",
"indices:data/read/mget",
"indices:data/read/msearch",
"indices:data/read/msearch/template",
"indices:data/read/mtv",
"indices:data/read/plugins/replication/file_chunk",
"indices:data/read/plugins/replication/changes",
"indices:data/read/scroll",
"indices:data/read/scroll/clear",
"indices:data/read/search",
"indices:data/read/search/template",
"indices:data/read/tv",
"indices:monitor/settings/get",
"indices:monitor/stats"
);

private static final List<String> disallowedDnfof = ImmutableList.of(
"indices:admin/aliases",
"indices:admin/aliases/exists",
"indices:admin/aliases/get",
"indices:admin/analyze",
"indices:admin/cache/clear",
"indices:admin/close",
"indices:admin/create",
"indices:admin/data_stream/create",
"indices:admin/data_stream/delete",
"indices:admin/data_stream/get",
"indices:admin/delete",
"indices:admin/exists",
"indices:admin/flush",
"indices:admin/forcemerge",
"indices:admin/get",
"indices:admin/mapping/put",
"indices:admin/mappings/get",
"indices:admin/open",
"indices:admin/plugins/replication/index/setup/validate",
"indices:admin/plugins/replication/index/start",
"indices:admin/plugins/replication/index/pause",
"indices:admin/plugins/replication/index/resume",
"indices:admin/plugins/replication/index/stop",
"indices:admin/plugins/replication/index/update",
"indices:admin/plugins/replication/index/status_check",
"indices:admin/refresh",
"indices:admin/rollover",
"indices:admin/seq_no/global_checkpoint_sync",
"indices:admin/settings/update",
"indices:admin/shrink",
"indices:admin/synced_flush",
"indices:admin/template/delete",
"indices:admin/template/get",
"indices:admin/template/put",
"indices:admin/types/exists",
"indices:admin/upgrade",
"indices:admin/validate/query",
"indices:data/write/bulk",
"indices:data/write/delete",
"indices:data/write/delete/byquery",
"indices:data/write/plugins/replication/changes",
"indices:data/write/index",
"indices:data/write/reindex",
"indices:data/write/update",
"indices:data/write/update/byquery",
"indices:monitor/data_stream/stats",
"indices:monitor/recovery",
"indices:monitor/segments",
"indices:monitor/shard_stores",
"indices:monitor/upgrade"
);

@Test
public void testClusterPerm() {
String multiSearchTemplate = "indices:data/read/msearch/template";
Expand All @@ -33,4 +114,18 @@ public void testClusterPerm() {
assertFalse(isClusterPerm(adminClose));
assertFalse(isClusterPerm(monitorUpgrade));
}

@Test
public void testDnfofPermissions_negative() {
for (final String permission : disallowedDnfof) {
assertThat(DNFOF_MATCHER.test(permission), equalTo(false));
}
}

@Test
public void testDnfofPermissions_positive() {
for (final String permission : allowedDnfof) {
assertThat(DNFOF_MATCHER.test(permission), equalTo(true));
}
}
}

0 comments on commit 8197431

Please sign in to comment.