ADORSYS-GIS / ADORSYS-GIS/lightbridge-authz

api_key_id claim reused for token-exchange session id causes gateway 403 + wrong budget-refill refusal

未關閉
#421 0 則留言 0 個 reaction 已指派 0 人 在 GitHub 檢視
bug Prod ticket
主要語言
Rust
星號
0
分支
1
平均合併
7 小時 7 分鐘
30 天內合併 PR
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
預估耗時
一週以上
活躍度
活躍
描述清晰度
基本清楚
新手友好度
35/100

把新 issue 寄到你的電子郵件信箱

精選適合新手參與的 GitHub issue 摘要。