feat(provider-did): wire the OWNER login contracts into the DID grant handler - #19
Merged
Conversation
…ON_CONTROL_LOGIN@1 into the DID grant handler
`handle()` in did.mts now dispatches on the configured `authContract`
instead of always running the LEGACY (relationship-blind) flow:
- LEGACY_DID_LOGIN@1: unchanged, byte-for-byte.
- OWNER_AUTHENTICATION_LOGIN@1 / OWNER_ASSERTION_CONTROL_LOGIN@1: the
signed request payload is parsed as a versioned login transcript
(`parseLoginTranscript`), the transcript's `verification_method` is
looked up via `selectVerificationMethod(doc, { did, methodId,
relationship })` — `authentication` for OWNER_AUTHENTICATION_LOGIN@1,
`assertionMethod` for OWNER_ASSERTION_CONTROL_LOGIN@1 (transcript.mts's
`AuthContractId` doc comment) — and `validateOwnerLogin` enforces the
three-way kid match (JWS header kid / transcript.verification_method /
resolver-selected method id), issuer, token_endpoint, and audience
allowlist binding. `audience` is one of the transcript's ten required
fields, so a missing audience is rejected before any other transcript
check runs, and a minted OWNER token always carries `aud`.
`EvaluationInput.relationship` and the minted `auth_contract_id` now
reflect the OWNER contract actually enforced.
Removed the `createDidGrant` construction-time throw for OWNER contracts
(the Option-B stopgap) now that the path is wired. The one boot-time
requirement that remains: `tokenEndpoint` is required (fail closed, no
default) when `authContract` is OWNER_*, mirroring the existing
`allowedAudiences` / `revocationLatencyBoundSec` asserts — needed because
`validateOwnerLogin` checks the transcript's `token_endpoint` field against
it.
A DID Document with more than one controller-matched verificationMethod
is not yet supported on the OWNER path: the crypto-key-selection step
(shared with LEGACY, unconditional) rejects it as ambiguous before the
OWNER-specific relationship check ever runs. This is an existing,
structural fail-closed limitation (MethodSelectionError
"ambiguous-legacy-selection"), not a new gap — flagged as follow-up work.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…r the wired path module.config.test.mts's "OWNER authContract fail-closed guard (Option-B stopgap)" describe block pinned the removed construction-time refusal — its two `.toThrow(/OWNER/)` assertions were about to start passing for the wrong reason (matching the new tokenEndpoint-required guard's error message by coincidence, not the behavior the test names described). Rewritten as "OWNER authContract no longer refuses at construction": construction now succeeds for an OWNER contract once `tokenEndpoint` is configured, and still fails closed when it is missing. Also flips "does NOT reach the legacyMaxTtlSec check for a non-LEGACY authContract" back to its original `.not.toThrow()` intent (noted in its own "Was:" comment) now that an OWNER contract with a valid `tokenEndpoint` constructs successfully — `legacyMaxTtlSec` is LEGACY_DID_LOGIN@1-only. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…CHANGELOG README.md § P0 Auth Contract and packages/provider-did/README.md both described `createDidGrant` as throwing at construction time for `OWNER_AUTHENTICATION_LOGIN@1` / `OWNER_ASSERTION_CONTROL_LOGIN@1`, with the OWNER validation path "built and unit-tested but not yet wired into the request handler." Update both to describe the now-wired enforcement: versioned login transcript, three-way kid match, Fork-Y relationship check (authentication / assertionMethod), and required audience — plus the one remaining boot-time requirement (tokenEndpoint) and the one remaining known limitation (no genuine multi-key-per-DID OWNER selection yet). docs/requirements.md's spec-mandate blockquote is updated the same way. CHANGELOG.md gets a new [Unreleased] entry; the already-released [0.2.1] entry documenting the since-removed construction-time stopgap is left untouched as historical record. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…loads Converged review finding (Claude + Codex): `ed25519Raw.mts` and `ed25519Prehash.mts` cast the signed JSON payload straight to `ParsedMessage` (`JSON.parse(body.message) as ParsedMessage`), so a signed top-level `headerKid` member flowed through to `parsedMessage.headerKid` unfiltered — even though neither format has a JWS protected header at all. That let a raw-path request forge the OWNER path's three-way kid match from attacker-controlled payload data instead of a real protected header (the only legitimate source, `jws.mts`'s `JwsVerifier`). Both verifiers now explicitly `delete` any `headerKid` member from the parsed payload before casting, so the property can never be forged from payload data — `headerKid` stays `undefined` for these formats by construction, matching what a real absent header already produced. RED: new test in each verifier's own test file signs a payload carrying a `headerKid` member and asserts `result.parsedMessage.headerKid` is `undefined` — failed before the fix (leaked the forged value), passes now. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… gate, enforce token/key binding Four review items addressed in did.mts (plus module.mts's now-stale doc comment): 1. Boot-time guard: an OWNER `authContract` now requires every configured `supportedAlgorithms` entry to be header-bearing (JWS-family: `ed25519_jws` / `es256_jws` / `es256k_jws`). `ed25519_raw` / `ed25519_prehash` sign no JWS protected header at all, so the OWNER path's three-way kid match was structurally unsatisfiable on them — including under the `didConfigSchema` default `supportedAlgorithms: ["ed25519_raw"]`, which would otherwise leave a freshly-configured OWNER contract unusable end-to-end. Fail-closed construction error instead. 2. Restored `ownerMigrationRatified` boot enforcement. The removed Option-B stopgap incidentally enforced `auth.migration.enable-gate` for a hand-built config too (it refused every OWNER `authContract` unconditionally); the guards that replaced it mirrored `allowedAudiences` / `revocationLatencyBoundSec` / `tokenEndpoint` but not this one. Added the same fail-closed re-assert `didConfigSchema`'s `superRefine` already performs at parse time. 3. Step 5b now asserts `ownerSelected.id === resolvedKey.id` (throwing `TranscriptError` otherwise) before calling `validateOwnerLogin`. Today this always holds — step 3's bare crypto-key selection requires exactly one controller-matched candidate, so the methodId-based OWNER selection can only succeed by finding that same candidate — but CHANGELOG.md flags genuine multi-key-per-DID OWNER selection as tracked follow-up work; this makes the minted token's `verification_method` binding an enforced invariant rather than an emergent side effect of today's single-candidate limitation. No accompanying test: the invariant is not reachable via real inputs under the current selection semantics (both selections are pure functions of the same (doc, did) pair), so a real (non-mocked) RED test cannot be constructed — see the code comment at the assert site. 4. Step 9's comment now notes the OWNER/LEGACY nonce-burn asymmetry: step 5b's OWNER transcript rejections run before step 8's nonce consumption (never burn the nonce), while a LEGACY audience failure runs after (does burn it). module.mts's `authContract` schema doc comment still said "NOT wired ... createDidGrant fails closed at construction time" — updated to describe the wired path and the three remaining boot-time requirements. RED: new module.config.test.mts tests for the two new boot guards (items 1 and 2) — failed before the fix (construction succeeded/threw the wrong message), pass now. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…must equal subject_did)
Codex P2, must-fix: every built-in signature verifier's own internal
binding check requires `parsedMessage.did === body.did`
(`ed25519Raw.mts` / `ed25519Prehash.mts` / `jws.mts`), but
`login-transcript-v1` only defined `subject_did` — no real signed payload
could satisfy both that check and `parseLoginTranscript`'s field
requirements at once. The did.owner.test.mts fixtures only passed because
they added an undocumented eleventh `did` property the schema didn't
actually require.
Resolution (approved contract decision — the transcript is unreleased, so
its shape can still change): `did` is now an explicit required member of
`LoginTranscript` (eleven fields total), and `validateOwnerLogin` checks
`transcript.did === transcript.subject_did`, throwing `TranscriptError("did",
...)` on mismatch.
Updated `parseLoginTranscript`'s and `validateOwnerLogin`'s doc comments
(ten -> eleven fields; documents why both `did` and `subject_did` exist;
the check-order listing now includes the new check). Also fixed the stale
"Not yet wired into createDidGrant's handler" sentence on
`validateOwnerLogin` — it is wired (did.mts's handle(), step 5b).
RED: transcript.test.mts's `REQUIRED_FIELDS`/`validPayload()` now include
`did`, so the existing describe.each loop generates "rejects when
missing"/"rejects when blank" coverage for it automatically; a new explicit
test asserts `did !== subject_did` is rejected. Both failed before the
schema/validator change (no such field existed to reject on), pass now.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…algorithm guard, ownerMigrationRatified README.md and packages/provider-did/README.md updated for the review fix set: - "all ten fields" -> "all eleven fields" (the new `did` field), and note it must equal `subject_did`. - Document the `supportedAlgorithms` JWS-family requirement for OWNER contracts (new Config keys row / P0 Auth Contract bullet). - Document the `ownerMigrationRatified` boot-time re-assert alongside `tokenEndpoint`. - Note the methodId-based OWNER selection is explicitly checked against the crypto-verified method (previously implicit). - packages/provider-did/README.md's `ParsedMessage` section previously claimed the raw Ed25519 verifiers "have no header/kid concept, so they can never satisfy the OWNER path's three-way match" as an inherent property — that became true only once the headerKid-stripping fix and the boot-time algorithm-family guard landed; reworded to attribute the guarantee to those two mechanisms rather than state it as a standing fact that predated them. CHANGELOG.md's `[Unreleased]` section is rewritten to describe the final, reviewed shape of this still-unreleased feature (eleven-field transcript, algorithm-family + ownerMigrationRatified boot guards, the headerKid stripping fix, the token/key binding assert) rather than layering patch-notes on top of itself; the already-released `[0.2.1]` entry documenting the removed construction-time stopgap is left untouched as historical record. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Closes the largest gap between the manifesto and the implementation: the right to claim a DID was verified relationship-blind. External review (P11) ranked this the top technical priority.
What
handle()dispatches onauthContract. OWNER path (step 5b): parse the signed payload as a versionedlogin-transcript-v1, certify the transcript'sverification_methodviaselectVerificationMethod(doc, {did, methodId, relationship})—authenticationforOWNER_AUTHENTICATION_LOGIN@1,assertionMethodforOWNER_ASSERTION_CONTROL_LOGIN@1— thenvalidateOwnerLogin(three-way kid match, audience/issuer/token_endpoint). Rejections happen before nonce consumption and map to a generic 400 (no error oracle).tokenEndpointrequired,ownerMigrationRatifiedrequired, and OWNER contracts require header-bearing JWS-family algorithms (see review trail).EvaluationInput.relationshipreflects the enforced relationship; a local assert pinsownerSelected.id === resolvedKey.id.Review trail (multi-agent, pre-PR)
Two independent reviewers (Claude / Codex) on the initial implementation — verdict NEEDS WORK, all items fixed in the follow-up commits:
headerKidvia raw verifiers.ed25519_raw/ed25519_prehashparsed the signed payload straight intoParsedMessage, so a payload-suppliedheaderKidcould satisfy the three-way match — including under the schema default algorithms. Fixed at both layers: raw verifiers stripheaderKid; OWNER contracts refuse non-JWS algorithms at boot.did; the transcript only definedsubject_did— initial tests hid this with an undocumented extra field). Resolved as a deliberate wire decision — see below.ownerMigrationRatifiedboot enforcement restored (was incidental in the old refusal).login-transcript-v1becomes an eleven-field transcript:didis now an explicit required member, andvalidateOwnerLoginenforcesdid === subject_did. The transcript has never shipped in a release, so the shape change is free today — but this is a wire-format decision and is the main thing to approve or veto in this PR.Also flagged: genuine multi-key-per-DID OWNER selection remains unsupported (step 3's single-key selection rejects it fail-closed first — pre-existing, documented in CHANGELOG); the binding assert in step 5b is deliberately untestable today (unreachable by construction) and becomes load-bearing when multi-key selection lands.
Tests
216 passed | 1 todo in provider-did (+24 vs base), full workspace suites, build, and typecheck clean. Every behavioral fix was confirmed RED before implementation.
🤖 Generated with Claude Code