From f29006be24e87aefc53cb0605f9a17b6274340c7 Mon Sep 17 00:00:00 2001 From: Evan Phyillaier Date: Sat, 8 Aug 2026 12:38:14 -0400 Subject: [PATCH] v1.9.1: the login button signed you in and left you logged out POST /auth/signinup answered status "OK", the client reported success, the URL went back to /, and the very next GET /api/me was a 401 - so the login screen came back with nothing wrong on it. SuperTokens picks the session's token transfer method at creation from the `st-auth-mode` request header, and defaults to "header" when it is absent (session/sessionRequestFunctions.js: "We default to header if we can't 'parse' it or if it's undefined"). The session came back in st-access-token / st-refresh-token response headers and no cookie was ever set. supertokens-web-js sends that header for you; v1.9.0 hand-rolled the calls without it. That is the one frontend-SDK responsibility hand-rolling inherited silently. Proved against a real core rather than from source. Same endpoint, both ways, and BOTH return 200 - which is why no layer reported anything: without st-auth-mode -> Set-Cookie: (none) st-access-token, st-refresh-token with st-auth-mode:cookie -> Set-Cookie: sAccessToken, sRefreshToken Also makes the failure impossible to ship again in silence: a sign-in the server calls OK that is followed by no session now says so on the login screen instead of returning a blank form. The absence of that message is the only reason v1.9.0 reached a production deploy. Guarded at both layers, and both confirmed by mutation - renaming the header fails the unit test and the smoke round trip. 698 vitest SQLite / 724 Postgres / 50 smoke. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 32 +++++++++++++++++++++++ Dockerfile | 2 +- client/src/App.jsx | 18 ++++++++++++- client/src/game/api.js | 8 ++++++ client/src/game/auth.js | 22 +++++++++++++++- docs/authentication-methods.md | 10 ++++++++ package.json | 2 +- tests/clientAuth.test.js | 47 ++++++++++++++++++++++++++++++++++ tests/e2e/smoke-v19.mjs | 39 ++++++++++++++++++++++++++++ 9 files changed, 176 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index da54650..a9600fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,37 @@ # Changelog +## v1.9.1 + +- **The SuperTokens login button signed you in and then left you logged out.** + `POST /auth/signinup` answered `status: "OK"`, the client reported success, + the URL went back to `/` — and the very next `GET /api/me` was a 401, so the + login screen came back with nothing wrong on it. + + SuperTokens picks the session's *token transfer method* at creation time from + the `st-auth-mode` request header, and when it is absent it defaults to + **header**, not cookies (`session/sessionRequestFunctions.js`: *"We default + to header if we can't 'parse' it or if it's undefined"*). The session came + back in `st-access-token` / `st-refresh-token` response headers; no cookie + was ever set. `supertokens-web-js` sends that header for you, and v1.9.0 + hand-rolled the calls without it — the one responsibility of the frontend SDK + that hand-rolling quietly inherited. + + Confirmed against a real core, same endpoint, both ways — note that **both + return 200**, which is why nothing anywhere reported an error: + + | request | `Set-Cookie` | response headers | + |---|---|---| + | without `st-auth-mode` | *(none)* | `st-access-token`, `st-refresh-token` | + | with `st-auth-mode: cookie` | `sAccessToken`, `sRefreshToken` | *(none)* | + + Both `/auth/signinup` and `/auth/session/refresh` now send it. + +- **A login that sets no session now says so.** If sign-in succeeds but the + session that follows does not exist, the login screen says the server did not + set a session and suggests checking cookies, instead of silently returning to + a blank login form. The absence of that message is the only reason v1.9.0's + bug reached a production deploy — every layer reported success. + ## v1.9.0 - **Every SuperTokens login was impossible, and had been since v1.8.** diff --git a/Dockerfile b/Dockerfile index c0ef19f..427f2a4 100644 --- a/Dockerfile +++ b/Dockerfile @@ -44,7 +44,7 @@ LABEL org.opencontainers.image.licenses="MIT" # only on a pushed vX.Y.Z tag, and docker/metadata-action derives the # published image's version label from that tag - so this literal only # affects locally-built images, not what GHCR publishes. -LABEL org.opencontainers.image.version="1.9.0" +LABEL org.opencontainers.image.version="1.9.1" VOLUME ["/app/data"] EXPOSE 3000 diff --git a/client/src/App.jsx b/client/src/App.jsx index 570e7bb..04472cf 100644 --- a/client/src/App.jsx +++ b/client/src/App.jsx @@ -27,10 +27,12 @@ export default function App() { // code has to happen BEFORE /api/me, because it is what creates the // session /api/me would otherwise report as absent. let callbackFailure = null; + let completedLoginFor = null; if (callbackProviderFromPath(window.location.pathname)) { const result = await completeSuperTokensLogin(); if (cancelled) return; - if (!result.ok) callbackFailure = result; + if (result.ok) completedLoginFor = result.provider; + else callbackFailure = result; // Replace rather than push, and always: leaving a spent ?code= in the // URL means a reload re-POSTs an authorisation code the provider has @@ -60,6 +62,20 @@ export default function App() { } if (cancelled) return; + + // A sign-in that the server called OK, followed by a session that does + // not exist. There is nothing wrong on the login screen to look at, so + // without this the player just bounces back to it and the only evidence + // is in devtools. That is how v1.9.0's missing `st-auth-mode: cookie` + // header survived a release: signinup answered OK, no cookie was set, + // and the app looked like it had simply forgotten the click. + if (completedLoginFor && !authed) { + window.history.replaceState( + {}, '', + `/?authError=${encodeURIComponent(completedLoginFor)}&authReason=no_session`, + ); + } + if (authed) { setUser(authed); setStatus('authed'); } else setStatus('anon'); })(); diff --git a/client/src/game/api.js b/client/src/game/api.js index d5f1125..077dfc5 100644 --- a/client/src/game/api.js +++ b/client/src/game/api.js @@ -77,6 +77,14 @@ function refreshSession() { const res = await fetch('/auth/session/refresh', { method: 'POST', credentials: 'include', + // Same reason as the signinup call in game/auth.js: this is what tells + // SuperTokens to put the rotated tokens back in cookies. Refresh + // tolerates its absence better than session creation does (it infers + // the method from the tokens it was given), but a refresh that + // silently switched the session to header transport would log the + // player out on the next request, which is the same invisible failure + // one step later. + headers: { 'st-auth-mode': 'cookie' }, }); return res.ok; } catch { diff --git a/client/src/game/auth.js b/client/src/game/auth.js index 57cac00..6acc6d9 100644 --- a/client/src/game/auth.js +++ b/client/src/game/auth.js @@ -181,7 +181,25 @@ export async function completeSuperTokensLogin({ // wholesale). const res = await requestJSON('/auth/signinup', { method: 'POST', - headers: { 'Content-Type': 'application/json' }, + headers: { + 'Content-Type': 'application/json', + // NOT optional, and its absence fails silently. SuperTokens resolves the + // token transfer method at session creation from this header, and when + // it is missing it defaults to "header" - see + // session/sessionRequestFunctions.js: "We default to header if we can't + // 'parse' it or if it's undefined". The session then comes back in + // st-access-token / st-refresh-token RESPONSE headers and no cookie is + // ever set, so signinup answers status "OK", this function reports + // success, and the very next GET /api/me is a 401. The player lands back + // on the login screen with nothing wrong on it. v1.9.0 shipped exactly + // that. + // + // supertokens-web-js sends this header for you; hand-rolling the calls + // means inheriting the responsibility. Cookies are the right choice here + // because the whole app already relies on them - `credentials: + // 'include'` everywhere, and the legacy JWT cookie works the same way. + 'st-auth-mode': 'cookie', + }, body: JSON.stringify({ thirdPartyId: provider, redirectURIInfo: { @@ -227,6 +245,8 @@ export function loginErrorMessage(provider, reason) { return `This account is not allowed to sign in with ${name}.`; case 'no_email': return `${name} did not share an email address, which this server requires.`; + case 'no_session': + return `${name} signed you in, but the server did not set a session. Check that cookies are allowed for this site.`; default: return `Login with ${name} failed. Try again.`; } diff --git a/docs/authentication-methods.md b/docs/authentication-methods.md index 249be07..ae91a22 100644 --- a/docs/authentication-methods.md +++ b/docs/authentication-methods.md @@ -233,6 +233,16 @@ serialised so a burst of concurrent 401s produces exactly one refresh call. `GET /api/auth-info` tells the client which stack to drive, so one build serves `passport` and `supertokens` alike and the rollback stays real. +> **If you touch those calls, keep the `st-auth-mode: cookie` header.** +> SuperTokens chooses the session's token transfer method at creation from that +> header, and **defaults to `header` when it is absent** — the session comes +> back in `st-access-token` response headers and no cookie is set. Every status +> code stays 200: `signinup` reports OK, the client believes it, and the next +> authenticated request is a 401. v1.9.0 shipped that and it survived a +> production deploy, because nothing in the chain reports an error. It is the +> one job `supertokens-web-js` does for you that hand-rolling silently +> inherits, and it applies to any future recipe you add, not just ThirdParty. + **What v1.9 also fixed, and why nothing here was ever exercised before.** `server/supertokens/init.js` passed its OAuth providers to `ThirdParty.init` as `signInUpFeature`. The SDK reads `signInAndUpFeature`. The key is optional diff --git a/package.json b/package.json index b956dc6..1a91dee 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "rackstack-server", - "version": "1.9.0", + "version": "1.9.1", "private": true, "type": "module", "scripts": { diff --git a/tests/clientAuth.test.js b/tests/clientAuth.test.js index 400b7ef..2633d1f 100644 --- a/tests/clientAuth.test.js +++ b/tests/clientAuth.test.js @@ -151,6 +151,29 @@ describe('completeSuperTokensLogin', () => { expect(body.oAuthTokens).toBeUndefined(); }); + it('asks for the session in COOKIES, which is what actually logs the player in', async () => { + // The v1.9.0 regression, and the reason it shipped. SuperTokens picks the + // token transfer method at session creation from this header, and with it + // absent defaults to "header" - the session comes back in st-access-token + // response headers, no cookie is set, signinup still answers status OK, + // and the next /api/me is a 401. Nothing in the flow reports an error. + // + // supertokens-web-js sends this for you. Hand-rolling means owning it. + const calls = []; + globalThis.fetch = vi.fn(async (url, opts) => { + calls.push({ url, opts }); + return jsonResponse({ status: 'OK', user: { id: 'github:37058311' } }); + }); + + await completeSuperTokensLogin({ + pathname: '/auth/callback/github', + search: '?code=abc123', + origin: ORIGIN, + }); + + expect(calls[0].opts.headers['st-auth-mode']).toBe('cookie'); + }); + it('does not POST when the player cancelled at the provider', async () => { globalThis.fetch = vi.fn(async () => jsonResponse({ status: 'OK' })); @@ -241,6 +264,30 @@ describe('refresh on 401', () => { expect(seen).toEqual(['/api/state', '/auth/session/refresh', '/api/state']); }); + it('asks for the refreshed session in cookies too', async () => { + configureAuthRefresh({ loginFlow: 'supertokens' }); + + const calls = []; + let refreshed = false; + globalThis.fetch = vi.fn(async (url, opts) => { + calls.push({ url, opts }); + if (url === '/auth/session/refresh') { + refreshed = true; + return { ok: true, status: 200, text: async () => '' }; + } + if (!refreshed) return jsonResponse({ error: 'unauthorized' }, { ok: false, status: 401 }); + return jsonResponse({ ok: true }); + }); + + await fetchState(); + + // A refresh that silently moved the session to header transport would log + // the player out on the next request - the same invisible failure as the + // signinup one, just deferred. + const refresh = calls.find((c) => c.url === '/auth/session/refresh'); + expect(refresh.opts.headers['st-auth-mode']).toBe('cookie'); + }); + it('gives up after a second 401 rather than looping', async () => { configureAuthRefresh({ loginFlow: 'supertokens' }); diff --git a/tests/e2e/smoke-v19.mjs b/tests/e2e/smoke-v19.mjs index 79dbf12..733f0ca 100644 --- a/tests/e2e/smoke-v19.mjs +++ b/tests/e2e/smoke-v19.mjs @@ -275,6 +275,7 @@ if (!playwright) { try { let authUrlCalls = 0; let signinupBody = null; + let signinupHeaders = null; // The provider, stubbed: send the browser straight back to our own // callback with a code, so nothing external is contacted. @@ -302,6 +303,7 @@ if (!playwright) { // the app genuinely transitions to its authenticated view. await page.route('**/auth/signinup', async (route) => { signinupBody = JSON.parse(route.request().postData() || '{}'); + signinupHeaders = route.request().headers(); await context.addCookies([{ name: COOKIE_NAME, value: cookieFor(user), url: BASE_URL, httpOnly: true, }]); @@ -341,6 +343,14 @@ if (!playwright) { // Never oAuthTokens - the server refuses those (rejectRawOAuthTokens). assert(!signinupBody.oAuthTokens, 'the client must not submit raw oAuthTokens'); + // Without this header SuperTokens returns the session in response + // headers instead of cookies, signinup still says OK, and the next + // /api/me is a 401. That is what v1.9.0 shipped. + assert( + signinupHeaders['st-auth-mode'] === 'cookie', + `signinup must ask for cookie transport, got ${signinupHeaders['st-auth-mode']}`, + ); + // The spent code must be replaced out of the URL: a reload that re-POSTs // a burned authorisation code fails and bounces the player to login. const url = new URL(page.url()); @@ -367,6 +377,35 @@ if (!playwright) { } finally { await context.close(); } }); + await check('a signin the server calls OK but that sets no session says so', async () => { + // The v1.9.0 failure mode, reproduced: signinup answers status OK and no + // session cookie is set. The player must be told, not silently returned to + // a login screen with nothing wrong on it - that is what made the missing + // st-auth-mode header survive a release and a production deploy. + const { context, page } = await newPage(); + try { + await page.route('**/auth/authorisationurl*', (route) => route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ + status: 'OK', + urlWithQueryParams: `${BASE_URL}/auth/callback/github?code=fake-code`, + }), + })); + + // OK, but deliberately no cookie - exactly what header transport does. + await page.route('**/auth/signinup', (route) => route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ status: 'OK', user: { id: 'github:1' } }), + })); + + await page.goto(`${BASE_URL}/`); + await page.locator('button', { hasText: 'Continue with GitHub' }).click(); + await page.locator('text=/did not set a session/i').waitFor({ timeout: 15000 }); + } finally { await context.close(); } + }); + await check('an unconfigured provider shows a message, not a blank screen', async () => { const { context, page } = await newPage(); try {