markhoerth opened a new issue, #14443:
URL: https://github.com/apache/gravitino/issues/14443
### Version
main branch, commit bb634defdc5663a8124be5fca1c8d9e52ac096ac (2.0.0-SNAPSHOT)
### Describe what's wrong
With authorization enabled, a user creates a policy, tag, role or job
template whose name contains `.`, for example `a.b`. Gravitino reserves `.` as
the qualified-name separator, so the request should be rejected with 400 and
nothing should be stored.
The request is rejected with 400 `IllegalNameIdentifierException`, but the
object has already been written to the entity store. After that:
- Creating the same name again returns 409 `...AlreadyExistsException`.
- Loading or deleting the object returns the same 400, so it cannot be
removed through the API.
- Listing that object type for the whole metalake (`GET /policies`, `/tags`,
`/roles`, `/jobs/templates`) returns 400 for every user, so one refused request
breaks the list for everyone in the metalake.
The only way out is to edit the backend store.
Cause: the hook dispatchers store the object first and only then convert its
name to a metadata object to set the owner, and that conversion is where the
name check runs.
- `NameIdentifierUtil.toMetadataObject` calls `checkMetadataObjectName`,
which throws for any name containing `.`
(`core/src/main/java/org/apache/gravitino/utils/NameIdentifierUtil.java:628-637`,
call at line 652).
- Policy: `PolicyHookDispatcher.createPolicy` stores at
`core/src/main/java/org/apache/gravitino/hook/PolicyHookDispatcher.java:72`,
then calls `toMetadataObject` at line 79.
- Job template: `JobHookDispatcher.registerJobTemplate` stores at
`core/src/main/java/org/apache/gravitino/hook/JobHookDispatcher.java:57`, then
calls `toMetadataObject` at line 64.
- Tag: `TagHookDispatcher.createTag` stores at
`core/src/main/java/org/apache/gravitino/hook/TagHookDispatcher.java:76`, then
calls `toMetadataObject` at line 83.
- Role: `AccessControlHookDispatcher.createRole` stores at
`core/src/main/java/org/apache/gravitino/hook/AccessControlHookDispatcher.java:218`,
then calls `toMetadataObject` at line 225.
- The REST resources do not check the name before dispatching (for example
`server/src/main/java/org/apache/gravitino/server/web/rest/PolicyOperations.java:163-171`
and
`server/src/main/java/org/apache/gravitino/server/web/rest/JobOperations.java:180-183`).
- Afterwards, list fails in `MetadataAuthzHelper.filterByExpression`, which
checks every listed name and fails the whole listing on the first bad one
(`server-common/src/main/java/org/apache/gravitino/server/authorization/MetadataAuthzHelper.java:421-423`).
Load and delete fail in `AuthorizationExpressionEvaluator.evaluate`
(`server-common/src/main/java/org/apache/gravitino/server/authorization/expression/AuthorizationExpressionEvaluator.java:187`).
The defect is in Gravitino's own code. A fix would call
`NameIdentifierUtil.checkMetadataObjectName` (or an equivalent check) before
anything is stored, in the REST request validation or at the start of each hook
dispatcher method, so the request is rejected with 400 and the name stays free.
### Error message and/or stacktrace
Create (rejected, but stored):
```
POST /api/metalakes/gvtrepro_dot1/policies
{"name":"a.b","comment":"","policyType":"system_iceberg_compaction","enabled":true,"content":{}}
HTTP 400
{"code":1001,"type":"IllegalNameIdentifierException","message":"Failed to
operate policy(s) [a.b] operation [CREATE] under object [gvtrepro_dot1], reason
[The POLICY name 'a.b' is unsupported because '.' is reserved as the
qualified-name
separator.]","stack":["org.apache.gravitino.exceptions.IllegalNameIdentifierException:
The POLICY name 'a.b' is unsupported because '.' is reserved as the
qualified-name separator.","\tat
org.apache.gravitino.utils.NameIdentifierUtil.checkMetadataObjectName(NameIdentifierUtil.java:636)","\tat
org.apache.gravitino.utils.NameIdentifierUtil.toMetadataObject(NameIdentifierUtil.java:652)","\tat
org.apache.gravitino.hook.PolicyHookDispatcher.createPolicy(PolicyHookDispatcher.java:79)",
...]}
```
Same create again:
```
HTTP 409
{"code":1004,"type":"PolicyAlreadyExistsException","message":"Failed to
operate policy(s) [a.b] operation [CREATE] under object [gvtrepro_dot1], reason
[Policy with name a.b under metalake gvtrepro_dot1 already exists]",...}
```
List:
```
GET /api/metalakes/gvtrepro_dot1/policies
HTTP 400
{"code":1001,"type":"IllegalNameIdentifierException","message":"Failed to
operate policy(s) operation [LIST] under object [gvtrepro_dot1], reason [The
POLICY name 'a.b' is unsupported because '.' is reserved as the qualified-name
separator.]","stack":["org.apache.gravitino.exceptions.IllegalNameIdentifierException:
...","\tat
org.apache.gravitino.utils.NameIdentifierUtil.checkMetadataObjectName(NameIdentifierUtil.java:636)","\tat
org.apache.gravitino.server.authorization.MetadataAuthzHelper.lambda$filterByExpression$8(MetadataAuthzHelper.java:423)",
...]}
```
Job templates, tags and roles give the same sequence (400 on create, 409 on
the second create, 400 on list, load and delete), with
`JobHookDispatcher.registerJobTemplate`, `TagHookDispatcher.createTag` and
`AccessControlHookDispatcher.createRole` in the create stack.
### How to reproduce
Gravitino server with authorization enabled and simple auth (`admin` is a
service admin).
```bash
A="Authorization: Basic $(printf admin:x | base64)"
G=http://127.0.0.1:8090/api
H=(-H "$A" -H 'Content-Type: application/json' -H 'Accept:
application/vnd.gravitino.v1+json')
```
1. Create a metalake:
```bash
curl -s "${H[@]}" -X POST $G/metalakes -d
'{"name":"gvtrepro_dot1","comment":"","properties":{}}'
```
2. Create a policy named `a.b` (400), create it again (409), list policies
(400), delete it (400):
```bash
curl -s -w '%{http_code}\n' "${H[@]}" -X POST
$G/metalakes/gvtrepro_dot1/policies -d
'{"name":"a.b","comment":"","policyType":"system_iceberg_compaction","enabled":true,"content":{}}'
curl -s -w '%{http_code}\n' "${H[@]}" -X POST
$G/metalakes/gvtrepro_dot1/policies -d
'{"name":"a.b","comment":"","policyType":"system_iceberg_compaction","enabled":true,"content":{}}'
curl -s -w '%{http_code}\n' "${H[@]}" $G/metalakes/gvtrepro_dot1/policies
curl -s -w '%{http_code}\n' "${H[@]}" -X DELETE
$G/metalakes/gvtrepro_dot1/policies/a.b
```
3. Job template, tag and role behave the same way:
```bash
curl -s -w '%{http_code}\n' "${H[@]}" -X POST
$G/metalakes/gvtrepro_dot1/jobs/templates -d
'{"jobTemplate":{"name":"a.b","jobType":"shell","comment":"","executable":"/bin/true"}}'
curl -s -w '%{http_code}\n' "${H[@]}"
$G/metalakes/gvtrepro_dot1/jobs/templates
curl -s -w '%{http_code}\n' "${H[@]}" -X POST
$G/metalakes/gvtrepro_dot1/tags -d '{"name":"t.x","comment":"","properties":{}}'
curl -s -w '%{http_code}\n' "${H[@]}" $G/metalakes/gvtrepro_dot1/tags
curl -s -w '%{http_code}\n' "${H[@]}" -X POST
$G/metalakes/gvtrepro_dot1/roles -d
'{"name":"r.x","properties":{},"securableObjects":[]}'
curl -s -w '%{http_code}\n' "${H[@]}" $G/metalakes/gvtrepro_dot1/roles
```
### Additional context
- Related: #14431 covers the rename path for policies (a rename to a dotted
name returns 200 and stores it). Here the create path rejects the name but
stores it anyway, and the same ordering affects tags, roles and job templates.
#12977 introduced the explicit dotted-name error.
- Also present in v1.3.1: the same store-then-check order in
`PolicyHookDispatcher.createPolicy`, `JobHookDispatcher.registerJobTemplate`,
`TagHookDispatcher.createTag` and `AccessControlHookDispatcher.createRole`,
with `checkMetadataObjectName` in `toMetadataObject` and in the list filter of
`MetadataAuthzHelper`.
- Found by the 2.0 OSS test suite checks
`test_dotted_policy_name_refused_without_side_effects[create]` and
`test_dotted_job_template_name_refused_without_side_effects`.
--
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]
