iprithv opened a new pull request, #4406:
URL: https://github.com/apache/polaris/pull/4406
## Description
`PolarisAuthorizerImpl.authorizeOrThrow` today produces only:
```
Principal 'alice' with activated PrincipalRoles '[reader]' and activated
grants via '[]' is not authorized for op CREATE_TABLE_DIRECT
```
the operator has no way to tell *which* privilege is missing or on *which*
entity, and routinely has to grep the codebase to figure out which grant to
add. The explicit TODO at the old short-circuit in `isAuthorized` ("Collect
missing privileges to report all at the end and/or return to code that throws
NotAuthorizedException for more useful messages.") calls this out.
This is a **diagnostic improvement, not a behavioural change**, every
operation that was authorized before is still authorized, every one denied is
still denied. Only the text of the 403 changes.
## What changes
- New `findMissingPrivileges` helper that mirrors the old `isAuthorized`
traversal but **collects** failures into `List<MissingPrivilege>` instead of
short-circuiting.
- `isAuthorized` is preserved bit-for-bit by delegating to
`findMissingPrivileges(...).isEmpty()`.
- `authorizeOrThrow` formats the collected list into the
`ForbiddenException` message. The legacy prefix (`Principal 'X' with activated
PrincipalRoles 'Y' and activated grants via 'Z' is not authorized for op OP`)
is preserved verbatim so log scrapers still match.
- A small nested `MissingPrivilege` record carries side (target /
secondary), the privilege, and the `PolarisResolvedPathWrapper` it was checked
against. Its `describe()` produces the per-entry fragment and is defensive
against a null leaf so the error-formatting path never throws.
After this change:
```
Principal 'alice' with activated PrincipalRoles '[reader]' and activated
grants via '[]' is not authorized for op CREATE_TABLE_DIRECT; missing
TABLE_CREATE on NAMESPACE 'ns1'
```
Secondary failures are labelled `(secondary)` so operators can tell which
side of a `RENAME_TABLE`-style op needs grants. Multiple missing privileges are
joined with `, ` in one message.
The new SPI's `AuthorizationDecision.deny(e.getMessage())` in
`PolarisAuthorizerImpl.authorize()` inherits the richer text automatically.
## Reproduction / verification
**Baseline (production change reverted on this branch):** the three new
public-API tests fail with the original generic message:
```
Expecting throwable message:
"Principal 'alice' with activated PrincipalRoles '[reader]' and activated
grants via '[]' is not authorized for op CREATE_TABLE_DIRECT"
to contain:
"missing TABLE_CREATE on NAMESPACE 'ns1'"
but did not.
```
**This branch:** all 5 new test cases plus the existing 110 in
`polaris-core` pass.
Local runs (Java 21):
- `./gradlew :polaris-core:check` โ `BUILD SUCCESSFUL`
- `./gradlew :polaris-runtime-service:test --tests "...service.auth.*"
--tests "...service.admin.*"` โ `BUILD SUCCESSFUL`
- `./gradlew :polaris-runtime-service:spotlessCheck :checkstyleMain
:checkstyleTest` โ `BUILD SUCCESSFUL`
- `./gradlew :polaris-extensions-auth-opa:check
:polaris-extensions-auth-ranger:check` โ `BUILD SUCCESSFUL`
## Checklist
- [x] ๐ก๏ธ Don't disclose security issues! (contact [email protected])
- [x] ๐ Clearly explained why the changes are needed (addresses the TODO at
`findMissingPrivileges`)
- [x] ๐งช Added/updated tests with good coverage
- [x] ๐ก Added comments for complex logic
- [x] ๐งพ Updated `CHANGELOG.md` (under `### Changes`)
- [ ] ๐ Updated documentation in `site/content/in-dev/unreleased` (N/A โ
diagnostic-text improvement, no user-facing config or schema)
--
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]
