feat: reconcile the nest-auth keyspace and ship five security items#95
Open
msalvatti wants to merge 73 commits into
Open
feat: reconcile the nest-auth keyspace and ship five security items#95msalvatti wants to merge 73 commits into
msalvatti wants to merge 73 commits into
Conversation
…ation Refresh rotation used a 30s grace window but never detected the replay of an already-consumed token: an in-grace replay forked a parallel session and a post-grace replay was rejected as a plain invalid, so a stolen refresh token stayed usable and theft was never surfaced (OWASP rotation with automatic reuse detection was not implemented). Track a refresh-token family (login lineage) minted at login and inherited unchanged across every rotation. On rotation the store now plants a consumed-token marker (`cf:`) that outlives the grace pointer and moves the hash in a family index (`fam:`). Presenting a consumed token past its grace window is therefore caught as a reuse: the store returns RotateOutcome::Reused carrying the compromised family, and the token manager revokes the entire family (every live descendant) before rejecting, forcing re-authentication. The atomic-rotation + grace-window concurrency semantics are preserved: an in-grace replay still recovers, and only a post-grace replay trips reuse. Legacy records with no family are handled gracefully (no marker, no index, never a reuse target). - SessionRecord gains `family_id` (serde-default, omitted when empty) - RotateOutcome gains `Reused(family_id)`; SessionStore gains `revoke_family` - refresh_rotate.lua plants the consumed marker + family index and detects reuse; new revoke_family.lua deletes a lineage atomically - in-memory test store mirrors the semantics for the core unit tier Tests prove: post-grace reuse revokes the live descendant (dashboard, platform, in-memory and Redis tiers); an in-grace concurrent rotation still succeeds; and a legacy no-family session never trips reuse. Coverage stays 100% on every changed file; clippy, cargo-deny, and the security-invariant gate pass.
…er-user epoch A password reset revoked the refresh sessions but left already-issued, stateless access JWTs valid for the remainder of their (up to 15-minute) lifetime: the jti-blacklist used on logout can only revoke a token you hold, and a reset does not possess the user's active access-token jtis — there is no per-user registry of them. Add a per-user token **epoch** (generation counter): stamped into the Dashboard/Platform claims at issuance (and rotation), read on every verification, and bumped on a password reset. A token stamped below the user's current epoch was issued before the reset and is now rejected on its next request, so a reset takes effect immediately instead of lingering for the access-token lifetime. The refresh sessions are revoked as before. Backward-safe: a legacy token with no epoch claim and a user with no stored epoch both read as 0, so the mechanism is inert (0 < 0 is false) until a first bump — no existing session is locked out. The epoch is server-internal (never surfaced in AuthResult); the edge WASM verifier carries the claim but does not consult it (no store), exactly like the jti-blacklist. - DashboardClaims/PlatformClaims gain `epoch` (serde-default); TS bindings regenerated - SessionStore gains current_epoch/bump_epoch; keys ep/pep; redis + in-memory impls - the dashboard password-reset flow bumps the epoch after revoking sessions Scope note: MFA-state changes and session revoke-all keep their existing "revoke other refresh sessions, current session continues" semantics and do not bump the epoch; the platform epoch mechanism is complete and symmetric, awaiting a platform credential-reset flow to trigger it. Tests prove a pre-reset access token is rejected after the bump (dashboard and platform), that a post-bump token still verifies, that the reset flow advances the epoch, and that a legacy no-epoch token stays valid. Coverage stays 100% on every changed file; clippy, cargo-deny, ts-export staleness, and the security-invariant gate pass.
Rotation minted access claims with `mfa_enabled: false` hardcoded, and the session record had no field to carry the real value. The MFA gate refuses a token only when `mfa_enabled && !mfa_verified`, so one routine refresh — every ~15 minutes for a normal client — produced a token that cleared every MFA-gated route without the holder ever completing a challenge. The bypass applied to the dashboard and platform planes alike, including the WebSocket ticket endpoint. - add `mfaEnabled` to SessionRecord, written at issuance from the account and inherited unchanged by `identity_record` / `platform_identity_record` so it survives every rotation. Field is `serde(default)`, so sessions written before it existed read back as `false` instead of failing the whole record and logging the live user base out. - read the flag in `rotated_claims` / `rotated_platform_claims` instead of hardcoding it. `mfa_verified` still resets to `false` by design: the second factor must be re-acquired through the challenge, only the enrollment state carries over. - populate the flag on the hook/eviction projections so a consumer's after-session-created payload reports the account's real MFA state. Wire parity: `mfaEnabled` is emitted unconditionally and sits last in the record, matching the field order nest-auth already writes to the shared Redis. Regression tests pin the invariant on both planes, plus the unenrolled mirror (so the flag is read, never hardcoded true) and a legacy-payload read.
…gate The proxy returned NextResponse.next() for any request it classified as a framework background fetch — on the unauthenticated path and, via redirectToLogin, on the blocked-status and RBAC-forbidden paths too. But the signals that classify a request as background (RSC, Next-Router-Prefetch, Next-Router-State-Tree, Purpose, Sec-Purpose) are plain request headers and therefore client-forgeable. Sending `RSC: 1` was enough to walk past the middleware's auth, account-status, and RBAC checks on any protected route and have the page's server components execute for an unauthenticated caller. - an unauthenticated background request now gets a bare 401 with `Cache-Control: no-store, no-cache`, matching nest-auth byte for byte. The redirect is still avoided (it would poison the client router cache), but the request is refused rather than admitted. - redirectToLogin no longer short-circuits on background requests, so blocked and forbidden callers get their refusal whatever headers they send. - detect Next-Router-State-Tree as a background signal, which nest-auth already treats as one. It carries a serialised tree, so presence is the signal. The classifier's doc now states the result is a hint about response shape only, never grounds to admit a request. Tests pin each path, including the headline case: a forged RSC header on a protected route with no session receives 401, not next(). Reverting the fix fails exactly these tests.
The brute-force lockout identifier is an HMAC of the email, and login, register, password reset, email verification, and platform login all derived it from the caller's raw input. Each casing of one address therefore had its own failure budget while resolving the same account, so rotating the spelling — user@x.com, USER@X.COM, User@X.Com — handed an attacker an unbounded supply of attempts and the lockout never fired. The same split let a single account own several concurrent OTP, cooldown, and reset records. Add a normalize_email helper (trim, then lowercase) and apply it at the engine boundary of every flow that accepts an address, before any identifier, lookup, or stored record is derived from it. Full Unicode lowercasing, not ASCII-only: nest-auth canonicalizes with JavaScript's Unicode-aware toLowerCase() and the two backends share one Redis, so an ASCII-only fold would make a non-ASCII address canonicalize differently per backend and split its keyspace. The invitation flow, which already normalized inline with to_ascii_lowercase, now routes through the same helper so that divergence cannot reappear. Password reset normalizes on both the initiate and confirm legs: the confirm step compares the supplied address against the one stored in the reset context, so canonicalizing only one side would break every reset. Tests spend the whole lockout budget across five different casings and assert the next attempt is already locked, and pin that an account still authenticates when the caller types it in a different case.
The two backends HMAC the same Redis identifiers with a key each derives from the JWT secret, but they derived it differently, and either difference alone was enough to make the keys disagree: - this side hashed `label || secret` with no separator, leaving the preimage ambiguous, while nest-auth hashes `label + ":" + secret`. - this side keyed the HMAC with the raw 32-byte digest, while nest-auth keys it with the 64-character hex TEXT of that digest. The result was that every keyed identifier landed in a different Redis slot per backend: brute-force lockout, OTP, resend cooldown, MFA setup, and the anti-replay record. On a shared Redis a lockout accrued through one backend was invisible to the other, so an attacker could halve the effective attempt budget by alternating which backend they hit. Adopt nest-auth's derivation verbatim. It is the published one (npm v1.0.11), so changing it there instead would have briefly cleared every in-flight lockout on live deployments — a worse trade for no cryptographic gain, since a hex-text key and a raw-byte key are equally sound. The separator is kept because the domain separation it provides is real. The key is written straight into a fixed-size buffer, so no heap copy of the key material outlives the derivation; the secret buffer and the intermediate digest are both zeroized. Both repos now carry the same known-answer vectors — a fixed secret, its derived key, and one identifier — so a drift on either side turns exactly one suite red instead of surfacing later as sessions and lockouts that silently miss each other.
The MFA temp token outlives the login-time status gate by its whole TTL, so an account blocked in that window could still clear the second factor and receive a full session. Revoking access must not depend on how far through the login the holder already was. Re-check the status once the account is loaded, before the code is verified, on both the dashboard and platform challenge paths. Ordering matters twice: a blocked account must be refused whatever it submits, and the recovery-code path costs one key derivation per stored code, so gating first also denies a revoked account that work. Extract the status gate into a shared status_gate module. There were already two copies of the status-to-error mapping (dashboard login and platform login) and this would have been a third; one implementation means the three planes cannot drift into different notions of "blocked". MfaService now carries the configured blocked statuses, wired from the resolved config by the builder.
Four Redis-contract divergences, one of which was a security bug rather than a
parity gap.
Session-index members were the bare token hash; nest-auth stores full key
suffixes (`rt:{hash}`, `prt:{hash}`) and also indexes the rotation grace
pointers (`rp:`/`prp:`). A bare hash cannot say which keyspace it belongs to, so
revoke_all was structurally unable to delete grace pointers: a refresh token
that had just been rotated away survived a logout-all for its entire grace
window and kept recovering sessions from it. Members now carry their own
prefix, the Lua deletes `{ns}:{member}` directly, and list_sessions filters to
the live prefix and strips it before building the detail key. The separate
`psess:`/`psd:` platform keyspace stays — only the member format converged.
`sd:`/`psd:` timestamps were RFC 3339 strings; nest-auth writes Unix
milliseconds and its listing rejects any record whose timestamps are not
numbers — then SREMs the member. So the divergence was not merely unreadable,
it was destructive: nest-auth evicted sessions this backend had written. Added
a `unix_millis` serde adapter for those two fields. `SessionRecord.created_at`
deliberately keeps RFC 3339, which is what nest-auth writes there.
Password-reset prefixes `pr:`/`prv:` renamed to `pw_reset:`/`pw_vtok:`, so a
reset link emailed by one backend resolves against the other.
The stored invitation gained `createdAt`, which nest-auth writes and validates.
Consumption is a single-use GETDEL, so nest-auth read an invitation written
here, failed validation on the missing field, and the token was already gone —
the invitation was destroyed instead of accepted. Encoding verified against
nest-auth rather than assumed: it writes an ISO string there, not the epoch
milliseconds the session detail uses.
The spec already described the prefixed-member format at §12.5; the
implementation had drifted from its own specification, not only from nest-auth.
…ts legacy tokens
Two cross-backend credential formats that did not line up.
The TOTP secret was encrypted as raw bytes; nest-auth encrypts the Base32 text.
Both backends read the same `mfaSecret` column and the same `mfa_setup:` record,
so decrypting a nest-written secret here handed Base32 ASCII to HMAC-SHA-1 as
the key and rejected every code the user's authenticator produced — and the
reverse broke the same way. Encrypting the presentation form is marginally
redundant, but the published side already stores it that way and re-encrypting
live MFA credentials to save twelve bytes is not a trade worth making. A
`decrypt_secret` helper now owns the decrypt-then-decode step so no call site
can reintroduce the mismatch, and every failure still collapses to one opaque
error rather than a format oracle.
The refresh-token shape guard accepted only the 256-bit hex form. nest-auth
minted UUID v4 before both sides converged, and those tokens live in the shared
Redis for a full refresh lifetime — seven days by default — so refusing the
shape refused to rotate sessions that were still valid with their `rt:{sha256}`
record right there. The legacy shape is accepted alongside the current one; it
grants nothing by itself, since the token still has to hash to a stored session.
Tests pin that the allowance is one exact shape and not "anything with dashes".
Everything this branch converged — key prefixes, index member shapes, record encodings, credential formats, the derived identifier key — is a contract between two implementations that can back the same deployment over one Redis. Nothing enforced it, which is how the divergences reached this point: each side was internally consistent and green. Add `conformance/wire-contract.json`, held byte-identical in both repos, and assert this implementation against it. The prefix catalog is checked against the `Prefix` enum, and the identifier-key known-answer vectors now come from the contract instead of being repeated in the test, so there is one copy of the truth rather than two that drift apart quietly. The contract records the parts that are easy to get wrong from either side: the two identity planes must never share an index prefix; the timestamp encoding is deliberately NOT uniform (the session detail is epoch milliseconds because the reader guards on a numeric type and evicts a member whose detail fails to parse, while everything else is an ISO string); `mfaEnabled` must ride on the refresh session or a rotation silently disables the second factor; and the invitation needs `createdAt` because a single-use GETDEL destroys a record the reader rejects. Changing a value there is a breaking change to the shared keyspace and has to land in both repos together.
Seven response-shape divergences from the published sibling, whose client
contract is already fixed. Two of them were more than cosmetic.
- logout accepted the refresh token only from a cookie. In a bearer deployment
there is no cookie, so the refresh session survived logout entirely — the
access token was blacklisted while the credential that mints new ones stayed
live. It now reads a body `refreshToken` as nest-auth does, cookie as fallback.
- POST /auth/mfa/challenge required the temp token in the body with no cookie
fallback, while this crate's own OAuth callback plants that token as an
HttpOnly cookie. The browser OAuth+MFA flow was therefore a dead end: the SPA
cannot read the cookie and the body was empty. The field is now optional with
the cookie behind it, a request carrying neither fails with the MFA temp-token
error rather than a generic validation 400, and the cookie is cleared on the
same policy nest-auth uses.
The rest are shapes a shared client would break on: GET /auth/me and
/auth/platform/me return the safe account as the top-level body instead of
wrapping it in `{ user }`; GET /auth/sessions returns the bare array instead of
`{ sessions }`; the platform auth body names the account `admin` and is
unconditionally bearer; both refresh endpoints echo the account alongside the
tokens; and both MFA setup routes answer 201.
The platform field name and the always-bearer delivery were already what this
crate's own specification called for at §7.11.1 — the implementation had drifted
from its spec, not only from nest-auth.
Also fixes the namespaced-key catalog test, which enumerates every allowed
prefix and so still listed the pre-rename `pr:`/`prv:` reset keys. It only
surfaced once a Docker daemon was available to run the Redis integration tier.
Two round-trip tests unwrapped `serde_json::from_str` with `?` on a multi-line call, which puts the operator's error arm on a line of its own. The literal being parsed always succeeds, so that arm is unreachable and the CI gate — which fails under 100% line coverage — tripped on it. The gate only surfaced this once the Docker-backed Redis tier was actually executing, since before that the run aborted earlier. Assert on the `Result` with `matches!` instead, which is the idiom the rest of this codebase already uses for exactly this reason and keeps the same assertions. Line coverage is back to 100.00% (16971/16971) with functions at 100%; `cargo llvm-cov --fail-under-lines 100` exits clean.
…eyspace Brings PR #71 (fix/refresh-reuse-detection) into the parity work and reconciles it with the keyspace this branch rewrote: - revoke_family prunes the PREFIXED session-index member (`rt:{hash}` / `prt:{hash}`), not the bare hash. The index format changed under the branch; pruning a bare hash would leave every revoked session listed until the index itself expired. - The e2e catalog keeps `pw_reset:`/`pw_vtok:` (the branch still carried the old `pr:`/`prv:`) and gains `cf`/`fam`/`pcf`/`pfam`/`ep`/`pep`. - SessionRecord carries both `mfaEnabled` (this branch) and `familyId` (the merged branch); both are omitted-or-defaulted for legacy records. Replaying a consumed token after a revoke-all is now reported as Reused rather than Invalid: the `cf:` marker deliberately outlives both the grace pointer and the revoke-all, so a replayed consumed token stays a theft signal even after the user logged everything out. It still never mints a session.
Reuse detection deletes the family's live sessions, but a grace pointer planted by an EARLIER rotation of the same lineage can still be inside its window when the reuse fires: detection only proves the REPLAYED token's own pointer expired, which says nothing about a younger sibling's. Recovering from that pointer minted a fresh session carrying the revoked family id and handed the thief back the lineage the revocation had just killed. The rotation script now decodes the recovered record and refuses the recovery when its family index is gone, so a recovery is valid only while its lineage is alive. A legacy record with no family recovers as before. The e2e test builds the three-token lineage and fails without the guard (verified by reverting it). Also closes the two uncovered lines the merge left in the failing-revoke test double: llvm-cov is back to 100% lines and 100% functions.
… the grace window Three changes that converge the rotation semantics with nest-auth: - The scripts no longer decode a stored record. The grace record's family and the family owner's id are parsed by the caller instead, with a real parser. nest-auth drives its end-to-end tier against an in-memory Redis whose Lua VM has no `cjson`, so a script that decodes JSON is one the shared contract cannot be exercised against on that side. - The grace window is now single-shot: the pointer is consumed on use. It used to be served on every request inside the window, so one captured consumed token could mint a fresh session over and over for the whole window. nest-auth already consumed it; this closes the divergence in the stricter direction. - The family-alive check on a grace recovery moves from the script into the store, where it reads the same. The revocation is monotonic, so checking it next to the recovery is no weaker than checking it inside the script — the session is created after the script returns either way.
Mirror of the nest-auth change: the shared contract gains the six new prefixes, the bare-hash family member shape, `familyId` on the session record, the `epoch` claim, and the rotation semantics both sides implement. The catalog test now asserts every prefix in the enum rather than a subset, so adding a keyspace on one side without the other turns this side red. Spec sections 7.3.3 / 12.4 / 12.5 are brought in line with the code: the rotation flow as it now runs (single atomic script, single-shot grace, reuse detection, family revocation), the new catalog rows, and the renumbered Lua subsections.
The three route handlers that bridge the browser's cookie session to the backend had no tests at all — 0% of `handlers.ts`, in the one module of this package that writes `Set-Cookie` back to a browser. Nine tests pin the contract that matters there: the rotated cookies are relayed verbatim, a failed refresh clears the session rather than leaving the browser holding cookies the backend no longer honors, a backend that is down fails the same way as a rejected refresh, and an attacker-supplied `redirectTo` cannot become the `Location`. The package now runs under a coverage ratchet — thresholds pinned just under what the suite reaches (86/78/90/88), so this layer can only go up. It is deliberately not 100%: the Rust crates are, this layer is the laggard, and a threshold that lies about where it is would be worse than one that admits it. CI runs `test:cov` instead of `test`. Coverage on the package: 68.7% -> 86.06% statements, and `handlers.ts` from 0% to 100% lines.
The parity half of the nest-auth change. `SameSite=None` is the one posture where the browser attaches the session cookie to a cross-site request, and it is the one this adapter had no second line of defense for. A tower layer, innermost so it sees the request exactly as the handler would, decides on headers a page cannot forge: safe methods pass, a request carrying no auth cookie passes (a bearer client has no ambient credential to spend), `Sec-Fetch-Site: same-origin`/`none` passes, and an `Origin` must appear in `cookies.trusted_origins`. Neither header at all is a non-browser caller and passes — a page cannot make a browser omit `Origin` cross-site, so the absence is evidence, not evasion. The request's own origin is never rebuilt from `Host`. Config validation refuses either half without the other, and refuses an entry that is not a bare absolute origin: a trailing slash, a path, a naked hostname or embedded userinfo can never equal an `Origin` header, so the origin the operator meant to allow would be silently blocked instead. The origin shape is checked with the crate's existing hand-rolled URL helpers rather than pulling in `url` — the dependency budget is a feature here. New `AuthError::UntrustedOrigin` / `auth.untrusted_origin`, 403, byte-identical to the nest-auth code.
The parity half of the nest-auth change: `PasswordBreachChecker` is consulted at the three places a password is set — register, reset, invitation acceptance — and never at login, where refusing a breached password someone already has would lock them out of the account they need to get into to change it. A seam, not a dependency. The default `AllowAllBreachChecker` approves everything and touches no network, so a crate upgrade never starts contacting a third party. The bundled `HibpBreachChecker` (feature `breach`) runs over the crate's existing `HttpClient` seam, so enabling the check pulls in no HTTP stack of its own — the deployment supplies the transport it already has. Only SHA-1 is added, behind the same feature, and it is the corpus's index rather than a security primitive: storage is still the configured KDF. The trait returns `bool`, not `Result`, because fail-open is the contract and not a policy the caller gets to choose: a corpus that is down, rate-limiting or answering garbage must approve the password. There is no error the engine would be right to act on. Also covers three lines the origin-check work left uncovered: a legacy record still recovers through its grace window (there is no family to check), and the family-owner walk skips a member whose record is gone, unreadable, or names no owner at all — an empty owner would build an index key every ownerless family would share. New `AuthError::PasswordCompromised` / `auth.password_compromised` (400), byte-identical to nest-auth. Coverage back to 100% lines and functions.
This adapter already enforces its limits — a governor layer per route, not a recommendation — so there was nothing to add. What was missing is the agreement: 21 numbers duplicated across two repos with nothing checking they matched. The contract now carries the table and both sides assert against it, so a limit changed on one side turns that side red rather than surfacing as the same client being throttled at different points depending on which backend answered. The storage difference is written down as deliberate and outside the contract: nest-auth counts in Redis and therefore shares the limit across instances, this adapter counts in process memory and therefore does not. Under horizontal scale the Redis one is the stricter of the two.
The parity half of the nest-auth change. `refresh_expires_in_days` bounds a single refresh token, not a session: a client rotating every fifteen minutes renews that lifetime indefinitely, so a session established once never has to be established again. `family_created_at` is stamped at login and carried unchanged through every rotation — deliberately not `created_at`, which is this session's own and resets each time; measuring from that would make the cap unreachable while looking like it worked. The rotation is refused once `jwt.absolute_session_lifetime_days` has passed, before the script runs, so nothing is consumed on the holder's behalf, and refused as a plain invalid refresh: the remedy is the same as any other, and a distinct code would only tell whoever holds the token how old the session is. Off by default, matching nest-auth: switching it on ends sessions already older than the cap, which is a deployment's decision rather than an upgrade's. A record written before the field carries no birth time and is not capped. The serde adapter is `Option`-aware so a legacy record round-trips as `None` instead of failing the whole record — but a present-and-malformed value is still an error, because a birth time that cannot be read is a cap that cannot be judged, and dropping it quietly would uncap the session.
… profile `create_with_oauth` was called with `email_verified: Some(true)` unconditionally, and `OAuthProfile` had no field for a provider to say otherwise. No bug today — the only bundled provider is Google's, which refuses an unverified profile before building one — but the first third-party provider written against that contract would have created a verified account from an address nobody proved they owned. That account belongs to whoever controls the OAuth account, not to whoever controls the mailbox, and marking it verified makes the consumer's "this email is proven" invariant false from the first login — which is how an account is taken over by registering with someone else's address at a provider that does not check. The profile now carries `email_verified` and the engine passes it through. Google's provider reports `true` unconditionally, which is honest precisely because its own guard already refused everything else. `MockOAuthProvider` gains an `unverified` constructor so the path can be driven end to end — the bundled provider structurally cannot produce it. Parity with the nest-auth change; the field cannot be defaulted to `true` without reintroducing exactly the assumption this removes.
Reuse detection, the token epoch, the absolute session lifetime, the cross-site check, the breach checker and the contract-pinned rate limits — all of which have a nest-auth counterpart, which is the point.
The previous commit swept in a live `cargo mutants --in-place` mutation: the token source had been replaced with `None`, which would have made every cookie-and-bearer read return nothing. Restores the real match. Lesson recorded in the run itself — `--in-place` mutates the working tree, so nothing may be staged while it runs.
Six survivors from `cargo mutants`, each a real gap rather than an equivalent: - `clear_session` could be replaced by a no-op: the logout test asserted no cookie carried a value, which an absent `Set-Cookie` satisfies just as well. It now asserts each cookie is expired back on the Path it was set with — a clearing header on the wrong path leaves a ghost cookie the browser keeps sending. - `clamp_secs` and `refresh_max_age_secs` could return any constant: no test read a cookie's Max-Age. A session cookie instead would log every user out when they closed the tab. - The refresh-name comparison in `carries_auth_cookie` could be inverted unnoticed: every cross-site test carried the access cookie, so the second clause was never decisive. Covered now from both sides — the refresh cookie alone is a credential, the session-signal cookie alone is not. - `forgot_password` and `resend_otp` could be replaced by an empty 200: the anti- enumeration test read only the status, and the two-step flow *skipped* itself when no OTP was minted. The uniform body is asserted, and a missing OTP now fails. `DEFAULT_MAX_BODY_BYTES` was asserted against itself, which accepts whatever the expression produces; it is pinned to the literal.
…uivalents
The v4 layout test drew a single UUID, so a broken mask left the version and variant
nibbles correct often enough to pass by luck — two of the four bit-twiddling mutants
survived on a lucky draw. It draws 64 now, which makes the assertion deterministic.
`AuthEngine::platform_auth` could return `None` unnoticed: every platform test recovers
the service with `let Some(..) else { return }`, so the whole tier skipped itself instead
of failing. It is asserted from an engine built the same way the disabled-domain test
builds one, where nothing can skip.
Three mutants are genuinely equivalent and are recorded in `mutants.toml` with the reason
each cannot be killed — the XOR/OR pair on already-masked bits, the builder whose
`Default` calls the body being replaced, and the `"inactive"` arm the wildcard already
answers identically. The patterns stay narrow so the killable mutants of the same
functions are still generated.
The fake answered `Ok(())` and the test asserted only that — so a repository that stored nothing would have let every platform test built on it pass while asserting nothing. Each update is now read back through `find_by_id`.
`token_key` could be replaced by a constant without failing anything: every test stored and consumed one token at a time, so a colliding key round-trips perfectly. Two live tokens are now asserted not to collide. The fake's `remaining_lockout_secs` guarded on a counter being positive, which it always is — `record_failure` inserts and increments under one lock and `reset` removes the entry — so the entry's existence was already the whole condition.
The enrolment gate is built on exactly those two operations — the first writer wins the setup slot, the completion reads it away so only one caller can finish — and neither was asserted against the double. A double that swallowed the value or always reported "already there" would make that gate untestable while every test stayed green.
The secret-entropy rule was only exercised well away from its floor, so tightening the comparison to reject a secret *at* 3.5 bits/char would have gone unnoticed — the boundary secret is built to land on the constant exactly, with no rounding. `mfa_encryption_key` could return a fixed or zeroed key without failing anything: the MFA tests only ever round-trip through the same resolver, so every deployment's TOTP secrets sealed under one shared key would still decrypt. It is asserted byte for byte, and absent when no MFA is configured.
Every branch answers `Ok(())` by design — that uniformity is the anti-enumeration contract — so the response cannot say which one ran, and the test asserted only the response. Both guards could be inverted unnoticed: the cooldown gate (first resend silently dropped, a resend inside the window sent) and the verified check (mail to accounts that already verified, none to those that need it). The OTP record distinguishes them.
…uard Three conditions whose two sides were never separated: - the verification gate had no verified-account case under `required = true`, so an either-or would have locked out every verified user the moment the flag was switched on; - the rehash guard was only ever exercised with both halves true, where `&&` and `||` read alike — a current hash is now asserted untouched after a login; - the invitation's structural check is three independent reasons and only one was ever tripped at a time, so each field is now broken on its own.
…tarted Both entry points canonicalize the address before anything derives a key from it, and every test used one spelling throughout — so dropping the canonicalization, and breaking every reset where the user types their address differently on the two screens, was invisible.
The direct-OTP path also proves the reset applied — the stored hash changes — which is what a body replaced by `Ok(())` cannot fake. The bridge now runs under three different spellings of the same address, one per step.
…fication Three gaps on the password-reset path: - the token flow planted its own token and reset with that, so an `initiate_reset` that stored and sent nothing still passed. It now uses the token a real recipient receives — the only version of the flow where the store write and the mail are both load-bearing. - the `after_password_reset` notification was never asserted. The projection that feeds it fails open (the reset has already succeeded), so a skipped hook is silent — and this is the "your password changed" mail, the one signal a takeover victim gets. - `resend_reset_otp` canonicalizes its address like the other two entry points, and every branch answers `Ok(())`, so only the OTP record shows the canonicalization happened.
Without canonicalization the uniqueness check misses a different spelling entirely: the tenant ends up with one row per casing and a later lookup resolves to whichever it happens to hit. The duplicate test used one spelling twice, where the guard reads the same either way.
The index is returned by a search over the very vector being spliced, cloned from the same `AuthUser` value, so the guard can never be false and `<=` reaches the identical `remove`. It stays in the source as insurance against a future refactor deriving the index elsewhere — a panic there would be a denial of service on the MFA challenge path.
…istration `is_totp_code` routes a submitted code to the TOTP verifier or the recovery-code scan, and both halves matter — six characters that are not digits, and digits that are not six characters, are both recovery-shaped. It is asserted directly because both paths answer the same `MfaInvalidCode`, so a misrouted code is invisible from outside. The challenge's session registration was equally invisible: the returned tokens look the same whether or not the session was recorded, and an unregistered session is missing from the session list, from the cap, and from sign-out-everywhere.
Turning off a second factor is exactly what an attacker holding the password does, so the mail and the hook are the owner's only warning — and both are fire-and-forget, so `disable` returns the same `Ok(())` whether they fired or not. The harness now takes an email provider and hooks so those notifications can be observed.
Replacing the old set is how an attacker locks the real owner out of their own fallback, and the notification for it was as unobserved as the disable one — same fire-and-forget shape, same uniform `Ok(())`.
A second factor appearing on an account is the owner's cue that either they did it or someone else did, and its notification was as unobserved as the other two. One test now walks enable → regenerate → disable and asserts every alert on the way.
2^14 is the documented minimum, not the first rejected value — and only a config on the boundary separates `<` from `<=`. Tightening it would reject the very parameters the constant advertises.
The dynamic-truncation assembly ORs four byte lanes shifted into disjoint bit ranges, so no two operands share a set bit and XOR produces the identical word. The `| with &` mutants of the same expression are not equivalent and the RFC 4226 vectors kill them.
`is_legacy` short-circuits `needs_rehash`, and a legacy `scrypt:hex:hex` string can never parse as a current PHC — so the fallback it skips answers `true` for exactly the same inputs. The `with true` mutant is not equivalent and is killed.
Every malformed-shape case went through `verify`, where a rejected parse and a wrong password both answer `Ok(false)` — so the three independent guards were indistinguishable and the key-length cap had no boundary. Asserted directly now: each field empty on its own, 64 bytes accepted, 65 rejected.
`(hi << 4) | lo` combines a high nibble with a value `hex_nibble` bounds to 0..=15, so the two never share a set bit and XOR writes the identical byte.
nest-auth's legacy corpus carries both cases, and a decoder that dropped the A–F arm would reject the upper-case rows as malformed — invisible through `verify`, where that looks exactly like a wrong password.
The system-clock path was asserted only with a token expiring at `i64::MAX`, which a clock stuck at the epoch accepts just as happily. A token that expired a minute ago under the real clock is the case that separates them — and it is the one that matters, because the failure mode is accepting every expired token forever.
…e they killed Re-running the sweep against the fixes showed four assertions that never touched their subject: - the builder's refresh-lifetime arithmetic feeds the *session service*, which uses it only on rotation — the login assertion was reading the token manager's separate copy. Pinned through an engine-built rotation. - `register`'s canonicalization was asserted by a duplicate conflict, but the in-memory repository compares case-insensitively, so the conflict proves nothing. The *stored* address is asserted instead — on a store that compares bytes, a row keyed by whatever the user typed resolves to a different identity. - the resend canonicalization was asserted on an account whose OTP an earlier `initiate` had already filed. It uses a fresh account, with the absence asserted first. - the challenge's session registration was asserted by the session list, which the token manager fills on its own. The new-session hook is the observation point that actually depends on the session service.
… one The registration at the top of the test had already fired the hook once, so asserting its presence held even with the challenge's own session registration removed. Counted across the challenge now, and red-checked: the test fails with `enforce_session_limit` short-circuited and passes without.
Three habits of mine cost coverage the repo holds at 100%: an `if let` whose `None` arm
never runs, a `let ... else { return }` split across lines (the same statement on one line
counts as covered), and an `assert!` failure message on its own line, which is only
evaluated when the assertion breaks. The two new email doubles also get their untouched
methods exercised, the way `FailingResetEmail` already did.
There was a problem hiding this comment.
Pull request overview
Aligns this crate with nest-auth so both implementations can safely share a single Redis deployment, while adding multiple security hardenings (origin enforcement, breached-password checks, session lifetime bounds, JWT epoch revocation, OAuth email verification semantics) and pinning cross-implementation contracts in code/tests.
Changes:
- Introduces/updates shared wire contracts (Redis key prefixes, rotation semantics, rate limits, claim fields) and enforces them via tests.
- Hardens auth flows: refresh reuse detection + family revocation, per-user token epoch invalidation, absolute session lifetime, origin checks for cookie-authenticated writes, and breached-password refusal (opt-in).
- Expands test coverage and CI gating (Vitest coverage ratchet, mutation config location fix, additional behavioral tests across Rust + TS packages).
Reviewed changes
Copilot reviewed 92 out of 92 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Documents new security features supported by the crate. |
| packages/rust-auth/vitest.config.ts | Adds coverage thresholds (“ratchet”) for the TS package. |
| packages/rust-auth/tests/nextjs.test.ts | Adds tests for background-request hardening and isBackgroundRequest. |
| packages/rust-auth/tests/handlers.test.ts | Adds full tests for Next.js cookie-bridging handlers (silent refresh/logout/client refresh). |
| packages/rust-auth/src/shared/jwt-payload.types.ts | Adds epoch to JWT payload types exported to TS consumers. |
| packages/rust-auth/src/shared/error-codes.ts | Extends error-code union with breached-password + untrusted-origin codes. |
| packages/rust-auth/src/nextjs/proxy.ts | Refuses unauthenticated background requests with a non-cacheable 401; expands background signals. |
| packages/rust-auth/package.json | Adds test:cov script for coverage runs. |
| mutants.toml | Removes root-level mutants config (moved under .cargo/). |
| crates/bymax-auth-types/tests/error_catalog.rs | Adds new PasswordCompromised + UntrustedOrigin to the error catalog test list. |
| crates/bymax-auth-types/src/error.rs | Adds new error variants/codes + HTTP status and message mapping. |
| crates/bymax-auth-types/src/claims.rs | Adds epoch claim to dashboard/platform claims with serde default + tests. |
| crates/bymax-auth-redis/tests/common/mod.rs | Adds raw Redis helpers for parity testing (set raw values, sorted set members). |
| crates/bymax-auth-redis/src/stores/single_use.rs | Renames password-reset key prefixes (pw_reset / pw_vtok). |
| crates/bymax-auth-redis/src/script.rs | Registers new revoke_family Lua script. |
| crates/bymax-auth-redis/src/lua/session_revoke.lua | Updates session-index member semantics to use full key suffixes. |
| crates/bymax-auth-redis/src/lua/revoke_family.lua | Adds family revocation Lua script for reuse detection response. |
| crates/bymax-auth-redis/src/lua/refresh_rotate.lua | Adds reuse detection + single-shot grace pointer; avoids JSON decode in Lua. |
| crates/bymax-auth-redis/src/lua/invalidate_user_sessions.lua | Updates revoke-all to delete members by key suffix and handle detail deletion correctly. |
| crates/bymax-auth-redis/src/keys.rs | Adds new prefixes (epoch, family markers/indexes) + contract tests vs wire-contract.json. |
| crates/bymax-auth-jwt/src/keys.rs | Updates tests for new epoch claim presence. |
| crates/bymax-auth-jwt/src/hs256.rs | Adds expiration realism test + includes epoch in signing/roundtrip tests. |
| crates/bymax-auth-crypto/src/password/tests.rs | Adds parser bounds tests + asserts floor inclusivity. |
| crates/bymax-auth-crypto/src/mac.rs | Adds SHA-1 helper behind breach feature with doc + example. |
| crates/bymax-auth-crypto/Cargo.toml | Adds breach feature for SHA-1 dependency gating. |
| crates/bymax-auth-core/tests/engine_assembly.rs | Updates HMAC key length expectation; adds breach-checker wiring test. |
| crates/bymax-auth-core/src/traits/oauth.rs | Adds email_verified to OAuth profile contract. |
| crates/bymax-auth-core/src/traits/mod.rs | Exposes breach-checker trait/default + optional HIBP checker re-export. |
| crates/bymax-auth-core/src/traits/hooks.rs | Updates test fixture OAuth profile with email_verified. |
| crates/bymax-auth-core/src/traits/breach.rs | Introduces breach-checker seam + HIBP implementation (feature-gated) and tests. |
| crates/bymax-auth-core/src/status_gate.rs | Centralizes blocked-status gating logic shared by flows/services. |
| crates/bymax-auth-core/src/services/session.rs | Extends session record fixtures + adds metadata composition tests and new store methods in double. |
| crates/bymax-auth-core/src/services/platform.rs | Canonicalizes email before lockout/repo lookup; uses shared status gate; HMAC key size update; adds tests. |
| crates/bymax-auth-core/src/services/password.rs | Wires breach checker into password setting flows; adds sentinel timing test; updates constructor signature/tests. |
| crates/bymax-auth-core/src/services/oauth.rs | Persists provider email_verified; adds tests guarding OAuth state binding and unverified-email behavior. |
| crates/bymax-auth-core/src/services/mod.rs | Adds legacy UUID refresh-token shape acceptance and strengthens UUID layout test. |
| crates/bymax-auth-core/src/services/mfa/setup.rs | Switches to secret decode path; revokes other sessions on MFA enable. |
| crates/bymax-auth-core/src/services/mfa/mod.rs | Adds blocked-status recheck support; updates identifier key size; adds secret decrypt helper and encoding parity. |
| crates/bymax-auth-core/src/services/mfa/manage.rs | Uses secret decrypt helper; revokes other sessions on MFA disable. |
| crates/bymax-auth-core/src/services/mfa/challenge.rs | Re-checks blocked status at challenge time; uses secret decrypt helper; adds TOTP code predicate tests. |
| crates/bymax-auth-core/src/services/auth/session_ops.rs | Updates test claims to include epoch. |
| crates/bymax-auth-core/src/services/auth/register.rs | Canonicalizes email; adds breached-password refusal on registration; adds casing/uniqueness assertions. |
| crates/bymax-auth-core/src/services/auth/mod.rs | Ensures session projection includes mfa_enabled (and explicitly excludes family id). |
| crates/bymax-auth-core/src/services/auth/login.rs | Canonicalizes email; uses shared status gate; adds tests for lockout bypass prevention and TTL arithmetic. |
| crates/bymax-auth-core/src/services/auth/invitation.rs | Canonicalizes email via shared helper; adds created_at to invitation; adds breached-password refusal on accept; adds malformed-field tests. |
| crates/bymax-auth-core/src/services/auth/email_verification.rs | Canonicalizes email; strengthens resend cooldown behavior assertions. |
| crates/bymax-auth-core/src/services/auth/detached.rs | Adds tests ensuring each detached wrapper invokes the correct hook/email send. |
| crates/bymax-auth-core/src/services/adapter_api.rs | Adds epoch to WS ticket claims; strengthens role/status/session delegation tests. |
| crates/bymax-auth-core/src/providers/google.rs | Adds email_verified field; adds tests for authorize URL query correctness. |
| crates/bymax-auth-core/src/normalize.rs | Adds centralized email canonicalization helper + tests (Unicode-aware). |
| crates/bymax-auth-core/src/lib.rs | Exposes normalize_email; registers new internal modules. |
| crates/bymax-auth-core/src/error.rs | Adds config validation errors for trusted-origin configuration. |
| crates/bymax-auth-core/src/engine/mod.rs | Updates password-reset store docs to new prefixes. |
| crates/bymax-auth-core/src/engine/builder.rs | Adds breach-checker wiring; propagates absolute session lifetime; passes blocked statuses into MFA service; updates HMAC key length tests. |
| crates/bymax-auth-core/src/config/mod.rs | Adds absolute session lifetime and trusted-origins config fields + defaults. |
| crates/bymax-auth-core/Cargo.toml | Adds breach feature and includes it in full. |
| crates/bymax-auth-client/src/lib.rs | Updates /auth/me parsing to match unwrapped safe-user body. |
| crates/bymax-auth-axum/tests/redis_e2e.rs | Updates response shapes for me and sessions list; makes platform routes bearer-only; asserts status codes and cookie absence. |
| crates/bymax-auth-axum/tests/common/mod.rs | Adds trusted-origins test harness wiring + updates session-store trait methods. |
| crates/bymax-auth-axum/src/trusted_origin.rs | Adds trusted-origin enforcement middleware for cookie-authenticated unsafe methods. |
| crates/bymax-auth-axum/src/test_support.rs | Includes trusted origin list in resolved config for tests. |
| crates/bymax-auth-axum/src/state.rs | Adds trusted origins into resolved cookie configuration; pins max-body-bytes literal. |
| crates/bymax-auth-axum/src/routes/sessions.rs | Returns bare array response for session listing (parity). |
| crates/bymax-auth-axum/src/routes/platform.rs | Makes entire platform group bearer-only; adjusts me, logout, refresh behavior and response shapes. |
| crates/bymax-auth-axum/src/routes/platform_mfa.rs | Aligns MFA setup status code to 201 (parity). |
| crates/bymax-auth-axum/src/routes/mod.rs | Adds extractor for platform bearer token independent of delivery mode. |
| crates/bymax-auth-axum/src/routes/mfa.rs | Aligns MFA setup status code; makes MFA temp token optional in DTO with cookie fallback; clears MFA temp cookie per policy. |
| crates/bymax-auth-axum/src/routes/auth.rs | Sources refresh token from cookie or body on logout; refresh now echoes user via AuthResult (parity). |
| crates/bymax-auth-axum/src/router.rs | Wires trusted-origin middleware with state; carries trusted origins into resolved config. |
| crates/bymax-auth-axum/src/rate_limit.rs | Pins default rate limits to wire-contract.json via tests. |
| crates/bymax-auth-axum/src/middleware.rs | Adds trusted-origin enforcement into middleware stack. |
| crates/bymax-auth-axum/src/lib.rs | Registers trusted-origin module. |
| crates/bymax-auth-axum/src/extractors/platform.rs | Forces platform tokens to come from bearer header only; updates tests. |
| crates/bymax-auth-axum/src/extractors/mod.rs | Adds shared platform bearer token sourcing helper. |
| crates/bymax-auth-axum/src/extractors/mfa.rs | Updates tests for new epoch claim presence. |
| crates/bymax-auth-axum/src/dto.rs | Makes MFA temp token optional (cookie fallback) with length caps. |
| crates/bymax-auth-axum/src/delivery.rs | Makes refresh delivery echo user; makes platform delivery always bearer; adds MFA temp cookie clearing helper + tests; makes /auth/me body unwrapped. |
| conformance/wire-contract.json | Adds byte-identical cross-implementation contract for keyspace, semantics, and limits. |
| bindings/bymax-auth-wasm/src/lib.rs | Updates WASM tests for new epoch claim presence. |
| bindings/bymax-auth-wasm/src/jwt_edge.rs | Updates edge verifier tests for new epoch claim presence. |
| .github/workflows/ci.yml | Fixes cargo-mutants invocation and runs Vitest under coverage thresholds. |
| .cargo/mutants.toml | Moves/expands cargo-mutants configuration and documents equivalent/untestable mutants. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+119
to
+130
| /// The legacy shape: a lower-case UUID v4, `8-4-4-4-12` hex with dashes at fixed offsets. | ||
| fn is_legacy_uuid_shape(raw: &str) -> bool { | ||
| const DASH_POSITIONS: [usize; 4] = [8, 13, 18, 23]; | ||
| raw.len() == 36 | ||
| && raw.bytes().enumerate().all(|(index, byte)| { | ||
| if DASH_POSITIONS.contains(&index) { | ||
| byte == b'-' | ||
| } else { | ||
| is_lower_hex(byte) | ||
| } | ||
| }) | ||
| } |
| * fixed HTTP status via [`AuthErrorCode::http_status`]. | ||
| */ | ||
| export type AuthErrorCode = "auth.invalid_credentials" | "auth.account_locked" | "auth.account_inactive" | "auth.account_suspended" | "auth.account_banned" | "auth.pending_approval" | "auth.token_expired" | "auth.token_revoked" | "auth.token_invalid" | "auth.refresh_token_invalid" | "auth.session_expired" | "auth.session_limit_reached" | "auth.session_not_found" | "auth.token_missing" | "auth.email_already_exists" | "auth.email_not_verified" | "auth.mfa_required" | "auth.mfa_invalid_code" | "auth.mfa_already_enabled" | "auth.mfa_not_enabled" | "auth.mfa_setup_required" | "auth.mfa_temp_token_invalid" | "auth.recovery_code_invalid" | "auth.password_too_weak" | "auth.password_reset_token_invalid" | "auth.password_reset_token_expired" | "auth.otp_invalid" | "auth.otp_expired" | "auth.otp_max_attempts" | "auth.insufficient_role" | "auth.forbidden" | "auth.invalid_invitation_token" | "auth.oauth_failed" | "auth.oauth_email_mismatch" | "auth.platform_auth_required" | "auth.validation" | "auth.too_many_requests" | "auth.internal"; | ||
| export type AuthErrorCode = "auth.invalid_credentials" | "auth.account_locked" | "auth.account_inactive" | "auth.account_suspended" | "auth.account_banned" | "auth.pending_approval" | "auth.token_expired" | "auth.token_revoked" | "auth.token_invalid" | "auth.refresh_token_invalid" | "auth.session_expired" | "auth.session_limit_reached" | "auth.session_not_found" | "auth.token_missing" | "auth.email_already_exists" | "auth.email_not_verified" | "auth.mfa_required" | "auth.mfa_invalid_code" | "auth.mfa_already_enabled" | "auth.mfa_not_enabled" | "auth.mfa_setup_required" | "auth.mfa_temp_token_invalid" | "auth.recovery_code_invalid" | "auth.password_too_weak" | "auth.password_compromised" | "auth.password_reset_token_invalid" | "auth.password_reset_token_expired" | "auth.otp_invalid" | "auth.otp_expired" | "auth.otp_max_attempts" | "auth.insufficient_role" | "auth.forbidden" | "auth.untrusted_origin" | "auth.invalid_invitation_token" | "auth.oauth_failed" | "auth.oauth_email_mismatch" | "auth.platform_auth_required" | "auth.validation" | "auth.too_many_requests" | "auth.internal"; |
The coverage badge still read `pre-release` and the testing section claimed 100% *region* coverage, which was never true — regions sit at 96.65%, and the gate the CI enforces is `--fail-under-lines 100`. Both now say what is measured, and a mutation badge points at the config that defines the gate. The configuration table was missing every option this cycle added: the absolute session lifetime, the trusted-origin allowlist, and the per-route rate limits. The two that are deliberately off by default now say why, and the breached-password checker gets the wiring example it needs — it rides the crate's own `HttpClient`, so a deployment supplies the transport it already has. `breach` joins the facade's feature list. The feature names on `bymax-auth` are a stable contract from the start even while the bodies are placeholders, so a capability that exists in `bymax-auth-core` and cannot be named through the facade is a gap in that contract. The README also gains the roadmap section its nest-auth counterpart has, and both security tables gain the five defences that landed here. The changelog's `[Unreleased]` covered the scaffolding and nothing since.
…goal The specification listed breach-check among the things the library deliberately does not do; it ships behind the `PasswordBreachChecker` seam now, opt-in, and the non-goal row would have sent a reader looking for a hook that already exists. The configuration schema gains `jwt.absolute_session_lifetime_days` and `cookies.trusted_origins`, and the startup-validation list gains the invariant that binds the allowlist to `SameSite::None` in both directions — either half alone fails quietly.
Comment on lines
+101
to
+103
| // The suite drives same-origin requests, which the origin check admits without | ||
| // consulting the list; a `SameSite::None` case names its own origin explicitly. | ||
| trusted_origins: vec!["https://app.example.com".to_owned()], |
Comment on lines
+52
to
+55
| let cookies = state.config().cookies.clone(); | ||
| if !carries_auth_cookie(&request, &cookies.access_name, &cookies.refresh_name) { | ||
| return next.run(request).await; | ||
| } |
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.
What this is
The rust half of a two-repo change.
nest-authand this crate can back the same deployment overone Redis, so anything touching keys, stored record shapes, JWT claims or Lua scripts is a
contract between them — and
conformance/wire-contract.jsonis held byte-identical in both, witha test on each side that reads it.
Companion PR: bymaxone/nest-auth#44.
Part 1 — parity and the credential path
Family-lineage reuse detection lands (PR #71's branch, reconciled with the keyspace this
branch rewrote).
revoke_familyprunes the prefixed index member (rt:{hash}), not the barehash — the index format changed underneath it, and pruning a bare hash would have left every
revoked session listed until the index itself expired.
Three findings, each red-checked (the test was confirmed to fail without the fix):
replayed token's own pointer expired; a pointer planted by an earlier rotation of the same
lineage can still be live. Recovering from it minted a session carrying the revoked family id.
A recovery now requires its family index to still exist.
window, so one captured consumed token could mint a session repeatedly. Single-shot now,
matching
nest-auth.Reusedrather thanInvalid:the
cf:marker deliberately outlives both the pointer and the revoke-all, so a replay stays atheft signal. It still never mints a session.
The rotation scripts no longer decode stored records.
nest-authdrives its end-to-end tieragainst an in-memory Redis whose Lua VM has no
cjson, so a script that decodes JSON is one theshared contract cannot be exercised against on that side. The grace record's family and the family
owner's id are parsed by the caller instead, with a real parser.
Part 2 — the five security items
SameSitecovers this forLax/Strict; it does not forNone.trusted_originsrequired withSameSite::NonePasswordBreachChecker, with a bundled HIBP implementation over the crate's existingHttpClientseam (featurebreach). Fails open by contract.AllowAllBreachCheckerrefresh_expires_in_daysbounds a token, not a session.email_verifiedon the OAuth profile.create_with_oauthwas called withSome(true)unconditionally.Item 2 adds no HTTP stack: the checker runs over the same seam the OAuth flows use, so a
deployment supplies the transport it already has. Only SHA-1 comes in, behind the
breachfeature, and it is the corpus's index rather than a security primitive — storage is still the
configured KDF. The trait returns
bool, notResult, because fail-open is the contract and nota policy the caller gets to choose.
Item 5 has no bug today — the bundled Google provider refuses an unverified profile before
building one — but the first third-party provider written against the old contract would have
created a verified account from an address nobody proved they owned.
The rate-limit storage difference is recorded in the contract as deliberate and outside it:
nest-authcounts in Redis and shares the limit across instances, this adapter counts in processmemory and does not. Under horizontal scale the Redis one is the stricter of the two.
The mutation sweep
The gate's configuration was never being read. cargo-mutants loads
.cargo/mutants.tomland nothing else; the file sat at the repository root, where it is ignored in silence. So
examine_globsscoped nothing,bindings/,examples/andfuzz/were being mutated despitethe excludes, and CI had been running the same way. Moved, with the requirement written into its
header and a one-line check (
cargo mutants --listmust print onlycrates/paths). Onefollow-on: with the file finally read,
additional_cargo_argssupplies--all-features, andcargo refuses the flag twice — the copy on CI's command line failed the baseline build, so it is
gone.
With the correct scope the sweep surfaced a class of test rather than a class of bug: tests that
cannot see their own subject.
clear_session -> ()Set-Cookiesatisfies that tooclamp_secs/refresh_max_age_secs-> constantMax-Age; a session cookie would log every user out when they closed the tabplatform_auth -> Nonelet Some(..) else { return }, so the whole tier skipped itselftoken_key,setup_key,challenge_bf_id,disable_bf_id,state_key-> a constantmfa_encryption_key -> [0; 32]assert_user_active,revoke_other_user_sessions->Ok(())refresh_ttl_secsarithmeticThe five key-derivation survivors are the ones worth reading twice, because each collapse is an
attack rather than a cosmetic gap: one shared MFA setup slot hands the second caller the winner's
record — their authenticator enrols against someone else's account, and they learn its TOTP secret
and recovery codes; one shared challenge or management counter lets anyone freeze any account's
MFA by failing their own attempts five times; and one shared OAuth
statekey means any forgedstatefinds the stored PKCE verifier, which is the entire CSRF protection. Each is now pinnedwith two users in play.
Three mutants are genuinely equivalent and are recorded in
.cargo/mutants.tomlwith the reasoneach cannot be killed — XOR over already-masked bits, a
Defaultthat calls the body beingreplaced, and an
"inactive"arm the wildcard answers identically. Two more are recorded asuntestable under this gate rather than equivalent: they live behind
cfg(not(feature = …))and the sweep builds
--all-features, so those lines are never compiled into the suite measuringthem. The patterns stay narrow so the killable mutants of the same functions are still generated;
where two
cfgtwins share a mutant name the entry is anchored to the line, and a shift makes itreappear as a survivor rather than disappear silently.
Also in here
handlers.tsinpackages/rust-authhad no tests at all — the one module that writesSet-Cookieback to a browser. Now at 100% lines, and the package runs under a coverage ratchetin CI (68.7% → 86.06% statements) so it can only go up.
cargo mutants --in-placemutation that agit add -Aswept into adocs commit.
--in-placeedits the working tree; nothing may be staged while it runs.Gates
cargo test --workspace --all-featuresgreen across 30 suites, 100% lines and functionsunder
cargo llvm-cov --fail-under-lines 100cargo fmt --checkandclippy --all-targets -D warningscleanredis:8containerRedis store, ~2 min per mutant here); every survivor it reported is closed — killed by a
test or recorded with its reason. Re-running
--iterateover the survivors is what caughtfour of my own fixes asserting the wrong thing, so each of those is red-checked by hand: the
test fails with the mutation applied and passes without it. CI runs the full sweep post-merge.