ADORSYS-GIS / ADORSYS-GIS/lightbridge-authz
api_key_id claim reused for token-exchange session id causes gateway 403 + wrong budget-refill refusal
- 主要言語
- Rust
- スター
- 0
- フォーク
- 1
- 平均マージ
- 7時間 7分
- マージ済み PR(30日)
- 237
説明
### Type
Bug
### Summary
We need to stop reusing the JWT claim name `api_key_id` for the native RFC 8693
token-exchange session id, because [ai-helm-values#290](https://github.com/ADORSYS-GIS/ai-helm-values/pull/290)
is a gateway-side mitigation for a symptom this repo causes, not a fix for the
cause itself, and a second, independent bug from the same cause was found while
investigating (`requestBudgetRefill` wrongly refusing legitimate human callers).
Expected result: a token-exchange-minted access token carries a claim set that
is unambiguously distinguishable, at every layer that reads it (this repo's own
`Procedures` RBAC gate, and the gateway's Authorino `AuthConfig`), from a real
self-signed API key — without relying on a side-channel discriminator like `azp`.
### Intent
This is the root-cause half of a confirmed production incident: a token minted
by lightbridge-authz's native RFC 8693 token-exchange (ADR-0011) failed against
the chat-completion endpoint, because the gateway's `lightbridge-key-active`
Authorino rule treated the exchanged token as a revoked API key. The immediate
fix is a gateway-side mitigation
([ai-helm-values#290](https://github.com/ADORSYS-GIS/ai-helm-values/pull/290)); this
ticket is the backend-side follow-up that removes the underlying cause so the
gateway doesn't need a fragile `azp`-based workaround, and so the second bug
found below (which the gateway cannot fix at all) gets closed too.
### Source of truth (links)
- Incident: repo owner report, 2026-08-22 ("a token minted by token-exchange
fails against the chat-completion endpoint").
- Mitigation PR (gateway side): https://github.com/ADORSYS-GIS/ai-helm-values/pull/290
- ADR-0011 (native token-exchange): `docs/adr/0011-*.md` in this repo.
- `docs/rbac.md`'s "Self-service refill and the admin review queue (#191)"
section, which documents the `caller_kind` refusal this issue shows is
currently wrong for the `self`-signing mode it claims is "fully closed."
### Current Behavior
`access_token_extra()` (`crates/lightbridge-authz-rest/src/signing.rs:186-227`)
is shared between two callers that mint semantically different tokens:
1. `ApiKeyJwtSigner::sign` — mints a real, persisted API key. `api_key_id` is a
genuine `api_keys.id`. `azp` is the fixed `oauth2.signing.audience` config
value (`"lightbridge-api-key"` in prod). `lightbridge_caller_kind` = `"api_key"`.
2. `oauth2_op::store::{handle_token_exchange, handle_refresh_token}` — mints a
human-plane session token for an authenticated OIDC caller.
`crates/lightbridge-authz-rest/src/oauth2_op/store.rs:222`/`397`: `let
session_id = cuid2();` is passed into `access_token_extra` as `api_key_id`.
`azp` is the requesting client's `client_id` (e.g. `governance-auth-cli`).
**`lightbridge_caller_kind` is *also* stamped `"api_key"`** — same constant,
same code path, no override.
Two confirmed, independent consequences of the same cause:
**A. Gateway 403 (the reported incident, mitigated in ai-helm-values#290).**
Authorino's `lightbridge-key-active`/`lightbridgeintrospect` rules gate on
`api_key_id != ""` to decide "is this an API-key-plane caller, worth an
introspection round-trip to check revocation." Since exchanged tokens satisfy
that too, the gateway introspects a session id that was never inserted into
`api_keys`, gets back `{"active": false}` from
`crates/lightbridge-authz-rest/src/handlers/introspect.rs` (via
`validate_api_key_context` in `handlers/opa.rs:42-59`, which hashes the
presented token and misses in `api_keys`), and denies the request as if the
key were revoked.
**B. `requestBudgetRefill` wrongly refuses exchanged-token human callers
(newly found while investigating — not previously reported, not gateway-fixable).**
`Procedures::request_budget_refill` (`crates/lightbridge-authz-rest/src/lib.rs:997-1002`):
```rust
let caller_kind = caller_kind_from_ctx(ctx);
...
if caller_kind.as_deref() == Some(lightbridge_authz_bearer::API_KEY_CALLER_KIND) {
return Err(CoolError::Forbidden(
"self-service budget refills are for OIDC human callers only".to_owned(),
));
}
```
`docs/rbac.md`'s "Internal/API-key-client refusal (#191/#216)" section claims
this is "fully closed" for `oauth2.type: self` because `ApiKeyJwtSigner` stamps
`lightbridge_caller_kind` "on every self-signed API-key JWT it mints,
unconditionally, so it is present exactly when the caller is API-key-derived."
That statement is now stale: `access_token_extra` is also called by the
token-exchange grant, which stamps the identical claim on genuinely-human
callers. A human who authenticates via the native RFC 8693 exchange (exactly
the flow ADR-0011 and the incident above are about) and then calls
`requestBudgetRefill` gets a 403 "for OIDC human callers only" — despite being
exactly that. This is entirely inside this repo; no gateway change can fix it.
No existing test catches either bug: `budget_refill_procedure_tests.rs`
constructs a `CoolContext` with `API_KEY_CALLER_KIND` directly rather than
exercising a real token-exchange grant
(`crates/lightbridge-authz-rest/tests/budget_refill_procedure_tests.rs:85-91`),
and `token_exchange_tests.rs` never asserts on `lightbridge_caller_kind` at all.
### Expected Behavior
Token-exchange-minted tokens carry claims that let every consumer (this repo's
own RBAC, and the gateway) distinguish them from real self-signed API keys
directly — not via a side-channel like `azp`/`client_id` matching (or not
matching) a magic string.
### Acceptance Criteria
- [ ] Given a token minted by `handle_token_exchange`/`handle_refresh_token`,
when `Procedures::request_budget_refill` reads its caller-kind signal,
then it is NOT treated as `API_KEY_CALLER_KIND` (bug B closed).
- [ ] Given the same token presented to authz-opa's introspection endpoint (or
to whatever the gateway keys the "is this an API key" `when` gate on
after this change), then it is not looked up as if it were an API key
secret, and does not resolve to `{"active": false}`.
- [ ] Given a real, revoked self-signed API key, existing revocation behavior
(RBAC + gateway `lightbridge-key-active`) is unchanged.
- [ ] A regression test exercises this through a *real* token-exchange grant
(not a hand-built `CoolContext`), asserting the resulting claim set is
distinguishable from a real API-key token's — closing the coverage gap
both `budget_refill_procedure_tests.rs` and `token_exchange_tests.rs`
currently leave.
- [ ] `docs/rbac.md`'s "Internal/API-key-client refusal (#191/#216)" section is
corrected — it currently asserts `self`-mode is "fully closed," which
this issue shows is false for the token-exchange sub-path.
- [ ] ai-helm-values' `lightbridge-key-active`/`lightbridgeintrospect`/
`lightbridge-model-allowed` `when` gates are revisited once this lands —
the `azp == "lightbridge-api-key"` mitigation in ai-helm-values#290 can
likely be simplified or removed if the new claim shape makes the
distinction direct.
### Out of Scope
- Implementing the fix — this ticket is deliberately assessment-only. Another
session is actively working in this repo's `oauth2_op`/`signing` area; this
is a breaking wire-format change (`api_key_id` claim semantics) and needs its
own scoped, reviewed PR, not a drive-by rename bundled with the incident
response.
- The ai-helm-values gateway mitigation (already shipped separately, see
ai-helm-values#290 above).
- Any change to `azp`/`aud` semantics for governance — only the
`api_key_id`/`lightbridge_caller_kind` reuse is in scope.
### Technical Context
Candidate directions (not a decision — needs its own design pass):
1. **Stop stamping `api_key_id` at all on exchange-minted tokens**, and give
the session id its own claim name (e.g. `session_id`, already effectively
what `sid` is for — worth checking whether `sid` can simply become the one
true session identifier and `api_key_id` can be omitted entirely on this
path). Lowest-risk in this repo, but is the literal breaking wire change
flagged above: every consumer that currently reads `api_key_id` unqualified
(this repo's `x-api-key-id` header mapping and RBAC checks, and the gateway
`when` gates) needs to be re-audited for "did it actually want *any*
api_key_id-shaped claim, or specifically a real API key's id."
2. **Give `lightbridge_caller_kind` a third value** (e.g. `"token_exchange"`)
distinct from `"api_key"` and `"oidc"`/absent, and fix
`request_budget_refill`'s check to compare against the exact kind it means
to exclude. Smaller blast radius than (1) alone, but does not fix the
gateway-side confusion unless the gateway is also taught to read this claim
(today it only reads `api_key_id`/`iss`, not `lightbridge_caller_kind`).
3. Both together is probably the real fix: rename `api_key_id` → something
session-scoped for the exchange path AND stop conflating
`lightbridge_caller_kind` between the two mint paths.
Every currently-known consumer of `api_key_id` on the human plane (grepped
across both repos, non-test code):
- `crates/lightbridge-authz-rest/src/handlers/introspect.rs`,
`handlers/opa.rs`, `handlers/mod.rs`, `models/authorino.rs`, `models/mod.rs`
— the real-API-key validation/introspection path this claim was designed
for; not itself broken, just fed a claim it wasn't meant to receive.
- `crates/lightbridge-authz-rest/src/oauth2_op/store.rs`,
`crates/lightbridge-authz-rest/src/signing.rs` — the two mint sites, both
described above.
- `crates/lightbridge-authz-usage/src/handlers/ingest.rs`,
`crates/lightbridge-authz-usage/src/repo.rs` — usage-event OTEL attribution
by `api_key_id`; ai-helm-values' `security-policies.yaml` already documents
that `x-api-key-id` currently lands as the literal `-` Envoy sentinel for
*every* row (nothing stamps the header today), so this particular consumer
is not live-broken by the claim reuse — it's separately broken/unwired.
- `crates/lightbridge-authz-api-key/src/repo.rs`,
`.../entities/api_key_validation_row.rs`, `crates/lightbridge-authz-core/src/dto.rs`
— real-API-key persistence/DTO layer; not exchange-path-reachable.
- `app/lightbridge-authz/src/mcp.rs` — sets `caller_kind: None` explicitly for
MCP's own auth path; not affected.
- Gateway side: `ai-helm-values` `environments/prod/values/security-policies.yaml`
— `lightbridgeintrospect`, `lightbridge-key-active`, `lightbridge-model-allowed`
`when` gates (all three), plus the `x-api-key-id`/`x-billing-plan`/
`x-project-id`/`x-project-role`/`x-quota-tier`/`x-project-quota` response-header
expressions, all of which branch on `has(auth.identity.api_key_id)` as their
plane discriminator today.
### Risks
- Breaking wire change: any external consumer that already inspects
`api_key_id` on an exchange-minted token (none known today outside this repo
and its gateway config, but not exhaustively verified beyond the two repos
above) would see it disappear or change shape.
- Coordinate timing with ai-helm-values#290's `azp`-based mitigation so the
gateway isn't left depending on a workaround indefinitely, but also so this
isn't rushed as a bundled fix to the live incident.
- Another session is actively working adjacent `oauth2_op`/RPC-authorize code
in this repo right now (recent commits: #383, #397, #379) — sequence after
checking for conflicts.
### Test Plan
- Extend `tests/token_exchange_tests.rs` to assert on whatever the new
claim shape is (not just `aud`/`azp` as it does today).
- Extend `crates/lightbridge-authz-rest/tests/budget_refill_procedure_tests.rs`
with a case that goes through a real token-exchange grant end-to-end, then
calls `request_budget_refill`, and asserts it succeeds (currently would
fail — that's bug B, unverified as fixed until this lands).
- Full `docs/rbac.md` "Internal/API-key-client refusal" section rewritten and
re-verified against the corrected code.
### Verification evidence
Bug A (gateway 403): verified by direct code read across
`signing.rs`/`oauth2_op/store.rs`/`handlers/introspect.rs`/`handlers/opa.rs`,
and cross-checked against the live `hetzner-prod` `kuadrant-policies-main`
`AuthConfig` (`kubectl get authconfig -o yaml`), which matches this repo's
`ai-helm-values` source exactly — see ai-helm-values#290's Verification
section for full command output.
Bug B (`requestBudgetRefill` refusal): verified by reading
`Procedures::request_budget_refill` (`crates/lightbridge-authz-rest/src/lib.rs:997-1002`)
against `access_token_extra`'s shared-caller-kind-stamping behavior, and
confirming via `budget_refill_procedure_tests.rs`/`token_exchange_tests.rs`
that no existing test exercises "a real token-exchange-minted token calling
`requestBudgetRefill`" — this is an assessment finding, not yet reproduced
against a running server or fixed with a failing-then-passing test.
### Human accountable owner
TBD — repo owner to assign.
### AI Usage Declaration
Understanding code, Proposing implementation, Drafting the ticket
### Human verification completed
_(none checked — this ticket is AI-drafted from direct code inspection and has
not yet had human review; opening for triage, not as reviewed/ready work.)_
コントリビューションガイド
調査の方向性
Read docs/adr/0011-*.md and the shared minting paths in crates/lightbridge-authz-rest/src/signing.rs and oauth2_op/store.rs; trace their claims into Procedures::request_budget_refill, handlers/introspect.rs, and handlers/opa.rs. Run tests/token_exchange_tests.rs and crates/lightbridge-authz-rest/tests/budget_refill_procedure_tests.rs, then review docs/rbac.md and the listed gateway consumers. Done means an agreed claim contract, updated consumers and documentation, and regression tests covering both token paths.
索引モデルが issue の本文から書いたものです。
評価
- 技術スタック
- rust
- 領域
- authentication, authorization, backend
- issue の種類
- バグ
- 難易度
- 5/5
- 見積もり時間
- 1週間以上
- 活発さ
- 活発
- 明瞭さ
- おおむね明確
- 初心者へのやさしさ
- 35/100