From d5330290f12f7c9b9fc57257e20d40ffc7a22927 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 20:35:07 +0000 Subject: [PATCH] fix(plugin-auth): 2FA verification echoes the session it installed (#10701) `/two-factor/verify-totp` answered 200 with two credentials that disagreed: the `Set-Cookie` named the caller's rotated session, while the JSON `token` named the session row the same request had just deleted. better-auth's `verifyTwoFactor` resolves the caller's session once at entry and closes over it, so `valid(ctx)` echoes the PRE-rotation token on the enrolment lane, where the route rotates the session before answering. Because `bearer()` overwrites the request's session cookie with whatever the Authorization header carries, a client that stored the echoed token did not merely fail to authenticate with it -- presenting it destroyed the still-valid rotated cookie and dropped the request to anonymous. The echoed value is now read back out of the response's own session cookie. Shape and meaning of the field are unchanged; only the value moves, from a deleted row to the live one. Resolver precedence is untouched, and two pins hold that line: anonymous is still refused, and a bogus bearer still overrides a valid cookie and still fails closed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx --- ...-factor-verify-echoes-installed-session.md | 64 ++++ .../plugins/plugin-auth/src/auth-manager.ts | 12 + .../src/two-factor-rotated-token-echo.test.ts | 311 ++++++++++++++++++ .../src/two-factor-rotated-token-echo.ts | 167 ++++++++++ 4 files changed, 554 insertions(+) create mode 100644 .changeset/two-factor-verify-echoes-installed-session.md create mode 100644 packages/plugins/plugin-auth/src/two-factor-rotated-token-echo.test.ts create mode 100644 packages/plugins/plugin-auth/src/two-factor-rotated-token-echo.ts diff --git a/.changeset/two-factor-verify-echoes-installed-session.md b/.changeset/two-factor-verify-echoes-installed-session.md new file mode 100644 index 0000000000..91536eddc6 --- /dev/null +++ b/.changeset/two-factor-verify-echoes-installed-session.md @@ -0,0 +1,64 @@ +--- +"@objectstack/plugin-auth": patch +--- + +fix(plugin-auth): a 2FA verification echoes the session it INSTALLED, not the one it deleted (#10701) + +`POST /api/v1/auth/two-factor/verify-totp` answered `200` with two credentials +that disagreed. The `Set-Cookie` named the caller's new session; the JSON +`token` named a session row the same request had just deleted. + +The cause is upstream and mechanical. better-auth's `verifyTwoFactor` helper +resolves the caller's session once, at entry, and closes over it: + +```js +valid: async (ctx) => ctx.json({ token: session.session.token, ... }) +``` + +On the enrolment lane — a signed-in user confirming a new TOTP factor — the +route rotates that session before it answers: it mints a new session, installs +it with `setSessionCookie`, and deletes the caller's original session row. Only +then does it call `valid(ctx)`, which still holds the pre-rotation session and +echoes the token of the row that no longer exists. (Measured on the installed +better-auth 1.7.1: `dist/plugins/two-factor/verify-two-factor.mjs` and +`dist/plugins/two-factor/totp/index.mjs`.) + +Every other auth response in this repo echoes `token` as the unsigned token of +a live session, and `bearer()` accepts exactly that — presented without a +signature it signs the value itself before verifying. Measured on +`/sign-up/email`, the body's `token` resolves to the user as a bearer. So a +client following that contract after enrolling in 2FA stored a revoked token. + +That did not merely fail to authenticate. `bearer()`'s before-hook OVERWRITES +the request's session cookie with whatever the `Authorization` header carries, +so a request presenting the still-valid rotated cookie *and* the dead token +resolved to nobody. Measured before the fix, on one enrolment: the cookie alone +resolved to the user (`get-totp-uri` `200`); the echoed token alone resolved to +nobody (`get-session` `200` and empty, `get-totp-uri` `401`); and the two +together also resolved to nobody (`401`). Fail-closed — no privilege was +available to gain — but a legitimate user was locked out of a session they +still held, which is the point of the report. + +The echoed value is now read back out of the response's own session cookie, so +the `token` names the session the response actually installed. This restores +the contract rather than changing it: the field keeps its shape (the unsigned +session token) and its meaning ("the session you now hold"), and only the value +moves, from a deleted row to the live one. Shipped as `patch` for that reason — +no consumer expression has to be rewritten, and the previous value was not a +usable credential for anything, so nothing could have depended on it. + +The repair is keyed on the mechanism, not on the enrolment branch: it applies +only when the response staged a session cookie whose token differs from the one +being echoed. On the sign-in-challenge lane, where the route mints the session +it echoes, the two agree and this is a no-op — pinned, along with the cookie +lane, so that fixing the broken lane could not quietly rewrite the others. +`/two-factor/verify-otp` carries the byte-identical rotate-then-answer block and +is covered by the same guard; `/two-factor/verify-backup-code` does not rotate +and is unaffected. + +Resolver precedence is deliberately untouched. Having the resolver fall back to +the cookie when a bearer is unusable was the other repair direction named in the +report, and it was ruled out of scope: it would stop an invalid credential from +failing loud. Two pins hold that line — anonymous is still refused, and a bogus +bearer still overrides a valid cookie and still fails closed — so an attempt to +loosen the resolver later reddens this suite instead of passing it. diff --git a/packages/plugins/plugin-auth/src/auth-manager.ts b/packages/plugins/plugin-auth/src/auth-manager.ts index 1044073db0..a512c8ae51 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.ts @@ -37,6 +37,7 @@ import { rotateCallerBearerOnImpersonation, withBearerAdminSessionRecovery, } from './impersonation-bearer-rotation.js'; +import { echoInstalledSessionToken } from './two-factor-rotated-token-echo.js'; import { applyPlatformAdminImpersonation, } from './admin-impersonate-endpoint.js'; @@ -1704,6 +1705,17 @@ export class AuthManager { // rotating that token, `/admin/impersonate-user` is a 200 no-op. await rotateCallerBearerOnImpersonation(ctx); + // ── #10701: a 2FA verification must echo the session it INSTALLED ─ + // On the enrolment lane `/two-factor/verify-totp` rotates the + // caller's session and then echoes the PRE-rotation token — the row + // it just deleted. A client that stores it (the console's + // `auth-session-token` pattern) is holding a revoked credential, and + // because `bearer()` overwrites the request cookie with whatever the + // Authorization header carries, presenting it destroys the valid + // rotated cookie too. See `two-factor-rotated-token-echo.ts`. This + // corrects the echoed VALUE only; resolver precedence is untouched. + await echoInstalledSessionToken(ctx); + // ── ADR-0069 D2: account lockout (counter) ────────────────── // better-auth catches an INVALID_EMAIL_OR_PASSWORD APIError and runs // the after-hook with it on `ctx.context.returned`; a success leaves diff --git a/packages/plugins/plugin-auth/src/two-factor-rotated-token-echo.test.ts b/packages/plugins/plugin-auth/src/two-factor-rotated-token-echo.test.ts new file mode 100644 index 0000000000..391c12c22c --- /dev/null +++ b/packages/plugins/plugin-auth/src/two-factor-rotated-token-echo.test.ts @@ -0,0 +1,311 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #10701 — `/two-factor/verify-totp` echoed a session token it had just +// DELETED, and presenting that token destroyed the caller's valid cookie. +// +// The shape of the defect dictates the shape of these tests. The endpoint +// answered 200, rotated the session cookie correctly, and emitted a correct +// `set-auth-token` — all of it right — and then echoed the PRE-rotation token +// in the JSON body. So a test asserting "verify-totp returns 200", or "a +// `token` came back", or "the response set a session cookie" would have been +// GREEN against the bug. +// +// Every assertion below therefore ends at the same question the runtime asks: +// WHICH PRINCIPAL does the next request resolve to, when the client presents +// the credential this response handed it? And the three cases from the card +// are measured side by side in ONE arrangement, because the finding is not +// "the bearer is useless" — it is that a useless bearer DESTROYS an otherwise +// valid cookie session. +// +// Real better-auth pipeline throughout (the precedent set by +// `impersonation-bearer-rotation.test.ts`): requests go in as `Request` +// objects through `AuthManager.handleRequest`, the tokens are the ones +// better-auth minted, and the resolution path is the real one. Where a test +// wants the seam the data routes actually use, it asks +// `auth.api.getSession({ headers })` — literally what +// `runtime/src/security/resolve-session-principal.ts` calls. + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { createHmac } from 'node:crypto'; +import { AuthManager } from './auth-manager'; +// The SAME in-memory engine the #8243 harness drives, deliberately: a second +// fake would be a second looseness risk and a new `check:engine-double-contract` +// ledger entry, for no added fidelity. +import { createMemoryEngine } from './impersonation-bearer-rotation.test'; + +const SECRET = 'test-secret-at-least-32-chars-long!!'; +const PASSWORD = 'S3cure!Passw0rd-10701'; +const BASE = 'http://localhost:3000/api/v1/auth'; + +// ── RFC 6238 TOTP ────────────────────────────────────────────────────────── +// Hand-rolled rather than imported, for the reason the #3624 dogfood harness +// gives: `@better-auth/utils/otp` is a transitive dependency, and taking a +// direct dependency on it to generate six digits would tie this suite to an +// internal package's resolution. better-auth's defaults are the RFC's +// (SHA-1, 6 digits, 30s), asserted by `enable`'s own otpauth:// URI below. + +function base32Decode(input: string): Buffer { + const ALPHABET = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ234567'; + const clean = input.replace(/=+$/, '').toUpperCase(); + let bits = 0; + let value = 0; + const out: number[] = []; + for (const char of clean) { + const idx = ALPHABET.indexOf(char); + if (idx === -1) throw new Error(`invalid base32 character: ${char}`); + value = (value << 5) | idx; + bits += 5; + if (bits >= 8) { + out.push((value >>> (bits - 8)) & 0xff); + bits -= 8; + } + } + return Buffer.from(out); +} + +/** The 6-digit TOTP for `secret` at the current 30-second step. */ +function totp(secret: Buffer): string { + const counter = Math.floor(Date.now() / 30_000); + const buf = Buffer.alloc(8); + buf.writeBigUInt64BE(BigInt(counter)); + const digest = createHmac('sha1', secret).update(buf).digest(); + const offset = digest[digest.length - 1] & 0x0f; + const code = + ((digest[offset] & 0x7f) << 24) | + ((digest[offset + 1] & 0xff) << 16) | + ((digest[offset + 2] & 0xff) << 8) | + (digest[offset + 3] & 0xff); + return String(code % 1_000_000).padStart(6, '0'); +} + +/** Collect a response's Set-Cookie values into a single request Cookie header. */ +const cookieHeader = (res: Response): string => + (res.headers.getSetCookie?.() ?? []).map((c) => c.split(';')[0]).join('; '); + +const makeManager = (engine: any) => + new AuthManager({ + secret: SECRET, + baseUrl: 'http://localhost:3000', + dataEngine: engine, + plugins: { twoFactor: true }, + } as any); + +const post = (manager: AuthManager, path: string, body: unknown, headers: Record = {}) => + manager.handleRequest( + new Request(`${BASE}${path}`, { + method: 'POST', + headers: { 'Content-Type': 'application/json', ...headers }, + body: JSON.stringify(body ?? {}), + }), + ); + +const sessionRows = (engine: any) => (engine.tables.get('sys_session') ?? []) as any[]; +const userIdFor = (engine: any, email: string): string => { + const row = ((engine.tables.get('sys_user') ?? []) as any[]).find((r) => r.email === email); + if (!row) throw new Error(`no sys_user row for ${email}`); + return String(row.id); +}; + +/** + * WHO does this set of request headers resolve to, asked through the exact + * seam the framework's data routes use. + * + * `null` for anonymous. Never a status code — better-auth answers a dead + * session with a 200 and a JSON `null`, so a status assertion is blind here. + * That is precisely how this defect read in the field: `get-session` came back + * 200 and EMPTY. + */ +const principalFor = async ( + manager: AuthManager, + headers: Record, +): Promise => { + const auth: any = await manager.getAuthInstance(); + const session = await auth.api.getSession({ headers: new Headers(headers) }).catch(() => null); + const id = session?.user?.id ?? session?.session?.userId; + return typeof id === 'string' && id.length > 0 ? id : null; +}; + +/** + * A real protected 2FA route, driven with the given credentials. `get-session` + * alone is not enough evidence: it answers 200 for anonymous. This is the + * route the card names, and it is the one that tells 200 from 401. + */ +const getTotpUri = (manager: AuthManager, headers: Record) => + post(manager, '/two-factor/get-totp-uri', { password: PASSWORD }, headers); + +const EMAIL = 'enroller@example.com'; + +/** + * A signed-in user who has just completed TOTP ENROLMENT — the lane the QA run + * surfaced, and the lane on which the vendor rotates the session mid-request. + * + * Returns everything the three cases need: the token the response echoed, the + * cookie it installed, and the token the caller was holding BEFORE enrolment + * (the value that used to be echoed, kept so the pins can name it exactly). + */ +const arrangeCompletedEnrolment = async () => { + const engine = createMemoryEngine(); + const manager = makeManager(engine); + + const signedUp = await post(manager, '/sign-up/email', { + email: EMAIL, + password: PASSWORD, + name: 'Enrolling User', + }); + expect(signedUp.status).toBe(200); + const preEnrolmentCookie = cookieHeader(signedUp); + const preEnrolmentToken = String(((await signedUp.json()) as any).token); + const userId = userIdFor(engine, EMAIL); + + // The premise: before enrolling, the echoed token IS an accepted bearer. + // Without this, a green suite could never tell "we fixed the echo" from + // "the bearer seam never worked here". + expect(await principalFor(manager, { authorization: `Bearer ${preEnrolmentToken}` })).toBe(userId); + + const enabled = await post(manager, '/two-factor/enable', { password: PASSWORD }, { cookie: preEnrolmentCookie }); + expect(enabled.status, `two-factor/enable: ${await enabled.clone().text()}`).toBe(200); + const { totpURI } = (await enabled.json()) as { totpURI: string }; + const uriSecret = new URL(totpURI.replace('otpauth://', 'https://')).searchParams.get('secret'); + expect(uriSecret, 'no secret in the otpauth URI').toBeTruthy(); + const secret = base32Decode(String(uriSecret)); + + const verified = await post(manager, '/two-factor/verify-totp', { code: totp(secret) }, { cookie: preEnrolmentCookie }); + expect(verified.status, `verify-totp (enrolment): ${await verified.clone().text()}`).toBe(200); + + const echoedToken = String(((await verified.clone().json()) as any).token); + const rotatedCookie = cookieHeader(verified); + expect(rotatedCookie, 'verify-totp installed no session cookie').toContain('session_token='); + + return { engine, manager, userId, secret, preEnrolmentToken, echoedToken, rotatedCookie }; +}; + +beforeEach(() => { + vi.spyOn(console, 'warn').mockImplementation(() => {}); + vi.spyOn(console, 'error').mockImplementation(() => {}); +}); +afterEach(() => vi.restoreAllMocks()); + +// ─────────────────────────────────────────────────────────────────────────── +describe('#10701 — the three cases from the card, one arrangement', () => { + it('the echoed token, the cookie, and the two together all resolve to the user', async () => { + const { manager, userId, echoedToken, rotatedCookie } = await arrangeCompletedEnrolment(); + + const bearerOnly = { authorization: `Bearer ${echoedToken}` }; + const cookieOnly = { cookie: rotatedCookie }; + const both = { ...cookieOnly, ...bearerOnly }; + + // ① the client can keep authenticating with what the response handed it. + // Against the unfixed route this was `null`. + expect(await principalFor(manager, bearerOnly)).toBe(userId); + + // ② the arm that was already right stays right. + expect(await principalFor(manager, cookieOnly)).toBe(userId); + + // ⭐ THE POINT OF THE CARD. Against the unfixed route this was `null`: the + // dead bearer overwrote a perfectly valid cookie and dropped the + // request to anonymous. A fix that only made the bearer work in + // isolation would not be enough — real clients send both. + expect(await principalFor(manager, both)).toBe(userId); + }, 60_000); + + it('the same three cases on a real protected route, not just `get-session`', async () => { + // `get-session` answers 200 for anonymous, so status codes there prove + // nothing. `get-totp-uri` is the route the card names, and against the + // unfixed endpoint it answered 401 for both bearer cases. + const { manager, echoedToken, rotatedCookie } = await arrangeCompletedEnrolment(); + + expect((await getTotpUri(manager, { authorization: `Bearer ${echoedToken}` })).status).toBe(200); + expect((await getTotpUri(manager, { cookie: rotatedCookie })).status).toBe(200); + expect( + (await getTotpUri(manager, { cookie: rotatedCookie, authorization: `Bearer ${echoedToken}` })).status, + ).toBe(200); + }, 60_000); +}); + +// ─────────────────────────────────────────────────────────────────────────── +describe('#10701 — the echoed token is the LIVE session, named exactly', () => { + it('it is the rotated session row, and not the deleted pre-enrolment one', async () => { + // Corroborates the resolution assertions at the storage layer, and pins + // the exact wrong value: asserting only "the echo changed" would be + // satisfied by echoing any other string. + const { engine, userId, preEnrolmentToken, echoedToken } = await arrangeCompletedEnrolment(); + + const live = sessionRows(engine).filter((r) => String(r.userId ?? r.user_id) === userId); + expect(live.map((r) => String(r.token))).toContain(echoedToken); + + expect(echoedToken).not.toBe(preEnrolmentToken); + expect(sessionRows(engine).map((r) => String(r.token))).not.toContain(preEnrolmentToken); + }, 60_000); + + it('it matches the session the response installed in its own cookie', async () => { + // The credential is READ BACK out of the response rather than minted, so + // this equality is the whole safety argument: the fix cannot hand a caller + // a session the request did not already grant it. + const { echoedToken, rotatedCookie } = await arrangeCompletedEnrolment(); + + const cookieValue = /session_token=([^;]+)/.exec(rotatedCookie)?.[1]; + expect(cookieValue, 'no session cookie to compare against').toBeTruthy(); + expect(decodeURIComponent(String(cookieValue)).split('.')[0]).toBe(echoedToken); + }, 60_000); +}); + +// ─────────────────────────────────────────────────────────────────────────── +describe('#10701 — nothing was loosened', () => { + // Direction (b) from the card — the resolver falling back to the cookie when + // the bearer is unusable — was ruled OUT of scope precisely because it stops + // an invalid credential from failing loud. These two pins are what would go + // red if someone later implemented it, so they are the guard on that ruling, + // not decoration. + + it('anonymous is still refused', async () => { + // Pinning only the success direction goes green on a loosened + // implementation that authenticates everybody. + const { manager } = await arrangeCompletedEnrolment(); + + expect(await principalFor(manager, {})).toBeNull(); + expect((await getTotpUri(manager, {})).status).toBe(401); + }, 60_000); + + it('a bogus bearer still overrides a valid cookie and still fails loud', async () => { + // Bearer-over-cookie precedence is better-auth's, and this card does NOT + // change it. An invalid credential must keep failing closed even when the + // request also carries a good cookie. + const { manager, userId, rotatedCookie } = await arrangeCompletedEnrolment(); + + const bogus = { authorization: 'Bearer not-a-real-session-token' }; + expect(await principalFor(manager, { cookie: rotatedCookie })).toBe(userId); + expect(await principalFor(manager, { cookie: rotatedCookie, ...bogus })).toBeNull(); + expect((await getTotpUri(manager, { cookie: rotatedCookie, ...bogus })).status).toBe(401); + }, 60_000); +}); + +// ─────────────────────────────────────────────────────────────────────────── +describe('#10701 — the sign-in-challenge lane is untouched', () => { + it('completing a 2FA SIGN-IN still echoes a token that authenticates', async () => { + // The lane where the vendor mints the session it echoes: the two already + // agreed, so the repair is a no-op here. Pinned because "fix the broken + // lane" must not become "rewrite every lane". + const { manager, userId, secret, rotatedCookie } = await arrangeCompletedEnrolment(); + + // A fresh sign-in now stops at the 2FA challenge instead of returning a + // session — that is what having enrolled means. + const challenged = await post(manager, '/sign-in/email', { email: EMAIL, password: PASSWORD }); + expect(challenged.status).toBe(200); + expect((await challenged.clone().json()) as any).toMatchObject({ twoFactorRedirect: true }); + + const completed = await post( + manager, + '/two-factor/verify-totp', + { code: totp(secret) }, + { cookie: cookieHeader(challenged) }, + ); + expect(completed.status, `verify-totp (sign-in): ${await completed.clone().text()}`).toBe(200); + + const signInToken = String(((await completed.clone().json()) as any).token); + expect(await principalFor(manager, { authorization: `Bearer ${signInToken}` })).toBe(userId); + expect((await getTotpUri(manager, { authorization: `Bearer ${signInToken}` })).status).toBe(200); + + // And the enrolment session is a different, still-independent session. + expect(signInToken).not.toBe(rotatedCookie); + }, 60_000); +}); diff --git a/packages/plugins/plugin-auth/src/two-factor-rotated-token-echo.ts b/packages/plugins/plugin-auth/src/two-factor-rotated-token-echo.ts new file mode 100644 index 0000000000..6b352697ea --- /dev/null +++ b/packages/plugins/plugin-auth/src/two-factor-rotated-token-echo.ts @@ -0,0 +1,167 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #10701 — a successful 2FA verification echoed a session token it had just + * DELETED, and a client that believed the echo locked itself out. + * + * ## The defect + * + * better-auth's `verifyTwoFactor` helper resolves the caller's session ONCE, + * at entry, and closes over it: + * + * valid: async (ctx) => ctx.json({ token: session.session.token, ... }) + * + * (`dist/plugins/two-factor/verify-two-factor.mjs`, measured 2026-08-21 + * against the installed better-auth `1.7.1`.) + * + * On the ENROLMENT lane — a caller who is already signed in confirming a new + * TOTP factor — `/two-factor/verify-totp` rotates that session before it + * answers: it mints a new session, installs it with `setSessionCookie`, and + * then deletes the caller's original session row + * (`dist/plugins/two-factor/totp/index.mjs`). Only afterwards does it call + * `valid(ctx)` — which still holds the pre-rotation session and echoes the + * token of the row that was just deleted. + * + * So the 200 carries two credentials that disagree. The `Set-Cookie` names the + * live session; the JSON `token` names a session that no longer exists. + * + * ## Why a dead echo is worse than no echo + * + * Every other auth response in this repo echoes `token` as the UNSIGNED token + * of a LIVE session, and `bearer()` accepts exactly that: presented without a + * signature it signs the value itself before verifying + * (`dist/plugins/bearer/index.mjs` — the no-`.` branch). Measured on + * `/sign-up/email`: the body's `token` resolves to the user as a bearer. + * + * A client following that contract after enrolling in 2FA therefore stores a + * revoked token — and `bearer()`'s before-hook does not merely fail to + * authenticate it, it OVERWRITES the request's session cookie with it. A + * request carrying the still-valid rotated cookie AND the dead bearer resolves + * to nobody. The dead echo does not just fail; it destroys a working session. + * Fail-closed, no privilege gained — and a legitimate user locked out. + * + * ## The fix — echo the session the response actually installed + * + * This restores the contract; it does not change it. The field keeps its + * shape (the unsigned token) and its meaning ("the session you now hold"). + * Only the value is corrected, from a deleted row to the live one — the very + * session the response staged in `Set-Cookie` a few lines earlier. + * + * Deliberately keyed on the MECHANISM rather than on the enrolment branch: the + * echo is repaired only when the response staged a session cookie whose token + * differs from the one being echoed. On the sign-in-challenge lane, where + * `valid()` mints the session it echoes, the two agree and this is a byte-for- + * byte no-op. Nothing is invented: the corrected value is read back out of the + * response's own cookie, so this cannot hand a caller a credential the request + * did not already grant it. + * + * ⚠️ NOT a resolver change. `bearer()`'s precedence over the cookie is + * untouched, and an invalid bearer still fails loud exactly as it does today — + * relaxing that rejection was considered and explicitly ruled out of scope for + * this card. The only thing that changes is which token we hand out. + * + * ## Scope + * + * `/two-factor/verify-otp` carries the byte-identical rotate-then-`valid(ctx)` + * block (same dist tree, `otp/index.mjs`), so it is the same defect and is + * covered by the same guard rather than left as a known-identical hole. The + * pins in `two-factor-rotated-token-echo.test.ts` drive the TOTP path, which is + * the one this repo's plugin wiring can exercise without OTP transport config. + * `/two-factor/verify-backup-code` does NOT rotate and is unaffected either + * way; it is listed for neither. + */ + +/** The 2FA verification routes whose vendor implementation rotates the session. */ +export const ROTATING_TWO_FACTOR_VERIFY_PATHS: readonly string[] = [ + '/two-factor/verify-totp', + '/two-factor/verify-otp', +]; + +/** Every `Set-Cookie` currently staged on the response, however the runtime spells it. */ +function stagedSetCookies(headers: Headers | undefined): string[] { + if (!headers) return []; + const viaGetter = (headers as any).getSetCookie?.(); + if (Array.isArray(viaGetter)) return viaGetter; + const joined = headers.get('set-cookie'); + return joined ? [joined] : []; +} + +/** + * Percent-decode a cookie value the way `bearer()` does — and, like it, use the + * value verbatim when it turns out not to be percent-encoded after all. + */ +function tryDecode(value: string): string { + try { + return decodeURIComponent(value); + } catch { + return value; + } +} + +/** + * The UNSIGNED session token the response is installing, or `undefined` when it + * is not installing one. + * + * The cookie carries `.`; `session.token` — the value every + * auth response body echoes — is the part before the signature. An expiring + * cookie (`max-age=0`) is a sign-OUT, not a rotation, and is ignored, matching + * the guard `bearer()`'s own after-hook uses before emitting `set-auth-token`. + */ +async function installedSessionToken(ctx: any): Promise { + const headers: Headers | undefined = ctx?.context?.responseHeaders; + const staged = stagedSetCookies(headers); + if (staged.length === 0) return undefined; + const cookieName: unknown = ctx?.context?.authCookies?.sessionToken?.name; + if (typeof cookieName !== 'string' || !cookieName) return undefined; + + const { parseSetCookieHeader } = await import('better-auth/cookies'); + for (const cookie of staged) { + const parsed = parseSetCookieHeader(cookie).get(cookieName); + if (!parsed?.value) continue; + if (parsed['max-age'] === 0) continue; + const unsigned = tryDecode(parsed.value).split('.')[0]; + if (unsigned) return unsigned; + } + return undefined; +} + +/** Did the route succeed, and does its payload echo a token? */ +async function echoedTokenPayload(ctx: any): Promise<{ token: string } | undefined> { + const returned = ctx?.context?.returned; + if (!returned || typeof returned !== 'object') return undefined; + try { + const { isAPIError } = await import('better-auth/api'); + if (isAPIError(returned)) return undefined; + } catch { + if (returned instanceof Error) return undefined; + } + return typeof (returned as any).token === 'string' && (returned as any).token + ? (returned as any) + : undefined; +} + +/** + * Repair the `token` a 2FA verification echoes, so it names the session the + * same response installed rather than the one it deleted. + * + * A no-op for every other path, for a failed verification, for a response that + * installs no session cookie, and — the common case — whenever the echoed token + * already matches the installed one. + * + * Never throws. A verification that genuinely succeeded must not be turned into + * a failure because the response body could not be tidied; the caller's cookie + * is valid either way, and this repair only widens which credentials from the + * response work. Any unexpected shape therefore leaves the payload untouched. + */ +export async function echoInstalledSessionToken(ctx: any): Promise { + try { + if (!ROTATING_TWO_FACTOR_VERIFY_PATHS.includes(ctx?.path)) return; + const payload = await echoedTokenPayload(ctx); + if (!payload) return; + const installed = await installedSessionToken(ctx); + if (!installed || installed === payload.token) return; + payload.token = installed; + } catch { + /* leave the payload exactly as the vendor route wrote it */ + } +}