This is an automated email from the ASF dual-hosted git repository.
yuqi1129 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 2ad6ceb6cf [#12559] feat(lance): Add namespace write authorization for
Lance REST (#12693)
2ad6ceb6cf is described below
commit 2ad6ceb6cfdfe6d2c4b68476a3b075f975dec9f6
Author: Qi Yu <[email protected]>
AuthorDate: Sat Aug 29 09:59:23 2026 +0800
[#12559] feat(lance): Add namespace write authorization for Lance REST
(#12693)
### What changes were proposed in this pull request?
Authorizes the Lance REST create-namespace and drop-namespace
operations.
- A one-level namespace identifier addresses a Gravitino catalog and a
two-level identifier a schema, so both expressions select the required
privileges with an `entityType` guard. Creating a catalog requires
`CREATE_CATALOG` on the metalake; creating a schema requires
`USE_CATALOG` together with `CREATE_SCHEMA`.
- Dropping a namespace requires ownership of it or of one of its
ancestors, matching the Gravitino and Iceberg REST surfaces.
- `mode=overwrite` replaces an existing namespace, so it is authorized
against the ownership expression through an `AuthorizationHandler`
rather than the create expression on the method. The mode travels in the
request body, so it cannot be expressed by the method annotation alone.
- The namespace expressions move into a new
`LanceAuthorizationExpressions` holder now that there is more than one
of them.
Assigning the caller as owner after a successful create needs no new
code: in auxiliary mode Lance runs its writes through the Gravitino
catalog and schema dispatchers, whose hooks already set the owner. The
integration test verifies this end to end.
### Why are the changes needed?
Namespace writes were the last unauthorized part of the Lance REST
namespace surface after #12558 added the framework and the read paths.
Fix: #12559
### Does this PR introduce _any_ user-facing change?
Yes. When `gravitino.authorization.enable` is true and Lance REST runs
as an auxiliary service with a metalake configured, create-namespace and
drop-namespace are now authorized and denied requests return 403 instead
of succeeding.
One behavior is worth calling out: `mode=overwrite` always requires
ownership, whether or not the namespace already exists. Deciding this
from the mode alone avoids an existence probe at authorization time,
which would race with the create that follows it and make the required
privilege depend on that race. A caller holding only `CREATE_SCHEMA`
should use `create` or `exist_ok`.
### How was this patch tested?
- `TestLanceMetadataAuthorizationMethodInterceptor`: four new tests
covering create at each level, the overwrite escalation guard,
owner-authorized overwrite and drop, and drop denial for a non-owner.
- `LanceNamespaceAuthorizationIT`: three new tests covering create
denial and success, ownership after create, a denied overwrite leaving
the stored properties untouched, and drop concealing a namespace the
caller may not see.
- `./gradlew :lance:lance-rest-server:build :lance:lance-common:build` —
171 tests, all passing.
---
.../lance/common/ops/gravitino/CommonUtil.java | 15 ++-
.../LanceAuthorizationExpressions.java | 73 ++++++++++++++
...anceMetadataAuthorizationMethodInterceptor.java | 91 +++++++++++++++++
.../service/rest/LanceNamespaceOperations.java | 14 +--
.../test/LanceNamespaceAuthorizationIT.java | 105 +++++++++++++++++++
...anceMetadataAuthorizationMethodInterceptor.java | 112 ++++++++++++++++++++-
6 files changed, 396 insertions(+), 14 deletions(-)
diff --git
a/lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/CommonUtil.java
b/lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/CommonUtil.java
index 056b2c235b..76b5593995 100644
---
a/lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/CommonUtil.java
+++
b/lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/CommonUtil.java
@@ -23,7 +23,7 @@ import java.util.Locale;
import org.lance.namespace.errors.InvalidInputException;
/** Utility methods used by Gravitino Lance namespace operations. */
-class CommonUtil {
+public class CommonUtil {
private CommonUtil() {}
@@ -31,7 +31,18 @@ class CommonUtil {
return Throwables.getStackTraceAsString(new RuntimeException("Captured
stacktrace"));
}
- static String normalizeToken(String value) {
+ /**
+ * Normalizes a request token the way every mode and behavior parameter is
read, so that callers
+ * deciding something from a token compare it exactly as the operation that
acts on it will.
+ *
+ * <p>Authorization relies on this: a mode that reaches the operation as
{@code OVERWRITE} has to
+ * be recognized as an overwrite while the request is being authorized,
whatever spacing or case
+ * the client sent.
+ *
+ * @param value the raw token, may be null
+ * @return the trimmed, upper-cased token, or an empty string when the value
is null
+ */
+ public static String normalizeToken(String value) {
return value == null ? "" : value.trim().toUpperCase(Locale.ROOT);
}
diff --git
a/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/authorization/LanceAuthorizationExpressions.java
b/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/authorization/LanceAuthorizationExpressions.java
new file mode 100644
index 0000000000..0ea9dc9131
--- /dev/null
+++
b/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/authorization/LanceAuthorizationExpressions.java
@@ -0,0 +1,73 @@
+/*
+ * 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.lance.service.authorization;
+
+import static
org.apache.gravitino.server.authorization.expression.AuthorizationExpressionConstants.CAN_ACCESS_METADATA;
+import static
org.apache.gravitino.server.authorization.expression.AuthorizationExpressionConstants.PROBE_SCHEMA_AUTHORIZATION_EXPRESSION;
+
+/**
+ * Authorization expressions for the Lance REST namespace surface.
+ *
+ * <p>A Lance namespace identifier addresses a Gravitino catalog at one level
and a Gravitino schema
+ * at two levels, so every expression here selects the privileges to require
with an {@code
+ * entityType} guard. The interceptor reports the addressed entity type, and
an identifier that
+ * resolves to any other type matches no branch and is therefore denied.
+ */
+public final class LanceAuthorizationExpressions {
+
+ /**
+ * Authorizes a namespace existence probe. Catalog probes use normal catalog
access. Schema probes
+ * additionally permit CREATE_SCHEMA, because clients commonly check
existence immediately before
+ * creating a schema.
+ */
+ public static final String PROBE_NAMESPACE_AUTHORIZATION_EXPRESSION =
+ CAN_ACCESS_METADATA
+ + " || (entityType == 'SCHEMA' && ("
+ + PROBE_SCHEMA_AUTHORIZATION_EXPRESSION
+ + "))";
+
+ /**
+ * Authorizes creating a namespace. A one-level namespace creates a catalog
and therefore requires
+ * CREATE_CATALOG on the metalake; a two-level namespace creates a schema
and requires
+ * CREATE_SCHEMA together with access to the parent catalog.
+ *
+ * <p>This expression covers the {@code create} and {@code exist_ok} modes
only. The {@code
+ * overwrite} mode alters an existing namespace, so it is authorized against
{@link
+ * #MODIFY_NAMESPACE_AUTHORIZATION_EXPRESSION} instead and a create
privilege alone never grants
+ * it.
+ */
+ public static final String CREATE_NAMESPACE_AUTHORIZATION_EXPRESSION =
+ """
+ (entityType == 'CATALOG' && (METALAKE::OWNER ||
METALAKE::CREATE_CATALOG)) ||
+ (entityType == 'SCHEMA' && (ANY(OWNER, METALAKE, CATALOG) ||
+ ANY_USE_CATALOG && ANY_CREATE_SCHEMA))
+ """;
+
+ /**
+ * Authorizes altering or dropping an existing namespace. Both require
ownership of the namespace
+ * or of one of its ancestors, matching the Gravitino and Iceberg REST
surfaces.
+ */
+ public static final String MODIFY_NAMESPACE_AUTHORIZATION_EXPRESSION =
+ """
+ (entityType == 'CATALOG' && ANY(OWNER, METALAKE, CATALOG)) ||
+ (entityType == 'SCHEMA' && (ANY(OWNER, METALAKE, CATALOG) ||
SCHEMA_OWNER_WITH_USE_CATALOG))
+ """;
+
+ private LanceAuthorizationExpressions() {}
+}
diff --git
a/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/authorization/LanceMetadataAuthorizationMethodInterceptor.java
b/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/authorization/LanceMetadataAuthorizationMethodInterceptor.java
index f2c4aea48d..3233976bc9 100644
---
a/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/authorization/LanceMetadataAuthorizationMethodInterceptor.java
+++
b/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/authorization/LanceMetadataAuthorizationMethodInterceptor.java
@@ -32,17 +32,22 @@ import org.aopalliance.intercept.MethodInterceptor;
import org.aopalliance.intercept.MethodInvocation;
import org.apache.gravitino.Entity;
import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.authorization.AuthorizationRequestContext;
import org.apache.gravitino.exceptions.ForbiddenException;
import org.apache.gravitino.lance.common.ops.NamespaceWrapper;
+import org.apache.gravitino.lance.common.ops.gravitino.CommonUtil;
import org.apache.gravitino.lance.common.ops.gravitino.ObjectIdentifier;
import org.apache.gravitino.lance.service.LanceExceptionMapper;
import
org.apache.gravitino.lance.service.authorization.annotations.LanceRootNamespace;
import
org.apache.gravitino.server.authorization.annotations.AuthorizationExpression;
+import
org.apache.gravitino.server.authorization.expression.AuthorizationExpressionEvaluator;
import
org.apache.gravitino.server.web.filter.BaseMetadataAuthorizationMethodInterceptor;
import org.apache.gravitino.utils.NameIdentifierUtil;
+import org.apache.gravitino.utils.PrincipalUtils;
import org.lance.namespace.errors.InvalidInputException;
import org.lance.namespace.errors.LanceNamespaceException;
import org.lance.namespace.errors.PermissionDeniedException;
+import org.lance.namespace.model.CreateNamespaceRequest;
/** Resolves Lance namespace IDs and maps shared authorization failures to
Lance REST responses. */
public class LanceMetadataAuthorizationMethodInterceptor
@@ -109,6 +114,27 @@ public class LanceMetadataAuthorizationMethodInterceptor
return new AuthorizationTarget(identifiers, Entity.EntityType.CATALOG);
}
+ /**
+ * Returns the handler that authorizes an overwrite of an existing
namespace. Overwrite is the
+ * only Lance namespace write whose required privileges are carried in the
request body rather
+ * than in the request path, so it cannot be expressed by the method
annotation alone.
+ *
+ * @param method invoked protocol method
+ * @param parameters invoked method parameters
+ * @param args invoked method arguments
+ * @return the overwrite handler when the request carries a create-namespace
body
+ */
+ @Override
+ protected Optional<AuthorizationHandler> createAuthorizationHandler(
+ Method method, Parameter[] parameters, Object[] args) {
+ for (Object arg : args) {
+ if (arg instanceof CreateNamespaceRequest) {
+ return Optional.of(new
CreateNamespaceAuthzHandler((CreateNamespaceRequest) arg));
+ }
+ }
+ return Optional.empty();
+ }
+
@Override
protected boolean shouldSkipExpressionEvaluation(AuthorizationTarget target)
{
// The root has no privilege-bearing Lance object. The shared pipeline
still validates the
@@ -137,6 +163,71 @@ public class LanceMetadataAuthorizationMethodInterceptor
return LanceExceptionMapper.toRESTResponse(namespaceId, exception);
}
+ /**
+ * Authorizes a create-namespace request whose mode overwrites an existing
namespace.
+ *
+ * <p>An overwrite replaces the properties of a namespace that already
exists, so it is a
+ * modification rather than a creation and is authorized against {@link
+ * LanceAuthorizationExpressions#MODIFY_NAMESPACE_AUTHORIZATION_EXPRESSION}.
The mode alone
+ * decides this, without probing whether the namespace exists: an existence
probe at authorization
+ * time would race with the create that follows it, and the resulting
privilege requirement would
+ * depend on that race.
+ */
+ private static final class CreateNamespaceAuthzHandler implements
AuthorizationHandler {
+
+ private static final String OVERWRITE_MODE = "OVERWRITE";
+
+ private final CreateNamespaceRequest request;
+ private boolean overwriteAuthorized;
+
+ private CreateNamespaceAuthzHandler(CreateNamespaceRequest request) {
+ this.request = request;
+ }
+
+ @Override
+ public void process(Map<Entity.EntityType, NameIdentifier>
nameIdentifierMap) {
+ if (!isOverwrite()) {
+ return;
+ }
+
+ // A namespace that is being overwritten already exists, so the create
expression on the
+ // method must not be evaluated: it would let CREATE_CATALOG or
CREATE_SCHEMA replace an
+ // object the caller does not own.
+ overwriteAuthorized = true;
+ Entity.EntityType entityType =
+ nameIdentifierMap.containsKey(Entity.EntityType.SCHEMA)
+ ? Entity.EntityType.SCHEMA
+ : Entity.EntityType.CATALOG;
+ boolean authorized =
+ new AuthorizationExpressionEvaluator(
+
LanceAuthorizationExpressions.MODIFY_NAMESPACE_AUTHORIZATION_EXPRESSION)
+ .evaluate(
+ nameIdentifierMap,
+ new HashMap<>(),
+ new AuthorizationRequestContext(),
+ Optional.of(entityType.name()));
+ if (!authorized) {
+ throw new ForbiddenException(
+ "User '%s' is not authorized to overwrite the namespace '%s'",
+ PrincipalUtils.getCurrentUserName(),
nameIdentifierMap.get(entityType));
+ }
+ }
+
+ @Override
+ public boolean authorizationCompleted() {
+ return overwriteAuthorized;
+ }
+
+ private boolean isOverwrite() {
+ // Read the mode through the same normalization the create operation
applies, so a token the
+ // operation will act on as an overwrite cannot be authorized as a plain
create. Comparing the
+ // raw string here would leave a gap: " overwrite " reaches the
operation as OVERWRITE but
+ // would not match, and a caller holding only a create privilege could
replace a namespace
+ // owned by somebody else.
+ return
OVERWRITE_MODE.equals(CommonUtil.normalizeToken(request.getMode()));
+ }
+ }
+
private Map<Entity.EntityType, NameIdentifier> baseIdentifiers() {
Map<Entity.EntityType, NameIdentifier> identifiers = new HashMap<>();
identifiers.put(Entity.EntityType.METALAKE,
NameIdentifierUtil.ofMetalake(metalakeName));
diff --git
a/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/rest/LanceNamespaceOperations.java
b/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/rest/LanceNamespaceOperations.java
index fac831a252..0001ec6ce9 100644
---
a/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/rest/LanceNamespaceOperations.java
+++
b/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/rest/LanceNamespaceOperations.java
@@ -19,8 +19,10 @@
package org.apache.gravitino.lance.service.rest;
import static
org.apache.gravitino.lance.common.ops.NamespaceWrapper.NAMESPACE_DELIMITER_DEFAULT;
+import static
org.apache.gravitino.lance.service.authorization.LanceAuthorizationExpressions.CREATE_NAMESPACE_AUTHORIZATION_EXPRESSION;
+import static
org.apache.gravitino.lance.service.authorization.LanceAuthorizationExpressions.MODIFY_NAMESPACE_AUTHORIZATION_EXPRESSION;
+import static
org.apache.gravitino.lance.service.authorization.LanceAuthorizationExpressions.PROBE_NAMESPACE_AUTHORIZATION_EXPRESSION;
import static
org.apache.gravitino.server.authorization.expression.AuthorizationExpressionConstants.CAN_ACCESS_METADATA;
-import static
org.apache.gravitino.server.authorization.expression.AuthorizationExpressionConstants.PROBE_SCHEMA_AUTHORIZATION_EXPRESSION;
import com.codahale.metrics.annotation.ResponseMetered;
import com.codahale.metrics.annotation.Timed;
@@ -56,14 +58,6 @@ public class LanceNamespaceOperations {
private static final String ROOT_NAMESPACE_ID = "";
- // Catalog probes use normal catalog access. Schema probes additionally
permit CREATE_SCHEMA,
- // because clients commonly check existence immediately before creating a
schema.
- private static final String PROBE_NAMESPACE_AUTHORIZATION_EXPRESSION =
- CAN_ACCESS_METADATA
- + " || (entityType == 'SCHEMA' && ("
- + PROBE_SCHEMA_AUTHORIZATION_EXPRESSION
- + "))";
-
private final NamespaceWrapper lanceNamespace;
@Inject
@@ -131,6 +125,7 @@ public class LanceNamespaceOperations {
@Path("/{id}/create")
@Timed(name = "create-namespaces." + MetricNames.HTTP_PROCESS_DURATION,
absolute = true)
@ResponseMetered(name = "create-namespaces", absolute = true)
+ @AuthorizationExpression(expression =
CREATE_NAMESPACE_AUTHORIZATION_EXPRESSION)
public Response createNamespace(
@PathParam("id") String namespaceId,
@DefaultValue(NAMESPACE_DELIMITER_DEFAULT) @QueryParam("delimiter")
String delimiter,
@@ -154,6 +149,7 @@ public class LanceNamespaceOperations {
@Path("/{id}/drop")
@Timed(name = "drop-namespaces." + MetricNames.HTTP_PROCESS_DURATION,
absolute = true)
@ResponseMetered(name = "drop-namespaces", absolute = true)
+ @AuthorizationExpression(expression =
MODIFY_NAMESPACE_AUTHORIZATION_EXPRESSION)
public Response dropNamespace(
@PathParam("id") String namespaceId,
@DefaultValue(NAMESPACE_DELIMITER_DEFAULT) @QueryParam("delimiter")
String delimiter,
diff --git
a/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/integration/test/LanceNamespaceAuthorizationIT.java
b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/integration/test/LanceNamespaceAuthorizationIT.java
index 309abd248e..5226a20beb 100644
---
a/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/integration/test/LanceNamespaceAuthorizationIT.java
+++
b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/integration/test/LanceNamespaceAuthorizationIT.java
@@ -27,6 +27,7 @@ import java.util.ArrayList;
import java.util.Base64;
import java.util.HashMap;
import java.util.List;
+import java.util.Map;
import org.apache.gravitino.Configs;
import org.apache.gravitino.auth.AuthConstants;
import org.apache.gravitino.authorization.Privileges;
@@ -40,6 +41,8 @@ import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.lance.namespace.model.CreateNamespaceRequest;
+import org.lance.namespace.model.DescribeNamespaceResponse;
+import org.lance.namespace.model.DropNamespaceRequest;
import org.lance.namespace.model.ListNamespacesResponse;
/** Verifies namespace authorization and list filtering through auxiliary-mode
Lance REST. */
@@ -47,10 +50,15 @@ public class LanceNamespaceAuthorizationIT extends BaseIT {
private static final String ADMIN = "lance_authz_admin";
private static final String USER = "lance_authz_user";
+ private static final String WRITER = "lance_authz_writer";
private static final String VISIBLE_CATALOG = "lance_authz_visible_catalog";
private static final String HIDDEN_CATALOG = "lance_authz_hidden_catalog";
private static final String VISIBLE_SCHEMA = "lance_authz_visible_schema";
private static final String HIDDEN_SCHEMA = "lance_authz_hidden_schema";
+ private static final String WRITER_CATALOG = "lance_authz_writer_catalog";
+ private static final String WRITER_SCHEMA = "lance_authz_writer_schema";
+ private static final String MISSING_CATALOG = "lance_authz_missing_catalog";
+ private static final String MARKER_PROPERTY = "lance-authz-marker";
private static final String DELIMITER = ".";
private final HttpClient httpClient = HttpClient.newHttpClient();
@@ -68,6 +76,7 @@ public class LanceNamespaceAuthorizationIT extends BaseIT {
client.createMetalake(metalakeName, "Lance authorization tests", null);
GravitinoMetalake metalake = client.loadMetalake(metalakeName);
metalake.addUser(USER);
+ metalake.addUser(WRITER);
createNamespace(ADMIN, VISIBLE_CATALOG);
createNamespace(ADMIN, HIDDEN_CATALOG);
createNamespace(ADMIN, id(VISIBLE_CATALOG, VISIBLE_SCHEMA));
@@ -85,6 +94,20 @@ public class LanceNamespaceAuthorizationIT extends BaseIT {
SecurableObjects.ofCatalog(VISIBLE_CATALOG, new ArrayList<>()),
VISIBLE_SCHEMA,
new ArrayList<>(List.of(Privileges.UseSchema.allow()))));
+
+ // The writer is a separate user so that granting create privileges cannot
change what the
+ // read-only user above is allowed to see.
+ metalake.createRole(
+ "lance_authz_writer_role",
+ new HashMap<>(),
+ List.of(
+ SecurableObjects.ofMetalake(
+ metalakeName, new
ArrayList<>(List.of(Privileges.CreateCatalog.allow()))),
+ SecurableObjects.ofCatalog(
+ VISIBLE_CATALOG,
+ new ArrayList<>(
+ List.of(Privileges.UseCatalog.allow(),
Privileges.CreateSchema.allow())))));
+ metalake.grantRolesToUser(List.of("lance_authz_writer_role"), WRITER);
}
@AfterAll
@@ -116,6 +139,48 @@ public class LanceNamespaceAuthorizationIT extends BaseIT {
assertStatus(200, post(ADMIN, HIDDEN_CATALOG, "describe"));
}
+ @Test
+ public void testCreateNamespaceRequiresCreatePrivilege() throws Exception {
+ // The read-only user holds no create privilege at either level.
+ assertStatus(403, create(USER, "lance_authz_denied_catalog", null,
Map.of()));
+ assertStatus(
+ 403, create(USER, id(VISIBLE_CATALOG, "lance_authz_denied_schema"),
null, Map.of()));
+
+ assertStatus(200, create(WRITER, WRITER_CATALOG, null, Map.of()));
+ assertStatus(200, create(WRITER, id(VISIBLE_CATALOG, WRITER_SCHEMA), null,
Map.of()));
+
+ // Creating assigns ownership, so the creator can drop what it created.
+ assertStatus(200, drop(WRITER, id(VISIBLE_CATALOG, WRITER_SCHEMA), null,
null));
+ assertStatus(200, drop(WRITER, WRITER_CATALOG, null, "cascade"));
+ }
+
+ @Test
+ public void testCreatePrivilegeCannotOverwriteOrDropAnotherOwnersNamespace()
throws Exception {
+ Map<String, String> marker = Map.of(MARKER_PROPERTY, "overwritten");
+ assertStatus(403, create(WRITER, VISIBLE_CATALOG, "overwrite", marker));
+ assertStatus(403, create(WRITER, id(VISIBLE_CATALOG, VISIBLE_SCHEMA),
"overwrite", marker));
+
+ // A denied overwrite must not have reached the metadata store.
+ Assertions.assertFalse(properties(ADMIN,
VISIBLE_CATALOG).containsKey(MARKER_PROPERTY));
+ Assertions.assertFalse(
+ properties(ADMIN, id(VISIBLE_CATALOG,
VISIBLE_SCHEMA)).containsKey(MARKER_PROPERTY));
+
+ // exist_ok is a create, not a modification, so the create privilege is
enough for it.
+ assertStatus(200, create(WRITER, VISIBLE_CATALOG, "exist_ok", Map.of()));
+ Assertions.assertFalse(properties(ADMIN,
VISIBLE_CATALOG).containsKey(MARKER_PROPERTY));
+
+ assertStatus(403, drop(WRITER, id(VISIBLE_CATALOG, HIDDEN_SCHEMA), null,
null));
+ assertStatus(200, post(ADMIN, id(VISIBLE_CATALOG, HIDDEN_SCHEMA),
"exists"));
+ }
+
+ @Test
+ public void testDropConcealsNamespacesTheCallerMayNotSee() throws Exception {
+ // A namespace the caller cannot drop is reported as forbidden whether or
not it exists, so a
+ // caller cannot probe for existence through the drop endpoint.
+ assertStatus(403, drop(WRITER, MISSING_CATALOG, "skip", null));
+ assertStatus(403, drop(USER, HIDDEN_CATALOG, "skip", null));
+ }
+
private void grant(GravitinoMetalake metalake, String role, SecurableObject
object) {
metalake.createRole(role, new HashMap<>(), List.of(object));
metalake.grantRolesToUser(List.of(role), USER);
@@ -151,6 +216,46 @@ public class LanceNamespaceAuthorizationIT extends BaseIT {
assertStatus(200, httpClient.send(request,
HttpResponse.BodyHandlers.ofString()));
}
+ private HttpResponse<String> create(
+ String user, String namespaceId, String mode, Map<String, String>
properties)
+ throws Exception {
+ CreateNamespaceRequest body = new CreateNamespaceRequest();
+ for (String level : namespaceId.split("\\" + DELIMITER)) {
+ body.addIdItem(level);
+ }
+ body.setMode(mode);
+ body.setProperties(new HashMap<>(properties));
+ return send(user, "/v1/namespace/" + namespaceId + "/create", body);
+ }
+
+ private HttpResponse<String> drop(String user, String namespaceId, String
mode, String behavior)
+ throws Exception {
+ DropNamespaceRequest body = new DropNamespaceRequest();
+ body.setMode(mode);
+ body.setBehavior(behavior);
+ return send(user, "/v1/namespace/" + namespaceId + "/drop", body);
+ }
+
+ private Map<String, String> properties(String user, String namespaceId)
throws Exception {
+ HttpResponse<String> response = post(user, namespaceId, "describe");
+ assertStatus(200, response);
+ Map<String, String> properties =
+ ObjectMapperProvider.objectMapper()
+ .readValue(response.body(), DescribeNamespaceResponse.class)
+ .getProperties();
+ return properties == null ? Map.of() : properties;
+ }
+
+ private HttpResponse<String> send(String user, String path, Object body)
throws Exception {
+ HttpRequest request =
+ request(user, path)
+ .POST(
+ HttpRequest.BodyPublishers.ofString(
+
ObjectMapperProvider.objectMapper().writeValueAsString(body)))
+ .build();
+ return httpClient.send(request, HttpResponse.BodyHandlers.ofString());
+ }
+
private HttpResponse<String> post(String user, String namespaceId, String
operation)
throws Exception {
HttpRequest request =
diff --git
a/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/authorization/TestLanceMetadataAuthorizationMethodInterceptor.java
b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/authorization/TestLanceMetadataAuthorizationMethodInterceptor.java
index 356c6089b0..1f41e7bc30 100644
---
a/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/authorization/TestLanceMetadataAuthorizationMethodInterceptor.java
+++
b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/authorization/TestLanceMetadataAuthorizationMethodInterceptor.java
@@ -21,6 +21,7 @@ package org.apache.gravitino.lance.service.authorization;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.mockStatic;
import static org.mockito.Mockito.never;
@@ -45,6 +46,8 @@ import org.apache.gravitino.utils.PrincipalUtils;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
+import org.lance.namespace.model.CreateNamespaceRequest;
+import org.lance.namespace.model.DropNamespaceRequest;
import org.lance.namespace.model.ErrorResponse;
import org.mockito.MockedStatic;
@@ -152,10 +155,109 @@ class TestLanceMetadataAuthorizationMethodInterceptor {
assertErrorResponse(interceptor.invoke(operation),
Response.Status.BAD_REQUEST);
}
+ @Test
+ void testCreateNamespaceRequiresCreatePrivilegeAtEachLevel() throws
Throwable {
+ allow(Privilege.Name.CREATE_CATALOG);
+ assertEquals(PROCEEDED, interceptor.invoke(createInvocation(CATALOG, "$",
"create")));
+ assertErrorResponse(
+ interceptor.invoke(createInvocation(CATALOG + "$" + SCHEMA, "$",
"create")),
+ Response.Status.FORBIDDEN);
+
+ allow(Privilege.Name.USE_CATALOG, Privilege.Name.CREATE_SCHEMA);
+ assertEquals(
+ PROCEEDED, interceptor.invoke(createInvocation(CATALOG + "$" + SCHEMA,
"$", "create")));
+ assertErrorResponse(
+ interceptor.invoke(createInvocation(CATALOG, "$", "create")),
Response.Status.FORBIDDEN);
+ }
+
+ @Test
+ void testCreatePrivilegeCannotOverwriteAnExistingNamespace() throws
Throwable {
+ allow(Privilege.Name.CREATE_CATALOG, Privilege.Name.USE_CATALOG,
Privilege.Name.CREATE_SCHEMA);
+
+ // The same privileges that authorize a create must not authorize an
overwrite, which replaces
+ // the properties of a namespace the caller does not own.
+ assertEquals(PROCEEDED, interceptor.invoke(createInvocation(CATALOG, "$",
"exist_ok")));
+ assertErrorResponse(
+ interceptor.invoke(createInvocation(CATALOG, "$", "overwrite")),
Response.Status.FORBIDDEN);
+ assertErrorResponse(
+ interceptor.invoke(createInvocation(CATALOG + "$" + SCHEMA, "$",
"OVERWRITE")),
+ Response.Status.FORBIDDEN);
+ }
+
+ @Test
+ void testOverwriteIsRecognizedWhateverSpacingTheClientSends() throws
Throwable {
+ allow(Privilege.Name.CREATE_CATALOG, Privilege.Name.USE_CATALOG,
Privilege.Name.CREATE_SCHEMA);
+
+ // The create operation reads the mode through CommonUtil.normalizeToken,
which trims and
+ // upper-cases, so all of these reach it as OVERWRITE. Authorization has
to read the mode the
+ // same way: a token this interceptor fails to recognize is authorized as
a plain create, and
+ // the create privileges above would then be enough to replace a namespace
owned by somebody
+ // else.
+ for (String mode : new String[] {" overwrite", "overwrite ", " OverWrite
", "\toverwrite\n"}) {
+ assertErrorResponse(
+ interceptor.invoke(createInvocation(CATALOG, "$", mode)),
+ Response.Status.FORBIDDEN,
+ "mode '" + mode + "' must be authorized as an overwrite");
+ assertErrorResponse(
+ interceptor.invoke(createInvocation(CATALOG + "$" + SCHEMA, "$",
mode)),
+ Response.Status.FORBIDDEN,
+ "mode '" + mode + "' must be authorized as an overwrite");
+ }
+ }
+
+ @Test
+ void testOwnerMayOverwriteAndDropNamespaces() throws Throwable {
+ when(authorizer.isOwner(any(), any(), any(), any())).thenReturn(true);
+ allow(Privilege.Name.USE_CATALOG);
+
+ assertEquals(PROCEEDED, interceptor.invoke(createInvocation(CATALOG, "$",
"overwrite")));
+ assertEquals(
+ PROCEEDED, interceptor.invoke(createInvocation(CATALOG + "$" + SCHEMA,
"$", "overwrite")));
+ assertEquals(PROCEEDED, interceptor.invoke(dropInvocation(CATALOG, "$")));
+ assertEquals(PROCEEDED, interceptor.invoke(dropInvocation(CATALOG + "$" +
SCHEMA, "$")));
+ }
+
+ @Test
+ void testDropNamespaceRequiresOwnership() throws Throwable {
+ allow(
+ Privilege.Name.USE_CATALOG,
+ Privilege.Name.CREATE_CATALOG,
+ Privilege.Name.CREATE_SCHEMA,
+ Privilege.Name.USE_SCHEMA);
+
+ MethodInvocation dropCatalog = dropInvocation(CATALOG, "$");
+ assertErrorResponse(interceptor.invoke(dropCatalog),
Response.Status.FORBIDDEN);
+ verify(dropCatalog, never()).proceed();
+
+ MethodInvocation dropSchema = dropInvocation(CATALOG + "$" + SCHEMA, "$");
+ assertErrorResponse(interceptor.invoke(dropSchema),
Response.Status.FORBIDDEN);
+ verify(dropSchema, never()).proceed();
+ }
+
+ private MethodInvocation createInvocation(String namespaceId, String
delimiter, String mode)
+ throws Throwable {
+ Method method =
+ LanceNamespaceOperations.class.getMethod(
+ "createNamespace", String.class, String.class,
CreateNamespaceRequest.class);
+ CreateNamespaceRequest request = new CreateNamespaceRequest();
+ request.setMode(mode);
+ return invocation(method, namespaceId, delimiter, request);
+ }
+
+ private MethodInvocation dropInvocation(String namespaceId, String
delimiter) throws Throwable {
+ Method method =
+ LanceNamespaceOperations.class.getMethod(
+ "dropNamespace", String.class, String.class,
DropNamespaceRequest.class);
+ return invocation(method, namespaceId, delimiter, new
DropNamespaceRequest());
+ }
+
private void allow(Privilege.Name... privileges) {
Set<Privilege.Name> allowed = Set.of(privileges);
- when(authorizer.authorize(any(), any(), any(), any(), any()))
- .thenAnswer(invocation -> allowed.contains(invocation.getArgument(3)));
+ // doAnswer, not when(...): a test that narrows the allowed privileges
calls this twice, and
+ // when(...) would invoke the already-stubbed answer with null arguments.
+ doAnswer(invocation -> allowed.contains(invocation.getArgument(3)))
+ .when(authorizer)
+ .authorize(any(), any(), any(), any(), any());
}
private Method namespaceMethod(String name) throws NoSuchMethodException {
@@ -171,8 +273,12 @@ class TestLanceMetadataAuthorizationMethodInterceptor {
}
private void assertErrorResponse(Object result, Response.Status
expectedStatus) {
+ assertErrorResponse(result, expectedStatus, null);
+ }
+
+ private void assertErrorResponse(Object result, Response.Status
expectedStatus, String message) {
Response response = assertInstanceOf(Response.class, result);
- assertEquals(expectedStatus.getStatusCode(), response.getStatus());
+ assertEquals(expectedStatus.getStatusCode(), response.getStatus(),
message);
assertInstanceOf(ErrorResponse.class, response.getEntity());
}
