Repository navigation
fix: no refresh replay after rotation, and origin check whatever the SameSite setting - #30
Merged
Merged
Conversation
A completed rotation was handed to any caller presenting the same old refresh token or cookie for the next 5 seconds, without calling the auth API. A stolen refresh token replayed inside that window got the new session, and the API's reuse detection never saw the replay. Parallel requests that arrive while a rotation runs still share it; a refresh token presented after it finished now goes to the auth API, for cookie and bearer transport alike. Refs #21
The cross-site check on state-changing requests only ran while cookies were
SameSite=None, and the JSON content type check only ran when a body was
present. A body-less form POST from a sibling subdomain, which is same-site
and so carries SameSite=Lax cookies, or from a browser that sends no fetch
metadata, reached cookie-authenticated routes such as /recovery-codes,
/totp/enroll/start and /admin/applications/{id}/secret.
- The origin check runs on every unsafe method, whatever CookieSameSite is.
With AllowedOrigins set it is enforced for same-site requests and for
requests without fetch metadata that carry an Origin.
- An unsafe request must declare a JSON Content-Type or carry the
X-Seamless-Auth-Transport header even when its body is empty, so a
cross-origin browser request needs a CORS preflight. Otherwise it answers
415 unsupported_media_type.
- A non-nil empty AllowedOrigins is configured and allows no origin.
- NewVerifier refuses a plain http JWKSURI when the Issuer is https.
Refs #23
The current client sends POST /refresh and DELETE /logout (and /logout/all) with no body and neither a JSON Content-Type nor the transport header, so refusing them would sign users out when the adapter upgrades ahead of the client. They are exempt from the content type gate for now and keep the origin check. Every other unsafe auth route still answers 415 without one. Refs #23
Adds the admin application link routes (GET /admin/applications/{id}/links,
PUT and DELETE /admin/applications/{id}/links/{linkedId}).
An Origin of null (trimmed, any case) is an opaque or sandboxed context and is cross-site whatever Sec-Fetch-Site says. It passed with same-site (no allowlist), same-origin or none, and with surrounding space or another case when the header was absent. It is now refused first on every unsafe request. Refs #23
The caller that leads a shared rotation cancels it for every waiter when its request goes away, and a transport error or 5xx during a refresh clears the session cookies as if the auth API had refused the token. Refs #22
A refresh rotation ran on the leading request's context, so a closed tab cancelled it for every caller sharing it, though the auth API may already have spent the old token. It now runs detached from the request, under the upstream timeout. The JWKS fetch is detached too: a first fetch failing on a cancelled caller held off verification for every waiter. Only the auth API answering 401 (or an unreadable refresh cookie) now clears the session cookies. A transport error answers 502 upstream_unavailable and another API answer is passed through, both with the cookies kept, on a silent refresh and on POST /refresh. Refs #22
… out Several tabs can hold one refresh cookie. A tab presenting it a moment after another tab's rotation finished reaches the auth API, which treats it as reuse, so the tab is signed out. A bearer replay must still reach the auth API, and a failed rotation must never be shared. Refs #21
Tabs holding one refresh cookie shared a rotation only while it was in flight, so a tab presenting the cookie a moment after another tab's rotation finished reached the auth API as reuse and was signed out. A successful cookie-transport rotation is now kept for 5 seconds, keyed as the in-flight map keys it, and a cookie presenting the same old refresh token within that window gets the same session without calling the auth API. Bearer transport never uses the window: a bearer refresh token presented after its rotation finished still goes to the auth API. Failed rotations are never kept, and expired entries are pruned in expiry order on every refresh. Refs #21
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 two v0.21 security review findings (epic fells-code/seamless-auth-api#558). Each was reproduced on
mainfirst:85d015cadds only the tests, and all 8 fail there.Closes #21
Closes #23
Refresh replay (#21)
refreshReuseWindowand the completed-result window are gone. A rotation is shared only with callers that arrive while it is in flight, for cookie and bearer transport alike. A refresh token presented after its rotation finished now reaches the auth API, whose reuse detection revokes the session family.Origin and content type (#23)
CookieSameSiteis. Under Lax,Sec-Fetch-Site: cross-site,Origin: null, and asame-siterequest from an origin not onAllowedOriginsget 403cross_site_request_blocked. This covers the auth handler andRequireAuthon cookie sessions.AllowedOriginsis configured and strict. Leave it nil for no allowlist.Content-Type: application/jsonorX-Seamless-Auth-Transport(any value), otherwise 415unsupported_media_type.POST /refresh,DELETE /logoutandDELETE /logout/allare exempt in v0.23 so current clients keep working, and still get the origin check. A refused logout clears nothing.NewVerifierrefuses an httpJWKSURIunder an httpsIssuer.This is the cross-adapter contract recorded in the v0.23 plan (fells-code/fellscode#32). The Node, Python and Rust adapters get the same behavior.
Also:
6bf02d7syncs the bundled manifest with seamless-auth-api 0.22.0 (admin application link routes), whichscripts/check-manifest.shwas failing on.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
gofmt -l .empty,go vet ./...clean,go test -race -count=1 ./...ok,scripts/check-manifest.shandscripts/api-check.shpass (suggests v0.5.1). The conformance suite was not run locally; CI runs it.Also in this PR
Closes #22
28a1beftest,a4d4708fix):Origin: null(trimmed, any case) is refused first on every unsafe request, beforeSec-Fetch-Siteand the allowlist.89b933atest,e8ba2f0fix): the rotation runs undercontext.WithoutCancelwith the upstream timeout, so a caller that goes away no longer cancels a rotation other callers wait on. The JWKS fetch runs detached too, because a cancelled leader's failed fetch set the 5 second backoff for everyone. Cookies are cleared only on a 401 from the API or an unreadable refresh cookie. A transport error is now 502upstream_unavailablewith cookies kept, and other non-200 refresh answers pass through with cookies kept.