feat(authz): external authorization via Envoy ext_authz gRPC#36
Conversation
Gate proxied API requests through an external authorization server using the Envoy ext_authz contract (envoy.service.auth.v3.Authorization/Check), so the proxy interoperates with OPA and any ext_authz server. - Before forwarding, the proxy calls Check with a CheckRequest built from the request's HTTP attributes (method, path, host, scheme, query, headers — including the identity headers the JWT layer injected). - An OK rpc status allows the request and applies any ok_response headers; a denied response returns its status / headers / body (default 403); a missing status denies. - When the authz call fails (unreachable / timeout), failure_mode_allow chooses fail-open (forward) or fail-closed (503); default is closed. - Mounted on the proxied API routes only, inside the auth layer, so health / metrics / discovery are never gated. AuthzConfig is reshaped to the ext_authz contract (endpoint, timeout_ms, failure_mode_allow); the prior service/method/template fields modeled a different contract and were unused. Closes #35
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds Envoy ChangesEnvoy ext_authz gRPC authorization
Sequence Diagram(s)sequenceDiagram
participant Client
participant ProxyRouter
participant authz_middleware
participant AuthorizationClient as Envoy ext_authz server
participant TranscodeUpstream
Client->>ProxyRouter: HTTP request (transcoding route)
ProxyRouter->>authz_middleware: dispatch via middleware layer
authz_middleware->>authz_middleware: build_check_request(headers, method, URI, scheme)
authz_middleware->>AuthorizationClient: Check(CheckRequest) with timeout
alt Allow (RPC status OK)
AuthorizationClient-->>authz_middleware: CheckResponse { ok_response headers }
authz_middleware->>TranscodeUpstream: forward + inject ok_response headers
TranscodeUpstream-->>Client: upstream response
else Deny (non-zero RPC status or denied_response)
AuthorizationClient-->>authz_middleware: CheckResponse { denied_response }
authz_middleware-->>Client: 403 (or authz-specified status/headers/body)
else gRPC failure (timeout / unreachable)
AuthorizationClient-->>authz_middleware: error
alt failure_mode_allow=true
authz_middleware->>TranscodeUpstream: forward (fail-open)
TranscodeUpstream-->>Client: upstream response
else failure_mode_allow=false
authz_middleware-->>Client: 503 {"error":"UNAVAILABLE"}
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/auth/authz.rs`:
- Around line 80-84: The error from the failed authz gRPC call is being
discarded without logging. In the two error handling branches (the `Err(_)`
pattern matches in the failure_mode_allow guard and the subsequent Err branch),
capture the actual error by changing the pattern from `Err(_)` to `Err(e)` to
bind the error value. Then add appropriate logging before returning: for the
fail-open case where `authz.failure_mode_allow` is true, log at warn level with
the error details to help operators diagnose connectivity issues; for the
fail-closed case, log at error level since this results in a 503 service
unavailable response. This will improve observability without changing the
current error handling behavior.
In `@src/config.rs`:
- Around line 579-583: The test in src/config.rs only verifies that config.auth
is Some but does not assert that the authz configuration fields were actually
parsed correctly. After the existing assertion for config.auth.is_some(), add
additional assertions to verify the authz configuration by unwrapping the auth
object, accessing the authz field, and then asserting that enabled is true,
endpoint equals the expected URL string "http://opa:9191", timeout_ms equals
200, and failure_mode_allow is false. This ensures all authz YAML fields were
parsed and deserialized into the correct values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2da4dd2b-ecbe-4202-b43a-0911f5905fac
📒 Files selected for processing (6)
Cargo.tomlREADME.mdsrc/auth/authz.rssrc/auth/mod.rssrc/config.rssrc/lib.rs
|
| Filename | Overview |
|---|---|
| src/auth/authz.rs | New module implementing the Envoy ext_authz gRPC middleware; logic is clean and well-tested, with one contract deviation: HeaderValueOption.append semantics are not honored (always appends regardless of authz server intent) |
| src/lib.rs | Authz middleware correctly wired onto transcode routes only, before JWT auth is applied to the full router, so Check sees injected identity headers |
| src/config.rs | AuthzConfig reshaped to the ext_authz contract; all fields have correct defaults; disabled block can omit endpoint; round-trip tests pass |
| Cargo.toml | Adds envoy-types 0.7 dependency for generated v3 proto types |
| src/auth/mod.rs | Exposes the new authz submodule as pub; no other changes |
| README.md | Moves ext_authz from roadmap to the features list; promotes BFF sessions as the only remaining roadmap item |
Reviews (2): Last reviewed commit: "fix(authz): default authz endpoint and p..." | Re-trigger Greptile
- Log a warning (with the gRPC error) when the ext_authz Check call fails, noting whether the request is being failed open or closed, so operators can diagnose authz connectivity. - Assert the parsed authz fields (enabled, endpoint, timeout_ms, failure_mode_allow) in the full-config deserialization test. Part of #35
A disabled `authz` block (`enabled: false`) without an `endpoint` fails deserialization with a missing-field error, forcing a dummy URL to stub authz out. Fails on current code; fix follows. Part of #35
- Give authz.endpoint a serde default so a disabled block
(`authz: { enabled: false }`) parses without supplying a dummy URL;
build() still short-circuits on !enabled before parsing the channel.
- Apply authz-supplied response headers with append, not insert, so
multiple values for the same name (e.g. Set-Cookie) are all preserved
instead of collapsing to the last. Applies to both the allowed-request
header injection and the denied response.
Part of #35
Summary
Gates proxied API requests through an external authorization server using the Envoy ext_authz gRPC contract (
envoy.service.auth.v3.Authorization/Check), so the proxy interoperates out of the box with OPA's Envoy plugin and any ext_authz server.What's added (when
authz.enabled)Checkwith aCheckRequestbuilt from the HTTP attributes (method, path, host, scheme, query, headers — including the identity headers the JWT layer injected).rpc.Status): forward, applying anyok_responseheaders the authz server returns.denied_responsestatus / headers / body (default 403). A missing status denies (never leaks through).failure_mode_allowchooses fail-open (forward) or fail-closed (503, distinct from a policy 403). Default is fail-closed.Config reshape
AuthzConfigis reshaped to the ext_authz contract:endpoint(gRPC address),timeout_ms,failure_mode_allow. The priorservice/method/*_templatefields modeled a different (custom) contract and were unused; the Envoy contract sends the full HTTP context with a fixed service/method.Design
Decision logic (build CheckRequest, interpret CheckResponse, map headers / denied status) is pure and unit-tested; the middleware is thin glue. Uses the
envoy-typescrate for the generated v3 types.Testing
7 tests: CheckRequest attribute mapping, scheme default, allow + ok-header collection, missing-status deny, denied-response status mapping, and fail-open / fail-closed against an unreachable authz server.
cargo nextest run --features redis: 117 passed. clippy (all-features) + fmt clean.Out of scope (last roadmap item)
BFF sessions (
bff).Docs
README: External AuthZ under Features; roadmap now tracks only BFF sessions.
Closes #35