markhoerth commented on issue #11230: URL: https://github.com/apache/gravitino/issues/11230#issuecomment-4677117592
Thanks for the structured comparison @jerryshao. Posting my recommendation here so the decision is recorded on the issue. My Recommendation: Approach B (servlet filter → EventBus → AuditLogManager). Reasoning: Gravitino enterprise deployments run inside a secure network perimeter. The failure types only A can capture (external scanner probes, bad TLS handshakes, malformed HTTP framing) are largely absent in that environment. The failures that matter come from authenticated principals: malformed JSON bodies, expired or invalid tokens, and calls to unregistered endpoints. B covers all of these, and they are exactly what a security test will probe. Principal attribution is the deciding factor. An access log records an IP; the audit requirement from our enterprise findings is to record who. Acceptance criteria for B to satisfy the original audit finding: 1. Synthetic failure events must capture the authenticated principal along with URL, HTTP method, and response status code. 2. Events must be distinguishable in the audit log from completed operation events, so reviewers can separate API access patterns from real operations. 3. The logging filter must be positioned before AuthenticationFilter, or 401 rejections will never reach it. The ordering should be explicit in code and covered by a test, since it is the kind of constraint that breaks silently. 4. The filter should skip responses for which a real event already fired (e.g. authorization denials once the interceptor emits a FailureEvent), so failures are not double-logged. -- 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]
