lasdf1234 commented on code in PR #14482: URL: https://github.com/apache/gravitino/pull/14482#discussion_r4465370776
########## iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/server/web/filter/RegisterTableAuthzHandler.java: ########## @@ -0,0 +1,196 @@ +/* + * 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.gravitino.server.web.filter; + +import static org.apache.gravitino.server.authorization.expression.AuthorizationExpressionConstants.LOAD_TABLE_AUTHORIZATION_EXPRESSION; +import static org.apache.gravitino.server.authorization.expression.AuthorizationExpressionConstants.MODIFY_TABLE_AUTHORIZATION_EXPRESSION; + +import java.lang.reflect.Parameter; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.regex.Pattern; +import org.apache.gravitino.Entity.EntityType; +import org.apache.gravitino.NameIdentifier; +import org.apache.gravitino.authorization.AuthorizationRequestContext; +import org.apache.gravitino.exceptions.ForbiddenException; +import org.apache.gravitino.iceberg.service.CatalogWrapperForREST; +import org.apache.gravitino.iceberg.service.authorization.IcebergRESTServerContext; +import org.apache.gravitino.iceberg.service.dispatcher.RegisterTableSourceResolver; +import org.apache.gravitino.server.authorization.annotations.IcebergAuthorizationMetadata; +import org.apache.gravitino.server.authorization.expression.AuthorizationExpressionEvaluator; +import org.apache.gravitino.server.web.filter.BaseMetadataAuthorizationMethodInterceptor.AuthorizationHandler; +import org.apache.gravitino.utils.HierarchicalSchemaUtil; +import org.apache.gravitino.utils.NameIdentifierUtil; +import org.apache.gravitino.utils.PrincipalUtils; +import org.apache.iceberg.TableMetadata; +import org.apache.iceberg.catalog.Namespace; +import org.apache.iceberg.catalog.TableIdentifier; +import org.apache.iceberg.rest.requests.RegisterTableRequest; + +/** + * Authorizes Iceberg REST {@code registerTable} requests that the method annotation alone cannot + * express. + * + * <p>Destination authorization runs first, before any metadata FileIO or catalog scan: overwrite + * uses the modify-table expression on the named table; a plain register uses the same create-table + * expression as {@code IcebergNamespaceOperations#registerTable}. Authorization then completes, so + * the shared interceptor does not re-evaluate the method expression. When the metadata file still + * belongs to another live table, the load-table expression on that table is required so {@code + * CREATE_TABLE} alone cannot adopt an unreadable table's metadata. + */ +@SuppressWarnings("FormatStringAnnotation") +public class RegisterTableAuthzHandler implements AuthorizationHandler { + + /** Same create expression as {@code IcebergNamespaceOperations#registerTable}. */ + private static final String CREATE_TABLE_EXPRESSION = + "ANY(OWNER, METALAKE, CATALOG) || " + + "SCHEMA_OWNER_WITH_USE_CATALOG || " + + "ANY_USE_CATALOG && ANY_USE_SCHEMA && ANY_CREATE_TABLE"; + + private static final Pattern SCHEMA_SPLIT = + Pattern.compile(Pattern.quote(HierarchicalSchemaUtil.schemaSeparator())); + + private final Parameter[] parameters; + private final Object[] args; + + /** + * Creates a handler for the register-table request arguments. + * + * @param parameters invoked method parameters + * @param args invoked method arguments + */ + public RegisterTableAuthzHandler(Parameter[] parameters, Object[] args) { + this.parameters = parameters; + this.args = args; + } + + @Override + public void process(Map<EntityType, NameIdentifier> nameIdentifierMap) { + RegisterTableRequest request = registerTableRequest(); + + NameIdentifier metalakeIdent = nameIdentifierMap.get(EntityType.METALAKE); + NameIdentifier catalogIdent = nameIdentifierMap.get(EntityType.CATALOG); + NameIdentifier schemaIdent = nameIdentifierMap.get(EntityType.SCHEMA); + if (metalakeIdent == null || catalogIdent == null || schemaIdent == null) { + throw new ForbiddenException( + "Missing metalake, catalog, or schema context for register-table authorization"); + } + + String metalakeName = metalakeIdent.name(); + String catalog = catalogIdent.name(); + String schema = schemaIdent.name(); + String tableName = request.name(); + nameIdentifierMap.put( + EntityType.TABLE, NameIdentifierUtil.ofTable(metalakeName, catalog, schema, tableName)); + + requireCanAccessDestination(nameIdentifierMap, request.overwrite()); + authorizeMetadataSources(metalakeName, catalog, schema, tableName, request); + } + + @Override + public boolean authorizationCompleted() { + // Destination create/overwrite and any source checks are finished in process(). + return true; + } + + @Override + public EntityType entityType(EntityType defaultType) { Review Comment: Done in d2133eb667. `AuthorizationHandler.entityType()` is now a no-arg method (no `defaultType` parameter). ########## server-common/src/main/java/org/apache/gravitino/server/web/filter/BaseMetadataAuthorizationMethodInterceptor.java: ########## Review Comment: Updated to your suggested assignment: ```java Entity.EntityType entityType = handler.isPresent() ? handler.get().entityType() : target.entityType(); deniedObject = nameIdentifierMap.get(entityType); ``` `RegisterTableAuthzHandler` binds the destination TABLE id when constructed so overwrite denials can attribute before `process()`. ########## server-common/src/main/java/org/apache/gravitino/server/web/filter/BaseMetadataAuthorizationMethodInterceptor.java: ########## @@ -336,7 +348,16 @@ private Object authorizeMethodInScope( if (!skipStandardCheck && handler.isPresent()) { AuthorizationHandler authzHandler = handler.get(); - authzHandler.process(nameIdentifierMap); + try { + authzHandler.process(nameIdentifierMap); + } finally { Review Comment: Removed the try/finally around `process()`. Denial attribution now uses only the assignment above. -- 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]
