[FEAT] OIDC relying party on the server and SDK login flow - #459
Open
justin13888 wants to merge 43 commits into
Open
justin13888 wants to merge 43 commits into
justin13888 wants to merge 43 commits into
Conversation
…ix/protocol-headers-every-route-404
…ix/protocol-headers-every-route-404
…rceptor pair The design puts three request headers and three response headers on every route; the server declared the request half on four upload operations and never sent the response half at all. A Kynos ApiError has no response-header seam, so routes/upload.rs rode X-Capsule-Protocol-Min/-Max as problem extension members while capsule-sdk reads them from headers and got None. The seam is on the interceptor. negotiation.rs adds two: Negotiation, mounted router-wide outside the body-size limit, attaches the window to every response the chain produces, errors and short-circuits included; ProtocolGate, on a Group, reads the three request headers and refuses 426 or 400 before the handler runs. Both read the one UploadPolicy window, which gains the advisory min_client_build. The group holds the four upload session operations that enforced the handshake per route until now; routes/upload.rs loses that duplication, and the 426 keeps no window members in its body. openapi::describe_negotiation_headers files the three response headers under every response of every operation, since Kynos describes an interceptor's headers on success responses only. The test fixture's client sends the handshake on every request with raw() for its absence, and conformance.rs gains a document census and a document-driven wire census that pin the gated and exempt sets. Refs #404
boot::assemble built the upload policy from UploadPolicy::default() regardless of PROTOCOL_MIN and PROTOCOL_MAX, so a deployment that narrowed its window published one range on /.well-known/capsule/server-info and enforced another on POST /v1/upload. The policy is now built from the configured window, which is also what the negotiation interceptors advertise and refuse against. The new boot test reads both back through the surface. Refs #404
Every gated operation requires X-Capsule-Protocol and refuses without it. The shared reqwest client behind the generated REST client now carries that header and X-Capsule-Crypto-Suite as defaults, from the build's own constants, so the generated operations and the hand-written paths over the same transport send them with no per-call argument. protocol_headers() is public so the SDK's other transports can carry the same handshake from the same source. Refs #404
The ProtocolGate group now holds every non-exempt operation; GET /v1/version,
the four /.well-known/capsule/* records, the three /s/{opaque_id}* reads and
the two /d/{opaque_id}* guest deposits stay on the router and carry only the
response headers. The census in tests/conformance.rs asserts the gated set is
exactly the complement of the pinned ten, the coverage walk produces the 426
and 400 every gated operation now declares, and one representative route per
module is refused before a credential or body is read. The boot tests that
register and sign in send the handshake, as every client does.
Refs #404
net::http_builder installs X-Capsule-Protocol and X-Capsule-Crypto-Suite as default headers once; http_client builds it and dial_client adds its connect timeout on top of the same builder. AuthClient::new, the sync consumer, the generated client behind AuthenticatedClient and the CLI's version probe all obtain their client there, so no SDK request reaches a gated route without the handshake. The document now declares X-Capsule-Protocol required on every gated operation, which puts the protocol date first in each generated signature; the SDK's callers pass the build's own constant, the same value the transport sends, and the sync consumer maps the 426 the feed can now answer. Refs #404
reqwest::ClientBuilder is already must_use; clippy's double_must_use refuses the second attribute under the repo's -D warnings. Refs #404
…eat/oidc-relying-party-407
The pending half of an OIDC authorization-code ceremony - the nonce the ID token must echo, the PKCE verifier the token endpoint must see, and the redirect URI that must be replayed verbatim - is a single-use, short-window credential keyed by `state`. That is the shape the typed ceremony stores exist for, so it becomes a fourth one beside the revoke-all challenge, the enrollment code and the relay channel rather than a field on the durable session store. `OidcAuthorizationStore::consume` is destructive on every attempt, like `ChallengeStore::consume`: a replayed `state` finds nothing, two callbacks racing one `state` resolve to one winner, and the nonce can never be checked twice. The TTL is the store's, fixed at ten minutes. `OidcState`, `OidcNonce`, `PkceVerifier` and `AuthorizationCode` join the secret identifier newtypes and redact themselves in `Debug`. The in-memory adapter joins `InMemoryStores`, and two conformance rows assert single use and expiry. The harness accessor for the new store is optional with a default of `None` so a container-backed harness written elsewhere keeps compiling until its adapter lands; `run_all` skips the two rows for such a harness and the rows themselves refuse to run against nothing.
… window threat-model/validation.md scopes the 426 to a write and says reads of any past version succeed. The gate is now two: ProtocolGate on the group of non-safe operations refuses a grammatical protocol date outside the window with 426, and ProtocolReadGate on the group of GET and HEAD operations admits any grammatical date and refuses only a missing or malformed handshake with 400. Because an interceptor's declaration is its type, a read now declares the 400 alone and no 426 it never renders; the census asserts exactly that, the coverage walk and the per-module table follow the split, the 413 loop and a new 401 case assert the window headers on refusals the gates did not make, and the feed's dead 426 arm leaves the SDK's sync consumer. Refs #404
`auth::oidc::claims::verify_id_token` takes the token, the key set it must verify under, what the relying party expects and the instant to judge time against, and returns either the identity the token establishes or the first check it failed. Nothing is fetched and no clock is read, so a foreign key, a wrong audience, an expired token and a replayed nonce are each a unit test against a key the test generated. `jsonwebtoken` is asked for the signature only. Its temporal checks read the system clock, which a test cannot move, and each claim check here - exact `iss` equality, `aud` containing the client, `azp` when present, `exp`/`nbf`/`iat` with a sixty-second skew, the nonce, a bounded `sub` - is a security decision this repository wants legible. The header's algorithm is allow-listed to RS256, ES256 and EdDSA before any key is consulted, a symmetric key in the set cannot verify anything, and a key published for one algorithm is not used under another. A header without `kid` resolves only against a set of exactly one key. `UnknownKey` is the one rejection a caller acts on: it is what asks the key cache to refetch.
Deploying capsule with
|
| Latest commit: |
f1ab7d2
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://4438422b.capsule-22k.pages.dev |
| Branch Preview URL: | https://feat-oidc-relying-party-407.capsule-22k.pages.dev |
Clippy's pedantic set flags the iter().any() spelling; the meaning is unchanged and the gate is what the workspace lint task runs.
PROTOCOL_MIN and PROTOCOL_MAX defaulted to the single day capsule-core speaks, collapsing the window a fresh deployment accepts to one date, and neither was checked for shape, so 2026-6-1 sorted before 2026-12-31 for the wrong reason. Both now default to the policy's year window, parse as strict YYYY-MM-DD dates, and may be equal. MIN_CLIENT_BUILD joins them, validated as MAJOR.MINOR.PATCH and handed to the upload policy, so the advisory X-Capsule-Min-Client-Build on every response is the operator's value rather than the crate default. With both validated at the boundary, an unencodable window value is a programming error and the response encoder says so. Refs #404
Every transport constructor that accepts a reqwest::Client names net::http_builder as its source; a client built any other way sends no protocol handshake and every gated route refuses it. Refs #404
Every gated route now refuses a request without X-Capsule-Protocol, and the hand-written web auth client sent none, so sign-in, registration, refresh and every authenticated call from the browser were refused. All five request builders send the protocol version this client is written against, restated from capsule_core::crypto::primitives::PROTOCOL_VERSION because the browser holds no Rust and the wasm surface does not export it. Refs #404
`auth::oidc::IdentityProvider` is the port the OIDC routes will drive: two methods - the URL to send a person to, and an identity for the code they come back with - because discovery is how both are answered rather than an operation of its own. It is the feature's one external boundary, so it is the one thing doubled; `Disabled` is the null object an unconfigured deployment runs with, so the routes have one shape whether or not `OIDC_ISSUER` is set. `HttpIdentityProvider` composes discovery (cached a day, refused if the document names another issuer or a plain-http endpoint off loopback), a key cache refetched on an unknown `kid` and floored at one fetch a minute, a form-encoded token exchange (HTTP Basic when a client secret is configured, PKCE-only otherwise) and the pure claim checks. Redirect URIs are client-supplied and allow-listed: the configured one exactly, plus loopback IP literals on any port by default (RFC 8252). `FederatedAccounts::resolve_or_create` maps `(issuer, subject)` to an account in one operation; an asserted address another account holds is `AddressTaken`, never a link. The in-memory adapter holds its own rows and says so - #460 owes the Postgres adapter over one account table. `Config` reads `OIDC_ISSUER`, `OIDC_CLIENT_ID`, `OIDC_CLIENT_SECRET`, `OIDC_REDIRECT_URL` and `OIDC_ALLOW_LOOPBACK_REDIRECT`, refusing half a relying party either way round; `boot::assemble` builds the relying party lazily so a provider that is down never stops a server booting, and refuses `OIDC_ISSUER` under the durable backends by name until #460 lands. `reqwest` is promoted to a workspace dependency and to `capsule-server` (rustls-tls, json); it was already in the lock file through the SDK, so this adds an edge and no crate. The test suite gains an in-process mock provider on loopback - discovery JSON, a JWK Set, a form-decoding token endpoint minting EdDSA ID tokens with a tamper per negative case - and drives the real adapter against it: caching, rotation, the refetch floor, a wrong verifier at the token endpoint, and every claim refusal.
api.ts is hand-written, so the header could silently go missing again. A recording mock of the global fetch drives the five request builders and checks X-Capsule-Protocol equals PROTOCOL_VERSION on each, and PROTOCOL_VERSION is compared with the literal in capsule-core/src/crypto/primitives.rs read at test time, so the restated constant cannot drift from its source of truth. Refs #404
`POST /v1/auth/oidc/authorize` begins a ceremony - fresh state, nonce
and PKCE material, the client's redirect URI admitted by the policy -
and answers the provider's authorization URL, the state and the
ceremony's deadline. `POST /v1/auth/oidc/callback` burns the state
first and whatever happens next, redeems the code through the provider
adapter, resolves the verified identity to an account keyed on
`(issuer, subject)`, honours a confirmed second factor with the same
`202` the password path issues, and opens the session through the same
`open_session_for`. Both mount inside the `ProtocolGate` group, as
every other non-safe operation does.
Every ID-token refusal is one code on the wire,
`error.auth.oidc_token_invalid`, with the specific reason in the log,
so the callback is not an oracle over which checks the relying party
runs. A burned, expired or unknown state is one code. An asserted
address another account holds is `409 error.auth.oidc_address_taken`
and never a link. The callback declares no `404`: an unconfigured
deployment holds no pending ceremony, so it answers the state code.
`server-info` gains `auth.oidc: { authorize, callback } | null` -
endpoints only, never the issuer or client id - which is how a login
chooser decides whether to offer the path. Seven `error.auth.oidc_*`
keys join the catalog and the OpenAPI document is regenerated with the
two operations.
The route suite drives the fixture's provider double for every declared
response, and the real adapter against the mock provider for the whole
handshake and every tamper through the route.
`AuthClient::begin_oidc_login(redirect_uri)` posts the client's own callback URI to `/v1/auth/oidc/authorize` and returns where to send the person plus the `state` the provider's redirect will echo; `complete_oidc_login(state, code)` posts the redirect's answer to `/v1/auth/oidc/callback` and returns the same `LoginOutcome` a password login does, so a second-factor challenge on a federated account reaches the caller as a value rather than a malformed pair. The browser leg between the two is the platform's - the CLI's loopback listener and the device grant are #461. The configured cohort hash rides the callback and not the authorize, because the callback is the request that opens the session. The `200` and `202` reading is shared with the password login rather than duplicated. `AuthError` gains the OIDC refusals, matched on the catalog code: `OidcNotConfigured`, `OidcRedirectInvalid`, `OidcAddressTaken`, and one `OidcRejected { code }` for the callback's three `401`s, every one of which means "start again". Proven against the crate's in-process mock server; the round trip against the real router lives in `capsule-server/tests/oidc.rs`.
…t provider `authentication.md` gains the section the design did not carry before the relying party was written: the two-request ceremony, the client-supplied and allow-listed redirect URI, the burned state, one wire code for every ID-token refusal, lazy discovery with the issuer mix-up defence, accounts keyed on `(issuer, subject)` and never linked by address, the second factor honoured, `openid email` with no `profile` scope, `auth.oidc` in `server-info`, the five variables, and the two deviations named rather than substituted. `dependencies.md` widens the HTTP-client row to the server's one egress and adds the OIDC row with the priced rejection of `openidconnect`. `capsule-server/compose.yaml` gains a dex service behind `--profile oidc` with a public `capsule` client and inline configuration, so a development server can be pointed at a real provider on loopback. `SLICES.md`: `S-N1` is done with its deviations recorded; `S-N2` is part - the SDK half landed, the CLI remainder is #461.
`every_declared_response_is_exercised` asserts, through one client, that every response the document promises was produced; the two OIDC operations added fourteen. Both paths join the body-size list, and an `oidc_block` after the second factor's drives the rest through the fixture's provider double and its switches: the extractor's 415/422 on both, the authorize's 200/404/500 (provider)/500 (store), and the callback's 401/500/200/409/202 - the 202 by enrolling a factor on the federated account the walk just created. The 400 and 426 are the gate's and already come from the document walk. Additive, in its own block, so a sibling adding its own block to the same walk merges mechanically.
Review round 1 on #459. An ID token with a non-empty `crit` header is refused (RFC 7515 §4.1.11: extensions the verifier must understand, and it implements none), and a token whose `aud` names several parties must carry `azp` (OpenID Connect Core §3.1.3.7 rule 4). The provider-supplied strings that reach a log line - the token's `iss` and `kid`, the token endpoint's `error` and `error_description`, a refused endpoint URL - are bounded at 255 bytes at construction. Under the loopback-issuer carve-out every plain-http endpoint must itself be loopback, so a provider on this machine cannot send the code and verifier off-box in the clear; `localhost` is no longer loopback here, for the reason the redirect policy refuses it (RFC 8252 §8.3). The key cache gains a one-hour ceiling beside the evidence-driven refetch and its floor: a key the provider revoked never produces the unknown-kid evidence, so without a ceiling it would verify until the next rotation. Past the ceiling a failed fetch is returned rather than served stale. The mock provider gains two wire tampers - a signature flipped under a published kid, and a symmetric key published under a kid a token then names - so those refusals are proven over the wire rather than only in the validator's unit tests; its tokens live three hours so the ceiling can be exercised against a clock the tests move.
Review round 1 on #459 (F1). `POST /v1/auth/oidc/authorize` is an unauthenticated write into a store, so it is bounded twice. A counter budget per redirect host - sixty a minute, `budgets::OIDC_AUTHORIZE`, keyed on the one fact the request carries that an attacker cannot vary freely, since the policy admits three hosts at most - answers `429 error.auth.rate_limited` before anything is generated or asked of the provider. The in-memory ceremony store purges expired records on every `begin`, as the other in-memory stores do, and refuses at a ceiling of ten thousand pending ceremonies with `StoreError::Rejected`, which the route renders as `503` under the existing retryable `error.auth.unavailable` code: the remedy is the one that code already tells a person. The Valkey adapter (#460) gets expiry from the TTL. The conformance walk and the route suite produce both new responses; the OpenAPI document is regenerated.
…ied it Review round 1 on #459 (decision 11). An address a provider asserts without `email_verified` is one anybody at that provider could have typed; reserving it would let a person register somebody else's address there, unverified, and hold the real owner out of signing in here. So an unverified address neither reserves nor collides: it is carried on the identity and otherwise ignored, which makes `email_verified` the claim's one production reader. Interim until #460 folds federated rows into the one account table, where the address is the verified one. The fixture double applies the same rule and now says it encodes #460's contract rather than shipped behaviour. `open_session_for` records why the OIDC door does not consult the password lockout (decision 15): the lockout counts failed credential presentations, a federated sign-in presents none, and refusing there would let a password-guesser lock a person out of single sign-on.
… to the scheme rule Review round 1 on #459 (decision 14, F8, F11). `OIDC_ALLOW_LOOPBACK_REDIRECT` now defaults to off: admitting a redirect to any loopback port is what a CLI's listener needs and is the one knob that widens where the server sends a person back to, so a deployment with such a client turns it on (#461's CLI flow will). The dex comment in the compose stack sets it. `OIDC_REDIRECT_URL` is validated as the issuer is - https, or http on a loopback literal - and a broken line continuation in the issuer fault's message is repaired. The callback body refuses unknown fields, like the authorize's, so a client that forwards the provider's whole redirect query gets a 422 naming the field; the OpenAPI document is regenerated.
Review round 1 on #459 (decision 18, F7). `OIDC_CA_BUNDLE` names a PEM bundle of additional trust anchors for reaching a provider behind a private CA - the ordinary enterprise case. The path is configuration and its contents are not: `Config` carries the path, `boot::assemble` reads and parses it once, refusing by name (never by content) when it is missing, not a certificate bundle, or empty, and hands the roots to the relying party's HTTP client through `add_root_certificate` - added to the public roots, never replacing them. Proven over a real handshake: the mock provider gains a TLS variant behind a private CA and a leaf it signed for `127.0.0.1`. Without the bundle the relying party reports the provider unavailable before a byte reaches it; with it the whole handshake round-trips. `rcgen`, `rustls` and `tokio-rustls` join the server's dev-dependencies with the SDK's exact versions and features, so nothing new enters the lock file. The bad-signature tamper now flips the first signature character rather than the last, whose padding bits made the flip sometimes malformed instead of mis-signed.
Review round 1 on #459 (decision 16). The `auth.rs` exemption from the generated client is for token orchestration - the pre-flight refresh, the 401-retry-once replay, the session store - and the OIDC legs are none of that. `begin_oidc_login` and `complete_oidc_login` now call the spargen-generated `rest::Client` over the SDK's one transport; the three hand-written body types are gone, and every body and every response, the 202 challenge included, is parsed by generated code. What stays hand-written is the mapping into `LoginOutcome` and `AuthError`, which now shares one status-to-variant table with the password path. The generated client is built on the server root, derived from the auth base by removing its `/v1/auth` suffix. Its transport-class failures are not `reqwest::Error`s, so they surface as a new `AuthError::Network` naming the endpoint. The in-crate mock now serves the generated paths and answers refusals as RFC 9457 problems carrying the code, which is what the generated client parses.
…settings Review round 1 on #459. `authentication.md` now states the loopback redirect arm as opt-in, the two bounds on beginning a ceremony, the loopback carve-out's endpoint rule, the key cache's one-hour ceiling beside its floor, `OIDC_CA_BUNDLE`, the verified-address rule for the 409, the lockout the OIDC door does not consult, and - named rather than substituted - the two account-table properties #460 owes rather than this change ships. `.env.example` gains the single sign-on section with all six settings, and `dependencies.md` records the server's TLS dev-dependencies for the private-CA test.
Review round 2 on #459 (decision 19, finding F-A). The authorize computed `CounterKey::OidcAuthorize(host)` from the caller-supplied `redirect_uri` before the redirect policy had ever looked at it - the policy runs inside `authorization_url()`, one step later. An unauthenticated caller looping the route with a fresh host each time therefore got a 400 every time, wrote nothing to the ceremony store, and still added one permanent row per request to the counter store, which is process-wide and shared with the login and second-factor limiters. `IdentityProvider` gains `admits_redirect`, the pure look-ahead over the same `RedirectPolicy` the adapter enforces with, so the route can pick its key from the policy's verdict: an admitted redirect keys on its host - three at most, which is what the module's doc comment always claimed - and every refusal shares the new unit key `OidcAuthorizeRefused` on its own deployment-wide budget. Refusals stay rate-limited; the key space is the small constant it was documented to be. The doc comments on the key and on the budget now say when the claim holds. Pinned by a test that fires sixty distinct invalid hosts and asserts one counter key afterwards, that the sixty-first refusal is a 429, and that the admitted host is a separate bucket the flood cannot deny. Without the reordering it observes sixty keys.
Review round 2 on #459 (decision 20). `InMemoryCounters` held every key it had ever seen: a window whose budget had lapsed decided nothing but kept its row, and no ceiling stopped the map. That is the facility every limiter on the surface shares, and several of its keys are derived from something a caller sent, so the growth was never one feature's problem. The same two bounds `InMemoryOidcAuthorizations` carries. Lapsed windows are purged on every `hit`, from a per-window purge hint recorded off the budget in force when the window opened - a hint only, so admission is still recomputed from `opened_at` against the budget the caller supplies and re-tuning a budget behaves exactly as it did. Past `WINDOW_CEILING` a key with no window yet is refused with `StoreError::Rejected`, while every key that already holds one keeps counting: a flood of new keys must not switch off a limiter that is already tracking somebody. Callers already treat a counter error as a refusal, so a full store fails closed through the path documented to do that rather than through a new one. Decision 19 is what bounds the OIDC key at its call site; this is the defence in depth beneath it, and the module doc says so rather than leaving the ceiling to be read as a licence to key a counter on an arbitrary string.
Review round 2 on #459 (decision 21). `error.auth.unavailable` carried both the 503 the authorize answers when the pending-ceremony store is at its ceiling and the 500 it answers when a store could not be reached at all. design/api-surfaces.md is explicit that REST status is coarse, that the stable error code is the precise discriminator, and that clients switch on the code and never on status alone - so two conditions with different statuses sharing one code leaves a conforming client unable to tell them apart at all. `error.auth.oidc_at_capacity` is the 503's own code. The remedies differ as well as the conditions: at capacity is a short timer, because pending ceremonies expire ten minutes after they start, while unavailable is an outage to surface. The catalog entry says which is which, and the wire test now asserts the two codes are the two conditions.
Review round 3 on #459 (decision 23). The redemption limiter keyed on `request.code.trim()` with no shape check at all, where the share and drop paths at least gate on `is_opaque_id` first. The presented value is an arbitrary caller-supplied string of arbitrary length, so an unauthenticated caller could fill the counter store a row at a time - the same lever decision 19 removed from the OIDC authorize. `is_enrollment_code` admits exactly the two spellings `mint` issues: a canonical hyphenated UUID, or eight ASCII digits. A code of that shape keeps its own per-code budget, which is what the entropy argument for the short transcribable fallback rests on; anything else is charged to `CounterKey::EnrollmentRedemptionMalformed`, one fixed bucket on a far more generous budget, because a malformed string cannot match a pending enrollment and so is not a guess at one. A shape check is not an existence check and must not become one. It reads only the presented string, never the store, so it cannot tell a pending code from an absent one and tells a prober nothing. The charge-before-resolve ordering that stops this route being a free existence oracle is untouched, and a malformed code still walks the same path to the same `error.enrollment.code_refused` it always did. Three tests spent the *per-code* budget with placeholder strings that are not shaped like codes - two in `tests/enroll.rs` and the conformance walk's 429 producer. All three now grind a well-formed guess, which is what the property was always about, and a new wire test pins that malformed attempts share one bucket and cannot deny a real redemption. `capsule-server/tests/enroll.rs` is a manifest widening the lane raises rather than assumes - see the decision record.
Review round 3 on #459 (decision 22). The single `WINDOW_CEILING` bought a memory bound at the cost of a shared fate. `InMemoryCounters` is the only `CounterStore` and is instantiated once per process, and three surfaces charge a caller-controlled key before resolving what it names - the share path, the drop path and the enrollment redemption. The drop path's window is an hour long, which makes it the cheapest to hold saturated: roughly six fabricated but well-formed ids a second held the whole map full. Saturated, `hit` refused a *new* key from *any* surface, and every route maps a counter error fail-closed to a 500 or 503 - so a first-time share view, a first enrollment redemption after a reboot and the first OIDC sign-in of the day could all be denied by a flood against the weakest of the four. The store now holds one partition per `CounterKey` variant, each with its own `CounterKey::ceiling` sized from that variant's window and its own plausible rate of distinct keys; `counter::ceilings` carries the arithmetic for every number. Filling one partition refuses new keys in that partition only. The totals are close to the single ceiling they replace, deliberately: the point is not to hold more windows, it is that the windows one surface holds are not the windows another is denied. `with_ceiling` now bounds every partition equally, which is what lets a test observe the partition boundary without writing twenty thousand keys. Pinned by a test that fills the drop path, then asserts a never-seen share link, OIDC host, enrollment code and second-factor challenge are all still admitted and that the flooded partition's own keys keep counting. Under one shared ceiling that test observes the share link denied.
…pe check Review round 3 on #459 (decisions 22 and 23). The authentication design said the counter store "holds a ceiling of its own", which after this round is one ceiling per key kind rather than one for everything - and the difference is the whole finding: a single shared ceiling let a flood against the cheapest caller-keyed surface deny a first-time key to every other, single sign-on included. The bullet now says that, and names the enrollment redemption's shape check as the same reasoning the redirect check here already carries: a shape check is not an existence check, so it bounds the key without reopening the oracle the charge-before-resolve ordering exists to prevent.
Review round 3 on #459 (decision 22). The cross-surface test bounds every partition with the uniform `with_ceiling` override so the boundary is legible at fifty keys rather than twenty thousand. That proves partitions are independent, but it does not spend the number a deployment actually runs with, so an edit that sized `DropLink` off another variant's constant would pass it. This one fills `DropLink` to `ceilings::DROP_LINK` itself, then asserts the two keys the finding named - a never-seen share link and a never-seen OIDC redirect host - are still admitted, and that the flooded partition keeps counting the keys it already holds. Ten seconds against a sixty-second slow threshold.
Review round 4 on #459 (decision 25). The drop, share and enrollment routes mapped *any* `Err` from `CounterContext::hit` to their generic `Unavailable` variant, so `StoreError::Rejected` - the partition ceiling refusing a new key, by design - surfaced as a 500 alongside a genuinely broken store. Decision 22 stopped a flood against one caller-keyed surface reaching the others; it did not change what the flooded surface's own new visitors are told, and `DropLink` is saturable at under six fabricated ids a second. While it is held, every first-time visitor to every drop link was told the server was broken, and neither a client nor an operator paged on 5xx could tell that from a real incident. The only signal was a server-side WARN. Ceiling exhaustion now leaves through the 429 path with a code of its own - `error.drop.at_capacity`, `error.share.at_capacity`, `error.enrollment.at_capacity` - and a `retry_after` bounded by that partition's own window, which is an upper bound because the earliest live window lapses no later than one window after it opened. Only a store that could not answer at all is still a 500. The refusal is fail-closed either way; what changed is what the caller is told. `counter::{is_at_capacity, capacity_retry_after}` and `CounterContext::capacity_refusal` keep the classification in one place, so three routes cannot answer differently, and `hit` now refuses under the `COUNTER_STORE` constant `is_at_capacity` matches on rather than a repeated literal. The spent-budget 429 also gains `retry_after`, taken from the `Verdict::Limited` every route was discarding. The module docs say the consequence out loud beside the ceiling arithmetic: partitioning bounds the blast radius and does not make the flooded surface well, and a per-source key is what would. Three EXTRAS rows describe `retry_after` on the five throttled operations, as that table requires for any non-`code` extension member.
Review round 4 on #459 (decision 28). `is_at_capacity` separates "the partition is full" from "the store is broken" by matching `StoreError::Rejected { store: COUNTER_STORE, .. }`. That is safe by construction today because `InMemoryCounters::hit`'s only error path is the ceiling, so no route can misclassify - and it stops being safe by construction the moment a second `CounterStore` exists. `COUNTER_STORE` is public, and a Valkey adapter (#460) refusing under that name for an unrelated reason would have a real outage rendered `429 error.*_at_capacity`: a caller told to retry a store that is down and an operator told nothing is wrong, which is worse than the 500-for-everything decision 25 replaced. Two tests, against the adapter that does not exist yet: the predicate is false for `Unavailable` and for a `Rejected` from any other store, and `capacity_refusal` - the method the routes actually call - answers `None` for both while still offering the one-window deadline for the ceiling. Dropping the store check from the match fails both.
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.
Description
OIDC relying party on the server (S-N1) and the SDK half of the OIDC login flow (S-N2): authorization code + PKCE against an external identity provider, discovery and JWKS handling, ID-token claim checks, a typed ceremony store for the pending authorization, account resolution keyed on
(issuer, subject), and the two routes that drive it, mounted inside the protocol gate. Stacked on #435 (W-SERVE) with #453 (W-HEADERS) merged in.Every slice of the plan has landed, review rounds 1 through 4 are answered, and every gate is green at head
f1ab7d2a.Related Issues
Closes #407. Defers the durable adapters to #460 and the CLI remainder of S-N2 to #461.
Contributor Checklist
Summary
Commit plan (Conventional Commits, each leaving the tree coherent):
b63cf556feat(server): add the OIDC authorization ceremony store—OidcState/OidcNonce/PkceVerifier/AuthorizationCodesecret ids, theOidcAuthorizationStoreport withPendingAuthorizationand a fixed ten-minute TTL, the in-memory adapter, two conformance rows (single-use consume, expiry with the store).302da2f0+671f83e6feat(server): verify OIDC ID tokens as a pure function—auth::oidc::claims::verify_id_token, signature byjsonwebtoken, every claim check Capsule's, judged against an injected instant; eighteen negative cases.c21188d9feat(server): add the OIDC identity-provider port and its HTTP adapter—IdentityProvider(two methods) withHttpIdentityProviderand theDisablednull object; discovery cached a day with the issuer mix-up defence; JWKS refetched on an unknownkid, floored at one a minute; form-encoded exchange (Basic when a secret is configured);RedirectPolicy;FederatedAccountswithInMemoryFederatedAccounts;OIDC_*config; boot wiring (lazy, durable refusal by name);tests/support/idp.rsmock provider; nine wire tests.c27e86b1feat(server): mount the OIDC authorize and callback routes—POST /v1/auth/oidc/{authorize,callback}insideProtocolGate; sevenerror.auth.oidc_*keys andmise run i18n;server-infoauth.oidc | null;openapi.jsonregenerated (+2 operations); twelve route tests over the double and the real adapter.2c41ec05feat(sdk): begin and complete an OIDC login—AuthClient::begin_oidc_login/complete_oidc_login,OidcAuthorization, typedAuthErrorrefusals on the catalog codes.73977f05docs(design): record the OIDC relying party and ship a dex development provider—authentication.mdsection,dependencies.mdrows, dex under--profile oidcincapsule-server/compose.yaml,SLICES.mdS-N1done*/ S-N2part.c080a44ftest(server): walk every declared response of the OIDC operations— both paths in the conformance walk's 413 list and an additiveoidc_blockproducing the fourteen declared responses (decision 10).Review round 1 on #459 added seven commits, each one routed decision or hardening finding:
273f42f9fix(server): harden the OIDC token and discovery checks— F3 (the loopback carve-out no longer admits a plain-HTTP endpoint under a non-loopback issuer, andlocalhostis not silently admitted anywhere:is_loopback_issuerdemandshttpplus an IP literal), F6 (iss,kidand the token endpoint'serror_descriptionbounded at 255 bytes before they reach a log line or an error), F9 (a non-emptycritheader is refused), F10 (a multi-audtoken withoutazpis refused, Core §3.1.3.7 rule 4), decision 12 (the JWKS cache gains a one-hourMAX_AGEceiling beside its one-minute refetch floor).91cf7a0cfeat(server): bound the OIDC authorize by budget and by store ceiling— decision 17:budgets::OIDC_AUTHORIZE(60/min per redirect host,CounterKey::OidcAuthorize, theS-C32limiter) answers429before anything is generated;InMemoryOidcAuthorizationspurges expired records on everybeginand refuses pastPENDING_AUTHORIZATION_CEILING(10 000) with a503; both new responses declared and walked.6512e70afix(server): reserve a federated address only when the provider verified it— decision 11 (an address asserted withoutemail_verifiedneither reserves nor collides) and decision 15 (the lockout the OIDC door does not consult, recorded onopen_session_for).d294ad76fix(server): make loopback redirects opt-in and hold the redirect URL to the scheme rule— decision 14 (OIDC_ALLOW_LOOPBACK_REDIRECTdefaults off), F11 (the broken line continuation in the issuer fault's message repaired;OIDC_REDIRECT_URLheld to the issuer's scheme rule), F8 (deny_unknown_fieldson the callback body; document regenerated).d73f235cfeat(server): trust a private CA for the identity provider— decision 18:OIDC_CA_BUNDLE, a PEM path read at boot into the relying party'sreqwestclient, refused by name when unusable.5e4d2e8arefactor(sdk): drive the OIDC login through the generated client— decision 16: both legs go through Spargen'srest::client and its typed error enums; the hand-written request building and response parsing is gone.e54cfb59docs(design): record the round-1 OIDC decisions and document the six settings— decision 13:authentication.mdnames server: Valkey adapter for OidcAuthorizationStore and Postgres adapter for FederatedAccounts #460 for the durable gap in its own bullet and states the two properties the fixture double encodes rather than the server ships;.env.examplegains the six settings.Merges:
8afeb9ec(#435 at459e8af3),1d471635(#453 at7526395b),e977c096(#453 at892f7575),62376d81(#453 final at032b6af2).Review round 2 confirmed every round-1 citation and left one High finding, delivered as three commits:
36c73417fix(server): charge the OIDC authorize budget under a bounded key— decision 19, finding F-A.d5b0afe5fix(server): purge and bound the in-memory counter windows— decision 20.739845d9feat(server): give the OIDC at-capacity refusal its own code— decision 21.Review round 3 confirmed decisions 19–21 at their cited lines and left one Medium, cross-surface finding, delivered as four commits:
856e7313fix(server): bound the enrollment redemption key before charging it— decision 23.04192ea0fix(server): give each counter key its own ceiling— decision 22.1a5b9e93docs(design): record the partitioned counter ceiling and the code shape check.c2a59e8atest(server): fill the drop partition to its shipped ceiling— decision 22's second test.Review round 4 confirmed decisions 22 and 23 and left one Medium finding, delivered as two commits:
47ed85a2fix(server): tell a caller a full limiter partition is not an outage— decision 25.b9e5b8ecdocs(server): name the issue the shared counter lock is deferred to— decision 27.Review round 4's follow-up closed the lane with one commit:
f1ab7d2atest(server): pin that only the ceiling reads as a capacity refusal— decision 28. Decision 29 is a record correction and changes no code.Validation
All run inside the worktree, in the foreground. Aggregate gates at head
c080a44f; the focused checks at the commit that introduced them.cargo nextest run -p capsule-server --lib store::cargo nextest run -p capsule-server --lib oidc::config:: boot::cargo nextest run -p capsule-server --test oidccargo nextest run -p capsule-sdk --lib auth::mise run i18n-checkmise run openapi-check-kynosmise run check-docs-truthmise run check-mdmise run check-docsmise run check-rustcargo nextest run -p capsule-server --test conformance(atc080a44f)mise run test-rust(atc080a44f)cargo nextest run --workspace1888 run, 1888 passed;-p capsule-core --features ffi729 passed;-p capsule-sdk --features ffi165 passedmise run test-rust(at73977f05, before the conformance block)every_declared_response_is_exercisedlisted the 14 declared responses of the two new operations its walk never producedcaused, resolved byc080a44fonce the orchestrator widened the manifest (decision 10)podman compose -f capsule-server/compose.yaml --profile oidc configconfigs.dex_config.contentis honouredcargo nextest listcounts after each test-touching commitoidc) → 21 (oidc); never droppedcargo nextest run -p capsule-server --lib oidc:: config:: store:: counter:: boot::(ate54cfb59)cargo nextest run -p capsule-server --test oidc --test conformance(ate54cfb59)cargo nextest run -p capsule-sdk --lib auth::(ate54cfb59)mise run format-check-rust(ate54cfb59)mise run lint-check-rust(ate54cfb59)mise run build-rust(ate54cfb59)mise run doc-check-rust(ate54cfb59)mise run i18n-check(ate54cfb59)mise run i18n-guard(ate54cfb59)mise run openapi-check-kynos(ate54cfb59)mise run architecture-check(ate54cfb59)mise run license-check(ate54cfb59)mise run translate-readme-check(ate54cfb59)mise run lint-check-ffi(ate54cfb59)mise run verify-examples(ate54cfb59)mise run build-check-wasm(ate54cfb59)mise run build-ffi(ate54cfb59)mise run gen-bindings(ate54cfb59,CARGO_TARGET_DIRunset — the task hardcodestarget/)mise run test-rust(ate54cfb59)capsule-core --features ffi), 166 passed (capsule-sdk --features ffi)mise run check-docs-truth(ate54cfb59)mise run check-md(ate54cfb59)mise run check-docs(ate54cfb59)mise run cli-surface-checkmise.toml;check-ruston this base does not aggregate itunavailablecargo nextest run -p capsule-server --test oidc --test conformance(at739845d9)cargo nextest run -p capsule-server --lib counter::(at739845d9)cargo nextest run -p capsule-server(atd5b0afe5)admits_redirectshort-circuited totrue)mise run format-check-rust(at739845d9)mise run lint-check-rust(at739845d9)mise run build-rust(at739845d9)mise run doc-check-rust(at739845d9)mise run i18n-check(at739845d9)mise run i18n-guard(at739845d9)error.auth.oidc_at_capacityis reported as a catalog code no Swift screen names — non-fatal by that gate's own asymmetry, and correct: it localizes throughErrorCode.unknown(_)and no client-side affordance is owed for itmise run openapi-check-kynos(at739845d9)problemextension value, not a schema member, so the contract is unchanged)mise run architecture-check(at739845d9)mise run license-check(at739845d9)mise run build-check-wasm/build-ffi/lint-check-ffi/translate-readme-check/verify-examples(at739845d9)mise run gen-bindings(at739845d9,CARGO_TARGET_DIRunset)mise run test-rust(at739845d9)capsule-core --features ffi), 166 passed (capsule-sdk --features ffi)mise run check-docs-truth(at739845d9)mise run check-md(at739845d9)mise run check-docs(at739845d9)mise run cli-surface-checkmise.tomlunavailable— nothing substitutedcargo nextest run -p capsule-server --lib counter::(atf1ab7d2a)is_at_capacitymatching anyRejected)Rejected { store: "something-else" }read as a capacity refusalmise run format-check-rust/lint-check-rust(atf1ab7d2a)mise run test-rust(atf1ab7d2a)capsule-core --features ffi), 166 passed (capsule-sdk --features ffi)cargo nextest run -p capsule-server --lib counter:: enroll:: oidc::(atc2a59e8a)cargo nextest run -p capsule-server --test enroll --test share --test drops --test oidc --test conformance --test limits(atc2a59e8a)share_linkdenied by a fulldrop_linkpartitionmise run format-check-rust(atc2a59e8a)mise run lint-check-rust(atc2a59e8a)mise run doc-check-rust(atc2a59e8a)mise run i18n-check(atc2a59e8a)mise run i18n-guard(atc2a59e8a)mise run openapi-check-kynos(atc2a59e8a)mise run architecture-check(atc2a59e8a)mise run license-check(atc2a59e8a)mise run translate-readme-check(atc2a59e8a)mise run build-rust/build-check-wasm/build-ffi/lint-check-ffi/verify-examples(atc2a59e8a)mise run gen-bindings(atc2a59e8a,CARGO_TARGET_DIRunset)mise run test-rust(atc2a59e8a)capsule-core --features ffi), 166 passed (capsule-sdk --features ffi)mise run check-docs-truth(atc2a59e8a)mise run check-md(atc2a59e8a)mise run check-docs(atc2a59e8a)mise run cli-surface-checkmise.toml(mise tasks ls --allmatches nothing)unavailable— nothing substitutedcargo nextest run -p capsule-server --lib counter:: enroll:: oidc:: openapi::(atb9e5b8ec)cargo nextest run -p capsule-server --test enroll --test share --test drops --test oidc --test conformance --test limits --test problem(atb9e5b8ec)capacity_refusalforced toNone)mise run format-check-rust(atb9e5b8ec)mise run lint-check-rust(atb9e5b8ec)mise run doc-check-rust(atb9e5b8ec)mise run i18n-check(atb9e5b8ec)mise run i18n-guard(atb9e5b8ec)*_at_capacitycodes are reported as catalog codes no Swift screen names, which that gate treats as non-fatal by designmise run openapi-check-kynos(atb9e5b8ec)retry_afteron five throttled operations)mise run architecture-check(atb9e5b8ec)mise run license-check(atb9e5b8ec)mise run translate-readme-check(atb9e5b8ec)mise run build-rust/build-check-wasm/build-ffi/lint-check-ffi/verify-examples(atb9e5b8ec)mise run gen-bindings(atb9e5b8ec,CARGO_TARGET_DIRunset)mise run test-rust(atb9e5b8ec)capsule-core --features ffi), 166 passed (capsule-sdk --features ffi)mise run check-docs-truth(atb9e5b8ec)mise run check-md(atb9e5b8ec)mise run check-docs(atb9e5b8ec)mise run format-check-web(first invocation)bunxwas resolving and downloading biome on first use; passed on immediate re-run with no change to the treeflaky(first-run dependency fetch)mise run check-web→build-webrsbuild: command not found;capsule-web/node_modulesis absent in this worktree, so the web build has never been installable here. Its siblingsformat-check-web,lint-check-web,build-wasmandshare-katall pass. Not run withCARGO_TARGET_DIRset —build-wasmhardcodestarget/, likegen-bindingsunavailable(environment); nothing substitutedmise run cli-surface-checkmise.tomlunavailable— nothing substitutedRisks and rollout
Every slice is dark unless
OIDC_ISSUERis set; backing out is unsetting one variable. Theopenapi.jsonand locale additions are additive. No persisted data changes for a deployment that never enables it.Decisions taken
Unresolved review notes
(append-only)
c080a44f(decision 10): the conformance walk now produces every declared response of the two OIDC operations.auth::oidc::InMemoryFederatedAccounts) hold their own rows becausecapsule-server/src/auth/accounts_memory.rswas outside this lane's manifest. The recommended shape isimpl FederatedAccounts for InMemoryAccountswith anOption<String>credential and a(issuer, subject)index, so that underserve --memoryan identity whose address matches a password account is refused with409and an OIDC account has a profile row - the same shape the Postgres adapter takes over one account table.OIDC_ALLOW_LOOPBACK_REDIRECTdefaults to on so a CLI's ephemeral loopback listener works unconfigured (RFC 8252 §7.3); a deployment that wants only its web redirect sets it tofalse. The plan chose this default; it is worth a reviewer's eye because it is the one policy knob that widens what the server will redirect to.OIDC_ALLOW_LOOPBACK_REDIRECTdefaults to off as shipped, sinced294ad76(decision 14):read_oidcmaps an absent variable tofalse,RedirectPolicyadmits only the exactly-configuredOIDC_REDIRECT_URLunless a deployment turns the loopback arm on, andauthentication.md,.env.exampleand the compose stack's dex profile all say so. The reason is the one the original note asked a reviewer to weigh: it is the single knob that widens where the server will send a person back to, so a deployment with only a web client should not get it unasked, and the CLI flow that needs it (auth: CLI OIDC login — loopback listener and device authorization grant (S-N2 remainder) #461, which carries one comment saying so) turns it on deliberately. No reviewer eye is owed on the default any more; what is still worth an eye is the policy shape — exact-string equality plus loopback IP literals on any port,localhostadmitted nowhere.auth/oidc/jwks.rs(MAX_AGE, decision 12),auth/oidc/accounts.rs(email_verified, decision 11),auth/oidc/claims.rs(bounded,crit,azp— F6/F9/F10),auth/oidc/discovery.rs(is_loopback_issuerandadmit— F3),routes/oidc.rs+store/memory.rs(budget, purge, ceiling — decision 17),config.rs+boot.rs(OIDC_ALLOW_LOOPBACK_REDIRECT,OIDC_CA_BUNDLE— decisions 14 and 18),capsule-sdk/src/auth.rs(generated client — decision 16).mise run cli-surface-checkis unavailable on this base. The task is not defined in this worktree'smise.tomlandcheck-rusthere does not aggregate it; it arrives with another lane. Nothing was substituted for it.RegistrationSourcewaits on. (2)WINDOW_CEILINGis 100 000 and untuned by any measurement — it is a backstop, chosen to be far above any honest rate, not a capacity plan. (3)ShareLink,DropLinkandEnrollmentRedemptionstill key on a caller-supplied string charged before the value is resolved; the record above says why that ordering is right for them and why decision 20 is their remedy, but a reviewer who disagrees should say so, because the alternative is an existence oracle rather than a smaller map.WINDOW_CEILINGis 100 000 and untuned by any measurement — a backstop". That single constant is gone as of04192ea0; there are now eleven, one perCounterKeyvariant, each with its window-length x key-rate arithmetic written beside it incounter::ceilings. The key-rate assumptions are still estimates rather than measurements, and that is the thing to weigh — if a deployment's real distinct-key rate exceeds one of them, the symptom is a fail-closed 500/503 for new keys on that one surface only, which is the whole gain over the shared ceiling. Also still true: (1)DropLinkremains the cheapest partition to saturate (20 000 fabricated ids over a one-hour window, under six a second) — partitioning contains the blast radius to the drop path, it does not make that surface strong, and a per-source key is what would (waiting on the same trusted client addressRegistrationSourcewaits on); (2)ShareLinkstill charges beforeis_opaque_idruns, unlikeDropLinkand nowEnrollmentRedemption— deliberate per its own docs, but it means the share partition accepts keys of arbitrary length, and a reviewer who wants that tightened should say so, since a shape check there would be the same not-an-existence-check argument decision 23 makes.capsule-server/tests/enroll.rsand is telling you rather than assuming. Decision 23 forced it — three tests ground a string that is not shaped like a code while asserting the per-code budget. Two live in that file; the third is intests/conformance.rs, already in the manifest since decision 10. See decision 24 for the full reasoning and what was rejected.SOURCE_ADDRESSis one constant shared byShareSource,DropSourceandRegistrationSource— and the per-variant total is 105 018, not the 115 017 decision 22 originally claimed. The shipped numbers were always individually sound and every window matches its budget; the addition in the record was wrong. Decision 26 records it. What remains for a reviewer: (1) the key-rate assumptions behind every ceiling are estimates, not measurements — if a real deployment's distinct-key rate exceeds one, the symptom is now an honest429 error.*_at_capacityon that one surface rather than a500, which is what decision 25 bought; (2)DropLinkis still the cheapest partition to saturate and partitioning contains the blast radius rather than removing it — a per-source key is the real fix and waits on a trusted client address; (3)ShareLinkstill charges beforeis_opaque_idruns, so its partition accepts keys of arbitrary length — deliberate per its own docs, but a shape check there would be the same not-an-existence-check argument decision 23 makes, and a reviewer who wants it should say so.InMemoryCounters(decision 27). All twelve partitions share one mutex. Admission is independent; latency is not. Not a practical denial at these sizes — no.awaitin the critical section,O(log n)over at most 20 000 entries — and deliberately not changed in a review round. The module docs say so and name the issue.is_at_capacity's string match, which is safe by construction only whileInMemoryCountersis the soleCounterStore; the Valkey adapter (server: Valkey adapter for OidcAuthorizationStore and Postgres adapter for FederatedAccounts #460) is the lane that would otherwise break it silently, and it will now fail a test instead. (2) Decision 29 corrects three file:line citations in decision 20 that drifted as later rounds grew the same files, and decision 25'sEXTRASrow count (five, not three). Every citation in this record was re-checked againstf1ab7d2aand is now anchored to the symbol it names as well as the line, so the next drift degrades a citation rather than breaking it. Nothing else in the record moved, and no shipped behaviour changed in either.