diff --git a/src/integrationTest/java/org/opensearch/security/DoNotFailOnForbiddenTests.java b/src/integrationTest/java/org/opensearch/security/DoNotFailOnForbiddenTests.java index fedcf0bbcb..138a57e60b 100644 --- a/src/integrationTest/java/org/opensearch/security/DoNotFailOnForbiddenTests.java +++ b/src/integrationTest/java/org/opensearch/security/DoNotFailOnForbiddenTests.java @@ -9,7 +9,11 @@ */ package org.opensearch.security; +import java.io.BufferedReader; import java.io.IOException; +import java.io.InputStreamReader; +import java.util.List; +import java.util.stream.Collectors; import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; import org.hamcrest.Matchers; @@ -31,6 +35,8 @@ import org.opensearch.action.search.SearchResponse; import org.opensearch.action.search.SearchScrollRequest; import org.opensearch.client.Client; +import org.opensearch.client.Request; +import org.opensearch.client.Response; import org.opensearch.client.RestHighLevelClient; import org.opensearch.test.framework.TestSecurityConfig; import org.opensearch.test.framework.TestSecurityConfig.User; @@ -38,12 +44,7 @@ import org.opensearch.test.framework.cluster.LocalCluster; import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.aMapWithSize; -import static org.hamcrest.Matchers.allOf; -import static org.hamcrest.Matchers.arrayContainingInAnyOrder; -import static org.hamcrest.Matchers.arrayWithSize; -import static org.hamcrest.Matchers.hasKey; -import static org.hamcrest.Matchers.nullValue; +import static org.hamcrest.Matchers.*; import static org.opensearch.action.admin.indices.alias.IndicesAliasesRequest.AliasActions.Type.ADD; import static org.opensearch.action.support.WriteRequest.RefreshPolicy.IMMEDIATE; import static org.opensearch.client.RequestOptions.DEFAULT; @@ -102,7 +103,9 @@ public class DoNotFailOnForbiddenTests { new TestSecurityConfig.Role("limited-role").clusterPermissions( "indices:data/read/mget", "indices:data/read/msearch", - "indices:data/read/scroll" + "indices:data/read/scroll", + "cluster:monitor/state", + "cluster:monitor/health" ) .indexPermissions( "indices:data/read/search", @@ -110,7 +113,9 @@ public class DoNotFailOnForbiddenTests { "indices:data/read/field_caps", "indices:data/read/field_caps*", "indices:data/read/msearch", - "indices:data/read/scroll" + "indices:data/read/scroll", + "indices:monitor/settings/get", + "indices:monitor/stats" ) .on(MARVELOUS_SONGS) ); @@ -408,4 +413,18 @@ public void shouldPerformStatAggregation_negative() throws IOException { } } + @Test + public void shouldPerformCatIndices_positive() throws IOException { + try (RestHighLevelClient restHighLevelClient = cluster.getRestHighLevelClient(LIMITED_USER)) { + Request getIndicesRequest = new Request("GET", "/_cat/indices"); + // High level client doesn't support _cat/_indices API + Response getIndicesResponse = restHighLevelClient.getLowLevelClient().performRequest(getIndicesRequest); + List indexes = new BufferedReader(new InputStreamReader(getIndicesResponse.getEntity().getContent())).lines() + .collect(Collectors.toList()); + + assertThat(indexes.size(), equalTo(1)); + assertThat(indexes.get(0), containsString("marvelous_songs")); + } + } + } diff --git a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java index 3ac120b35a..83dba986ee 100644 --- a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java +++ b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java @@ -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; @@ -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()); @@ -473,7 +480,7 @@ public PrivilegesEvaluatorResponse evaluate( } } - if (dnfofEnabled && DNFOF_PATTERNS.matcher(action0).matches()) { + if (dnfofEnabled && DNFOF_MATCHER.test(action0)) { if (requestedResolved.getAllIndices().isEmpty()) { presponse.missingPrivileges.clear(); diff --git a/src/test/java/org/opensearch/security/privileges/PrivilegesEvaluatorUnitTest.java b/src/test/java/org/opensearch/security/privileges/PrivilegesEvaluatorUnitTest.java index e7412f43b4..1b1874a106 100644 --- a/src/test/java/org/opensearch/security/privileges/PrivilegesEvaluatorUnitTest.java +++ b/src/test/java/org/opensearch/security/privileges/PrivilegesEvaluatorUnitTest.java @@ -8,14 +8,94 @@ 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.isClusterPerm; +import static org.opensearch.security.privileges.PrivilegesEvaluator.*; public class PrivilegesEvaluatorUnitTest { + private static final List 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 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"; @@ -33,4 +113,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)); + } + } }