bharos opened a new pull request, #12167: URL: https://github.com/apache/gravitino/pull/12167
### What changes were proposed in this pull request? Validate the `X-Gravitino-Active-Roles` header against the roles the caller actually holds. When a request declares named active roles the caller does **not** hold (directly or through a group), it is rejected with `403 Forbidden` before object-level authorization runs, instead of the previous behavior of silently ignoring the unheld names. - `GravitinoAuthorizer`: add `findUnheldRoles(principal, metalake, declaredRoleNames, requestContext)`, defaulting to an empty set so authorizers without role membership (e.g. `PassThroughAuthorizer` when authorization is disabled) never block role assumption. - `JcasbinAuthorizer`: implement `findUnheldRoles()` by resolving the caller's held role ids (direct + group-inherited) and returning the declared names that are not held; fails closed if membership cannot be resolved. - `GravitinoInterceptionService` (native REST) and `BaseMetadataAuthorizationMethodInterceptor` (Iceberg REST): after user validation, reject `NAMED` declarations that contain unheld roles (`403` via `Utils.forbidden` / `IcebergExceptionMapper`). `ALL`/`NONE` name nothing and are not validated. This is a validation of the *declaration* (like SQL `SET ROLE` / assume-role): a caller can only activate roles they were granted, independent of whether the role is needed for the target object. The object-level access decision is unchanged — it just evaluates over the narrowed role set from #11967. ### Why are the changes needed? [#12044](https://github.com/apache/gravitino/issues/11967) added narrowing enforcement and #12096 wired the header onto the principal, but a declared role the caller does not hold was silently dropped. This completes subtask [#12095](https://github.com/apache/gravitino/issues/12095) by rejecting such declarations with `403`. ### Does this PR introduce any user-facing change? Yes. A request that sends `X-Gravitino-Active-Roles` naming a role the caller does not hold now returns `403 Forbidden` (previously the unheld role was ignored). Absent header, `ALL`, and `NONE` are unaffected. When authorization is disabled, role assumption is never rejected. ### How was this patch tested? New unit tests: - `TestJcasbinAuthorizer`: `findUnheldRoles` returns empty when all declared roles are held (direct and group-inherited), returns the unheld names when a declared role is not assigned, and treats a non-existent role name as unheld. - `TestGravitinoInterceptionService`: a `NAMED` declaration with an unheld role produces a `403` and does not proceed to the method. Verified with `test` and `spotlessCheck` for `core`, `server-common`, `server`, and `iceberg-rest-server`. Part of [#11965](https://github.com/apache/gravitino/issues/11965). -- 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]
