This is an automated email from the ASF dual-hosted git repository.
okumin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hive.git
The following commit(s) were added to refs/heads/master by this push:
new 99b60fdd4ed HIVE-30002 : Restrict Iceberg REST catalog table
registration and drop-purge to authorized/contained locations (#6812)
99b60fdd4ed is described below
commit 99b60fdd4edd7f5f91784c7473e14f273cc76e65
Author: Henrib <[email protected]>
AuthorDate: Fri Oct 9 10:37:41 2026 +0200
HIVE-30002 : Restrict Iceberg REST catalog table registration and
drop-purge to authorized/contained locations (#6812)
* HIVE-30004: Restrict Iceberg REST catalog table registration and
drop-purge to authorized/contained locations
Reject REGISTER_TABLE requests whose metadata location or embedded
table location is not authorized or contained within the namespace.
Authorize DROP_TABLE purge against the table's location, and fence
purge deletions to the table's own location as defense in depth.
Co-Authored-By: Claude Code <[email protected]>
* HIVE-30002 : Address review feedback on Iceberg REST catalog location
checks
- Replace hand-rolled string-prefix containment in IcebergAuthorizer and
ScopedDeleteFileIO with the shared, depth-based
FileUtils.isPathWithinSubtree (metastore-common)
- Drop the redundant embedded-metadata-location check in
validateRegisterTable; it's already covered by HMS's own CREATE_TABLE
authorization. This removes the need for HiveCatalog#io() and the
ClassCastException risk against HMSCachingCatalog
- Rely on namespaceMetadata's own "location" instead of re-deriving it
with a hand-rolled DB-location convention that mishandles exceptions
(e.g. the default database)
- Downgrade the skipped-delete log line from WARN to DEBUG
- Strengthen the purge test to also assert a nested file is deleted,
not just that an outside file survives
Co-Authored-By: Claude Code <[email protected]>
* HIVE-30002 : Qualify scheme-less purge paths against the default FS, add
FileUtils test coverage
Address okumin's review on #6812: ScopedDeleteFileIO now qualifies
scheme-less paths against the cluster's default filesystem
(fs.defaultFS) instead of borrowing a scheme from the other side of
the comparison, and metastore-common's FileUtils.isPathWithinSubtree
gets the test coverage ported from common's original implementation.
Co-Authored-By: Claude Code <[email protected]>
* HIVE-30002: Require WRITE on drop-table-purge location
Pass the table location as both input and output to checkPrivileges in
validateDropTablePurge, so authorizers that only enforce write/delete
privileges on outputs (e.g. Ranger) require WRITE rather than READ alone,
matching that purge deletes files there.
Co-Authored-By: Claude Code <[email protected]>
---------
Co-authored-by: Claude Code <[email protected]>
---
.../java/org/apache/iceberg/hive/HiveCatalog.java | 2 +-
.../apache/iceberg/hive/ScopedDeleteFileIO.java | 105 ++++++++++++++++
.../hadoop/hive/metastore/utils/FileUtils.java | 24 ++++
.../hadoop/hive/metastore/utils/TestFileUtils.java | 58 +++++++++
.../org/apache/iceberg/rest/HMSCatalogAdapter.java | 10 +-
.../org/apache/iceberg/rest/IcebergAuthorizer.java | 105 ++++++++++++++++
.../apache/iceberg/rest/BaseRESTCatalogTests.java | 71 +++++++++++
.../apache/iceberg/rest/TestIcebergAuthorizer.java | 134 +++++++++++++++++++++
8 files changed, 506 insertions(+), 3 deletions(-)
diff --git
a/iceberg/iceberg-catalog/src/main/java/org/apache/iceberg/hive/HiveCatalog.java
b/iceberg/iceberg-catalog/src/main/java/org/apache/iceberg/hive/HiveCatalog.java
index b7bd371dfe1..95749370c82 100644
---
a/iceberg/iceberg-catalog/src/main/java/org/apache/iceberg/hive/HiveCatalog.java
+++
b/iceberg/iceberg-catalog/src/main/java/org/apache/iceberg/hive/HiveCatalog.java
@@ -271,7 +271,7 @@ public boolean dropTable(TableIdentifier identifier,
boolean purge) {
});
if (purge && lastMetadata != null) {
- CatalogUtil.dropTableData(ops.io(), lastMetadata);
+ CatalogUtil.dropTableData(new ScopedDeleteFileIO(ops.io(),
lastMetadata.location(), conf), lastMetadata);
}
LOG.info("Dropped table: {}", identifier);
diff --git
a/iceberg/iceberg-catalog/src/main/java/org/apache/iceberg/hive/ScopedDeleteFileIO.java
b/iceberg/iceberg-catalog/src/main/java/org/apache/iceberg/hive/ScopedDeleteFileIO.java
new file mode 100644
index 00000000000..1a3e584c2e5
--- /dev/null
+++
b/iceberg/iceberg-catalog/src/main/java/org/apache/iceberg/hive/ScopedDeleteFileIO.java
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.apache.iceberg.hive;
+
+import java.net.URI;
+import java.util.Map;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hive.metastore.utils.FileUtils;
+import org.apache.iceberg.io.FileIO;
+import org.apache.iceberg.io.InputFile;
+import org.apache.iceberg.io.OutputFile;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * A {@link FileIO} decorator used by {@link
HiveCatalog#dropTable(org.apache.iceberg.catalog.TableIdentifier,
+ * boolean)} to fence purge deletions to files under a table's own location,
regardless of what a table's
+ * metadata or manifests actually reference.
+ *
+ * <p>This does not implement {@link
org.apache.iceberg.io.SupportsBulkOperations} or
+ * {@link org.apache.iceberg.io.SupportsPrefixOperations} even when the
delegate does, so that
+ * {@code CatalogUtil.dropTableData} is forced to route every deletion through
{@link #deleteFile(String)}.
+ */
+class ScopedDeleteFileIO implements FileIO {
+ private static final Logger LOG =
LoggerFactory.getLogger(ScopedDeleteFileIO.class);
+
+ private final FileIO delegate;
+ private final URI defaultUri;
+ private final Path location;
+
+ ScopedDeleteFileIO(FileIO delegate, String location, Configuration conf) {
+ this.delegate = delegate;
+ this.defaultUri = FileSystem.getDefaultUri(conf);
+ this.location = qualify(new Path(location));
+ }
+
+ @Override
+ public InputFile newInputFile(String path) {
+ return delegate.newInputFile(path);
+ }
+
+ @Override
+ public OutputFile newOutputFile(String path) {
+ return delegate.newOutputFile(path);
+ }
+
+ @Override
+ public void deleteFile(String path) {
+ var candidate = qualify(new Path(path));
+ if (!FileUtils.isPathWithinSubtree(candidate, location)) {
+ LOG.debug("Skipping delete outside table location {}: {}", location,
path);
+ return;
+ }
+ delegate.deleteFile(path);
+ }
+
+ /**
+ * Manifests can hold data file paths written without a scheme, and a
table's own location can
+ * equally be scheme-less (e.g. when a caller supplies one explicitly).
{@link Path#equals} is
+ * scheme-sensitive, so without this a schemeless path would never match its
scheme-qualified
+ * counterpart in {@link FileUtils#isPathWithinSubtree} even when it is
nested directly under it.
+ * A scheme-less path refers to a location on the cluster's default
filesystem (the one
+ * configured via {@code fs.defaultFS} in {@code core-site.xml}), so that is
what it is qualified
+ * against here, rather than against whatever scheme the other side of the
comparison happens to
+ * carry. A path that already carries a scheme is returned unchanged, so a
genuine mismatch (e.g.
+ * a different scheme or authority) is still correctly treated as outside
the table location.
+ */
+ private Path qualify(Path path) {
+ return path.makeQualified(defaultUri, new Path(Path.SEPARATOR));
+ }
+
+ @Override
+ public Map<String, String> properties() {
+ return delegate.properties();
+ }
+
+ @Override
+ public void initialize(Map<String, String> properties) {
+ delegate.initialize(properties);
+ }
+
+ @Override
+ public void close() {
+ delegate.close();
+ }
+}
diff --git
a/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/FileUtils.java
b/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/FileUtils.java
index 7b045ce2065..4bbbbf99fa4 100644
---
a/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/FileUtils.java
+++
b/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/FileUtils.java
@@ -658,4 +658,28 @@ void findNext() throws IOException {
nextFile = null;
}
}
+
+ /**
+ * Checks whether {@code path} is {@code subtree} itself or nested under it,
by walking up
+ * {@code path}'s ancestors looking for a match. Depth-based rather than a
string-prefix
+ * comparison, so it isn't fooled by sibling paths that merely share a
prefix (e.g. {@code /a/bc}
+ * is not within {@code /a/b}).
+ *
+ * @param path the path to check
+ * @param subtree the candidate ancestor, or exact match
+ * @return true if {@code path} equals {@code subtree} or is one of its
descendants
+ */
+ public static boolean isPathWithinSubtree(final Path path, final Path
subtree) {
+ final int subtreeDepth = subtree.depth();
+ int pathDepth = path.depth();
+ if (pathDepth < subtreeDepth) {
+ return false;
+ }
+ Path subpath = path;
+ while (pathDepth > subtreeDepth) {
+ subpath = subpath.getParent();
+ pathDepth -= 1;
+ }
+ return subtree.equals(subpath);
+ }
}
diff --git
a/standalone-metastore/metastore-common/src/test/java/org/apache/hadoop/hive/metastore/utils/TestFileUtils.java
b/standalone-metastore/metastore-common/src/test/java/org/apache/hadoop/hive/metastore/utils/TestFileUtils.java
new file mode 100644
index 00000000000..b42e814247e
--- /dev/null
+++
b/standalone-metastore/metastore-common/src/test/java/org/apache/hadoop/hive/metastore/utils/TestFileUtils.java
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.apache.hadoop.hive.metastore.utils;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import org.apache.hadoop.fs.Path;
+import org.junit.Test;
+
+public class TestFileUtils {
+
+ @Test
+ public void isPathWithinSubtree_samePrefix() {
+ Path path = new Path("/somedir1");
+ Path subtree = new Path("/somedir");
+
+ assertFalse(FileUtils.isPathWithinSubtree(path, subtree));
+ }
+
+ @Test
+ public void isPathWithinSubtree_rootIsInside() {
+ Path path = new Path("/foo");
+ Path subtree = new Path("/foo");
+ assertTrue(FileUtils.isPathWithinSubtree(path, subtree));
+ }
+
+ @Test
+ public void isPathWithinSubtree_descendantInside() {
+ Path path = new Path("/foo/bar");
+ Path subtree = new Path("/foo");
+ assertTrue(FileUtils.isPathWithinSubtree(path, subtree));
+ }
+
+ @Test
+ public void isPathWithinSubtree_relativeWalk() {
+ Path path = new Path("foo/../../bar");
+ Path subtree = new Path("../bar");
+ assertTrue(FileUtils.isPathWithinSubtree(path, subtree));
+ }
+}
diff --git
a/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogAdapter.java
b/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogAdapter.java
index aa3813e533f..859a8ebe601 100644
---
a/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogAdapter.java
+++
b/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogAdapter.java
@@ -359,10 +359,13 @@ private LoadTableResponse createTable(Map<String, String>
vars, Object body) {
}
private RESTResponse dropTable(Map<String, String> vars) {
+ TableIdentifier ident = identFromPathVars(vars);
if (PropertyUtil.propertyAsBoolean(vars, "purgeRequested", false)) {
- CatalogHandlers.purgeTable(catalog, identFromPathVars(vars));
+ String location = catalog.loadTable(ident).location();
+ icebergAuthorizer.validateDropTablePurge(catalogName, ident, location);
+ CatalogHandlers.purgeTable(catalog, ident);
} else {
- CatalogHandlers.dropTable(catalog, identFromPathVars(vars));
+ CatalogHandlers.dropTable(catalog, ident);
}
return null;
}
@@ -381,6 +384,9 @@ private LoadTableResponse loadTable(Map<String, String>
vars) {
private LoadTableResponse registerTable(Map<String, String> vars, Object
body) {
Namespace namespace = namespaceFromPathVars(vars);
RegisterTableRequest request = castRequest(RegisterTableRequest.class,
body);
+ request.validate();
+ Map<String, String> namespaceMetadata =
asNamespaceCatalog.loadNamespaceMetadata(namespace);
+ icebergAuthorizer.validateRegisterTable(catalogName, namespace,
namespaceMetadata, request);
return CatalogHandlers.registerTable(catalog, namespace, request);
}
diff --git
a/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/IcebergAuthorizer.java
b/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/IcebergAuthorizer.java
index 2df051105b7..38069fea81c 100644
---
a/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/IcebergAuthorizer.java
+++
b/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/IcebergAuthorizer.java
@@ -30,9 +30,11 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.Path;
import org.apache.hadoop.hive.conf.HiveConf;
import org.apache.hadoop.hive.metastore.api.PrincipalType;
import org.apache.hadoop.hive.metastore.conf.MetastoreConf;
+import org.apache.hadoop.hive.metastore.utils.FileUtils;
import org.apache.hadoop.hive.ql.metadata.HiveException;
import org.apache.hadoop.hive.ql.metadata.HiveUtils;
import
org.apache.hadoop.hive.ql.security.authorization.plugin.HiveAccessControlException;
@@ -45,9 +47,11 @@
import
org.apache.hadoop.hive.ql.security.authorization.plugin.HivePrivilegeObject;
import
org.apache.hadoop.hive.ql.security.authorization.plugin.metastore.HiveMetaStoreAuthorizer;
import org.apache.iceberg.catalog.Namespace;
+import org.apache.iceberg.catalog.TableIdentifier;
import org.apache.iceberg.exceptions.ForbiddenException;
import org.apache.iceberg.hive.HiveHadoopUtil;
import org.apache.iceberg.rest.requests.CreateTableRequest;
+import org.apache.iceberg.rest.requests.RegisterTableRequest;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -161,4 +165,105 @@ void validateStageCreateTable(String catalogName,
Namespace namespace, Map<Strin
throw new IllegalStateException("Failed to check privileges
stage-create", e);
}
}
+
+ /**
+ * Enforces authorization for REGISTER_TABLE. The request's {@code
metadataLocation} must be authorized, since
+ * REGISTER_TABLE is otherwise an arbitrary-file-read primitive that returns
any metadata file's contents to the
+ * caller. The {@code location()} embedded in that metadata file (which
becomes the table's HMS
+ * {@code StorageDescriptor.location}) is not checked here: it is authorized
transitively by HMS's own
+ * CREATE_TABLE authorization when {@code CatalogHandlers.registerTable}
creates the metastore table.
+ *
+ * <p>When no {@code HiveAuthorizer} is configured, falls back to requiring
the metadata location to be
+ * contained in the namespace's external root, since there is no policy to
otherwise decide whether the caller
+ * may read an arbitrary location with service credentials.
+ *
+ * @param catalogName the Hive catalog name
+ * @param namespace the Iceberg namespace
+ * @param namespaceMetadata the Iceberg namespace metadata
+ * @param request the register table request
+ * @throws ForbiddenException if the location is not authorized, or not
contained in the namespace
+ * @throws IllegalStateException if the authorization plugin fails
+ */
+ void validateRegisterTable(String catalogName, Namespace namespace,
Map<String, String> namespaceMetadata,
+ RegisterTableRequest request) {
+ Preconditions.checkArgument(namespace.levels().length == 1, "Hive does not
support multi-level namespaces");
+ var databaseName = namespace.level(0);
+ var commandString = "register table " + request.name();
+ checkLocationAuthorized(catalogName, databaseName, namespaceMetadata,
request.metadataLocation(), commandString);
+ }
+
+ /**
+ * Enforces authorization for DROP_TABLE with {@code purge=true}. Purge
deletes every file referenced by the
+ * table's current metadata using the catalog's shared, service-level {@code
FileIO}, so the location must be
+ * authorized like any other DFS_URI access. The location is passed as both
an input and an output so that
+ * authorizers which only enforce write/delete privileges on outputs (e.g.
Ranger, where a DFS_URI input is
+ * READ-checked and a DFS_URI output is WRITE-checked) require WRITE on the
location rather than READ alone,
+ * matching the fact that purge deletes files there.
+ *
+ * <p>Unlike {@link #validateRegisterTable}, there is no
namespace-containment fallback here: the structural
+ * fence in {@code HiveCatalog.dropTable} already restricts purge deletions
to files under the table's own
+ * location regardless of whether a {@code HiveAuthorizer} is configured, so
a deployment without one relies on
+ * that fence rather than this check.
+ *
+ * @param catalogName the Hive catalog name
+ * @param identifier the table identifier being dropped
+ * @param location the table's current location
+ * @throws ForbiddenException if the location is not authorized
+ * @throws IllegalStateException if the authorization plugin fails
+ */
+ void validateDropTablePurge(String catalogName, TableIdentifier identifier,
String location) {
+ var authorizer = authorizerSupplier.get();
+ if (authorizer == null) {
+ LOG.info("No pre-event listener is configured for catalog {}, skipping
drop-table-purge authorization for {}",
+ catalogName, identifier);
+ return;
+ }
+
+ var inputs = Collections.singletonList(
+ new
HivePrivilegeObject(HivePrivilegeObject.HivePrivilegeObjectType.DFS_URI,
location));
+ var outputs = Collections.singletonList(
+ new
HivePrivilegeObject(HivePrivilegeObject.HivePrivilegeObjectType.DFS_URI,
location));
+ var builder = new HiveAuthzContext.Builder();
+ builder.setCommandString("drop table " + identifier.name());
+ try {
+ authorizer.checkPrivileges(HiveOperationType.DROPTABLE, inputs, outputs,
builder.build());
+ } catch (HiveAccessControlException e) {
+ throw new ForbiddenException(e, e.getMessage());
+ } catch (HiveAuthzPluginException e) {
+ throw new IllegalStateException("Failed to check privileges
drop-table-purge", e);
+ }
+ }
+
+ private void checkLocationAuthorized(String catalogName, String databaseName,
+ Map<String, String> namespaceMetadata, String location, String
commandString) {
+ var authorizer = authorizerSupplier.get();
+ if (authorizer == null) {
+ LOG.info("No pre-event listener is configured for catalog {}, falling
back to namespace containment for {}",
+ catalogName, location);
+ checkContainedInNamespace(databaseName, namespaceMetadata, location);
+ return;
+ }
+
+ var inputs = Collections.singletonList(
+ new
HivePrivilegeObject(HivePrivilegeObject.HivePrivilegeObjectType.DFS_URI,
location));
+ var builder = new HiveAuthzContext.Builder();
+ builder.setCommandString(commandString);
+ try {
+ authorizer.checkPrivileges(HiveOperationType.CREATETABLE, inputs,
Collections.emptyList(), builder.build());
+ } catch (HiveAccessControlException e) {
+ throw new ForbiddenException(e, e.getMessage());
+ } catch (HiveAuthzPluginException e) {
+ throw new IllegalStateException("Failed to check privileges for " +
commandString, e);
+ }
+ }
+
+ private void checkContainedInNamespace(String databaseName, Map<String,
String> namespaceMetadata,
+ String location) {
+ var externalRoot = namespaceMetadata.get("location");
+ if (externalRoot != null && FileUtils.isPathWithinSubtree(new
Path(location), new Path(externalRoot))) {
+ return;
+ }
+ throw new ForbiddenException(
+ "Location %s is not authorized and is not contained in namespace %s",
location, databaseName);
+ }
}
diff --git
a/standalone-metastore/metastore-rest-catalog/src/test/java/org/apache/iceberg/rest/BaseRESTCatalogTests.java
b/standalone-metastore/metastore-rest-catalog/src/test/java/org/apache/iceberg/rest/BaseRESTCatalogTests.java
index 206d72c9cb1..b4cd3c15b83 100644
---
a/standalone-metastore/metastore-rest-catalog/src/test/java/org/apache/iceberg/rest/BaseRESTCatalogTests.java
+++
b/standalone-metastore/metastore-rest-catalog/src/test/java/org/apache/iceberg/rest/BaseRESTCatalogTests.java
@@ -20,15 +20,21 @@
package org.apache.iceberg.rest;
import java.io.IOException;
+import java.nio.file.Files;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.Optional;
+import org.apache.hadoop.conf.Configuration;
import org.apache.iceberg.BaseTable;
+import org.apache.iceberg.DataFile;
+import org.apache.iceberg.DataFiles;
+import org.apache.iceberg.FileFormat;
import org.apache.iceberg.PartitionSpec;
import org.apache.iceberg.Schema;
import org.apache.iceberg.Table;
import org.apache.iceberg.TableMetadata;
+import org.apache.iceberg.TableMetadataParser;
import org.apache.iceberg.Transaction;
import org.apache.iceberg.catalog.Catalog;
import org.apache.iceberg.catalog.CatalogTests;
@@ -37,6 +43,7 @@
import org.apache.iceberg.catalog.TableIdentifier;
import org.apache.iceberg.exceptions.ForbiddenException;
import org.apache.iceberg.exceptions.NoSuchTableException;
+import org.apache.iceberg.hadoop.HadoopFileIO;
import org.apache.iceberg.rest.extension.MockHiveAuthorizer;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.Assertions;
@@ -277,4 +284,68 @@ void testStageCreateTableWithDeniedLocation() {
Assertions.assertThrows(ForbiddenException.class,
builder::createTransaction);
Assertions.assertThrows(NoSuchTableException.class, () ->
catalog.loadTable(tableIdentifier));
}
+
+ private static String writeMetadataFile(String directory, String
tableLocation) throws IOException {
+ var metadataLocation = directory + "/v1.metadata.json";
+ Files.deleteIfExists(java.nio.file.Path.of(metadataLocation));
+ var io = new HadoopFileIO(new Configuration(false));
+ var metadata = TableMetadata.newTableMetadata(new Schema(),
PartitionSpec.unpartitioned(), tableLocation,
+ Collections.emptyMap());
+ TableMetadataParser.write(metadata, io.newOutputFile(metadataLocation));
+ return metadataLocation;
+ }
+
+ @Test
+ void testRegisterTableWithDeniedLocation() {
+ var tableIdentifier = TableIdentifier.of("default",
"register-table-denied");
+ var metadataLocation = MockHiveAuthorizer.DENIED_PREFIX +
"/register-table-denied/v1.metadata.json";
+ Assertions.assertThrows(ForbiddenException.class, () ->
catalog.registerTable(tableIdentifier, metadataLocation));
+ Assertions.assertThrows(NoSuchTableException.class, () ->
catalog.loadTable(tableIdentifier));
+ }
+
+ @Test
+ void testRegisterTableWithDeniedEmbeddedLocation() throws IOException {
+ var tableIdentifier = TableIdentifier.of("default",
"register-table-embedded-denied");
+ var tableLocation = MockHiveAuthorizer.DENIED_PREFIX +
"/register-table-embedded-denied";
+ var metadataLocation = writeMetadataFile(
+ MockHiveAuthorizer.ALLOWED_PREFIX + "/register-table-embedded-denied",
tableLocation);
+ Assertions.assertThrows(ForbiddenException.class, () ->
catalog.registerTable(tableIdentifier, metadataLocation));
+ Assertions.assertThrows(NoSuchTableException.class, () ->
catalog.loadTable(tableIdentifier));
+ }
+
+ @Test
+ void testDropTablePurgeDoesNotDeleteFilesOutsideTableLocation() throws
IOException {
+ var victimDirectory =
java.nio.file.Path.of(MockHiveAuthorizer.ALLOWED_PREFIX,
"structural-fence-victim");
+ Files.createDirectories(victimDirectory);
+ var victimFile = victimDirectory.resolve("victim-data.txt");
+ Files.writeString(victimFile, "victim data");
+
+ var tableIdentifier = TableIdentifier.of("default",
"structural-fence-attacker");
+ var tableLocation = MockHiveAuthorizer.ALLOWED_PREFIX +
"/structural-fence-attacker";
+ Table table = catalog.buildTable(tableIdentifier, new
Schema()).withLocation(tableLocation).create();
+
+ var ownedDirectory =
java.nio.file.Path.of(MockHiveAuthorizer.ALLOWED_PREFIX,
"structural-fence-attacker", "data");
+ Files.createDirectories(ownedDirectory);
+ var ownedFile = ownedDirectory.resolve("owned-data.txt");
+ Files.writeString(ownedFile, "owned data");
+
+ DataFile victimDataFile = DataFiles.builder(table.spec())
+ .withPath(victimFile.toUri().toString())
+ .withFormat(FileFormat.PARQUET)
+ .withFileSizeInBytes(Files.size(victimFile))
+ .withRecordCount(1)
+ .build();
+ DataFile ownedDataFile = DataFiles.builder(table.spec())
+ .withPath(ownedFile.toUri().toString())
+ .withFormat(FileFormat.PARQUET)
+ .withFileSizeInBytes(Files.size(ownedFile))
+ .withRecordCount(1)
+ .build();
+
table.newAppend().appendFile(victimDataFile).appendFile(ownedDataFile).commit();
+
+ Assertions.assertTrue(catalog.dropTable(tableIdentifier, true));
+ Assertions.assertThrows(NoSuchTableException.class, () ->
catalog.loadTable(tableIdentifier));
+ Assertions.assertTrue(Files.exists(victimFile), "purge must not delete
files outside the table location");
+ Assertions.assertFalse(Files.exists(ownedFile), "purge must delete files
nested inside the table location");
+ }
}
diff --git
a/standalone-metastore/metastore-rest-catalog/src/test/java/org/apache/iceberg/rest/TestIcebergAuthorizer.java
b/standalone-metastore/metastore-rest-catalog/src/test/java/org/apache/iceberg/rest/TestIcebergAuthorizer.java
index 0d13414a007..bf71bfe7b74 100644
---
a/standalone-metastore/metastore-rest-catalog/src/test/java/org/apache/iceberg/rest/TestIcebergAuthorizer.java
+++
b/standalone-metastore/metastore-rest-catalog/src/test/java/org/apache/iceberg/rest/TestIcebergAuthorizer.java
@@ -45,10 +45,12 @@
import org.apache.hadoop.security.UserGroupInformation;
import org.apache.iceberg.Schema;
import org.apache.iceberg.catalog.Namespace;
+import org.apache.iceberg.catalog.TableIdentifier;
import org.apache.iceberg.exceptions.ForbiddenException;
import org.apache.iceberg.rest.extension.MockHiveAuthorizer;
import org.apache.iceberg.rest.extension.MockHiveAuthorizerFactory;
import org.apache.iceberg.rest.requests.CreateTableRequest;
+import org.apache.iceberg.rest.requests.ImmutableRegisterTableRequest;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.mockito.ArgumentCaptor;
@@ -265,4 +267,136 @@ void testTranslateAuthorizationPluginException() throws
Exception {
Assertions.assertEquals("Failed to check privileges stage-create",
exception.getMessage());
Assertions.assertSame(failure, exception.getCause());
}
+
+ @Test
+ @SuppressWarnings("unchecked")
+ void testValidateRegisterTableAuthorized() throws Exception {
+ var hiveAuthorizer = mock(HiveAuthorizer.class);
+ var icebergAuthorizer = new IcebergAuthorizer(() -> hiveAuthorizer);
+
+ var metadataLocation = LOCATION + "/metadata/v1.metadata.json";
+ var request =
ImmutableRegisterTableRequest.builder().name(TABLE_NAME).metadataLocation(metadataLocation).build();
+
+ icebergAuthorizer.validateRegisterTable(CATALOG_NAME, NAMESPACE, Map.of(),
request);
+
+ var operation = ArgumentCaptor.forClass(HiveOperationType.class);
+ var inputs = ArgumentCaptor.forClass(List.class);
+ var context = ArgumentCaptor.forClass(HiveAuthzContext.class);
+ verify(hiveAuthorizer).checkPrivileges(operation.capture(),
inputs.capture(), anyList(), context.capture());
+
+ Assertions.assertEquals(HiveOperationType.CREATETABLE,
operation.getValue());
+ Assertions.assertEquals("register table " + TABLE_NAME,
context.getValue().getCommandString());
+
+ var checkedLocation = (HivePrivilegeObject) inputs.getValue().getFirst();
+
assertThat(checkedLocation.getType()).isEqualTo(HivePrivilegeObjectType.DFS_URI);
+ assertThat(checkedLocation.getObjectName()).isEqualTo(metadataLocation);
+ }
+
+ @Test
+ void testValidateRegisterTableDeniedMetadataLocation() throws Exception {
+ var hiveAuthorizer = mock(HiveAuthorizer.class);
+ var failure = new HiveAccessControlException("access denied");
+ doThrow(failure).when(hiveAuthorizer).checkPrivileges(any(), anyList(),
anyList(), any());
+ var icebergAuthorizer = new IcebergAuthorizer(() -> hiveAuthorizer);
+
+ var request =
ImmutableRegisterTableRequest.builder().name(TABLE_NAME).metadataLocation(LOCATION).build();
+ var exception = Assertions.assertThrows(ForbiddenException.class, () ->
+ icebergAuthorizer.validateRegisterTable(CATALOG_NAME, NAMESPACE,
Map.of(), request));
+ Assertions.assertEquals("access denied", exception.getMessage());
+ }
+
+ @Test
+ void testValidateRegisterTableWithMultiLevelNamespace() {
+ var hiveAuthorizer = mock(HiveAuthorizer.class);
+ var icebergAuthorizer = new IcebergAuthorizer(() -> hiveAuthorizer);
+ var nestedNamespace = Namespace.of("db", "nested");
+ var request =
ImmutableRegisterTableRequest.builder().name(TABLE_NAME).metadataLocation(LOCATION).build();
+
+ var exception = Assertions.assertThrows(IllegalArgumentException.class, ()
->
+ icebergAuthorizer.validateRegisterTable(CATALOG_NAME, nestedNamespace,
Map.of(), request));
+ Assertions.assertEquals("Hive does not support multi-level namespaces",
exception.getMessage());
+ Mockito.verifyNoInteractions(hiveAuthorizer);
+ }
+
+ @Test
+ void testValidateRegisterTableNoAuthorizerFallbackAllowed() {
+ var icebergAuthorizer = new IcebergAuthorizer(() -> null);
+
+ var externalRoot = "file:/warehouse/external";
+ var namespaceMetadata = Map.of("location", externalRoot);
+ var metadataLocation = externalRoot + "/table/metadata/v1.metadata.json";
+ var request =
ImmutableRegisterTableRequest.builder().name(TABLE_NAME).metadataLocation(metadataLocation).build();
+
+ icebergAuthorizer.validateRegisterTable(CATALOG_NAME, NAMESPACE,
namespaceMetadata, request);
+ }
+
+ @Test
+ void testValidateRegisterTableNoAuthorizerFallbackDenied() {
+ var icebergAuthorizer = new IcebergAuthorizer(() -> null);
+
+ var request =
ImmutableRegisterTableRequest.builder().name(TABLE_NAME).metadataLocation(LOCATION).build();
+ Assertions.assertThrows(ForbiddenException.class, () ->
+ icebergAuthorizer.validateRegisterTable(CATALOG_NAME, NAMESPACE,
Map.of(), request));
+ }
+
+ @Test
+ @SuppressWarnings("unchecked")
+ void testValidateDropTablePurgeAuthorized() throws Exception {
+ var hiveAuthorizer = mock(HiveAuthorizer.class);
+ var icebergAuthorizer = new IcebergAuthorizer(() -> hiveAuthorizer);
+ var identifier = TableIdentifier.of(NAMESPACE, TABLE_NAME);
+
+ icebergAuthorizer.validateDropTablePurge(CATALOG_NAME, identifier,
LOCATION);
+
+ var operation = ArgumentCaptor.forClass(HiveOperationType.class);
+ var inputs = ArgumentCaptor.forClass(List.class);
+ var outputs = ArgumentCaptor.forClass(List.class);
+ var context = ArgumentCaptor.forClass(HiveAuthzContext.class);
+ verify(hiveAuthorizer).checkPrivileges(operation.capture(),
inputs.capture(), outputs.capture(), context.capture());
+
+ Assertions.assertEquals(HiveOperationType.DROPTABLE, operation.getValue());
+ Assertions.assertEquals(1, inputs.getValue().size());
+ var inputLocation = (HivePrivilegeObject) inputs.getValue().getFirst();
+
assertThat(inputLocation.getType()).isEqualTo(HivePrivilegeObjectType.DFS_URI);
+ assertThat(inputLocation.getObjectName()).isEqualTo(LOCATION);
+ Assertions.assertEquals(1, outputs.getValue().size());
+ var outputLocation = (HivePrivilegeObject) outputs.getValue().getFirst();
+
assertThat(outputLocation.getType()).isEqualTo(HivePrivilegeObjectType.DFS_URI);
+ assertThat(outputLocation.getObjectName()).isEqualTo(LOCATION);
+ Assertions.assertEquals("drop table " + TABLE_NAME,
context.getValue().getCommandString());
+ }
+
+ @Test
+ void testValidateDropTablePurgeDenied() throws Exception {
+ var hiveAuthorizer = mock(HiveAuthorizer.class);
+ var failure = new HiveAccessControlException("access denied");
+ doThrow(failure).when(hiveAuthorizer).checkPrivileges(any(), anyList(),
anyList(), any());
+ var icebergAuthorizer = new IcebergAuthorizer(() -> hiveAuthorizer);
+ var identifier = TableIdentifier.of(NAMESPACE, TABLE_NAME);
+
+ var exception = Assertions.assertThrows(ForbiddenException.class, () ->
+ icebergAuthorizer.validateDropTablePurge(CATALOG_NAME, identifier,
LOCATION));
+ Assertions.assertEquals("access denied", exception.getMessage());
+ Assertions.assertSame(failure, exception.getCause());
+ }
+
+ @Test
+ void testValidateDropTablePurgeTranslatesPluginException() throws Exception {
+ var hiveAuthorizer = mock(HiveAuthorizer.class);
+ var failure = new HiveAuthzPluginException("plugin failure");
+ doThrow(failure).when(hiveAuthorizer).checkPrivileges(any(), anyList(),
anyList(), any());
+ var icebergAuthorizer = new IcebergAuthorizer(() -> hiveAuthorizer);
+ var identifier = TableIdentifier.of(NAMESPACE, TABLE_NAME);
+
+ var exception = Assertions.assertThrows(IllegalStateException.class, () ->
+ icebergAuthorizer.validateDropTablePurge(CATALOG_NAME, identifier,
LOCATION));
+ Assertions.assertEquals("Failed to check privileges drop-table-purge",
exception.getMessage());
+ Assertions.assertSame(failure, exception.getCause());
+ }
+
+ @Test
+ void testValidateDropTablePurgeWithoutAuthorizer() {
+ var icebergAuthorizer = new IcebergAuthorizer(() -> null);
+ icebergAuthorizer.validateDropTablePurge(CATALOG_NAME,
TableIdentifier.of(NAMESPACE, TABLE_NAME), LOCATION);
+ }
}
