Skip to content

docs(spec): validate every claim on the token /revoke acts on - #19

Draft
antstanley wants to merge 1 commit into
spec/audit-and-throttle-auth-failuresfrom
spec/validate-revoke-token-claims
Draft

docs(spec): validate every claim on the token /revoke acts on#19
antstanley wants to merge 1 commit into
spec/audit-and-throttle-auth-failuresfrom
spec/validate-revoke-token-claims

Conversation

@antstanley

@antstanley antstanley commented Aug 5, 2026

Copy link
Copy Markdown
Owner

Note

Stacked PR 5 of 16 — part of stack #35, which targets main.
Base: #24 (spec/audit-and-throttle-auth-failures) · Followed by: #21

This PR's diff shows only its own change. Merge the stack bottom-up; do not merge this before its base.

Why it sits at position 5: Supersedes #24's revoke-path audit variant (SessionsRevokedTokenRevocation) and inherits its mandatory channel. Establishes sid as the session identifier, which #22 later re-points to family_id.

Scope of this PR

This PR currently carries the change spec. It will also carry the implementation plan and the build work for that spec — plan and code land as later commits on this same branch, so the spec, its plan, and its implementation review and merge as one unit.

Review order within the PR: spec first, then plan, then implementation.


Change spec proposing full claim validation on POST /revoke, and a bounded revocation authority.

Problem. crates/core/src/service/revoke.rs verifies the JWS signature and then acts on the token's sub, without checking exp, iss, aud, or token type. Any access token the service ever issued — including one expired months ago — is a repeatable, unauthenticated primitive that destroys every session belonging to that subject.

Proposed delta. Replace the signature-only verify_and_extract_sub with a single first-party validator AppService::validate_access_token, sited next to build_access_token, which pins the header (alg/kid/typ: at+jwt), verifies the signature before reading any claim, and enforces exp/iat/iss/aud (plus nbf when present, 60s skew) by deserializing into a typed AccessTokenClaims so required-claim presence is a parse result rather than an omittable check.

It also fixes the authority model rather than only the checks: a new required sid claim (the session's existing refresh_token_hash, so no store migration and no new port method) binds each access token to the one session it was minted for. The access-token branch revokes that session; revoke_all_user_sessions becomes unreachable from the public endpoint and stays on the authenticated admin path.

Failure is fail-closed: nothing is revoked, ValidationFailed is emitted at Notice — above the Debug that the default emit_threshold = "info" would drop, below the default blocking threshold so an audit outage cannot turn a 200 into a 503 — and the client still sees RFC 7009's indistinguishable 200.

Notes.

  • RESERVED_CLAIMS must gain sid and nbf, or a per-user claim named sid collides with the flattened struct field.
  • There was no existing first-party validation path to share: refresh.rs does no JWT validation (opaque token, store lookup) and exchange.rs delegates to the provider adapters. The spec states this rather than implying reuse.

Drafted with the spec-creator plugin from a deep security scan of the repository at 53cbdec9. Spec only — no source changes.


Resolved: the audit conflict with #24

An earlier revision of this PR flagged a contradiction with its base, #24, and asked for a human decision. It has been settled, and both specs were changed — the resolution is recorded here because the reasoning matters more than the outcome.

The two positions were:

Position Stated argument
This PR (#19) Failed revocation is recorded A repeatable unauthenticated primitive that answers 200 either way is invisible without a record
#24 Revocation stays silent on failure "An event per unknown token would reconstruct the existence oracle RFC 7009 §2.2 forbids"

#24's argument does not survive its own durability model. Its mandatory SecurityEvent channel ignores thresholds, and under durability = "enforce" a write failure propagates Err and the operation fails. Combine that with emitting only on success, and during a sink outage:

Presented token Silence-on-failure (#24 as written) Symmetric emission (now both)
Existed emits → write fails → 503 emits → fails → 503
Unknown / unverifiable emits nothing → 200 emits → fails → 503

The asymmetry is the oracle. Silence on the failure branch reintroduces, as degraded-mode behaviour, exactly what §2.2 forbids — and #24 explicitly accepts enforce as "a deliberate reduction in availability", so a sink outage is an anticipated operating state, not an edge case.

RFC 7009 §2.2 constrains the client-visible channel. An audit record is operator-facing; an attacker who can read the sink has already won. Recording a rejection discloses nothing to the caller — provided the emission decision is identical on both branches, which is what keeps the response indistinguishable.

What changed:

  • docs(spec): audit and throttle authentication failures #24 — the Decision is now Revocation records both outcomes; the 03-service-flows.md Revocation block specifies an AuthenticationFailed event on the rejection branch and explains why symmetry rather than silence is the control; implementation note 5 no longer exempts revoke from the single-exit wrapper; and a regression test asserts that under enforce with a failing sink, /revoke returns the same status for a token that exists and one that does not.
  • This PR — the Decision is restated for the mandatory-channel model. The original Notice severity was chosen to clear emit_threshold while staying under blocking_threshold; docs(spec): audit and throttle authentication failures #24 removes both levers, so severity no longer carries that argument and the spec now uses docs(spec): audit and throttle authentication failures #24's AuthenticationFailed classification.

The concern the silence was really carrying — unbounded audit volume from unauthenticated probing — is a sink-cost argument, not an oracle argument, and is handled by the per-IP limiter #24 already introduces.

@antstanley
antstanley force-pushed the spec/validate-revoke-token-claims branch 2 times, most recently from 66bbd28 to b096ad4 Compare August 5, 2026 08:22
@antstanley
antstanley changed the base branch from main to spec/audit-and-throttle-auth-failures August 5, 2026 08:22
@antstanley
antstanley marked this pull request as draft August 5, 2026 08:25
@antstanley
antstanley force-pushed the spec/validate-revoke-token-claims branch from b096ad4 to cfd6244 Compare August 5, 2026 09:00
@antstanley
antstanley force-pushed the spec/validate-revoke-token-claims branch from cfd6244 to 9000429 Compare August 5, 2026 11:29
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@antstanley
antstanley force-pushed the spec/validate-revoke-token-claims branch from 9000429 to 5d139d7 Compare August 5, 2026 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant