feat(oidc): serve OpenID discovery document and JWKS endpoint#30
Conversation
When oidc_discovery.enabled, the proxy now serves the OpenID Provider metadata and a JWKS so it can front an identity provider: - GET /.well-known/openid-configuration built from config: issuer, the optional authorization/token/userinfo endpoints, jwks_uri, and the standard advertised fields (response_types, subject_types, id_token_signing_alg_values from the signing key, scopes, auth methods). - A JWKS served at the path of jwks_uri (default /.well-known/jwks.json) built from the Ed25519 public-key PEM (OKP / Ed25519 JWK, kid derived from the key). Non-EdDSA signing keys are rejected with a clear error. Routes are public, alongside the health endpoints. RSA/EC signing keys in the JWKS are left for a follow-up. Closes #29
|
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 (1)
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughImplements OIDC discovery serving: adds a new ChangesOIDC Discovery and JWKS Endpoint
Sequence Diagram(s)sequenceDiagram
participant Client
participant AxumRouter
participant Oidc
participant ed25519_jwk
Note over AxumRouter,Oidc: Startup — ProxyServer::router()
AxumRouter->>Oidc: Oidc::build(OidcDiscoveryConfig)
Oidc->>ed25519_jwk: decode PEM → compute kid via SHA-256
ed25519_jwk-->>Oidc: OKP JWK (kty, crv, x, kid)
Oidc-->>AxumRouter: routes merged (discovery + JWKS)
Note over Client,AxumRouter: Runtime requests
Client->>AxumRouter: GET /.well-known/openid-configuration
AxumRouter-->>Client: OpenID Provider metadata JSON
Client->>AxumRouter: GET /.well-known/jwks.json
AxumRouter-->>Client: JWK Set JSON
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 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 |
|
| Filename | Overview |
|---|---|
| src/oidc/mod.rs | New OIDC discovery module: builds and serves /.well-known/openid-configuration and a JWKS endpoint; key parsing is properly validated; previous review issues all addressed. Minor: id_token_signing_alg_values_supported hard-defaults to ["EdDSA"] when no signing key is configured, creating a semantic mismatch with the empty JWKS. |
| src/lib.rs | Wires OIDC routes into the main router at startup, propagating config errors cleanly; routes are merged in the correct public position alongside health and OpenAPI routes. |
| README.md | OIDC discovery promoted from Roadmap to Features; [roadmap] tag removed from config comment; jwks_uri example added with an inline clarifying note. |
Reviews (2): Last reviewed commit: "fix(oidc): validate Ed25519 SPKI, always..." | Re-trigger Greptile
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/oidc/mod.rs`:
- Around line 45-57: The `Oidc::build` function unconditionally creates a
default `jwks_uri` and passes it to `discovery_document`, but the `routes()`
function only serves JWKS when `config.signing_key` is present, causing the
discovery document to advertise a URI that returns 404. Make the `jwks_uri`
creation conditional on the presence of `config.signing_key` (similar to how
`jwks` is already conditionally created), so the discovery document only
advertises a JWKS endpoint when it will actually be served by the routes.
- Around line 149-154: The current validation in the Ed25519 key extraction
logic only checks if the DER payload is at least 32 bytes long, but does not
validate the actual SPKI structure format to confirm it is a valid Ed25519
public key. This allows non-Ed25519 keys to pass validation if they happen to be
>= 32 bytes. Before extracting the raw key material from the DER bytes using the
slice operation at `&der[der.len() - ED25519_PUBLIC_KEY_LEN..]`, add proper SPKI
structure validation that checks the algorithm identifier and other structural
markers to ensure the DER payload is a legitimate Ed25519 SPKI-encoded public
key, not just any 32+ byte payload.
🪄 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: e0ca9021-5eaf-47aa-bbd4-f45ebd0040f1
📒 Files selected for processing (3)
README.mdsrc/lib.rssrc/oidc/mod.rs
- A non-Ed25519 key (EC P-256) under algorithm "EdDSA" is accepted by the loose length check and its last 32 bytes are published as a bogus key. - When no signing key is set, the discovery document still advertises a local jwks_uri, but that path returns 404 (no JWKS route mounted). Both tests fail on current code; fixes follow. Part of #29
- ed25519_jwk now requires the exact Ed25519 SPKI shape (12-byte prefix + 32-byte key) instead of any payload >= 32 bytes, so an RSA/EC key under algorithm "EdDSA" fails loudly at startup rather than publishing its tail bytes as a bogus Ed25519 key. - The JWKS endpoint is always mounted (an empty key set when no signing key is configured), so the advertised jwks_uri never returns 404. - The JWKS is served with the RFC 7517 media type application/jwk-set+json instead of application/json. Part of #29
Summary
oidc_discoveryconfig was parsed but no endpoints were served. This implements the OIDC discovery surface so the proxy can front an identity provider, completing the last Roadmap-tagged config block.What's added (when
oidc_discovery.enabled)issuer, optionalauthorization_endpoint/token_endpoint/userinfo_endpoint,jwks_uri, and the standard advertised fields (response_types_supported,subject_types_supported,id_token_signing_alg_values_supportedfrom the signing key,scopes_supported,token_endpoint_auth_methods_supported). Absent endpoints are omitted, not null.jwks_uri(default/.well-known/jwks.json), built from the Ed25519 public-key PEM as an OKP/Ed25519 JWK (x= the 32 SPKI public bytes,kidderived from the key). Non-EdDSA signing keys are rejected with a clear error.Routes are public, alongside the health endpoints (and covered by the default maintenance exempt paths).
Design
Discovery doc, JWKS, PEM→JWK, and URI-path extraction are pure, unit-tested functions; responses are precomputed at startup and served as static JSON.
Testing
7 tests: discovery fields (incl. endpoint omission),
jwks_uripath extraction, Ed25519 PEM → OKP JWK, non-EdDSA rejection, build wiring, and an end-to-end test serving both endpoints through a real router.cargo nextest run --features redis: 92 passed. clippy (all-features) + fmt clean.Out of scope (follow-up)
RSA / EC signing keys in the JWKS, dynamic key rotation.
Docs
README: OIDC discovery moved from Roadmap to Features; the last
[roadmap]config tag is removed.Closes #29