From 94c756fde0443a04cfbd98e81b6957aab4600404 Mon Sep 17 00:00:00 2001 From: Nils Bandener Date: Fri, 19 Jul 2024 08:49:40 +0200 Subject: [PATCH 1/2] Fixed READ_ACTIONS required by TermsAggregationEvaluator The READ_ACTIONS array is passed by TermsAggregationEvaluator to securityRoles.getAllPermittedIndicesForDashboards() which checks whether privileges are available for all actions specified in READ_ACTIONS. READ_ACTIONS also contained the string "indices:data/read/field_caps*", which is actually wrong, because it is not an action but looks like pattern. However, the code behind securityRoles.getAllPermittedIndicesForDashboards() will never treat these strings as patterns. The "*" is only considered a normal, bare character. Patterns (via WildcardMatcher class) will be only applied to these strings. This had the effect that a bare privilege "indices:data/read/field_caps" was not sufficient to fulfill the requirement. It was necessary to have either "indices:data/read/field_caps*" in ones roles.yml or something broader like "indices:data/read/*". The latter is the most likely case, which is the reason why in most cases this gets unnoticed. Signed-off-by: Nils Bandener --- .../security/privileges/TermsAggregationEvaluator.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/main/java/org/opensearch/security/privileges/TermsAggregationEvaluator.java b/src/main/java/org/opensearch/security/privileges/TermsAggregationEvaluator.java index d06a45726a..f15ca0ebf9 100644 --- a/src/main/java/org/opensearch/security/privileges/TermsAggregationEvaluator.java +++ b/src/main/java/org/opensearch/security/privileges/TermsAggregationEvaluator.java @@ -53,9 +53,7 @@ public class TermsAggregationEvaluator { "indices:data/read/mget", "indices:data/read/get", "indices:data/read/search", - "indices:data/read/field_caps*" - // "indices:admin/mappings/fields/get*" - }; + "indices:data/read/field_caps" }; private static final QueryBuilder NONE_QUERY = new MatchNoneQueryBuilder(); From 57bd4f354dfd8e1db4ab197d5f3c6405bc021bd7 Mon Sep 17 00:00:00 2001 From: Nils Bandener Date: Tue, 23 Jul 2024 20:21:02 +0200 Subject: [PATCH 2/2] Replaced hard-coded action names by constants from core Signed-off-by: Nils Bandener --- .../privileges/TermsAggregationEvaluator.java | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/opensearch/security/privileges/TermsAggregationEvaluator.java b/src/main/java/org/opensearch/security/privileges/TermsAggregationEvaluator.java index f15ca0ebf9..cc0bf25b5e 100644 --- a/src/main/java/org/opensearch/security/privileges/TermsAggregationEvaluator.java +++ b/src/main/java/org/opensearch/security/privileges/TermsAggregationEvaluator.java @@ -32,6 +32,11 @@ import org.apache.logging.log4j.Logger; import org.opensearch.action.ActionRequest; +import org.opensearch.action.fieldcaps.FieldCapabilitiesAction; +import org.opensearch.action.get.GetAction; +import org.opensearch.action.get.MultiGetAction; +import org.opensearch.action.search.MultiSearchAction; +import org.opensearch.action.search.SearchAction; import org.opensearch.action.search.SearchRequest; import org.opensearch.cluster.metadata.IndexNameExpressionResolver; import org.opensearch.cluster.service.ClusterService; @@ -49,11 +54,11 @@ public class TermsAggregationEvaluator { protected final Logger log = LogManager.getLogger(this.getClass()); private static final String[] READ_ACTIONS = new String[] { - "indices:data/read/msearch", - "indices:data/read/mget", - "indices:data/read/get", - "indices:data/read/search", - "indices:data/read/field_caps" }; + MultiSearchAction.NAME, + MultiGetAction.NAME, + GetAction.NAME, + SearchAction.NAME, + FieldCapabilitiesAction.NAME }; private static final QueryBuilder NONE_QUERY = new MatchNoneQueryBuilder();