Repository navigation
fix: origin check whatever the SameSite setting, no JWKS lock across the fetch, no refresh replay - #34
Merged
Conversation
A body-less POST with Sec-Fetch-Site same-site or absent reaches a cookie-authenticated route under SameSite=Lax, the origin check never runs outside SameSite=None, and an empty configured allowlist is read as not configured. Refs #24
The cross-site check ran only with SameSite=None cookies, and the JSON content type was required only when a body was present, so a body-less form POST from a sibling subdomain or a browser without fetch metadata reached cookie-authenticated routes. - run the origin check on unsafe methods for every SameSite setting - when allowed_origins is set, enforce it for same-site requests and requests without fetch metadata, and treat an empty list as strict - require Content-Type: application/json or x-seamless-auth-transport on unsafe manifest routes even with no body, which forces a preflight Refs #24
While a refetch triggered by an unknown kid is in flight, verifying a token whose kid is cached and fresh waits for that fetch to finish. Refs #25
Every verification took the cache lock, which a refetch held for up to the 5 second fetch timeout, so a slow JWKS endpoint queued every verification behind it, including tokens whose kid was cached and fresh. The key set is now behind a lock held only to read or update it. A cached kid is served without waiting on a fetch in progress, and the refetch runs single-flight on its own lock, which an unknown kid waits on so one fetch answers all of them. Refs #25
After a rotation succeeds, presenting the same refresh token or cookie again within 5 seconds gets the new session without the auth API being called, so its reuse detection never sees the replay. Refs #26
After a rotation succeeded, any caller presenting the same refresh token or cookie within 5 seconds got the new session without the auth API being called, so a stolen token replayed in that window took over the session and the API's reuse detection never saw it. A rotation is now shared only with callers that arrive while it is in flight. Once it settles nothing is kept, and a later caller reaches the auth API, which answers a replayed token as reuse. Refs #26
POST /refresh, DELETE /logout and DELETE /logout/all pass body-less with no content type or transport header, as the client SDKs send them, but still get the origin check under every SameSite setting, and a refused logout clears no cookies. Refs #24
A Sec-Fetch-Site: same-site request carrying Origin: null passes when no allowlist is configured, while every other unsafe request with a null Origin is refused. Refs #24
A null Origin is opaque or sandboxed, so it is refused on every unsafe request, including a Sec-Fetch-Site: same-site one with no allowlist. Refs #24
…tadata A null Origin, in any case or with surrounding space, should be refused on every unsafe request before Sec-Fetch-Site and the allowlist are consulted. Sec-Fetch-Site same-origin or none lets it through. Refs #24
A null Origin, trimmed and in any case, is now refused before Sec-Fetch-Site and the allowlist are consulted, so same-origin or none fetch metadata no longer lets it through. This folds the earlier same-site special case into the single early check. Refs #24
…d ported proxy hops An access route that silently refreshes and then gets an unverifiable or unstorable session back answers 502 without the rotated access and refresh cookies. A trusted X-Forwarded-For hop written as ip:port or [v6]:port resolves to no client address. Logout clearing a pre-auth cookie with its own name is already covered by logout_clears_a_pre_auth_cookie_with_its_own_name since c6edea8. Refs #27
…ops with ports A manifest route that silently refreshed the session and then got back a session it could not verify or store answered 502 without the rotated access and refresh cookies. The browser kept the spent refresh token and was signed out by reuse detection on its next request. Both invalid_upstream_session replies now carry the rotated cookies. TrustedProxies read an X-Forwarded-For hop written as ip:port or [v6]:port as unreadable and gave no client address, so the auth API limited on the adapter's own address. Hops are now read past the port and brackets; a hop whose port is not a port is still unreadable. Refs #27
…window A tab that presents a refresh cookie just after another tab rotated it must get the same rotated session without a call to the auth API, for five seconds. After that, and for bearer transport always, the replay reaches the API. Expired entries must be pruned. Refs #26
A tab that sends the old refresh cookie just after another tab rotated it now gets the same rotated session without calling the auth API, for five seconds after the rotation settles. The completed result is stored under the same lock that ends the flight, so no caller can slip between the two and replay the token. Only successful, verified rotations are kept, entries are pruned on every access, and bearer transport never uses the window: a bearer replay reaches the API, whose reuse detection revokes the family. Refs #26
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.
Fixes three v0.21 security review findings (epic fells-code/seamless-auth-api#558). Each was reproduced on
mainfirst:74d2262,120a428anda076584add only tests, which fail there (5, 1 and 2 failures).Closes #24
Closes #25
Closes #26
Origin and content type (#24)
require_authcookie sessions..allowed_origins(empty)now means configured and strict.Content-Type: application/jsonor anx-seamless-auth-transportheader (any value; onlybearerselects bearer transport), otherwise 415unsupported_media_type./refresh,/logoutand/logout/allare exempt in v0.23 so current clients keep working, and still get the origin check. A refused logout clears nothing.This is the cross-adapter contract recorded in the v0.23 plan (fells-code/fellscode#32). The Node, Go and Python adapters get the same behavior.
JWKS lock (#25)
The key set sits behind a short lock that is never held across a fetch. Refetches are single flight on a separate lock. A cached kid, fresh or stale, is served without waiting; an unknown kid waits for the fetch in flight and re-checks, so a burst of unknown kids costs one fetch. Discovery in
verifier.rswas checked and is not affected: its lock is only held before any keys are cached.Refresh replay (#26)
REUSE_WINDOWis gone. A rotation is shared only with callers that arrive while it is in flight. A refresh token presented after its rotation finished reaches the auth API, which treats it as reuse.Merge order
Ready:
@seamless-auth/client0.8.3 (fells-code/seamless-auth-js#234) and seamless-cli 0.27.2 (fells-code/seamless-cli#345), which sendx-seamless-auth-transporton unsafe requests, are released. Conformance runs withSEAMLESS_VERIFY_CONTRACTS=content-type-gate, so the 415 case is enforced here.Refresh sharing was revised on 2026-11-03: a completed rotation is kept for 5 seconds for cookie transport only (concurrent tabs keep their session), and bearer replays always reach the auth API.
Checks
cargo fmt --check,cargo clippy --all-targets -- -D warnings(default,--no-default-features, andnative-tls),cargo test(117 + 1 doctest), andcargo +1.88 test(MSRV) pass. The conformance suite was not run locally; CI runs it.Also in this PR
Closes #27
a191b6d,5e725b2tests;02f6f5f,9890782fixes):Origin: null(trimmed, any case) is refused first on every unsafe request.3021fbdtest,8f3933bfix): a manifest route that silently refreshes and then gets a session it cannot verify or store now returns its 502 with the rotated cookies, so the browser does not keep a spent refresh token.TrustedProxiesreadsip:port,[v6]:portand bare[v6]hops. The pre-auth cookie part was already fixed inc6edea8; its existing test fails with that fix removed.