okumin commented on code in PR #6812:
URL: https://github.com/apache/hive/pull/6812#discussion_r4083904705
##########
standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/IcebergAuthorizer.java:
##########
@@ -161,4 +175,133 @@ 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} is fetched with the
+ * catalog's shared, service-level {@link FileIO}, so both that location and
the {@code location()} embedded in
+ * the metadata file it points to (which becomes the table's HMS {@code
StorageDescriptor.location}, and is what
+ * a later purge trusts as its deletion root, see {@link
#validateDropTablePurge}) must be authorized. Otherwise
+ * REGISTER_TABLE is an arbitrary-file-read primitive that returns any
metadata file's contents to the caller.
+ *
+ * <p>When no {@code HiveAuthorizer} is configured, falls back to requiring
both locations to be contained in
+ * the namespace's external or managed 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
+ * @param io the {@link FileIO} used to read the metadata file
+ * @throws ForbiddenException if a 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, FileIO io) {
+ 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);
+
+ var metadata = TableMetadataParser.read(io, request.metadataLocation());
+ checkLocationAuthorized(catalogName, databaseName, namespaceMetadata,
metadata.location(), 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 {@link
FileIO}, so the location must be
+ * authorized like any other DFS_URI access.
Review Comment:
I guess this is unnecessary, since a user is typically authorized per table,
not per directory.
When a user has INSERT permission on table X, they can add a new data file
to the location. Similarly, with DROP permission, they can delete files in the
location.
This assumption is compromised when a compromised location belongs to a
table. I think CREATE TABLE or ALTER TABLE properly checks DFS_URI, so DROP
TABLE can assume the location is valid.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
