From 9a5ce15f3a6e9902e6394f5e31419c4149fd8d53 Mon Sep 17 00:00:00 2001 From: Sriharsha Chintalapani Date: Fri, 13 Sep 2024 13:37:27 -0700 Subject: [PATCH] Add OpenSearch query builders and tests --- .../queries/ElasticQueryBuilder.java | 7 +- .../queries/OpenSearchQueryBuilder.java | 151 +++++++++++++ .../OpenSearchQueryBuilderFactory.java | 38 ++++ .../search/queries/OMQueryBuilder.java | 2 - .../search/security/ConditionCollector.java | 26 --- .../security/RBACConditionEvaluator.java | 16 +- ...sticSearchRBACConditionEvaluatorTest.java} | 54 +++-- .../OpenSearchRBACConditionEvaluatorTest.java | 199 ++++++++++++++++++ .../openmetadata/service/util/TestUtils.java | 19 ++ 9 files changed, 449 insertions(+), 63 deletions(-) create mode 100644 openmetadata-service/src/main/java/org/openmetadata/service/search/opensearch/queries/OpenSearchQueryBuilder.java create mode 100644 openmetadata-service/src/main/java/org/openmetadata/service/search/opensearch/queries/OpenSearchQueryBuilderFactory.java rename openmetadata-service/src/test/java/org/openmetadata/service/search/security/{RBACConditionEvaluatorTest.java => ElasticSearchRBACConditionEvaluatorTest.java} (94%) create mode 100644 openmetadata-service/src/test/java/org/openmetadata/service/search/security/OpenSearchRBACConditionEvaluatorTest.java diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/search/elasticsearch/queries/ElasticQueryBuilder.java b/openmetadata-service/src/main/java/org/openmetadata/service/search/elasticsearch/queries/ElasticQueryBuilder.java index 110610b18eae..e6bf617d6327 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/search/elasticsearch/queries/ElasticQueryBuilder.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/search/elasticsearch/queries/ElasticQueryBuilder.java @@ -8,7 +8,7 @@ import org.openmetadata.service.search.queries.OMQueryBuilder; public class ElasticQueryBuilder implements OMQueryBuilder { - + private boolean isMustNot = false; private QueryBuilder query; public ElasticQueryBuilder() { @@ -85,11 +85,6 @@ public OMQueryBuilder should(OMQueryBuilder query) { return should(List.of(query)); } - @Override - public OMQueryBuilder mustNot(OMQueryBuilder query) { - return mustNot(List.of(query)); - } - @Override public boolean hasClauses() { if (query instanceof BoolQueryBuilder) { diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/search/opensearch/queries/OpenSearchQueryBuilder.java b/openmetadata-service/src/main/java/org/openmetadata/service/search/opensearch/queries/OpenSearchQueryBuilder.java new file mode 100644 index 000000000000..869b360e9c91 --- /dev/null +++ b/openmetadata-service/src/main/java/org/openmetadata/service/search/opensearch/queries/OpenSearchQueryBuilder.java @@ -0,0 +1,151 @@ +package org.openmetadata.service.search.opensearch.queries; + +import java.util.List; +import org.openmetadata.service.search.queries.OMQueryBuilder; +import os.org.opensearch.index.query.BoolQueryBuilder; +import os.org.opensearch.index.query.MatchAllQueryBuilder; +import os.org.opensearch.index.query.QueryBuilder; +import os.org.opensearch.index.query.QueryBuilders; + +public class OpenSearchQueryBuilder implements OMQueryBuilder { + private boolean isMustNot = false; + private QueryBuilder query; + + public OpenSearchQueryBuilder() { + // Default constructor + } + + public OpenSearchQueryBuilder(QueryBuilder query) { + this.query = query; + } + + @Override + public boolean isEmpty() { + return query == null; + } + + @Override + public boolean isMatchNone() { + // Check if the query is a bool query with must_not match_all + if (query instanceof BoolQueryBuilder) { + BoolQueryBuilder boolQuery = (BoolQueryBuilder) query; + return boolQuery.must().isEmpty() + && boolQuery.should().isEmpty() + && boolQuery.mustNot().size() == 1 + && boolQuery.mustNot().get(0) instanceof MatchAllQueryBuilder; + } + return false; + } + + @Override + public boolean isMatchAll() { + return query instanceof MatchAllQueryBuilder; + } + + @Override + public OMQueryBuilder must(List queries) { + BoolQueryBuilder boolQuery = getOrCreateBoolQuery(); + for (OMQueryBuilder q : queries) { + OpenSearchQueryBuilder eqb = (OpenSearchQueryBuilder) q; + boolQuery.must(eqb.build()); + } + this.query = boolQuery; + return this; + } + + @Override + public OMQueryBuilder should(List queries) { + BoolQueryBuilder boolQuery = getOrCreateBoolQuery(); + for (OMQueryBuilder q : queries) { + OpenSearchQueryBuilder eqb = (OpenSearchQueryBuilder) q; + boolQuery.should(eqb.build()); + } + this.query = boolQuery; + return this; + } + + @Override + public OMQueryBuilder mustNot(List queries) { + BoolQueryBuilder boolQuery = getOrCreateBoolQuery(); + for (OMQueryBuilder q : queries) { + OpenSearchQueryBuilder eqb = (OpenSearchQueryBuilder) q; + boolQuery.mustNot(eqb.build()); + } + this.query = boolQuery; + return this; + } + + @Override + public OMQueryBuilder must(OMQueryBuilder query) { + return must(List.of(query)); + } + + @Override + public OMQueryBuilder should(OMQueryBuilder query) { + return should(List.of(query)); + } + + @Override + public boolean hasClauses() { + if (query instanceof BoolQueryBuilder) { + BoolQueryBuilder boolQuery = (BoolQueryBuilder) query; + return !boolQuery.must().isEmpty() + || !boolQuery.should().isEmpty() + || !boolQuery.mustNot().isEmpty(); + } + return query != null; + } + + public QueryBuilder build() { + return query; + } + + public OpenSearchQueryBuilder setQuery(QueryBuilder query) { + this.query = query; + return this; + } + + // Helper methods + + public OpenSearchQueryBuilder matchNoneQuery() { + this.query = QueryBuilders.boolQuery().mustNot(QueryBuilders.matchAllQuery()); + return this; + } + + public OpenSearchQueryBuilder matchAllQuery() { + this.query = QueryBuilders.matchAllQuery(); + return this; + } + + public OpenSearchQueryBuilder boolQuery() { + this.query = QueryBuilders.boolQuery(); + return this; + } + + public OpenSearchQueryBuilder termQuery(String field, String value) { + this.query = QueryBuilders.termQuery(field, value); + return this; + } + + public OpenSearchQueryBuilder termsQuery(String field, List values) { + this.query = QueryBuilders.termsQuery(field, values); + return this; + } + + public OpenSearchQueryBuilder existsQuery(String field) { + this.query = QueryBuilders.existsQuery(field); + return this; + } + + private BoolQueryBuilder getOrCreateBoolQuery() { + if (query instanceof BoolQueryBuilder) { + return (BoolQueryBuilder) query; + } else { + BoolQueryBuilder boolQuery = QueryBuilders.boolQuery(); + if (query != null) { + boolQuery.must(query); + } + return boolQuery; + } + } +} diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/search/opensearch/queries/OpenSearchQueryBuilderFactory.java b/openmetadata-service/src/main/java/org/openmetadata/service/search/opensearch/queries/OpenSearchQueryBuilderFactory.java new file mode 100644 index 000000000000..2ad5b6864d0c --- /dev/null +++ b/openmetadata-service/src/main/java/org/openmetadata/service/search/opensearch/queries/OpenSearchQueryBuilderFactory.java @@ -0,0 +1,38 @@ +package org.openmetadata.service.search.opensearch.queries; + +import java.util.List; +import org.openmetadata.service.search.queries.OMQueryBuilder; +import org.openmetadata.service.search.queries.QueryBuilderFactory; + +public class OpenSearchQueryBuilderFactory implements QueryBuilderFactory { + + @Override + public OMQueryBuilder matchNoneQuery() { + return new OpenSearchQueryBuilder().matchNoneQuery(); + } + + @Override + public OMQueryBuilder matchAllQuery() { + return new OpenSearchQueryBuilder().matchAllQuery(); + } + + @Override + public OMQueryBuilder boolQuery() { + return new OpenSearchQueryBuilder().boolQuery(); + } + + @Override + public OMQueryBuilder termQuery(String field, String value) { + return new OpenSearchQueryBuilder().termQuery(field, value); + } + + @Override + public OMQueryBuilder termsQuery(String field, List values) { + return new OpenSearchQueryBuilder().termsQuery(field, values); + } + + @Override + public OMQueryBuilder existsQuery(String field) { + return new OpenSearchQueryBuilder().existsQuery(field); + } +} diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/search/queries/OMQueryBuilder.java b/openmetadata-service/src/main/java/org/openmetadata/service/search/queries/OMQueryBuilder.java index a52b75598054..a280c34284a6 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/search/queries/OMQueryBuilder.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/search/queries/OMQueryBuilder.java @@ -20,7 +20,5 @@ public interface OMQueryBuilder { OMQueryBuilder should(OMQueryBuilder query); - OMQueryBuilder mustNot(OMQueryBuilder query); - boolean hasClauses(); } diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/search/security/ConditionCollector.java b/openmetadata-service/src/main/java/org/openmetadata/service/search/security/ConditionCollector.java index 4439d9098159..e3dce036bf43 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/search/security/ConditionCollector.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/search/security/ConditionCollector.java @@ -46,32 +46,6 @@ public void addMustNot(OMQueryBuilder query) { } } - public void mergeMust(ConditionCollector other) { - if (other.isMatchNothing()) { - this.setMatchNothing(true); - } else { - this.mustQueries.addAll(other.mustQueries); - this.mustNotQueries.addAll(other.mustNotQueries); - this.shouldQueries.addAll(other.shouldQueries); - } - } - - public void mergeShould(ConditionCollector other) { - if (!other.isMatchNothing()) { - this.shouldQueries.addAll(other.mustQueries); - this.shouldQueries.addAll(other.shouldQueries); - this.mustNotQueries.addAll(other.mustNotQueries); - } - } - - public void mergeMustNot(ConditionCollector other) { - if (!other.isMatchNothing()) { - this.mustNotQueries.addAll(other.mustQueries); - this.mustNotQueries.addAll(other.shouldQueries); - this.mustQueries.addAll(other.mustNotQueries); - } - } - public boolean isMatchAllQuery() { if (mustQueries.size() == 1 && shouldQueries.isEmpty() && mustNotQueries.isEmpty()) { OMQueryBuilder query = mustQueries.get(0); diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/search/security/RBACConditionEvaluator.java b/openmetadata-service/src/main/java/org/openmetadata/service/search/security/RBACConditionEvaluator.java index 3088fa91fc69..e900f2a40883 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/search/security/RBACConditionEvaluator.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/search/security/RBACConditionEvaluator.java @@ -111,13 +111,13 @@ private void preprocessExpression(SpelNode node, ConditionCollector collector) { preprocessExpression(node.getChild(0), subCollector); if (subCollector.isMatchAllQuery()) { - collector.setMatchNothing(true); // NOT TRUE == FALSE + collector.setMatchNothing(true); } else if (subCollector.isMatchNothing()) { collector.addMust(queryBuilderFactory.matchAllQuery()); } else { OMQueryBuilder subQuery = subCollector.buildFinalQuery(); if (subQuery != null && !subQuery.isEmpty()) { - collector.addMustNot(subQuery); // Add to must_not for negation + collector.addMustNot(subQuery); // Add must_not without extra nesting } } } else if (node instanceof MethodReference) { @@ -214,13 +214,13 @@ public void hasAnyRole(List roles, ConditionCollector collector) { public void hasDomain(ConditionCollector collector) { User user = (User) spelContext.lookupVariable("user"); if (user.getDomain() == null) { - collector.setMatchNothing(true); - return; + OMQueryBuilder existsQuery = queryBuilderFactory.existsQuery("domain.id"); + collector.addMustNot(existsQuery); + } else { + String userDomainId = user.getDomain().getId().toString(); + OMQueryBuilder domainQuery = queryBuilderFactory.termQuery("domain.id", userDomainId); + collector.addMust(domainQuery); } - String userDomainId = user.getDomain().getId().toString(); - - OMQueryBuilder domainQuery = queryBuilderFactory.termQuery("domain.id", userDomainId); - collector.addMust(domainQuery); } public void inAnyTeam(List teamNames, ConditionCollector collector) { diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/search/security/RBACConditionEvaluatorTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/search/security/ElasticSearchRBACConditionEvaluatorTest.java similarity index 94% rename from openmetadata-service/src/test/java/org/openmetadata/service/search/security/RBACConditionEvaluatorTest.java rename to openmetadata-service/src/test/java/org/openmetadata/service/search/security/ElasticSearchRBACConditionEvaluatorTest.java index 35806d54af8c..368d2b907bd3 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/search/security/RBACConditionEvaluatorTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/search/security/ElasticSearchRBACConditionEvaluatorTest.java @@ -4,16 +4,16 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.*; +import static org.openmetadata.service.util.TestUtils.assertFieldDoesNotExist; +import static org.openmetadata.service.util.TestUtils.assertFieldExists; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import com.jayway.jsonpath.DocumentContext; import com.jayway.jsonpath.JsonPath; -import com.jayway.jsonpath.PathNotFoundException; import es.org.elasticsearch.index.query.QueryBuilder; import java.io.IOException; import java.util.List; -import java.util.Map; import java.util.UUID; import java.util.concurrent.atomic.AtomicInteger; import org.junit.jupiter.api.BeforeEach; @@ -28,7 +28,7 @@ import org.openmetadata.service.security.policyevaluator.CompiledRule; import org.openmetadata.service.security.policyevaluator.SubjectContext; -class RBACConditionEvaluatorTest { +class ElasticSearchRBACConditionEvaluatorTest { private RBACConditionEvaluator evaluator; private User mockUser; @@ -333,12 +333,17 @@ void testConditionUserLacksDomain() throws IOException { OMQueryBuilder finalQuery = evaluator.evaluateConditions(mockSubjectContext); QueryBuilder elasticQuery = ((ElasticQueryBuilder) finalQuery).build(); String generatedQuery = elasticQuery.toString(); + DocumentContext jsonContext = JsonPath.parse(generatedQuery); + assertFieldExists( + jsonContext, "$.bool.must_not[?(@.exists.field=='domain.id')]", "must_not for domain.id"); - assertTrue( - generatedQuery.contains("\"must_not\""), "The query should contain 'must_not' clause."); - assertTrue( - generatedQuery.contains("\"match_all\""), - "The must_not clause should contain 'match_all' query."); + // Check for owner ID and Public tag in the query + assertFieldExists( + jsonContext, + "$.bool.should[?(@.term['owner.id'].value=='" + mockUser.getId().toString() + "')]", + "owner.id"); + assertFieldExists( + jsonContext, "$.bool.should[?(@.term['tags.tagFQN'].value=='Public')]", "Public tag"); } @Test @@ -690,19 +695,26 @@ void testComplexConditionWithMultipleNegations() throws IOException { assertFieldExists(jsonContext, "$.bool.should[?(@.term['owner.id'])]", "owner.id"); } - private void assertFieldExists(DocumentContext jsonContext, String jsonPath, String fieldName) { - List> result = jsonContext.read(jsonPath, List.class); - assertTrue(result.size() > 0, "The query should contain '" + fieldName + "' term."); - } + @Test + void testNotHasDomainWhenUserHasNoDomain() throws IOException { + setupMockPolicies("!hasDomain() && isOwner()", "ALLOW"); + when(mockUser.getDomain()).thenReturn(null); + when(mockUser.getId()).thenReturn(UUID.randomUUID()); - private void assertFieldDoesNotExist( - DocumentContext jsonContext, String jsonPath, String fieldName) { - try { - List> result = jsonContext.read(jsonPath, List.class); - assertTrue(result.isEmpty(), "The query should not contain '" + fieldName + "' term."); - } catch (PathNotFoundException e) { - // If the path doesn't exist, this is expected behavior, so the test should pass. - assertTrue(true, "The path does not exist as expected: " + jsonPath); - } + OMQueryBuilder finalQuery = evaluator.evaluateConditions(mockSubjectContext); + QueryBuilder elasticQuery = ((ElasticQueryBuilder) finalQuery).build(); + String generatedQuery = elasticQuery.toString(); + + DocumentContext jsonContext = JsonPath.parse(generatedQuery); + + assertFieldExists( + jsonContext, + "$.bool.must_not[0].bool.must_not[?(@.exists.field=='domain.id')]", + "must_not for hasDomain"); + assertFieldExists( + jsonContext, + "$.bool.should[?(@.term['owner.id'].value=='" + mockUser.getId().toString() + "')]", + "owner.id"); + assertFieldDoesNotExist(jsonContext, "$.bool[?(@.match_none)]", "match_none should not exist"); } } diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/search/security/OpenSearchRBACConditionEvaluatorTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/search/security/OpenSearchRBACConditionEvaluatorTest.java new file mode 100644 index 000000000000..cfcfa7729f38 --- /dev/null +++ b/openmetadata-service/src/test/java/org/openmetadata/service/search/security/OpenSearchRBACConditionEvaluatorTest.java @@ -0,0 +1,199 @@ +package org.openmetadata.service.search.security; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static org.openmetadata.service.util.TestUtils.assertFieldDoesNotExist; +import static org.openmetadata.service.util.TestUtils.assertFieldExists; + +import com.jayway.jsonpath.DocumentContext; +import com.jayway.jsonpath.JsonPath; +import java.util.List; +import java.util.UUID; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.openmetadata.schema.entity.teams.User; +import org.openmetadata.schema.type.EntityReference; +import org.openmetadata.schema.type.MetadataOperation; +import org.openmetadata.service.search.opensearch.queries.OpenSearchQueryBuilder; +import org.openmetadata.service.search.opensearch.queries.OpenSearchQueryBuilderFactory; +import org.openmetadata.service.search.queries.OMQueryBuilder; +import org.openmetadata.service.search.queries.QueryBuilderFactory; +import org.openmetadata.service.security.policyevaluator.CompiledRule; +import org.openmetadata.service.security.policyevaluator.SubjectContext; +import os.org.opensearch.index.query.QueryBuilder; + +class OpenSearchRBACConditionEvaluatorTest { + + private RBACConditionEvaluator evaluator; + private User mockUser; + private SubjectContext mockSubjectContext; + private QueryBuilderFactory queryBuilderFactory; + + @BeforeEach + public void setUp() { + queryBuilderFactory = new OpenSearchQueryBuilderFactory(); + evaluator = new RBACConditionEvaluator(queryBuilderFactory); + } + + private void setupMockPolicies(String expression, String effect) { + // Mock the user + mockUser = mock(User.class); + EntityReference mockUserReference = mock(EntityReference.class); + when(mockUser.getEntityReference()).thenReturn(mockUserReference); + when(mockUserReference.getId()).thenReturn(UUID.randomUUID()); + when(mockUser.getId()).thenReturn(UUID.randomUUID()); + when(mockUser.getName()).thenReturn("testUser"); + + // Mock the policy context and rules + SubjectContext.PolicyContext mockPolicyContext = mock(SubjectContext.PolicyContext.class); + when(mockPolicyContext.getPolicyName()).thenReturn("TestPolicy"); + + CompiledRule mockRule = mock(CompiledRule.class); + when(mockRule.getOperations()) + .thenReturn(List.of(MetadataOperation.VIEW_BASIC)); // Mock operation + when(mockRule.getCondition()).thenReturn(expression); + + // Mock the effect of the rule (ALLOW/DENY) + CompiledRule.Effect mockEffect = CompiledRule.Effect.valueOf(effect.toUpperCase()); + when(mockRule.getEffect()).thenReturn(mockEffect); + + when(mockPolicyContext.getRules()).thenReturn(List.of(mockRule)); + + // Mock the subject context with this policy + mockSubjectContext = mock(SubjectContext.class); + when(mockSubjectContext.getPolicies(any())).thenReturn(List.of(mockPolicyContext).iterator()); + when(mockSubjectContext.user()).thenReturn(mockUser); + } + + @Test + void testOpenSearchSimpleRoleAndTagMatching() { + setupMockPolicies("hasAnyRole('Admin') && matchAnyTag('Finance', 'Confidential')", "ALLOW"); + + // Mock user roles + EntityReference role = new EntityReference(); + role.setName("Admin"); + when(mockUser.getRoles()).thenReturn(List.of(role)); + + // Use the OpenSearchQueryBuilder + OMQueryBuilder finalQuery = evaluator.evaluateConditions(mockSubjectContext); + QueryBuilder openSearchQuery = ((OpenSearchQueryBuilder) finalQuery).build(); + String generatedQuery = openSearchQuery.toString(); + + DocumentContext jsonContext = JsonPath.parse(generatedQuery); + + // Assertions + assertFieldExists( + jsonContext, "$.bool.must[?(@.match_all)]", "match_all for hasAnyRole 'Admin'"); + assertFieldExists( + jsonContext, "$.bool.should[?(@.term['tags.tagFQN'].value=='Finance')]", "Finance tag"); + assertFieldExists( + jsonContext, + "$.bool.should[?(@.term['tags.tagFQN'].value=='Confidential')]", + "Confidential tag"); + } + + @Test + void testOpenSearchRoleAndDomainCheck() { + setupMockPolicies("hasAnyRole('DataSteward') && hasDomain()", "ALLOW"); + + // Mock user roles + EntityReference role = new EntityReference(); + role.setName("DataSteward"); + when(mockUser.getRoles()).thenReturn(List.of(role)); + + // Mock user domain + EntityReference domain = new EntityReference(); + domain.setId(UUID.randomUUID()); + when(mockUser.getDomain()).thenReturn(domain); + + // Use the OpenSearchQueryBuilder + OMQueryBuilder finalQuery = evaluator.evaluateConditions(mockSubjectContext); + QueryBuilder openSearchQuery = ((OpenSearchQueryBuilder) finalQuery).build(); + String generatedQuery = openSearchQuery.toString(); + + DocumentContext jsonContext = JsonPath.parse(generatedQuery); + + // Assertions + assertFieldExists( + jsonContext, "$.bool.must[?(@.match_all)]", "match_all for hasAnyRole 'DataSteward'"); + assertFieldExists( + jsonContext, + "$.bool.must[?(@.term['domain.id'].value=='" + domain.getId().toString() + "')]", + "domain.id"); + } + + @Test + void testOpenSearchNegationWithDomainAndOwnerChecks() { + setupMockPolicies("!hasDomain() && isOwner()", "ALLOW"); + + // Mock user ownership + when(mockUser.getId()).thenReturn(UUID.randomUUID()); + + // Use the OpenSearchQueryBuilder + OMQueryBuilder finalQuery = evaluator.evaluateConditions(mockSubjectContext); + QueryBuilder openSearchQuery = ((OpenSearchQueryBuilder) finalQuery).build(); + String generatedQuery = openSearchQuery.toString(); + + DocumentContext jsonContext = JsonPath.parse(generatedQuery); + + // Assertions + assertFieldExists( + jsonContext, + "$.bool.must_not[0].bool.must_not[?(@.exists.field=='domain.id')]", + "must_not for hasDomain"); + assertFieldExists( + jsonContext, + "$.bool.should[?(@.term['owner.id'].value=='" + mockUser.getId().toString() + "')]", + "owner.id"); + assertFieldDoesNotExist(jsonContext, "$.bool[?(@.match_none)]", "match_none should not exist"); + } + + @Test + void testOpenSearchComplexCombination() { + setupMockPolicies( + "hasAnyRole('Admin') && matchAnyTag('Sensitive', 'Confidential') && hasDomain() && inAnyTeam('Analytics')", + "ALLOW"); + + // Mock user roles, domain, and teams + EntityReference role = new EntityReference(); + role.setName("Admin"); + when(mockUser.getRoles()).thenReturn(List.of(role)); + + EntityReference domain = new EntityReference(); + domain.setId(UUID.randomUUID()); + when(mockUser.getDomain()).thenReturn(domain); + + EntityReference team = new EntityReference(); + team.setId(UUID.randomUUID()); + team.setName("Analytics"); + when(mockUser.getTeams()).thenReturn(List.of(team)); + + // Use the OpenSearchQueryBuilder + OMQueryBuilder finalQuery = evaluator.evaluateConditions(mockSubjectContext); + QueryBuilder openSearchQuery = ((OpenSearchQueryBuilder) finalQuery).build(); + String generatedQuery = openSearchQuery.toString(); + + DocumentContext jsonContext = JsonPath.parse(generatedQuery); + + assertFieldExists( + jsonContext, "$.bool.must[?(@.match_all)]", "match_all for hasAnyRole 'Admin'"); + assertFieldExists( + jsonContext, + "$.bool.must[?(@.term['domain.id'].value=='" + domain.getId().toString() + "')]", + "domain.id"); + assertFieldExists( + jsonContext, "$.bool.must[?(@.match_all)]", "match_all for inAnyTeam 'Analytics'"); + + // Assertions for should clause (matchAnyTag) + assertFieldExists( + jsonContext, "$.bool.should[?(@.term['tags.tagFQN'].value=='Sensitive')]", "Sensitive tag"); + assertFieldExists( + jsonContext, + "$.bool.should[?(@.term['tags.tagFQN'].value=='Confidential')]", + "Confidential tag"); + + // Ensure no match_none condition exists + assertFieldDoesNotExist(jsonContext, "$.bool[?(@.match_none)]", "match_none should not exist"); + } +} diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/util/TestUtils.java b/openmetadata-service/src/test/java/org/openmetadata/service/util/TestUtils.java index e27f01224a4e..96d31db0b754 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/util/TestUtils.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/util/TestUtils.java @@ -28,6 +28,8 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import com.flipkart.zjsonpatch.JsonDiff; +import com.jayway.jsonpath.DocumentContext; +import com.jayway.jsonpath.PathNotFoundException; import io.github.resilience4j.core.functions.CheckedRunnable; import io.github.resilience4j.retry.Retry; import io.github.resilience4j.retry.RetryConfig; @@ -704,4 +706,21 @@ public static JsonNode getJsonPatch(String originalJson, String updatedJson) { } return null; } + + public static void assertFieldExists( + DocumentContext jsonContext, String jsonPath, String fieldName) { + List> result = jsonContext.read(jsonPath, List.class); + assertTrue(result.size() > 0, "The query should contain '" + fieldName + "' term."); + } + + public static void assertFieldDoesNotExist( + DocumentContext jsonContext, String jsonPath, String fieldName) { + try { + List> result = jsonContext.read(jsonPath, List.class); + assertTrue(result.isEmpty(), "The query should not contain '" + fieldName + "' term."); + } catch (PathNotFoundException e) { + // If the path doesn't exist, this is expected behavior, so the test should pass. + assertTrue(true, "The path does not exist as expected: " + jsonPath); + } + } }