markhoerth opened a new issue, #14431:
URL: https://github.com/apache/gravitino/issues/14431
### Version
main branch, commit bb634defdc5663a8124be5fca1c8d9e52ac096ac (2.0.0-SNAPSHOT)
### Describe what's wrong
A policy owner (or the metalake owner) renames an existing policy with `PUT
/api/metalakes/{metalake}/policies/{policy}` and a `rename` update whose
`newName` contains a `.`, for example `a.b`. Gravitino reserves `.` as the
qualified-name separator, so the rename should be rejected with a 400 and the
policy should keep its old name.
Instead the rename returns 200 and the new name is stored. From then on the
policy can no longer be used through the REST API:
- `GET /policies/a.b` returns 400 `IllegalNameIdentifierException`.
- Renaming it back (`PUT /policies/a.b`) returns the same 400.
- `DELETE /policies/a.b` returns the same 400, so the policy cannot be
removed.
- `GET /policies` and `GET /policies?details=true` for the whole metalake
return 400, so no caller can list any policy in that metalake.
The problem shows up whenever authorization is enabled, because the failing
check runs inside the authorization layer. The only way out is to edit the
backend store.
Cause:
- The rename path never checks the new name for `.`.
`RenamePolicyRequest.validate()` only checks that `newName` is not blank
(`common/src/main/java/org/apache/gravitino/dto/requests/PolicyUpdateRequest.java:87`).
`PolicyOperations.alterPolicy`
(`server/src/main/java/org/apache/gravitino/server/web/rest/PolicyOperations.java:215`,
call at line 239) passes the change through, and
`PolicyManager.updatePolicyEntity` takes the new name as-is
(`core/src/main/java/org/apache/gravitino/policy/PolicyManager.java:335`) and
stores it (line 374, via `entityStore.update` in `alterPolicy` at line 155).
- Every later request that names the policy converts it to a metadata
object. `NameIdentifierUtil.toMetadataObject` calls `checkMetadataObjectName`
(`core/src/main/java/org/apache/gravitino/utils/NameIdentifierUtil.java:652`
and `628-637`), which throws `IllegalNameIdentifierException` for any name with
`.`. For load, alter and delete it is reached from
`AuthorizationExpressionEvaluator.evaluate`
(`server-common/src/main/java/org/apache/gravitino/server/authorization/expression/AuthorizationExpressionEvaluator.java:187`).
For list it is reached from `MetadataAuthzHelper.filterByExpression`
(`server-common/src/main/java/org/apache/gravitino/server/authorization/MetadataAuthzHelper.java:421-423`),
which checks every listed name and fails the whole listing on the first bad
one.
The defect is in Gravitino's own code. A fix would validate the new name
with `NameIdentifierUtil.checkMetadataObjectName` (or an equivalent check) on
the rename path before anything is stored, so the request is rejected with 400
and the policy keeps its name.
### Error message and/or stacktrace
Rename (accepted):
```
PUT /api/metalakes/gvt_repro_polrename/policies/plain
{"updates":[{"@type":"rename","newName":"a.b"}]}
HTTP 200
{"code":0,"policy":{"name":"a.b","comment":"","policyType":"system_iceberg_compaction","enabled":true,...}}
```
List afterwards:
```
GET /api/metalakes/gvt_repro_polrename/policies
HTTP 400
{"code":1001,"type":"IllegalNameIdentifierException","message":"Failed to
operate policy(s) operation [LIST] under object [gvt_repro_polrename], 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.server.authorization.MetadataAuthzHelper.lambda$filterByExpression$8(MetadataAuthzHelper.java:423)",
...]}
```
Load, rename back and delete afterwards (all the same):
```
DELETE /api/metalakes/gvt_repro_polrename/policies/a.b
HTTP 400
{"code":1001,"type":"IllegalNameIdentifierException","message":"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.server.authorization.expression.AuthorizationExpressionEvaluator.lambda$evaluate$1(AuthorizationExpressionEvaluator.java:187)",
...]}
```
### How to reproduce
Server with authorization enabled and simple auth, `admin` in the service
admins.
```bash
G=http://127.0.0.1:8090/api
AUTH="Authorization: Basic $(echo -n admin:x | base64)"
HDR=(-H "$AUTH" -H 'Content-Type: application/json' -H 'Accept:
application/vnd.gravitino.v1+json')
```
1. Create a clean metalake:
`curl -s "${HDR[@]}" -X POST $G/metalakes -d
'{"name":"gvt_repro_polrename","comment":"","properties":{}}'`
2. Create a policy with a plain name:
`curl -s "${HDR[@]}" -X POST $G/metalakes/gvt_repro_polrename/policies -d
'{"name":"plain","comment":"","policyType":"system_iceberg_compaction","enabled":true,"content":{}}'`
3. Rename it to a dotted name. Expected 400; actual 200:
`curl -s "${HDR[@]}" -X PUT
$G/metalakes/gvt_repro_polrename/policies/plain -d
'{"updates":[{"@type":"rename","newName":"a.b"}]}'`
4. List the policies. Expected 200; actual 400:
`curl -s "${HDR[@]}" $G/metalakes/gvt_repro_polrename/policies`
5. Try to load, rename back or delete the policy. All return 400:
`curl -s "${HDR[@]}" $G/metalakes/gvt_repro_polrename/policies/a.b`
`curl -s "${HDR[@]}" -X PUT $G/metalakes/gvt_repro_polrename/policies/a.b
-d '{"updates":[{"@type":"rename","newName":"plain"}]}'`
`curl -s "${HDR[@]}" -X DELETE
$G/metalakes/gvt_repro_polrename/policies/a.b`
### Additional context
Related: #12977 (clear 400 for dotted names of external entities, closed)
and #10909 (dotted object names across list and load, closed). Both cover names
that come from external systems. Here the dotted name is stored by Gravitino's
own rename API for a Gravitino-managed policy.
Found by the Gravitino 2.0 OSS test suite check
`test_dotted_policy_name_refused_without_side_effects[rename]`.
--
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]
