diff --git a/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TestCaseCsvDomainIsolationIT.java b/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TestCaseCsvDomainIsolationIT.java new file mode 100644 index 000000000000..99f22575d58f --- /dev/null +++ b/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TestCaseCsvDomainIsolationIT.java @@ -0,0 +1,497 @@ +/* + * Copyright 2026 Collate. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.openmetadata.it.tests; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.openmetadata.common.utils.CommonUtil.listOrEmpty; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import java.time.Duration; +import java.util.ArrayDeque; +import java.util.Deque; +import java.util.List; +import java.util.Map; +import org.awaitility.Awaitility; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.parallel.Execution; +import org.junit.jupiter.api.parallel.ExecutionMode; +import org.openmetadata.it.factories.DatabaseSchemaTestFactory; +import org.openmetadata.it.factories.DatabaseServiceTestFactory; +import org.openmetadata.it.util.SdkClients; +import org.openmetadata.it.util.TestNamespace; +import org.openmetadata.it.util.TestNamespaceExtension; +import org.openmetadata.schema.api.data.CreateTable; +import org.openmetadata.schema.api.domains.CreateDomain; +import org.openmetadata.schema.api.teams.CreateUser; +import org.openmetadata.schema.entity.data.DatabaseSchema; +import org.openmetadata.schema.entity.data.Table; +import org.openmetadata.schema.entity.domains.Domain; +import org.openmetadata.schema.entity.services.DatabaseService; +import org.openmetadata.schema.entity.teams.Role; +import org.openmetadata.schema.entity.teams.User; +import org.openmetadata.schema.tests.TestCase; +import org.openmetadata.schema.tests.TestSuite; +import org.openmetadata.schema.type.ApiStatus; +import org.openmetadata.schema.type.Column; +import org.openmetadata.schema.type.ColumnDataType; +import org.openmetadata.schema.type.csv.CsvImportResult; +import org.openmetadata.schema.utils.JsonUtils; +import org.openmetadata.sdk.client.OpenMetadataClient; +import org.openmetadata.sdk.fluent.builders.TestCaseBuilder; +import org.openmetadata.sdk.models.ListParams; +import org.openmetadata.sdk.network.HttpMethod; +import org.openmetadata.sdk.network.RequestOptions; + +/** + * Integration tests for GitHub issue #28463 — the Data Quality CSV export / bulk-edit import path + * bypasses domain RBAC. + * + *

A user holding the seeded {@code DomainOnlyAccessRole} must not be able to read test cases + * belonging to tables outside their domains through {@code GET + * /v1/dataQuality/testCases/name/{name}/export}, nor create test cases on out-of-domain tables + * through {@code PUT /v1/dataQuality/testCases/name/{name}/import}. + */ +@Execution(ExecutionMode.CONCURRENT) +@ExtendWith(TestNamespaceExtension.class) +public class TestCaseCsvDomainIsolationIT { + + private static final ObjectMapper MAPPER = new ObjectMapper(); + private static final Column COLUMN = new Column().withName("id").withDataType(ColumnDataType.INT); + private static final String ROW_COUNT_TEST_DEFINITION = "tableRowCountToEqual"; + private static final String TARGET_ENTITY_TYPE_TABLE = "table"; + private static final String PLATFORM_WIDE_EXPORT = "*"; + private static final String DOMAIN_ONLY_ACCESS_ROLE = "DomainOnlyAccessRole"; + + @Test + void test_exportCsv_domainRestrictedUserCannotReadForeignDomainTestCases(TestNamespace ns) { + OpenMetadataClient admin = SdkClients.adminClient(); + Deque cleanup = new ArrayDeque<>(); + try { + String shortId = ns.uniqueShortId(); + Domain ownDomain = createDomain(admin, "d1_" + shortId, cleanup); + Domain foreignDomain = createDomain(admin, "d2_" + shortId, cleanup); + DatabaseSchema schema = createSchema(ns, shortId, cleanup); + Table ownTable = createTable(admin, "t1_" + shortId, schema, ownDomain, cleanup); + Table foreignTable = createTable(admin, "t2_" + shortId, schema, foreignDomain, cleanup); + + TestCase ownTestCase = createTestCase(admin, "tc1_" + shortId, ownTable); + TestCase foreignTestCase = createTestCase(admin, "tc2_" + shortId, foreignTable); + + OpenMetadataClient restricted = + createRestrictedUserClient(admin, shortId, ownDomain, cleanup); + + String platformWideCsv = restricted.testCases().exportCsv(PLATFORM_WIDE_EXPORT); + assertTrue( + platformWideCsv.contains(ownTestCase.getName()), + "Domain-restricted user must still export their own-domain test case"); + assertFalse( + platformWideCsv.contains(foreignTestCase.getName()), + "Platform-wide export must NOT leak a foreign-domain test case"); + + String foreignTableCsv = + restricted.testCases().exportCsv(foreignTable.getFullyQualifiedName()); + assertFalse( + foreignTableCsv.contains(foreignTestCase.getName()), + "Table-scoped export of a foreign-domain table must NOT leak its test cases"); + } finally { + drain(cleanup); + } + } + + @Test + void test_importCsv_domainRestrictedUserCannotWriteToForeignDomainTable(TestNamespace ns) { + OpenMetadataClient admin = SdkClients.adminClient(); + Deque cleanup = new ArrayDeque<>(); + try { + String shortId = ns.uniqueShortId(); + Domain ownDomain = createDomain(admin, "d1_" + shortId, cleanup); + Domain foreignDomain = createDomain(admin, "d2_" + shortId, cleanup); + DatabaseSchema schema = createSchema(ns, shortId, cleanup); + Table ownTable = createTable(admin, "t1_" + shortId, schema, ownDomain, cleanup); + Table foreignTable = createTable(admin, "t2_" + shortId, schema, foreignDomain, cleanup); + + String seedName = "seed_" + shortId; + createTestCase(admin, seedName, foreignTable); + String seededCsv = admin.testCases().exportCsv(foreignTable.getFullyQualifiedName()); + + String injectedName = "evil_" + shortId; + String maliciousCsv = seededCsv.replace(seedName, injectedName); + assertNotEquals( + seededCsv, maliciousCsv, "Test setup must produce a CSV row for a new test case name"); + + OpenMetadataClient restricted = + createRestrictedUserClient(admin, shortId, ownDomain, cleanup); + + // The path name is the user's OWN in-domain table — only the CSV rows point elsewhere. + CsvImportResult result = importAs(restricted, ownTable.getFullyQualifiedName(), maliciousCsv); + + assertNotEquals( + ApiStatus.SUCCESS, + result.getStatus(), + "Import of an out-of-domain row must not report success: " + + result.getImportResultsCsv()); + assertFalse( + testCaseNamesFor(admin, foreignTable).contains(injectedName), + "Domain-restricted user must NOT create a test case on a foreign-domain table"); + } finally { + drain(cleanup); + } + } + + /** + * Pins the entityFQN (column 4) gate on its own. The sibling test's CSV is exported verbatim from + * the foreign table, so its testSuite column also names a foreign suite and the row would be + * rejected by the suite gate even if the target gate were removed. Leaving column 5 empty here + * means only the target gate can reject the row. + */ + @Test + void test_importCsv_foreignTargetIsRejectedEvenWhenNoTestSuiteIsNamed(TestNamespace ns) { + OpenMetadataClient admin = SdkClients.adminClient(); + Deque cleanup = new ArrayDeque<>(); + try { + String shortId = ns.uniqueShortId(); + Domain ownDomain = createDomain(admin, "d1_" + shortId, cleanup); + Domain foreignDomain = createDomain(admin, "d2_" + shortId, cleanup); + DatabaseSchema schema = createSchema(ns, shortId, cleanup); + Table ownTable = createTable(admin, "t1_" + shortId, schema, ownDomain, cleanup); + Table foreignTable = createTable(admin, "t2_" + shortId, schema, foreignDomain, cleanup); + + String seedName = "seed_" + shortId; + createTestCase(admin, seedName, foreignTable); + + String injectedName = "evil2_" + shortId; + String maliciousCsv = + admin + .testCases() + .exportCsv(foreignTable.getFullyQualifiedName()) + .replace(basicSuiteFqn(foreignTable), "") + .replace(seedName, injectedName); + assertFalse( + maliciousCsv.contains(basicSuiteFqn(foreignTable)), + "Test setup must leave the testSuite column empty so only the target gate can reject"); + + OpenMetadataClient restricted = + createRestrictedUserClient(admin, shortId, ownDomain, cleanup); + CsvImportResult result = importAs(restricted, ownTable.getFullyQualifiedName(), maliciousCsv); + + assertNotEquals( + ApiStatus.SUCCESS, + result.getStatus(), + "A row targeting a foreign-domain table must not succeed: " + + result.getImportResultsCsv()); + assertFalse( + testCaseNamesFor(admin, foreignTable).contains(injectedName), + "Domain-restricted user must NOT create a test case on a foreign-domain table"); + } finally { + drain(cleanup); + } + } + + @Test + void test_importCsvAsync_domainRestrictedUserCannotVersionForeignDomainTable(TestNamespace ns) { + OpenMetadataClient admin = SdkClients.adminClient(); + Deque cleanup = new ArrayDeque<>(); + try { + String shortId = ns.uniqueShortId(); + Domain ownDomain = createDomain(admin, "d1_" + shortId, cleanup); + Domain foreignDomain = createDomain(admin, "d2_" + shortId, cleanup); + DatabaseSchema schema = createSchema(ns, shortId, cleanup); + Table ownTable = createTable(admin, "t1_" + shortId, schema, ownDomain, cleanup); + Table foreignTable = createTable(admin, "t2_" + shortId, schema, foreignDomain, cleanup); + + // Every row is legitimately the attacker's own; the attack is in the request PATH, which + // becomes the bulk-import versioning target. Two rows are required: createBulkImportVersion + // only versions when numberOfRowsProcessed > 1. + String firstSeed = "seeda_" + shortId; + String secondSeed = "seedb_" + shortId; + createTestCase(admin, firstSeed, ownTable); + createTestCase(admin, secondSeed, ownTable); + String ownRowsCsv = + admin + .testCases() + .exportCsv(ownTable.getFullyQualifiedName()) + .replace(firstSeed, "async1_" + shortId) + .replace(secondSeed, "async2_" + shortId); + + Table foreignBefore = admin.tables().get(foreignTable.getId()); + OpenMetadataClient restricted = + createRestrictedUserClient(admin, shortId, ownDomain, cleanup); + + restricted + .testCases() + .importCsvAsync( + foreignTable.getFullyQualifiedName(), ownRowsCsv, false, TARGET_ENTITY_TYPE_TABLE); + + // The job is asynchronous: wait for the imported rows to land, which is the point after which + // the versioning step would already have run. + Awaitility.await("async import applied") + .atMost(Duration.ofSeconds(60)) + .pollInterval(Duration.ofSeconds(1)) + .until( + () -> { + List names = testCaseNamesFor(admin, ownTable); + return names.contains("async1_" + shortId) && names.contains("async2_" + shortId); + }); + + Table foreignAfter = admin.tables().get(foreignTable.getId()); + assertEquals( + foreignBefore.getVersion(), + foreignAfter.getVersion(), + "A foreign-domain table must not be versioned by a domain-restricted user's async import"); + assertEquals( + foreignBefore.getUpdatedBy(), + foreignAfter.getUpdatedBy(), + "A foreign-domain table's updatedBy must not be overwritten by a foreign importer"); + } finally { + drain(cleanup); + } + } + + @Test + void test_importCsv_domainRestrictedUserCannotAttachToForeignDomainTestSuite(TestNamespace ns) { + OpenMetadataClient admin = SdkClients.adminClient(); + Deque cleanup = new ArrayDeque<>(); + try { + String shortId = ns.uniqueShortId(); + Domain ownDomain = createDomain(admin, "d1_" + shortId, cleanup); + Domain foreignDomain = createDomain(admin, "d2_" + shortId, cleanup); + DatabaseSchema schema = createSchema(ns, shortId, cleanup); + Table ownTable = createTable(admin, "t1_" + shortId, schema, ownDomain, cleanup); + Table foreignTable = createTable(admin, "t2_" + shortId, schema, foreignDomain, cleanup); + + // Both basic suites must really exist, otherwise the import would be refused merely as + // "test suite not found" and the domain gate would never be exercised. + String seedName = "seed_" + shortId; + createTestCase(admin, seedName, ownTable); + createTestCase(admin, "seed2_" + shortId, foreignTable); + String foreignSuiteFqn = basicSuiteFqn(foreignTable); + TestSuite foreignSuiteBefore = admin.testSuites().getByName(foreignSuiteFqn); + + // Row targets the user's OWN table (passes the entityFQN gate) but names the FOREIGN + // domain's basic suite in the testSuite column. + String injectedName = "suiteevil_" + shortId; + String maliciousCsv = + admin + .testCases() + .exportCsv(ownTable.getFullyQualifiedName()) + .replace(basicSuiteFqn(ownTable), foreignSuiteFqn) + .replace(seedName, injectedName); + + OpenMetadataClient restricted = + createRestrictedUserClient(admin, shortId, ownDomain, cleanup); + CsvImportResult result = importAs(restricted, ownTable.getFullyQualifiedName(), maliciousCsv); + + assertNotEquals( + ApiStatus.SUCCESS, + result.getStatus(), + "Import naming a foreign-domain test suite must not succeed: " + + result.getImportResultsCsv()); + assertFalse( + testCaseNamesFor(admin, ownTable).contains(injectedName), + "The row must be rejected outright, not written with a foreign-domain suite"); + assertEquals( + foreignSuiteBefore.getVersion(), + admin.testSuites().getByName(foreignSuiteFqn).getVersion(), + "A foreign-domain test suite must not be mutated by a domain-restricted user's import"); + } finally { + drain(cleanup); + } + } + + @Test + void test_importCsv_domainRestrictedUserCanStillWriteToOwnDomainTable(TestNamespace ns) { + OpenMetadataClient admin = SdkClients.adminClient(); + Deque cleanup = new ArrayDeque<>(); + try { + String shortId = ns.uniqueShortId(); + Domain ownDomain = createDomain(admin, "d1_" + shortId, cleanup); + DatabaseSchema schema = createSchema(ns, shortId, cleanup); + Table ownTable = createTable(admin, "t1_" + shortId, schema, ownDomain, cleanup); + + String seedName = "seed_" + shortId; + createTestCase(admin, seedName, ownTable); + String seededCsv = admin.testCases().exportCsv(ownTable.getFullyQualifiedName()); + + String addedName = "added_" + shortId; + String csv = seededCsv.replace(seedName, addedName); + + OpenMetadataClient restricted = + createRestrictedUserClient(admin, shortId, ownDomain, cleanup); + CsvImportResult result = importAs(restricted, ownTable.getFullyQualifiedName(), csv); + + assertNotEquals( + ApiStatus.ABORTED, + result.getStatus(), + "In-domain import must not be aborted: " + result.getImportResultsCsv()); + assertTrue( + testCaseNamesFor(admin, ownTable).contains(addedName), + "Domain-restricted user must still import test cases on their own-domain table"); + } finally { + drain(cleanup); + } + } + + private String basicSuiteFqn(Table table) { + return table.getFullyQualifiedName() + ".testSuite"; + } + + private CsvImportResult importAs(OpenMetadataClient client, String tableFqn, String csv) { + return JsonUtils.readValue( + client.testCases().importCsv(tableFqn, csv, false, TARGET_ENTITY_TYPE_TABLE), + CsvImportResult.class); + } + + private List testCaseNamesFor(OpenMetadataClient admin, Table table) { + ListParams params = + new ListParams() + .setLimit(1000) + .addFilter( + "entityLink", String.format("<#E::table::%s>", table.getFullyQualifiedName())) + .addFilter("includeAllTests", "true"); + return admin.testCases().list(params).getData().stream().map(TestCase::getName).toList(); + } + + private TestCase createTestCase(OpenMetadataClient admin, String name, Table table) { + return TestCaseBuilder.create(admin) + .name(name) + .forTable(table) + .testDefinition(ROW_COUNT_TEST_DEFINITION) + .parameter("value", "100") + .create(); + } + + private Domain createDomain(OpenMetadataClient admin, String name, Deque cleanup) { + CreateDomain create = + new CreateDomain() + .withName(name) + .withDomainType(CreateDomain.DomainType.AGGREGATE) + .withDescription("Test case CSV domain isolation"); + Domain domain = admin.domains().create(create); + cleanup.push(() -> admin.domains().delete(domain.getId().toString())); + return domain; + } + + private DatabaseSchema createSchema(TestNamespace ns, String shortId, Deque cleanup) { + DatabaseService service = + DatabaseServiceTestFactory.createPostgresWithName("svc_" + shortId, ns); + cleanup.push( + () -> + SdkClients.adminClient() + .databaseServices() + .delete( + service.getId().toString(), Map.of("recursive", "true", "hardDelete", "true"))); + return DatabaseSchemaTestFactory.createSimpleWithName("s_" + shortId, ns, service); + } + + private Table createTable( + OpenMetadataClient admin, + String name, + DatabaseSchema schema, + Domain domain, + Deque cleanup) { + CreateTable create = + new CreateTable() + .withName(name) + .withDatabaseSchema(schema.getFullyQualifiedName()) + .withColumns(List.of(COLUMN)) + .withDomains(List.of(domain.getFullyQualifiedName())); + Table table = admin.tables().create(create); + cleanup.push(() -> admin.tables().delete(table.getId())); + return table; + } + + private OpenMetadataClient createRestrictedUserClient( + OpenMetadataClient admin, String shortId, Domain allowedDomain, Deque cleanup) { + Role domainOnlyRole = admin.roles().getByName(DOMAIN_ONLY_ACCESS_ROLE); + String name = "u_" + shortId; + String email = name + "@test.openmetadata.org"; + CreateUser request = + new CreateUser() + .withName(name) + .withEmail(email) + .withDomains(List.of(allowedDomain.getFullyQualifiedName())) + .withRoles(List.of(domainOnlyRole.getId())); + User user = admin.users().create(request); + cleanup.push(() -> admin.users().delete(user.getId())); + // Precondition: without the role and the domain actually attached, every domain assertion in + // this class would pass vacuously. Fail loudly here instead. + User stored = admin.users().get(user.getId().toString(), "roles,domains"); + assertTrue( + listOrEmpty(stored.getRoles()).stream() + .anyMatch(role -> DOMAIN_ONLY_ACCESS_ROLE.equals(role.getName())), + "Restricted user must carry DomainOnlyAccessRole, got: " + stored.getRoles()); + assertTrue( + listOrEmpty(stored.getDomains()).stream() + .anyMatch(d -> allowedDomain.getId().equals(d.getId())), + "Restricted user must carry the allowed domain, got: " + stored.getDomains()); + OpenMetadataClient client = SdkClients.createClient(email, email, new String[] {}); + awaitDomainRestrictionActive(client, allowedDomain); + return client; + } + + /** + * Blocks until the server resolves this principal as domain-restricted, then returns. + * + *

This is a stabilisation wait of unknown cause. An earlier run of this class saw the domain + * assertions fail as though no narrowing had been applied; the wait made it reproducibly stable + * and the cause was never established. It cannot mask a real regression: it asserts nothing, and + * on timeout it fails the test loudly rather than letting an assertion pass vacuously — both + * guard-neutralisation probes still go RED with it in place. + * + *

Domain listing is narrowed by the same SubjectContext the CSV paths use, so it is a usable + * readiness signal: an unrestricted view would include the other tests' domains. + */ + private void awaitDomainRestrictionActive(OpenMetadataClient client, Domain allowedDomain) { + Awaitility.await("domain restriction active") + .atMost(Duration.ofSeconds(60)) + .pollInterval(Duration.ofSeconds(1)) + .ignoreExceptions() + .until(() -> onlyOwnDomainsVisible(client, allowedDomain)); + } + + private boolean onlyOwnDomainsVisible(OpenMetadataClient client, Domain allowedDomain) + throws Exception { + String response = + client + .getHttpClient() + .executeForString( + HttpMethod.GET, "/v1/domains?limit=1000", null, RequestOptions.builder().build()); + JsonNode data = MAPPER.readTree(response).path("data"); + boolean sawOwn = false; + for (JsonNode domain : data) { + String fqn = domain.path("fullyQualifiedName").asText(""); + if (fqn.equals(allowedDomain.getFullyQualifiedName())) { + sawOwn = true; + } else if (!fqn.startsWith(allowedDomain.getFullyQualifiedName() + ".")) { + return false; + } + } + return sawOwn; + } + + private void drain(Deque cleanup) { + while (!cleanup.isEmpty()) { + try { + cleanup.pop().run(); + } catch (Exception ignored) { + // Best-effort teardown; concurrent namespaces keep tests isolated regardless. + } + } + } +} diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/csv/CsvImportExportJobHandler.java b/openmetadata-service/src/main/java/org/openmetadata/service/csv/CsvImportExportJobHandler.java index ee428bd38fa4..d93585b0f971 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/csv/CsvImportExportJobHandler.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/csv/CsvImportExportJobHandler.java @@ -27,7 +27,6 @@ import org.openmetadata.schema.jobs.BackgroundJob; import org.openmetadata.schema.search.SearchRequest; import org.openmetadata.schema.type.ApiStatus; -import org.openmetadata.schema.type.Include; import org.openmetadata.schema.type.csv.CsvImportResult; import org.openmetadata.schema.utils.JsonUtils; import org.openmetadata.service.Entity; @@ -37,11 +36,11 @@ import org.openmetadata.service.jobs.JobHandler; import org.openmetadata.service.search.SearchRepository; import org.openmetadata.service.search.SearchResultCsvExporter; +import org.openmetadata.service.security.policyevaluator.DomainAccessFilter; import org.openmetadata.service.security.policyevaluator.SubjectContext; import org.openmetadata.service.socket.WebSocketManager; import org.openmetadata.service.util.CSVExportMessage; import org.openmetadata.service.util.CSVImportMessage; -import org.openmetadata.service.util.EntityUtil.Fields; import org.openmetadata.service.util.FullyQualifiedName; @Slf4j @@ -261,15 +260,12 @@ private void createBulkImportVersion( if (!versioningRepo.supportsBulkImportVersioning()) { return; } - versioningRepo.createChangeEventForBulkOperation( - versioningRepo.getByName( - null, - args.getTargetFqn(), - new Fields(versioningRepo.getAllowedFields(), ""), - Include.NON_DELETED, - false), - result, - updatedBy); + EntityInterface versionedEntity = + DomainAccessFilter.resolveAccessibleVersioningTarget( + versioningRepo, args.getTargetFqn(), updatedBy); + if (versionedEntity != null) { + versioningRepo.createChangeEventForBulkOperation(versionedEntity, result, updatedBy); + } } private void handleCancellation( diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseRepository.java index 906b9eaadda2..fe535833f8f9 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseRepository.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseRepository.java @@ -10,6 +10,7 @@ import static org.openmetadata.schema.type.Include.ALL; import static org.openmetadata.schema.type.Include.NON_DELETED; import static org.openmetadata.service.Entity.FIELD_DATA_PRODUCTS; +import static org.openmetadata.service.Entity.FIELD_DOMAINS; import static org.openmetadata.service.Entity.FIELD_FOLLOWERS; import static org.openmetadata.service.Entity.FIELD_OWNERS; import static org.openmetadata.service.Entity.FIELD_REVIEWERS; @@ -112,6 +113,8 @@ import org.openmetadata.service.search.SearchSortFilter; import org.openmetadata.service.search.vector.TestCaseBodyTextContributor; import org.openmetadata.service.security.AuthorizationException; +import org.openmetadata.service.security.policyevaluator.DomainAccessFilter; +import org.openmetadata.service.security.policyevaluator.SubjectContext; import org.openmetadata.service.util.AsyncService; import org.openmetadata.service.util.EntityUtil; import org.openmetadata.service.util.EntityUtil.Fields; @@ -129,6 +132,17 @@ public class TestCaseRepository extends EntityRepository { "owners,entityLink,testSuite,testSuites,testDefinition,dimensionColumns,topDimensions"; private static final String PATCH_FIELDS = "owners,entityLink,testSuite,testSuites,testDefinition,computePassedFailedRowCount,useDynamicAssertion,dimensionColumns,topDimensions"; + private static final String PLATFORM_WIDE_EXPORT = "*"; + // `domains` is required so the CSV paths can post-filter on the domain each test case inherits + // from its linked table — test cases never materialize a domain relationship of their own. + private static final String EXPORT_FIELDS = "testDefinition,testSuite,domains"; + private static final String PLATFORM_WIDE_EXPORT_FIELDS = + "testDefinition,testSuite,dataProducts,domains"; + private static final String OUT_OF_DOMAIN_MESSAGE = + "Entity '%s' is outside the domains you have access to"; + // One message for "absent" and for "outside your domains" — telling them apart would turn the + // row detail into a cross-domain existence oracle. + private static final String TEST_SUITE_UNAVAILABLE_MESSAGE = "Test suite '%s' not found"; public static final String FAILED_ROWS_SAMPLE_EXTENSION = "testCase.failedRowsSample"; public static final String TEST_SUITES_REVISION_EXTENSION = "internal.testCase.testSuitesRevision"; @@ -1978,7 +1992,9 @@ public String exportToCsv( String name, String user, boolean recursive, CsvExportProgressCallback callback) throws IOException { List testCases = getTestCasesForExport(name, recursive); - return new TestCaseCsv(user, null).exportCsv(testCases, callback); + List visibleTestCases = + DomainAccessFilter.retainAccessible(testCases, DomainAccessFilter.resolveSubject(user)); + return new TestCaseCsv(user, null).exportCsv(visibleTestCases, callback); } @Override @@ -2011,10 +2027,7 @@ public CsvImportResult importFromCsv( boolean recursive, String targetEntityType) throws IOException { - TestSuite targetBundleSuite = - TEST_SUITE.equals(targetEntityType) - ? Entity.getEntityByName(TEST_SUITE, name, "", Include.ALL) - : null; + TestSuite targetBundleSuite = resolveTargetBundleSuite(name, targetEntityType, user); return new TestCaseCsv(user, targetBundleSuite).importCsv(csv, dryRun); } @@ -2028,23 +2041,39 @@ public CsvImportResult importFromCsv( String targetEntityType, CsvImportProgressCallback callback) throws IOException { - TestSuite targetBundleSuite = - TEST_SUITE.equals(targetEntityType) - ? Entity.getEntityByName(TEST_SUITE, name, "", Include.ALL) - : null; + TestSuite targetBundleSuite = resolveTargetBundleSuite(name, targetEntityType, user); return new TestCaseCsv(user, targetBundleSuite).importCsv(csv, dryRun); } + /** + * Resolves the Bundle Suite the imported test cases get attached to, rejecting a suite outside the + * importing user's domains — attaching test cases to it is a write the domain policy must gate. + */ + private TestSuite resolveTargetBundleSuite(String name, String targetEntityType, String user) { + TestSuite targetBundleSuite = null; + if (TEST_SUITE.equals(targetEntityType)) { + // Unlike the per-row suite, this one distinguishes 403 (exists, foreign) from 404 (absent). + // Nothing is written yet, so refusing loudly is the useful behaviour, and the bit it reveals + // is already available from GET /v1/dataQuality/testSuites/name/{fqn}. + targetBundleSuite = Entity.getEntityByName(TEST_SUITE, name, FIELD_DOMAINS, Include.ALL); + SubjectContext subjectContext = DomainAccessFilter.resolveSubject(user); + if (!DomainAccessFilter.isAccessible(subjectContext, targetBundleSuite.getDomains())) { + throw new AuthorizationException( + String.format(OUT_OF_DOMAIN_MESSAGE, targetBundleSuite.getFullyQualifiedName())); + } + } + return targetBundleSuite; + } + private List getTestCasesForExport(String name, boolean recursive) { // The name parameter can be: // 1. A table FQN - export test cases for that table // 2. A test suite FQN - export test cases in that test suite // 3. "*" - export all test cases (platform-wide) - if ("*".equals(name)) { + if (PLATFORM_WIDE_EXPORT.equals(name)) { // Platform-wide export - return listAll( - new Fields(allowedFields, "testDefinition,testSuite, dataProducts"), new ListFilter()); + return listAll(new Fields(allowedFields, PLATFORM_WIDE_EXPORT_FIELDS), new ListFilter()); } // Try to determine if name is a table or test suite @@ -2071,13 +2100,13 @@ private List getTestCasesForTable(String tableFqn) { ListFilter filter = new ListFilter(ALL); filter.addQueryParam("entityFQN", tableFqn); filter.addQueryParam("includeAllTests", "true"); - return new ArrayList<>(listAll(new Fields(allowedFields, "testDefinition,testSuite"), filter)); + return new ArrayList<>(listAll(new Fields(allowedFields, EXPORT_FIELDS), filter)); } private List getTestCasesForTestSuite(UUID testSuiteId) { ListFilter filter = new ListFilter(Include.NON_DELETED); filter.addQueryParam("testSuiteId", testSuiteId.toString()); - return listAll(new Fields(allowedFields, "testDefinition,testSuite"), filter); + return listAll(new Fields(allowedFields, EXPORT_FIELDS), filter); } public static class TestCaseCsv extends EntityCsv { @@ -2088,10 +2117,12 @@ public static class TestCaseCsv extends EntityCsv { private final Map importedTestSuiteIds = new HashMap<>(); private final EntityRepository versioningRepo = (EntityRepository) Entity.getEntityRepository(TEST_SUITE); + private final SubjectContext subjectContext; TestCaseCsv(String user, TestSuite targetBundleSuite) { super(TEST_CASE, HEADERS, user); this.targetBundleSuite = targetBundleSuite; + this.subjectContext = DomainAccessFilter.resolveSubject(user); } @Override @@ -2120,6 +2151,12 @@ protected void createEntity(CSVPrinter printer, List csvRecords) thro // Convert entityFQN to EntityLink String entityLink = convertFQNToEntityLink(entityFQN); + // Rows may target any entity, not just the one the endpoint was authorized against, so + // the domain policy has to be enforced per row before anything is created or updated. + if (rejectIfTargetOutOfDomain(printer, csvRecord, entityFQN, entityLink)) { + continue; + } + // Get test definition TestDefinition testDefinition = Entity.getEntityByName(TEST_DEFINITION, testDefinitionFqn, "", Include.NON_DELETED); @@ -2163,18 +2200,12 @@ protected void createEntity(CSVPrinter printer, List csvRecords) thro // Get test suite if provided, otherwise get or create default test suite if (testSuiteFqn != null && !testSuiteFqn.trim().isEmpty()) { - try { - TestSuite testSuite = - Entity.getEntityByName(TEST_SUITE, testSuiteFqn, "", Include.NON_DELETED); - testCase.withTestSuite(testSuite.getEntityReference()); - importedTestSuiteIds.putIfAbsent( - testSuite.getFullyQualifiedName(), testSuite.getId()); - } catch (EntityNotFoundException e) { - importFailure( - printer, String.format("Test suite '%s' not found", testSuiteFqn), csvRecord); - importResult.withStatus(ApiStatus.ABORTED); + TestSuite testSuite = resolveAccessibleTestSuite(printer, csvRecord, testSuiteFqn); + if (testSuite == null) { continue; } + testCase.withTestSuite(testSuite.getEntityReference()); + importedTestSuiteIds.putIfAbsent(testSuite.getFullyQualifiedName(), testSuite.getId()); } else { // No test suite provided - get or create the default basic test suite EntityReference testSuite = repository.getOrCreateTestSuite(testCase); @@ -2276,6 +2307,64 @@ protected void createEntity(CSVPrinter printer, List csvRecords) thro } } + /** + * Rejects a row whose test case would land on an entity outside the importing user's domains, + * returning true when the row was rejected. The row is reported as a failure rather than + * aborting the whole import: rows are applied one by one and are not rolled back, so aborting + * would misreport the rows already written. + * + *

The lookup is per row rather than batched because the row's target is only known once the + * row is parsed; it reads through the 30-second entity cache, so a CSV repeating a table costs + * one query, not one per row. + */ + private boolean rejectIfTargetOutOfDomain( + CSVPrinter printer, CSVRecord csvRecord, String entityFQN, String entityLink) + throws IOException { + boolean outOfDomain = false; + if (DomainAccessFilter.shouldApply(subjectContext)) { + EntityInterface target = + Entity.getEntity(EntityLink.parse(entityLink), FIELD_DOMAINS, Include.NON_DELETED); + outOfDomain = !subjectContext.hasDomains(target.getDomains()); + } + if (outOfDomain) { + importFailure(printer, String.format(OUT_OF_DOMAIN_MESSAGE, entityFQN), csvRecord); + importResult.withStatus(ApiStatus.FAILURE); + } + return outOfDomain; + } + + /** + * Resolves the test suite named by the row, or reports the row as failed and returns null. The + * CSV-supplied suite needs the same domain gate as the row's target entity: attaching a test + * case adds a CONTAINS relationship from the suite and bumps its version, so an + * attacker-controlled column must not reach a suite outside the importing user's domains. + * + *

"Does not exist" and "outside your domains" deliberately take the identical branch, so the + * row detail is the same for both and the response cannot be used as a cross-domain existence + * oracle. (The status set here is advisory only — {@code EntityCsv.setFinalStatus} recomputes the + * reported status from the row counts — but taking one branch keeps the two indistinguishable + * regardless of what any future caller does with it.) + */ + private TestSuite resolveAccessibleTestSuite( + CSVPrinter printer, CSVRecord csvRecord, String testSuiteFqn) throws IOException { + TestSuite testSuite = null; + try { + TestSuite candidate = + Entity.getEntityByName(TEST_SUITE, testSuiteFqn, FIELD_DOMAINS, Include.NON_DELETED); + if (DomainAccessFilter.isAccessible(subjectContext, candidate.getDomains())) { + testSuite = candidate; + } + } catch (EntityNotFoundException e) { + LOG.debug("Test suite '{}' named in the imported CSV does not exist", testSuiteFqn); + } + if (testSuite == null) { + importFailure( + printer, String.format(TEST_SUITE_UNAVAILABLE_MESSAGE, testSuiteFqn), csvRecord); + importResult.withStatus(ApiStatus.ABORTED); + } + return testSuite; + } + @Override protected void addRecord(CsvFile csvFile, TestCase testCase) { // Headers: name, displayName, description, testDefinition, entityFQN, testSuite, diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/resources/EntityResource.java b/openmetadata-service/src/main/java/org/openmetadata/service/resources/EntityResource.java index ce3f720528fb..9aac8bb9c09d 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/resources/EntityResource.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/resources/EntityResource.java @@ -96,6 +96,7 @@ import org.openmetadata.service.security.Authorizer; import org.openmetadata.service.security.ImpersonationContext; import org.openmetadata.service.security.policyevaluator.CreateResourceContext; +import org.openmetadata.service.security.policyevaluator.DomainAccessFilter; import org.openmetadata.service.security.policyevaluator.OperationContext; import org.openmetadata.service.security.policyevaluator.ResourceContext; import org.openmetadata.service.security.policyevaluator.ResourceContextInterface; @@ -1172,15 +1173,12 @@ protected void processChangeEventForBulkImport( SecurityContext securityContext, String name, CsvImportResult result) { - versioningRepo.createChangeEventForBulkOperation( - versioningRepo.getByName( - uriInfo, - name, - new Fields(versioningRepo.getAllowedFields(), ""), - Include.NON_DELETED, - false), - result, - securityContext.getUserPrincipal().getName()); + String updatedBy = securityContext.getUserPrincipal().getName(); + EntityInterface versionedEntity = + DomainAccessFilter.resolveAccessibleVersioningTarget(versioningRepo, name, updatedBy); + if (versionedEntity != null) { + versioningRepo.createChangeEventForBulkOperation(versionedEntity, result, updatedBy); + } } protected ResourceContext getResourceContext() { diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/search/lineage/LineageDomainFilter.java b/openmetadata-service/src/main/java/org/openmetadata/service/search/lineage/LineageDomainFilter.java index f24f12a45f8c..60b158e7f2ad 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/search/lineage/LineageDomainFilter.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/search/lineage/LineageDomainFilter.java @@ -30,6 +30,7 @@ import org.openmetadata.schema.api.lineage.SearchLineageResult; import org.openmetadata.schema.type.EntityReference; import org.openmetadata.schema.type.lineage.NodeInformation; +import org.openmetadata.service.security.policyevaluator.DomainAccessFilter; import org.openmetadata.service.security.policyevaluator.SubjectContext; /** @@ -47,10 +48,7 @@ public final class LineageDomainFilter { private LineageDomainFilter() {} public static boolean shouldApply(SubjectContext subjectContext) { - return subjectContext != null - && !subjectContext.isAdmin() - && !subjectContext.isBot() - && subjectContext.hasDomainOnlyAccessRole(); + return DomainAccessFilter.shouldApply(subjectContext); } /** diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/security/policyevaluator/DomainAccessFilter.java b/openmetadata-service/src/main/java/org/openmetadata/service/security/policyevaluator/DomainAccessFilter.java new file mode 100644 index 000000000000..aa2767df93d7 --- /dev/null +++ b/openmetadata-service/src/main/java/org/openmetadata/service/security/policyevaluator/DomainAccessFilter.java @@ -0,0 +1,145 @@ +/* + * Copyright 2026 Collate. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.openmetadata.service.security.policyevaluator; + +import static org.openmetadata.common.utils.CommonUtil.listOrEmpty; +import static org.openmetadata.common.utils.CommonUtil.nullOrEmpty; + +import java.util.List; +import lombok.extern.slf4j.Slf4j; +import org.openmetadata.schema.EntityInterface; +import org.openmetadata.schema.type.EntityReference; +import org.openmetadata.schema.type.Include; +import org.openmetadata.service.Entity; +import org.openmetadata.service.exception.EntityNotFoundException; +import org.openmetadata.service.jdbi3.EntityRepository; +import org.openmetadata.service.security.AuthorizationException; +import org.openmetadata.service.util.EntityUtil.Fields; + +/** + * Post-filter for subjects restricted by the seeded {@code DomainOnlyAccessRole}. + * + *

{@code DomainOnlyAccessPolicy} grants "All operations on All resources" whenever + * {@link RuleEvaluator#hasDomain()} passes, and {@code hasDomain()} deliberately returns {@code + * true} for a resource context that carries no concrete entity, leaving the narrowing to the caller. + * The Data Quality CSV export and import flows resolve their test cases inside the repository, after + * that decision has been made, so they apply the narrowing here. The rule mirrors the entity-level + * decision {@code hasDomain()} makes: domainless entities are visible to everyone, otherwise the + * subject must own the entity's domain or one of its ancestors. + */ +@Slf4j +public final class DomainAccessFilter { + + /** The principal {@code NoopFilter} installs when the deployment runs without authentication. */ + private static final String NO_AUTH_PRINCIPAL = "anonymous"; + + private static final String UNRESOLVED_PRINCIPAL_MESSAGE = + "Principal '%s' cannot be resolved to a user; refusing to run without domain filtering"; + + private DomainAccessFilter() {} + + /** Returns true when the subject's view must be narrowed to its own domain hierarchy. */ + public static boolean shouldApply(SubjectContext subjectContext) { + return subjectContext != null + && !subjectContext.isAdmin() + && !subjectContext.isBot() + && subjectContext.hasDomainOnlyAccessRole(); + } + + /** + * Resolves the subject behind a principal name. Background CSV jobs and repositories only carry + * the principal name, never the request {@code SecurityContext}, so this is the same resolution + * {@code CsvImportExportJobHandler} already performs for search-backed exports. + * + *

Fails closed. A principal that cannot be resolved to a user aborts the operation instead of + * letting it run unfiltered — the subject resolves at execution time, not request time, so a user + * deleted or renamed while an async export sat queued would otherwise silently disable filtering. + * The single exemption is the fixed principal {@code NoopFilter} installs when the deployment runs + * without authentication, and only when it resolves to no user at all — so a real user named + * "anonymous" is filtered normally for as long as that user exists. The exemption is not reachable + * from an authenticated request: {@code DefaultAuthorizer.authorize} resolves the same principal + * first and rejects an unknown one before any of this runs. + */ + public static SubjectContext resolveSubject(String principalName) { + if (nullOrEmpty(principalName)) { + throw new AuthorizationException(String.format(UNRESOLVED_PRINCIPAL_MESSAGE, principalName)); + } + SubjectContext subjectContext = null; + try { + subjectContext = SubjectContext.getSubjectContext(principalName); + } catch (EntityNotFoundException e) { + if (!NO_AUTH_PRINCIPAL.equals(principalName)) { + throw new AuthorizationException( + String.format(UNRESOLVED_PRINCIPAL_MESSAGE, principalName), e); + } + LOG.debug("Principal '{}' has no user entity; running without authentication", principalName); + } + return subjectContext; + } + + /** + * Keeps only the entities whose domains the subject may see. Always returns an immutable list, + * never {@code null}, whether or not any narrowing applied — callers must not rely on getting + * their own list back. + */ + public static List retainAccessible( + List entities, SubjectContext subjectContext) { + List candidates = listOrEmpty(entities); + return shouldApply(subjectContext) + ? candidates.stream().filter(e -> subjectContext.hasDomains(e.getDomains())).toList() + : List.copyOf(candidates); + } + + /** + * Loads the entity whose version a bulk CSV import is about to bump, or {@code null} when it must + * be left alone. + * + *

Both the target FQN and the repository it is resolved against come straight from the request, + * so a caller picks the type and the name of the entity that gets a version bump, a + * {@code bulkImport} ChangeDescription and a ChangeEvent — all writes, and all of which a target + * outside the caller's domains must not receive. + * + *

A missing target and an inaccessible one are both skipped rather than failing the job: by + * this point the rows are already written, so failing would misreport the import, and treating the + * two alike keeps the job outcome from becoming a cross-domain existence oracle. + */ + public static EntityInterface resolveAccessibleVersioningTarget( + EntityRepository versioningRepo, String targetFqn, String principalName) { + EntityInterface target = null; + try { + String fields = versioningRepo.isSupportsDomains() ? Entity.FIELD_DOMAINS : ""; + EntityInterface candidate = + versioningRepo.getByName( + null, + targetFqn, + new Fields(versioningRepo.getAllowedFields(), fields), + Include.NON_DELETED, + false); + if (isAccessible(resolveSubject(principalName), candidate.getDomains())) { + target = candidate; + } + } catch (EntityNotFoundException e) { + LOG.debug("Bulk import versioning target '{}' no longer exists", targetFqn); + } + if (target == null) { + LOG.info("Skipping bulk import versioning for target '{}'", targetFqn); + } + return target; + } + + /** Returns true when the subject may read or write an entity carrying {@code domains}. */ + public static boolean isAccessible(SubjectContext subjectContext, List domains) { + return !shouldApply(subjectContext) || subjectContext.hasDomains(domains); + } +}