feat(core)!: make the auth handshake a wire contract the adapter performs - #158
Open
cuibonobo wants to merge 3 commits into
Open
feat(core)!: make the auth handshake a wire contract the adapter performs#158cuibonobo wants to merge 3 commits into
cuibonobo wants to merge 3 commits into
Conversation
…orms
Every wire contract here is pinned by conformance fixtures except the one
that decides whether a client can connect at all. The challenge-response
handshake was a sketch explicitly marked non-normative, so `APIAdapter`
took a pre-acquired static token and nothing else, and `signWithDid()`
had no client caller anywhere. That made "token issuance stops being an
out-of-band secret handoff" false for every server, since each would
invent its own endpoints, and "the same client works against localhost or
a remote provider" false at connecting.
`POST /auth/challenge` and `POST /auth/token` are now normative, with
fixtures covering success, a rejected signature, and a stale nonce.
`APIAdapter` takes `{ did, sign }` — a signing callback, never a private
key, so custody stays with the app — performs the handshake on open, and
renews on 401 transparently. Concurrent requests finding the same token
stale share one handshake rather than spending a signature each, and a
401 that survives renewal is `APIAdapterReauthError`, distinguishable
from a token that was never valid.
**What is signed is not the nonce.** It is a domain-separated payload
binding the server's origin, built by one exported constructor both sides
call, because two derivations of "the same string" diverge on the first
ambiguity. The origin is what makes the handshake safe to perform against
a server whose identity rests on transport: without it, a server a client
connects to can fetch a challenge from that client's real stack, pass the
nonce along, and redeem the answer there. Signing the origin the client
believes it is talking to makes such a signature verify nowhere else. A
server must therefore build the payload from its own configured public
origin, never from a client-controlled `Host` header.
Auth failures get their own vocabulary outside `WireErrorCode` — no Stack
operation has begun, so none is a `StackError` with a class to
reconstruct. The split that earns them codes at all is retryable versus
fatal: a stale nonce is not a credential failure and warrants one fresh
handshake, while a rejected signature will be rejected identically
forever, so a client retrying it would loop.
**A token names two identities, and the handshake establishes only one.**
Proving key possession proves the principal and says nothing about whom
that key may act for, so `/auth/token` never delegates and offers no
field that could ask it to — an app choosing its own subject would be
choosing its own authority. `StackTokenStore` grows `onBehalfOf` for the
owner to assert a binding out of band, which is safe unproven because
effective authority is the intersection of both parties' grants.
`lookupToken()` returns both identities always populated, so the mapping
onto `asEntity(principal, { onBehalfOf: subject })` is mechanical rather
than a rule about which field stands in for the other.
Scoped consent — where the subject authorizes an app directly — issues a
token of exactly this shape through another route and advertises itself
as another entry in `auth.methods`, so it needs nothing here to change.
BREAKING CHANGE: `StackTokenStore.createToken()` takes a principal and an
optional `onBehalfOf`; `lookupToken()` returns `{ principalId, subjectId }`
rather than `{ entityId }`, and `TokenInfo` carries the same pair. The
tokens table replaces `entity_id` with `principal_id` and `subject_id`.
`APIAdapter.open()` refuses `token` and `credential` together.
Refs #138.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KawUMrf9qvEm8zxetRxJqr
The field is optional, so 401-driven renewal is the floor a client needs whatever else it does — which makes expiresAt advisory rather than a schedule clients keep. Saying so tells a server implementer that an aggressive expiry costs a wasted round-trip per lifetime, instead of leaving them to assume clients renew ahead of it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KawUMrf9qvEm8zxetRxJqr
Core runs no server, so every security-critical control in the handshake lives in the implementer's code — and the thing implementers will reach for as their check is the conformance fixtures, which pin the shape of a request and its response. Three of the obligations that matter most are properties of state, configuration, or argument order, so a server could satisfy every fixture and still be unsafe. That is confidence aimed exactly where it is least warranted. **Single use gets a fixture that can express it.** A pair of steps whose requests are byte-identical and whose responses differ: the first earns a token, the second is refused. Nothing else distinguishes them, which is what makes it a replay rather than a differently-shaped request, and a server that checks a nonce exists without spending it now fails something. The format grows the minimum to say that — an ordered list of ordinary fixtures — since a step is only a fixture whose position matters. Both steps assume issued-but-unspent prior state, the same way every other fixture assumes the state its description names, so the data stays self-contained with no templating between steps. **A nonce belongs to the DID it was issued for**, pinned by a redemption that is internally consistent and must still be refused: a second DID's own valid signature over the same origin and nonce. A server storing nonces without recording who each was issued to passes every other fixture and fails this one. **`forSession()` takes the pair a token names whole.** Both identities are DIDs, so passing them positionally leaves nothing to catch a swap, and a swapped pair is invisible undelegated where the two are equal — it would surface only once delegation is in use, as authority no longer fenced by the app's grants and every write attributed to the app rather than the person. Removing the order removes the mistake. What cannot be fixed in code is collected instead: a server checklist naming the controls no fixture reaches, chief among them deriving the signing origin from configuration rather than the `Host` header. That one is the natural implementation, it passes every fixture, and it silently restores the relay the origin binding exists to prevent. Refs #138. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KawUMrf9qvEm8zxetRxJqr
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
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.
Closes #138.
Summary
Every wire contract in this spec is pinned by conformance fixtures except the one that decides whether a client can connect at all. The challenge–response handshake was a sketch, explicitly marked "not a normative wire contract" — so
APIAdaptertook a pre-acquired statictokenand nothing else, andsignWithDid()had no client caller anywhere in the codebase. That made §Identity's own claim false: token issuance stops being an out-of-band secret handoff only if clients can perform the handshake, and with non-normative endpoints every server invents its own. "The same client works againstlocalhostor a remote provider" failed at connecting.POST /auth/challengeandPOST /auth/tokenare now normative, with fixtures.APIAdaptertakes{ did, sign }, performs the handshake on open, and renews on 401 transparently — so a caller never handles token lifecycle by hand. The credential is a signing callback, never a private key: custody stays with the app, consistent with the stated stance, anddidCredentialFromKeypair()is the convenience wrapper for callers who do hold key material in process.What gets signed is not the nonce
It is a domain-separated payload binding the server's origin, built by one exported constructor both sides call:
The tag answers #138's domain-separation question — a future signing primitive takes its own, so no signature made for one purpose ever verifies as another. The origin answers a question the issue didn't ask, and it turned out to be the load-bearing one.
Discovery identity is only trusted on transport (#140, just merged), so a client can be talking to a server that isn't the one it meant. Signing a bare nonce there is worse than it looks: the impostor fetches a challenge from the client's real stack, passes that nonce along as its own, and redeems the returned signature there. The nonce doesn't stop it — the nonce came from the real server. So a hostile URL escalates from "collects what you write" to "holds a token as you." Signing the origin the client believes it is talking to makes such a signature verify nowhere else.
Two rules follow, and a server skipping either has a conformance gap rather than a lenient implementation: it MUST build the payload from its own configured public origin, never a client-controlled
Host/X-Forwarded-Host, and it MUST verify against the payload it builds itself.This also composes with
expectedOwnerin the right order: the handshake runs last inopen(), after version negotiation and the owner check, so a credential is never spent on a server this client has already decided to refuse.A token names two identities; the handshake establishes one
This is where #139-as-landed reshapes the issue.
StackTokenStorewas single-identity (createToken(entityId)→lookupToken→{ entityId }), which a server built onScopedStackcannot turn intoasEntity(principal, { onBehalfOf })at all.Proving key possession proves the principal and says nothing whatever about whom that key may act for. So
/auth/tokennever delegates, and offers no field that could ask it to — an app that could name its own subject would be choosing its own authority, and containment would evaporate.StackTokenStoregrowsonBehalfOffor the owner to assert a binding out of band, which is safe unproven precisely because effective authority is the intersection of both parties' grants.lookupToken()returns both identities always populated, rather than mirroring the record convention whereprincipalIdis omitted when it equalsentityId. Records omit it because storage is per-row; a session is resolved once, at the call site where getting the two backwards is an escalation./auth/tokenreports the pair too, and always reports them equal. Carrying two always-equal fields is deliberate: scoped consent is the extension point this shape reserves. A flow where the subject authorizes an app directly issues a token of exactly this shape through another route and advertises itself as another entry inauth.methods— nothing here changes. That is also why the discovery block is{ "methods": [...] }rather than a boolean.Auth errors are their own vocabulary
Outside
WireErrorCode, deliberately: no Stack operation has begun, so none of these is aStackErrorwith a class to reconstruct — the same reasonInvalidDidErroralready sits outside that hierarchy.invalid_didunknown_nonceexpired_nonceinvalid_signatureThe retryable column is the whole reason these carry codes rather than being bodyless 401s. A stale nonce is not a credential failure — the window between issuing and signing is small but real — so a client re-runs the handshake once. A rejected signature will be rejected identically forever, so a client retrying it would loop. A server MUST NOT distinguish never-issued from already-spent: they differ only in what an attacker learns.
Renewal
Concurrent requests finding the same token stale share one handshake rather than spending a signature each. Renewal is capped at one retry per request: a token minted seconds ago and refused is a credential that no longer authorizes this request, not a stale session. A 401 surviving renewal is
APIAdapterReauthError— distinguishable fromAPIAdapterAuthError, which means the token was never good. It extendsAPIAdapterAuthError, so existing catches still work.Closing the gaps the fixtures cannot check
Core runs no server, so every security-critical control here lives in the implementer's code — and the thing implementers will reach for as their check is the fixtures, which pin the shape of a request and its response. Several of the obligations that matter most are properties of state, configuration, or argument order, so a server could satisfy every fixture and still be unsafe. That is confidence aimed exactly where it is least warranted, so three changes address it.
The fixture format grows to express ordering.
ConformanceSequenceFixtureis an ordered list of ordinary fixtures — a step is just a fixture whose position matters, so the format grows the minimum needed to say "and not a second time".auth-nonce-is-single-useis two byte-identical requests with different expected responses: the first earns a token, the second is refused. Nothing distinguishes them except that the first already happened, which is what makes it a replay rather than a differently-shaped request. A server that checks a nonce exists without spending it now fails something. Both steps assume issued-but-unspent prior state, the same way every other fixture assumes the state its description names, so the data stays self-contained with no templating between steps.A nonce belongs to the DID it was issued for, pinned by
auth-token-rejects-nonce-issued-to-another-did: a redemption that is internally consistent — a second DID's own valid signature over the same origin and nonce — and must still be refused. A server storing nonces without recording who each was issued to passes every other fixture and fails this one.Stack.forSession(session)takes the pair a token names whole:Both identities are DIDs, so passing them positionally to
asEntity()leaves nothing to catch a swap — and a swapped pair is undetectable in the undelegated case, where the two are equal. It survives every test a single-app deployment can write and surfaces only once delegation is in use, as authority no longer fenced by the app's grants and every write attributed to the app rather than the person. Removing the order removes the mistake.asEntity()stays the direct form for callers that genuinely hold one identity.What cannot be fixed in code is collected instead — a Server implementation checklist naming the controls no fixture reaches. Chief among them: derive the signing origin from configuration, never the
Hostheader. That is the natural implementation in every framework, it passes every fixture, and it silently restores the relay the origin binding exists to prevent — the impostor replays a harvested signature with the matchingHostand it verifies. The failure has no symptom at all.Spec
Observable behavior changes throughout. Sections updated:
expiresAt), the signed payload and both server-side MUSTs, the auth error table, and § The session a token namesauthdiscovery block, and why it is an objectauthin the responseforSession(), and what a swapped pair costsVerification
All five green. 174 tests in
adapter-api, 1125 across 8 packages.Checked by hand:
AUTH_FIXTURE_PAYLOAD, the fixture signature verifies against it, and it does not verify for another origin. The bad-signature fixture is a genuine 64-byte signature by a different key, and the wrong-DID fixture's signature genuinely verifies for its own DID — both are asserted, since a fixture that pins a rejected credential must not be passing by being malformedforSession()'s argument order (swapped → two tests fail), and the replay fixture's identity (second step's nonce changed → the byte-identical guard fires)open()was verified by moving it ahead of the version and owner checks rather than removing it — the "does not handshake against a server failingexpectedOwner" test fails there, so it pins the order rather than the outcome\nplus a forged origin and DID is refused rather than producing a payload that reads as a different challengeNotes for reviewers
Breaking changes:
StackTokenStore.createToken()takes a principal plus optionalonBehalfOf;lookupToken()returns{ principalId, subjectId }instead of{ entityId };TokenInfocarries the same pairentity_idwithprincipal_idandsubject_id. No install base, so no migration path — per AGENTS.mdAPIAdapter.open()throws when given bothtokenandcredentialDeliberate, and worth disagreeing with if you do:
token+credentialis refused rather than resolved by precedence. One would be silently ignored and neither name suggests which.lookupToken()returns both identities always populated, diverging from the record convention. Records optimize a per-row storage cost; a session is resolved once, at the call site where confusing the two is an escalation./auth/tokenreports two fields it always sets equal. Redundant today; the alternative needs a shape change the moment any issuance path delegates.forSession()is added rather than changingasEntity().asEntity()has real callers that hold exactly one identity, and making it take a session object would push those into constructing{ principalId: x, subjectId: x }— which reintroduces the confusion by another route./auth/challengein step 1. A real server issues its own random nonce, so a step 1 that actually ran would make the fixture non-self-contained and force templating between steps. Assuming prior state is what every other fixture here already does.StackErrorCode. Adding one would put a code in the taxonomy with no class behind it.verifyAuthChallenge()returnsfalsefor an undecodable DID rather than throwing. The caller asked whether it verifies; a server that had to catch as well as branch would eventually catch too broadly.Known limits, documented rather than fixed:
expectedOwnernarrows misdirection and origin binding stops the relay, but neither proves a server↔owner binding — that stays deferred alongside key rotationexpiresAtis documented as advisory. The one case where the wasted round-trip is expensive is a large attachment upload, which re-sends its body on retry — worth revisiting if it bites, and nothing here forecloses it (the field is already on the wire,performHandshake()already returns it, and the coalescing guard already generalizes)