Merge pull request #14 from NeverEndingCode/v1.9.1-cookie-transport

v1.9.1: fix the SuperTokens login that signed you in and left you logged out

Author: NeverEndingCode

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.**

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

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');
     })();
 

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 {

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.`;
   }

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

package.json

@@ -1,6 +1,6 @@
 {
   "name": "rackstack-server",
-  "version": "1.9.0",
+  "version": "1.9.1",
   "private": true,
   "type": "module",
   "scripts": {

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' });
 

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 {