sachinnn99 commented on code in PR #10699:
URL: https://github.com/apache/gravitino/pull/10699#discussion_r3049980900
##########
iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/CatalogWrapperForREST.java:
##########
@@ -130,6 +131,18 @@ public LoadTableResponse loadTable(
return loadTableResponse;
}
+ public LoadTableResponse registerTable(
+ Namespace namespace, RegisterTableRequest request, boolean
requestCredential) {
+ LoadTableResponse loadTableResponse = super.registerTable(namespace,
request);
+ if (shouldGenerateCredential(loadTableResponse, requestCredential)) {
+ return injectCredentialConfig(
+ TableIdentifier.of(namespace, request.name()),
+ loadTableResponse,
+ CredentialPrivilege.WRITE);
Review Comment:
You're right, and after digging in I'm switching to READ.
Specifically:
1. **registerTable writes no files at the storage layer.** Verified
end-to-end: `CatalogHandlers.registerTable` →
`BaseMetastoreCatalog.registerTable` → `ops.commit(null, metadata)` →
`BaseMetastoreTableOperations.writeNewMetadataIfRequired(true, metadata)`
early-returns the existing `metadata.metadataFileLocation()` without writing,
then `JdbcTableOperations` just inserts the catalog row. So the storage-layer
asymmetry you flagged with `createTable` is real — `createTable` proves write
capability by writing a metadata.json file, `registerTable` doesn't.
2. **registerTable also doesn't set ownership.**
`IcebergNamespaceOperationExecutor.registerTable` is a straight delegation to
the wrapper — unlike `IcebergTableOperationExecutor.createTable` and
`IcebergNamespaceOperationExecutor.createNamespace`, which both explicitly set
`IcebergConstants.OWNER` to `context.userName()`. So a user with only
`ANY_CREATE_TABLE` privilege never becomes the table owner via register.
3. **WRITE was therefore a one-off privilege escalation.** That same user
cannot pass `FILTER_MODIFY_TABLE_AUTHORIZATION_EXPRESSION` (`ANY(OWNER,
METALAKE, CATALOG, SCHEMA, TABLE) || ANY_MODIFY_TABLE`) on the registered table
via any other API path. A follow-up `loadTable` with vending would return READ
for them via `getCredentialPrivilege`. Hardcoding WRITE here was vending
storage credentials that let them perform actions gravitino's authz model
explicitly forbids.
Switching to READ. Higher-privileged users (catalog/metalake owners,
`MODIFY_TABLE` holders) who legitimately need to write to a registered table
can call `loadTable` with vending afterwards and get WRITE through the dynamic
helper, or be granted `MODIFY_TABLE` explicitly.
I've added a code comment documenting all of this.
--
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]
