jerryshao opened a new pull request, #12922: URL: https://github.com/apache/gravitino/pull/12922
> **Note:** This PR should be merged *after* #12891. That PR touches the same filter-registration block in `GravitinoServer.java` (it changes `RequestContextFilter`'s constructor to accept an `eventBus`, enabling query-parameter capture, and gives `HttpAuditFilter` a success-event fallback so audited paths get logged on *every* request, not just failures). Merging #12891 first avoids a rebase on this side, and — once this PR lands on top of it with `/configs`, `/configs/secrets/providers`, `/metrics`, and `/prometheus/metrics` updated to use the `eventBus`-aware constructor too — those paths get full audit coverage (success + redacted query params) immediately, instead of a two-step gap where they only get failure-audit until #12891 catches up. I verified this combination locally (merged both branches, built a distribution, confirmed audit entries with query-param redaction on all four paths) before reverting the merge to keep this PR's diff self-contained. ### What changes were proposed in this pull request? `GravitinoServer` bound `RequestContextFilter`, `HttpAuditFilter`, custom filters, and `VersioningFilter` to `/api/*` only. `ConfigServlet` (`/configs`), `SecretProvidersConfigServlet` (`/configs/secrets/providers`), and the `/metrics`/`/prometheus/metrics` servlets are all mounted outside that pathspec and were silently bypassing every one of those filters, with nothing in the build catching it. This PR binds the same filters to those paths too, skipping `VersioningFilter` for `/metrics`/`/prometheus/metrics` since that filter negotiates Gravitino's own REST API media type and has no meaning for metrics output. Authentication stays `/api/*`-only: `/configs` must remain open for the Web UI's pre-login OAuth bootstrap, and `/configs/secrets/providers` is left open pending #12921, which will add an operator-controlled authorization gate for it specifically. A regression test (`TestGravitinoServer#testEveryServletPathIsCoveredByAuditFilter`) introspects the live `ServletContextHandler` after `initialize()` and fails if any registered servlet path lacks `HttpAuditFilter` coverage, unless it's in a small documented exemption list — so a future servlet added the same way gets caught at test time instead of silently repeating this bug. ### Why are the changes needed? An endpoint that answers without appearing in the audit log is a compliance gap independent of what it returns. `/configs/secrets/providers` in particular can expose internal secret-provider endpoint URIs (e.g. Vault/OpenBao) with zero audit trail today. Fix: #12760 ### Does this PR introduce _any_ user-facing change? Yes: requests to `/configs`, `/configs/secrets/providers`, `/metrics`, and `/prometheus/metrics` that fail (4xx/5xx) now produce an audit log entry, same as `/api/*` failures already do. No config keys added, removed, or renamed. No change to authentication — those endpoints remain open exactly as before. ### How was this patch tested? - New unit test `TestGravitinoServer#testEveryServletPathIsCoveredByAuditFilter`; verified it actually catches the regression by temporarily removing a path from coverage and confirming the test fails with an actionable message. - Full `:server` module suite: 446 tests, 0 failures. - Manual verification against a compiled distribution (`compileDistribution -PskipWeb=true -x test`) with `gravitino.audit.enabled=true`: confirmed `PUT /configs/secrets/providers` (405) and `POST /metrics` (405) now produce `FAILURE` audit entries, which they did not before this fix. -- 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]
