You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@polaris.apache.org by di...@apache.org on 2026/06/26 19:03:43 UTC
(polaris) branch main updated: Fix native catalog credential vending skipping allowedLocations re-validation (#4860)
This is an automated email from the ASF dual-hosted git repository.
dimas-b pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/polaris.git
The following commit(s) were added to refs/heads/main by this push:
new 85a0c29257 Fix native catalog credential vending skipping allowedLocations re-validation (#4860)
85a0c29257 is described below
commit 85a0c29257a5ba498d9867c95bf76e3306baf2f5
Author: vignesh_A <16...@users.noreply.github.com>
AuthorDate: Sat Jun 27 00:33:37 2026 +0530
Fix native catalog credential vending skipping allowedLocations re-validation (#4860)
* Update CHANGELOG for native credential vending allowedLocations fix
* Fix native catalog credential vending skipping allowedLocations re-validation
- Always validate table locations before vending credentials for native catalogs.
- Added regression test for post-creation allowedLocations shrink.
* Update CHANGELOG for native credential vending allowedLocations fix
* Fix comment formatting in IcebergAllowedLocationTest
* Fix CatalogFederationIT assertions for updated credential vending error message
---
CHANGELOG.md | 3 +-
.../it/test/CatalogFederationIntegrationTest.java | 4 +-
.../catalog/iceberg/IcebergCatalogHandler.java | 24 +++---
.../iceberg/IcebergAllowedLocationTest.java | 86 ++++++++++++++++++++++
4 files changed, 104 insertions(+), 13 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index c77a12acfa..b100ab1be3 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -104,6 +104,7 @@ request adding CHANGELOG notes for breaking (!) changes and possibly other secti
- The configuration option `polaris.event-listener.type` is deprecated and will be removed later. Please use `polaris.event-listener.types` instead.
### Fixes
+- Fixed native catalog credential vending paths (`loadCredentials` and `loadTable` with delegation) to re-validate locations against the *current* catalog `allowedLocations`. Previously these paths trusted persisted table entity locations, allowing stale credentials after an admin tightened allowed locations on a native catalog. (The federated path had a partial check.)
## [1.4.0]
@@ -157,7 +158,6 @@ request adding CHANGELOG notes for breaking (!) changes and possibly other secti
- `PolarisConfigurationStore` has been deprecated for removal.
### Fixes
-
- Fixed error propagation in drop operations (`dropTable`, `dropView`, `dropNamespace`). Server errors now return appropriate HTTP status codes based on persistence result instead of always returning NotFound
- Enable non-AWS STS role ARNs
- Helm chart: fixed a bug that prevented CORS settings to be properly applied. A new setting `cors.enabled` has been introduced in the chart as part of the fix.
@@ -235,7 +235,6 @@ request adding CHANGELOG notes for breaking (!) changes and possibly other secti
endpoints at `/q/metrics` and `/q/health` instead.
### Fixes
-
- Fixed incorrect Azure expires at field for the credentials refresh response, leading to client failure via #2633
## [1.1.0-incubating]
diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/CatalogFederationIntegrationTest.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/CatalogFederationIntegrationTest.java
index 3e4738f538..bdd338c8f6 100644
--- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/CatalogFederationIntegrationTest.java
+++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/CatalogFederationIntegrationTest.java
@@ -483,7 +483,7 @@ public class CatalogFederationIntegrationTest {
assertThatThrownBy(() -> spark.sql("SELECT * FROM ns3.test_table ORDER BY id").collectAsList())
.isInstanceOf(ForbiddenException.class)
.hasMessageContaining(
- "Table 'ns3.test_table' in remote catalog has locations outside catalog's allowed locations:");
+ "Table 'ns3.test_table' has locations outside the catalog's current allowed locations:");
// Case 3: TABLE_WRITE_DATA
managementApi.revokeGrant(federatedCatalogName, federatedCatalogRoleName, tableReadDataGrant);
@@ -501,6 +501,6 @@ public class CatalogFederationIntegrationTest {
() -> spark.sql("INSERT INTO ns3.test_table VALUES (3, 'Charlie')").collectAsList())
.isInstanceOf(ForbiddenException.class)
.hasMessageContaining(
- "Table 'ns3.test_table' in remote catalog has locations outside catalog's allowed locations:");
+ "Table 'ns3.test_table' has locations outside the catalog's current allowed locations:");
}
}
diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java
index 7269307503..930ce96672 100644
--- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java
+++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java
@@ -1006,10 +1006,10 @@ public abstract class IcebergCatalogHandler extends CatalogHandler implements Au
Set<String> tableLocations = StorageUtil.getLocationsUsedByTable(tableMetadata);
- // For federated catalogs, validate that table locations are within allowed locations
- if (isFederated) {
- validateRemoteTableLocations(tableIdentifier, tableLocations, resolvedStoragePath);
- }
+ // Validate that the table's locations are still within the catalog's current
+ // allowedLocations before vending credentials. This protects against cases where
+ // allowedLocations were tightened after the table was created.
+ validateTableLocations(tableIdentifier, tableLocations, resolvedStoragePath);
StorageAccessConfig storageAccessConfig =
storageAccessConfigProvider()
@@ -1043,13 +1043,15 @@ public abstract class IcebergCatalogHandler extends CatalogHandler implements Au
return responseBuilder;
}
- private void validateRemoteTableLocations(
+ private void validateTableLocations(
TableIdentifier tableIdentifier,
Set<String> tableLocations,
PolarisResolvedPathWrapper resolvedStoragePath) {
try {
- // Delegate to common validation logic
+ // Delegate to common validation logic. This is called for both native and federated
+ // catalogs before vending credentials to ensure locations are still within the
+ // current catalog's allowedLocations (defense against policy changes after table creation).
CatalogUtils.validateLocationsForTableLike(
realmConfig(), tableIdentifier, tableLocations, resolvedStoragePath);
@@ -1057,15 +1059,15 @@ public abstract class IcebergCatalogHandler extends CatalogHandler implements Au
.atInfo()
.addKeyValue("tableIdentifier", tableIdentifier)
.addKeyValue("tableLocations", tableLocations)
- .log("Validated federated table locations");
+ .log("Validated table locations for credential vending");
} catch (ForbiddenException e) {
LOGGER
.atError()
.addKeyValue("tableIdentifier", tableIdentifier)
.addKeyValue("tableLocations", tableLocations)
- .log("Federated table locations validation failed");
+ .log("Table locations validation failed for credential vending");
throw new ForbiddenException(
- "Table '%s' in remote catalog has locations outside catalog's allowed locations: %s",
+ "Table '%s' has locations outside the catalog's current allowed locations: %s",
tableIdentifier, e.getMessage());
}
}
@@ -1552,6 +1554,10 @@ public abstract class IcebergCatalogHandler extends CatalogHandler implements Au
return null;
}
+ // Re-validate before vending in case this is called from other paths in the future.
+ // Primary validation for loadCredentials and delegation happens at call sites.
+ validateTableLocations(tableIdentifier, tableLocations, resolvedStoragePath);
+
return storageAccessConfigProvider()
.getStorageAccessConfig(
tableIdentifier,
diff --git a/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/IcebergAllowedLocationTest.java b/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/IcebergAllowedLocationTest.java
index a8f3476aa1..8334853144 100644
--- a/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/IcebergAllowedLocationTest.java
+++ b/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/IcebergAllowedLocationTest.java
@@ -34,6 +34,7 @@ import java.nio.file.Paths;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
import java.util.UUID;
import org.apache.iceberg.MetadataUpdate;
import org.apache.iceberg.catalog.Namespace;
@@ -51,6 +52,7 @@ import org.apache.polaris.core.admin.model.CatalogProperties;
import org.apache.polaris.core.admin.model.CreateCatalogRequest;
import org.apache.polaris.core.admin.model.FileStorageConfigInfo;
import org.apache.polaris.core.admin.model.StorageConfigInfo;
+import org.apache.polaris.core.admin.model.UpdateCatalogRequest;
import org.apache.polaris.service.TestServices;
import org.jetbrains.annotations.NotNull;
import org.junit.jupiter.api.Test;
@@ -549,4 +551,88 @@ public class IcebergAllowedLocationTest {
assertThat(response.getStatus()).isEqualTo(Response.Status.OK.getStatusCode());
}
}
+
+ private void updateCatalogAllowedLocations(
+ TestServices services, List<String> newAllowedLocations) {
+ // Fetch current catalog to get entity version for update.
+ Catalog fetched;
+ try (Response getResp =
+ services
+ .catalogsApi()
+ .getCatalog(catalog, services.realmContext(), services.securityContext())) {
+ assertThat(getResp.getStatus()).isEqualTo(Response.Status.OK.getStatusCode());
+ fetched = getResp.readEntity(Catalog.class);
+ }
+
+ StorageConfigInfo newConfig =
+ FileStorageConfigInfo.builder()
+ .setStorageType(StorageConfigInfo.StorageTypeEnum.FILE)
+ .setAllowedLocations(newAllowedLocations)
+ .build();
+
+ UpdateCatalogRequest updateRequest =
+ UpdateCatalogRequest.builder()
+ .setCurrentEntityVersion(fetched.getEntityVersion())
+ .setStorageConfigInfo(newConfig)
+ .build();
+
+ try (Response updateResp =
+ services
+ .catalogsApi()
+ .updateCatalog(
+ catalog, updateRequest, services.realmContext(), services.securityContext())) {
+ assertThat(updateResp.getStatus()).isEqualTo(Response.Status.OK.getStatusCode());
+ }
+ }
+
+ @Test
+ void testNativeCredentialVendingRejectsStaleLocationsAfterAllowedLocationsShrink(
+ @TempDir Path tmpDir) {
+ var services = getTestServices();
+
+ var baseDir = tmpDir.resolve(catalog).toAbsolutePath().toUri().toString();
+ var allowedWhenCreated = baseDir + "/original-data";
+ var disallowedAfterShrink = baseDir + "/other-data";
+
+ // Catalog initially allows the original location
+ createCatalog(services, Map.of(), baseDir, List.of(allowedWhenCreated));
+ createNamespace(services, allowedWhenCreated + "/" + namespace);
+
+ String tableName = getTableName();
+ TableIdentifier tableId = TableIdentifier.of(Namespace.of(namespace), tableName);
+
+ var createReq =
+ CreateTableRequest.builder()
+ .withName(tableName)
+ .withSchema(SCHEMA)
+ .withLocation(allowedWhenCreated + "/" + namespace + "/" + tableName)
+ .build();
+
+ // Create succeeds under current allowed
+ try (Response createResp =
+ services
+ .restApi()
+ .createTable(
+ catalog,
+ namespace,
+ createReq,
+ "vended-credentials",
+ IDEMPOTENCY_KEY,
+ services.realmContext(),
+ services.securityContext())) {
+ assertThat(createResp.getStatus()).isEqualTo(Response.Status.OK.getStatusCode());
+ }
+
+ // Now shrink: update catalog to only allow a completely different location
+ updateCatalogAllowedLocations(services, List.of(disallowedAfterShrink));
+
+ // Credential vending for the existing table (which has stale location in its entity)
+ // must now be rejected.
+ IcebergCatalogHandler handler =
+ services.catalogAdapter().newHandler(services.securityContext(), catalog);
+
+ assertThatThrownBy(() -> handler.loadCredentials(tableId, Optional.empty()))
+ .isInstanceOf(ForbiddenException.class)
+ .hasMessageContaining("outside the catalog's current allowed locations");
+ }
}