Skip to content

Let an application authenticate the sign-in it is told about - #101

Merged
woksin merged 3 commits into
mainfrom
feat/signed-signin-notification
Aug 11, 2026
Merged

Let an application authenticate the sign-in it is told about#101
woksin merged 3 commits into
mainfrom
feat/signed-signin-notification

Conversation

@woksin

@woksin woksin commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Added

Security

woksin and others added 2 commits August 11, 2026 18:40
The invitation issuer already minted RS256 envelopes with key rotation,
issuer and audience binding, a lifetime and a random identifier, but it
did so behind a private method, so a second caller had no way to reach it
without duplicating the signing or the configuration checks that guard it.

Lifting it out is deliberate: one signing implementation is one place to
audit, one place where a key is loaded, and one place a mistake can live.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UWugkmN6NmoemKeTvSKNDg
The sign-in notification was posted as unsigned JSON, so anything that
could reach that endpoint chose which user the application recorded as
having signed in, including the subject and the identity provider.

The notification can now carry an envelope binding six facts: who signed
it, which application it is for, the method and target it was sent to, a
digest of the exact bytes posted, when it was issued, and a single-use
identifier. Route and body use the RFC 9449 claim names so this is a
profile of an existing scheme rather than a private one.

The digest is taken over the serialized bytes rather than the object they
came from, because only the former is what the verifier will actually see.

It is opt-in and gated on configuration alone, so a deployment that does
not configure it keeps today's behavior byte for byte. When it is
configured and an envelope cannot be issued, nothing is posted at all --
a notification that cannot be authenticated is worth less than none.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UWugkmN6NmoemKeTvSKNDg
@woksin woksin added the minor label Aug 11, 2026
@woksin

woksin commented Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

Reviewer context, kept out of the body. Intent was posted on #82 before any code was written; this implements that shape.

Why the envelope reuses the invitation stack

The invitation attestation issuer already minted RS256 envelopes with kid rotation, issuer/audience, iat/nbf/exp and a random 256-bit jti — four of the six bindings, already specced. Rather than stand up a second signing implementation, the first commit lifts its private issuing path into Attestations/AttestationSigner, and the invitation issuer now delegates to it. One signing implementation is one place to audit.

The two additions are htm/htu for the route and a digest for the body, under RFC 9449 (DPoP) claim names so this is a profile of an existing scheme.

Two deliberate deviations from RFC 9449, both worth a reviewer's attention:

  • body_hash is an AuthProxy extension. RFC 9449 has no body digest. It borrows that spec's ath construction but deliberately does not reuse the ath name, which means access-token hash — a DPoP-aware verifier would be actively misled by it.
  • htu strips query and fragment, exactly as RFC 9449 requires. NotifyUrl is a fixed configured URL, so scheme + authority + path is the honest binding.

A purpose: sign-in-notification claim prevents an invitation attestation being replayed as a sign-in notification.

Compatibility

Gated on the presence of SignIn:Attestation alone. when_signing_is_not_configured proves the unconfigured path byte-for-byte: it builds a notifier through the released four-argument constructor over the same context and principal and compares — same JSON string, same byte count, non-empty on both sides, and no Authorization header. and_the_body_is_compared_to_the_released_notification runs the same comparison with signing on, proving the envelope is header-only and the body is untouched.

A real defect the implementation pass found and fixed

Convention-based DI resolves the greediest satisfiable constructor. ICanonicalIdentityResolver is not registered by AddSignIns, so adding the signer as a sixth parameter made the container silently fall back to the four-argument constructor — every signed notification would have failed closed in production while every spec stayed green. AddSignIns now constructs the notifier explicitly, as AddInvites already did.

Independently re-verified before merge

I ran the security-critical mutation myself rather than accepting the report. Rewriting the fail-closed branch to fail open — post unsigned when the envelope cannot be issued — fails 4 assertions, and importantly not just the result code: should_not_post_anything_at_all fails in both and_no_signer_was_supplied and and_the_envelope_cannot_be_signed. The negative is paired with the posting fact, so it cannot pass because the notifier is merely broken. Restored byte-identically afterwards, confirmed with diff.

The one assertion that cannot be made to fail — stated, not hidden

The digest is taken over await content.ReadAsByteArrayAsync(), which is the correct construction. But that choice is currently unfalsifiable: mutating it to digest the payload object instead (JsonSerializer.SerializeToUtf8Bytes(content.Value!)) leaves all 394 sign-in specs green, because the anonymous payload is already camelCase and both paths emit identical bytes.

An attempt to close it by pinning that the content had been buffered was reverted — HttpContentHeaders.ContentLength does not reflect LoadIntoBufferAsync, so no observable trace exists. The guarantee is structural: it holds for any future payload whose serializer options diverge, which is exactly when it would start to matter. The strongest falsifiable form is what and_the_envelope_is_verified.should_bind_the_digest_of_the_bytes_that_crossed_the_wire asserts.

Similarly, a hard-coded htm of "POST" is unfalsifiable at the notifier, which only ever posts. The signer unit spec kills it by signing a PUT.

Prerequisite the owner should decide separately

AuthProxy exposes no JWKS endpoint. A verifying application pins public keys by configuration and selects by kid, matching how invitation attestation is consumed today. That works, but it makes key rotation a coordinated config change on both sides rather than a fetch. Worth its own decision.

Deliberately out of scope

The invite-exchange and credential-link back-channels post similar payloads; the invite arm already carries an attestation but binds neither route nor body. Both are now one call on AttestationSigner. The YARP arm and IdentityDetailsResolver's /.cratis/me call are also named in the original request and are not SignInNotifier. Replay memory is the verifier's job — AuthProxy emits a unique jti and does not remember it.

Gates

Debug and Release, both --no-incremental: 0 warnings, 0 errors. AuthProxy.Specs: 1937 passed, 0 failed (baseline 1848). AuthProxy.Security.Specs, touched by the DI change: 228 passed, 0 failed.

Not verified

No real application verified a real envelope end to end — verification is exercised by a spec-side verifier built from the published contract, not by a separate implementation. No key rotation was performed against a live deployment.

Every constant in the published envelope -- the claim names and the value
that separates a sign-in notification from an invitation attestation --
appeared in exactly one file, and every assertion read that constant and
compared it against itself. Renaming the separating value to the
invitation's left the whole suite green while every deployed verifier
broke and the separation the documentation promises collapsed. The
contract is now pinned as literals, and asserted to differ from both
invitation purposes.

The duplicate-key guard was live but unpinned for the case that matters:
the only spec duplicated the active key, so it died to the active-key
rule rather than to uniqueness, and the guard could have been deleted
unnoticed. Rotation to a duplicated identifier would then have thrown out
of the signer and broken the sign-in it was only supposed to record --
so the lookup now takes the first match rather than demanding a single
one, and fails closed if configuration ever lets one through.

An undersized RSA key signed happily, because the identity library does
not refuse one and the signer is reachable without the configuration
validator. A signing endpoint carrying a query was accepted while the
route binding deliberately excludes the query, leaving a replay window to
a different query string. The signing contract's generated ToString
printed the private key, and a key identifier of null crashed the
configuration check instead of reporting it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UWugkmN6NmoemKeTvSKNDg
@woksin

woksin commented Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

Hardening pass — addresses both independent reviews. a0dbe20.

The finding that mattered most

Every constant in the published envelope — htm, htu, body_hash, and the purpose value separating a sign-in notification from an invitation attestation — appeared in exactly one file, and every assertion read that constant and compared it against itself. The whole wire contract was self-referential.

Verified independently before and after. Renaming NotificationPurpose to "invite-stage" previously left all 1937 specs green while every deployed verifier would break and the cross-protocol separation the documentation promises silently collapsed. The same mutation now fails 2 assertions, including should_separate_the_notification_from_invitation_staging. Restored byte-identically, confirmed with diff.

One review claim was refuted, and I checked that myself

The code review reported the duplicate-key guard as only catching a duplicate of the active key, with [A, A, B] + ActiveKeyId = B accepted — and derived a rotation-time crash from it. That premise is wrong. Both validators do GroupBy(_ => _.KeyId, StringComparer.Ordinal).Where(_ => _.Count() > 1) across every key, so that configuration was already rejected. InvitationAttestationConfigurationValidator is therefore unmodified.

What was true is the coverage half: mutating > 1> 2 killed nothing, because the only existing spec duplicated the active key and so died to the active-key rule rather than to uniqueness. The guard was live but unpinned, one refactor from silent deletion. Two and_two_signing_keys_share_an_identifier specs now pin it in both validators.

The SingleOrDefaultFirstOrDefault change stands regardless, as belt and braces: if a duplicate ever did reach the signer it threw, and that exception escaped TryIssue and Notify — breaking the sign-in a notifier is documented never to break. It is now pinned by should_not_throw_out_of_the_sign_in on both paths.

Everything else applied

Item Fix
Undersized keys signed happily (1024-bit probed working — the identity library does not refuse one, and the signer is public static, so the config validator is not the only door) MinimumKeySize = 2048 enforced in TryCreateSigningCredentials, with the validator now reading the same constant
NotifyUrl query neither rejected nor bound, leaving a ≤60s replay window to a different query string rejected at startup, sign-in only — shared validation untouched, invites unaffected
AttestationSigningContract.ToString() printed the private key overridden to omit it, while still naming key, issuer and audience
A null KeyId NRE'd startup validation instead of reporting it guarded
Nothing pinned the signing algorithm _token.Alg.ShouldEqual("RS256")
Empty SigningKeys uncovered spec added

Mutation evidence

22 mutations, each applied, rebuilt and re-run against the full suite — not reasoned. All 32 new assertions confirmed load-bearing, each dying to a single named line. Every negative is paired with a positive baseline dying to the opposite mutation of the same line (e.g. the key-size floor: < 2048< 1024 kills the refusal assertions, < 4096 kills the acceptance ones), so no "rejected" assertion can pass because the unit rejects everything.

Still unfalsifiable, unchanged and still disclosed

The body digest over serialized bytes versus the payload object remains unkillable — both emit identical bytes while the payload is camelCase. The guarantee is structural and matters the moment a payload's serializer options diverge.

Gates

Debug and Release, both --no-incremental: 0 warnings, 0 errors. AuthProxy.Specs 1969 passed (1937 before; +32, exactly the new assertions). AuthProxy.Security.Specs 228 passed.

Deliberately not done

Caching SigningCredentials per key — a real cost on the sign-in hot path, but lifted verbatim from the shipped invitation path, so it is a pre-existing concern that deserves its own change rather than riding along here.

@woksin
woksin merged commit e28e839 into main Aug 11, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant