Repository navigation
fix(core): align upstream calls with the other adapters - #267
Merged
Merged
Conversation
Request OTP codes and magic links with POST, time out calls to the auth API after 15 seconds and answer 502 upstream_unavailable when it cannot be reached, keep the authServerUrl path prefix when fetching the key set, and verify the registration token before setting its cookie. Also bumps @seamless-auth/types to ^0.35.0 in core and nextjs, which the types-current CI check requires. Closes #261
Bccorb
added a commit
that referenced
this pull request
Nov 3, 2026
…esh replay after rotation (#270) * test: reproduce body-less form POST CSRF under SameSite=Lax A same-site or fetch-metadata-free form POST carrying a Lax access cookie reaches cookie-authenticated routes on Express, Fastify (text/plain or no content type) and Next.js, and checkOrigin ignores cross-site and allowedOrigins unless cookies are SameSite=None. These tests fail on main. Refs #256 * fix: guard unsafe auth requests whatever the SameSite setting The origin check only ran for SameSite=None cookies, and a body-less form POST reached cookie-authenticated routes because Express and Next.js forward urlencoded and multipart as an empty body and Fastify accepts text/plain and a missing content type. checkOrigin now runs whatever the SameSite setting and enforces allowedOrigins (an empty list included) for same-site and metadata-free requests. A new checkContentType refuses a state-changing request to the auth routes with 415 unsupported_media_type unless it declares application/json or carries an x-seamless-auth-transport header, even with no body. POST /refresh, DELETE /logout and DELETE /logout/all are exempt for now because the current browser client sends them bare. Fastify accepts an empty JSON body on the plugin's routes and Express no longer reads one as {}, so a body-less JSON request forwards no body. Refs #256 * test(core): reproduce replay of a completed refresh rotation A second caller presenting the same refresh cookie, session token or bearer refresh token after a completed rotation gets the new session from the 5 second completed-result window, and the auth API is never called. These tests fail on main. Refs #258 * fix(core): share a refresh rotation only while it is in flight dedupeRefresh kept a completed rotation for 5 seconds and handed it to any caller presenting the same refresh cookie, session token or bearer refresh token, without calling the auth API. A stolen refresh token replayed in that window got the new session, and the API's reuse detection never saw it. Drop the completed-result window for every transport. Callers that race one rotation still share it; a caller that arrives after it completed goes to the auth API as a replay. Refs #258 * test: reproduce guards that skip the origin check Express and Fastify requireAuth let a cross-site, unlisted same-site or null-Origin state change through on a cookie session, where the Next.js guard refuses it. checkOrigin also passes a literal null Origin when Sec-Fetch-Site is same-origin, same-site or none. These tests fail on the branch. Refs #256 * fix: run the origin check in the Express and Fastify guards requireAuth in Express and Fastify never called checkOrigin, so a cross-site, unlisted same-site or null-Origin state change on a cookie session reached the adopter's route, where the Next.js guard and the Go, Python and Rust adapters refuse it. Run the origin check on a cookie session, before authentication, with a new allowedOrigins option. Bearer requests are not checked and no content type is required. checkOrigin now refuses a literal null Origin whatever Sec-Fetch-Site and allowedOrigins say, instead of only when Sec-Fetch-Site is absent. Refs #256 * docs: describe the content-type gate and guard origin check The Fastify and Next.js option tables gain the 415 gate, its exemptions and the null-Origin rule, matching the Express README. The Express and Fastify guard sections describe the origin check and allowedOrigins. Refs #256 * test(express): reproduce forwarding a client IP under a trust-everything proxy setting Refs #257 * fix(express): detect any trust-everything proxy setting before forwarding the client IP Probe the compiled trust proxy function with documentation addresses, as the Fastify adapter does, so subnets covering every address or a function that always trusts no longer let a caller choose x-seamless-client-ip. Refs #257 * test(core): keep keys served at the host root from verifying prefixed auth server tokens The JWKS prefix was already kept by #267, so this does not reproduce on the current branch. Against the pre-#267 URL resolution it fails: a token signed with the host root key verifies. Refs #259 * test(core): reproduce client credentials tokens that look like users Refs #260 * fix(core): mark client credentials tokens on the resource server user A verified user now carries isClient, true when sub equals client_id, so adopters can tell a machine acting for itself from a user before reading id as a user id. Refs #260 * test(core): reproduce resource server guards that cannot require a user Adds shared vectors for isClient on verified tokens and for an auth guard that refuses client credentials tokens with 403 user_required. Both are additive: is_client sits beside each verify entry's user, and the new requests live in their own user_required_requests list, so the Go, Python and Rust copies keep passing until those adapters take them up. Refs #260 * feat: let verify-only resource servers require a user requireAuth from createResourceServer (and the Next.js requirements, and core authenticate) take allowClients. With false, a verified client credentials token is refused with 403 user_required and no challenge, the same shape as insufficient_role. Invalid tokens still get 401 first, and the default keeps today's behavior. Refs #260 * ci: run conformance from seamless-cli v0.27.2 and hold Next.js to the content-type gate * test(core): reproduce a second tab losing its session on a late refresh cookie Browser tabs share one cookie jar, so a second tab can send the old refresh cookie a moment after the first tab's rotation completed. That replay should get the session the first tab got for 5 seconds, without calling the auth API. After 5 seconds, or for a failed rotation, it goes to the auth API. A bearer replay always goes to the auth API, even inside the 5 seconds. The two cookie window tests fail on this branch. Refs #258 * fix: keep a completed refresh for cookie transport for five seconds Browser tabs share one refresh cookie, so a second tab can send the old cookie just after the first tab's rotation completed. Sending it to the auth API trips reuse detection and signs both tabs out, which breaks the multi-tab conformance case. Keep a successful cookie rotation for 5 seconds, keyed on a hash of the old refresh cookie, and hand it to a caller presenting that cookie without calling the auth API. Failed rotations are not kept, expired entries are pruned on insert and the map is capped. Bearer and session token refreshes keep sharing only a rotation in flight, so a late bearer replay still reaches the auth API. Refs #258 --------- Co-authored-by: Brandon Corbett <Bccorb@users.noreply.github.com>
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.
Closes #261.
Four places where
@seamless-auth/corecalled the auth API differently from the Go, Python and Rust adapters.Changes
requestOtpHandlerandrequestMagicLinkHandlercallPOST /otp/generate-*andPOST /magic-link, the routes the manifest declares. TheGETroutes are deprecated in the API and markedadapter: false.redirectUristays a query parameter, which is where the POST route reads it.authFetchsetsAbortSignal.timeout(timeoutMs), 15 seconds by default (DEFAULT_UPSTREAM_TIMEOUT_MS). A network failure or timeout resolves to502 {"error":"upstream_unavailable"}, the same way a redirect already resolves to502 upstream_redirect, so every handler and all three bindings return it without binding changes. The logged error is passed throughredactSensitiveText.authServerUrl(trailing slashes trimmed) plus/.well-known/jwks.json, sohttps://example.com/auth-apifetcheshttps://example.com/auth-api/.well-known/jwks.json.registerHandlerrunsverifyUpstreamSessionwithtypephemeralbefore it sets the registration cookie or returns the body to a bearer client, and answers502 {"error":"invalid_upstream_session"}when it fails.RegisterOptionsnow takesaudienceandauthServerIssuer. Express, Fastify and Next.js pass both, and their register routes treat anerrorCoderesult as a failure so external delivery does not run.@seamless-auth/typesis bumped to^0.35.0in core and nextjs, because thecheck:types-currentCI step fails on any branch while it pins^0.34.0.Release
Changeset: core
minor, becauseRegisterOptions.audienceis a new required field for code that callsregisterHandlerdirectly. Express, Fastify and Next.js arepatch. The other three behaviour changes are fixes:502where it used to answer500 internal_error.Tests
502 upstream_unavailable. That includes the redaction test for the new log line. The generic500 internal_errorpaths are still covered, using a response whose body cannot be read.The conformance workflow runs on this PR.