markhoerth opened a new issue, #14436:
URL: https://github.com/apache/gravitino/issues/14436
### Version
main branch, commit bb634defdc5663a8124be5fca1c8d9e52ac096ac (2.0.0-SNAPSHOT)
### Describe what's wrong
A user who holds only USE_CATALOG, USE_SCHEMA and CREATE_TABLE on a schema
sends a Lance REST CreateTable with `mode=exist_ok` for a table that another
user already owns. The expected result is that the existing table is returned
unchanged and its owner stays the same. Instead the request returns 200 and the
caller becomes the owner of the existing table. With ownership the caller can
then describe the table (which returned 403 before) and drop it, and the drop
purges the table's Lance dataset. It happens whenever authorization is enabled
and the Lance REST service runs inside the Gravitino server with the
`gravitino` namespace backend. Namespace CreateNamespace with `mode=exist_ok`
does not have the problem: the schema owner is unchanged.
Cause (defect in Gravitino's own code):
-
`lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/rest/LanceTableOperations.java:127`
authorizes CreateTable with `CREATE_TABLE_AUTHORIZATION_EXPRESSION`, which by
design covers both `create` and `exist_ok`
(`lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/authorization/LanceAuthorizationExpressions.java:111-124`).
So a create-only caller is allowed to reach an existing table.
-
`lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/GravitinoLanceTableOperations.java:197-202`
handles every mode with one `createTable` call on
`namespaceWrapper.asTableCatalog(catalog)`. In aux mode that adapter calls
`GravitinoEnv.getInstance().tableDispatcher()`
(`lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/GravitinoLanceNamespaceWrapper.java:287-294`,
`:317-323`, `:396-406`), which is the hooked dispatcher chain.
- For `EXIST_OK`,
`catalogs/catalog-lakehouse-generic/src/main/java/org/apache/gravitino/catalog/lakehouse/lance/LanceTableOperations.java:221-228`
loads the existing table and returns it as if it had been created.
-
`core/src/main/java/org/apache/gravitino/hook/TableHookDispatcher.java:87-100`
then sets the current user as the owner after every successful `createTable`,
without knowing that no table was created.
The fix needs the exist_ok path to skip the owner hook when the table
already existed (for example by returning the existing table before the hooked
`createTable`, or by telling the hook whether a table was created). #12955
(authorize the metadata returned by exist_ok) would not cover this on its own,
because the owner is already changed inside the dispatcher before the REST
layer returns.
### Error message and/or stacktrace
Before the exist_ok request, the second user cannot read or drop the table:
```
POST /lance/v1/table/lc$s1$t1/describe -> 403
{"error":"User 'eo_user' is not authorized to perform operation
'describeTable' on metadata 'lance_ml.lc.s1.t1' with expression 'entityType ==
'TABLE' && (ANY(OWNER, METALAKE, CATALOG) || ... && (TABLE::OWNER ||
ANY_SELECT_TABLE || ANY_MODIFY_TABLE)) ...","code":15,...}
POST /lance/v1/table/lc$s1$t1/drop -> 403
```
The exist_ok create succeeds and changes the owner:
```
POST /lance/v1/table/lc$s1$t1/create?mode=exist_ok -> 200
{"location":"file:///tmp/lc/s1/t1/","version":1,"storage_options":{},"properties":{"format":"lance","lance.version":"1","external":"true","location":"file:///tmp/lc/s1/t1/","lance.creation-mode":"CREATE"}}
GET /api/metalakes/lance_ml/owners/table/lc.s1.t1
before: {"code":0,"owner":{"name":"admin","type":"user"}}
after: {"code":0,"owner":{"name":"eo_user","type":"user"}}
```
Afterwards the second user's describe returns 200, drop returns 200, and the
dataset directory `/tmp/lc/s1/t1/` is deleted.
### How to reproduce
Server with `gravitino.authorization.enable = true`,
`gravitino.authorization.serviceAdmins = admin`, simple authentication,
`gravitino.auxService.names = iceberg-rest,lance-rest`,
`gravitino.lance-rest.namespace-backend = gravitino` and
`gravitino.lance-rest.gravitino-metalake = lance_ml`.
```
A="Authorization: Basic $(printf 'admin:x' | base64)"
U="Authorization: Basic $(printf 'eo_user:x' | base64)"
G=http://127.0.0.1:8090/api; L=http://127.0.0.1:9101/lance; J='Content-Type:
application/json'
```
1. Create the metalake, a lakehouse-generic catalog, a namespace and a table
as admin (`schema.arrow` is a schema-only Arrow IPC stream, for example `id
int64 not null, name utf8`):
```
curl -s -H "$A" -H "$J" -X POST $G/metalakes -d
'{"name":"lance_ml","comment":"","properties":{}}'
curl -s -H "$A" -H "$J" -X POST $G/metalakes/lance_ml/catalogs -d
'{"name":"lc","type":"RELATIONAL","provider":"lakehouse-generic","comment":"","properties":{"location":"file:///tmp/lc"}}'
curl -s -H "$A" -H "$J" -X POST "$L/v1/namespace/lc\$s1/create" -d
'{"mode":"create","properties":{}}'
curl -s -H "$A" -H 'Content-Type: application/vnd.apache.arrow.stream'
--data-binary @schema.arrow -X POST "$L/v1/table/lc\$s1\$t1/create"
curl -s -H "$A" $G/metalakes/lance_ml/owners/table/lc.s1.t1
```
2. Create a user with only USE_CATALOG, USE_SCHEMA and CREATE_TABLE:
```
curl -s -H "$A" -H "$J" -X POST $G/metalakes/lance_ml/users -d
'{"name":"eo_user"}'
curl -s -H "$A" -H "$J" -X POST $G/metalakes/lance_ml/roles -d
'{"name":"eo_role","properties":{},"securableObjects":[{"fullName":"lc","type":"CATALOG","privileges":[{"name":"USE_CATALOG","condition":"ALLOW"}]},{"fullName":"lc.s1","type":"SCHEMA","privileges":[{"name":"USE_SCHEMA","condition":"ALLOW"},{"name":"CREATE_TABLE","condition":"ALLOW"}]}]}'
curl -s -H "$A" -H "$J" -X PUT
$G/metalakes/lance_ml/permissions/users/eo_user/grant -d
'{"roleNames":["eo_role"]}'
```
3. Check that the user cannot describe or drop the table (both 403):
```
curl -s -w '\n%{http_code}\n' -H "$U" -H "$J" -X POST
"$L/v1/table/lc\$s1\$t1/describe" -d '{}'
curl -s -w '\n%{http_code}\n' -H "$U" -H "$J" -X POST
"$L/v1/table/lc\$s1\$t1/drop" -d '{}'
```
4. Send the exist_ok create as the user, then read the owner:
```
curl -s -w '\n%{http_code}\n' -H "$U" -H 'Content-Type:
application/vnd.apache.arrow.stream' --data-binary @schema.arrow -X POST
"$L/v1/table/lc\$s1\$t1/create?mode=exist_ok"
curl -s -H "$A" $G/metalakes/lance_ml/owners/table/lc.s1.t1
```
The owner is now `eo_user`.
5. Drop the table as the user: 200, and `/tmp/lc/s1/t1/` is gone.
```
curl -s -w '\n%{http_code}\n' -H "$U" -H "$J" -X POST
"$L/v1/table/lc\$s1\$t1/drop" -d '{}'
ls /tmp/lc/s1/t1/
```
### Additional context
- Related: #12955 (exist_ok returns the metadata of an existing table to a
create-only caller), #12956 (ownership during Lance overwrite), #12561 (Lance
REST table creation authorization and ownership).
- Found by the 2.0 OSS test suite check
`test_exist_ok_create_does_not_change_owner`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
