From 111415bda2911a96de81c3c87cea26990fe38285 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BC=A0=E4=B8=87=E4=B9=89?= Date: Wed, 16 Sep 2026 15:25:23 +0800 Subject: [PATCH 1/7] feat: register matchAnyDomain as a policy condition function --- .../policyevaluator/RuleEvaluator.java | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/security/policyevaluator/RuleEvaluator.java b/openmetadata-service/src/main/java/org/openmetadata/service/security/policyevaluator/RuleEvaluator.java index e9e3f2a182d9..b16041a31323 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/security/policyevaluator/RuleEvaluator.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/security/policyevaluator/RuleEvaluator.java @@ -247,6 +247,43 @@ public boolean noDomain() { return nullOrEmpty(resourceContext.getDomains()); } + @Function( + name = "matchAnyDomain", + input = "List of comma separated Domain fully qualified names", + description = + "Returns true if the entity being accessed belongs to at least one of the domains given as input", + examples = {"matchAnyDomain('Domain.Finance', 'Domain.Procurement')"}) + @SuppressWarnings("unused") + public boolean matchAnyDomain(String... domainFQNs) { + if (expressionValidation) { + for (String domainFQN : domainFQNs) { + validateEntityReference(Entity.DOMAIN, domainFQN); + } + return false; + } + if (resourceContext == null) { + return false; + } + List domains = resourceContext.getDomains(); + if (nullOrEmpty(domains)) { + LOG.debug("No Domains found for resource"); + return false; + } + LOG.debug( + "matchAnyDomain {} resourceDomains {}", + Arrays.toString(domainFQNs), + Arrays.toString(domains.toArray())); + for (String domainFQN : domainFQNs) { + boolean found = + domains.stream() + .anyMatch(d -> domainFQN.equals(d.getFullyQualifiedName())); + if (found) { + return true; + } + } + return false; + } + @Function( name = "matchAllTags", input = "List of comma separated tag or glossary fully qualified names", From 59c5bda242d01633890956728101a5160ddcc3c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BC=A0=E4=B8=87=E4=B9=89?= Date: Wed, 16 Sep 2026 15:25:48 +0800 Subject: [PATCH 2/7] feat: translate matchAnyDomain policy condition to search query --- .../security/RBACConditionEvaluator.java | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) 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 60b09817a814..717fbcce00e7 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 @@ -252,6 +252,10 @@ private void handleMethodReference(SpelNode node, ConditionCollector collector) hasAnyRole(roles, collector); } case "hasDomain" -> hasDomain(collector); + case "matchAnyDomain" -> { + List domainFqns = extractMethodArguments(methodRef); + matchAnyDomain(domainFqns, collector); + } case "inAnyTeam" -> { List teams = extractMethodArguments(methodRef); inAnyTeam(teams, collector); @@ -404,6 +408,20 @@ private void matchAnyServiceType(List serviceTypes, ConditionCollector c clauses.add(queryBuilderFactory.termsQuery("id.keyword", ids)); } collector.addMust(queryBuilderFactory.boolQuery().should(clauses)); + + public void matchAnyDomain(List domainFqns, ConditionCollector collector) { + List domainQueries = new ArrayList<>(); + for (String domainFqn : domainFqns) { + domainQueries.add(queryBuilderFactory.termQuery("domains.fullyQualifiedName", domainFqn)); + } + OMQueryBuilder domainQueryCombined; + if (domainQueries.size() == 1) { + domainQueryCombined = domainQueries.get(0); + } else { + domainQueryCombined = queryBuilderFactory.boolQuery().should(domainQueries); + } + collector.addMust(domainQueryCombined); + } public void matchAnyCertification( From a70f48cd1f496e560e193be90a6e38773a3607cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BC=A0=E4=B8=87=E4=B9=89?= Date: Wed, 16 Sep 2026 15:27:11 +0800 Subject: [PATCH 3/7] test: cover matchAnyDomain policy condition evaluation --- .../policyevaluator/RuleEvaluatorTest.java | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/security/policyevaluator/RuleEvaluatorTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/security/policyevaluator/RuleEvaluatorTest.java index b466974cc44c..32660ce76c0b 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/security/policyevaluator/RuleEvaluatorTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/security/policyevaluator/RuleEvaluatorTest.java @@ -482,6 +482,70 @@ void test_matchAnyTag() { assertTrue(evaluateExpression("!matchAnyTag('tag4')")); } + @Test + void test_matchAnyDomain() { + EntityReference finance = + new EntityReference() + .withId(UUID.randomUUID()) + .withName("Finance") + .withFullyQualifiedName("Domain.Finance"); + EntityReference procurement = + new EntityReference() + .withId(UUID.randomUUID()) + .withName("Procurement") + .withFullyQualifiedName("Domain.Procurement"); + + CreateResourceContext contextWithDomains = mock(CreateResourceContext.class); + Mockito.when(contextWithDomains.getDomains()).thenReturn(listOf(finance, procurement)); + StandardEvaluationContext ctx = + new StandardEvaluationContext(new RuleEvaluator(null, subjectContext, contextWithDomains)); + + // Resource belongs to Finance; both the first and the second input match + assertTrue(parseExpression("matchAnyDomain('Domain.Finance', 'Domain.Engineering')").getValue(ctx, Boolean.class)); + assertTrue(parseExpression("matchAnyDomain('Domain.Engineering', 'Domain.Procurement')").getValue(ctx, Boolean.class)); + assertTrue(parseExpression("matchAnyDomain('Domain.Finance')").getValue(ctx, Boolean.class)); + assertFalse(parseExpression("!matchAnyDomain('Domain.Finance')").getValue(ctx, Boolean.class)); + + // Domain.Finance is not present in the resource's domains + assertFalse(parseExpression("matchAnyDomain('Domain.Engineering')").getValue(ctx, Boolean.class)); + assertTrue(parseExpression("!matchAnyDomain('Domain.Engineering')").getValue(ctx, Boolean.class)); + assertFalse( + parseExpression("matchAnyDomain('Domain.Engineering', 'Domain.Marketing')") + .getValue(ctx, Boolean.class)); + + // Resource with no domains never matches + CreateResourceContext contextNoDomains = mock(CreateResourceContext.class); + Mockito.when(contextNoDomains.getDomains()).thenReturn(new ArrayList<>()); + StandardEvaluationContext emptyCtx = + new StandardEvaluationContext(new RuleEvaluator(null, subjectContext, contextNoDomains)); + assertFalse(parseExpression("matchAnyDomain('Domain.Finance')").getValue(emptyCtx, Boolean.class)); + assertTrue(parseExpression("!matchAnyDomain('Domain.Finance')").getValue(emptyCtx, Boolean.class)); + + // getDomains() returning null must not blow up + CreateResourceContext contextNullDomains = mock(CreateResourceContext.class); + Mockito.when(contextNullDomains.getDomains()).thenReturn(null); + StandardEvaluationContext nullCtx = + new StandardEvaluationContext(new RuleEvaluator(null, subjectContext, contextNullDomains)); + assertFalse(parseExpression("matchAnyDomain('Domain.Finance')").getValue(nullCtx, Boolean.class)); + + // A null resource context never matches + StandardEvaluationContext noResourceCtx = + new StandardEvaluationContext(new RuleEvaluator(null, subjectContext, null)); + assertFalse(parseExpression("matchAnyDomain('Domain.Finance')").getValue(noResourceCtx, Boolean.class)); + } + + @Test + void test_matchAnyDomain_duringExpressionValidation() { + // The validating RuleEvaluator has no resource context; the function must only resolve the + // referenced domains and return false instead of throwing or NPE-ing. + StandardEvaluationContext ctx = new StandardEvaluationContext(new RuleEvaluator(false)); + assertFalse( + parseExpression("matchAnyDomain('Domain.Finance', 'Domain.Procurement')") + .getValue(ctx, Boolean.class)); + assertTrue( + parseExpression("!matchAnyDomain('Domain.Finance')").getValue(ctx, Boolean.class)); + } + @Test void test_matchAnyCertification() { // Certification is not Present From a99c1343306dc087ae2204e3399f2e192aa782ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BC=A0=E4=B8=87=E4=B9=89?= Date: Wed, 16 Sep 2026 15:28:08 +0800 Subject: [PATCH 4/7] test: cover matchAnyDomain search query translation --- ...asticSearchRBACConditionEvaluatorTest.java | 39 +++++++++++++++++++ .../OpenSearchRBACConditionEvaluatorTest.java | 22 +++++++++++ 2 files changed, 61 insertions(+) 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 3f2111eb82be..3a15c3b4189f 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 @@ -350,6 +350,45 @@ void testHasDomain() { "The query should contain the user's domain ID."); } + @Test + void testMatchAnyDomain() { + setupMockPolicies( + "matchAnyDomain('Domain.Finance', 'Domain.Procurement')", "ALLOW"); + + OMQueryBuilder finalQuery = evaluator.evaluateConditions(mockSubjectContext); + Query elasticQuery = ((ElasticQueryBuilder) finalQuery).build(); + String generatedQuery = serializeQueryToJson(elasticQuery); + DocumentContext jsonContext = JsonPath.parse(generatedQuery); + + assertTrue( + generatedQuery.contains("domains.fullyQualifiedName"), + "The query should contain 'domains.fullyQualifiedName'."); + assertFieldExists( + jsonContext, + "$.bool.should[?(@.term['domains.fullyQualifiedName'].value=='Domain.Finance')]", + "Domain.Finance should be in a should (OR) clause"); + assertFieldExists( + jsonContext, + "$.bool.should[?(@.term['domains.fullyQualifiedName'].value=='Domain.Procurement')]", + "Domain.Procurement should be in a should (OR) clause"); + } + + @Test + void testMatchAnyDomainWithSingleDomain() { + setupMockPolicies("matchAnyDomain('Domain.Finance')", "ALLOW"); + + OMQueryBuilder finalQuery = evaluator.evaluateConditions(mockSubjectContext); + Query elasticQuery = ((ElasticQueryBuilder) finalQuery).build(); + String generatedQuery = serializeQueryToJson(elasticQuery); + + assertTrue( + generatedQuery.contains("domains.fullyQualifiedName"), + "The query should contain 'domains.fullyQualifiedName'."); + assertTrue( + generatedQuery.contains("Domain.Finance"), + "The query should contain the requested domain FQN."); + } + @Test void testHasDomainWithMultipleDomains() { setupMockPolicies("hasDomain()", "ALLOW"); 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 a0ff785e2074..a240541fb0fc 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 @@ -162,6 +162,28 @@ void testOpenSearchRoleAndDomainCheck() { "domains.id"); } + @Test + void testMatchAnyDomain() { + setupMockPolicies("matchAnyDomain('Domain.Finance', 'Domain.Procurement')", "ALLOW"); + + OMQueryBuilder finalQuery = evaluator.evaluateConditions(mockSubjectContext); + Query openSearchQuery = ((OpenSearchQueryBuilder) finalQuery).build(); + String generatedQuery = openSearchQuery.toJsonString(); + DocumentContext jsonContext = JsonPath.parse(generatedQuery); + + assertTrue( + generatedQuery.contains("domains.fullyQualifiedName"), + "The query should contain 'domains.fullyQualifiedName'."); + assertFieldExists( + jsonContext, + "$..bool.should[?(@.term['domains.fullyQualifiedName'].value=='Domain.Finance')]", + "Domain.Finance should be in a should (OR) clause"); + assertFieldExists( + jsonContext, + "$..bool.should[?(@.term['domains.fullyQualifiedName'].value=='Domain.Procurement')]", + "Domain.Procurement should be in a should (OR) clause"); + } + @Test void testHasDomainWithMultipleDomains() { setupMockPolicies("hasDomain()", "ALLOW"); From 3ed13e7e0de51113e73193b988bcdb901a990af2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BC=A0=E4=B8=87=E4=B9=89?= Date: Wed, 16 Sep 2026 15:49:23 +0800 Subject: [PATCH 5/7] style: apply google-java-format to matchAnyDomain tests --- .../policyevaluator/RuleEvaluatorTest.java | 39 ++++++++++++------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/security/policyevaluator/RuleEvaluatorTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/security/policyevaluator/RuleEvaluatorTest.java index 32660ce76c0b..522375668703 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/security/policyevaluator/RuleEvaluatorTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/security/policyevaluator/RuleEvaluatorTest.java @@ -501,14 +501,20 @@ void test_matchAnyDomain() { new StandardEvaluationContext(new RuleEvaluator(null, subjectContext, contextWithDomains)); // Resource belongs to Finance; both the first and the second input match - assertTrue(parseExpression("matchAnyDomain('Domain.Finance', 'Domain.Engineering')").getValue(ctx, Boolean.class)); - assertTrue(parseExpression("matchAnyDomain('Domain.Engineering', 'Domain.Procurement')").getValue(ctx, Boolean.class)); + assertTrue( + parseExpression("matchAnyDomain('Domain.Finance', 'Domain.Engineering')") + .getValue(ctx, Boolean.class)); + assertTrue( + parseExpression("matchAnyDomain('Domain.Engineering', 'Domain.Procurement')") + .getValue(ctx, Boolean.class)); assertTrue(parseExpression("matchAnyDomain('Domain.Finance')").getValue(ctx, Boolean.class)); assertFalse(parseExpression("!matchAnyDomain('Domain.Finance')").getValue(ctx, Boolean.class)); // Domain.Finance is not present in the resource's domains - assertFalse(parseExpression("matchAnyDomain('Domain.Engineering')").getValue(ctx, Boolean.class)); - assertTrue(parseExpression("!matchAnyDomain('Domain.Engineering')").getValue(ctx, Boolean.class)); + assertFalse( + parseExpression("matchAnyDomain('Domain.Engineering')").getValue(ctx, Boolean.class)); + assertTrue( + parseExpression("!matchAnyDomain('Domain.Engineering')").getValue(ctx, Boolean.class)); assertFalse( parseExpression("matchAnyDomain('Domain.Engineering', 'Domain.Marketing')") .getValue(ctx, Boolean.class)); @@ -518,20 +524,24 @@ void test_matchAnyDomain() { Mockito.when(contextNoDomains.getDomains()).thenReturn(new ArrayList<>()); StandardEvaluationContext emptyCtx = new StandardEvaluationContext(new RuleEvaluator(null, subjectContext, contextNoDomains)); - assertFalse(parseExpression("matchAnyDomain('Domain.Finance')").getValue(emptyCtx, Boolean.class)); - assertTrue(parseExpression("!matchAnyDomain('Domain.Finance')").getValue(emptyCtx, Boolean.class)); + assertFalse( + parseExpression("matchAnyDomain('Domain.Finance')").getValue(emptyCtx, Boolean.class)); + assertTrue( + parseExpression("!matchAnyDomain('Domain.Finance')").getValue(emptyCtx, Boolean.class)); // getDomains() returning null must not blow up CreateResourceContext contextNullDomains = mock(CreateResourceContext.class); Mockito.when(contextNullDomains.getDomains()).thenReturn(null); StandardEvaluationContext nullCtx = new StandardEvaluationContext(new RuleEvaluator(null, subjectContext, contextNullDomains)); - assertFalse(parseExpression("matchAnyDomain('Domain.Finance')").getValue(nullCtx, Boolean.class)); + assertFalse( + parseExpression("matchAnyDomain('Domain.Finance')").getValue(nullCtx, Boolean.class)); // A null resource context never matches StandardEvaluationContext noResourceCtx = new StandardEvaluationContext(new RuleEvaluator(null, subjectContext, null)); - assertFalse(parseExpression("matchAnyDomain('Domain.Finance')").getValue(noResourceCtx, Boolean.class)); + assertFalse( + parseExpression("matchAnyDomain('Domain.Finance')").getValue(noResourceCtx, Boolean.class)); } @Test @@ -542,8 +552,7 @@ void test_matchAnyDomain_duringExpressionValidation() { assertFalse( parseExpression("matchAnyDomain('Domain.Finance', 'Domain.Procurement')") .getValue(ctx, Boolean.class)); - assertTrue( - parseExpression("!matchAnyDomain('Domain.Finance')").getValue(ctx, Boolean.class)); + assertTrue(parseExpression("!matchAnyDomain('Domain.Finance')").getValue(ctx, Boolean.class)); } @Test @@ -843,8 +852,8 @@ private Boolean evaluateUserResourceCondition(User target, String condition) { * A ResourceContext built with neither an id nor a name never resolves an entity, so every * attribute a policy condition reads comes back empty - and nothing reports that it did. A tag * condition then answers the same way whether or not the tag is present, which makes a Deny fire - * on every entity in one polarity and on none in the other. This is the root cause of #31941; - * the fix is that callers holding a specific entity must pass its identity in. + * on every entity in one polarity and on none in the other. This is the root cause of #31941; the + * fix is that callers holding a specific entity must pass its identity in. */ @Test void test_bareResourceContextCannotSeeTags() { @@ -1122,13 +1131,15 @@ void test_hasDomain_edgeCases_realFQNFormat() { table.setDomains(List.of(corpSubDomain.getEntityReference())); assertTrue( evaluateExpression("hasDomain()"), - "User with '\"Corp.Domain\"' should have access to '\"Corp.Domain\".\"Sub.Division\"' resources"); + "User with '\"Corp.Domain\"' should have access to '\"Corp.Domain\".\"Sub.Division\"'" + + " resources"); user.setDomains(List.of(corpDomain.getEntityReference())); table.setDomains(List.of(corpDifferent.getEntityReference())); assertTrue( evaluateExpression("hasDomain()"), - "User with '\"Corp.Domain\"' should have access to '\"Corp.Domain\".\"Other.Division\"' resources"); + "User with '\"Corp.Domain\"' should have access to '\"Corp.Domain\".\"Other.Division\"'" + + " resources"); // Edge Case 2: Similar but different root domains Domain engineering = createDomain("Engineering", "Engineering"); From e66159839c08c001224f544af7b826dd695c3817 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BC=A0=E4=B8=87=E4=B9=89?= Date: Tue, 29 Sep 2026 16:28:18 +0800 Subject: [PATCH 6/7] fix: fail closed when matchAnyDomain/matchAnyCertification have no arguments matchAnyDomain() with an empty argument list produced an empty bool query (should: []), which Elasticsearch and OpenSearch evaluate as match_all. An ALLOW rule carrying such a condition therefore granted access to every searchable resource, diverging from the direct RuleEvaluator which returns false for the same condition. Emit no clause and mark the rule match_nothing instead, so an empty condition degrades to deny-all. Applied to matchAnyCertification as well (same pattern, currently unreachable via the SpEL signature but identical bug class). Adds regression tests on Elastic and OpenSearch asserting the generated query is match_none and contains no domain clause and no empty bool clause. --- .../security/RBACConditionEvaluator.java | 27 +++++++++------- ...asticSearchRBACConditionEvaluatorTest.java | 32 +++++++++++++++++++ .../OpenSearchRBACConditionEvaluatorTest.java | 32 +++++++++++++++++++ 3 files changed, 79 insertions(+), 12 deletions(-) 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 717fbcce00e7..02b4231a1535 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 @@ -414,13 +414,15 @@ public void matchAnyDomain(List domainFqns, ConditionCollector collector for (String domainFqn : domainFqns) { domainQueries.add(queryBuilderFactory.termQuery("domains.fullyQualifiedName", domainFqn)); } - OMQueryBuilder domainQueryCombined; - if (domainQueries.size() == 1) { - domainQueryCombined = domainQueries.get(0); - } else { - domainQueryCombined = queryBuilderFactory.boolQuery().should(domainQueries); + switch (domainQueries.size()) { + case 0 -> + // No domains requested: the condition can never match, so make the rule deny-all instead + // of adding an empty bool query (Elasticsearch/OpenSearch treat an empty bool query as + // match_all, which would turn an ALLOW rule into a full index grant). + collector.setMatchNothing(true); + case 1 -> collector.addMust(domainQueries.get(0)); + default -> collector.addMust(queryBuilderFactory.boolQuery().should(domainQueries)); } - collector.addMust(domainQueryCombined); } @@ -431,13 +433,14 @@ public void matchAnyCertification( certificationQueries.add( queryBuilderFactory.termQuery("certification.tagLabel.tagFQN", certificationLabel)); } - OMQueryBuilder certificationQueriesCombined; - if (certificationQueries.size() == 1) { - certificationQueriesCombined = certificationQueries.get(0); - } else { - certificationQueriesCombined = queryBuilderFactory.boolQuery().should(certificationQueries); + switch (certificationQueries.size()) { + case 0 -> + // No certifications requested: deny-all instead of emitting an empty bool query, which + // Elasticsearch/OpenSearch evaluate as match_all and would turn an ALLOW into a grant. + collector.setMatchNothing(true); + case 1 -> collector.addMust(certificationQueries.get(0)); + default -> collector.addMust(queryBuilderFactory.boolQuery().should(certificationQueries)); } - collector.addMust(certificationQueriesCombined); } public void isOwner(User user, ConditionCollector collector) { 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 3a15c3b4189f..95fa73683037 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 @@ -1645,4 +1645,36 @@ void testStaticNestedQueryDoesNotFailForUnmappedFields() { json.contains("\"ignore_unmapped\":true"), "must set ignore_unmapped so query works on indexes without this nested field"); } + + /** + * Regression for the empty-condition ALLOW leak: {@code matchAnyDomain()} with zero arguments used + * to emit an empty bool query, which Elasticsearch evaluates as match_all, so an ALLOW rule + * granted access to every searchable resource. The generated query must fail closed (match_none) + * and must not contain an empty must/should clause. + */ + @Test + void testMatchAnyDomainWithNoArgumentsDoesNotGrantAccess() { + setupMockPolicies("matchAnyDomain()", "ALLOW"); + + OMQueryBuilder finalQuery = evaluator.evaluateConditions(mockSubjectContext); + Query elasticQuery = ((ElasticQueryBuilder) finalQuery).build(); + String generatedQuery = serializeQueryToJson(elasticQuery); + + DocumentContext jsonContext = JsonPath.parse(generatedQuery); + // RBACConditionEvaluator fails closed by returning ConditionCollector's match_none query, which + // serializes as {"bool":{"must_not":[{"match_all":{}}]}} — a query that can never match. + assertFieldExists( + jsonContext, + "$.bool.must_not[?(@.match_all)]", + "an ALLOW rule with an empty matchAnyDomain() must fail closed to a never-matching query"); + assertFalse( + generatedQuery.contains("domains.fullyQualifiedName"), + "no domain clause should be emitted for an empty argument list"); + assertFalse( + generatedQuery.contains("\"should\":[]"), + "no empty bool clause may be emitted, it is treated as match_all by Elasticsearch"); + // Safe against PathNotFoundException: a match_none query omits `must` entirely. + assertFieldDoesNotExist( + jsonContext, "$.bool.must", "must (an empty bool is treated as match_all by Elasticsearch)"); + } } 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 a240541fb0fc..8ae82201f88e 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 @@ -448,4 +448,36 @@ void testStaticNestedQueryDoesNotFailForUnmappedFields() { json.contains("\"ignore_unmapped\":true"), "must set ignore_unmapped so query works on indexes without this nested field"); } + + /** + * Regression for the empty-condition ALLOW leak: {@code matchAnyDomain()} with zero arguments used + * to emit an empty bool query, which OpenSearch evaluates as match_all, so an ALLOW rule granted + * access to every searchable resource. The generated query must fail closed (match_none) and must + * not contain an empty must/should clause. + */ + @Test + void testMatchAnyDomainWithNoArgumentsDoesNotGrantAccess() { + setupMockPolicies("matchAnyDomain()", "ALLOW"); + + OMQueryBuilder finalQuery = evaluator.evaluateConditions(mockSubjectContext); + Query openSearchQuery = ((OpenSearchQueryBuilder) finalQuery).build(); + String generatedQuery = openSearchQuery.toJsonString(); + + DocumentContext jsonContext = JsonPath.parse(generatedQuery); + // RBACConditionEvaluator fails closed by returning ConditionCollector's match_none query, which + // serializes as {"bool":{"must_not":[{"match_all":{}}]}} — a query that can never match. + assertFieldExists( + jsonContext, + "$.bool.must_not[?(@.match_all)]", + "an ALLOW rule with an empty matchAnyDomain() must fail closed to a never-matching query"); + assertFalse( + generatedQuery.contains("domains.fullyQualifiedName"), + "no domain clause should be emitted for an empty argument list"); + assertFalse( + generatedQuery.contains("\"should\":[]"), + "no empty bool clause may be emitted, it is treated as match_all by OpenSearch"); + // Safe against PathNotFoundException: a match_none query omits `must` entirely. + assertFieldDoesNotExist( + jsonContext, "$.bool.must", "must (an empty bool is treated as match_all by OpenSearch)"); + } } From f963c0ff0847cb1b3499716684587fa497a6f348 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BC=A0=E4=B8=87=E4=B9=89?= Date: Thu, 8 Oct 2026 19:51:58 +0800 Subject: [PATCH 7/7] fix: restore missing closing brace before matchAnyDomain in RBACConditionEvaluator matchAnyDomain was inserted before the closing brace of matchAnyServiceType, so it became a nested method declaration. That is an illegal start of expression and broke compilation of openmetadata-service (CI: illegal start of expression at RBACConditionEvaluator.java:412). Close matchAnyServiceType first so matchAnyDomain is a proper class member. --- .../service/search/security/RBACConditionEvaluator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 02b4231a1535..5cbb3c4bee8f 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 @@ -408,6 +408,7 @@ private void matchAnyServiceType(List serviceTypes, ConditionCollector c clauses.add(queryBuilderFactory.termsQuery("id.keyword", ids)); } collector.addMust(queryBuilderFactory.boolQuery().should(clauses)); + } public void matchAnyDomain(List domainFqns, ConditionCollector collector) { List domainQueries = new ArrayList<>(); @@ -423,7 +424,6 @@ public void matchAnyDomain(List domainFqns, ConditionCollector collector case 1 -> collector.addMust(domainQueries.get(0)); default -> collector.addMust(queryBuilderFactory.boolQuery().should(domainQueries)); } - } public void matchAnyCertification(