feat(auth): inbound OIDC/JWT authentication — trust providers + jwt_subject key binding - #825
Merged
Conversation
…ubject key binding
Adds the data-plane half of AISIX-Cloud#1080/#1081: requests may now
authenticate with an IdP-issued JWT (Keycloak or any OIDC provider)
instead of a gateway API key.
New etcd kind `oidc_providers` (env-scoped rows: issuer, audiences,
optional jwks_uri with OIDC-discovery fallback, identity_claim,
required_scopes, bound_claims, leeway_secs, enabled). When at least one
enabled provider exists, a JWT-shaped bearer (three segments whose first
decodes to a JOSE header) takes the JWT path at the single auth choke
point — inherited by every proxy surface including /mcp, /a2a, realtime
subprotocol auth, and passthrough — with no fallback to key lookup on
failure.
Validation chain (RFC 7519 §4.1): asymmetric algorithms only (RS*/PS*/
ES256/384/EdDSA — HMAC excluded to keep a public JWKS from doubling as a
shared secret), signature against the provider's JWKS, `exp` required,
`iss` pinned to the matched provider (unlisted issuers rejected — the
provider list is an allow-list), `aud` required and matched against the
provider's accepted audiences, `nbf` honored when present, then
required_scopes / bound_claims (dot-paths traverse nested claims, e.g.
realm_access.roles).
Identity mapping: the provider's identity_claim value selects the API
key whose new `jwt_subject` field equals it; the request then proceeds
as that key, so allowed_models, rate limits, budgets, usage attribution,
and MCP/A2A ACLs all apply unchanged. An unmapped identity is rejected
(jwt_identity_unmapped), never an anonymous pass.
JWKS handling: per-URL process cache (10 min TTL), redirect-refusing
fetch with timeout and size cap, serve-stale on re-fetch failure, and a
rate-limited immediate re-fetch on unknown `kid` so IdP key rotation is
picked up within ~1s with no restart.
Auth decisions become observable: new aisix_auth_decisions_total
{method, result, reason} metric plus target="aisix::auth" decision logs
(denials at WARN with reason class, issuer, kid, subject — never the
token). Previously a 401 was invisible: the extractor short-circuits
before every handler, so no counter or log ever fired. Client-visible
error codes extend the existing lifecycle-code convention: jwt_expired,
jwt_invalid (signature/issuer/audience collapsed to avoid an oracle),
jwt_claims_rejected (403), jwt_identity_unmapped, jwks_unavailable
(503, retryable — IdP unreachable is not a credential judgment).
The kind is wired through the full config surface: loader, watch
supervisor (apply/delete/clone/counts — the every-ResourceTable merge
loops), standalone resources_file (with a duplicate-jwt_subject load
error mirroring the duplicate-credential rule), snapshot export, and
schemas/resources via dump-schema.
Reference-implementation comparison: mainstream LLM proxies implement
this same shape — external JWT verified against a JWKS, then mapped to
an internal virtual key that carries the budget/limit/model policy; the
scheme-level JWT/key discrimination on the bearer is also the
established pattern. Deliberate divergences, all tightening: iss/aud/exp
are mandatory here while the common global path verifies iss/aud only
when configured and accepts exp-less tokens; unknown issuers hard-fail
instead of falling back to a catch-all verification path; unknown-kid
triggers a rate-limited refetch instead of waiting out the cache TTL
(rotation outage otherwise lasts up to the TTL); an unmapped identity is
rejected instead of falling back to a group-based identity; and JIT
identity auto-registration is out of scope (the DP is an etcd read-only
consumer — pre-created keys cover the fleet-onboarding case via the
Admin API).
E2E (tests/e2e/src/cases/jwt-auth-e2e.test.ts, 20 cases): happy path,
bound-key allowed_models/rate_limit inheritance, expired/tampered/
exp-less/wrong-aud/cross-issuer/unknown-issuer rejections with stable
codes and zero upstream hits, scope/bound-claim 403s, unmapped identity,
disabled key, discovery-resolved provider, key rotation without restart,
provider deletion failing closed, Anthropic envelope on /v1/messages,
metric exposure, dotted custom keys unaffected, and no-JWKS-fetch when
no provider is configured. Unit tests cover the validation matrix with a
fixture RSA keypair, claim helpers, tie-breaks, and the filesource/
supervisor kind wiring.
The paired control-plane exposure (cp-admin.yaml `oidc_providers` CRUD +
ApiKey.jwt_subject, dashboard, CP↔DP e2e) follows in AISIX-Cloud; this
PR intentionally does not add an Admin API CRUD surface for the kind
(same posture as mcp_policies — the resources file covers standalone).
Fixes api7/AISIX-Cloud#1080
🤖 Generated with [Claude Code](https://claude.com/claude-code)
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 8 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughAdds configurable OIDC providers and JWT bearer authentication, including issuer and audience validation, JWKS discovery and rotation, claim-based API-key binding, structured errors and metrics, resource propagation, exports, and end-to-end coverage. ChangesOIDC provider contracts and storage
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant ProxyAuth
participant JWTAuth
participant OIDCProvider
participant JWKS
participant APIKey
Client->>ProxyAuth: send bearer JWT
ProxyAuth->>JWTAuth: authenticate token
JWTAuth->>OIDCProvider: select issuer and provider
JWTAuth->>JWKS: resolve and fetch signing keys
JWKS-->>JWTAuth: return verification keys
JWTAuth->>APIKey: map identity claim to jwt_subject
APIKey-->>JWTAuth: return bound key
JWTAuth-->>ProxyAuth: allow or return structured error
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (8)
crates/aisix-proxy/src/jwt.rs (2)
386-389: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueComment contradicts the code.
set_required_spec_claims(&["exp", "iss", "aud"])makes those claims mandatory; the sentence reads as if they are optional-when-present. Suggest restating.✏️ Wording
- // `aud`/`iss` are only checked when present — requiring them makes - // absence a rejection (default deny), alongside the always-required - // `exp`. + // Default deny: `exp`, `iss`, and `aud` are all mandatory. Without + // listing them here, jsonwebtoken skips `iss`/`aud` when the claim + // is absent, which would let a token with no `aud` through. validation.set_required_spec_claims(&["exp", "iss", "aud"]);🤖 Prompt for 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. In `@crates/aisix-proxy/src/jwt.rs` around lines 386 - 389, Update the comment above set_required_spec_claims in the JWT validation setup to state that exp, iss, and aud are always required and that omitting any of them causes rejection; keep the validation code unchanged.
700-729: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueChecked-in RSA private key will keep tripping secret scanners.
It is a genuine test fixture, not a leak — but
betterleaksflags it, and every future scan will too. Either generate the keypair at test time (deriving the JWK from the public half, soTEST_JWKSstays in sync automatically) or add the scanner's inline allow annotation so the finding is explicitly triaged rather than re-litigated each run.🤖 Prompt for 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. In `@crates/aisix-proxy/src/jwt.rs` around lines 700 - 729, Remove the checked-in private key from the `TEST_RSA_PEM` fixture to prevent secret-scanner findings. Generate the RSA keypair during test setup and derive the corresponding JWK used by `TEST_JWKS` from its public key so both fixtures remain synchronized; alternatively, add the repository-supported inline `betterleaks` allow annotation directly for this intentionally triaged fixture.Source: Linters/SAST tools
crates/aisix-proxy/src/error.rs (1)
284-284: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value
JwksUnavailableis a retryable 503 with noRetry-After.
retry_after_secs()returnsNonefor this variant, so clients get no backoff hint on the one JWT error that is explicitly documented as retryable. A constant matchingJWKS_REFRESH_MIN_INTERVALwould be enough.🤖 Prompt for 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. In `@crates/aisix-proxy/src/error.rs` at line 284, Update retry_after_secs() for ProxyError::JwksUnavailable to return a retry delay matching JWKS_REFRESH_MIN_INTERVAL instead of None, while preserving the existing 503 status mapping.tests/e2e/src/harness/jwks-mock.ts (2)
136-139: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueUnnecessary PEM round-trip on every
sign().
signer.privateKeyis already aKeyObject; exporting it to PKCS#8 PEM and re-importing it per token is pure overhead in a helper the suite calls dozens of times.♻️ Simplification
const sig = createSign("RSA-SHA256") .update(signingInput) - .sign(createPrivateKey(signer.privateKey.export({ format: "pem", type: "pkcs8" }))); + .sign(signer.privateKey);(and drop the now-unused
createPrivateKeyimport)🤖 Prompt for 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. In `@tests/e2e/src/harness/jwks-mock.ts` around lines 136 - 139, Update the signing logic around signer.privateKey to pass the existing KeyObject directly to sign(), removing the per-token PKCS#8 PEM export and re-import. Remove the now-unused createPrivateKey import while preserving the generated token format.
168-170: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUnused re-export.
The comment describes a scenario no test in this PR exercises — the cross-issuer test at
tests/e2e/src/cases/jwt-auth-e2e.test.tsLine 267 just signs with the secondMockIdp. Drop it unless a follow-up needs it.As per coding guidelines, "avoid speculative features, unnecessary abstractions, configurability".
🤖 Prompt for 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. In `@tests/e2e/src/harness/jwks-mock.ts` around lines 168 - 170, Remove the unused createPublicKey re-export from the JWKS mock module, along with its speculative explanatory comment. Keep the underlying helper private and preserve the existing MockIdp test behavior.Source: Coding guidelines
crates/aisix-obs/src/metrics.rs (1)
383-397: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider
&'static strformethod/reasonto type-enforce bounded cardinality.The doc comment promises bounded labels, but the signature accepts any
&str.record_usage_event_emitin this same file already uses&'static strfor exactly this reason (audit MEDIUM-3). Every current call site passes a literal, so the change is mechanical.♻️ Proposed tightening
- pub fn record_auth_decision(&self, method: &str, allowed: bool, reason: &str) { + pub fn record_auth_decision(&self, method: &'static str, allowed: bool, reason: &'static str) { metrics::with_local_recorder(&self.inner.recorder, || { metrics::counter!( M_AUTH_DECISIONS_TOTAL, - "method" => method.to_string(), - "result" => if allowed { "allowed" } else { "denied" }.to_string(), - "reason" => if reason.is_empty() { "none" } else { reason }.to_string(), + "method" => method, + "result" => if allowed { "allowed" } else { "denied" }, + "reason" => if reason.is_empty() { "none" } else { reason }, ) .increment(1); }); }🤖 Prompt for 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. In `@crates/aisix-obs/src/metrics.rs` around lines 383 - 397, Update record_auth_decision to accept &'static str for both method and reason, matching the bounded-cardinality contract and the existing record_usage_event_emit signature. Keep the label normalization and counter behavior unchanged, and ensure all current call sites continue passing string literals.tests/e2e/src/cases/jwt-auth-e2e.test.ts (2)
356-363: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the error code on the retired-key token.
A bare
401passes for any credential failure. Pinningjwt_invalidis what actually proves the retired key stopped verifying rather than some unrelated rejection.💚 Tighter assertion
expect(stale.status).toBe(401); - await stale.text(); + expect(await errorCode(stale)).toBe("jwt_invalid");🤖 Prompt for 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. In `@tests/e2e/src/cases/jwt-auth-e2e.test.ts` around lines 356 - 363, Update the retired-key token assertions in the JWT authentication test to verify the response error code is exactly jwt_invalid, in addition to the existing 401 status check. Use the response from chat and parse its body as needed before asserting the code, while preserving the stale response consumption.
392-401: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the metrics assertions self-contained.
reason="jwt_expired"andresult="allowed"only appear because earlier tests in this file happened to produce them. Minting one valid and one expired token inside this test before scraping removes the ordering dependency and keeps the assertions meaningful if the file is ever reordered or run with--shard.As per coding guidelines, "Avoid explicit dependencies between tests and hidden execution order assumptions."
🤖 Prompt for 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. In `@tests/e2e/src/cases/jwt-auth-e2e.test.ts` around lines 392 - 401, The metrics test `auth decisions surface on aisix_auth_decisions_total` currently depends on earlier tests to emit the expected labels. Before scraping metrics, create and exercise one valid JWT and one expired JWT within this test, then retain assertions for `method="jwt"`, `result="allowed"`, and `reason="jwt_expired"` so the test is self-contained and order-independent.Source: Coding guidelines
🤖 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 `@crates/aisix-core/src/models/oidc_provider.rs`:
- Around line 52-56: Enforce uniqueness of enabled issuer values during both
file and etcd provider ingestion, using the OIDC provider loading/validation
paths around the issuer field. Reject or fail closed when multiple enabled
providers share the same issuer instead of allowing lowest-ID selection;
preserve normal matching when issuers are unique.
In `@crates/aisix-etcd/src/loader.rs`:
- Around line 285-296: Enforce unique jwt_subject values for OIDC providers in
the etcd loader: add a snapshot-wide duplicate check during the “oidc_providers”
loading path, alongside validate_and_parse and snapshot.oidc_providers
insertion. Update apply_put to reject or otherwise fail closed when a new
provider conflicts with an existing subject, ensuring ambiguous subjects cannot
reach key_for_subject and select an arbitrary provider.
In `@crates/aisix-proxy/src/jwt.rs`:
- Around line 673-689: Update fetch_json to reject responses whose
Content-Length already exceeds JWKS_MAX_BYTES, then consume the response body as
a stream while tracking accumulated bytes and aborting as soon as the cap is
exceeded; only parse JSON after the bounded stream completes, preserving the
existing error context for request, body-read, oversize, and JSON failures.
In `@crates/aisix-server/src/export/document.rs`:
- Around line 306-319: Update OIDC provider ingestion to reject jwks_uri values
containing URL userinfo or credential-bearing query parameters, while preserving
valid public URLs. In the oidc_providers export block using emit_entries, redact
legacy credential-bearing jwks_uri values whenever reveal_secrets is false;
never serialize tokens or authentication credentials, and retain non-sensitive
provider fields.
In `@schemas/resources/oidc_provider.schema.json`:
- Around line 11-18: Add "minItems": 1 to the array schema for
BoundClaimExpect::Any in the generated OIDC provider schema, preserving its
existing string-item and array structure so empty bound_claims values are
rejected.
In `@tests/e2e/src/cases/jwt-auth-e2e.test.ts`:
- Around line 128-135: Separate the rate-limit test’s credentials from the
shared agent-1 setup: seed a dedicated agent-ratelimited JWT subject/API key
with rpm: 2, and remove the rate limit from agent-1’s key. Update the rate-limit
test to authenticate as agent-ratelimited, while leaving the other agent-1 tests
independent of request order.
---
Nitpick comments:
In `@crates/aisix-obs/src/metrics.rs`:
- Around line 383-397: Update record_auth_decision to accept &'static str for
both method and reason, matching the bounded-cardinality contract and the
existing record_usage_event_emit signature. Keep the label normalization and
counter behavior unchanged, and ensure all current call sites continue passing
string literals.
In `@crates/aisix-proxy/src/error.rs`:
- Line 284: Update retry_after_secs() for ProxyError::JwksUnavailable to return
a retry delay matching JWKS_REFRESH_MIN_INTERVAL instead of None, while
preserving the existing 503 status mapping.
In `@crates/aisix-proxy/src/jwt.rs`:
- Around line 386-389: Update the comment above set_required_spec_claims in the
JWT validation setup to state that exp, iss, and aud are always required and
that omitting any of them causes rejection; keep the validation code unchanged.
- Around line 700-729: Remove the checked-in private key from the `TEST_RSA_PEM`
fixture to prevent secret-scanner findings. Generate the RSA keypair during test
setup and derive the corresponding JWK used by `TEST_JWKS` from its public key
so both fixtures remain synchronized; alternatively, add the
repository-supported inline `betterleaks` allow annotation directly for this
intentionally triaged fixture.
In `@tests/e2e/src/cases/jwt-auth-e2e.test.ts`:
- Around line 356-363: Update the retired-key token assertions in the JWT
authentication test to verify the response error code is exactly jwt_invalid, in
addition to the existing 401 status check. Use the response from chat and parse
its body as needed before asserting the code, while preserving the stale
response consumption.
- Around line 392-401: The metrics test `auth decisions surface on
aisix_auth_decisions_total` currently depends on earlier tests to emit the
expected labels. Before scraping metrics, create and exercise one valid JWT and
one expired JWT within this test, then retain assertions for `method="jwt"`,
`result="allowed"`, and `reason="jwt_expired"` so the test is self-contained and
order-independent.
In `@tests/e2e/src/harness/jwks-mock.ts`:
- Around line 136-139: Update the signing logic around signer.privateKey to pass
the existing KeyObject directly to sign(), removing the per-token PKCS#8 PEM
export and re-import. Remove the now-unused createPrivateKey import while
preserving the generated token format.
- Around line 168-170: Remove the unused createPublicKey re-export from the JWKS
mock module, along with its speculative explanatory comment. Keep the underlying
helper private and preserve the existing MockIdp test behavior.
🪄 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: CHILL
Plan: Pro
Run ID: 93f4e01b-ea1e-424e-a59e-cd35c9ba60b0
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (24)
crates/aisix-core/src/bin/dump-schema.rscrates/aisix-core/src/filesource/mod.rscrates/aisix-core/src/filesource/tests.rscrates/aisix-core/src/models/apikey.rscrates/aisix-core/src/models/mod.rscrates/aisix-core/src/models/oidc_provider.rscrates/aisix-core/src/models/schema.rscrates/aisix-core/src/models/snapshot.rscrates/aisix-etcd/src/loader.rscrates/aisix-etcd/src/supervisor.rscrates/aisix-obs/src/metrics.rscrates/aisix-proxy/Cargo.tomlcrates/aisix-proxy/src/auth.rscrates/aisix-proxy/src/error.rscrates/aisix-proxy/src/jwt.rscrates/aisix-proxy/src/lib.rscrates/aisix-proxy/src/realtime.rscrates/aisix-server/src/export/document.rsschemas/resources/api_key.schema.jsonschemas/resources/oidc_provider.schema.jsontests/e2e/src/cases/jwt-auth-e2e.test.tstests/e2e/src/harness/index.tstests/e2e/src/harness/jwks-mock.tstests/e2e/src/harness/seed.ts
…y, JWKS rate limiting, hardening Independent audit findings on the inbound OIDC/JWT path: H1 (auth bypass) — cross-provider identity impersonation. key_for_subject resolved jwt_subject against the whole api_key table, ignoring which trust provider asserted it. With two providers enabled, provider B could mint a token carrying provider A's subject and inherit A's key. Fixed by namespacing the binding: ApiKey gains `jwt_provider` (the oidc_providers name allowed to assert its jwt_subject), the lookup now matches (jwt_provider, jwt_subject), and the filesource rejects jwt_subject set without jwt_provider and enforces per-pair (not per-subject) uniqueness. H2 (availability + amplification) — JWKS/discovery were fetched on every request once the cache went stale: the 1s rate limiter only covered the unknown-kid path. A down IdP added a 5s timeout to every authenticated request and one outbound fetch per inbound JWT. The primary get_jwks path and OIDC discovery now share the rate limiter and serve-stale, and discovery tracks last_attempt so a down issuer isn't re-probed per request. M3 — attacker-controlled `kid`/`iss` were logged with Display (raw, unescaped, uncapped) on every deny before verification: log injection + flooding. Now logged with Debug (escapes control bytes) and truncated; pre-verification reason classes drop to debug. M4 — JWKS_MAX_BYTES was checked after buffering the whole body (no real bound). Now enforced while streaming, with an early content-length reject. M5 — a discovery-advertised jwks_uri was trusted verbatim (SSRF / trust relocation). Now the document's issuer must match the configured issuer (OIDC Discovery §4.3) and jwks_uri must stay on the issuer's origin. M6 — the use/alg JWK filter was applied only on the no-kid branch; a `use:enc` or wrong-alg key whose kid was named was accepted for verification. Now filtered on both branches (RFC 7517 §4.2/§4.4). M8 — the auth path cloned the whole apikeys/oidc_providers tables per request (three Vec allocations + per-row Arc clones). Added non-materialising ResourceTable::any / find_min_by_id and use them. M9 — the API-key path logged a WARN per 401 for unknown_key (the scanner probe shape), an unbounded log-volume regression. Dropped to debug, same as the extractor's missing_credentials. M10 — e2e now pins the headline invariants: no-fallback (a JWT-shaped bearer that is also a valid key stays rejected as a JWT), alg-confusion (HS256 rejected), no-kid single-key verify, JWKS caching (a burst does not refetch per request), and cross-provider impersonation blocked. L12/L13/L14 — recover poisoned JWKS-cache locks instead of propagating the panic process-wide; pass the already-loaded snapshot into authenticate_jwt; cap bearer length before the base64/JSON work. Verified-correct findings (alg:none impossible, HS-with-JWKS blocked at two layers, exp/iss/aud presence genuinely enforced, no peek/validate split-brain, byte-exact issuer match) needed no change. The paired CP change adds `jwt_provider` to the api_key surface and its projection. Fixes api7/AISIX-Cloud#1080 🤖 Generated with [Claude Code](https://claude.com/claude-code)
The no-kid test signed as agent-1 (rpm=2) and expected 200, but earlier successful auths can exhaust that key's rate-limit window, so CI saw a 429. Sign as the unrestricted agent-2 key: the case verifies the no-kid JWKS fall-through, not rate limiting.
…l-free JWKS URLs, schema minItems, test isolation - Ambiguous issuer / (jwt_provider, jwt_subject) now fail closed instead of picking the lowest-id match: two enabled providers on one issuer, or two keys on one binding, carry different policy and neither can be chosen safely. Added ResourceTable::find_unique_by; the file loader also rejects duplicate enabled issuers at load. The CP's per-env unique indexes make this a defense-in-depth guard for etcd races. - Reject credential-bearing issuer/jwks_uri (userinfo or a token query param) at file-load and at DP resolve time — JWKS material is public, so embedded credentials would only leak (e.g. via a snapshot export). Never stored, so nothing to redact downstream. - bound_claims: inject minItems:1 into the BoundClaimExpect array branch (schemars drops the length constraint on an untagged enum variant), so the schema matches the model's non-empty contract. - e2e: give the rate-limit test a dedicated rpm=2 identity (agent-rl) so the shared agent-1 key is no longer exhaustible by earlier requests — removes the coupling behind the earlier no-kid flake. The streaming JWKS size cap CodeRabbit flagged was already fixed in the prior security-review commit.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds the data-plane half of AISIX-Cloud#1080 / AISIX-Cloud#1081: requests may now authenticate with an IdP-issued JWT (Keycloak or any OIDC-compliant provider) instead of a gateway API key. The paired control-plane PR (cp-admin.yaml CRUD + dashboard + CP↔DP e2e) follows in AISIX-Cloud.
What
New etcd kind
oidc_providers— env-scoped trust rows:{ "name": "corp-keycloak", "issuer": "https://sso.example.com/realms/agents", // exact-match allow-list "audiences": ["aisix-gateway"], // token aud must intersect "jwks_uri": "https://…/certs", // optional; OIDC discovery fallback "identity_claim": "sub", // dot-paths supported "required_scopes": ["ai.access"], "bound_claims": { "department": "ai-lab", "realm_access.roles": ["agent"] }, "leeway_secs": 0, "enabled": true }JWT path at the single auth choke point (
authenticate_token): inherited by every proxy surface — chat/completions/messages/responses/embeddings/rerank/audio/images/videos/files/batches/fine-tuning,/mcp,/a2a, realtime WebSocket subprotocol auth, and passthrough — with no fallback to key lookup once a real JWT enters the path. A bearer is treated as a JWT only when it has three segments and the first decodes to a JOSE header, so dotted custom-imported keys keep working.Validation chain (RFC 7519 §4.1, all default-deny):
RS*/PS*/ES256/384/EdDSA) — HMAC excluded so a public JWKS can never double as a shared signing secretexprequired and valid;nbfhonored when present; per-providerleeway_secsisspinned to the matched provider; an issuer matching no enabled row is rejected (the provider list is an allow-list, no catch-all path)audrequired, matched againstaudiencesrequired_scopes(space-delimited string or arrayscopeclaim) andbound_claims(dot-paths traverse nested claims) →403 jwt_claims_rejectedIdentity mapping — external token → internal API key: the provider's
identity_claimvalue selects the key whose newApiKey.jwt_subjectequals it. The request then proceeds as that key, soallowed_models, rate limits, budgets, usage attribution, and MCP/A2A ACLs all apply unchanged; an unmapped identity is401 jwt_identity_unmapped, never an anonymous pass. One key per agent identity gives 110+ agents per-agent budget/limit/attribution with zero changes downstream.JWKS lifecycle: per-URL process cache (10 min TTL); fetches refuse redirects, carry a 5 s timeout and a 512 KB cap; a failed re-fetch serves the stale set (network-partition tolerance); an unknown
kidtriggers a rate-limited (1/s) immediate re-fetch, so IdP key rotation is picked up within ~1 s with no gateway restart.Auth decisions become observable (the audit half of AISIX-Cloud#1081): new
aisix_auth_decisions_total{method, result, reason}metric +target="aisix::auth"decision logs (denials at WARN with reason class / issuer / kid / subject — never the token; API-key path included). Previously a 401 was invisible — the extractor short-circuits before every handler, so no counter or log ever fired. New stableerror.codevalues follow the existing lifecycle-code convention:jwt_expired,jwt_invalid(signature/issuer/audience collapsed to avoid an oracle),jwt_claims_rejected(403),jwt_identity_unmapped,jwks_unavailable(503 — IdP unreachable is retryable, not a credential judgment)./v1/messagesrenders all of them in the Anthropic envelope via the existing status mapping.Full config-surface wiring for the new kind: loader, watch supervisor (apply/delete/clone/counts merge loops — the initial gap here was caught by the e2e and is now pinned by the every-kind supervisor tests), standalone
resources_file(with a duplicate-jwt_subjectload error mirroring the duplicate-credential rule), snapshot export,schemas/resources/via dump-schema. No Admin API CRUD surface for the kind in this PR (same posture asmcp_policies; the resources file covers standalone deployments).Reference-implementation comparison
Mainstream LLM proxies implement this same overall shape: an external JWT verified against a cached JWKS, then mapped to an internal virtual key that carries budget/limit/model policy (claim-value → key mapping), with structural JWT/key discrimination on the bearer and no fallback after a failed JWT validation. Upstream specs: RFC 7519 (JWT), RFC 7517 (JWK), OIDC Discovery 1.0 §4.
Deliberate divergences, all tightening:
iss/aud/expkidTests
tests/e2e/src/cases/jwt-auth-e2e.test.ts(20 cases, real binary + etcd + mock IdP): happy path; bound-keyallowed_models/rate_limitinheritance; expired / tampered / exp-less / wrong-aud / cross-issuer / unknown-issuer rejections with stable codes and zero upstream hits; scope / bound-claim 403s; unmapped identity; disabled key; discovery-resolved provider; key rotation without restart; provider deletion failing closed; Anthropic envelope; metric exposure; dotted custom keys unaffected; and zero JWKS fetches when no provider is configured.Fixes api7/AISIX-Cloud#1080
🤖 Generated with Claude Code
Summary by CodeRabbit
oidc_providerssupport end-to-end (schema, loading, snapshots, and exports).aisix_auth_decisions_totalmetric for authentication outcomes.jwt_subjectwithoutjwt_providerand detecting duplicate(jwt_provider, jwt_subject)mappings.oidc_providerand JWT subject binding fields.