change the backend role filtering to keep consistent with alerting pl…

…ugin (#383)
opendistro-for-elasticsearch · Feb 24, 2021 · 87fe5bb · 87fe5bb
1 parent 515c176
commit 87fe5bb
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 37 deletions.
diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/ad/util/ParseUtils.java b/src/main/java/com/amazon/opendistroforelasticsearch/ad/util/ParseUtils.java
@@ -47,7 +47,6 @@
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.common.xcontent.XContentType;
 import org.elasticsearch.index.query.BoolQueryBuilder;
-import org.elasticsearch.index.query.ExistsQueryBuilder;
 import org.elasticsearch.index.query.NestedQueryBuilder;
 import org.elasticsearch.index.query.QueryBuilder;
 import org.elasticsearch.index.query.QueryBuilders;
@@ -70,6 +69,7 @@
 import com.amazon.opendistroforelasticsearch.ad.transport.GetAnomalyDetectorResponse;
 import com.amazon.opendistroforelasticsearch.commons.ConfigConstants;
 import com.amazon.opendistroforelasticsearch.commons.authuser.User;
+import com.google.common.collect.ImmutableList;
 
 /**
  * Parsing utility functions.
@@ -416,31 +416,17 @@ public static List<FeatureData> getFeatureData(double[] currentFeature, AnomalyD
     }
 
     public static SearchSourceBuilder addUserBackendRolesFilter(User user, SearchSourceBuilder searchSourceBuilder) {
+        if (user == null) {
+            return searchSourceBuilder;
+        }
         BoolQueryBuilder boolQueryBuilder = new BoolQueryBuilder();
         String userFieldName = "user";
         String userBackendRoleFieldName = "user.backend_roles.keyword";
-        if (user == null) {
-            // For old monitor and detector, they have no user field, user = null
-            ExistsQueryBuilder userRolesFilterQuery = QueryBuilders.existsQuery(userFieldName);
-            NestedQueryBuilder nestedQueryBuilder = new NestedQueryBuilder(userFieldName, userRolesFilterQuery, ScoreMode.None);
-            boolQueryBuilder.mustNot(nestedQueryBuilder);
-        } else if (user.getBackendRoles() == null || user.getBackendRoles().size() == 0) {
-            // For simple FGAC user, they may have no backend roles, these users should be able to see detectors
-            // of other users whose backend role is empty. user != null, user.backend_role == null
-            ExistsQueryBuilder userRolesFilterQuery = QueryBuilders.existsQuery(userBackendRoleFieldName);
-            NestedQueryBuilder nestedQueryBuilder = new NestedQueryBuilder(userFieldName, userRolesFilterQuery, ScoreMode.None);
-
-            ExistsQueryBuilder userExistsQuery = QueryBuilders.existsQuery(userFieldName);
-            NestedQueryBuilder userExistsNestedQueryBuilder = new NestedQueryBuilder(userFieldName, userExistsQuery, ScoreMode.None);
-
-            boolQueryBuilder.mustNot(nestedQueryBuilder);
-            boolQueryBuilder.must(userExistsNestedQueryBuilder);
-        } else {
-            // For normal case, user should have backend roles.
-            TermsQueryBuilder userRolesFilterQuery = QueryBuilders.termsQuery(userBackendRoleFieldName, user.getBackendRoles());
-            NestedQueryBuilder nestedQueryBuilder = new NestedQueryBuilder(userFieldName, userRolesFilterQuery, ScoreMode.None);
-            boolQueryBuilder.must(nestedQueryBuilder);
-        }
+        List<String> backendRoles = user.getBackendRoles() != null ? user.getBackendRoles() : ImmutableList.of();
+        // For normal case, user should have backend roles.
+        TermsQueryBuilder userRolesFilterQuery = QueryBuilders.termsQuery(userBackendRoleFieldName, backendRoles);
+        NestedQueryBuilder nestedQueryBuilder = new NestedQueryBuilder(userFieldName, userRolesFilterQuery, ScoreMode.None);
+        boolQueryBuilder.must(nestedQueryBuilder);
         QueryBuilder query = searchSourceBuilder.query();
         if (query == null) {
             searchSourceBuilder.query(boolQueryBuilder);

diff --git a/src/test/java/com/amazon/opendistroforelasticsearch/ad/util/ParseUtilsTests.java b/src/test/java/com/amazon/opendistroforelasticsearch/ad/util/ParseUtilsTests.java
@@ -114,12 +114,7 @@ public void testGenerateInternalFeatureQueryTemplate() throws IOException {
     public void testAddUserRoleFilterWithNullUser() {
         SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder();
         addUserBackendRolesFilter(null, searchSourceBuilder);
-        assertEquals(
-            "{\"query\":{\"bool\":{\"must_not\":[{\"nested\":{\"query\":{\"exists\":{\"field\":\"user\",\"boost\":1.0}},"
-                + "\"path\":\"user\",\"ignore_unmapped\":false,\"score_mode\":\"none\",\"boost\":1.0}}],\"adjust_pure_negative\":true,"
-                + "\"boost\":1.0}}}",
-            searchSourceBuilder.toString()
-        );
+        assertEquals("{}", searchSourceBuilder.toString());
     }
 
     public void testAddUserRoleFilterWithNullUserBackendRole() {
@@ -129,10 +124,9 @@ public void testAddUserRoleFilterWithNullUserBackendRole() {
             searchSourceBuilder
         );
         assertEquals(
-            "{\"query\":{\"bool\":{\"must\":[{\"nested\":{\"query\":{\"exists\":{\"field\":\"user\",\"boost\":1.0}},"
-                + "\"path\":\"user\",\"ignore_unmapped\":false,\"score_mode\":\"none\",\"boost\":1.0}}],\"must_not\":[{\"nested\":"
-                + "{\"query\":{\"exists\":{\"field\":\"user.backend_roles.keyword\",\"boost\":1.0}},\"path\":\"user\",\"ignore_unmapped\""
-                + ":false,\"score_mode\":\"none\",\"boost\":1.0}}],\"adjust_pure_negative\":true,\"boost\":1.0}}}",
+            "{\"query\":{\"bool\":{\"must\":[{\"nested\":{\"query\":{\"terms\":{\"user.backend_roles.keyword\":[],"
+                + "\"boost\":1.0}},\"path\":\"user\",\"ignore_unmapped\":false,\"score_mode\":\"none\",\"boost\":1.0}}],"
+                + "\"adjust_pure_negative\":true,\"boost\":1.0}}}",
             searchSourceBuilder.toString()
         );
     }
@@ -149,10 +143,9 @@ public void testAddUserRoleFilterWithEmptyUserBackendRole() {
             searchSourceBuilder
         );
         assertEquals(
-            "{\"query\":{\"bool\":{\"must\":[{\"nested\":{\"query\":{\"exists\":{\"field\":\"user\",\"boost\":1.0}},"
-                + "\"path\":\"user\",\"ignore_unmapped\":false,\"score_mode\":\"none\",\"boost\":1.0}}],\"must_not\":[{\"nested\":"
-                + "{\"query\":{\"exists\":{\"field\":\"user.backend_roles.keyword\",\"boost\":1.0}},\"path\":\"user\",\"ignore_unmapped\""
-                + ":false,\"score_mode\":\"none\",\"boost\":1.0}}],\"adjust_pure_negative\":true,\"boost\":1.0}}}",
+            "{\"query\":{\"bool\":{\"must\":[{\"nested\":{\"query\":{\"terms\":{\"user.backend_roles.keyword\":[],"
+                + "\"boost\":1.0}},\"path\":\"user\",\"ignore_unmapped\":false,\"score_mode\":\"none\",\"boost\":1.0}}],"
+                + "\"adjust_pure_negative\":true,\"boost\":1.0}}}",
             searchSourceBuilder.toString()
         );
     }