From af0732407651c4298b2d9fa2418eb2ac596fc9db Mon Sep 17 00:00:00 2001 From: yuqi Date: Mon, 7 Sep 2026 16:05:04 +0800 Subject: [PATCH 1/9] [#12574] docs(lance): document authentication and authorization with HTTP integration coverage --- docs/lance-rest-integration.md | 47 ++++ docs/lance-rest-service.md | 114 +++++++++- .../test/LanceNamespaceAuthorizationIT.java | 201 ++++++++++++++++++ 3 files changed, 356 insertions(+), 6 deletions(-) diff --git a/docs/lance-rest-integration.md b/docs/lance-rest-integration.md index 1ae023fa23a..ad09cf14282 100644 --- a/docs/lance-rest-integration.md +++ b/docs/lance-rest-integration.md @@ -109,6 +109,53 @@ Before proceeding, ensure the following requirements are met: - For Spark integration: `pyspark` - For Ray integration: `ray`, `lance-namespace`, `lance-ray` +## Authentication and authorization + +For per-user metadata authorization, connect engines to the auxiliary Lance REST service with +`gravitino.authorization.enable=true`. Configure each engine's REST client to send the caller's +`Authorization` header on every namespace and table request. If supported by that client version, +`X-Gravitino-Active-Roles` can restrict the active roles. See the +[Lance REST authentication and privilege matrix](./lance-rest-service.md#authentication-and-authorization). + +For example, with development-only `simple` authentication, this request lists only tables that +`user1` may access (the password is not validated): + +```shell +curl --user 'user1:unused' \ + -H 'X-Gravitino-Active-Roles: ALL' \ + 'http://localhost:9101/lance/v1/namespace/lance_catalog.sales/table/list?delimiter=.' +``` + +Connector header configuration depends on the connector version. The Spark and Ray examples +below omit credentials and assume the default simple-authentication setup; in auxiliary mode +such requests use the configured Lance service identity. They do not demonstrate per-user +access control. In standalone mode, all metadata requests to Gravitino use the backend service +identity even when an engine supplies its own incoming credentials. + +Engines that probe before creating need the corresponding creation privileges. Reading table +metadata requires `SELECT_TABLE` or `MODIFY_TABLE` with parent access, while overwriting requires +`MODIFY_TABLE` and dropping requires ownership. Metadata authorization does not authorize direct +reads or writes to object storage: configure storage access independently. Lance REST does not +vend per-user storage credentials. + +### Verify authentication and authorization locally + +The HTTP integration suites start Gravitino with the Lance auxiliary service and exercise +caller identity, service identity fallback, active roles, namespace and table privileges, +filtered listings, and denied mutations. They also start a separate Lance listener in standalone +mode to verify its outbound service identity against the Gravitino HTTP API. + +```shell +./gradlew :lance:lance-rest-server:test \ + --tests '*LanceRESTServiceAuthIT' \ + --tests '*LanceNamespaceAuthorizationIT' \ + --tests '*LanceTableAuthorizationIT' \ + -PskipDockerTests=true +``` + +These suites use `simple` authentication and local storage. They do not validate an external +OAuth2/Kerberos provider or object-store access policies. + ## Spark Integration ### Configuration diff --git a/docs/lance-rest-service.md b/docs/lance-rest-service.md index 0b464c5451d..f99f582be87 100644 --- a/docs/lance-rest-service.md +++ b/docs/lance-rest-service.md @@ -135,10 +135,10 @@ To enable the Lance REST service within Gravitino server, configure the followin **Authentication to the Gravitino Server** -The Lance REST service makes its own requests to the Gravitino server. Those requests must carry -credentials, otherwise a Gravitino server configured with an authenticator other than `simple` -rejects them and every Lance operation fails. Configure the auth type to match the Gravitino -server: +In standalone mode, the Lance REST service makes HTTP requests to the Gravitino server using +its configured service credentials. Configure the auth type to match the Gravitino server. +Auxiliary mode uses internal APIs and preserves the authenticated caller instead; the simple +user name below is only the fallback for requests accepted as anonymous. | Configuration Property | Description | Default Value | Required | |----------------------------------------------------|------------------------------------------------------------------------------------|---------------------|-------------------| @@ -149,8 +149,8 @@ server: | `gravitino.lance-rest.gravitino-oauth2.token-path` | Path on the OAuth2 server used to request the token | (none) | Yes, for `oauth2` | | `gravitino.lance-rest.gravitino-oauth2.scope` | Scope of the requested OAuth2 token | (none) | Yes, for `oauth2` | -This setting controls how the service authenticates to the Gravitino server. It does not change how -callers authenticate to the Lance REST service itself. +These settings control outbound authentication in standalone mode. They do not configure inbound +authentication to Lance REST. See [Authentication and authorization](#authentication-and-authorization). **Example Configuration:** @@ -263,6 +263,108 @@ URL encoded: lance_catalog%24schema%24table01 - Namespace deletion is recursive and irreversible ::: +## Authentication and authorization + +### Authentication and deployment modes + +Lance REST uses Gravitino's `gravitino.authenticators` configuration for incoming requests in +both auxiliary and standalone mode. See [Authentication](./security/how-to-authenticate.md) for +configuring the authenticators and their credentials. Health check endpoints bypass authentication. +Authentication errors use the Lance JSON error format; unsupported credentials return HTTP `401`. + +| Mode | Identity used for Gravitino metadata operations | Metadata authorization | +|------|------------------------------------------------|------------------------| +| Auxiliary (running with Gravitino) | Authenticated caller, including active roles; anonymous requests fall back to `gravitino.lance-rest.gravitino-simple.user-name` (default `lance-rest-server`) | Enabled by `gravitino.authorization.enable=true` with a configured metalake | +| Standalone | Configured service credentials (`gravitino.lance-rest.gravitino-auth-type` and its simple/OAuth2 settings) | No Lance REST per-user metadata authorization; the remote Gravitino server checks the service identity if its authorization is enabled | + +The auxiliary fallback applies only after authentication accepts an anonymous request. It does +not recover a rejected authentication attempt. Authenticated callers keep their own privileges, +active roles, ownership and audit identity; they do not inherit the service user's privileges. +The fallback service user itself needs the privileges required by the requested operation. + +With `simple` authentication, a Basic header supplies a user name without validating a password, +and a request without credentials is accepted as anonymous. Some malformed Basic credentials +also resolve to anonymous. Use an authenticator that validates credentials when caller identity +must be verified; `simple` is not password authentication. + +Standalone authenticates incoming requests, but does not forward their identities or active roles +to its Gravitino backend. All callers use the configured backend service identity. Standalone +per-user authorization and storage credential vending are outside the supported scope. The +backend service identity needs privileges for all underlying Gravitino calls, including existence +checks performed before mutations (for example, catalog access before creating a namespace). + +### Enable auxiliary metadata authorization + +Configure `${GRAVITINO_HOME}/conf/gravitino.conf`: + +```properties +gravitino.auxService.names = lance-rest +gravitino.lance-rest.gravitino-metalake = my_metalake +gravitino.authorization.enable = true +gravitino.authorization.serviceAdmins = adminUser +# Development example: simple accepts the supplied user name without password validation. +gravitino.authenticators = simple +gravitino.lance-rest.gravitino-simple.user-name = lance-rest-server +``` + +Create the metalake, add users, and grant roles through the Gravitino API as described in +[Access Control](./security/access-control.md). The Lance service exposes this configured metalake: +a one-level namespace identifies a catalog, a two-level namespace identifies a schema, and a +three-level table identifier identifies a table. + +Requests may set `X-Gravitino-Active-Roles` to `ALL` (also the default when omitted), `NONE`, or +a comma-separated list of assigned role names. This selection reaches both operation checks +and listing filters. Malformed selections return `400`; selecting an unassigned role is forbidden. +Ownership is independent of role selection, so `NONE` does not remove ownership privileges. + +### Required privileges + +The following rules use the same Gravitino privileges and ownership rules as +[Iceberg REST authorization](./iceberg-rest-service.md). Privileges can be inherited from +ancestor scopes as described in Access Control. Service administrators and metalake owners +can operate throughout the metalake; catalog owners can operate within their catalogs. +Schema owners additionally need `USE_CATALOG`, and table owners need `USE_CATALOG` and +`USE_SCHEMA`. The ownership alternatives below include these ancestor owners. + +| Namespace operation | Required privileges or ownership | +|---------------------|----------------------------------| +| `ListNamespaces` at root | Membership in the metalake; returns only accessible catalogs | +| `ListNamespaces` under a catalog; `DescribeNamespace` for a catalog; `NamespaceExists` for a catalog | `USE_CATALOG`, or ownership | +| `ListNamespaces` under a schema; `DescribeNamespace` for a schema; `ListTables` | `USE_CATALOG` and `USE_SCHEMA`, or ownership | +| `NamespaceExists` for a schema | `USE_CATALOG` and either `USE_SCHEMA` or `CREATE_SCHEMA`, or ownership | +| `CreateNamespace` for a catalog (`create`, `exist_ok`) | `CREATE_CATALOG` on the metalake, or metalake ownership | +| `CreateNamespace` for a schema (`create`, `exist_ok`) | `USE_CATALOG` and `CREATE_SCHEMA`, or catalog/metalake ownership | +| `CreateNamespace` (`overwrite`); `DropNamespace` | Ownership of the namespace or an ancestor | + +| Table operation | Required privileges or ownership | +|-----------------|----------------------------------| +| `DescribeTable` | `USE_CATALOG`, `USE_SCHEMA`, and either `SELECT_TABLE` or `MODIFY_TABLE`, or ownership | +| `TableExists` | Same as `DescribeTable`, or `USE_CATALOG`, `USE_SCHEMA`, and either `PROBE_TABLE_LIKE` or `CREATE_TABLE` | +| `CreateTable` (`create`, `exist_ok`); `RegisterTable` (`create`); `DeclareTable` | `USE_CATALOG`, `USE_SCHEMA`, and `CREATE_TABLE`, or schema/ancestor ownership | +| `CreateTable` or `RegisterTable` (`overwrite`); `AlterColumns`; `DropColumns` | `USE_CATALOG`, `USE_SCHEMA`, and `MODIFY_TABLE`, or ownership | +| `DropTable`; `DeregisterTable` | Ownership of the table or an ancestor | + +`CREATE_TABLE` and `PROBE_TABLE_LIKE` can authorize an existence probe without granting table +metadata reads. `CREATE_TABLE` alone cannot overwrite another owner's table, and `MODIFY_TABLE` +alone cannot drop or deregister it. Similarly, namespace creation privileges do not authorize +overwriting or dropping another owner's namespace. Successful creation assigns ownership to +the effective caller. + +### Listings and concealed objects + +Namespace listings omit inaccessible catalogs and schemas. Table listings omit tables for which +the caller has neither ownership nor `SELECT_TABLE`/`MODIFY_TABLE`. Filtering happens before +pagination; hidden entries do not consume page slots. Access to the parent is checked separately. + +Direct operations on objects the caller cannot access return `403`, whether or not the target +exists, without returning its stored metadata or location. An authorized caller can distinguish +an existing object from a missing one (`404`). Concealment therefore does not mean that every +inaccessible object returns `404`. + +Authorization governs metadata requests. Engines access Lance data files directly using their +own storage configuration and credentials; these metadata privileges do not enforce data-file +access or provide storage credentials. + ## Examples The following examples demonstrate how to interact with Lance REST service using different programming languages and tools. 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 5226a20beb9..1e7536e7d05 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 @@ -35,6 +35,8 @@ import org.apache.gravitino.authorization.SecurableObjects; import org.apache.gravitino.client.GravitinoMetalake; import org.apache.gravitino.integration.test.util.BaseIT; +import org.apache.gravitino.lance.LanceRESTService; +import org.apache.gravitino.rest.RESTUtils; import org.apache.gravitino.server.web.ObjectMapperProvider; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.Assertions; @@ -43,6 +45,7 @@ import org.lance.namespace.model.CreateNamespaceRequest; import org.lance.namespace.model.DescribeNamespaceResponse; import org.lance.namespace.model.DropNamespaceRequest; +import org.lance.namespace.model.ErrorResponse; import org.lance.namespace.model.ListNamespacesResponse; /** Verifies namespace authorization and list filtering through auxiliary-mode Lance REST. */ @@ -70,6 +73,7 @@ public void startIntegrationTest() throws Exception { customConfigs.put(Configs.SERVICE_ADMINS.getKey(), ADMIN); customConfigs.put(Configs.AUTHENTICATORS.getKey(), "simple"); customConfigs.put("SimpleAuthUserName", ADMIN); + customConfigs.put("gravitino.lance-rest.gravitino-simple.user-name", WRITER); super.startIntegrationTest(); String metalakeName = getLanceRESTServerMetalakeName(); @@ -181,6 +185,203 @@ public void testDropConcealsNamespacesTheCallerMayNotSee() throws Exception { assertStatus(403, drop(USER, HIDDEN_CATALOG, "skip", null)); } + /** Verifies that active roles survive authentication and reach namespace authorization. */ + @Test + public void testActiveRolesReachAuthorizationAndListingFilters() throws Exception { + String schemaPath = "/v1/namespace/" + id(VISIBLE_CATALOG, VISIBLE_SCHEMA) + "/describe"; + assertStatus(200, sendWithRoles(USER, schemaPath, "ALL")); + assertStatus(403, sendWithRoles(USER, schemaPath, "NONE")); + assertStatus(403, sendWithRoles(USER, schemaPath, "lance_authz_catalog_role")); + assertStatus( + 200, sendWithRoles(USER, schemaPath, "lance_authz_catalog_role,lance_authz_schema_role")); + assertStatus(403, sendWithRoles(USER, schemaPath, "lance_authz_writer_role")); + + HttpResponse response = + httpClient.send( + request(USER, "/v1/namespace/" + VISIBLE_CATALOG + "/list") + .setHeader( + AuthConstants.X_GRAVITINO_ACTIVE_ROLES_HEADER, "lance_authz_catalog_role") + .GET() + .build(), + HttpResponse.BodyHandlers.ofString()); + assertStatus(200, response); + Assertions.assertTrue( + ObjectMapperProvider.objectMapper() + .readValue(response.body(), ListNamespacesResponse.class) + .getNamespaces() + .isEmpty()); + // Narrowing one request must not affect subsequent requests on the same server. + Assertions.assertEquals(List.of(VISIBLE_SCHEMA), list(USER, VISIBLE_CATALOG)); + } + + /** Verifies authentication failures stop before metadata writes or service identity fallback. */ + @Test + public void testAuthenticationErrorsUseLanceJsonAndDoNotCreateMetadata() throws Exception { + String catalog = "lance_authz_rejected_auth_catalog"; + CreateNamespaceRequest body = new CreateNamespaceRequest(); + body.addIdItem(catalog); + String json = ObjectMapperProvider.objectMapper().writeValueAsString(body); + HttpResponse unauthorized = + httpClient.send( + request(ADMIN, "/v1/namespace/" + catalog + "/create") + .setHeader(AuthConstants.HTTP_HEADER_AUTHORIZATION, "Bearer unsupported-token") + .POST(HttpRequest.BodyPublishers.ofString(json)) + .build(), + HttpResponse.BodyHandlers.ofString()); + assertError(401, unauthorized); + HttpResponse malformedRoles = + httpClient.send( + request(ADMIN, "/v1/namespace/" + catalog + "/create") + .setHeader(AuthConstants.X_GRAVITINO_ACTIVE_ROLES_HEADER, "ALL,NONE") + .POST(HttpRequest.BodyPublishers.ofString(json)) + .build(), + HttpResponse.BodyHandlers.ofString()); + assertError(400, malformedRoles); + assertStatus(404, post(ADMIN, catalog, "exists")); + } + + /** Verifies anonymous fallback uses the service user's privileges and records its ownership. */ + @Test + public void testServiceIdentityFallbackIsAuthorized() throws Exception { + String catalog = "lance_authz_fallback_catalog"; + CreateNamespaceRequest body = new CreateNamespaceRequest(); + body.addIdItem(catalog); + HttpRequest.Builder anonymous = + HttpRequest.newBuilder() + .uri( + URI.create( + String.format("http://localhost:%d/lance", getLanceRESTServerPort()) + + "/v1/namespace/" + + catalog + + "/create?delimiter=.")) + .header("Content-Type", "application/json"); + assertStatus( + 200, + httpClient.send( + anonymous + .POST( + HttpRequest.BodyPublishers.ofString( + ObjectMapperProvider.objectMapper().writeValueAsString(body))) + .build(), + HttpResponse.BodyHandlers.ofString())); + GravitinoMetalake metalake = client.loadMetalake(getLanceRESTServerMetalakeName()); + Assertions.assertEquals(WRITER, metalake.loadCatalog(catalog).auditInfo().creator()); + // Ownership is usable by the real service user after creation through anonymous fallback. + assertStatus(200, drop(WRITER, catalog, null, "cascade")); + + HttpRequest denied = + HttpRequest.newBuilder() + .uri( + URI.create( + String.format("http://localhost:%d/lance", getLanceRESTServerPort()) + + "/v1/namespace/" + + HIDDEN_CATALOG + + "/describe?delimiter=.")) + .header("Content-Type", "application/json") + .POST(HttpRequest.BodyPublishers.ofString("{}")) + .build(); + assertStatus(403, httpClient.send(denied, HttpResponse.BodyHandlers.ofString())); + // An authenticated reader cannot borrow the fallback user's CREATE_CATALOG privilege. + assertStatus(403, create(USER, catalog, null, Map.of())); + } + + /** Verifies standalone HTTP backend calls use service credentials rather than caller roles. */ + @Test + public void testStandaloneUsesBackendServiceIdentity() throws Exception { + int port = RESTUtils.findAvailablePort(10000, 11000); + String catalog = "lance_authz_standalone_catalog"; + String serviceUser = "lance_authz_standalone_user"; + GravitinoMetalake metalake = client.loadMetalake(getLanceRESTServerMetalakeName()); + metalake.addUser(serviceUser); + metalake.createRole( + "lance_authz_standalone_role", + new HashMap<>(), + List.of( + SecurableObjects.ofMetalake( + metalake.name(), + new ArrayList<>( + List.of(Privileges.UseCatalog.allow(), Privileges.CreateCatalog.allow()))))); + metalake.grantRolesToUser(List.of("lance_authz_standalone_role"), serviceUser); + LanceRESTService standalone = new LanceRESTService(); + try { + standalone.serviceInit( + Map.of( + "httpPort", + String.valueOf(port), + "gravitino-uri", + "http://localhost:" + getGravitinoServerPort(), + "gravitino-metalake", + getLanceRESTServerMetalakeName(), + "gravitino-auth-type", + "simple", + "gravitino-simple.user-name", + serviceUser), + false); + standalone.serviceStart(); + CreateNamespaceRequest body = new CreateNamespaceRequest(); + body.addIdItem(catalog); + HttpRequest request = + request(USER, "/v1/namespace/" + catalog + "/create") + .uri( + URI.create( + "http://localhost:" + + port + + "/lance/v1/namespace/" + + catalog + + "/create?delimiter=.")) + .setHeader(AuthConstants.X_GRAVITINO_ACTIVE_ROLES_HEADER, "NONE") + .POST( + HttpRequest.BodyPublishers.ofString( + ObjectMapperProvider.objectMapper().writeValueAsString(body))) + .build(); + // USER cannot create catalogs in auxiliary mode. The remote backend receives the service + // user's + // credentials and roles, despite USER selecting NONE on this incoming request. + assertStatus(200, httpClient.send(request, HttpResponse.BodyHandlers.ofString())); + Assertions.assertEquals( + serviceUser, + client + .loadMetalake(getLanceRESTServerMetalakeName()) + .loadCatalog(catalog) + .auditInfo() + .creator()); + assertStatus(200, drop(serviceUser, catalog, null, "cascade")); + } finally { + standalone.serviceStop(); + } + } + + /** Verifies health endpoints remain reachable even when credentials would be rejected. */ + @Test + public void testHealthBypassesAuthentication() throws Exception { + HttpRequest health = + request(USER, "/health") + .setHeader(AuthConstants.HTTP_HEADER_AUTHORIZATION, "Bearer unsupported-token") + .GET() + .build(); + assertStatus(200, httpClient.send(health, HttpResponse.BodyHandlers.ofString())); + } + + private HttpResponse sendWithRoles(String user, String path, String roles) + throws Exception { + return httpClient.send( + request(user, path) + .setHeader(AuthConstants.X_GRAVITINO_ACTIVE_ROLES_HEADER, roles) + .POST(HttpRequest.BodyPublishers.ofString("{}")) + .build(), + HttpResponse.BodyHandlers.ofString()); + } + + private void assertError(int status, HttpResponse response) throws Exception { + assertStatus(status, response); + Assertions.assertTrue( + response.headers().firstValue("Content-Type").orElse("").startsWith("application/json")); + ErrorResponse error = + ObjectMapperProvider.objectMapper().readValue(response.body(), ErrorResponse.class); + Assertions.assertEquals(status, error.getCode()); + Assertions.assertFalse(error.getError().isEmpty()); + } + private void grant(GravitinoMetalake metalake, String role, SecurableObject object) { metalake.createRole(role, new HashMap<>(), List.of(object)); metalake.grantRolesToUser(List.of(role), USER); From f4fd8a20da9aa9d123182b56479748c5541f5a5d Mon Sep 17 00:00:00 2001 From: yuqi Date: Mon, 7 Sep 2026 16:23:57 +0800 Subject: [PATCH 2/9] [#12574] docs(lance): align tables and clarify current operation semantics --- docs/lance-rest-integration.md | 7 +-- docs/lance-rest-service.md | 98 +++++++++++++++++++--------------- 2 files changed, 58 insertions(+), 47 deletions(-) diff --git a/docs/lance-rest-integration.md b/docs/lance-rest-integration.md index ad09cf14282..bdf876d4e1f 100644 --- a/docs/lance-rest-integration.md +++ b/docs/lance-rest-integration.md @@ -21,7 +21,7 @@ This documentation assumes familiarity with the Lance REST service setup as desc The following table outlines the tested compatibility between Gravitino versions and Lance connector versions: | Gravitino Version (Lance REST) | Supported lance-spark Versions | Supported lance-ray Versions | -|--------------------------------|--------------------------------|-----------------------------------------------| +| ------------------------------ | ------------------------------ | --------------------------------------------- | | 1.1.1 - 1.2.1 | 0.0.10 - 0.0.15 | 0.0.6 - 0.0.8 | | 1.3.0 | 0.2.0, 0.4.0, 0.5.1 | 0.3.0 - 0.4.2 (0.2.0 conditionally supported) | @@ -135,8 +135,9 @@ identity even when an engine supplies its own incoming credentials. Engines that probe before creating need the corresponding creation privileges. Reading table metadata requires `SELECT_TABLE` or `MODIFY_TABLE` with parent access, while overwriting requires `MODIFY_TABLE` and dropping requires ownership. Metadata authorization does not authorize direct -reads or writes to object storage: configure storage access independently. Lance REST does not -vend per-user storage credentials. +reads or writes to object storage: configure storage access independently. Lance REST responses +can return shared storage credentials configured on the catalog or table; it does not issue +per-user, scoped storage credentials. ### Verify authentication and authorization locally diff --git a/docs/lance-rest-service.md b/docs/lance-rest-service.md index f99f582be87..6e945a53019 100644 --- a/docs/lance-rest-service.md +++ b/docs/lance-rest-service.md @@ -57,7 +57,7 @@ The Lance REST service acts as a bridge between Lance datasets and applications: ``` **Key Features:** -- Full compliance with Lance REST API specification +- Support for the Lance REST operations listed below - Can run standalone or integrated with Gravitino server - Support for namespace and table management - Metadata stored in Gravitino for unified governance @@ -67,7 +67,7 @@ The Lance REST service acts as a bridge between Lance datasets and applications: The Lance REST service provides comprehensive support for namespace management and table management. Index operations are not supported yet. The table below lists all supported operations: | Operation | Description | HTTP Method | Endpoint Pattern | -|-------------------|---------------------------------------------------------------------------|-------------|---------------------------------------| +| ----------------- | ------------------------------------------------------------------------- | ----------- | ------------------------------------- | | CreateNamespace | Create a new Lance namespace | POST | `/lance/v1/namespace/{id}/create` | | ListNamespaces | List all namespaces under a parent namespace | GET | `/lance/v1/namespace/{parent}/list` | | DescribeNamespace | Retrieve detailed information about a specific namespace | POST | `/lance/v1/namespace/{id}/describe` | @@ -99,17 +99,24 @@ REST-style canonical form. - `overwrite`: Replaces existing namespace **DropNamespace** behavior: -- Recursively deletes all child namespaces and tables -- Deletes both metadata and Lance data files -- Operation is irreversible +- Defaults to `behavior=restrict`; non-empty namespaces cannot be dropped in this mode +- `behavior=cascade` removes child metadata recursively +- Cascading a schema drop uses table drop semantics: external Lance datasets are preserved +- Use `DropTable` explicitly to delete a table's data; namespace deletion is not a storage purge #### Table Operations **RegisterTable vs CreateTable**: - **RegisterTable**: Links existing Lance datasets into Gravitino catalog without data movement -- **CreateTable**: Creates new Lance table with schema and write metadata files +- **CreateTable**: Creates an empty Lance dataset using the schema from the Arrow IPC stream :::note -The `version` field of `CreateTable` response is always null, which stands for the latest version. +The current `CreateTable` implementation reads the Arrow stream schema but does not ingest its +record batches. Send a schema-only stream and write records through a Lance client or engine +after creation. + +The `version` field of `CreateTable` reports the stored Lance dataset version when available. +`DescribeTable` currently returns the latest metadata even when a historical `version` is requested; +versioned metadata reads are not implemented. ::: **DropTable vs DeregisterTable**: @@ -124,7 +131,7 @@ The `version` field of `CreateTable` response is always null, which stands for t To enable the Lance REST service within Gravitino server, configure the following properties in your Gravitino configuration file `${GRAVITINO_HOME}/conf/gravitino.conf`: | Configuration Property | Description | Default Value | Required | -|-------------------------------------------|------------------------------------------------------------------------------|-------------------------|----------| +| ----------------------------------------- | ---------------------------------------------------------------------------- | ----------------------- | -------- | | `gravitino.auxService.names` | Auxiliary services to run. Include `lance-rest` to enable Lance REST service | iceberg-rest,lance-rest | Yes | | `gravitino.lance-rest.classpath` | Classpath for Lance REST service, relative to Gravitino home directory | lance-rest-server/libs | Yes | | `gravitino.lance-rest.httpPort` | Port number for Lance REST service | 9101 | No | @@ -141,7 +148,7 @@ Auxiliary mode uses internal APIs and preserves the authenticated caller instead user name below is only the fallback for requests accepted as anonymous. | Configuration Property | Description | Default Value | Required | -|----------------------------------------------------|------------------------------------------------------------------------------------|---------------------|-------------------| +| -------------------------------------------------- | ---------------------------------------------------------------------------------- | ------------------- | ----------------- | | `gravitino.lance-rest.gravitino-auth-type` | Auth type used to reach the Gravitino server. Supported values: `simple`, `oauth2` | `simple` | No | | `gravitino.lance-rest.gravitino-simple.user-name` | User name presented when the auth type is `simple` | `lance-rest-server` | No | | `gravitino.lance-rest.gravitino-oauth2.server-uri` | OAuth2 server URI | (none) | Yes, for `oauth2` | @@ -176,7 +183,7 @@ To run Lance REST service independently without Gravitino server (You need to st Configure the service by editing `{GRAVITINO_HOME}/conf/gravitino-lance-rest-server.conf` or passing command-line arguments: | Configuration Property | Description | Default Value | Required | -|-------------------------------------------|----------------------------|-----------------------|----------| +| ----------------------------------------- | -------------------------- | --------------------- | -------- | | `gravitino.lance-rest.namespace-backend` | Namespace metadata backend | gravitino | Yes | | `gravitino.lance-rest.gravitino-uri` | Gravitino server URI | http://localhost:8090 | Yes | | `gravitino.lance-rest.gravitino-metalake` | Gravitino metalake name | (none) | Yes | @@ -205,7 +212,7 @@ Access the service at `http://localhost:9101`. **Environment Variables:** | Environment Variable | Configuration Property | Required | Default Value | -|--------------------------------------|-------------------------------------------|----------|-------------------------| +| ------------------------------------ | ----------------------------------------- | -------- | ----------------------- | | `LANCE_REST_NAMESPACE_BACKEND` | `gravitino.lance-rest.namespace-backend` | Yes | `gravitino` | | `LANCE_REST_GRAVITINO_METALAKE_NAME` | `gravitino.lance-rest.gravitino-metalake` | Yes | (none) | | `LANCE_REST_GRAVITINO_URI` | `gravitino.lance-rest.gravitino-uri` | Yes | `http://localhost:8090` | @@ -260,7 +267,7 @@ URL encoded: lance_catalog%24schema%24table01 - Supports only **two levels of namespaces** before tables - Tables **cannot** be nested deeper than schema level - Parent catalog must be created in Gravitino before using Lance REST API -- Namespace deletion is recursive and irreversible +- Namespace deletion defaults to `restrict`; use `cascade` to remove child metadata ::: ## Authentication and authorization @@ -272,10 +279,10 @@ both auxiliary and standalone mode. See [Authentication](./security/how-to-authe configuring the authenticators and their credentials. Health check endpoints bypass authentication. Authentication errors use the Lance JSON error format; unsupported credentials return HTTP `401`. -| Mode | Identity used for Gravitino metadata operations | Metadata authorization | -|------|------------------------------------------------|------------------------| -| Auxiliary (running with Gravitino) | Authenticated caller, including active roles; anonymous requests fall back to `gravitino.lance-rest.gravitino-simple.user-name` (default `lance-rest-server`) | Enabled by `gravitino.authorization.enable=true` with a configured metalake | -| Standalone | Configured service credentials (`gravitino.lance-rest.gravitino-auth-type` and its simple/OAuth2 settings) | No Lance REST per-user metadata authorization; the remote Gravitino server checks the service identity if its authorization is enabled | +| Mode | Identity used for Gravitino metadata operations | Metadata authorization | +| ---------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------- | +| Auxiliary (running with Gravitino) | Authenticated caller, including active roles; anonymous requests fall back to `gravitino.lance-rest.gravitino-simple.user-name` (default `lance-rest-server`) | Enabled by `gravitino.authorization.enable=true` with a configured metalake | +| Standalone | Configured service credentials (`gravitino.lance-rest.gravitino-auth-type` and its simple/OAuth2 settings) | No Lance REST per-user metadata authorization; the remote Gravitino server checks the service identity if its authorization is enabled | The auxiliary fallback applies only after authentication accepts an anonymous request. It does not recover a rejected authentication attempt. Authenticated callers keep their own privileges, @@ -289,7 +296,7 @@ must be verified; `simple` is not password authentication. Standalone authenticates incoming requests, but does not forward their identities or active roles to its Gravitino backend. All callers use the configured backend service identity. Standalone -per-user authorization and storage credential vending are outside the supported scope. The +per-user authorization and scoped temporary credential vending are outside the supported scope. The backend service identity needs privileges for all underlying Gravitino calls, including existence checks performed before mutations (for example, catalog access before creating a namespace). @@ -326,27 +333,27 @@ can operate throughout the metalake; catalog owners can operate within their cat Schema owners additionally need `USE_CATALOG`, and table owners need `USE_CATALOG` and `USE_SCHEMA`. The ownership alternatives below include these ancestor owners. -| Namespace operation | Required privileges or ownership | -|---------------------|----------------------------------| -| `ListNamespaces` at root | Membership in the metalake; returns only accessible catalogs | -| `ListNamespaces` under a catalog; `DescribeNamespace` for a catalog; `NamespaceExists` for a catalog | `USE_CATALOG`, or ownership | -| `ListNamespaces` under a schema; `DescribeNamespace` for a schema; `ListTables` | `USE_CATALOG` and `USE_SCHEMA`, or ownership | -| `NamespaceExists` for a schema | `USE_CATALOG` and either `USE_SCHEMA` or `CREATE_SCHEMA`, or ownership | -| `CreateNamespace` for a catalog (`create`, `exist_ok`) | `CREATE_CATALOG` on the metalake, or metalake ownership | -| `CreateNamespace` for a schema (`create`, `exist_ok`) | `USE_CATALOG` and `CREATE_SCHEMA`, or catalog/metalake ownership | -| `CreateNamespace` (`overwrite`); `DropNamespace` | Ownership of the namespace or an ancestor | - -| Table operation | Required privileges or ownership | -|-----------------|----------------------------------| -| `DescribeTable` | `USE_CATALOG`, `USE_SCHEMA`, and either `SELECT_TABLE` or `MODIFY_TABLE`, or ownership | -| `TableExists` | Same as `DescribeTable`, or `USE_CATALOG`, `USE_SCHEMA`, and either `PROBE_TABLE_LIKE` or `CREATE_TABLE` | -| `CreateTable` (`create`, `exist_ok`); `RegisterTable` (`create`); `DeclareTable` | `USE_CATALOG`, `USE_SCHEMA`, and `CREATE_TABLE`, or schema/ancestor ownership | -| `CreateTable` or `RegisterTable` (`overwrite`); `AlterColumns`; `DropColumns` | `USE_CATALOG`, `USE_SCHEMA`, and `MODIFY_TABLE`, or ownership | -| `DropTable`; `DeregisterTable` | Ownership of the table or an ancestor | - -`CREATE_TABLE` and `PROBE_TABLE_LIKE` can authorize an existence probe without granting table -metadata reads. `CREATE_TABLE` alone cannot overwrite another owner's table, and `MODIFY_TABLE` -alone cannot drop or deregister it. Similarly, namespace creation privileges do not authorize +| Namespace operation | Required privileges or ownership | +| ---------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------- | +| `ListNamespaces` at root | Membership in the metalake; returns only accessible catalogs | +| `ListNamespaces` under a catalog; `DescribeNamespace` for a catalog; `NamespaceExists` for a catalog | `USE_CATALOG`, or ownership | +| `ListNamespaces` under a schema; `DescribeNamespace` for a schema; `ListTables` | `USE_CATALOG` and `USE_SCHEMA`, or ownership | +| `NamespaceExists` for a schema | `USE_CATALOG` and either `USE_SCHEMA` or `CREATE_SCHEMA`, or ownership | +| `CreateNamespace` for a catalog (`create`, `exist_ok`) | `CREATE_CATALOG` on the metalake, or metalake ownership | +| `CreateNamespace` for a schema (`create`, `exist_ok`) | `USE_CATALOG` and `CREATE_SCHEMA`, or catalog/metalake ownership | +| `CreateNamespace` (`overwrite`); `DropNamespace` | Ownership of the namespace or an ancestor | + +| Table operation | Required privileges or ownership | +| -------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------- | +| `DescribeTable` | `USE_CATALOG`, `USE_SCHEMA`, and either `SELECT_TABLE` or `MODIFY_TABLE`, or ownership | +| `TableExists` | Same as `DescribeTable`, or `USE_CATALOG`, `USE_SCHEMA`, and either `PROBE_TABLE_LIKE` or `CREATE_TABLE` | +| `CreateTable` (`create`, `exist_ok`); `RegisterTable` (`create`); `DeclareTable` | `USE_CATALOG`, `USE_SCHEMA`, and `CREATE_TABLE`, or schema/ancestor ownership | +| `CreateTable` or `RegisterTable` (`overwrite`); `AlterColumns`; `DropColumns` | `USE_CATALOG`, `USE_SCHEMA`, and `MODIFY_TABLE`, or ownership | +| `DropTable`; `DeregisterTable` | Ownership of the table or an ancestor | + +`CREATE_TABLE` and `PROBE_TABLE_LIKE` authorize `TableExists` without authorizing +`DescribeTable`. `CREATE_TABLE` alone does not authorize overwrite requests. The `DropTable` +and `DeregisterTable` endpoints require ownership rather than `MODIFY_TABLE`. Similarly, namespace creation privileges do not authorize overwriting or dropping another owner's namespace. Successful creation assigns ownership to the effective caller. @@ -356,14 +363,17 @@ Namespace listings omit inaccessible catalogs and schemas. Table listings omit t the caller has neither ownership nor `SELECT_TABLE`/`MODIFY_TABLE`. Filtering happens before pagination; hidden entries do not consume page slots. Access to the parent is checked separately. -Direct operations on objects the caller cannot access return `403`, whether or not the target -exists, without returning its stored metadata or location. An authorized caller can distinguish -an existing object from a missing one (`404`). Concealment therefore does not mean that every +When a caller lacks an endpoint's required privileges, the request returns `403`, whether or not +the target exists, without returning its stored metadata or location. An authorized caller can +distinguish an existing object from a missing one (`404`). Concealment therefore does not mean that every inaccessible object returns `404`. -Authorization governs metadata requests. Engines access Lance data files directly using their -own storage configuration and credentials; these metadata privileges do not enforce data-file -access or provide storage credentials. +Authorization governs metadata requests; engines access Lance data files directly. Responses can +include configured storage credentials: namespace descriptions resolve secret properties, and +table descriptions, creation and declaration responses return effective `storage_options` from +catalog defaults and table overrides. These are shared configured credentials, not temporary +credentials restricted to the caller's table privileges. Access to data files depends on the +permissions of those credentials. Per-user, scoped credential vending is not implemented. ## Examples From a918163364ed5b867161c8632a1addfa894f1098 Mon Sep 17 00:00:00 2001 From: yuqi Date: Mon, 7 Sep 2026 16:34:25 +0800 Subject: [PATCH 3/9] [#12574] fix(lance): reject unsupported rows and preserve backend auth errors --- docs/lance-rest-integration.md | 5 +- docs/lance-rest-service.md | 10 ++- .../GravitinoLanceTableOperations.java | 4 +- .../lance/common/utils/ArrowUtils.java | 30 +++++++ .../lance/common/utils/TestArrowUtils.java | 55 +++++++++++++ .../lance/service/LanceExceptionMapper.java | 12 ++- ...etadataAuthorizationMethodInterceptor.java | 6 +- .../LanceRESTAuthInterceptionService.java | 3 +- .../test/LanceNamespaceAuthorizationIT.java | 23 +++++- .../test/LanceTableAuthorizationIT.java | 64 +++++++++++++++ .../service/TestLanceExceptionMapper.java | 79 +++++++++++++++++++ .../rest/TestLanceNamespaceOperations.java | 29 ++++--- 12 files changed, 291 insertions(+), 29 deletions(-) create mode 100644 lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/TestLanceExceptionMapper.java diff --git a/docs/lance-rest-integration.md b/docs/lance-rest-integration.md index bdf876d4e1f..41df4c7336f 100644 --- a/docs/lance-rest-integration.md +++ b/docs/lance-rest-integration.md @@ -143,8 +143,9 @@ per-user, scoped storage credentials. The HTTP integration suites start Gravitino with the Lance auxiliary service and exercise caller identity, service identity fallback, active roles, namespace and table privileges, -filtered listings, and denied mutations. They also start a separate Lance listener in standalone -mode to verify its outbound service identity against the Gravitino HTTP API. +filtered listings, denied mutations, and rejection of non-empty Arrow creates without side effects. +They also start a separate Lance listener in standalone mode to verify its outbound service +identity and propagation of backend authorization denials through the Gravitino HTTP API. ```shell ./gradlew :lance:lance-rest-server:test \ diff --git a/docs/lance-rest-service.md b/docs/lance-rest-service.md index 6e945a53019..ee0d7787d2d 100644 --- a/docs/lance-rest-service.md +++ b/docs/lance-rest-service.md @@ -110,9 +110,9 @@ REST-style canonical form. - **RegisterTable**: Links existing Lance datasets into Gravitino catalog without data movement - **CreateTable**: Creates an empty Lance dataset using the schema from the Arrow IPC stream :::note -The current `CreateTable` implementation reads the Arrow stream schema but does not ingest its -record batches. Send a schema-only stream and write records through a Lance client or engine -after creation. +The current `CreateTable` implementation accepts schema-only Arrow streams, including zero-row +batches. A stream containing rows returns HTTP `406` before any metadata or dataset changes, +including for `overwrite`. Write records through a Lance client or engine after creation. The `version` field of `CreateTable` reports the stored Lance dataset version when available. `DescribeTable` currently returns the latest metadata even when a historical `version` is requested; @@ -278,6 +278,10 @@ Lance REST uses Gravitino's `gravitino.authenticators` configuration for incomin both auxiliary and standalone mode. See [Authentication](./security/how-to-authenticate.md) for configuring the authenticators and their credentials. Health check endpoints bypass authentication. Authentication errors use the Lance JSON error format; unsupported credentials return HTTP `401`. +In standalone mode, backend authentication and authorization failures retain HTTP `401` and `403` +respectively. Authentication/authorization failures do not include internal stack traces in `detail`. +Unexpected failures return HTTP `500` with a generic message; the server logs retain the exception +for diagnosis. | Mode | Identity used for Gravitino metadata operations | Metadata authorization | | ---------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------- | diff --git a/lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/GravitinoLanceTableOperations.java b/lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/GravitinoLanceTableOperations.java index 08bd9bae3a9..49bd407e7ee 100644 --- a/lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/GravitinoLanceTableOperations.java +++ b/lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/GravitinoLanceTableOperations.java @@ -168,11 +168,11 @@ public CreateTableResponse createTable( Preconditions.checkArgument( nsId.levels() == 3, "Expected at 3-level namespace but got: %s", nsId.levels()); - // Parser column information. + // Reject unsupported record batches before any metadata or storage mutation. List columns = Lists.newArrayList(); if (arrowStreamBody != null) { org.apache.arrow.vector.types.pojo.Schema schema = - ArrowUtils.parseArrowIpcStream(arrowStreamBody); + ArrowUtils.parseSchemaOnlyIpcStream(arrowStreamBody); columns = extractColumns(schema); } diff --git a/lance/lance-common/src/main/java/org/apache/gravitino/lance/common/utils/ArrowUtils.java b/lance/lance-common/src/main/java/org/apache/gravitino/lance/common/utils/ArrowUtils.java index 5d8508ee459..d2213e5e748 100644 --- a/lance/lance-common/src/main/java/org/apache/gravitino/lance/common/utils/ArrowUtils.java +++ b/lance/lance-common/src/main/java/org/apache/gravitino/lance/common/utils/ArrowUtils.java @@ -57,16 +57,46 @@ public static byte[] generateIpcStream(Schema arrowSchema) throws IOException { } public static Schema parseArrowIpcStream(byte[] stream) { + return parseArrowIpcStream(stream, false); + } + + /** + * Parses a schema-only Arrow IPC stream, rejecting record batches containing rows. + * + * @param stream the Arrow IPC stream + * @return the stream schema + * @throws UnsupportedOperationException if any record batch contains rows + * @throws IllegalArgumentException if the stream cannot be parsed + */ + public static Schema parseSchemaOnlyIpcStream(byte[] stream) { + return parseArrowIpcStream(stream, true); + } + + private static Schema parseArrowIpcStream(byte[] stream, boolean requireEmpty) { Schema schema; + boolean containsRows = false; try (BufferAllocator allocator = new RootAllocator(); ByteArrayInputStream bais = new ByteArrayInputStream(stream); ArrowStreamReader reader = new ArrowStreamReader(bais, allocator)) { schema = reader.getVectorSchemaRoot().getSchema(); + if (requireEmpty) { + while (reader.loadNextBatch()) { + if (reader.getVectorSchemaRoot().getRowCount() > 0) { + containsRows = true; + break; + } + } + } } catch (Exception e) { throw new IllegalArgumentException("Failed to parse Arrow IPC stream", e); } Preconditions.checkArgument(schema != null, "No schema found in Arrow IPC stream"); + if (containsRows) { + throw new UnsupportedOperationException( + "CreateTable only supports schema-only Arrow streams; " + + "write records through a Lance client or engine after creation"); + } return schema; } } diff --git a/lance/lance-common/src/test/java/org/apache/gravitino/lance/common/utils/TestArrowUtils.java b/lance/lance-common/src/test/java/org/apache/gravitino/lance/common/utils/TestArrowUtils.java index 43f0bf6ec6f..8390e2a1bf0 100644 --- a/lance/lance-common/src/test/java/org/apache/gravitino/lance/common/utils/TestArrowUtils.java +++ b/lance/lance-common/src/test/java/org/apache/gravitino/lance/common/utils/TestArrowUtils.java @@ -18,7 +18,13 @@ */ package org.apache.gravitino.lance.common.utils; +import java.io.ByteArrayOutputStream; import java.util.Arrays; +import java.util.List; +import org.apache.arrow.memory.RootAllocator; +import org.apache.arrow.vector.IntVector; +import org.apache.arrow.vector.VectorSchemaRoot; +import org.apache.arrow.vector.ipc.ArrowStreamWriter; import org.apache.arrow.vector.types.pojo.ArrowType; import org.apache.arrow.vector.types.pojo.Field; import org.apache.arrow.vector.types.pojo.Schema; @@ -39,4 +45,53 @@ public void testParseArrowIpcStream() throws Exception { Assertions.assertEquals(schema, parsedSchema); } + /** Verifies schema-only streams and zero-row batches remain supported. */ + @Test + public void testSchemaOnlyStreams() throws Exception { + Schema expected = new Schema(List.of(Field.nullable("id", new ArrowType.Int(32, true)))); + Assertions.assertEquals(expected, ArrowUtils.parseSchemaOnlyIpcStream(streamWithRows())); + Assertions.assertEquals(expected, ArrowUtils.parseSchemaOnlyIpcStream(streamWithRows(0, 0))); + } + + /** Verifies that a non-empty batch is rejected, including after empty batches. */ + @Test + public void testRejectRecordBatchesWithRows() throws Exception { + for (byte[] stream : List.of(streamWithRows(1), streamWithRows(0, 1))) { + UnsupportedOperationException exception = + Assertions.assertThrows( + UnsupportedOperationException.class, + () -> ArrowUtils.parseSchemaOnlyIpcStream(stream)); + Assertions.assertTrue(exception.getMessage().contains("schema-only")); + // Existing callers of the general schema parser retain their previous behavior. + Assertions.assertEquals(1, ArrowUtils.parseArrowIpcStream(stream).getFields().size()); + } + } + + /** Verifies malformed input is reported as invalid rather than as unsupported data. */ + @Test + public void testRejectMalformedSchemaOnlyStream() { + Assertions.assertThrows( + IllegalArgumentException.class, + () -> ArrowUtils.parseSchemaOnlyIpcStream(new byte[] {1, 2, 3})); + } + + private byte[] streamWithRows(int... batches) throws Exception { + Schema schema = new Schema(List.of(Field.nullable("id", new ArrowType.Int(32, true)))); + try (RootAllocator allocator = new RootAllocator(); + VectorSchemaRoot root = VectorSchemaRoot.create(schema, allocator); + ByteArrayOutputStream output = new ByteArrayOutputStream(); + ArrowStreamWriter writer = new ArrowStreamWriter(root, null, output)) { + root.allocateNew(); + writer.start(); + for (int rows : batches) { + for (int i = 0; i < rows; i++) { + ((IntVector) root.getVector("id")).setSafe(i, i); + } + root.setRowCount(rows); + writer.writeBatch(); + } + writer.end(); + return output.toByteArray(); + } + } } diff --git a/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/LanceExceptionMapper.java b/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/LanceExceptionMapper.java index 2078b75fe6f..45d4cb4f47f 100644 --- a/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/LanceExceptionMapper.java +++ b/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/LanceExceptionMapper.java @@ -23,8 +23,10 @@ import javax.ws.rs.core.Response; import javax.ws.rs.ext.ExceptionMapper; import javax.ws.rs.ext.Provider; +import org.apache.gravitino.exceptions.ForbiddenException; import org.apache.gravitino.exceptions.NoSuchTableException; import org.apache.gravitino.exceptions.NotFoundException; +import org.apache.gravitino.exceptions.UnauthorizedException; import org.lance.namespace.errors.ConcurrentModificationException; import org.lance.namespace.errors.InternalException; import org.lance.namespace.errors.InvalidInputException; @@ -66,7 +68,13 @@ public Response toResponse(Exception ex) { } private static LanceNamespaceException toLanceNamespaceException(String instance, Exception ex) { - if (ex instanceof NoSuchTableException) { + if (ex instanceof ForbiddenException) { + return new PermissionDeniedException(ex.getMessage(), "", instance); + + } else if (ex instanceof UnauthorizedException) { + return new UnauthenticatedException(ex.getMessage(), "", instance); + + } else if (ex instanceof NoSuchTableException) { return new TableNotFoundException(ex.getMessage(), getStackTrace(ex), instance); } else if (ex instanceof NotFoundException) { @@ -84,7 +92,7 @@ private static LanceNamespaceException toLanceNamespaceException(String instance } else { LOG.warn("Lance REST server unexpected exception:", ex); - return new InternalException(ex.getMessage(), getStackTrace(ex), instance); + return new InternalException("Internal server error", "", instance); } } 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 88b3d645ecd..3ec6e9c9037 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 @@ -18,8 +18,6 @@ */ package org.apache.gravitino.lance.service.authorization; -import static org.apache.commons.lang3.exception.ExceptionUtils.getStackTrace; - import java.lang.reflect.Method; import java.lang.reflect.Parameter; import java.util.HashMap; @@ -188,9 +186,7 @@ protected Object toErrorResponse(Method method, Object[] args, Throwable throwab String namespaceId = pathArgument(method.getParameters(), args, "id").orElse(""); Exception exception; if (throwable instanceof ForbiddenException) { - exception = - new PermissionDeniedException( - throwable.getMessage(), getStackTrace(throwable), namespaceId); + exception = new PermissionDeniedException(throwable.getMessage(), "", namespaceId); } else if (throwable instanceof Exception) { exception = (Exception) throwable; } else { diff --git a/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/authorization/LanceRESTAuthInterceptionService.java b/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/authorization/LanceRESTAuthInterceptionService.java index ae10436f69d..4a717aefc77 100644 --- a/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/authorization/LanceRESTAuthInterceptionService.java +++ b/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/authorization/LanceRESTAuthInterceptionService.java @@ -42,8 +42,7 @@ public class LanceRESTAuthInterceptionService implements InterceptionService { public static final String METALAKE_BINDING = "lanceAuthorizationMetalake"; // Membership here only routes a class through the interceptor; each method still opts in with - // @AuthorizationExpression, and a method without one runs unauthorized. The table writes - // (create, register, drop, alter) are still to be annotated. + // @AuthorizationExpression. Endpoint coverage tests ensure no REST operation omits it. private static final Set INTERCEPTED_CLASSES = ImmutableSet.of( LanceNamespaceOperations.class.getName(), LanceTableOperations.class.getName()); 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 1e7536e7d05..6fddbbea88c 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 @@ -334,8 +334,7 @@ public void testStandaloneUsesBackendServiceIdentity() throws Exception { HttpRequest.BodyPublishers.ofString( ObjectMapperProvider.objectMapper().writeValueAsString(body))) .build(); - // USER cannot create catalogs in auxiliary mode. The remote backend receives the service - // user's + // USER cannot create catalogs in auxiliary mode. The backend receives the service user's // credentials and roles, despite USER selecting NONE on this incoming request. assertStatus(200, httpClient.send(request, HttpResponse.BodyHandlers.ofString())); Assertions.assertEquals( @@ -345,6 +344,26 @@ public void testStandaloneUsesBackendServiceIdentity() throws Exception { .loadCatalog(catalog) .auditInfo() .creator()); + // The backend service user cannot read this admin-owned schema. Even an incoming admin + // must receive the backend's 403, rather than 500 or the incoming caller's privileges. + HttpRequest deniedRequest = + request(ADMIN, "/v1/namespace/" + id(VISIBLE_CATALOG, VISIBLE_SCHEMA) + "/describe") + .uri( + URI.create( + "http://localhost:" + + port + + "/lance/v1/namespace/" + + id(VISIBLE_CATALOG, VISIBLE_SCHEMA) + + "/describe?delimiter=.")) + .POST(HttpRequest.BodyPublishers.ofString("{}")) + .build(); + HttpResponse deniedResponse = + httpClient.send(deniedRequest, HttpResponse.BodyHandlers.ofString()); + assertStatus(403, deniedResponse); + ErrorResponse error = + ObjectMapperProvider.objectMapper().readValue(deniedResponse.body(), ErrorResponse.class); + Assertions.assertEquals("", error.getDetail()); + Assertions.assertTrue(error.getError().contains(serviceUser), error.getError()); assertStatus(200, drop(serviceUser, catalog, null, "cascade")); } finally { standalone.serviceStop(); diff --git a/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/integration/test/LanceTableAuthorizationIT.java b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/integration/test/LanceTableAuthorizationIT.java index c7722b05235..25a91d79a7f 100644 --- a/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/integration/test/LanceTableAuthorizationIT.java +++ b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/integration/test/LanceTableAuthorizationIT.java @@ -18,16 +18,22 @@ */ package org.apache.gravitino.lance.integration.test; +import java.io.ByteArrayOutputStream; import java.net.URI; import java.net.http.HttpClient; import java.net.http.HttpRequest; import java.net.http.HttpResponse; import java.nio.charset.StandardCharsets; +import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; import java.util.Base64; import java.util.HashMap; import java.util.List; +import org.apache.arrow.memory.RootAllocator; +import org.apache.arrow.vector.IntVector; +import org.apache.arrow.vector.VectorSchemaRoot; +import org.apache.arrow.vector.ipc.ArrowStreamWriter; import org.apache.arrow.vector.types.pojo.ArrowType; import org.apache.arrow.vector.types.pojo.Field; import org.apache.arrow.vector.types.pojo.Schema; @@ -49,6 +55,7 @@ import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import org.lance.Dataset; import org.lance.namespace.model.AlterTableDropColumnsRequest; import org.lance.namespace.model.CreateNamespaceRequest; import org.lance.namespace.model.DeclareTableRequest; @@ -343,6 +350,63 @@ public void testMutationConcealsTablesTheCallerMayNotSee() throws Exception { assertStatus(404, table(ADMIN, MISSING_TABLE, "deregister")); } + /** Verifies unsupported input cannot silently discard records or destroy an existing table. */ + @Test + public void testCreateRejectsNonEmptyArrowWithoutSideEffects() throws Exception { + byte[] data = arrowStreamWithRecord(); + for (String mode : List.of("create", "exist_ok")) { + String name = "nonempty_" + mode; + assertStatus(406, createWithData(PROBER, name, mode, data)); + assertStatus(404, table(ADMIN, WRITE_SCHEMA, name, "exists")); + Assertions.assertFalse(Files.exists(tempDir.resolve(name))); + } + + String original = "nonempty_overwrite"; + createTable(WRITE_SCHEMA, original); + assertStatus(406, createWithData(MUTATOR, original, "overwrite", data)); + Assertions.assertEquals( + List.of("id", "value"), + describe(ADMIN, WRITE_SCHEMA, original).getSchema().getFields().stream() + .map(field -> field.getName()) + .toList()); + try (Dataset dataset = Dataset.open().uri(location(original)).build()) { + Assertions.assertEquals(0, dataset.countRows()); + Assertions.assertEquals(2, dataset.getSchema().getFields().size()); + } + } + + private HttpResponse createWithData( + String user, String tableName, String mode, byte[] data) throws Exception { + HttpRequest req = + request( + user, + "/v1/table/" + id(CATALOG, WRITE_SCHEMA, tableName) + "/create", + "&mode=" + mode) + .setHeader("Content-Type", "application/vnd.apache.arrow.stream") + .setHeader(LanceConstants.LANCE_TABLE_LOCATION_HEADER, location(tableName)) + .POST(HttpRequest.BodyPublishers.ofByteArray(data)) + .build(); + return httpClient.send(req, HttpResponse.BodyHandlers.ofString()); + } + + private byte[] arrowStreamWithRecord() throws Exception { + Schema schema = new Schema(List.of(Field.nullable("id", new ArrowType.Int(32, true)))); + try (RootAllocator allocator = new RootAllocator(); + VectorSchemaRoot root = VectorSchemaRoot.create(schema, allocator); + ByteArrayOutputStream output = new ByteArrayOutputStream(); + ArrowStreamWriter writer = new ArrowStreamWriter(root, null, output)) { + root.allocateNew(); + root.setRowCount(0); + writer.start(); + writer.writeBatch(); + ((IntVector) root.getVector("id")).setSafe(0, 42); + root.setRowCount(1); + writer.writeBatch(); + writer.end(); + return output.toByteArray(); + } + } + private HttpResponse dropColumns(String user, String tableName, String column) throws Exception { return dropColumns(user, WRITE_SCHEMA, tableName, column); diff --git a/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/TestLanceExceptionMapper.java b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/TestLanceExceptionMapper.java new file mode 100644 index 00000000000..aea2c57943a --- /dev/null +++ b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/TestLanceExceptionMapper.java @@ -0,0 +1,79 @@ +/* + * 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; + +import javax.ws.rs.core.Response; +import org.apache.gravitino.exceptions.ForbiddenException; +import org.apache.gravitino.exceptions.UnauthorizedException; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.lance.namespace.errors.InvalidInputException; +import org.lance.namespace.model.ErrorResponse; + +/** Verifies backend authentication failures retain their protocol status without stack traces. */ +public class TestLanceExceptionMapper { + + /** Verifies backend authorization failures use the Lance forbidden response. */ + @Test + public void testBackendForbidden() { + assertAuthenticationError(new ForbiddenException("Access denied"), 403); + } + + /** Verifies backend authentication failures use the Lance unauthenticated response. */ + @Test + public void testBackendUnauthorized() { + assertAuthenticationError(new UnauthorizedException("Invalid credentials"), 401); + } + + /** Verifies unexpected exceptions do not expose internal details in the response. */ + @Test + public void testInternalFailureDoesNotExposeException() { + try (Response response = + LanceExceptionMapper.toRESTResponse( + "catalog.schema.table", new RuntimeException("private-backend-detail"))) { + Assertions.assertEquals(500, response.getStatus()); + ErrorResponse error = (ErrorResponse) response.getEntity(); + Assertions.assertEquals("Internal server error", error.getError()); + Assertions.assertEquals("", error.getDetail()); + } + } + + /** Verifies intentional protocol validation details remain available to callers. */ + @Test + public void testProtocolValidationDetailsArePreserved() { + try (Response response = + LanceExceptionMapper.toRESTResponse( + "table", + new InvalidInputException("Invalid field", "field must be positive", "table"))) { + Assertions.assertEquals(400, response.getStatus()); + Assertions.assertEquals( + "field must be positive", ((ErrorResponse) response.getEntity()).getDetail()); + } + } + + private void assertAuthenticationError(Exception exception, int status) { + try (Response response = LanceExceptionMapper.toRESTResponse("catalog", exception)) { + Assertions.assertEquals(status, response.getStatus()); + ErrorResponse error = (ErrorResponse) response.getEntity(); + Assertions.assertEquals(exception.getMessage(), error.getError()); + Assertions.assertEquals("", error.getDetail()); + Assertions.assertEquals("catalog", error.getInstance()); + } + } +} diff --git a/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/rest/TestLanceNamespaceOperations.java b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/rest/TestLanceNamespaceOperations.java index a80c70a06b5..87069562bd1 100644 --- a/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/rest/TestLanceNamespaceOperations.java +++ b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/rest/TestLanceNamespaceOperations.java @@ -208,10 +208,9 @@ public void testListNamespaces() { ErrorResponse errorResp = resp.readEntity(ErrorResponse.class); Assertions.assertEquals(18, errorResp.getCode()); - Assertions.assertEquals("Test exception", errorResp.getError()); + Assertions.assertEquals("Internal server error", errorResp.getError()); + Assertions.assertEquals("", errorResp.getDetail()); Assertions.assertEquals("ns1.ns2", errorResp.getInstance()); - Assertions.assertNotNull(errorResp.getDetail()); - Assertions.assertTrue(errorResp.getDetail().contains("Test exception")); // root endpoint should use explicit root identifier instead of delimiter in error instance resp = @@ -262,7 +261,8 @@ public void testDescribeNamespace() { ErrorResponse errorResp = resp.readEntity(ErrorResponse.class); Assertions.assertEquals(18, errorResp.getCode()); - Assertions.assertEquals("Test exception", errorResp.getError()); + Assertions.assertEquals("Internal server error", errorResp.getError()); + Assertions.assertEquals("", errorResp.getDetail()); } @Test @@ -321,7 +321,8 @@ public void testCreateNamespace() { ErrorResponse errorResp = resp.readEntity(ErrorResponse.class); Assertions.assertEquals(18, errorResp.getCode()); - Assertions.assertEquals("Test exception", errorResp.getError()); + Assertions.assertEquals("Internal server error", errorResp.getError()); + Assertions.assertEquals("", errorResp.getDetail()); } @Test @@ -393,7 +394,8 @@ public void testDropNamespace() { ErrorResponse errorResp = resp.readEntity(ErrorResponse.class); Assertions.assertEquals(18, errorResp.getCode()); - Assertions.assertEquals("Test exception", errorResp.getError()); + Assertions.assertEquals("Internal server error", errorResp.getError()); + Assertions.assertEquals("", errorResp.getDetail()); } @Test @@ -457,7 +459,8 @@ void testCreateTable() { Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), resp.getStatus()); Assertions.assertEquals(MediaType.APPLICATION_JSON_TYPE, resp.getMediaType()); ErrorResponse errorResp = resp.readEntity(ErrorResponse.class); - Assertions.assertEquals("Runtime exception", errorResp.getError()); + Assertions.assertEquals("Internal server error", errorResp.getError()); + Assertions.assertEquals("", errorResp.getDetail()); } @Test @@ -513,7 +516,8 @@ void testRegisterTable() { Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), resp.getStatus()); Assertions.assertEquals(MediaType.APPLICATION_JSON_TYPE, resp.getMediaType()); ErrorResponse errorResp = resp.readEntity(ErrorResponse.class); - Assertions.assertEquals("Runtime exception", errorResp.getError()); + Assertions.assertEquals("Internal server error", errorResp.getError()); + Assertions.assertEquals("", errorResp.getDetail()); } @Test @@ -620,7 +624,8 @@ void testDeregisterTable() { Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), resp.getStatus()); Assertions.assertEquals(MediaType.APPLICATION_JSON_TYPE, resp.getMediaType()); ErrorResponse errorResp = resp.readEntity(ErrorResponse.class); - Assertions.assertEquals("Runtime exception", errorResp.getError()); + Assertions.assertEquals("Internal server error", errorResp.getError()); + Assertions.assertEquals("", errorResp.getDetail()); } @Test @@ -677,7 +682,8 @@ void testDescribeTable() { Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), resp.getStatus()); Assertions.assertEquals(MediaType.APPLICATION_JSON_TYPE, resp.getMediaType()); ErrorResponse errorResp = resp.readEntity(ErrorResponse.class); - Assertions.assertEquals("Runtime exception", errorResp.getError()); + Assertions.assertEquals("Internal server error", errorResp.getError()); + Assertions.assertEquals("", errorResp.getDetail()); } @Test @@ -999,6 +1005,7 @@ void testDeclareTable() { Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), resp.getStatus()); Assertions.assertEquals(MediaType.APPLICATION_JSON_TYPE, resp.getMediaType()); ErrorResponse errorResp = resp.readEntity(ErrorResponse.class); - Assertions.assertEquals("Runtime exception", errorResp.getError()); + Assertions.assertEquals("Internal server error", errorResp.getError()); + Assertions.assertEquals("", errorResp.getDetail()); } } From bdb9bc140360f8d2adf9cced3b66976a0d3df2d3 Mon Sep 17 00:00:00 2001 From: yuqi Date: Mon, 7 Sep 2026 19:08:54 +0800 Subject: [PATCH 4/9] [#12574] test(lance): isolate standalone auth integration service in its own JVM --- docs/lance-rest-integration.md | 5 +- lance/lance-rest-server/build.gradle.kts | 1 + .../test/LanceNamespaceAuthorizationIT.java | 74 ++++++++++++++----- 3 files changed, 60 insertions(+), 20 deletions(-) diff --git a/docs/lance-rest-integration.md b/docs/lance-rest-integration.md index 41df4c7336f..d03573a99f4 100644 --- a/docs/lance-rest-integration.md +++ b/docs/lance-rest-integration.md @@ -144,8 +144,9 @@ per-user, scoped storage credentials. The HTTP integration suites start Gravitino with the Lance auxiliary service and exercise caller identity, service identity fallback, active roles, namespace and table privileges, filtered listings, denied mutations, and rejection of non-empty Arrow creates without side effects. -They also start a separate Lance listener in standalone mode to verify its outbound service -identity and propagation of backend authorization denials through the Gravitino HTTP API. +They also start standalone Lance REST through its production entry point in a separate JVM to verify +its outbound service identity and propagation of backend authorization denials through the Gravitino +HTTP API. ```shell ./gradlew :lance:lance-rest-server:test \ diff --git a/lance/lance-rest-server/build.gradle.kts b/lance/lance-rest-server/build.gradle.kts index 176c4f16b7a..db4408e7850 100644 --- a/lance/lance-rest-server/build.gradle.kts +++ b/lance/lance-rest-server/build.gradle.kts @@ -193,6 +193,7 @@ tasks { val primaryBundleDir = lanceSparkBundleDirFor(primaryLanceSparkBundleVersion) doFirst { + systemProperty("lance.test.runtimeClasspath", sourceSets["main"].runtimeClasspath.asPath) val bundleJar = primaryBundleDir.get().asFile.listFiles()?.singleOrNull { it.extension == "jar" } ?: throw GradleException( 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 6fddbbea88c..ae4bffe98ad 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 @@ -18,16 +18,21 @@ */ package org.apache.gravitino.lance.integration.test; +import java.io.Writer; import java.net.URI; import java.net.http.HttpClient; import java.net.http.HttpRequest; import java.net.http.HttpResponse; import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.ArrayList; import java.util.Base64; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Properties; +import java.util.concurrent.TimeUnit; import org.apache.gravitino.Configs; import org.apache.gravitino.auth.AuthConstants; import org.apache.gravitino.authorization.Privileges; @@ -35,13 +40,16 @@ import org.apache.gravitino.authorization.SecurableObjects; import org.apache.gravitino.client.GravitinoMetalake; import org.apache.gravitino.integration.test.util.BaseIT; -import org.apache.gravitino.lance.LanceRESTService; +import org.apache.gravitino.integration.test.util.HttpUtils; +import org.apache.gravitino.lance.server.GravitinoLanceRESTServer; import org.apache.gravitino.rest.RESTUtils; import org.apache.gravitino.server.web.ObjectMapperProvider; +import org.awaitility.Awaitility; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; import org.lance.namespace.model.CreateNamespaceRequest; import org.lance.namespace.model.DescribeNamespaceResponse; import org.lance.namespace.model.DropNamespaceRequest; @@ -287,7 +295,7 @@ public void testServiceIdentityFallbackIsAuthorized() throws Exception { /** Verifies standalone HTTP backend calls use service credentials rather than caller roles. */ @Test - public void testStandaloneUsesBackendServiceIdentity() throws Exception { + public void testStandaloneUsesBackendServiceIdentity(@TempDir Path directory) throws Exception { int port = RESTUtils.findAvailablePort(10000, 11000); String catalog = "lance_authz_standalone_catalog"; String serviceUser = "lance_authz_standalone_user"; @@ -302,22 +310,47 @@ public void testStandaloneUsesBackendServiceIdentity() throws Exception { new ArrayList<>( List.of(Privileges.UseCatalog.allow(), Privileges.CreateCatalog.allow()))))); metalake.grantRolesToUser(List.of("lance_authz_standalone_role"), serviceUser); - LanceRESTService standalone = new LanceRESTService(); + Properties config = new Properties(); + config.setProperty(Configs.AUTHENTICATORS.getKey(), "simple"); + config.setProperty("gravitino.lance-rest.httpPort", String.valueOf(port)); + config.setProperty( + "gravitino.lance-rest.gravitino-uri", "http://localhost:" + getGravitinoServerPort()); + config.setProperty("gravitino.lance-rest.gravitino-metalake", getLanceRESTServerMetalakeName()); + config.setProperty("gravitino.lance-rest.gravitino-auth-type", "simple"); + config.setProperty("gravitino.lance-rest.gravitino-simple.user-name", serviceUser); + Path configFile = directory.resolve("standalone.conf"); + try (Writer writer = Files.newBufferedWriter(configFile)) { + config.store(writer, "Standalone Lance REST integration test"); + } + Path logFile = directory.resolve("standalone.log"); + // Use the production bootstrap in its own JVM: deploy mode has no local GravitinoEnv, + // while embedded mode must not share its backend environment with the standalone service. + ProcessBuilder builder = + new ProcessBuilder( + Path.of(System.getProperty("java.home"), "bin", "java").toString(), + "--add-opens=java.base/java.nio=ALL-UNNAMED", + "-cp", + System.getProperty("lance.test.runtimeClasspath"), + GravitinoLanceRESTServer.class.getName(), + configFile.toString()) + .redirectErrorStream(true) + .redirectOutput(logFile.toFile()); + builder.environment().put("GRAVITINO_TEST", "true"); + Process standalone = builder.start(); try { - standalone.serviceInit( - Map.of( - "httpPort", - String.valueOf(port), - "gravitino-uri", - "http://localhost:" + getGravitinoServerPort(), - "gravitino-metalake", - getLanceRESTServerMetalakeName(), - "gravitino-auth-type", - "simple", - "gravitino-simple.user-name", - serviceUser), - false); - standalone.serviceStart(); + try { + Awaitility.await() + .atMost(60, TimeUnit.SECONDS) + .until( + () -> { + Assertions.assertTrue(standalone.isAlive(), "Standalone process exited"); + // Namespace initialization is lazy and occurs on the first metadata request. + return HttpUtils.isHttpServerUp( + "http://localhost:" + port + "/lance/health/live"); + }); + } catch (Exception | AssertionError e) { + throw new AssertionError("Standalone startup failed:\n" + Files.readString(logFile), e); + } CreateNamespaceRequest body = new CreateNamespaceRequest(); body.addIdItem(catalog); HttpRequest request = @@ -366,7 +399,12 @@ public void testStandaloneUsesBackendServiceIdentity() throws Exception { Assertions.assertTrue(error.getError().contains(serviceUser), error.getError()); assertStatus(200, drop(serviceUser, catalog, null, "cascade")); } finally { - standalone.serviceStop(); + standalone.destroy(); + if (!standalone.waitFor(10, TimeUnit.SECONDS)) { + standalone.destroyForcibly(); + Assertions.assertTrue( + standalone.waitFor(10, TimeUnit.SECONDS), "Standalone process did not stop"); + } } } From b0de2086c1e61226fcff48f533f0d683397c0d33 Mon Sep 17 00:00:00 2001 From: yuqi Date: Tue, 8 Sep 2026 16:46:52 +0800 Subject: [PATCH 5/9] docs(lance): separate runtime fixes from authentication documentation --- .../GravitinoLanceTableOperations.java | 4 +- .../lance/common/utils/ArrowUtils.java | 30 ----- .../lance/common/utils/TestArrowUtils.java | 55 -------- lance/lance-rest-server/build.gradle.kts | 1 - .../lance/service/LanceExceptionMapper.java | 12 +- ...etadataAuthorizationMethodInterceptor.java | 6 +- .../test/LanceNamespaceAuthorizationIT.java | 125 ------------------ .../test/LanceTableAuthorizationIT.java | 64 --------- .../service/TestLanceExceptionMapper.java | 79 ----------- .../rest/TestLanceNamespaceOperations.java | 29 ++-- 10 files changed, 20 insertions(+), 385 deletions(-) delete mode 100644 lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/TestLanceExceptionMapper.java diff --git a/lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/GravitinoLanceTableOperations.java b/lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/GravitinoLanceTableOperations.java index 49bd407e7ee..08bd9bae3a9 100644 --- a/lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/GravitinoLanceTableOperations.java +++ b/lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/GravitinoLanceTableOperations.java @@ -168,11 +168,11 @@ public CreateTableResponse createTable( Preconditions.checkArgument( nsId.levels() == 3, "Expected at 3-level namespace but got: %s", nsId.levels()); - // Reject unsupported record batches before any metadata or storage mutation. + // Parser column information. List columns = Lists.newArrayList(); if (arrowStreamBody != null) { org.apache.arrow.vector.types.pojo.Schema schema = - ArrowUtils.parseSchemaOnlyIpcStream(arrowStreamBody); + ArrowUtils.parseArrowIpcStream(arrowStreamBody); columns = extractColumns(schema); } diff --git a/lance/lance-common/src/main/java/org/apache/gravitino/lance/common/utils/ArrowUtils.java b/lance/lance-common/src/main/java/org/apache/gravitino/lance/common/utils/ArrowUtils.java index d2213e5e748..5d8508ee459 100644 --- a/lance/lance-common/src/main/java/org/apache/gravitino/lance/common/utils/ArrowUtils.java +++ b/lance/lance-common/src/main/java/org/apache/gravitino/lance/common/utils/ArrowUtils.java @@ -57,46 +57,16 @@ public static byte[] generateIpcStream(Schema arrowSchema) throws IOException { } public static Schema parseArrowIpcStream(byte[] stream) { - return parseArrowIpcStream(stream, false); - } - - /** - * Parses a schema-only Arrow IPC stream, rejecting record batches containing rows. - * - * @param stream the Arrow IPC stream - * @return the stream schema - * @throws UnsupportedOperationException if any record batch contains rows - * @throws IllegalArgumentException if the stream cannot be parsed - */ - public static Schema parseSchemaOnlyIpcStream(byte[] stream) { - return parseArrowIpcStream(stream, true); - } - - private static Schema parseArrowIpcStream(byte[] stream, boolean requireEmpty) { Schema schema; - boolean containsRows = false; try (BufferAllocator allocator = new RootAllocator(); ByteArrayInputStream bais = new ByteArrayInputStream(stream); ArrowStreamReader reader = new ArrowStreamReader(bais, allocator)) { schema = reader.getVectorSchemaRoot().getSchema(); - if (requireEmpty) { - while (reader.loadNextBatch()) { - if (reader.getVectorSchemaRoot().getRowCount() > 0) { - containsRows = true; - break; - } - } - } } catch (Exception e) { throw new IllegalArgumentException("Failed to parse Arrow IPC stream", e); } Preconditions.checkArgument(schema != null, "No schema found in Arrow IPC stream"); - if (containsRows) { - throw new UnsupportedOperationException( - "CreateTable only supports schema-only Arrow streams; " - + "write records through a Lance client or engine after creation"); - } return schema; } } diff --git a/lance/lance-common/src/test/java/org/apache/gravitino/lance/common/utils/TestArrowUtils.java b/lance/lance-common/src/test/java/org/apache/gravitino/lance/common/utils/TestArrowUtils.java index 8390e2a1bf0..43f0bf6ec6f 100644 --- a/lance/lance-common/src/test/java/org/apache/gravitino/lance/common/utils/TestArrowUtils.java +++ b/lance/lance-common/src/test/java/org/apache/gravitino/lance/common/utils/TestArrowUtils.java @@ -18,13 +18,7 @@ */ package org.apache.gravitino.lance.common.utils; -import java.io.ByteArrayOutputStream; import java.util.Arrays; -import java.util.List; -import org.apache.arrow.memory.RootAllocator; -import org.apache.arrow.vector.IntVector; -import org.apache.arrow.vector.VectorSchemaRoot; -import org.apache.arrow.vector.ipc.ArrowStreamWriter; import org.apache.arrow.vector.types.pojo.ArrowType; import org.apache.arrow.vector.types.pojo.Field; import org.apache.arrow.vector.types.pojo.Schema; @@ -45,53 +39,4 @@ public void testParseArrowIpcStream() throws Exception { Assertions.assertEquals(schema, parsedSchema); } - /** Verifies schema-only streams and zero-row batches remain supported. */ - @Test - public void testSchemaOnlyStreams() throws Exception { - Schema expected = new Schema(List.of(Field.nullable("id", new ArrowType.Int(32, true)))); - Assertions.assertEquals(expected, ArrowUtils.parseSchemaOnlyIpcStream(streamWithRows())); - Assertions.assertEquals(expected, ArrowUtils.parseSchemaOnlyIpcStream(streamWithRows(0, 0))); - } - - /** Verifies that a non-empty batch is rejected, including after empty batches. */ - @Test - public void testRejectRecordBatchesWithRows() throws Exception { - for (byte[] stream : List.of(streamWithRows(1), streamWithRows(0, 1))) { - UnsupportedOperationException exception = - Assertions.assertThrows( - UnsupportedOperationException.class, - () -> ArrowUtils.parseSchemaOnlyIpcStream(stream)); - Assertions.assertTrue(exception.getMessage().contains("schema-only")); - // Existing callers of the general schema parser retain their previous behavior. - Assertions.assertEquals(1, ArrowUtils.parseArrowIpcStream(stream).getFields().size()); - } - } - - /** Verifies malformed input is reported as invalid rather than as unsupported data. */ - @Test - public void testRejectMalformedSchemaOnlyStream() { - Assertions.assertThrows( - IllegalArgumentException.class, - () -> ArrowUtils.parseSchemaOnlyIpcStream(new byte[] {1, 2, 3})); - } - - private byte[] streamWithRows(int... batches) throws Exception { - Schema schema = new Schema(List.of(Field.nullable("id", new ArrowType.Int(32, true)))); - try (RootAllocator allocator = new RootAllocator(); - VectorSchemaRoot root = VectorSchemaRoot.create(schema, allocator); - ByteArrayOutputStream output = new ByteArrayOutputStream(); - ArrowStreamWriter writer = new ArrowStreamWriter(root, null, output)) { - root.allocateNew(); - writer.start(); - for (int rows : batches) { - for (int i = 0; i < rows; i++) { - ((IntVector) root.getVector("id")).setSafe(i, i); - } - root.setRowCount(rows); - writer.writeBatch(); - } - writer.end(); - return output.toByteArray(); - } - } } diff --git a/lance/lance-rest-server/build.gradle.kts b/lance/lance-rest-server/build.gradle.kts index db4408e7850..176c4f16b7a 100644 --- a/lance/lance-rest-server/build.gradle.kts +++ b/lance/lance-rest-server/build.gradle.kts @@ -193,7 +193,6 @@ tasks { val primaryBundleDir = lanceSparkBundleDirFor(primaryLanceSparkBundleVersion) doFirst { - systemProperty("lance.test.runtimeClasspath", sourceSets["main"].runtimeClasspath.asPath) val bundleJar = primaryBundleDir.get().asFile.listFiles()?.singleOrNull { it.extension == "jar" } ?: throw GradleException( diff --git a/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/LanceExceptionMapper.java b/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/LanceExceptionMapper.java index 45d4cb4f47f..2078b75fe6f 100644 --- a/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/LanceExceptionMapper.java +++ b/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/LanceExceptionMapper.java @@ -23,10 +23,8 @@ import javax.ws.rs.core.Response; import javax.ws.rs.ext.ExceptionMapper; import javax.ws.rs.ext.Provider; -import org.apache.gravitino.exceptions.ForbiddenException; import org.apache.gravitino.exceptions.NoSuchTableException; import org.apache.gravitino.exceptions.NotFoundException; -import org.apache.gravitino.exceptions.UnauthorizedException; import org.lance.namespace.errors.ConcurrentModificationException; import org.lance.namespace.errors.InternalException; import org.lance.namespace.errors.InvalidInputException; @@ -68,13 +66,7 @@ public Response toResponse(Exception ex) { } private static LanceNamespaceException toLanceNamespaceException(String instance, Exception ex) { - if (ex instanceof ForbiddenException) { - return new PermissionDeniedException(ex.getMessage(), "", instance); - - } else if (ex instanceof UnauthorizedException) { - return new UnauthenticatedException(ex.getMessage(), "", instance); - - } else if (ex instanceof NoSuchTableException) { + if (ex instanceof NoSuchTableException) { return new TableNotFoundException(ex.getMessage(), getStackTrace(ex), instance); } else if (ex instanceof NotFoundException) { @@ -92,7 +84,7 @@ private static LanceNamespaceException toLanceNamespaceException(String instance } else { LOG.warn("Lance REST server unexpected exception:", ex); - return new InternalException("Internal server error", "", instance); + return new InternalException(ex.getMessage(), getStackTrace(ex), instance); } } 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 3ec6e9c9037..88b3d645ecd 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 @@ -18,6 +18,8 @@ */ package org.apache.gravitino.lance.service.authorization; +import static org.apache.commons.lang3.exception.ExceptionUtils.getStackTrace; + import java.lang.reflect.Method; import java.lang.reflect.Parameter; import java.util.HashMap; @@ -186,7 +188,9 @@ protected Object toErrorResponse(Method method, Object[] args, Throwable throwab String namespaceId = pathArgument(method.getParameters(), args, "id").orElse(""); Exception exception; if (throwable instanceof ForbiddenException) { - exception = new PermissionDeniedException(throwable.getMessage(), "", namespaceId); + exception = + new PermissionDeniedException( + throwable.getMessage(), getStackTrace(throwable), namespaceId); } else if (throwable instanceof Exception) { exception = (Exception) throwable; } else { 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 ae4bffe98ad..a3e6b42ad89 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 @@ -18,21 +18,16 @@ */ package org.apache.gravitino.lance.integration.test; -import java.io.Writer; import java.net.URI; import java.net.http.HttpClient; import java.net.http.HttpRequest; import java.net.http.HttpResponse; import java.nio.charset.StandardCharsets; -import java.nio.file.Files; -import java.nio.file.Path; import java.util.ArrayList; import java.util.Base64; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Properties; -import java.util.concurrent.TimeUnit; import org.apache.gravitino.Configs; import org.apache.gravitino.auth.AuthConstants; import org.apache.gravitino.authorization.Privileges; @@ -40,16 +35,11 @@ import org.apache.gravitino.authorization.SecurableObjects; import org.apache.gravitino.client.GravitinoMetalake; import org.apache.gravitino.integration.test.util.BaseIT; -import org.apache.gravitino.integration.test.util.HttpUtils; -import org.apache.gravitino.lance.server.GravitinoLanceRESTServer; -import org.apache.gravitino.rest.RESTUtils; import org.apache.gravitino.server.web.ObjectMapperProvider; -import org.awaitility.Awaitility; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.io.TempDir; import org.lance.namespace.model.CreateNamespaceRequest; import org.lance.namespace.model.DescribeNamespaceResponse; import org.lance.namespace.model.DropNamespaceRequest; @@ -293,121 +283,6 @@ public void testServiceIdentityFallbackIsAuthorized() throws Exception { assertStatus(403, create(USER, catalog, null, Map.of())); } - /** Verifies standalone HTTP backend calls use service credentials rather than caller roles. */ - @Test - public void testStandaloneUsesBackendServiceIdentity(@TempDir Path directory) throws Exception { - int port = RESTUtils.findAvailablePort(10000, 11000); - String catalog = "lance_authz_standalone_catalog"; - String serviceUser = "lance_authz_standalone_user"; - GravitinoMetalake metalake = client.loadMetalake(getLanceRESTServerMetalakeName()); - metalake.addUser(serviceUser); - metalake.createRole( - "lance_authz_standalone_role", - new HashMap<>(), - List.of( - SecurableObjects.ofMetalake( - metalake.name(), - new ArrayList<>( - List.of(Privileges.UseCatalog.allow(), Privileges.CreateCatalog.allow()))))); - metalake.grantRolesToUser(List.of("lance_authz_standalone_role"), serviceUser); - Properties config = new Properties(); - config.setProperty(Configs.AUTHENTICATORS.getKey(), "simple"); - config.setProperty("gravitino.lance-rest.httpPort", String.valueOf(port)); - config.setProperty( - "gravitino.lance-rest.gravitino-uri", "http://localhost:" + getGravitinoServerPort()); - config.setProperty("gravitino.lance-rest.gravitino-metalake", getLanceRESTServerMetalakeName()); - config.setProperty("gravitino.lance-rest.gravitino-auth-type", "simple"); - config.setProperty("gravitino.lance-rest.gravitino-simple.user-name", serviceUser); - Path configFile = directory.resolve("standalone.conf"); - try (Writer writer = Files.newBufferedWriter(configFile)) { - config.store(writer, "Standalone Lance REST integration test"); - } - Path logFile = directory.resolve("standalone.log"); - // Use the production bootstrap in its own JVM: deploy mode has no local GravitinoEnv, - // while embedded mode must not share its backend environment with the standalone service. - ProcessBuilder builder = - new ProcessBuilder( - Path.of(System.getProperty("java.home"), "bin", "java").toString(), - "--add-opens=java.base/java.nio=ALL-UNNAMED", - "-cp", - System.getProperty("lance.test.runtimeClasspath"), - GravitinoLanceRESTServer.class.getName(), - configFile.toString()) - .redirectErrorStream(true) - .redirectOutput(logFile.toFile()); - builder.environment().put("GRAVITINO_TEST", "true"); - Process standalone = builder.start(); - try { - try { - Awaitility.await() - .atMost(60, TimeUnit.SECONDS) - .until( - () -> { - Assertions.assertTrue(standalone.isAlive(), "Standalone process exited"); - // Namespace initialization is lazy and occurs on the first metadata request. - return HttpUtils.isHttpServerUp( - "http://localhost:" + port + "/lance/health/live"); - }); - } catch (Exception | AssertionError e) { - throw new AssertionError("Standalone startup failed:\n" + Files.readString(logFile), e); - } - CreateNamespaceRequest body = new CreateNamespaceRequest(); - body.addIdItem(catalog); - HttpRequest request = - request(USER, "/v1/namespace/" + catalog + "/create") - .uri( - URI.create( - "http://localhost:" - + port - + "/lance/v1/namespace/" - + catalog - + "/create?delimiter=.")) - .setHeader(AuthConstants.X_GRAVITINO_ACTIVE_ROLES_HEADER, "NONE") - .POST( - HttpRequest.BodyPublishers.ofString( - ObjectMapperProvider.objectMapper().writeValueAsString(body))) - .build(); - // USER cannot create catalogs in auxiliary mode. The backend receives the service user's - // credentials and roles, despite USER selecting NONE on this incoming request. - assertStatus(200, httpClient.send(request, HttpResponse.BodyHandlers.ofString())); - Assertions.assertEquals( - serviceUser, - client - .loadMetalake(getLanceRESTServerMetalakeName()) - .loadCatalog(catalog) - .auditInfo() - .creator()); - // The backend service user cannot read this admin-owned schema. Even an incoming admin - // must receive the backend's 403, rather than 500 or the incoming caller's privileges. - HttpRequest deniedRequest = - request(ADMIN, "/v1/namespace/" + id(VISIBLE_CATALOG, VISIBLE_SCHEMA) + "/describe") - .uri( - URI.create( - "http://localhost:" - + port - + "/lance/v1/namespace/" - + id(VISIBLE_CATALOG, VISIBLE_SCHEMA) - + "/describe?delimiter=.")) - .POST(HttpRequest.BodyPublishers.ofString("{}")) - .build(); - HttpResponse deniedResponse = - httpClient.send(deniedRequest, HttpResponse.BodyHandlers.ofString()); - assertStatus(403, deniedResponse); - ErrorResponse error = - ObjectMapperProvider.objectMapper().readValue(deniedResponse.body(), ErrorResponse.class); - Assertions.assertEquals("", error.getDetail()); - Assertions.assertTrue(error.getError().contains(serviceUser), error.getError()); - assertStatus(200, drop(serviceUser, catalog, null, "cascade")); - } finally { - standalone.destroy(); - if (!standalone.waitFor(10, TimeUnit.SECONDS)) { - standalone.destroyForcibly(); - Assertions.assertTrue( - standalone.waitFor(10, TimeUnit.SECONDS), "Standalone process did not stop"); - } - } - } - /** Verifies health endpoints remain reachable even when credentials would be rejected. */ @Test public void testHealthBypassesAuthentication() throws Exception { diff --git a/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/integration/test/LanceTableAuthorizationIT.java b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/integration/test/LanceTableAuthorizationIT.java index 25a91d79a7f..c7722b05235 100644 --- a/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/integration/test/LanceTableAuthorizationIT.java +++ b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/integration/test/LanceTableAuthorizationIT.java @@ -18,22 +18,16 @@ */ package org.apache.gravitino.lance.integration.test; -import java.io.ByteArrayOutputStream; import java.net.URI; import java.net.http.HttpClient; import java.net.http.HttpRequest; import java.net.http.HttpResponse; import java.nio.charset.StandardCharsets; -import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; import java.util.Base64; import java.util.HashMap; import java.util.List; -import org.apache.arrow.memory.RootAllocator; -import org.apache.arrow.vector.IntVector; -import org.apache.arrow.vector.VectorSchemaRoot; -import org.apache.arrow.vector.ipc.ArrowStreamWriter; import org.apache.arrow.vector.types.pojo.ArrowType; import org.apache.arrow.vector.types.pojo.Field; import org.apache.arrow.vector.types.pojo.Schema; @@ -55,7 +49,6 @@ import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; -import org.lance.Dataset; import org.lance.namespace.model.AlterTableDropColumnsRequest; import org.lance.namespace.model.CreateNamespaceRequest; import org.lance.namespace.model.DeclareTableRequest; @@ -350,63 +343,6 @@ public void testMutationConcealsTablesTheCallerMayNotSee() throws Exception { assertStatus(404, table(ADMIN, MISSING_TABLE, "deregister")); } - /** Verifies unsupported input cannot silently discard records or destroy an existing table. */ - @Test - public void testCreateRejectsNonEmptyArrowWithoutSideEffects() throws Exception { - byte[] data = arrowStreamWithRecord(); - for (String mode : List.of("create", "exist_ok")) { - String name = "nonempty_" + mode; - assertStatus(406, createWithData(PROBER, name, mode, data)); - assertStatus(404, table(ADMIN, WRITE_SCHEMA, name, "exists")); - Assertions.assertFalse(Files.exists(tempDir.resolve(name))); - } - - String original = "nonempty_overwrite"; - createTable(WRITE_SCHEMA, original); - assertStatus(406, createWithData(MUTATOR, original, "overwrite", data)); - Assertions.assertEquals( - List.of("id", "value"), - describe(ADMIN, WRITE_SCHEMA, original).getSchema().getFields().stream() - .map(field -> field.getName()) - .toList()); - try (Dataset dataset = Dataset.open().uri(location(original)).build()) { - Assertions.assertEquals(0, dataset.countRows()); - Assertions.assertEquals(2, dataset.getSchema().getFields().size()); - } - } - - private HttpResponse createWithData( - String user, String tableName, String mode, byte[] data) throws Exception { - HttpRequest req = - request( - user, - "/v1/table/" + id(CATALOG, WRITE_SCHEMA, tableName) + "/create", - "&mode=" + mode) - .setHeader("Content-Type", "application/vnd.apache.arrow.stream") - .setHeader(LanceConstants.LANCE_TABLE_LOCATION_HEADER, location(tableName)) - .POST(HttpRequest.BodyPublishers.ofByteArray(data)) - .build(); - return httpClient.send(req, HttpResponse.BodyHandlers.ofString()); - } - - private byte[] arrowStreamWithRecord() throws Exception { - Schema schema = new Schema(List.of(Field.nullable("id", new ArrowType.Int(32, true)))); - try (RootAllocator allocator = new RootAllocator(); - VectorSchemaRoot root = VectorSchemaRoot.create(schema, allocator); - ByteArrayOutputStream output = new ByteArrayOutputStream(); - ArrowStreamWriter writer = new ArrowStreamWriter(root, null, output)) { - root.allocateNew(); - root.setRowCount(0); - writer.start(); - writer.writeBatch(); - ((IntVector) root.getVector("id")).setSafe(0, 42); - root.setRowCount(1); - writer.writeBatch(); - writer.end(); - return output.toByteArray(); - } - } - private HttpResponse dropColumns(String user, String tableName, String column) throws Exception { return dropColumns(user, WRITE_SCHEMA, tableName, column); diff --git a/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/TestLanceExceptionMapper.java b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/TestLanceExceptionMapper.java deleted file mode 100644 index aea2c57943a..00000000000 --- a/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/TestLanceExceptionMapper.java +++ /dev/null @@ -1,79 +0,0 @@ -/* - * 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; - -import javax.ws.rs.core.Response; -import org.apache.gravitino.exceptions.ForbiddenException; -import org.apache.gravitino.exceptions.UnauthorizedException; -import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.Test; -import org.lance.namespace.errors.InvalidInputException; -import org.lance.namespace.model.ErrorResponse; - -/** Verifies backend authentication failures retain their protocol status without stack traces. */ -public class TestLanceExceptionMapper { - - /** Verifies backend authorization failures use the Lance forbidden response. */ - @Test - public void testBackendForbidden() { - assertAuthenticationError(new ForbiddenException("Access denied"), 403); - } - - /** Verifies backend authentication failures use the Lance unauthenticated response. */ - @Test - public void testBackendUnauthorized() { - assertAuthenticationError(new UnauthorizedException("Invalid credentials"), 401); - } - - /** Verifies unexpected exceptions do not expose internal details in the response. */ - @Test - public void testInternalFailureDoesNotExposeException() { - try (Response response = - LanceExceptionMapper.toRESTResponse( - "catalog.schema.table", new RuntimeException("private-backend-detail"))) { - Assertions.assertEquals(500, response.getStatus()); - ErrorResponse error = (ErrorResponse) response.getEntity(); - Assertions.assertEquals("Internal server error", error.getError()); - Assertions.assertEquals("", error.getDetail()); - } - } - - /** Verifies intentional protocol validation details remain available to callers. */ - @Test - public void testProtocolValidationDetailsArePreserved() { - try (Response response = - LanceExceptionMapper.toRESTResponse( - "table", - new InvalidInputException("Invalid field", "field must be positive", "table"))) { - Assertions.assertEquals(400, response.getStatus()); - Assertions.assertEquals( - "field must be positive", ((ErrorResponse) response.getEntity()).getDetail()); - } - } - - private void assertAuthenticationError(Exception exception, int status) { - try (Response response = LanceExceptionMapper.toRESTResponse("catalog", exception)) { - Assertions.assertEquals(status, response.getStatus()); - ErrorResponse error = (ErrorResponse) response.getEntity(); - Assertions.assertEquals(exception.getMessage(), error.getError()); - Assertions.assertEquals("", error.getDetail()); - Assertions.assertEquals("catalog", error.getInstance()); - } - } -} diff --git a/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/rest/TestLanceNamespaceOperations.java b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/rest/TestLanceNamespaceOperations.java index 87069562bd1..a80c70a06b5 100644 --- a/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/rest/TestLanceNamespaceOperations.java +++ b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/rest/TestLanceNamespaceOperations.java @@ -208,9 +208,10 @@ public void testListNamespaces() { ErrorResponse errorResp = resp.readEntity(ErrorResponse.class); Assertions.assertEquals(18, errorResp.getCode()); - Assertions.assertEquals("Internal server error", errorResp.getError()); - Assertions.assertEquals("", errorResp.getDetail()); + Assertions.assertEquals("Test exception", errorResp.getError()); Assertions.assertEquals("ns1.ns2", errorResp.getInstance()); + Assertions.assertNotNull(errorResp.getDetail()); + Assertions.assertTrue(errorResp.getDetail().contains("Test exception")); // root endpoint should use explicit root identifier instead of delimiter in error instance resp = @@ -261,8 +262,7 @@ public void testDescribeNamespace() { ErrorResponse errorResp = resp.readEntity(ErrorResponse.class); Assertions.assertEquals(18, errorResp.getCode()); - Assertions.assertEquals("Internal server error", errorResp.getError()); - Assertions.assertEquals("", errorResp.getDetail()); + Assertions.assertEquals("Test exception", errorResp.getError()); } @Test @@ -321,8 +321,7 @@ public void testCreateNamespace() { ErrorResponse errorResp = resp.readEntity(ErrorResponse.class); Assertions.assertEquals(18, errorResp.getCode()); - Assertions.assertEquals("Internal server error", errorResp.getError()); - Assertions.assertEquals("", errorResp.getDetail()); + Assertions.assertEquals("Test exception", errorResp.getError()); } @Test @@ -394,8 +393,7 @@ public void testDropNamespace() { ErrorResponse errorResp = resp.readEntity(ErrorResponse.class); Assertions.assertEquals(18, errorResp.getCode()); - Assertions.assertEquals("Internal server error", errorResp.getError()); - Assertions.assertEquals("", errorResp.getDetail()); + Assertions.assertEquals("Test exception", errorResp.getError()); } @Test @@ -459,8 +457,7 @@ void testCreateTable() { Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), resp.getStatus()); Assertions.assertEquals(MediaType.APPLICATION_JSON_TYPE, resp.getMediaType()); ErrorResponse errorResp = resp.readEntity(ErrorResponse.class); - Assertions.assertEquals("Internal server error", errorResp.getError()); - Assertions.assertEquals("", errorResp.getDetail()); + Assertions.assertEquals("Runtime exception", errorResp.getError()); } @Test @@ -516,8 +513,7 @@ void testRegisterTable() { Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), resp.getStatus()); Assertions.assertEquals(MediaType.APPLICATION_JSON_TYPE, resp.getMediaType()); ErrorResponse errorResp = resp.readEntity(ErrorResponse.class); - Assertions.assertEquals("Internal server error", errorResp.getError()); - Assertions.assertEquals("", errorResp.getDetail()); + Assertions.assertEquals("Runtime exception", errorResp.getError()); } @Test @@ -624,8 +620,7 @@ void testDeregisterTable() { Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), resp.getStatus()); Assertions.assertEquals(MediaType.APPLICATION_JSON_TYPE, resp.getMediaType()); ErrorResponse errorResp = resp.readEntity(ErrorResponse.class); - Assertions.assertEquals("Internal server error", errorResp.getError()); - Assertions.assertEquals("", errorResp.getDetail()); + Assertions.assertEquals("Runtime exception", errorResp.getError()); } @Test @@ -682,8 +677,7 @@ void testDescribeTable() { Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), resp.getStatus()); Assertions.assertEquals(MediaType.APPLICATION_JSON_TYPE, resp.getMediaType()); ErrorResponse errorResp = resp.readEntity(ErrorResponse.class); - Assertions.assertEquals("Internal server error", errorResp.getError()); - Assertions.assertEquals("", errorResp.getDetail()); + Assertions.assertEquals("Runtime exception", errorResp.getError()); } @Test @@ -1005,7 +999,6 @@ void testDeclareTable() { Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), resp.getStatus()); Assertions.assertEquals(MediaType.APPLICATION_JSON_TYPE, resp.getMediaType()); ErrorResponse errorResp = resp.readEntity(ErrorResponse.class); - Assertions.assertEquals("Internal server error", errorResp.getError()); - Assertions.assertEquals("", errorResp.getDetail()); + Assertions.assertEquals("Runtime exception", errorResp.getError()); } } From 84519b74cbbf64c12e2a2549c599a780388432f3 Mon Sep 17 00:00:00 2001 From: yuqi Date: Tue, 8 Sep 2026 17:12:57 +0800 Subject: [PATCH 6/9] fix(lance): retain authentication error fixes with auth documentation --- lance/lance-rest-server/build.gradle.kts | 1 + .../lance/service/LanceExceptionMapper.java | 12 +- ...etadataAuthorizationMethodInterceptor.java | 6 +- .../test/LanceNamespaceAuthorizationIT.java | 125 ++++++++++++++++++ .../service/TestLanceExceptionMapper.java | 79 +++++++++++ .../rest/TestLanceNamespaceOperations.java | 29 ++-- 6 files changed, 234 insertions(+), 18 deletions(-) create mode 100644 lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/TestLanceExceptionMapper.java diff --git a/lance/lance-rest-server/build.gradle.kts b/lance/lance-rest-server/build.gradle.kts index 176c4f16b7a..db4408e7850 100644 --- a/lance/lance-rest-server/build.gradle.kts +++ b/lance/lance-rest-server/build.gradle.kts @@ -193,6 +193,7 @@ tasks { val primaryBundleDir = lanceSparkBundleDirFor(primaryLanceSparkBundleVersion) doFirst { + systemProperty("lance.test.runtimeClasspath", sourceSets["main"].runtimeClasspath.asPath) val bundleJar = primaryBundleDir.get().asFile.listFiles()?.singleOrNull { it.extension == "jar" } ?: throw GradleException( diff --git a/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/LanceExceptionMapper.java b/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/LanceExceptionMapper.java index 2078b75fe6f..45d4cb4f47f 100644 --- a/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/LanceExceptionMapper.java +++ b/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/LanceExceptionMapper.java @@ -23,8 +23,10 @@ import javax.ws.rs.core.Response; import javax.ws.rs.ext.ExceptionMapper; import javax.ws.rs.ext.Provider; +import org.apache.gravitino.exceptions.ForbiddenException; import org.apache.gravitino.exceptions.NoSuchTableException; import org.apache.gravitino.exceptions.NotFoundException; +import org.apache.gravitino.exceptions.UnauthorizedException; import org.lance.namespace.errors.ConcurrentModificationException; import org.lance.namespace.errors.InternalException; import org.lance.namespace.errors.InvalidInputException; @@ -66,7 +68,13 @@ public Response toResponse(Exception ex) { } private static LanceNamespaceException toLanceNamespaceException(String instance, Exception ex) { - if (ex instanceof NoSuchTableException) { + if (ex instanceof ForbiddenException) { + return new PermissionDeniedException(ex.getMessage(), "", instance); + + } else if (ex instanceof UnauthorizedException) { + return new UnauthenticatedException(ex.getMessage(), "", instance); + + } else if (ex instanceof NoSuchTableException) { return new TableNotFoundException(ex.getMessage(), getStackTrace(ex), instance); } else if (ex instanceof NotFoundException) { @@ -84,7 +92,7 @@ private static LanceNamespaceException toLanceNamespaceException(String instance } else { LOG.warn("Lance REST server unexpected exception:", ex); - return new InternalException(ex.getMessage(), getStackTrace(ex), instance); + return new InternalException("Internal server error", "", instance); } } 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 88b3d645ecd..3ec6e9c9037 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 @@ -18,8 +18,6 @@ */ package org.apache.gravitino.lance.service.authorization; -import static org.apache.commons.lang3.exception.ExceptionUtils.getStackTrace; - import java.lang.reflect.Method; import java.lang.reflect.Parameter; import java.util.HashMap; @@ -188,9 +186,7 @@ protected Object toErrorResponse(Method method, Object[] args, Throwable throwab String namespaceId = pathArgument(method.getParameters(), args, "id").orElse(""); Exception exception; if (throwable instanceof ForbiddenException) { - exception = - new PermissionDeniedException( - throwable.getMessage(), getStackTrace(throwable), namespaceId); + exception = new PermissionDeniedException(throwable.getMessage(), "", namespaceId); } else if (throwable instanceof Exception) { exception = (Exception) throwable; } else { 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 a3e6b42ad89..ae4bffe98ad 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 @@ -18,16 +18,21 @@ */ package org.apache.gravitino.lance.integration.test; +import java.io.Writer; import java.net.URI; import java.net.http.HttpClient; import java.net.http.HttpRequest; import java.net.http.HttpResponse; import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.ArrayList; import java.util.Base64; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Properties; +import java.util.concurrent.TimeUnit; import org.apache.gravitino.Configs; import org.apache.gravitino.auth.AuthConstants; import org.apache.gravitino.authorization.Privileges; @@ -35,11 +40,16 @@ import org.apache.gravitino.authorization.SecurableObjects; import org.apache.gravitino.client.GravitinoMetalake; import org.apache.gravitino.integration.test.util.BaseIT; +import org.apache.gravitino.integration.test.util.HttpUtils; +import org.apache.gravitino.lance.server.GravitinoLanceRESTServer; +import org.apache.gravitino.rest.RESTUtils; import org.apache.gravitino.server.web.ObjectMapperProvider; +import org.awaitility.Awaitility; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; import org.lance.namespace.model.CreateNamespaceRequest; import org.lance.namespace.model.DescribeNamespaceResponse; import org.lance.namespace.model.DropNamespaceRequest; @@ -283,6 +293,121 @@ public void testServiceIdentityFallbackIsAuthorized() throws Exception { assertStatus(403, create(USER, catalog, null, Map.of())); } + /** Verifies standalone HTTP backend calls use service credentials rather than caller roles. */ + @Test + public void testStandaloneUsesBackendServiceIdentity(@TempDir Path directory) throws Exception { + int port = RESTUtils.findAvailablePort(10000, 11000); + String catalog = "lance_authz_standalone_catalog"; + String serviceUser = "lance_authz_standalone_user"; + GravitinoMetalake metalake = client.loadMetalake(getLanceRESTServerMetalakeName()); + metalake.addUser(serviceUser); + metalake.createRole( + "lance_authz_standalone_role", + new HashMap<>(), + List.of( + SecurableObjects.ofMetalake( + metalake.name(), + new ArrayList<>( + List.of(Privileges.UseCatalog.allow(), Privileges.CreateCatalog.allow()))))); + metalake.grantRolesToUser(List.of("lance_authz_standalone_role"), serviceUser); + Properties config = new Properties(); + config.setProperty(Configs.AUTHENTICATORS.getKey(), "simple"); + config.setProperty("gravitino.lance-rest.httpPort", String.valueOf(port)); + config.setProperty( + "gravitino.lance-rest.gravitino-uri", "http://localhost:" + getGravitinoServerPort()); + config.setProperty("gravitino.lance-rest.gravitino-metalake", getLanceRESTServerMetalakeName()); + config.setProperty("gravitino.lance-rest.gravitino-auth-type", "simple"); + config.setProperty("gravitino.lance-rest.gravitino-simple.user-name", serviceUser); + Path configFile = directory.resolve("standalone.conf"); + try (Writer writer = Files.newBufferedWriter(configFile)) { + config.store(writer, "Standalone Lance REST integration test"); + } + Path logFile = directory.resolve("standalone.log"); + // Use the production bootstrap in its own JVM: deploy mode has no local GravitinoEnv, + // while embedded mode must not share its backend environment with the standalone service. + ProcessBuilder builder = + new ProcessBuilder( + Path.of(System.getProperty("java.home"), "bin", "java").toString(), + "--add-opens=java.base/java.nio=ALL-UNNAMED", + "-cp", + System.getProperty("lance.test.runtimeClasspath"), + GravitinoLanceRESTServer.class.getName(), + configFile.toString()) + .redirectErrorStream(true) + .redirectOutput(logFile.toFile()); + builder.environment().put("GRAVITINO_TEST", "true"); + Process standalone = builder.start(); + try { + try { + Awaitility.await() + .atMost(60, TimeUnit.SECONDS) + .until( + () -> { + Assertions.assertTrue(standalone.isAlive(), "Standalone process exited"); + // Namespace initialization is lazy and occurs on the first metadata request. + return HttpUtils.isHttpServerUp( + "http://localhost:" + port + "/lance/health/live"); + }); + } catch (Exception | AssertionError e) { + throw new AssertionError("Standalone startup failed:\n" + Files.readString(logFile), e); + } + CreateNamespaceRequest body = new CreateNamespaceRequest(); + body.addIdItem(catalog); + HttpRequest request = + request(USER, "/v1/namespace/" + catalog + "/create") + .uri( + URI.create( + "http://localhost:" + + port + + "/lance/v1/namespace/" + + catalog + + "/create?delimiter=.")) + .setHeader(AuthConstants.X_GRAVITINO_ACTIVE_ROLES_HEADER, "NONE") + .POST( + HttpRequest.BodyPublishers.ofString( + ObjectMapperProvider.objectMapper().writeValueAsString(body))) + .build(); + // USER cannot create catalogs in auxiliary mode. The backend receives the service user's + // credentials and roles, despite USER selecting NONE on this incoming request. + assertStatus(200, httpClient.send(request, HttpResponse.BodyHandlers.ofString())); + Assertions.assertEquals( + serviceUser, + client + .loadMetalake(getLanceRESTServerMetalakeName()) + .loadCatalog(catalog) + .auditInfo() + .creator()); + // The backend service user cannot read this admin-owned schema. Even an incoming admin + // must receive the backend's 403, rather than 500 or the incoming caller's privileges. + HttpRequest deniedRequest = + request(ADMIN, "/v1/namespace/" + id(VISIBLE_CATALOG, VISIBLE_SCHEMA) + "/describe") + .uri( + URI.create( + "http://localhost:" + + port + + "/lance/v1/namespace/" + + id(VISIBLE_CATALOG, VISIBLE_SCHEMA) + + "/describe?delimiter=.")) + .POST(HttpRequest.BodyPublishers.ofString("{}")) + .build(); + HttpResponse deniedResponse = + httpClient.send(deniedRequest, HttpResponse.BodyHandlers.ofString()); + assertStatus(403, deniedResponse); + ErrorResponse error = + ObjectMapperProvider.objectMapper().readValue(deniedResponse.body(), ErrorResponse.class); + Assertions.assertEquals("", error.getDetail()); + Assertions.assertTrue(error.getError().contains(serviceUser), error.getError()); + assertStatus(200, drop(serviceUser, catalog, null, "cascade")); + } finally { + standalone.destroy(); + if (!standalone.waitFor(10, TimeUnit.SECONDS)) { + standalone.destroyForcibly(); + Assertions.assertTrue( + standalone.waitFor(10, TimeUnit.SECONDS), "Standalone process did not stop"); + } + } + } + /** Verifies health endpoints remain reachable even when credentials would be rejected. */ @Test public void testHealthBypassesAuthentication() throws Exception { diff --git a/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/TestLanceExceptionMapper.java b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/TestLanceExceptionMapper.java new file mode 100644 index 00000000000..aea2c57943a --- /dev/null +++ b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/TestLanceExceptionMapper.java @@ -0,0 +1,79 @@ +/* + * 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; + +import javax.ws.rs.core.Response; +import org.apache.gravitino.exceptions.ForbiddenException; +import org.apache.gravitino.exceptions.UnauthorizedException; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.lance.namespace.errors.InvalidInputException; +import org.lance.namespace.model.ErrorResponse; + +/** Verifies backend authentication failures retain their protocol status without stack traces. */ +public class TestLanceExceptionMapper { + + /** Verifies backend authorization failures use the Lance forbidden response. */ + @Test + public void testBackendForbidden() { + assertAuthenticationError(new ForbiddenException("Access denied"), 403); + } + + /** Verifies backend authentication failures use the Lance unauthenticated response. */ + @Test + public void testBackendUnauthorized() { + assertAuthenticationError(new UnauthorizedException("Invalid credentials"), 401); + } + + /** Verifies unexpected exceptions do not expose internal details in the response. */ + @Test + public void testInternalFailureDoesNotExposeException() { + try (Response response = + LanceExceptionMapper.toRESTResponse( + "catalog.schema.table", new RuntimeException("private-backend-detail"))) { + Assertions.assertEquals(500, response.getStatus()); + ErrorResponse error = (ErrorResponse) response.getEntity(); + Assertions.assertEquals("Internal server error", error.getError()); + Assertions.assertEquals("", error.getDetail()); + } + } + + /** Verifies intentional protocol validation details remain available to callers. */ + @Test + public void testProtocolValidationDetailsArePreserved() { + try (Response response = + LanceExceptionMapper.toRESTResponse( + "table", + new InvalidInputException("Invalid field", "field must be positive", "table"))) { + Assertions.assertEquals(400, response.getStatus()); + Assertions.assertEquals( + "field must be positive", ((ErrorResponse) response.getEntity()).getDetail()); + } + } + + private void assertAuthenticationError(Exception exception, int status) { + try (Response response = LanceExceptionMapper.toRESTResponse("catalog", exception)) { + Assertions.assertEquals(status, response.getStatus()); + ErrorResponse error = (ErrorResponse) response.getEntity(); + Assertions.assertEquals(exception.getMessage(), error.getError()); + Assertions.assertEquals("", error.getDetail()); + Assertions.assertEquals("catalog", error.getInstance()); + } + } +} diff --git a/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/rest/TestLanceNamespaceOperations.java b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/rest/TestLanceNamespaceOperations.java index a80c70a06b5..87069562bd1 100644 --- a/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/rest/TestLanceNamespaceOperations.java +++ b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/rest/TestLanceNamespaceOperations.java @@ -208,10 +208,9 @@ public void testListNamespaces() { ErrorResponse errorResp = resp.readEntity(ErrorResponse.class); Assertions.assertEquals(18, errorResp.getCode()); - Assertions.assertEquals("Test exception", errorResp.getError()); + Assertions.assertEquals("Internal server error", errorResp.getError()); + Assertions.assertEquals("", errorResp.getDetail()); Assertions.assertEquals("ns1.ns2", errorResp.getInstance()); - Assertions.assertNotNull(errorResp.getDetail()); - Assertions.assertTrue(errorResp.getDetail().contains("Test exception")); // root endpoint should use explicit root identifier instead of delimiter in error instance resp = @@ -262,7 +261,8 @@ public void testDescribeNamespace() { ErrorResponse errorResp = resp.readEntity(ErrorResponse.class); Assertions.assertEquals(18, errorResp.getCode()); - Assertions.assertEquals("Test exception", errorResp.getError()); + Assertions.assertEquals("Internal server error", errorResp.getError()); + Assertions.assertEquals("", errorResp.getDetail()); } @Test @@ -321,7 +321,8 @@ public void testCreateNamespace() { ErrorResponse errorResp = resp.readEntity(ErrorResponse.class); Assertions.assertEquals(18, errorResp.getCode()); - Assertions.assertEquals("Test exception", errorResp.getError()); + Assertions.assertEquals("Internal server error", errorResp.getError()); + Assertions.assertEquals("", errorResp.getDetail()); } @Test @@ -393,7 +394,8 @@ public void testDropNamespace() { ErrorResponse errorResp = resp.readEntity(ErrorResponse.class); Assertions.assertEquals(18, errorResp.getCode()); - Assertions.assertEquals("Test exception", errorResp.getError()); + Assertions.assertEquals("Internal server error", errorResp.getError()); + Assertions.assertEquals("", errorResp.getDetail()); } @Test @@ -457,7 +459,8 @@ void testCreateTable() { Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), resp.getStatus()); Assertions.assertEquals(MediaType.APPLICATION_JSON_TYPE, resp.getMediaType()); ErrorResponse errorResp = resp.readEntity(ErrorResponse.class); - Assertions.assertEquals("Runtime exception", errorResp.getError()); + Assertions.assertEquals("Internal server error", errorResp.getError()); + Assertions.assertEquals("", errorResp.getDetail()); } @Test @@ -513,7 +516,8 @@ void testRegisterTable() { Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), resp.getStatus()); Assertions.assertEquals(MediaType.APPLICATION_JSON_TYPE, resp.getMediaType()); ErrorResponse errorResp = resp.readEntity(ErrorResponse.class); - Assertions.assertEquals("Runtime exception", errorResp.getError()); + Assertions.assertEquals("Internal server error", errorResp.getError()); + Assertions.assertEquals("", errorResp.getDetail()); } @Test @@ -620,7 +624,8 @@ void testDeregisterTable() { Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), resp.getStatus()); Assertions.assertEquals(MediaType.APPLICATION_JSON_TYPE, resp.getMediaType()); ErrorResponse errorResp = resp.readEntity(ErrorResponse.class); - Assertions.assertEquals("Runtime exception", errorResp.getError()); + Assertions.assertEquals("Internal server error", errorResp.getError()); + Assertions.assertEquals("", errorResp.getDetail()); } @Test @@ -677,7 +682,8 @@ void testDescribeTable() { Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), resp.getStatus()); Assertions.assertEquals(MediaType.APPLICATION_JSON_TYPE, resp.getMediaType()); ErrorResponse errorResp = resp.readEntity(ErrorResponse.class); - Assertions.assertEquals("Runtime exception", errorResp.getError()); + Assertions.assertEquals("Internal server error", errorResp.getError()); + Assertions.assertEquals("", errorResp.getDetail()); } @Test @@ -999,6 +1005,7 @@ void testDeclareTable() { Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), resp.getStatus()); Assertions.assertEquals(MediaType.APPLICATION_JSON_TYPE, resp.getMediaType()); ErrorResponse errorResp = resp.readEntity(ErrorResponse.class); - Assertions.assertEquals("Runtime exception", errorResp.getError()); + Assertions.assertEquals("Internal server error", errorResp.getError()); + Assertions.assertEquals("", errorResp.getDetail()); } } From 286fd982272a320551e192b080bb54cb0cad271f Mon Sep 17 00:00:00 2001 From: yuqi Date: Fri, 11 Sep 2026 14:25:16 +0800 Subject: [PATCH 7/9] docs(lance): clarify optional auxiliary fallback identity --- docs/lance-rest-service.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/lance-rest-service.md b/docs/lance-rest-service.md index 8af59f2e8c1..82a53a15693 100644 --- a/docs/lance-rest-service.md +++ b/docs/lance-rest-service.md @@ -303,6 +303,8 @@ The auxiliary fallback applies only after authentication accepts an anonymous re not recover a rejected authentication attempt. Authenticated callers keep their own privileges, active roles, ownership and audit identity; they do not inherit the service user's privileges. The fallback service user itself needs the privileges required by the requested operation. +Setting `gravitino.lance-rest.gravitino-simple.user-name` explicitly is optional in auxiliary mode; +configure it only to override the default anonymous fallback identity, `lance-rest-server`. With `simple` authentication, a Basic header supplies a user name without validating a password, and a request without credentials is accepted as anonymous. Some malformed Basic credentials @@ -326,7 +328,6 @@ gravitino.authorization.enable = true gravitino.authorization.serviceAdmins = adminUser # Development example: simple accepts the supplied user name without password validation. gravitino.authenticators = simple -gravitino.lance-rest.gravitino-simple.user-name = lance-rest-server ``` Create the metalake, add users, and grant roles through the Gravitino API as described in From ff61fec1ae26eb5184a0eb576db8a0145c1855c1 Mon Sep 17 00:00:00 2001 From: yuqi Date: Fri, 11 Sep 2026 15:21:17 +0800 Subject: [PATCH 8/9] [#12574] docs(lance): document authorization differences between modes Auxiliary mode authorizes each Lance endpoint once, in Lance REST, against an expression written for that endpoint. Standalone mode has no Lance-side authorization at all: every underlying Gravitino call is checked by the Gravitino server with the rules of that call. The same request can therefore be authorized differently depending on the mode. Record the known differences so users find them in the documentation rather than in an unexpected 403. The clearest one needs no race: TableExists on an existing table succeeds with CREATE_TABLE in auxiliary mode, but standalone maps it to loadTable, whose existence allowance only applies when the table is absent, so the same caller gets 403. Closing the gap is tracked in #13089. Claude-Session: https://claude.ai/code/session_011A4FqHJarzs2xvs7WbusMT --- docs/lance-rest-service.md | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/docs/lance-rest-service.md b/docs/lance-rest-service.md index 82a53a15693..fa96b32eb4e 100644 --- a/docs/lance-rest-service.md +++ b/docs/lance-rest-service.md @@ -391,6 +391,44 @@ catalog defaults and table overrides. These are shared configured credentials, n credentials restricted to the caller's table privileges. Access to data files depends on the permissions of those credentials. Per-user, scoped credential vending is not implemented. +### Authorization differences between deployment modes + +The two modes enforce authorization at different places, so the same request can be authorized +differently depending on how Lance REST is deployed. + +In auxiliary mode, each Lance REST endpoint is checked once, before any metadata call, against an +expression written for that endpoint's Lance semantics (the tables above). In standalone mode, +Lance REST performs no authorization of its own: every underlying Gravitino REST call that an +endpoint makes is authorized by the Gravitino server, using the rules of that underlying call and +the backend identity described above. A Lance endpoint that maps to several Gravitino calls is +therefore checked several times, and a Lance endpoint whose semantics differ from the Gravitino +call it maps to follows the Gravitino rule. + +| Aspect | Auxiliary | Standalone | +| ------------------------- | -------------------------------------------------------- | -------------------------------------------------------------------------- | +| Where authorization runs | Lance REST, once per endpoint, before any metadata call | Gravitino server, once per underlying REST call | +| Rules applied | Lance endpoint expressions listed in Required privileges | Gravitino rules for `loadTable`, `createSchema`, `alterCatalog`, and so on | +| Checks per Lance endpoint | One | One per underlying call, including existence checks made before a mutation | +| Listing filters | Lance REST filters before pagination | Gravitino's own listing filters | + +Known differences today: + +- **`TableExists` on an existing table.** Auxiliary mode authorizes the probe with `PROBE_TABLE_LIKE` + or `CREATE_TABLE`. Standalone mode implements the probe as a Gravitino `loadTable`, whose + existence-check allowance only applies when the table is absent. A caller holding `CREATE_TABLE` + but not `SELECT_TABLE` therefore receives `200` in auxiliary mode and `403` in standalone mode + for the same existing table. +- **Mutations preceded by a read.** `CreateNamespace` loads the target before creating it, so in + standalone mode the backend identity needs read access to the parent in addition to the create + privilege. Auxiliary mode evaluates a single create expression and does not require the read. +- **Overwrite and drop.** Auxiliary mode requires ownership as listed above. Standalone mode + requires whatever the underlying Gravitino `alter` or `drop` call requires, which may differ. + +These differences are a property of the current architecture rather than a configuration choice. +Closing them is tracked in [#13089](https://github.com/apache/gravitino/issues/13089). +Until then, deployments that need identical authorization decisions on both paths should run +Lance REST in auxiliary mode. + ## Examples The following examples demonstrate how to interact with Lance REST service using different programming languages and tools. From 572e3e7f3202358ff0b48f36e672d845cf286ac7 Mon Sep 17 00:00:00 2001 From: yuqi Date: Fri, 11 Sep 2026 16:03:47 +0800 Subject: [PATCH 9/9] docs(lance): recommend auxiliary mode for authorization --- docs/lance-rest-service.md | 51 ++++++++++++-------------------------- 1 file changed, 16 insertions(+), 35 deletions(-) diff --git a/docs/lance-rest-service.md b/docs/lance-rest-service.md index fa96b32eb4e..14b9c61a019 100644 --- a/docs/lance-rest-service.md +++ b/docs/lance-rest-service.md @@ -393,41 +393,22 @@ permissions of those credentials. Per-user, scoped credential vending is not imp ### Authorization differences between deployment modes -The two modes enforce authorization at different places, so the same request can be authorized -differently depending on how Lance REST is deployed. - -In auxiliary mode, each Lance REST endpoint is checked once, before any metadata call, against an -expression written for that endpoint's Lance semantics (the tables above). In standalone mode, -Lance REST performs no authorization of its own: every underlying Gravitino REST call that an -endpoint makes is authorized by the Gravitino server, using the rules of that underlying call and -the backend identity described above. A Lance endpoint that maps to several Gravitino calls is -therefore checked several times, and a Lance endpoint whose semantics differ from the Gravitino -call it maps to follows the Gravitino rule. - -| Aspect | Auxiliary | Standalone | -| ------------------------- | -------------------------------------------------------- | -------------------------------------------------------------------------- | -| Where authorization runs | Lance REST, once per endpoint, before any metadata call | Gravitino server, once per underlying REST call | -| Rules applied | Lance endpoint expressions listed in Required privileges | Gravitino rules for `loadTable`, `createSchema`, `alterCatalog`, and so on | -| Checks per Lance endpoint | One | One per underlying call, including existence checks made before a mutation | -| Listing filters | Lance REST filters before pagination | Gravitino's own listing filters | - -Known differences today: - -- **`TableExists` on an existing table.** Auxiliary mode authorizes the probe with `PROBE_TABLE_LIKE` - or `CREATE_TABLE`. Standalone mode implements the probe as a Gravitino `loadTable`, whose - existence-check allowance only applies when the table is absent. A caller holding `CREATE_TABLE` - but not `SELECT_TABLE` therefore receives `200` in auxiliary mode and `403` in standalone mode - for the same existing table. -- **Mutations preceded by a read.** `CreateNamespace` loads the target before creating it, so in - standalone mode the backend identity needs read access to the parent in addition to the create - privilege. Auxiliary mode evaluates a single create expression and does not require the read. -- **Overwrite and drop.** Auxiliary mode requires ownership as listed above. Standalone mode - requires whatever the underlying Gravitino `alter` or `drop` call requires, which may differ. - -These differences are a property of the current architecture rather than a configuration choice. -Closing them is tracked in [#13089](https://github.com/apache/gravitino/issues/13089). -Until then, deployments that need identical authorization decisions on both paths should run -Lance REST in auxiliary mode. +:::warning +Lance REST metadata authorization is currently supported only in **auxiliary mode**. +**Standalone mode is not recommended**: its authorization decisions can differ from auxiliary +mode and may produce unexpected results, even for the same user and privileges. Use auxiliary +mode for deployments that require Lance REST authorization. +::: + +Auxiliary mode applies the Lance endpoint authorization rules and listing filters described above. +Standalone mode does not apply this authorization pipeline. Instead, the remote Gravitino server +checks each underlying REST call using its own rules and the backend identity described above. +These checks do not provide equivalent Lance REST authorization: an operation allowed in one mode +may be denied in the other, and ownership requirements and metadata visibility can also differ. +Forwarding the caller's identity alone does not eliminate these differences. + +Alignment of the authorization behavior is tracked in +[#13089](https://github.com/apache/gravitino/issues/13089). ## Examples