From 854a2d5b1975ecc9cb6acef6d0e4f55de4ae0379 Mon Sep 17 00:00:00 2001 From: Evan Phyillaier Date: Sat, 8 Aug 2026 10:08:24 -0400 Subject: [PATCH 1/3] Fix the typo that made every SuperTokens login impossible init.js handed the provider list to ThirdParty.init as `signInUpFeature`. The SDK reads `signInAndUpFeature`. The key is optional in its TypeInput and JavaScript has no excess-property check at runtime, so the wrong spelling was dropped in silence - no throw, no warning, no log. The result: a stack that initialises cleanly, passes every existing test, logs `providers=github,discord` (it logs what buildProviders returned, not what the recipe received), and then answers every /auth/authorisationurl with 400 "the provider github could not be found in the configuration". That is what production returns today on v1.8.4, for both providers. v1.8 tested both halves and never the seam: buildProviders() returns the right list, init refuses to start when the list is empty, and nothing asserted the list reaches the recipe. buildProviders' own empty-list guard cannot catch this - the list is non-empty, it just never arrives. `dual` hid it entirely, because the client still logs in through passport and nothing reached the endpoint. The new test runs the config init.js actually passes through the SDK's own normaliser, so it fails on a wrong key and on a future SDK rename alike, rather than merely restating the fix. Co-Authored-By: Claude Opus 5 --- .../2026-08-08-v1.9-supertokens-client.md | 52 ++++++++ server/supertokens/init.js | 13 +- tests/supertokens.providers.wiring.test.js | 122 ++++++++++++++++++ 3 files changed, 186 insertions(+), 1 deletion(-) create mode 100644 tests/supertokens.providers.wiring.test.js diff --git a/docs/superpowers/plans/2026-08-08-v1.9-supertokens-client.md b/docs/superpowers/plans/2026-08-08-v1.9-supertokens-client.md index 451e220..49336a5 100644 --- a/docs/superpowers/plans/2026-08-08-v1.9-supertokens-client.md +++ b/docs/superpowers/plans/2026-08-08-v1.9-supertokens-client.md @@ -221,3 +221,55 @@ refresh call; passport mode never calls refresh. ## Findings and deviations _(record as work proceeds, so the plan does not quietly diverge)_ + +### Task 1 found a server bug. The plan's "no server work" premise is void. + +Running Step 1's first curl against production returned, for **both** providers: + +``` +HTTP 400 {"message":"the provider github could not be found in the configuration"} +``` + +Root cause: `server/supertokens/init.js:358` passed the provider list to +`ThirdParty.init` as **`signInUpFeature`**. The SDK reads +**`signInAndUpFeature`**. Confirmed present in `origin/main` and in tag +`v1.8.4`, the build running in production. + +Proven from the shipped SDK rather than inferred: +`lib/build/recipe/thirdparty/utils.js`'s `validateAndNormaliseSignInAndUpConfig` +reads only `config.signInAndUpFeature` and returns `{ providers: [] }` +otherwise; `lib/build/recipe/thirdparty/api/authorisationUrl.js:51` throws the +exact string production returned. Demonstrated directly: + +``` +signInUpFeature -> { "providers": [] } +signInAndUpFeature -> { "providers": [ { "config": { "thirdPartyId": "github" } } ] } +``` + +Why nothing caught it — worth carrying forward, it is the same shape as the +`superTokensUserId` capital-T bug: + +- The correct key is **optional** in the SDK's `TypeInput`, and JS has no + excess-property check at runtime. The wrong key is dropped with no throw, no + warning, no log. +- v1.8 tested **both halves and not the seam**: `buildProviders()` returns the + right list (tested), init refuses to start when the list is empty (tested), + and nothing asserted the list reaches the recipe. +- `buildProviders`' own "no OAuth provider is configured" guard is structurally + incapable of catching it — the list is non-empty, it just never arrives. +- The boot log even prints `providers=github,discord`, because it logs what + `buildProviders` returned, not what the recipe received. +- `dual` hid it completely: the client logs in via passport, so no traffic ever + reached `/auth/authorisationurl`. + +**Fixed** in this branch, plus `tests/supertokens.providers.wiring.test.js`, +which runs the config init.js actually passes through the SDK's own normaliser +— so it fails on a wrong key *and* on a future SDK rename, rather than merely +restating the fix. + +**Consequence for Task 1's remaining steps:** Steps 2 and 3 assert against a +real login, which cannot happen until the fixed build is deployed. They are +**not done** and are the first thing to run after this branch ships. There is +no container runtime on this machine, so a local SuperTokens core was not an +option either. Tasks 2–3 were built anyway: the client work is required +regardless of this bug, and it is what makes the end-to-end proof possible. diff --git a/server/supertokens/init.js b/server/supertokens/init.js index 0114580..cc9cd89 100644 --- a/server/supertokens/init.js +++ b/server/supertokens/init.js @@ -355,7 +355,18 @@ export async function initSuperTokens({ env = process.env, mode } = {}) { }, recipeList: [ ThirdParty.init({ - signInUpFeature: { providers }, + // `signInAndUpFeature`, NOT `signInUpFeature`. The key is optional in + // the SDK's TypeInput and there is no excess-property check at + // runtime, so the wrong spelling is dropped in silence: + // validateAndNormaliseSignInAndUpConfig reads only this key and falls + // back to `providers: []`. The result is a SuperTokens stack that + // initialises cleanly, passes every containment and config test, and + // then answers every /auth/authorisationurl with + // "the provider could not be found in the configuration" - which + // is what production did until v1.9. Note that buildProviders' own + // "no OAuth provider is configured" guard above cannot catch it: the + // list is non-empty, it just never reaches the recipe. + signInAndUpFeature: { providers }, override: { // The IDENTITY MAPPING override is on `functions` (the recipe // function), NOT `apis`. SuperTokens creates the session in the API diff --git a/tests/supertokens.providers.wiring.test.js b/tests/supertokens.providers.wiring.test.js new file mode 100644 index 0000000..6d84045 --- /dev/null +++ b/tests/supertokens.providers.wiring.test.js @@ -0,0 +1,122 @@ +// Does the provider list buildProviders() returns actually REACH the recipe? +// +// This file exists because v1.8 tested both halves of that question and never +// the seam between them: supertokens.init.test.js asserts buildProviders() +// returns the right list, and separately asserts init refuses to start when +// the list is empty - so both were green while init.js handed the list to +// ThirdParty.init under `signInUpFeature`, a key the SDK does not read. +// +// The failure mode is total and silent. `signInAndUpFeature` is optional in +// the SDK's TypeInput and JavaScript has no excess-property check at runtime, +// so the misspelled key is dropped without a throw, a warning or a log line. +// The stack initialises, every existing test stays green, and the only symptom +// is at the HTTP edge: every /auth/authorisationurl answers +// 400 {"message":"the provider github could not be found in the configuration"} +// which is exactly what production returned throughout the v1.8 `dual` rollout. +// It went unnoticed because the client still logged in via passport, so +// nothing ever drove the SuperTokens flow. +// +// The assertion below deliberately runs the captured config through the SDK's +// OWN normaliser rather than checking the key name by hand. Checking the name +// would only restate the fix; running the normaliser tests the property that +// actually matters - that the providers survive into the recipe - and so it +// also fails if a future SDK version renames or restructures the key. + +process.env.JWT_SECRET = 'test-secret-st-provider-wiring'; + +import { describe, it, expect, beforeEach, vi } from 'vitest'; + +// Captures the config object init.js passes to ThirdParty.init, then calls +// through to the real implementation so supertokens.init() still receives a +// genuine recipe. init.js imports this module dynamically; vi.mock intercepts +// dynamic imports too. +let capturedConfig; + +vi.mock('supertokens-node/recipe/thirdparty', async (importOriginal) => { + const actual = await importOriginal(); + const real = actual.default ?? actual; + return { + ...actual, + default: { + init: (config) => { + capturedConfig = config; + return real.init(config); + }, + }, + }; +}); + +const { initSuperTokens, __resetForTests } = await import('../server/supertokens/init.js'); + +// The SDK's own normaliser - the single source of truth for which key it +// reads. Deep-imported from the build output, the same way the capital-T +// mapping test in supertokens.init.test.js pins createUserIdMapping's +// signature against the shipped declaration file. +const { validateAndNormaliseUserInput } = await import( + 'supertokens-node/lib/build/recipe/thirdparty/utils.js' +); + +const CREDS = { + GITHUB_CLIENT_ID: 'gh-id', + GITHUB_CLIENT_SECRET: 'gh-secret', + DISCORD_CLIENT_ID: 'dc-id', + DISCORD_CLIENT_SECRET: 'dc-secret', + PUBLIC_ORIGIN: 'https://rackstack.example.com', + SUPERTOKENS_CONNECTION_URI: 'http://supertokens:3567', + SUPERTOKENS_API_KEY: 'test-core-api-key', +}; + +beforeEach(() => { + __resetForTests(); + capturedConfig = undefined; +}); + +describe('the configured providers reach the ThirdParty recipe', () => { + it('survives the SDK\'s own normalisation, rather than being silently dropped', async () => { + await initSuperTokens({ env: { ...CREDS, AUTH_MODE: 'dual' }, mode: 'dual' }); + + expect(capturedConfig, 'ThirdParty.init was never called').toBeDefined(); + + const normalised = validateAndNormaliseUserInput(undefined, capturedConfig); + const ids = normalised.signInAndUpFeature.providers.map((p) => p.config.thirdPartyId); + + // Both configured providers must be present. An empty array here is the + // production bug: init succeeded, no provider is reachable. + expect(ids).toEqual(['github', 'discord']); + }); + + it('registers providers under the ids stored in the identities table', async () => { + // The thirdPartyId is what the signInUp override looks identities up by, + // so a provider that arrives under the wrong id is as bad as one that does + // not arrive at all - every existing player would be treated as new. + await initSuperTokens({ env: { ...CREDS, AUTH_MODE: 'supertokens' }, mode: 'supertokens' }); + + const normalised = validateAndNormaliseUserInput(undefined, capturedConfig); + const ids = normalised.signInAndUpFeature.providers.map((p) => p.config.thirdPartyId); + + expect(ids).toContain('github'); + expect(ids).toContain('discord'); + }); + + it('omits a provider whose credentials are absent, all the way through', async () => { + const { DISCORD_CLIENT_ID: _id, DISCORD_CLIENT_SECRET: _secret, ...github } = CREDS; + await initSuperTokens({ env: { ...github, AUTH_MODE: 'dual' }, mode: 'dual' }); + + const normalised = validateAndNormaliseUserInput(undefined, capturedConfig); + const ids = normalised.signInAndUpFeature.providers.map((p) => p.config.thirdPartyId); + + expect(ids).toEqual(['github']); + }); + + it('documents the trap: the misspelled key normalises to no providers at all', () => { + // Not a test of our code - a test of the claim this whole file rests on, + // so that the comment above cannot rot into a plausible-sounding lie. + const providers = [{ config: { thirdPartyId: 'github', clients: [] } }]; + + const wrong = validateAndNormaliseUserInput(undefined, { signInUpFeature: { providers } }); + expect(wrong.signInAndUpFeature.providers).toEqual([]); + + const right = validateAndNormaliseUserInput(undefined, { signInAndUpFeature: { providers } }); + expect(right.signInAndUpFeature.providers).toHaveLength(1); + }); +}); From 9bd19e280418256b9b9b7fa3e18923471a32dee5 Mon Sep 17 00:00:00 2001 From: Evan Phyillaier Date: Sat, 8 Aug 2026 10:26:56 -0400 Subject: [PATCH 2/3] Teach the client to log in through SuperTokens, and to refresh The client half of Phase 5. Three hand-rolled fetch calls rather than supertokens-web-js, whose signOut() targets the /auth/signout v1.8 removed on purpose - it clears only the SuperTokens half of a dual-stack session. GET /api/auth-info (public) is how the client learns which stack to drive. It cannot live on /api/config as the plan proposed: that route is behind requireAuth and the caller is by definition not logged in yet. `loginFlow` is the server's decision rather than something the client re-derives, so the policy lives in one place - in `dual` it says `supertokens`, because exercising that path is the entire point of dual. Both stacks work from one bundle. Hardcoding SuperTokens would break `passport`, which is still the default and the documented rollback. Session refresh serialises through one shared promise. A refresh per 401 would send N concurrent calls presenting the SAME refresh token; SuperTokens rotates it on use, so calls 2..N look like token theft to the core and it revokes the session - a routine renewal becoming a forced logout, and only under concurrency. Same shape as server/userLock.js. Two of the new tests were confirmed by mutation rather than by inspection: deleting the single-flight guard makes the concurrency test report 3 refreshes instead of 1, and disabling the client's callback leg fails two smoke checks. The first version of the concurrency test passed with the guard deleted (its stub keyed "already expired" off the URL, so only one of the three requests ever got a 401) - the comment in the test records that so it cannot regress. 627 -> 696 vitest (SQLite), 39 -> 49 smoke. Co-Authored-By: Claude Opus 5 --- client/src/App.jsx | 66 +++- client/src/Login.jsx | 90 ++++- client/src/game/api.js | 89 ++++- client/src/game/auth.js | 233 +++++++++++ server/auth.js | 23 ++ server/routes/authRoutes.js | 28 +- tests/authInfo.test.js | 158 ++++++++ tests/clientAuth.test.js | 332 ++++++++++++++++ tests/e2e/smoke-v19.mjs | 427 +++++++++++++++++++++ tests/supertokens.providers.wiring.test.js | 5 +- 10 files changed, 1420 insertions(+), 31 deletions(-) create mode 100644 client/src/game/auth.js create mode 100644 tests/authInfo.test.js create mode 100644 tests/clientAuth.test.js create mode 100644 tests/e2e/smoke-v19.mjs diff --git a/client/src/App.jsx b/client/src/App.jsx index ecc041e..570e7bb 100644 --- a/client/src/App.jsx +++ b/client/src/App.jsx @@ -1,19 +1,69 @@ import React, { useEffect, useState } from 'react'; import Login from './Login.jsx'; import RackStack from './RackStack.jsx'; +import { + callbackProviderFromPath, completeSuperTokensLogin, fetchAuthInfo, FALLBACK_AUTH_INFO, +} from './game/auth.js'; +import { configureAuthRefresh } from './game/api.js'; export default function App() { const [status, setStatus] = useState('checking'); // checking | anon | authed const [user, setUser] = useState(null); + const [authInfo, setAuthInfo] = useState(FALLBACK_AUTH_INFO); useEffect(() => { - fetch('/api/me', { credentials: 'include' }) - .then((r) => { - if (!r.ok) throw new Error('not authenticated'); - return r.json(); - }) - .then((u) => { setUser(u); setStatus('authed'); }) - .catch(() => setStatus('anon')); + let cancelled = false; + + (async () => { + // Started first and awaited later: it is independent of the callback + // exchange below, and one round trip here serves both the login screen + // (which buttons to draw) and api.js (whether to refresh on a 401). + const infoPromise = fetchAuthInfo(); + + // The SuperTokens redirect leg (v1.9). The provider sends the player to + // /auth/callback/?code=..., which the server does not handle - + // SuperTokens' ThirdParty recipe serves only POST /auth/callback/apple, + // so the request falls through to the SPA and lands here. Exchanging the + // code has to happen BEFORE /api/me, because it is what creates the + // session /api/me would otherwise report as absent. + let callbackFailure = null; + if (callbackProviderFromPath(window.location.pathname)) { + const result = await completeSuperTokensLogin(); + if (cancelled) return; + if (!result.ok) 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 + // already burned, which fails and bounces a logged-in player back to + // the login screen. Replacing also keeps the code out of the back + // button and out of any link the player might copy. + const next = result.ok + ? '/' + : `/?authError=${encodeURIComponent(result.provider ?? '')}&authReason=${encodeURIComponent(result.reason)}`; + window.history.replaceState({}, '', next); + } + + const info = await infoPromise; + if (cancelled) return; + const resolved = info && !info.error ? info : FALLBACK_AUTH_INFO; + configureAuthRefresh(resolved); + setAuthInfo(resolved); + + // Deliberately skipped when the exchange just failed: there is no + // session to find, and asking anyway only delays the login screen. + let authed = null; + if (!callbackFailure) { + try { + const res = await fetch('/api/me', { credentials: 'include' }); + if (res.ok) authed = await res.json(); + } catch { /* offline or server down - treated as anonymous below */ } + } + + if (cancelled) return; + if (authed) { setUser(authed); setStatus('authed'); } else setStatus('anon'); + })(); + + return () => { cancelled = true; }; }, []); if (status === 'checking') { @@ -27,7 +77,7 @@ export default function App() { ); } - if (status === 'anon') return ; + if (status === 'anon') return ; return ; } diff --git a/client/src/Login.jsx b/client/src/Login.jsx index 5bc69a2..4673eff 100644 --- a/client/src/Login.jsx +++ b/client/src/Login.jsx @@ -1,9 +1,44 @@ -import React from 'react'; +import React, { useState } from 'react'; import { Cpu } from 'lucide-react'; +import { startSuperTokensLogin, loginErrorMessage, FALLBACK_AUTH_INFO } from './game/auth.js'; -export default function Login() { +// Display order, independent of the order the server lists providers in. +// Keeps the screen looking the same as it did before v1.9 regardless of how +// configuredProviders() happens to sort. +const PROVIDER_STYLE = { + discord: { label: 'Continue with Discord', background: '#5865F2', color: '#fff' }, + github: { label: 'Continue with GitHub', background: '#EAEFF5', color: '#0E141B' }, +}; +const DISPLAY_ORDER = ['discord', 'github']; + +// `authInfo` comes from App, which fetches GET /api/auth-info once at boot and +// falls back to FALLBACK_AUTH_INFO when it cannot be reached - so this +// component always has a usable answer and never fetches it a second time. +export default function Login({ authInfo = FALLBACK_AUTH_INFO }) { const params = new URLSearchParams(window.location.search); const authError = params.get('authError'); + const authReason = params.get('authReason'); + + const [failure, setFailure] = useState(null); + const [pending, setPending] = useState(null); + const info = authInfo; + + // The redirect leg carries its failure in the URL (App.jsx puts it there + // before handing over); a failure to *start* the login is held in state. + const shownError = failure || (authError ? { provider: authError, reason: authReason } : null); + + async function onSuperTokensLogin(providerId) { + setPending(providerId); + setFailure(null); + const res = await startSuperTokensLogin(providerId); + // Resolves only when the navigation never happened. + if (!res.ok) { + setFailure({ provider: providerId, reason: res.reason }); + setPending(null); + } + } + + const providers = DISPLAY_ORDER.filter((id) => info.providers?.includes(id)); return (
RACKSTACK

spare pi to hyperscale

- {authError && ( + {shownError && (
- Login with {authError} failed. Try again. + {loginErrorMessage(shownError.provider, shownError.reason)}
)} - - Continue with Discord - - - Continue with GitHub - + {providers.length === 0 && ( +
+ No login provider is configured on this server. +
+ )} + + {providers.map((id) => { + const { label, ...css } = PROVIDER_STYLE[id]; + const className = 'block w-full rounded-lg py-3 mb-3 text-sm font-semibold'; + + // passport mode is a plain link to a server route that 302s to the + // provider. SuperTokens needs a fetch first, to be told where to go. + if (info.loginFlow === 'passport') { + return ( + + {label} + + ); + } + + return ( + + ); + })}
); diff --git a/client/src/game/api.js b/client/src/game/api.js index d190601..d5f1125 100644 --- a/client/src/game/api.js +++ b/client/src/game/api.js @@ -28,10 +28,92 @@ import { ACTION_FLUSH_MS, ACTION_RETRY_MAX_MS } from './constants.js'; +// --------------------------------------------------------------------------- +// Session refresh (v1.9) +// --------------------------------------------------------------------------- +// +// A SuperTokens access token expires long before the session does; renewing it +// is POST /auth/session/refresh, using the refresh cookie the browser already +// holds. Without this the player is silently logged out mid-session the first +// time the access token lapses. +// +// Off unless the server says the client is driving SuperTokens. In `passport` +// mode a 401 means "not logged in" and there is nothing to renew - the refresh +// endpoint is not even mounted - so attempting it would add a doomed round +// trip to every unauthenticated request. + +let refreshEnabled = false; + +/** Called once at boot with the payload of GET /api/auth-info. */ +export function configureAuthRefresh(authInfo) { + refreshEnabled = authInfo?.loginFlow === 'supertokens'; +} + +// Exported for tests only - module-level state otherwise leaks between cases. +export function __resetAuthRefreshForTests() { + refreshEnabled = false; + inFlightRefresh = null; +} + +// The single shared in-flight refresh. +// +// This is the part most likely to be got wrong, and the failure is not merely +// wasteful. The app fires several requests at once, so an access token that +// has just expired produces a burst of simultaneous 401s. Refreshing per-401 +// would send N concurrent refreshes with the SAME refresh token; SuperTokens +// rotates that token on use, so the first call invalidates the token the other +// N-1 are still presenting. Those look exactly like token theft to the core, +// which responds by revoking the session - turning a routine renewal into a +// forced logout, and only ever under concurrency. +// +// One promise, shared by every caller, same shape as server/userLock.js. +let inFlightRefresh = null; + +function refreshSession() { + if (inFlightRefresh) return inFlightRefresh; + + inFlightRefresh = (async () => { + try { + const res = await fetch('/auth/session/refresh', { + method: 'POST', + credentials: 'include', + }); + return res.ok; + } catch { + return false; + } finally { + // Cleared before this promise settles, so the NEXT 401 starts a fresh + // attempt rather than re-awaiting a completed one. + inFlightRefresh = null; + } + })(); + + return inFlightRefresh; +} + +/** + * fetch + one refresh-and-retry on 401. + * + * `allowRefresh` is what bounds it to a single attempt: the retry passes + * false, so a second 401 is returned to the caller rather than starting a + * refresh loop. A 401 after a successful refresh means the session is + * genuinely gone, not stale. + * + * Throws on network failure, like fetch - callers translate that. + */ +async function fetchWithRefresh(path, opts, allowRefresh = true) { + const res = await fetch(path, { credentials: 'include', ...opts }); + if (res.status !== 401 || !allowRefresh || !refreshEnabled) return res; + + const refreshed = await refreshSession(); + if (!refreshed) return res; + return fetchWithRefresh(path, opts, false); +} + async function request(path, opts) { let res; try { - res = await fetch(path, { credentials: 'include', ...opts }); + res = await fetchWithRefresh(path, opts); } catch (e) { return { status: 0, error: 'network_error' }; } @@ -251,7 +333,10 @@ export function fetchEventParticipation(id) { export async function fetchChangelog() { let res; try { - res = await fetch('/api/changelog', { credentials: 'include' }); + // Refresh-aware like every other call: this sits behind requireAuth too, + // so an expired access token would otherwise render an empty changelog + // rather than renewing and succeeding. + res = await fetchWithRefresh('/api/changelog', {}); } catch (e) { return { status: 0, error: 'network_error' }; } diff --git a/client/src/game/auth.js b/client/src/game/auth.js new file mode 100644 index 0000000..57cac00 --- /dev/null +++ b/client/src/game/auth.js @@ -0,0 +1,233 @@ +// The SuperTokens login flow, hand-rolled (v1.9). +// +// Three calls, deliberately not the `supertokens-web-js` SDK: +// +// GET /auth/authorisationurl - ask the server where to send the player +// POST /auth/signinup - exchange the provider's ?code for a session +// POST /auth/session/refresh - renew an expired access token (see api.js) +// +// Those three, plus the /auth/callback/* redirect target, are EXACTLY the four +// entries in server/app.js's `gateSuperTokensPaths` allowlist. That is not a +// coincidence and it is the reason to hand-roll: the SDK assumes a surface we +// deliberately do not serve. Its signOut() targets POST /auth/signout, which +// v1.8 removed on purpose because it clears only the SuperTokens half of a +// dual-stack session and leaves the legacy JWT cookie authenticating the very +// next request. Adopting the SDK would mean either re-opening that endpoint or +// re-pointing it at /auth/logout - and a login that half-works is worse than +// one that fails loudly. +// +// Error convention matches game/api.js: these functions RETURN failure, they +// never throw. +// +// `credentials: 'include'` is mandatory on both calls. The session cookies +// SuperTokens sets on the signinup response are the entire point; without it +// the request succeeds, the player is handed a session, and the browser throws +// it away. + +// The path the provider redirects back to. Registered on the GitHub/Discord +// OAuth apps alongside the passport callbacks (runbook Part A widens them +// rather than replacing them, so both stacks keep working during a rollout). +export const CALLBACK_PREFIX = '/auth/callback/'; + +// SuperTokens' ThirdParty recipe handles exactly one callback route, +// POST /auth/callback/apple. A GET to /auth/callback/github is not an API it +// serves, so its middleware calls next() and the request falls through to the +// SPA - which is what lets this module handle the redirect in the client at +// all. tests/supertokens.middleware.test.js pins that fallthrough. +export function callbackProviderFromPath(pathname) { + if (typeof pathname !== 'string' || !pathname.startsWith(CALLBACK_PREFIX)) return null; + const rest = pathname.slice(CALLBACK_PREFIX.length).replace(/\/+$/, ''); + // One segment only: '/auth/callback/github/extra' is not a provider. + if (!rest || rest.includes('/')) return null; + return rest; +} + +// sessionStorage rather than localStorage: the verifier is single-use and +// scoped to this one login attempt, so it must not outlive the tab or leak +// into a second one. Absent PKCE (neither GitHub nor Discord uses it today) +// nothing is ever stored - but the SDK may return a verifier for a provider +// that does, and silently dropping it would break that provider's login in a +// way that only shows up at the exchange. +const PKCE_KEY = 'rackstack.st.pkce'; + +function storePkce(verifier) { + try { + if (verifier) window.sessionStorage.setItem(PKCE_KEY, verifier); + else window.sessionStorage.removeItem(PKCE_KEY); + } catch { /* private mode, storage disabled - PKCE providers simply won't work */ } +} + +function takePkce() { + try { + const v = window.sessionStorage.getItem(PKCE_KEY); + window.sessionStorage.removeItem(PKCE_KEY); + return v || undefined; + } catch { return undefined; } +} + +async function requestJSON(path, opts) { + let res; + try { + res = await fetch(path, { credentials: 'include', ...opts }); + } catch { + return { status: 0, error: 'network_error' }; + } + const text = await res.text(); + let body = null; + if (text) { + try { body = JSON.parse(text); } catch { body = null; } + } + if (!res.ok) { + const errBody = (body && typeof body === 'object') ? body : {}; + return { status: res.status, ...errBody, error: errBody.error || 'request_failed' }; + } + return body ?? {}; +} + +// GET /api/auth-info -> { authMode, loginFlow: 'passport'|'supertokens', providers: [id] } +// +// Public by design - it is what the login screen reads before anyone is +// authenticated, which is why it is not on /api/config (that route is behind +// requireAuth). `loginFlow` is the server's decision, not something the client +// re-derives from authMode: in `dual` both stacks work and the server says to +// drive SuperTokens, because exercising that path is the whole point of dual. +export function fetchAuthInfo() { + return requestJSON('/api/auth-info'); +} + +// What to assume when /api/auth-info cannot be reached. `passport` is the +// server's default mode and the documented rollback, so falling back to it +// keeps a player able to log in on a server that is otherwise healthy - the +// alternative is a login screen with no buttons on it. It also leaves refresh +// disabled, which is the safe direction: a pointless refresh attempt in +// passport mode is a doomed round trip on every 401. +export const FALLBACK_AUTH_INFO = Object.freeze({ + authMode: 'passport', + loginFlow: 'passport', + providers: Object.freeze(['github', 'discord']), +}); + +/** + * Step 1 of the login: ask where to send the player, then send them. + * + * Navigates away on success, so it resolves only on failure. Callers should + * treat a returned value as "the login did not start" and show it. + */ +export async function startSuperTokensLogin(providerId, { + origin = window.location.origin, + assign = (url) => window.location.assign(url), +} = {}) { + const redirectURI = `${origin}${CALLBACK_PREFIX}${providerId}`; + const query = new URLSearchParams({ + thirdPartyId: providerId, + redirectURIOnProviderDashboard: redirectURI, + }); + + const res = await requestJSON(`/auth/authorisationurl?${query}`); + if (res.error || res.status !== 'OK' || !res.urlWithQueryParams) { + // The 400 worth naming, because it is what a misconfigured deployment + // returns and it reads as a generic failure otherwise: SuperTokens says + // "the provider could not be found in the configuration" when the + // recipe has no such provider. Through all of v1.8 that was every request, + // because init.js registered the providers under a key the SDK ignores. + return { ok: false, provider: providerId, reason: 'unavailable' }; + } + + // Only meaningful for providers that use PKCE; undefined for GitHub/Discord. + storePkce(res.pkceCodeVerifier); + assign(res.urlWithQueryParams); + return { ok: true }; +} + +/** + * Step 2: the provider has redirected back to /auth/callback/. + * Exchange the code for a session. + * + * Returns { ok: true } once the session cookies are set, or + * { ok: false, provider, reason } for the login screen to render. + */ +export async function completeSuperTokensLogin({ + pathname = window.location.pathname, + search = window.location.search, + origin = window.location.origin, +} = {}) { + const provider = callbackProviderFromPath(pathname); + if (!provider) return { ok: false, provider: null, reason: 'failed' }; + + const params = new URLSearchParams(search); + + // The provider itself refused or the player cancelled. There is no code to + // exchange, and POSTing anyway turns a clear "you cancelled" into an opaque + // server error. + const providerError = params.get('error'); + if (providerError) { + return { + ok: false, + provider, + reason: providerError === 'access_denied' ? 'denied' : 'failed', + }; + } + if (!params.get('code')) return { ok: false, provider, reason: 'failed' }; + + const redirectURIQueryParams = {}; + for (const [key, value] of params.entries()) redirectURIQueryParams[key] = value; + + const pkceCodeVerifier = takePkce(); + + // redirectURIInfo, never oAuthTokens. Submitting tokens directly is refused + // server-side by rejectRawOAuthTokens: accepting caller-supplied tokens as + // proof of identity was an account-takeover bypass, because GitHub's + // validateAccessToken never runs (providers/github.js replaces getUserInfo + // wholesale). + const res = await requestJSON('/auth/signinup', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + thirdPartyId: provider, + redirectURIInfo: { + // Must be byte-identical to the value sent to /auth/authorisationurl: + // the provider echoes it back and SuperTokens compares them. + redirectURIOnProviderDashboard: `${origin}${CALLBACK_PREFIX}${provider}`, + redirectURIQueryParams, + ...(pkceCodeVerifier ? { pkceCodeVerifier } : {}), + }, + }), + }); + + if (res.error) { + // rejectRawOAuthTokens and any other server-side refusal land here. + return { ok: false, provider, reason: 'failed' }; + } + + switch (res.status) { + case 'OK': + return { ok: true, provider, user: res.user }; + case 'SIGN_IN_UP_NOT_ALLOWED': + return { ok: false, provider, reason: 'not_allowed' }; + case 'NO_EMAIL_GIVEN_BY_PROVIDER': + // Should be unreachable: providers.js sets requireEmail:false for + // Discord precisely so the narrowed 'identify' scope cannot trip this. + // Handled anyway, because the alternative is a blank screen. + return { ok: false, provider, reason: 'no_email' }; + default: + return { ok: false, provider, reason: 'failed' }; + } +} + +// Human-readable text for the login screen. Kept next to the reasons that +// produce it so the two cannot drift. +export function loginErrorMessage(provider, reason) { + const name = provider === 'github' ? 'GitHub' : provider === 'discord' ? 'Discord' : 'the provider'; + switch (reason) { + case 'denied': + return `Sign-in with ${name} was cancelled.`; + case 'unavailable': + return `${name} sign-in is not available on this server right now.`; + case 'not_allowed': + 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.`; + default: + return `Login with ${name} failed. Try again.`; + } +} diff --git a/server/auth.js b/server/auth.js index f2bfdca..4690d6a 100644 --- a/server/auth.js +++ b/server/auth.js @@ -59,6 +59,29 @@ export function requireRole(role) { }; } +/** + * Which OAuth providers this deployment actually has credentials for. + * + * The login screen needs this BEFORE anyone is authenticated: rendering a + * button for a provider with no credentials produces a dead end in either + * stack (passport has no strategy registered, SuperTokens has no provider in + * the recipe), and the player has no way to tell that from a broken login. + * + * The credential conditions here are deliberately the same ones + * configurePassport() below and buildProviders() in supertokens/providers.js + * apply. That duplication is guarded by a test asserting all three agree - + * v1.8 lost a week to two copies of a probe drifting the moment one was + * fixed, so a third copy does not get to drift silently. + */ +export function configuredProviders(env = process.env) { + const providers = []; + // Ordered to match PROVIDER_IDS in supertokens/providers.js, so the two + // lists compare equal as arrays and not merely as sets. + if (env.GITHUB_CLIENT_ID && env.GITHUB_CLIENT_SECRET) providers.push('github'); + if (env.DISCORD_CLIENT_ID && env.DISCORD_CLIENT_SECRET) providers.push('discord'); + return providers; +} + export function configurePassport() { let configured = 0; diff --git a/server/routes/authRoutes.js b/server/routes/authRoutes.js index e3e1386..a15aeba 100644 --- a/server/routes/authRoutes.js +++ b/server/routes/authRoutes.js @@ -13,8 +13,8 @@ import express from 'express'; import passport from 'passport'; -import { issueToken, COOKIE_NAME } from '../auth.js'; -import { isPassportEnabled } from '../authMode.js'; +import { issueToken, COOKIE_NAME, configuredProviders } from '../auth.js'; +import { isPassportEnabled, isSuperTokensEnabled } from '../authMode.js'; import { isSuperTokensReady, loadSessionRecipe } from '../supertokens/init.js'; const COOKIE_OPTS = { @@ -42,6 +42,30 @@ function finishLogin(req, res) { export function createAuthRouter({ mode }) { const router = express.Router(); + // The one route here that is NOT a login route, and the one place in the + // codebase serving /api/* from outside routes/api.js. + // + // It lives here because it answers a question only this file knows: which + // stack the client must drive. It cannot live on GET /api/config, which the + // v1.9 plan originally proposed, because that route sits behind requireAuth + // and the caller is by definition not logged in yet. + // + // Deliberately public. It reveals which login buttons to draw and nothing + // else - no credentials, no ids, no per-user state - and every fact in it is + // already observable by looking at the login screen or watching a redirect. + // + // `loginFlow` is a server DECISION, not raw configuration, so the policy + // lives in one place. In `dual` both stacks accept a session, and the client + // is told to use SuperTokens: exercising that path is the entire point of + // dual, and the passport routes stay registered underneath as the rollback. + router.get('/api/auth-info', (req, res) => { + res.json({ + authMode: mode, + loginFlow: isSuperTokensEnabled(mode) ? 'supertokens' : 'passport', + providers: configuredProviders(), + }); + }); + if (isPassportEnabled(mode)) { router.get('/auth/discord', passport.authenticate('discord', { session: false })); router.get( diff --git a/tests/authInfo.test.js b/tests/authInfo.test.js new file mode 100644 index 0000000..6413e6a --- /dev/null +++ b/tests/authInfo.test.js @@ -0,0 +1,158 @@ +// GET /api/auth-info (v1.9) and the two facts the client login flow is built +// on top of. +// +// The endpoint exists because the client must not hardcode either stack: +// hardcoding SuperTokens breaks `passport` mode, which is still the default +// and the documented rollback, and both have to work from one build. +// +// It is NOT on GET /api/config, which the v1.9 plan originally proposed. That +// route sits behind requireAuth and the caller here is by definition not +// logged in yet - the whole point is to decide which login buttons to draw. + +process.env.JWT_SECRET = 'test-secret-auth-info'; +process.env.SUPER_ADMIN_IDS = ''; + +// configurePassport() reads process.env directly, so the strategies need +// credentials here or `passport.authenticate` throws on the passport app. +process.env.GITHUB_CLIENT_ID = 'gh-id'; +process.env.GITHUB_CLIENT_SECRET = 'gh-secret'; +process.env.GITHUB_CALLBACK_URL = 'https://rackstack.example.com/auth/github/callback'; +process.env.DISCORD_CLIENT_ID = 'dc-id'; +process.env.DISCORD_CLIENT_SECRET = 'dc-secret'; +process.env.DISCORD_CALLBACK_URL = 'https://rackstack.example.com/auth/discord/callback'; + +import { + describe, it, expect, beforeAll, afterAll, +} from 'vitest'; +import request from 'supertest'; +import { provisionDatabase } from './helpers/backend.js'; + +const provisioned = await provisionDatabase(); + +const { buildApp } = await import('../server/app.js'); +const { ensureConfig } = await import('../server/configService.js'); +const { driver } = await import('../server/db.js'); +const { configuredProviders } = await import('../server/auth.js'); +const { buildProviders } = await import('../server/supertokens/providers.js'); + +await ensureConfig(); + +const ST_ENV = { + ...process.env, + SUPERTOKENS_CONNECTION_URI: 'http://supertokens.invalid:3567', + SUPERTOKENS_API_KEY: 'test-core-api-key', + PUBLIC_ORIGIN: 'https://rackstack.example.com', +}; + +const apps = {}; + +beforeAll(async () => { + apps.passport = await buildApp({ env: { ...process.env, AUTH_MODE: 'passport' } }); + apps.dual = await buildApp({ env: { ...ST_ENV, AUTH_MODE: 'dual' } }); + apps.supertokens = await buildApp({ env: { ...ST_ENV, AUTH_MODE: 'supertokens' } }); +}); + +afterAll(async () => { + if (driver.__backend === 'pg') await driver.__raw.end(); + await provisioned.cleanup(); +}); + +describe('GET /api/auth-info', () => { + it('is reachable without a session, in every mode', async () => { + // The defining property. A login screen that has to be logged in to learn + // how to log in is useless. + for (const mode of ['passport', 'dual', 'supertokens']) { + const res = await request(apps[mode]).get('/api/auth-info'); + expect(res.status, mode).toBe(200); + } + }); + + it('tells a passport deployment to drive passport', async () => { + const res = await request(apps.passport).get('/api/auth-info'); + expect(res.body.authMode).toBe('passport'); + expect(res.body.loginFlow).toBe('passport'); + }); + + it('tells a dual deployment to drive SuperTokens', async () => { + // In `dual` both stacks accept a session and the passport routes stay + // registered as the rollback - but the client is told to use SuperTokens, + // because exercising that path is the entire point of dual. Until v1.9 the + // client logged in via passport, so `dual` never ran the signInUp mapping + // even once in production. + const res = await request(apps.dual).get('/api/auth-info'); + expect(res.body.authMode).toBe('dual'); + expect(res.body.loginFlow).toBe('supertokens'); + }); + + it('tells a supertokens deployment to drive SuperTokens', async () => { + const res = await request(apps.supertokens).get('/api/auth-info'); + expect(res.body.authMode).toBe('supertokens'); + expect(res.body.loginFlow).toBe('supertokens'); + }); + + it('lists the providers that actually have credentials', async () => { + const res = await request(apps.dual).get('/api/auth-info'); + expect(res.body.providers).toEqual(['github', 'discord']); + }); + + it('leaks nothing beyond what the login screen needs', async () => { + // It is public, so the shape is the security boundary: which buttons to + // draw, and nothing else. Asserted as an exact key set so a future field + // has to be added here deliberately rather than by accident. + const res = await request(apps.dual).get('/api/auth-info'); + expect(Object.keys(res.body).sort()).toEqual(['authMode', 'loginFlow', 'providers']); + expect(JSON.stringify(res.body)).not.toContain('secret'); + }); +}); + +describe('configuredProviders agrees with the SuperTokens provider list', () => { + // Three copies of "does this provider have credentials" now exist: + // configurePassport(), buildProviders() and configuredProviders(). v1.8 lost + // a release to two copies of a core probe drifting the moment one of them + // was fixed, so this pins the two that are machine-comparable. + const CASES = [ + ['both configured', { GITHUB_CLIENT_ID: 'a', GITHUB_CLIENT_SECRET: 'b', DISCORD_CLIENT_ID: 'c', DISCORD_CLIENT_SECRET: 'd' }], + ['github only', { GITHUB_CLIENT_ID: 'a', GITHUB_CLIENT_SECRET: 'b' }], + ['discord only', { DISCORD_CLIENT_ID: 'c', DISCORD_CLIENT_SECRET: 'd' }], + ['neither', {}], + ['id without secret', { GITHUB_CLIENT_ID: 'a', DISCORD_CLIENT_ID: 'c' }], + ['secret without id', { GITHUB_CLIENT_SECRET: 'b', DISCORD_CLIENT_SECRET: 'd' }], + ]; + + it.each(CASES)('%s', (_name, env) => { + const fromAuth = configuredProviders(env); + const fromSuperTokens = buildProviders(env).map((p) => p.config.thirdPartyId); + expect(fromAuth).toEqual(fromSuperTokens); + }); +}); + +describe('the provider redirect target reaches the SPA', () => { + // client/src/game/auth.js handles /auth/callback/ in the browser, + // which is only possible because SuperTokens' middleware does not answer it. + // If a future SDK version starts handling that path, the client's callback + // leg silently stops running and every login dead-ends on a blank page. + + it('SuperTokens handles /auth/authorisationurl but not /auth/callback/github', async () => { + // Differential, deliberately. Asserting only "the callback is not JSON" + // would pass just as well on an app where the SuperTokens middleware was + // never mounted at all - the control request is what proves the SDK is + // live and answering in this very app before the callback is checked. + const handled = await request(apps.dual).get('/auth/authorisationurl'); + expect(handled.headers['content-type']).toMatch(/application\/json/); + + const fellThrough = await request(apps.dual).get('/auth/callback/github'); + expect(fellThrough.headers['content-type'] || '').not.toMatch(/application\/json/); + }); + + it('the ThirdParty recipe handles exactly three APIs, and only apple has a callback', async () => { + // The precise fact the differential test above demonstrates, pinned + // against the SDK itself: APPLE_REDIRECT_HANDLER is '/callback/apple' and + // it is a POST, so a GET to /auth/callback/github matches nothing. + const { readFileSync } = await import('node:fs'); + const constants = readFileSync( + new URL('../node_modules/supertokens-node/lib/build/recipe/thirdparty/constants.js', import.meta.url), + 'utf8', + ); + expect(constants).toContain('APPLE_REDIRECT_HANDLER = "/callback/apple"'); + }); +}); diff --git a/tests/clientAuth.test.js b/tests/clientAuth.test.js new file mode 100644 index 0000000..400b7ef --- /dev/null +++ b/tests/clientAuth.test.js @@ -0,0 +1,332 @@ +// The client half of the SuperTokens flow (v1.9): client/src/game/auth.js and +// the refresh-on-401 wrapper in client/src/game/api.js. +// +// Both are exercised against a stubbed global fetch rather than a browser. +// What matters here is the protocol - which endpoint, which body, how many +// times - and that is exactly what a stub can assert and a rendered component +// cannot. + +process.env.JWT_SECRET = 'test-secret-client-auth'; + +import { + describe, it, expect, beforeEach, afterEach, vi, +} from 'vitest'; + +import { + callbackProviderFromPath, + startSuperTokensLogin, + completeSuperTokensLogin, + loginErrorMessage, + FALLBACK_AUTH_INFO, +} from '../client/src/game/auth.js'; + +import { + fetchState, configureAuthRefresh, __resetAuthRefreshForTests, +} from '../client/src/game/api.js'; + +const ORIGIN = 'https://rackstack.example.com'; + +function jsonResponse(body, { ok = true, status = 200 } = {}) { + return { + ok, + status, + text: async () => JSON.stringify(body), + }; +} + +let originalFetch; + +beforeEach(() => { + originalFetch = globalThis.fetch; + __resetAuthRefreshForTests(); +}); + +afterEach(() => { + globalThis.fetch = originalFetch; + vi.restoreAllMocks(); +}); + +describe('callbackProviderFromPath', () => { + it('recognises the provider redirect target', () => { + expect(callbackProviderFromPath('/auth/callback/github')).toBe('github'); + expect(callbackProviderFromPath('/auth/callback/discord')).toBe('discord'); + }); + + it('tolerates a trailing slash', () => { + expect(callbackProviderFromPath('/auth/callback/github/')).toBe('github'); + }); + + it('is not fooled by a deeper path or a lookalike prefix', () => { + // A deeper path is not a provider, and must not be handed to the server as + // one - `/auth/callback/github/../x` style values are how a redirect + // target turns into an open redirect. + expect(callbackProviderFromPath('/auth/callback/github/extra')).toBeNull(); + expect(callbackProviderFromPath('/auth/callbackfoo/github')).toBeNull(); + expect(callbackProviderFromPath('/auth/callback/')).toBeNull(); + expect(callbackProviderFromPath('/')).toBeNull(); + expect(callbackProviderFromPath(undefined)).toBeNull(); + }); +}); + +describe('startSuperTokensLogin', () => { + it('asks for the authorisation url and navigates to it', async () => { + const calls = []; + globalThis.fetch = vi.fn(async (url, opts) => { + calls.push({ url, opts }); + return jsonResponse({ status: 'OK', urlWithQueryParams: 'https://github.com/login/oauth?x=1' }); + }); + + let navigatedTo = null; + const res = await startSuperTokensLogin('github', { + origin: ORIGIN, + assign: (url) => { navigatedTo = url; }, + }); + + expect(res.ok).toBe(true); + expect(navigatedTo).toBe('https://github.com/login/oauth?x=1'); + + const requested = new URL(calls[0].url, ORIGIN); + expect(requested.pathname).toBe('/auth/authorisationurl'); + expect(requested.searchParams.get('thirdPartyId')).toBe('github'); + // The redirect target the provider will send the player back to, and the + // value SuperTokens will compare against at the exchange. + expect(requested.searchParams.get('redirectURIOnProviderDashboard')) + .toBe(`${ORIGIN}/auth/callback/github`); + expect(calls[0].opts.credentials).toBe('include'); + }); + + it('reports an unconfigured provider instead of navigating', async () => { + // The exact production failure v1.9 fixes: SuperTokens 400s with "the + // provider github could not be found in the configuration" when the recipe + // has no such provider. The player must see a message, not a blank screen. + globalThis.fetch = vi.fn(async () => jsonResponse( + { message: 'the provider github could not be found in the configuration' }, + { ok: false, status: 400 }, + )); + + let navigated = false; + const res = await startSuperTokensLogin('github', { + origin: ORIGIN, + assign: () => { navigated = true; }, + }); + + expect(navigated).toBe(false); + expect(res).toMatchObject({ ok: false, provider: 'github', reason: 'unavailable' }); + expect(loginErrorMessage('github', res.reason)).toMatch(/not available/i); + }); +}); + +describe('completeSuperTokensLogin', () => { + it('exchanges the code via redirectURIInfo and reports the mapped user', async () => { + const calls = []; + globalThis.fetch = vi.fn(async (url, opts) => { + calls.push({ url, opts }); + return jsonResponse({ status: 'OK', user: { id: 'github:37058311' } }); + }); + + const res = await completeSuperTokensLogin({ + pathname: '/auth/callback/github', + search: '?code=abc123&state=xyz', + origin: ORIGIN, + }); + + expect(res.ok).toBe(true); + // The whole release rests on this being the pre-existing users.id rather + // than a SuperTokens UUID. + expect(res.user.id).toBe('github:37058311'); + + expect(calls[0].url).toBe('/auth/signinup'); + expect(calls[0].opts.method).toBe('POST'); + expect(calls[0].opts.credentials).toBe('include'); + + const body = JSON.parse(calls[0].opts.body); + expect(body.thirdPartyId).toBe('github'); + expect(body.redirectURIInfo.redirectURIQueryParams).toEqual({ code: 'abc123', state: 'xyz' }); + expect(body.redirectURIInfo.redirectURIOnProviderDashboard) + .toBe(`${ORIGIN}/auth/callback/github`); + + // Never oAuthTokens: submitting tokens directly is refused server-side by + // rejectRawOAuthTokens, because accepting caller-supplied tokens as proof + // of identity was an account-takeover bypass. + expect(body.oAuthTokens).toBeUndefined(); + }); + + it('does not POST when the player cancelled at the provider', async () => { + globalThis.fetch = vi.fn(async () => jsonResponse({ status: 'OK' })); + + const res = await completeSuperTokensLogin({ + pathname: '/auth/callback/github', + search: '?error=access_denied', + origin: ORIGIN, + }); + + // POSTing a callback that carries no code turns a clear "you cancelled" + // into an opaque server error. + expect(globalThis.fetch).not.toHaveBeenCalled(); + expect(res).toMatchObject({ ok: false, provider: 'github', reason: 'denied' }); + expect(loginErrorMessage('github', res.reason)).toMatch(/cancelled/i); + }); + + it('does not POST when there is no code at all', async () => { + globalThis.fetch = vi.fn(async () => jsonResponse({ status: 'OK' })); + + const res = await completeSuperTokensLogin({ + pathname: '/auth/callback/github', + search: '', + origin: ORIGIN, + }); + + expect(globalThis.fetch).not.toHaveBeenCalled(); + expect(res.ok).toBe(false); + }); + + it('surfaces SIGN_IN_UP_NOT_ALLOWED as its own message', async () => { + globalThis.fetch = vi.fn(async () => jsonResponse({ + status: 'SIGN_IN_UP_NOT_ALLOWED', reason: 'nope', + })); + + const res = await completeSuperTokensLogin({ + pathname: '/auth/callback/discord', + search: '?code=abc', + origin: ORIGIN, + }); + + expect(res).toMatchObject({ ok: false, provider: 'discord', reason: 'not_allowed' }); + expect(loginErrorMessage('discord', res.reason)).toMatch(/not allowed/i); + }); + + it('treats a server refusal (GENERAL_ERROR / rejectRawOAuthTokens) as a failure', async () => { + globalThis.fetch = vi.fn(async () => jsonResponse( + { status: 'GENERAL_ERROR', message: 'refused' }, + { ok: false, status: 400 }, + )); + + const res = await completeSuperTokensLogin({ + pathname: '/auth/callback/github', + search: '?code=abc', + origin: ORIGIN, + }); + + expect(res).toMatchObject({ ok: false, reason: 'failed' }); + }); +}); + +describe('the fallback auth info', () => { + it('assumes passport, which leaves refresh off', () => { + // The safe direction on an unreachable /api/auth-info: buttons still + // render (passport is the server default and the documented rollback) and + // no doomed refresh is attempted on every 401. + expect(FALLBACK_AUTH_INFO.loginFlow).toBe('passport'); + configureAuthRefresh(FALLBACK_AUTH_INFO); + }); +}); + +describe('refresh on 401', () => { + it('refreshes once and retries the original request', async () => { + configureAuthRefresh({ loginFlow: 'supertokens' }); + + const seen = []; + let stateCalls = 0; + globalThis.fetch = vi.fn(async (url) => { + seen.push(url); + if (url === '/auth/session/refresh') return { ok: true, status: 200, text: async () => '' }; + stateCalls += 1; + if (stateCalls === 1) return jsonResponse({ error: 'unauthorized' }, { ok: false, status: 401 }); + return jsonResponse({ run: { level: 3 } }); + }); + + const res = await fetchState(); + + expect(res).toEqual({ run: { level: 3 } }); + expect(seen).toEqual(['/api/state', '/auth/session/refresh', '/api/state']); + }); + + it('gives up after a second 401 rather than looping', async () => { + configureAuthRefresh({ loginFlow: 'supertokens' }); + + let refreshes = 0; + globalThis.fetch = vi.fn(async (url) => { + if (url === '/auth/session/refresh') { + refreshes += 1; + return { ok: true, status: 200, text: async () => '' }; + } + return jsonResponse({ error: 'unauthorized' }, { ok: false, status: 401 }); + }); + + const res = await fetchState(); + + // A 401 after a successful refresh means the session is genuinely gone. + expect(refreshes).toBe(1); + expect(res.status).toBe(401); + }); + + it('sends exactly ONE refresh for concurrent 401s', async () => { + configureAuthRefresh({ loginFlow: 'supertokens' }); + + let refreshes = 0; + let refreshResolve; + const refreshGate = new Promise((resolve) => { refreshResolve = resolve; }); + // The access token is expired for EVERY request until a refresh completes. + // (An earlier version of this test keyed "already seen" off the URL, so + // only the first of the three ever got a 401 - and it passed with the + // single-flight guard deleted. The gate below is what makes all three + // 401 concurrently, which is the whole scenario.) + let refreshed = false; + + globalThis.fetch = vi.fn(async (url) => { + if (url === '/auth/session/refresh') { + refreshes += 1; + await refreshGate; + refreshed = true; + return { ok: true, status: 200, text: async () => '' }; + } + if (!refreshed) return jsonResponse({ error: 'unauthorized' }, { ok: false, status: 401 }); + return jsonResponse({ ok: true }); + }); + + // Three requests fire together, all hit an expired access token. A refresh + // per 401 would send three concurrent calls presenting the SAME refresh + // token; SuperTokens rotates it on use, so the second and third look like + // token theft to the core and it revokes the session. One shared promise + // is what prevents a routine renewal becoming a forced logout. + const all = Promise.all([fetchState(), fetchState(), fetchState()]); + await new Promise((r) => setTimeout(r, 10)); + refreshResolve(); + await all; + + expect(refreshes).toBe(1); + }); + + it('never refreshes in passport mode', async () => { + configureAuthRefresh({ loginFlow: 'passport' }); + + const seen = []; + globalThis.fetch = vi.fn(async (url) => { + seen.push(url); + return jsonResponse({ error: 'unauthorized' }, { ok: false, status: 401 }); + }); + + const res = await fetchState(); + + // In passport mode a 401 means "not logged in" and the refresh endpoint is + // not even mounted. + expect(seen).toEqual(['/api/state']); + expect(res.status).toBe(401); + }); + + it('does not retry when the refresh itself fails', async () => { + configureAuthRefresh({ loginFlow: 'supertokens' }); + + const seen = []; + globalThis.fetch = vi.fn(async (url) => { + seen.push(url); + if (url === '/auth/session/refresh') return { ok: false, status: 401, text: async () => '' }; + return jsonResponse({ error: 'unauthorized' }, { ok: false, status: 401 }); + }); + + const res = await fetchState(); + + expect(seen).toEqual(['/api/state', '/auth/session/refresh']); + expect(res.status).toBe(401); + }); +}); diff --git a/tests/e2e/smoke-v19.mjs b/tests/e2e/smoke-v19.mjs new file mode 100644 index 0000000..79dbf12 --- /dev/null +++ b/tests/e2e/smoke-v19.mjs @@ -0,0 +1,427 @@ +#!/usr/bin/env node +// v1.9 SuperTokens client login - end-to-end smoke suite (Task 2 Step 5). +// +// Covers: +// +// 1. GET /api/auth-info is reachable with no session and reports the mode. +// 2. In `dual` it says loginFlow=supertokens - the client drives SuperTokens +// even though the passport routes are still registered underneath. +// 3. GET /auth/callback/github reaches the SPA rather than being answered by +// SuperTokens' middleware. The entire client callback leg depends on it. +// 4. The login screen renders SuperTokens BUTTONS in dual mode. +// 5. The full redirect round trip over the built client: click -> +// /auth/authorisationurl -> provider redirect -> /auth/callback/github +// -> POST /auth/signinup -> session -> the game renders, with the spent +// ?code= replaced out of the URL. +// 6. A provider that refuses (error=access_denied) lands back on the login +// screen with a readable message, and never POSTs signinup. +// 7. An unconfigured provider (the exact 400 production returned all through +// v1.8) shows a message instead of a blank screen. +// 8. Restarted in `passport` mode, the SAME build renders plain links to +// the passport routes - both stacks from one bundle. +// +// The provider is stubbed by pointing `urlWithQueryParams` back at this +// server's own /auth/callback/github, so no external service is contacted and +// no SuperTokens core is required. POST /auth/signinup is intercepted too and +// answers with a legacy JWT cookie, which `dual` accepts - so the assertion +// "the client completed the flow and the app came up authenticated" is real +// even though the core round trip is stubbed. +// +// What this suite deliberately does NOT prove: that the signInUp mapping +// resolves a returning player to their existing users.id. That needs a real +// SuperTokens core and is Task 1 of the v1.9 plan, to be run against the box +// once this ships. +// +// Same harness shape as smoke-v16.mjs. Every check prints `PASS ` or +// `FAIL : `; `=== ERRORS ===` at the end lists failures or NONE. + +import { spawn } from 'node:child_process'; +import { rmSync, existsSync, readdirSync } from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const REPO_ROOT = path.join(__dirname, '..', '..'); + +const PORT = 3809; +const BASE_URL = `http://localhost:${PORT}`; +const DB_PATH = '/tmp/e2e-v19.db'; +const JWT_SECRET = '1a2b3c4d5e6f708192a3b4c5d6e7f8091a2b3c4d5e6f708192a3b4c5d6e7f80'; + +for (const ext of ['', '-wal', '-shm']) { + try { rmSync(DB_PATH + ext, { force: true }); } catch (e) { /* ignore */ } +} + +process.env.JWT_SECRET = JWT_SECRET; +process.env.DB_PATH = DB_PATH; +process.env.NODE_ENV = 'test'; + +// Both stacks read these. The SuperTokens core is never reachable and never +// contacted: every endpoint that would touch it is intercepted in the browser. +process.env.GITHUB_CLIENT_ID = 'gh-id'; +process.env.GITHUB_CLIENT_SECRET = 'gh-secret'; +process.env.GITHUB_CALLBACK_URL = `${BASE_URL}/auth/github/callback`; +process.env.DISCORD_CLIENT_ID = 'dc-id'; +process.env.DISCORD_CLIENT_SECRET = 'dc-secret'; +process.env.DISCORD_CALLBACK_URL = `${BASE_URL}/auth/discord/callback`; +process.env.PUBLIC_ORIGIN = BASE_URL; +process.env.SUPERTOKENS_CONNECTION_URI = 'http://supertokens.invalid:3567'; +process.env.SUPERTOKENS_API_KEY = 'test-core-api-key'; + +const { upsertUser, putSave, driver } = await import(path.join(REPO_ROOT, 'server', 'db.js')); +const { issueToken, COOKIE_NAME } = await import(path.join(REPO_ROOT, 'server', 'auth.js')); +const { initialState } = await import(path.join(REPO_ROOT, 'shared', 'state.js')); + +if (driver.__backend === 'sqlite') { + driver.__raw.pragma('busy_timeout = 5000'); +} + +let serverProc = null; +let shuttingDown = false; + +function killServer() { + if (serverProc && !serverProc.killed) { + try { serverProc.kill('SIGTERM'); } catch (e) { /* ignore */ } + } +} +process.on('exit', killServer); +process.on('SIGINT', () => { killServer(); process.exit(130); }); +process.on('SIGTERM', () => { killServer(); process.exit(143); }); + +async function startServer(authMode) { + serverProc = spawn(process.execPath, [path.join(REPO_ROOT, 'server', 'index.js')], { + cwd: REPO_ROOT, + env: { ...process.env, PORT: String(PORT), AUTH_MODE: authMode }, + stdio: ['ignore', 'pipe', 'pipe'], + }); + let out = ''; + serverProc.stdout.on('data', (d) => { out += d.toString(); }); + serverProc.stderr.on('data', (d) => { out += d.toString(); }); + serverProc.on('exit', (code, signal) => { + if (code !== null && code !== 0 && !shuttingDown) { + console.error(`\n[server] exited early (code=${code} signal=${signal}); output:\n${out}`); + } + }); + + const deadline = Date.now() + 20000; + for (;;) { + try { + const res = await fetch(`${BASE_URL}/api/auth-info`); + if (res.ok) break; + } catch (e) { /* not up yet */ } + if (Date.now() > deadline) { + throw new Error(`server did not become ready within 20s; output:\n${out}`); + } + // eslint-disable-next-line no-await-in-loop + await new Promise((r) => setTimeout(r, 150)); + } +} + +async function stopServer() { + if (!serverProc || serverProc.killed) return; + const exited = new Promise((resolve) => serverProc.once('exit', resolve)); + serverProc.kill('SIGTERM'); + await Promise.race([exited, new Promise((r) => setTimeout(r, 5000))]); + serverProc = null; +} + +// --------------------------------------------------------------------------- +// Playwright resolution: plain import first, scratchpad fallback second. +// --------------------------------------------------------------------------- + +function findScratchpadPlaywright() { + const found = []; + const tmp = '/tmp'; + let claudeDirs = []; + try { + claudeDirs = readdirSync(tmp).filter((d) => d.startsWith('claude-') || d === 'e2e-verify'); + } catch (e) { + return found; + } + function walk(dir, depth) { + if (depth > 6) return; + let entries; + try { + entries = readdirSync(dir, { withFileTypes: true }); + } catch (e) { + return; + } + for (const ent of entries) { + if (!ent.isDirectory()) continue; + const full = path.join(dir, ent.name); + if (ent.name === 'playwright' && full.includes('node_modules')) { + const idx = path.join(full, 'index.mjs'); + if (existsSync(idx)) found.push(idx); + } + if (ent.name !== 'playwright') walk(full, depth + 1); + } + } + for (const d of claudeDirs) walk(path.join(tmp, d), 0); + return found; +} + +async function loadPlaywrightOrNull() { + try { + return await import('playwright'); + } catch (e) { + for (const c of findScratchpadPlaywright()) { + try { + // eslint-disable-next-line no-await-in-loop + return await import(`file://${c}`); + } catch (e2) { /* try the next candidate */ } + } + return null; + } +} + +const failures = []; + +async function check(name, fn) { + try { + await fn(); + console.log(`PASS ${name}`); + } catch (e) { + console.log(`FAIL ${name}: ${e && e.message ? e.message : e}`); + failures.push({ name, message: e && e.message ? e.message : String(e) }); + } +} + +function skip(name, why) { + console.log(`SKIP ${name}: ${why}`); +} + +function assert(cond, message) { + if (!cond) throw new Error(message); +} + +let seq = 0; +async function seedUser() { + seq += 1; + const user = await upsertUser({ + provider: 'github', providerId: `v19-${seq}`, username: `v19user${seq}`, avatarUrl: null, + }); + await putSave(user.id, initialState(), Date.now()); + return user; +} + +function cookieFor(user) { + return issueToken({ id: user.id, username: user.username, avatar_url: user.avatar_url }); +} + +// --------------------------------------------------------------------------- + +await startServer('dual'); + +await check('GET /api/auth-info answers without a session', async () => { + const res = await fetch(`${BASE_URL}/api/auth-info`); + assert(res.status === 200, `expected 200, got ${res.status}`); + const body = await res.json(); + assert(Array.isArray(body.providers), 'providers should be an array'); + assert(body.providers.includes('github'), 'github should be configured'); +}); + +await check('dual mode tells the client to drive SuperTokens', async () => { + const body = await (await fetch(`${BASE_URL}/api/auth-info`)).json(); + assert(body.authMode === 'dual', `authMode was ${body.authMode}`); + assert(body.loginFlow === 'supertokens', `loginFlow was ${body.loginFlow}`); +}); + +await check('the passport routes are still registered underneath in dual', async () => { + // The rollback path has to stay live: dual means both stacks work. + const res = await fetch(`${BASE_URL}/auth/github`, { redirect: 'manual' }); + assert(res.status === 302, `expected a 302 to GitHub, got ${res.status}`); +}); + +await check('GET /auth/callback/github reaches the SPA, not SuperTokens', async () => { + // SuperTokens' ThirdParty recipe serves only POST /auth/callback/apple, so + // this falls through to the SPA - which is the only reason the client can + // handle the redirect at all. If a future SDK starts answering here, every + // login dead-ends and this is the check that says so. + const res = await fetch(`${BASE_URL}/auth/callback/github?code=x`); + const type = res.headers.get('content-type') || ''; + assert(type.includes('text/html'), `expected the SPA's HTML, got content-type ${type}`); +}); + +const playwright = await loadPlaywrightOrNull(); + +if (!playwright) { + skip('the client login flow', 'playwright is not installed'); +} else { + const browser = await playwright.chromium.launch(); + + const newPage = async () => { + const context = await browser.newContext({ baseURL: BASE_URL }); + const page = await context.newPage(); + return { context, page }; + }; + + await check('the login screen renders SuperTokens buttons in dual mode', async () => { + const { context, page } = await newPage(); + try { + await page.goto(`${BASE_URL}/`); + const button = page.locator('button', { hasText: 'Continue with GitHub' }); + await button.waitFor({ timeout: 10000 }); + // A