From b5f4aee676c12255cd969e55f963495e39465e60 Mon Sep 17 00:00:00 2001 From: Sriharsha Chintalapani Date: Tue, 12 Nov 2024 09:11:02 -0800 Subject: [PATCH] Search RBAC improvements (#18591) * Search RBAC: process multiple view policies that can grant access to different set of entities * Search RBAC: process multiple view policies that can grant access to different set of entities * Search RBAC: process multiple view policies that can grant access to different set of entities --- .../search/security/ConditionCollector.java | 16 +- .../security/RBACConditionEvaluator.java | 118 ++++---- ...asticSearchRBACConditionEvaluatorTest.java | 256 ++++++++++-------- .../OpenSearchRBACConditionEvaluatorTest.java | 31 +-- .../openmetadata/service/util/TestUtils.java | 3 +- 5 files changed, 235 insertions(+), 189 deletions(-) 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 41aff89dfb3..c4b2cb8fffc 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 @@ -14,6 +14,7 @@ public class ConditionCollector { private final List shouldQueries = new ArrayList<>(); @Getter @Setter private boolean matchNothing = false; + @Getter @Setter private OMQueryBuilder finalQuery; // New field to hold the final query public ConditionCollector(QueryBuilderFactory queryBuilderFactory) { this.queryBuilderFactory = queryBuilderFactory; @@ -46,11 +47,12 @@ public class ConditionCollector { } public boolean isMatchAllQuery() { - if (mustQueries.size() == 1 && shouldQueries.isEmpty() && mustNotQueries.isEmpty()) { - OMQueryBuilder query = mustQueries.get(0); - return query.isMatchAll(); - } - return false; + // If the collector has no clauses and is not set to match nothing, it represents a match_all + return !hasClauses() && !matchNothing; + } + + public boolean hasClauses() { + return !mustQueries.isEmpty() || !shouldQueries.isEmpty() || !mustNotQueries.isEmpty(); } public OMQueryBuilder buildFinalQuery() { @@ -58,6 +60,10 @@ public class ConditionCollector { return queryBuilderFactory.matchNoneQuery(); } + if (finalQuery != null) { + return finalQuery; + } + if (mustQueries.size() == 1 && shouldQueries.isEmpty() && mustNotQueries.isEmpty()) { return 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 9d713d7a224..ac319e8cf9b 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 @@ -1,6 +1,7 @@ package org.openmetadata.service.search.security; import java.util.*; +import org.openmetadata.schema.entity.policies.accessControl.Rule; import org.openmetadata.schema.entity.teams.User; import org.openmetadata.schema.type.EntityReference; import org.openmetadata.schema.type.MetadataOperation; @@ -36,23 +37,19 @@ public class RBACConditionEvaluator { User user = subjectContext.user(); spelContext.setVariable("user", user); - ConditionCollector finalCollector = new ConditionCollector(queryBuilderFactory); + List allowQueries = new ArrayList<>(); + List denyQueries = new ArrayList<>(); for (Iterator it = subjectContext.getPolicies(List.of(user.getEntityReference())); it.hasNext(); ) { SubjectContext.PolicyContext context = it.next(); - ConditionCollector policyCollector = new ConditionCollector(queryBuilderFactory); - List allowRuleQueries = new ArrayList<>(); - List denyRuleQueries = new ArrayList<>(); - for (CompiledRule rule : context.getRules()) { - boolean isDenyRule = rule.getEffect().toString().equalsIgnoreCase("DENY"); + boolean isDenyRule = rule.getEffect() == Rule.Effect.DENY; Set ruleOperations = new HashSet<>(rule.getOperations()); ruleOperations.retainAll(SEARCH_RELEVANT_OPS); if (ruleOperations.isEmpty()) { - // Skip this rule as it does not affect search results continue; } @@ -60,47 +57,59 @@ public class RBACConditionEvaluator { if (ruleQuery == null || ruleQuery.isEmpty()) { continue; } + if (isDenyRule) { - denyRuleQueries.add(ruleQuery); + denyQueries.add(ruleQuery); } else { - allowRuleQueries.add(ruleQuery); + allowQueries.add(ruleQuery); } } - - if (!denyRuleQueries.isEmpty()) { - if (denyRuleQueries.size() == 1) { - policyCollector.addMustNot(denyRuleQueries.get(0)); - } else { - OMQueryBuilder denyQuery = queryBuilderFactory.boolQuery().should(denyRuleQueries); - policyCollector.addMustNot(denyQuery); - } - } - - if (!allowRuleQueries.isEmpty()) { - if (allowRuleQueries.size() == 1) { - policyCollector.addMust(allowRuleQueries.get(0)); - } else { - OMQueryBuilder allowQuery = queryBuilderFactory.boolQuery().should(allowRuleQueries); - policyCollector.addMust(allowQuery); - } - } else { - policyCollector.addMust(queryBuilderFactory.matchAllQuery()); - } - - OMQueryBuilder policyFinalQuery = policyCollector.buildFinalQuery(); - if (policyFinalQuery != null && !policyFinalQuery.isEmpty()) { - finalCollector.addMust(policyFinalQuery); - } } - return finalCollector.buildFinalQuery(); + OMQueryBuilder finalQuery; + + if (!allowQueries.isEmpty()) { + OMQueryBuilder finalAllowQuery = + (allowQueries.size() == 1) + ? allowQueries.get(0) + : queryBuilderFactory.boolQuery().should(allowQueries); + + finalQuery = finalAllowQuery; + + if (!denyQueries.isEmpty()) { + OMQueryBuilder finalDenyQuery = + (denyQueries.size() == 1) + ? denyQueries.get(0) + : queryBuilderFactory.boolQuery().should(denyQueries); + + finalQuery = + queryBuilderFactory + .boolQuery() + .must(Collections.singletonList(finalAllowQuery)) + .mustNot(Collections.singletonList(finalDenyQuery)); + } + } else if (!denyQueries.isEmpty()) { + OMQueryBuilder finalDenyQuery = + (denyQueries.size() == 1) + ? denyQueries.get(0) + : queryBuilderFactory.boolQuery().should(denyQueries); + + finalQuery = + queryBuilderFactory + .boolQuery() + .must(queryBuilderFactory.matchAllQuery()) + .mustNot(Collections.singletonList(finalDenyQuery)); + } else { + finalQuery = queryBuilderFactory.matchAllQuery(); + } + + return finalQuery; } private OMQueryBuilder buildRuleQuery(CompiledRule rule, User user) { ConditionCollector ruleCollector = new ConditionCollector(queryBuilderFactory); spelContext.setVariable("user", user); - // Apply index filtering if resources are specified and not "All" if (!rule.getResources().isEmpty() && !rule.getResources().contains("All")) { OMQueryBuilder indexFilter = getIndexFilter(rule.getResources()); ruleCollector.addMust(indexFilter); @@ -131,8 +140,8 @@ public class RBACConditionEvaluator { } } else if (node instanceof OpOr) { List orQueries = new ArrayList<>(); - boolean allMatchNothing = true; boolean hasTrueCondition = false; + boolean allMatchNothing = true; for (int i = 0; i < node.getChildCount(); i++) { ConditionCollector childCollector = new ConditionCollector(queryBuilderFactory); @@ -144,7 +153,7 @@ public class RBACConditionEvaluator { if (childCollector.isMatchAllQuery()) { hasTrueCondition = true; - break; + break; // Short-circuit since one condition is always true } OMQueryBuilder childQuery = childCollector.buildFinalQuery(); @@ -155,13 +164,17 @@ public class RBACConditionEvaluator { } if (hasTrueCondition) { + // One of the OR conditions is always true; the entire OR condition is true + // No need to add any queries for this OR expression + // Optionally, you can add a match_all query to represent this collector.addMust(queryBuilderFactory.matchAllQuery()); } else if (allMatchNothing) { + // All OR conditions are impossible; set matchNothing to true collector.setMatchNothing(true); } else { - for (OMQueryBuilder orQuery : orQueries) { - collector.addShould(orQuery); - } + // Combine the collected queries using a should clause + OMQueryBuilder orQuery = queryBuilderFactory.boolQuery().should(orQueries); + collector.addMust(orQuery); } } else if (node instanceof OperatorNot) { ConditionCollector subCollector = new ConditionCollector(queryBuilderFactory); @@ -220,10 +233,18 @@ public class RBACConditionEvaluator { } public void matchAnyTag(List tags, ConditionCollector collector) { + List tagQueries = new ArrayList<>(); for (String tag : tags) { OMQueryBuilder tagQuery = queryBuilderFactory.termQuery("tags.tagFQN", tag); - collector.addShould(tagQuery); + tagQueries.add(tagQuery); } + OMQueryBuilder tagQueryCombined; + if (tagQueries.size() == 1) { + tagQueryCombined = tagQueries.get(0); + } else { + tagQueryCombined = queryBuilderFactory.boolQuery().should(tagQueries); + } + collector.addMust(tagQueryCombined); } public void matchAllTags(List tags, ConditionCollector collector) { @@ -243,14 +264,19 @@ public class RBACConditionEvaluator { } } - for (OMQueryBuilder ownerQuery : ownerQueries) { - collector.addShould(ownerQuery); + OMQueryBuilder ownerQuery; + if (ownerQueries.size() == 1) { + ownerQuery = ownerQueries.get(0); + } else { + ownerQuery = queryBuilderFactory.boolQuery().should(ownerQueries); } + + collector.addMust(ownerQuery); } public void noOwner(ConditionCollector collector) { OMQueryBuilder existsQuery = queryBuilderFactory.existsQuery("owners.id"); - collector.addMustNot(existsQuery); + collector.addMustNot(existsQuery); // Wrap existsQuery in a List } public void hasAnyRole(List roles, ConditionCollector collector) { @@ -274,7 +300,7 @@ public class RBACConditionEvaluator { User user = (User) spelContext.lookupVariable("user"); if (user.getDomain() == null) { OMQueryBuilder existsQuery = queryBuilderFactory.existsQuery("domain.id"); - collector.addMustNot(existsQuery); + collector.addMustNot(existsQuery); // Wrap existsQuery in a List } else { String userDomainId = user.getDomain().getId().toString(); OMQueryBuilder domainQuery = queryBuilderFactory.termQuery("domain.id", userDomainId); diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/search/security/ElasticSearchRBACConditionEvaluatorTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/search/security/ElasticSearchRBACConditionEvaluatorTest.java index 8807db6bb4b..b4b51bb8292 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/search/security/ElasticSearchRBACConditionEvaluatorTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/search/security/ElasticSearchRBACConditionEvaluatorTest.java @@ -45,11 +45,15 @@ class ElasticSearchRBACConditionEvaluatorTest { private RBACConditionEvaluator evaluator; private User mockUser; private SubjectContext mockSubjectContext; + private List policies; @BeforeEach public void setUp() { + policies = new ArrayList<>(); // Initialize the policies list + QueryBuilderFactory queryBuilderFactory = new ElasticQueryBuilderFactory(); evaluator = new RBACConditionEvaluator(queryBuilderFactory); + SearchRepository mockSearchRepository = mock(SearchRepository.class); when(mockSearchRepository.getIndexOrAliasName(anyString())) .thenAnswer( @@ -58,6 +62,7 @@ class ElasticSearchRBACConditionEvaluatorTest { return resource.toLowerCase(); }); Entity.setSearchRepository(mockSearchRepository); + mockSubjectContext = mock(SubjectContext.class); mockUser = mock(User.class); EntityReference mockUserReference = mock(EntityReference.class); @@ -66,6 +71,9 @@ class ElasticSearchRBACConditionEvaluatorTest { when(mockUser.getId()).thenReturn(UUID.randomUUID()); when(mockUser.getName()).thenReturn("testUser"); when(mockSubjectContext.user()).thenReturn(mockUser); + + // Set up the mock behavior to return the policies iterator + when(mockSubjectContext.getPolicies(any())).thenAnswer(invocation -> policies.iterator()); } @AfterEach @@ -85,8 +93,8 @@ class ElasticSearchRBACConditionEvaluatorTest { List mockRules = new ArrayList<>(); for (int i = 0; i < expressions.size(); i++) { CompiledRule mockRule = mock(CompiledRule.class); - when(mockRule.getOperations()).thenReturn(List.of(MetadataOperation.VIEW_BASIC)); when(mockRule.getCondition()).thenReturn(expressions.get(i)); + List resources = (resourcesList.size() > i) ? resourcesList.get(i) : List.of("All"); when(mockRule.getResources()).thenReturn(resources); @@ -96,11 +104,14 @@ class ElasticSearchRBACConditionEvaluatorTest { CompiledRule.Effect mockEffect = CompiledRule.Effect.valueOf(effect.toUpperCase()); when(mockRule.getEffect()).thenReturn(mockEffect); + mockRules.add(mockRule); } when(mockPolicyContext.getRules()).thenReturn(mockRules); - when(mockSubjectContext.getPolicies(any())).thenReturn(List.of(mockPolicyContext).iterator()); + + // Add the policy context to the policies list + policies.add(mockPolicyContext); } private void setupMockPolicies(String expression, String effect, List resources) { @@ -190,43 +201,37 @@ class ElasticSearchRBACConditionEvaluatorTest { "(matchAnyTag('PII.Sensitive') || matchAllTags('Test.Test1', 'Test.Test2')) && (!isOwner() || noOwner())", "ALLOW"); - // Evaluate condition and build query OMQueryBuilder finalQuery = evaluator.evaluateConditions(mockSubjectContext); QueryBuilder elasticQuery = ((ElasticQueryBuilder) finalQuery).build(); String generatedQuery = elasticQuery.toString(); - // Parse the generated query DocumentContext jsonContext = JsonPath.parse(generatedQuery); - // Check for the presence of the PII.Sensitive tag in the "should" clause assertFieldExists( jsonContext, - "$.bool.should[0].bool.should[?(@.term['tags.tagFQN'].value=='PII.Sensitive')]", + "$.bool.must[0].bool.should[?(@.term['tags.tagFQN'].value=='PII.Sensitive')]", "PII.Sensitive tag"); - // Check for the presence of Test.Test1 and Test.Test2 tags in the "must" clause assertFieldExists( jsonContext, - "$.bool.should[1].bool.must[?(@.term['tags.tagFQN'].value=='Test.Test1')]", + "$.bool.must[0].bool.should[1].bool.must[?(@.term['tags.tagFQN'].value=='Test.Test1')]", "Test.Test1 tag"); + assertFieldExists( jsonContext, - "$.bool.should[1].bool.must[?(@.term['tags.tagFQN'].value=='Test.Test2')]", + "$.bool.must[0].bool.should[1].bool.must[?(@.term['tags.tagFQN'].value=='Test.Test2')]", "Test.Test2 tag"); - // Check for the presence of owner.id in the "must_not" clause for the negation assertFieldExists( jsonContext, - "$.bool.should[2].bool.must_not[0].bool.should[?(@.term['owners.id'])]", + "$.bool.must[1].bool.should[0].bool.must_not[?(@.term['owners.id'])]", "owners.id in must_not"); - // Check for the presence of must_not for the case where there is no owner assertFieldExists( jsonContext, - "$.bool.should[3].bool.must_not[?(@.exists.field=='owners.id')]", + "$.bool.must[1].bool.should[1].bool.must_not[?(@.exists.field=='owners.id')]", "no owner must_not clause"); - // Count the number of bool clauses ObjectMapper objectMapper = new ObjectMapper(); JsonNode rootNode = objectMapper.readTree(generatedQuery); AtomicInteger boolQueryCount = new AtomicInteger(0); @@ -280,24 +285,42 @@ class ElasticSearchRBACConditionEvaluatorTest { DocumentContext jsonContext = JsonPath.parse(generatedQuery); - // Assert that the query contains 'domain.id' assertFieldExists(jsonContext, "$.bool.must[?(@.term['domain.id'])]", "domain.id"); - // Assert that the query contains the user's domain ID assertFieldExists( jsonContext, "$.bool.must[?(@.term['domain.id'].value=='" + domain.getId().toString() + "')]", "user's domain ID"); - // Assert that the query contains 'inAnyTeam' logic for 'Analytics' assertFieldExists( jsonContext, "$.bool.must[?(@.match_all)]", "match_all for inAnyTeam 'Analytics'"); - // Ensure no match_any_tag query is processed since inAnyTeam('Analytics') is true assertFieldDoesNotExist( - jsonContext, "$.bool.should[?(@.term['tags.tagFQN'])]", "matchAnyTag 'Sensitive'"); + jsonContext, "$.bool.must[?(@.term['tags.tagFQN'])]", "matchAnyTag 'Sensitive'"); + + assertFieldDoesNotExist(jsonContext, "$.bool[?(@.match_none)]", "match_none"); + } + + @Test + void testAndConditionWithMatchAnyTagAndInAnyTeam() { + setupMockPolicies("matchAnyTag('Sensitive') && inAnyTeam('Analytics')", "ALLOW"); + + EntityReference team = new EntityReference(); + team.setId(UUID.randomUUID()); + team.setName("Analytics"); + when(mockUser.getTeams()).thenReturn(List.of(team)); + + OMQueryBuilder finalQuery = evaluator.evaluateConditions(mockSubjectContext); + QueryBuilder elasticQuery = ((ElasticQueryBuilder) finalQuery).build(); + String generatedQuery = elasticQuery.toString(); + + DocumentContext jsonContext = JsonPath.parse(generatedQuery); + + assertFieldExists( + jsonContext, + "$.bool.must[?(@.term['tags.tagFQN'].value=='Sensitive')]", + "tags.tagFQN 'Sensitive'"); - // Ensure the query does not contain a match_none condition assertFieldDoesNotExist(jsonContext, "$.bool[?(@.match_none)]", "match_none"); } @@ -311,7 +334,6 @@ class ElasticSearchRBACConditionEvaluatorTest { QueryBuilder elasticQuery = ((ElasticQueryBuilder) finalQuery).build(); String generatedQuery = elasticQuery.toString(); - // Adjust the assertion assertTrue( generatedQuery.contains("\"must_not\""), "The query should contain 'must_not' clause."); assertTrue( @@ -372,7 +394,7 @@ class ElasticSearchRBACConditionEvaluatorTest { @Test void testConditionUserLacksDomain() { - setupMockPolicies("hasDomain() && isOwner() && matchAnyTag('Public')", "ALLOW"); + setupMockPolicies("hasDomain() && isOwner() && matchAnyTag('Public', 'Private')", "ALLOW"); when(mockUser.getDomain()).thenReturn(null); OMQueryBuilder finalQuery = evaluator.evaluateConditions(mockSubjectContext); QueryBuilder elasticQuery = ((ElasticQueryBuilder) finalQuery).build(); @@ -381,13 +403,14 @@ class ElasticSearchRBACConditionEvaluatorTest { assertFieldExists( jsonContext, "$.bool.must_not[?(@.exists.field=='domain.id')]", "must_not for domain.id"); - // Check for owner ID and Public tag in the query assertFieldExists( jsonContext, - "$.bool.should[?(@.term['owners.id'].value=='" + mockUser.getId().toString() + "')]", + "$.bool.must[?(@.term['owners.id'].value=='" + mockUser.getId().toString() + "')]", "owner.id"); assertFieldExists( - jsonContext, "$.bool.should[?(@.term['tags.tagFQN'].value=='Public')]", "Public tag"); + jsonContext, + "$.bool.must[1].bool.should[?(@.term['tags.tagFQN'].value=='Public')]", + "Public tag"); } @Test @@ -460,21 +483,14 @@ class ElasticSearchRBACConditionEvaluatorTest { DocumentContext jsonContext = JsonPath.parse(generatedQuery); - // Assertions - assertFieldExists( - jsonContext, "$.bool.must[?(@.match_all)]", "match_all for hasAnyRole 'Admin'"); - - // Ensure no further processing for matchAnyTag('Confidential') or hasDomain() assertFieldDoesNotExist( jsonContext, "$.bool.must[?(@.term['tags.tagFQN'])]", "matchAnyTag 'Confidential'"); assertFieldDoesNotExist( jsonContext, "$.bool.must[?(@.term['domain.id'])]", "hasDomain 'domain.id'"); - // Ensure that the query does not check for noOwner since inAnyTeam('HR') is false assertFieldDoesNotExist( jsonContext, "$.bool.must_not[?(@.exists.field=='owners.id')]", "noOwner clause"); - // Ensure the query does not contain a match_none condition assertFieldDoesNotExist(jsonContext, "$.bool[?(@.match_none)]", "match_none"); } @@ -500,9 +516,7 @@ class ElasticSearchRBACConditionEvaluatorTest { "$.bool.must_not[0].bool.should[0].bool.must[?(@.term['tags.tagFQN'].value=='Internal')]", "Internal"); assertFieldExists( - jsonContext, - "$.bool.must_not[0].bool.should[1].bool.should[?(@.term['owners.id'])]", - "owners.id"); + jsonContext, "$.bool.must_not[0].bool.should[?(@.term['owners.id'])]", "owners.id"); } @Test @@ -581,56 +595,45 @@ class ElasticSearchRBACConditionEvaluatorTest { "(hasAnyRole('Admin') && hasDomain() && matchAllTags('PII', 'Sensitive')) || (isOwner() && !matchAnyTag('Restricted'))", "ALLOW"); - // Mock user roles EntityReference role = new EntityReference(); role.setName("Admin"); when(mockUser.getRoles()).thenReturn(List.of(role)); - // Mock user domain EntityReference domain = new EntityReference(); domain.setId(UUID.randomUUID()); when(mockUser.getDomain()).thenReturn(domain); - // Mock user ownership when(mockUser.getId()).thenReturn(UUID.randomUUID()); EntityReference userRef = new EntityReference(); userRef.setId(mockUser.getId()); userRef.setType("user"); when(mockUser.getEntityReference()).thenReturn(userRef); - // Evaluate condition and build query OMQueryBuilder finalQuery = evaluator.evaluateConditions(mockSubjectContext); QueryBuilder elasticQuery = ((ElasticQueryBuilder) finalQuery).build(); String generatedQuery = elasticQuery.toString(); - // Parse the generated query DocumentContext jsonContext = JsonPath.parse(generatedQuery); - // Assertions - - // Check for the presence of domain.id assertFieldExists( - jsonContext, "$.bool.should[0].bool.must[?(@.term['domain.id'])]", "domain.id"); + jsonContext, "$.bool.should[?(@.bool.must[?(@.term['domain.id'])])]", "domain.id"); - // Check for the presence of PII and Sensitive tags in the matchAllTags clause assertFieldExists( jsonContext, - "$.bool.should[0].bool.must[?(@.term['tags.tagFQN'].value=='PII')]", + "$.bool.should[?(@.bool.must[?(@.term['tags.tagFQN'].value=='PII')])]", "PII tag"); assertFieldExists( jsonContext, - "$.bool.should[0].bool.must[?(@.term['tags.tagFQN'].value=='Sensitive')]", + "$.bool.should[?(@.bool.must[?(@.term['tags.tagFQN'].value=='Sensitive')])]", "Sensitive tag"); - // Check for the presence of must_not for matchAnyTag('Restricted') assertFieldExists( jsonContext, - "$.bool.should[1].bool.must_not[0].bool.should[?(@.term['tags.tagFQN'].value=='Restricted')]", + "$.bool.should[?(@.bool.must_not[?(@.term['tags.tagFQN'].value=='Restricted')])]", "must_not for matchAnyTag 'Restricted'"); - // Check for the presence of owner.id in the second should block assertFieldExists( - jsonContext, "$.bool.should[1].bool.should[?(@.term['owners.id'])]", "owners.id"); + jsonContext, "$.bool.should[?(@.bool.must[?(@.term['owners.id'])])]", "owners.id"); } @Test @@ -639,18 +642,15 @@ class ElasticSearchRBACConditionEvaluatorTest { "(hasAnyRole('Admin') || hasAnyRole('DataSteward')) && (matchAnyTag('Finance') || matchAllTags('Confidential', 'Internal')) && !inAnyTeam('Data')", "ALLOW"); - // Mock user roles EntityReference role = new EntityReference(); role.setName("DataSteward"); when(mockUser.getRoles()).thenReturn(List.of(role)); - // Mock user teams EntityReference team = new EntityReference(); team.setId(UUID.randomUUID()); team.setName("Engineering"); when(mockUser.getTeams()).thenReturn(List.of(team)); - // Evaluate condition OMQueryBuilder finalQuery = evaluator.evaluateConditions(mockSubjectContext); QueryBuilder elasticQuery = ((ElasticQueryBuilder) finalQuery).build(); String generatedQuery = elasticQuery.toString(); @@ -665,17 +665,17 @@ class ElasticSearchRBACConditionEvaluatorTest { // matchAllTags('Confidential', 'Internal') assertFieldExists( jsonContext, - "$.bool.should[0].bool.should[?(@.term['tags.tagFQN'].value=='Finance')]", + "$.bool.must[1].bool.should[?(@.term['tags.tagFQN'].value=='Finance')]", "Finance tag"); assertFieldExists( jsonContext, - "$.bool.should[1].bool.must[?(@.term['tags.tagFQN'].value=='Confidential')]", + "$.bool.must[1].bool.should[1].bool.must[?(@.term['tags.tagFQN'].value=='Confidential')]", "Confidential tag"); assertFieldExists( jsonContext, - "$.bool.should[1].bool.must[?(@.term['tags.tagFQN'].value=='Internal')]", + "$.bool.must[1].bool.should[1].bool.must[?(@.term['tags.tagFQN'].value=='Internal')]", "Internal tag"); // Ensure no must_not for inAnyTeam('Data') since the user is in 'Engineering' @@ -688,23 +688,19 @@ class ElasticSearchRBACConditionEvaluatorTest { "!((hasAnyRole('Admin') || inAnyTeam('Engineering')) && matchAnyTag('Confidential') && matchAllTags('Sensitive', 'Classified')) && hasDomain() && isOwner()", "ALLOW"); - // Mock user roles EntityReference role = new EntityReference(); role.setName("Admin"); when(mockUser.getRoles()).thenReturn(List.of(role)); - // Mock user teams EntityReference team = new EntityReference(); team.setId(UUID.randomUUID()); team.setName("Engineering"); when(mockUser.getTeams()).thenReturn(List.of(team)); - // Mock user domain EntityReference domain = new EntityReference(); domain.setId(UUID.randomUUID()); when(mockUser.getDomain()).thenReturn(domain); - // Mock user ownership when(mockUser.getId()).thenReturn(UUID.randomUUID()); EntityReference userRef = new EntityReference(); userRef.setId(mockUser.getId()); @@ -716,10 +712,8 @@ class ElasticSearchRBACConditionEvaluatorTest { QueryBuilder elasticQuery = ((ElasticQueryBuilder) finalQuery).build(); String generatedQuery = elasticQuery.toString(); DocumentContext jsonContext = JsonPath.parse(generatedQuery); - // `domain.id` should be in `must` assertFieldExists(jsonContext, "$.bool.must[?(@.term['domain.id'])]", "domain.id"); - // `Sensitive` and `Classified` tags should be in `must_not[0].bool.must` assertFieldExists( jsonContext, "$.bool.must_not[0].bool.must[?(@.term['tags.tagFQN'].value=='Sensitive')]", @@ -729,14 +723,14 @@ class ElasticSearchRBACConditionEvaluatorTest { "$.bool.must_not[0].bool.must[?(@.term['tags.tagFQN'].value=='Classified')]", "Classified"); - // `Confidential` tag should be in `must_not[0].bool.should` assertFieldExists( jsonContext, - "$.bool.must_not[0].bool.should[?(@.term['tags.tagFQN'].value=='Confidential')]", + "$.bool.must_not[0].bool.must[?(@.term['tags.tagFQN'].value=='Confidential')]", "Confidential"); // Ownership (isOwner condition) should be in `should` - assertFieldExists(jsonContext, "$.bool.should[?(@.term['owners.id'])]", "owners.id"); + assertFieldExists( + jsonContext, "$.bool.must[1].bool.should[?(@.term['owners.id'])]", "owners.id"); } @Test @@ -757,41 +751,35 @@ class ElasticSearchRBACConditionEvaluatorTest { "must_not for hasDomain"); assertFieldExists( jsonContext, - "$.bool.should[?(@.term['owners.id'].value=='" + mockUser.getId().toString() + "')]", + "$.bool.must[?(@.term['owners.id'].value=='" + mockUser.getId().toString() + "')]", "owners.id"); assertFieldDoesNotExist(jsonContext, "$.bool[?(@.match_none)]", "match_none should not exist"); } @Test void testIndexFilteringBasedOnResource() { - // Assume the rule applies to 'Table' resource setupMockPolicies("hasAnyRole('Admin') && matchAnyTag('Sensitive')", "ALLOW", List.of("Table")); - // Mock user roles EntityReference role = new EntityReference(); role.setName("Admin"); when(mockUser.getRoles()).thenReturn(List.of(role)); - // Evaluate condition and build query OMQueryBuilder finalQuery = evaluator.evaluateConditions(mockSubjectContext); QueryBuilder elasticQuery = ((ElasticQueryBuilder) finalQuery).build(); String generatedQuery = elasticQuery.toString(); DocumentContext jsonContext = JsonPath.parse(generatedQuery); - // Assert that the query contains the appropriate index (e.g., 'table_search_index') assertFieldExists( jsonContext, "$.bool.must[?(@.terms._index && @.terms._index[?(@ == 'table')])]", "Index filtering for 'Table' resource"); - // Assert that the query contains 'tags.tagFQN' for 'Sensitive' tag assertFieldExists( - jsonContext, "$.bool.should[?(@.term['tags.tagFQN'].value=='Sensitive')]", "Sensitive tag"); + jsonContext, "$.bool.must[?(@.term['tags.tagFQN'].value=='Sensitive')]", "Sensitive tag"); } @Test void testComplexConditionWithIndexFiltering() { - // Assume the rule applies to 'Database' and 'Table' resources setupMockPolicies( "hasDomain() && matchAnyTag('Sensitive')", "ALLOW", List.of("Database", "Table")); @@ -806,7 +794,6 @@ class ElasticSearchRBACConditionEvaluatorTest { DocumentContext jsonContext = JsonPath.parse(generatedQuery); - // Assert that the query contains the appropriate indices for 'Database' and 'Table' assertFieldExists( jsonContext, "$.bool.must[?(@.terms._index && @.terms._index[?(@ == 'database')])]", @@ -816,9 +803,8 @@ class ElasticSearchRBACConditionEvaluatorTest { "$.bool.must[?(@.terms._index && @.terms._index[?(@ == 'table')])]", "Index filtering for 'Table' resource"); - // Assert that the query contains 'tags.tagFQN' for 'Sensitive' tag assertFieldExists( - jsonContext, "$.bool.should[?(@.term['tags.tagFQN'].value=='Sensitive')]", "Sensitive tag"); + jsonContext, "$.bool.must[?(@.term['tags.tagFQN'].value=='Sensitive')]", "Sensitive tag"); assertFieldExists( jsonContext, "$.bool.must[?(@.term['domain.id'].value=='" + domain.getId().toString() + "')]", @@ -827,7 +813,6 @@ class ElasticSearchRBACConditionEvaluatorTest { @Test void testMultipleRulesInPolicy() { - // Set up policies with multiple rules setupMockPolicies( List.of( "hasAnyRole('Admin') && matchAnyTag('Sensitive')", @@ -836,7 +821,6 @@ class ElasticSearchRBACConditionEvaluatorTest { List.of(List.of("Table"), List.of("Dashboard")), List.of(List.of(MetadataOperation.VIEW_BASIC))); - // Mock user roles and teams EntityReference role = new EntityReference(); role.setName("Admin"); when(mockUser.getRoles()).thenReturn(List.of(role)); @@ -846,15 +830,12 @@ class ElasticSearchRBACConditionEvaluatorTest { team.setName("Engineering"); when(mockUser.getTeams()).thenReturn(List.of(team)); - // Evaluate conditions OMQueryBuilder finalQuery = evaluator.evaluateConditions(mockSubjectContext); QueryBuilder elasticQuery = ((ElasticQueryBuilder) finalQuery).build(); String generatedQuery = elasticQuery.toString(); - // Assertions DocumentContext jsonContext = JsonPath.parse(generatedQuery); - // Check for the "Table" index condition assertFieldExists( jsonContext, "$.bool.should[0].bool.must[?(@.terms._index && @.terms._index[?(@ == 'table')])]", @@ -865,14 +846,13 @@ class ElasticSearchRBACConditionEvaluatorTest { "match_all for hasAnyRole 'Admin'"); assertFieldExists( jsonContext, - "$.bool.should[0].bool.should[?(@.term['tags.tagFQN'].value=='Sensitive')]", + "$.bool.should[0].bool.must[?(@.term['tags.tagFQN'].value=='Sensitive')]", "Sensitive tag"); - // Check for the "Dashboard" index condition assertFieldExists( jsonContext, - "$.bool.should[1].bool.must[?(@.terms._index && @.terms._index[?(@ == 'database')])]", - "Index filtering for 'Database' resource"); + "$.bool.should[1].bool.must[?(@.terms._index && @.terms._index[?(@ == 'Dashboard')])]", + "Index filtering for 'Dashboard' resource"); assertFieldExists( jsonContext, "$.bool.should[1].bool.must[?(@.match_all)]", @@ -889,40 +869,32 @@ class ElasticSearchRBACConditionEvaluatorTest { @Test void testMultiplePoliciesInRole() { - // Mock multiple policies in a single role setupMockPolicies( List.of("hasDomain() && matchAnyTag('Public')", "!inAnyTeam('HR') || isOwner()"), "ALLOW", List.of(List.of("Table"), List.of("Dashboard")), List.of(List.of(MetadataOperation.VIEW_BASIC))); - // Mock user roles EntityReference role = new EntityReference(); role.setName("DataSteward"); when(mockUser.getRoles()).thenReturn(List.of(role)); - // Mock user teams EntityReference team = new EntityReference(); team.setId(UUID.randomUUID()); team.setName("Finance"); when(mockUser.getTeams()).thenReturn(List.of(team)); - // Mock user domain EntityReference domain = new EntityReference(); domain.setId(UUID.randomUUID()); when(mockUser.getDomain()).thenReturn(domain); - // Mock user ownership when(mockUser.getId()).thenReturn(UUID.randomUUID()); - // Evaluate the condition OMQueryBuilder finalQuery = evaluator.evaluateConditions(mockSubjectContext); QueryBuilder elasticQuery = ((ElasticQueryBuilder) finalQuery).build(); String generatedQuery = elasticQuery.toString(); DocumentContext jsonContext = JsonPath.parse(generatedQuery); - // Assertions - // Check for domain filtering in the "Table" index clause assertFieldExists( jsonContext, "$.bool.should[0].bool.must[?(@.term['domain.id'].value=='" @@ -930,16 +902,14 @@ class ElasticSearchRBACConditionEvaluatorTest { + "')]", "user's domain ID"); - // Check for the matchAnyTag clause for Public tag in the "Table" index clause assertFieldExists( jsonContext, - "$.bool.should[0].bool.should[?(@.term['tags.tagFQN'].value=='Public')]", + "$.bool.should[0].bool.must[?(@.term['tags.tagFQN'].value=='Public')]", "Public tag"); } @Test void testRoleAndPolicyInheritanceFromTeams() { - // Mock policies inherited through team hierarchy setupMockPolicies( List.of( "hasAnyRole('Manager') && hasDomain()", @@ -948,7 +918,6 @@ class ElasticSearchRBACConditionEvaluatorTest { List.of(List.of("All"), List.of("All")), List.of(List.of(MetadataOperation.VIEW_BASIC))); - // Mock user teams with inherited roles EntityReference team = new EntityReference(); team.setId(UUID.randomUUID()); team.setName("Engineering"); @@ -959,20 +928,16 @@ class ElasticSearchRBACConditionEvaluatorTest { when(mockUser.getRoles()) .thenReturn(List.of(inheritedRole)); // User inherits the 'Manager' role - // Mock user domain EntityReference domain = new EntityReference(); domain.setId(UUID.randomUUID()); domain.setName("Operations"); when(mockUser.getDomain()).thenReturn(domain); - // Evaluate the condition OMQueryBuilder finalQuery = evaluator.evaluateConditions(mockSubjectContext); QueryBuilder elasticQuery = ((ElasticQueryBuilder) finalQuery).build(); String generatedQuery = elasticQuery.toString(); DocumentContext jsonContext = JsonPath.parse(generatedQuery); - // Assertions - // Adjust the assertion for the hasDomain clause assertFieldExists( jsonContext, "$.bool.should[0].bool.must[?(@.term['domain.id'].value=='" @@ -980,33 +945,28 @@ class ElasticSearchRBACConditionEvaluatorTest { + "')]", "user's domain ID"); - // Check for the inAnyTeam('Engineering') clause assertFieldExists( jsonContext, "$.bool.should[1].bool.must[?(@.match_all)]", "match_all for inAnyTeam 'Engineering'"); - // Check for the matchAnyTag clause for Critical tag assertFieldExists( jsonContext, - "$.bool.should[1].bool.should[?(@.term['tags.tagFQN'].value=='Critical')]", + "$.bool.should[1].bool.must[?(@.term['tags.tagFQN'].value=='Critical')]", "Critical tag"); } @Test void testRuleWithNonViewOperationIgnored() { - // Rule with operation EditDescription, which should be ignored in search RBAC setupMockPolicies( List.of("isOwner()"), "ALLOW", List.of(List.of("All")), List.of(List.of(MetadataOperation.EDIT_DESCRIPTION))); - // Mock user ownership UUID userId = UUID.randomUUID(); when(mockUser.getId()).thenReturn(userId); - // Evaluate the condition OMQueryBuilder finalQuery = evaluator.evaluateConditions(mockSubjectContext); QueryBuilder elasticQuery = ((ElasticQueryBuilder) finalQuery).build(); String generatedQuery = elasticQuery.toString(); @@ -1015,23 +975,19 @@ class ElasticSearchRBACConditionEvaluatorTest { @Test void testRuleWithViewBasicOperationApplied() { - // Rule with operation ViewBasic, which should affect search results setupMockPolicies( List.of("isOwner()"), "ALLOW", List.of(List.of("All")), List.of(List.of(MetadataOperation.VIEW_BASIC))); - // Mock user ownership UUID userId = UUID.randomUUID(); when(mockUser.getId()).thenReturn(userId); - // Evaluate the condition OMQueryBuilder finalQuery = evaluator.evaluateConditions(mockSubjectContext); QueryBuilder elasticQuery = ((ElasticQueryBuilder) finalQuery).build(); String generatedQuery = elasticQuery.toString(); - // The rule should affect the search query assertTrue(generatedQuery.contains("owners.id"), "The query should contain 'owner.id'"); assertTrue( generatedQuery.contains(userId.toString()), "The query should contain the user's ID"); @@ -1040,7 +996,7 @@ class ElasticSearchRBACConditionEvaluatorTest { @Test void testDenyAllOperationsOnTableResource() { setupMockPolicies( - List.of(""), "DENY", List.of(List.of("Table")), List.of(List.of(MetadataOperation.ALL))); + List.of(""), "DENY", List.of(List.of("table")), List.of(List.of(MetadataOperation.ALL))); UUID userId = UUID.randomUUID(); when(mockUser.getId()).thenReturn(userId); @@ -1055,7 +1011,6 @@ class ElasticSearchRBACConditionEvaluatorTest { "$.bool.must_not[0].bool.must[?(@.terms._index && @.terms._index[?(@ == 'table')])]", "must_not clause excluding 'table'"); - // Assertions to ensure 'table' is not included in 'must' or 'should' clauses assertFieldDoesNotExist( jsonContext, "$.bool.must[?(@.terms._index && @.terms._index[?(@ == 'table')])]", @@ -1068,7 +1023,6 @@ class ElasticSearchRBACConditionEvaluatorTest { @Test void testUserSeesOwnedAndUnownedResourcesIncludingTeamOwnership() { - // Set up a policy with three rules setupMockPolicies( List.of( "noOwner()", // Rule 1 condition (ViewBasic) @@ -1151,7 +1105,6 @@ class ElasticSearchRBACConditionEvaluatorTest { "The query should include 'isOwner' or 'matchAnyTag' conditions where applicable"); } - // Helper methods private CompiledRule createCompiledRule(Map policyDef) { CompiledRule rule = mock(CompiledRule.class); when(rule.getName()).thenReturn((String) policyDef.get("name")); @@ -1227,4 +1180,75 @@ class ElasticSearchRBACConditionEvaluatorTest { return policyDefs; } + + @Test + void testAllowPoliciesWithoutMinimumShouldMatch() { + setupMockPolicies( + List.of(""), // No condition + "ALLOW", + List.of(List.of("table", "dashboard")), + List.of(List.of(MetadataOperation.VIEW_ALL))); + + setupMockPolicies( + List.of(""), // No condition + "ALLOW", + List.of(List.of("glossary", "glossaryTerm")), + List.of(List.of(MetadataOperation.VIEW_ALL))); + + OMQueryBuilder finalQuery = evaluator.evaluateConditions(mockSubjectContext); + QueryBuilder elasticQuery = ((ElasticQueryBuilder) finalQuery).build(); + String generatedQuery = elasticQuery.toString(); + DocumentContext jsonContext = JsonPath.parse(generatedQuery); + + assertFieldExists( + jsonContext, + "$.bool.should[*].bool.must[?(@.terms._index && @.terms._index[?(@ == 'table' || @ == 'dashboard')])]", + "Allow policy should include 'table' and 'dashboard' in should clause"); + + assertFieldExists( + jsonContext, + "$.bool.should[*].bool.must[?(@.terms._index && @.terms._index[?(@ == 'glossary' || @ == 'glossaryterm')])]", + "Allow policy should include 'glossary' and 'glossaryTerm' in should clause"); + } + + @Test + void testPoliciesWithDeny() { + setupMockPolicies( + List.of(""), + "ALLOW", + List.of(List.of("table", "dashboard")), + List.of(List.of(MetadataOperation.VIEW_ALL))); + + setupMockPolicies( + List.of(""), + "ALLOW", + List.of(List.of("glossary", "glossaryTerm")), + List.of(List.of(MetadataOperation.VIEW_ALL))); + + setupMockPolicies( + List.of(""), + "DENY", + List.of(List.of("glossary", "glossaryTerm")), + List.of(List.of(MetadataOperation.VIEW_ALL))); + + OMQueryBuilder finalQuery = evaluator.evaluateConditions(mockSubjectContext); + QueryBuilder elasticQuery = ((ElasticQueryBuilder) finalQuery).build(); + String generatedQuery = elasticQuery.toString(); + DocumentContext jsonContext = JsonPath.parse(generatedQuery); + + assertFieldExists( + jsonContext, + "$.bool.must[0].bool.should[*].bool.must[?(@.terms._index && @.terms._index[?(@ == 'table' || @ == 'dashboard')])]", + "Allow policy should include 'table' and 'dashboard' in should clause"); + + assertFieldExists( + jsonContext, + "$.bool.must[0].bool.should[*].bool.must[?(@.terms._index && @.terms._index[?(@ == 'glossary' || @ == 'glossaryterm')])]", + "Allow policy should include 'glossary' and 'glossaryTerm' in should clause"); + + assertFieldExists( + jsonContext, + "$.bool.must_not[*].bool.must[?(@.terms._index && @.terms._index[?(@ == 'glossary' || @ == 'glossaryterm')])]", + "Deny policy should exclude 'glossary' and 'glossaryTerm' in must_not clause"); + } } 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 index 3d3e0bb19fc..21bb4c4dc0b 100644 --- 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 @@ -69,26 +69,25 @@ class OpenSearchRBACConditionEvaluatorTest { 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"); + jsonContext, + "$.bool.must[1].bool.should[?(@.term['tags.tagFQN'].value=='Finance')]", + "Finance tag"); assertFieldExists( jsonContext, - "$.bool.should[?(@.term['tags.tagFQN'].value=='Confidential')]", + "$.bool.must[1].bool.should[?(@.term['tags.tagFQN'].value=='Confidential')]", "Confidential tag"); } @@ -96,24 +95,20 @@ class OpenSearchRBACConditionEvaluatorTest { 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( @@ -125,25 +120,21 @@ class OpenSearchRBACConditionEvaluatorTest { @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['owners.id'].value=='" + mockUser.getId().toString() + "')]", + "$.bool.must[?(@.term['owners.id'].value=='" + mockUser.getId().toString() + "')]", "owner.id"); assertFieldDoesNotExist(jsonContext, "$.bool[?(@.match_none)]", "match_none should not exist"); } @@ -154,7 +145,6 @@ class OpenSearchRBACConditionEvaluatorTest { "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)); @@ -168,7 +158,6 @@ class OpenSearchRBACConditionEvaluatorTest { 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(); @@ -184,15 +173,15 @@ class OpenSearchRBACConditionEvaluatorTest { 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')]", + "$.bool.must[1].bool.should[?(@.term['tags.tagFQN'].value=='Sensitive')]", + "Sensitive tag"); + assertFieldExists( + jsonContext, + "$.bool.must[1].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 a633b3b8cee..e39f2d05a6e 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 @@ -710,7 +710,8 @@ public final class TestUtils { 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."); + assertFalse( + (result == null || result.isEmpty()), "The query should contain '" + fieldName + "' term."); } public static void assertFieldDoesNotExist(