Repository navigation
feat(api): authenticate Claude Code to /v1/* with Entra ID SSO - #14665
diegosouzapw merged 14 commits into
Conversation
|
Thanks @BenjaminAronsson, this is a carefully built feature. The tid pinning, the non-forgeable auth stamp and the shadow-key bridge into the existing policy layer are exactly right, and the negative tests (wrong audience, wrong tenant, expired, unknown kid, missing oid) all pass locally. A few things before we can merge:
Also worth knowing: |
|
Re-homed to |
Developers sign in with Microsoft Entra ID instead of holding a static OmniRoute API key. Their Entra security groups map onto existing key groups, so the rate limits, model allowlists, budgets and combo permissions operators already configure apply per person, and usage is attributed per person in the call log. Static API keys keep working — this only adds a way to authenticate. Server: - verify access tokens against the tenant JWKS (jose), asserting tid as well as aud so a multi-tenant registration cannot mint a token another tenant's user can replay - map Entra group objectIds to key_groups, with an opt-in Microsoft Graph fallback for users whose groups claim overflows past 150 entries - provision a shadow api_keys row per Entra oid on first sight, so group policy and call-log attribution work with no SSO branches downstream - cache the verdict per token so group reconciliation runs about once per user per helper refresh rather than once per request The SSO branch resolves before validateApiKey and before the x-api-key extractor: Claude Code sends an apiKeyHelper credential in both Authorization and x-api-key, so without that ordering the policy layer would receive the raw JWT, resolve it to no key, and silently drop every limit the user is under — the GHSA-2phc-xp22-9f56 class. A bearer that presents itself as an Entra token always gets a final verdict, so an identity-provider outage cannot degrade to anonymous access. getApiKeys/getApiKeyById blank the secret of source='sso' rows. Exposing it would mint a permanent static credential that bypasses SSO and survives offboarding. Client: scripts/cli/omniroute-sso.mjs is dependency-free so it can be handed to developers who never install the omniroute CLI. `login` runs authorization code + PKCE on a loopback port; `token` prints the credential alone, as apiKeyHelper requires; `install` merges the block into ~/.claude/settings.json. Docs also correct a stale migration count (178 -> 182) that the counts gate blocks on; the llm.txt mirrors are regenerated by scripts/i18n/sync-llm-mirrors.mjs.
getApiKeyById redacts the secret of source='sso' rows, so the reveal endpoint answered 200 with an empty string and the dashboard silently copied nothing. Return 403 with an explanation instead. The accompanying test asserts a normal key still reveals at 200 in the same conditions — reveal is feature-gated behind ALLOW_API_KEY_REVEAL and returns 403 when disabled, so without that the SSO assertion would pass for the wrong reason.
Rebase onto release/v3.8.51 surfaced three problems. Migration 185 collided with 185_usage_history_cpa_auth_index from cheaper-inference#14544, which the migration runner rejects outright. Renumbered to 186; no retroactive isSchemaAlreadyApplied guard is needed because this migration has never shipped under the old number. The SSO branch claimed any RS256 JWT bearer even with SSO switched off, so a client sending its own unrelated JWT — extractBearer already notes VS Code Copilot does this — would hard-401 instead of degrading to anonymous under REQUIRE_API_KEY=false. It is now gated on isEntraSsoEnabled(), which answers false if the settings read throws so an unreadable config falls through to the pre-SSO path rather than 500-ing auth. The new test is RED against the previous commit. The migration count landed off by one: cheaper-inference#14559 had already moved the docs to 181 and the base to 182, and the rebase merged both edits cleanly into a number that no longer matched the 183 files on disk.
… SSO test The authority host was hardcoded to login.microsoftonline.com, so the feature could not work for a tenant on a sovereign cloud (login.microsoftonline.us, login.partner.microsoftonline.cn). It is now `entraAuthorityHost`, published through /api/auth/sso/config so the sign-in helper follows the server with no per-machine configuration. Plaintext http is refused unless the host is loopback: the authority decides which keys sign the tokens we trust, so an http authority reachable over the network would let anyone on the path mint accepted identities. The new end-to-end test runs the real helper as a subprocess against a stub authority that verifies PKCE the way Entra does, then feeds the resulting token to the real auth policy. It found two client bugs that the pure-function tests could not reach: - The login timeout was never cleared, so after printing "Signed in" the process kept the event loop alive for the full five minutes and looked hung. Login now takes 278ms instead of 300s. - openBrowser passed a missing binary to spawn, which reports failure through an async 'error' event rather than a throw. Without a listener Node raised it as uncaught and killed the login — on exactly the headless machines that have no opener to begin with.
… groups, pin token type and authority Review items 1-5 from cheaper-inference#14665. Migration: renumbered to 197. The review asked for 187, but release/v3.8.52 already carries 185, 186 and 187 (and up to 196), so the next free number is 197 and the docs count becomes 194 rather than 184. Group mapping no longer fails open. checkKeyModelAccess treats a key in zero groups as unrestricted, and addKeyToGroup swallows a bad id, so a mapping pointing at a typo'd, deleted or deactivated key group silently produced a shadow key with every model allowed. Group ids are now narrowed to those that exist AND are active (filterExistingActiveKeyGroupIds) before they can grant anything; resolving to none denies with AUTH_SSO_NO_GROUP; provisioning deactivates a key that still lands in zero groups; and PATCH /api/settings rejects a config referencing unknown or inactive groups. Three tests, all RED against the previous commit. Token type: `aud` plus a valid signature did not prove a user token — an id_token or an app-only client-credentials token from the same tenant carries the same audience. Now rejects idtyp=app, requires the delegated scp claim, and pins algorithms: ["RS256"]. The pin test signs PS256 with a key published in the same JWKS, so it fails for the right reason rather than key-not-found. Client helper: the authority, tenant, client id and scope are pinned into the local cache at login and refresh no longer re-reads /api/auth/sso/config, so a gateway compromised after enrolment cannot redirect the refresh grant. http is refused for both the base URL and the authority unless the host is loopback. Docs: corrected the claim that group mapping carries budgets and rate limits. It carries model allow/deny rules only — budgets, rate limits, usage caps, schedules and combo allowlists are per-key columns on api_keys, so SSO users get the defaults for those. Added the table and noted that /v1/models and /v1/embeddings re-validate the raw key in-handler and will 401 an SSO token.
bda25f5 to
c9b7172
Compare
|
Thanks — all five are addressed in 1. Migration collision. Renumbered, but to 197 rather than 187: 2. Group mapping failed open. Confirmed exactly as you described, and worse than a typo case:
Three tests cover the typo, the deactivated group, and a bad default. I reverted the fix and confirmed all three go RED before restoring it. 3. Token type. 4. Helper authority. Authority, tenant, client id and scope are now pinned into the cache at login, and the refresh path no longer reads 5. Docs. You're right, and the error was mine — I had it in the PR description too. Key groups carry model allow/deny rules only; budgets, rate limits, usage caps, schedules and combo allowlists are per-key columns on Also folded in while setting this up locally: the authority was hardcoded to Still outstanding and understood: the dashboard settings form (the i18n new-key gate wants all 66 locales), and no live-tenant run yet — everything here is verified against a stub authority. Happy to do the live validation if you want it before merge. And noted on the |
The suite sat in the always-on `test:integration` glob — the gate cheaper-inference#15306 reports as killed at its 2400s CI ceiling — and spawns the helper as a subprocess fourteen times. Renamed to `.int.test.ts` and self-skipping unless RUN_ENTRA_SSO_E2E=1, matching the RUN_SERVICES_INT and RUN_CONTRACT_INT suites, so it cannot be blamed for that timeout. Always-on coverage is unchanged: the unit suite already exercises the real jose path including createRemoteJWKSet, plus every negative case. What the env flag guards is the client round trip. Also adds a "Validation status" section to ENTRA_SSO.md that says plainly what is and is not proven. No token in any of these suites was minted by Microsoft, so consent behavior, real claim shapes, Conditional Access and the Graph overage path against live responses are unverified. Hard Rule cheaper-inference#18 names OAuth upstream flows as needing real-environment validation; this records that gap rather than leaving a reviewer to find it.
|
Rebased onto
What I need from you: CI has never run on this PR. Only semgrep (passing) and a skipping Mergify have reported, and The only Also opened #15428 separately: |
# Conflicts: # AGENTS.md # README.md # docs/i18n/am/llm.txt # docs/i18n/ar/llm.txt # docs/i18n/az/llm.txt # docs/i18n/bg/llm.txt # docs/i18n/bn/llm.txt # docs/i18n/bs/llm.txt # docs/i18n/cs/llm.txt # docs/i18n/da/llm.txt # docs/i18n/de/llm.txt # docs/i18n/el/llm.txt # docs/i18n/es/llm.txt # docs/i18n/et/llm.txt # docs/i18n/fa/llm.txt # docs/i18n/fi/llm.txt # docs/i18n/fr/llm.txt # docs/i18n/ga/llm.txt # docs/i18n/gu/llm.txt # docs/i18n/ha/llm.txt # docs/i18n/he/llm.txt # docs/i18n/hi/llm.txt # docs/i18n/hr/llm.txt # docs/i18n/hu/llm.txt # docs/i18n/hy/llm.txt # docs/i18n/id/llm.txt # docs/i18n/ig/llm.txt # docs/i18n/it/llm.txt # docs/i18n/ja/llm.txt # docs/i18n/ka/llm.txt # docs/i18n/km/llm.txt # docs/i18n/kn/llm.txt # docs/i18n/ko/llm.txt # docs/i18n/lt/llm.txt # docs/i18n/lv/llm.txt # docs/i18n/ml/llm.txt # docs/i18n/mr/llm.txt # docs/i18n/ms/llm.txt # docs/i18n/mt/llm.txt # docs/i18n/my/llm.txt # docs/i18n/ne/llm.txt # docs/i18n/nl/llm.txt # docs/i18n/no/llm.txt # docs/i18n/or/llm.txt # docs/i18n/pa/llm.txt # docs/i18n/phi/llm.txt # docs/i18n/pl/llm.txt # docs/i18n/pt-BR/llm.txt # docs/i18n/pt/llm.txt # docs/i18n/ro/llm.txt # docs/i18n/ru/llm.txt # docs/i18n/si/llm.txt # docs/i18n/sk/llm.txt # docs/i18n/sl/llm.txt # docs/i18n/sr/llm.txt # docs/i18n/sv/llm.txt # docs/i18n/sw/llm.txt # docs/i18n/ta/llm.txt # docs/i18n/te/llm.txt # docs/i18n/th/llm.txt # docs/i18n/tr/llm.txt # docs/i18n/uk-UA/llm.txt # docs/i18n/ur/llm.txt # docs/i18n/uz/llm.txt # docs/i18n/vi/llm.txt # docs/i18n/yo/llm.txt # docs/i18n/zh-CN/llm.txt # docs/i18n/zh-TW/llm.txt # llm.txt
Reconcile with the moved base tip (23a1148 -> 3def9ae): - Renumber the SSO migration 197 -> 206. The base added 197..205 since this branch was cut, so 197_entra_sso_identities.sql collided head-on with 197_proxy_subscription_core_config.sql. - Resolve the 69 docs-count conflicts (llm.txt, AGENTS.md, README.md and the 66 docs/i18n/<locale>/llm.txt mirrors) to the real count: 203 migrations. Base said 202, this branch said 194; both are stale now. Verified on the merged tree: typecheck:core, lint, check:docs-all, check:changelog-integrity all exit 0; entra-sso-auth 36/36, entra-sso-client-script 13/13, entra-sso-e2e 8/8 (RUN_ENTRA_SSO_E2E=1), api-key policy/lifecycle/access 70/70.
The PR head had its own merge of an older release/v3.8.52 tip (pushed outside this session). Merge it in rather than force-push, so no history is rewritten. Both sides independently renumbered the SSO migration to 206 — no conflict there. The 69 docs-count files conflicted (270 hunks, all count lines only); resolved to this branch's 203, which is the real count against the current base tip. The other side said 200 because it merged a base that predated migrations 204 and 205. Verified after resolution: typecheck:core, check:docs-all, check:docs-counts exit 0 (203 confirmed strict); entra-sso unit suites 49/49.
|
Hi @BenjaminAronsson — closing this pull request to clear the open queue. The branch |
|
Hi @BenjaminAronsson — reopening this pull request. The close was a mistake, and it is back in the merge queue. The branch |
|
Re-homed to |
Resolve the conflicts with release/v3.8.53 and fix what the merge would break: - Renumber the migration 206_entra_sso_identities.sql to 217: the release already ships 206_codex_wakeup_tasks.sql and runs up to 216, so boot threw a version collision. - The release moved the settings re-auth list into src/shared/constants/securityImpactingSettings.ts; the PR's local SECURITY_IMPACTING_KEYS is gone, so every entra* settings key now lives in the shared list (re-auth gated, operator-only, dropped by the JSON import). Pinned by a new test. - Move the Entra PATCH /api/settings checks into validateEntraSsoPatch(), next to the release's validateOidcPolicyPatch(), so PATCH does not grow; three new route tests cover the unknown-key-group, incomplete and no-group-policy rejections. - clientApi policy: run the SSO check on the highest-precedence credential from the release's extractApiKeyCandidates(); apiKeyPolicy: resolvePolicyApiKey() consults the SSO shadow key first; the key reveal route keeps the SSO 403 and adds the release's management-scope check. - Settings defaults and schema keep both the release's oidcAdmissionClaim and the entra* keys. - AGENTS.md, llm.txt and the docs/i18n mirrors are taken from the release: the PR's migration-count hunks are stale and the counts are refreshed after merge. README.md carries the migration count of this tree (214). Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
…with release 217) Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
1e78e24
into
cheaper-inference:release/v3.8.53
Summary
/v1/*with a short-lived Microsoft Entra ID token instead of a static OmniRoute API key. Their Entra security groups map onto existing key groups, and usage is attributed per person inusage_history.entraSsoEnableddefaults tofalse.jose, already a dependency) assertingtidas well asaud, group → key-group mapping with an opt-in Graph fallback for claim overage, and a just-in-time "shadow"api_keysrow per Entraoid. That row is the design:key_group_membershas a FK toapi_keys(id), so group rules, call-log attribution and the instantis_active=0kill-switch all work with no SSO branches downstream.scripts/cli/omniroute-sso.mjs, dependency-free (Node 18+) so it can be handed to developers who never install the omniroute CLI.login(auth code + PKCE on a loopback port),token(theapiKeyHelpertarget — prints the credential alone),install,logout.api_keys, so group membership does not confer them. An earlier revision of this description claimed otherwise.Related Issues
Validation
npm run lint— exit 0release/v3.8.52); focused checks rerun afterwardtests/unit/entra-sso-auth.test.tstests/unit/entra-sso-client-script.test.tstests/integration/entra-sso-e2e.int.test.ts(opt-in)RUN_ENTRA_SSO_E2E=1; skipped otherwisetypecheck:core·check:cycles:ratchet·check:docs-all·check:changelog-integrity· prettiertest:vitestInherited from the base tip, untouched by this PR: the unit/integration/package gate timeouts, the vitest
MCP audit shutdown > checkpointsfailure, and thecheck:ai-attributiongit-range error.tests/unit/stream-readiness-fallback.test.tsis also red on the base — fixed separately in #15428.Merge integrity (changelog + generated skills)is base-red, not this PR. The job fails oncheck:agent-skills-sync, which reports 20 driftedSKILL.mdfiles. Checking out the cleanrelease/v3.8.52tip (3def9ae) and runningnpm run check:agent-skills-syncthere reproduces the identical 20-skill list, byte for byte — this PR touches neitherdocs/openapi.yamlnorskills/, the generator's only two inputs. Per the base-red rule it is deliberately not fixed here; it needs its ownfix/release-v3.8.52-*PR. (check:changelog-integrity, the job's other gate, passes.)Tests Added Or Updated
tests/unit/entra-sso-auth.test.ts(new) — local RSA keypair + stubbed JWKS, so the realjosepath includingcreateRemoteJWKSetruns offline. Covers valid token, wrongaud, wrongtid, expired, signature not in the JWKS, missingoid,idtyp=app, missing delegatedscp, RS256 pin, group mapping, default-group fallback, group drift, overage with and without Graph, Graph unreachable → 503, JIT race → exactly one key, deactivation kill-switch, static-key coexistence, unrelated JWT while SSO is off, and the auth→policy bridge including the JWT arriving inx-api-key.tests/unit/entra-sso-client-script.test.ts(new) — PKCE derivation, token-freshness skew, settings merge.tests/integration/entra-sso-e2e.int.test.ts(new, opt-in) — the real helper driven as a subprocess against a stub authority that verifies PKCE the way Entra does; loopback callback, code exchange, refresh rotation, authority pinning,0600cache.Each of the fail-closed group-mapping tests and the RS256 pin test was confirmed RED against the preceding commit before the fix was restored.
Coverage Notes
The new unit suite covers every branch of
src/server/authz/entra/*, thesso_userbranch inclientApiPolicy,resolveSsoApiKeySecretinapiKeyPolicy.ts, and thesource='sso'redaction inapiKeys.ts. The client script's pure functions are covered unconditionally; its network and filesystem paths are covered by the opt-in e2e.Reviewer Notes
206. It has been renumbered twice as the base moved. It was197, which then collided head-on with the base's own197_proxy_subscription_core_config.sql;release/v3.8.52now tops out at 205. Docs migration count becomes 203 (the base's own line said 202 and was already stale against its tip).validateApiKeyand beforeextractUngatedClientApiKey: Claude Code sends theapiKeyHelpercredential in bothAuthorization: Bearerandx-api-key, so without that the policy layer receives the raw JWT and silently drops every limit — the GHSA-2phc-xp22-9f56 class. But the branch only claims a bearer while SSO is configured, so with SSO off a client's own JWT still degrades to anonymous underREQUIRE_API_KEY=false.checkKeyModelAccesstreats a key in zero groups as unrestricted, so a mapping pointing at a typo'd/deleted/inactive key group used to widen access. Ids are now narrowed to existing+active before granting, zero groups denies, provisioning deactivates a key that still lands in none, andPATCH /api/settingsrejects unknown groups.getApiKeys/getApiKeyByIdblank them forsource='sso', and/api/keys/[id]/reveal403s rather than returning"".AGENTS.md/llm.txtsign-off: the only change in those files is the migration count line, now202 → 203, forced by the strict docs-counts gate. No instruction text is modified.RUN_ENTRA_SSO_E2E=1) because it spawns the helper fourteen times and 🔴 Release branch not green: release/v3.8.52 #15306 reports the always-ontest:integrationgate as already killed at its ceiling.check-new-key-coverage, which wants all 66 locales); folding/v1/modelsand/v1/embeddingsonto the central auth decision — they re-validate the raw key in-handler and will 401 an SSO token.