diff --git a/.changeset/client-adopts-rotated-session-token.md b/.changeset/client-adopts-rotated-session-token.md new file mode 100644 index 0000000000..7347744748 --- /dev/null +++ b/.changeset/client-adopts-rotated-session-token.md @@ -0,0 +1,27 @@ +--- +"@objectstack/client": minor +--- + +feat(client): a bearer-mode `ObjectStackClient` keeps the session the server rotates it onto (#16534) + +Three better-auth routes ROTATE the caller's session on success — they mint a new session, install it in `Set-Cookie` (and, through `bearer()`, in the `set-auth-token` response header), and DELETE the row the caller was presenting: + +| route | where the new credential is | +| --- | --- | +| `auth.twoFactor.verifyTotp()` on the enrolment lane | body — `token`, and it is the LIVE one (plugin-auth's `two-factor-rotated-token-echo` repairs the vendor's stale echo) | +| `auth.changePassword({ revokeOtherSessions: true })` | body — `token` | +| `auth.twoFactor.disable()` | **response header only** — the body is `{ status: true }` | + +A browser is carried across all three by its own cookie. A bearer client — this SDK's own mode — kept presenting the DELETED session's token, so its very next call answered `401 UNAUTHORIZED`. Measured against a real `AuthManager` (better-auth 1.7.2) over a real driver, driven through the real `ObjectStackClient`, `login → enable → verifyTotp → disable → deleteUser` could not run to the end without the caller re-seating `client.token` by hand between the steps. + +The three methods now adopt the rotated credential themselves, the way `login()` already adopts the token it is handed. The `token` members stay on the wire and stay declared, so a caller that keeps its own credential store is unaffected; what changes is that it no longer has to. + +**No public surface moves.** No new export, no new option or flag, no new key on any declared request or response type — the SDK stores a token the server already sends and this package already declares. Graded `minor` rather than `patch` because the published runtime behaviour of three methods moves for existing callers. + +## What does NOT change, deliberately + +The adoption is on those three routes only, never in the shared `fetch` wrapper. `set-auth-token` rides **every** response that stages a session cookie — `POST /update-user` stages one to carry the updated user without rotating anything — and it carries the SIGNED `.` spelling while every JSON `token` echo carries the UNSIGNED one. A wrapper-level read would therefore rewrite the stored credential into a different spelling of the SAME session on ordinary traffic. `auth.me()`, `auth.sessions.list()`, `auth.updateUser()` and `auth.twoFactor.verifyBackupCode()` (which does not rotate — the vendor echoes the session it resolved at entry) all leave the stored credential byte-identical, and that is pinned. + +A cookie-only deployment sends no `set-auth-token`; there is then nothing to adopt and `twoFactor.disable()` leaves the stored credential exactly as it was. `changePassword` without `revokeOtherSessions` answers `token: null` and likewise stores nothing. + +The three TSDoc warnings that told bearer callers "this SDK does not store it" are updated in the same change. diff --git a/packages/client/package.json b/packages/client/package.json index a2def6c7bd..cdcd20c91a 100644 --- a/packages/client/package.json +++ b/packages/client/package.json @@ -32,6 +32,8 @@ "@objectstack/metadata-core": "workspace:*", "@objectstack/metadata-protocol": "workspace:*", "@objectstack/objectql": "workspace:*", + "@objectstack/platform-objects": "workspace:*", + "@objectstack/plugin-auth": "workspace:*", "@objectstack/plugin-hono-server": "workspace:*", "@objectstack/rest": "workspace:*", "@objectstack/runtime": "workspace:*", diff --git a/packages/client/src/auth-rotated-session-token.test.ts b/packages/client/src/auth-rotated-session-token.test.ts new file mode 100644 index 0000000000..6eb528daed --- /dev/null +++ b/packages/client/src/auth-rotated-session-token.test.ts @@ -0,0 +1,476 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #16534 — a bearer-mode `ObjectStackClient` was signed out by the three +// better-auth routes that ROTATE the caller's session. +// +// ## Why the server here is the real one +// +// The claim under test is "the SDK keeps the session the REAL server just +// handed it". Nothing short of the real `AuthManager` supports it: the +// rotation, the `set-auth-token` response header, and plugin-auth's own +// `two-factor-rotated-token-echo` repair — which is what makes `verify-totp`'s +// echoed token the LIVE one rather than the vendor's stale pre-rotation +// snapshot — are all server-side facts. A hand-written stand-in for the server +// would let this suite certify the SDK against a rotation this file invented. +// +// So the arrangement is the card's own probe with only the socket stood in +// for: a real `AuthManager` (better-auth 1.7.2, `bearer()` + `twoFactor`) over +// a real `ObjectQL` on a real `SqliteWasmDriver`, and an `ObjectStackClient` +// whose `fetch` hands the `Request` straight to `AuthManager.handleRequest`. +// Everything the client sends is what it would put on a socket, and everything +// it reads is what better-auth wrote. +// +// Deliberately NO cookie jar. The defect is bearer-only — a browser is carried +// across every rotation by its own cookie — so a jar would hide exactly the +// failure this suite exists to measure. +// +// ## What each block is for +// +// - `① the card's probe` — the end-to-end sequence from the card body with the +// line `(the probe re-set client.token by hand here to continue)` DELETED. +// Its absence is the acceptance criterion; there is no manual credential +// repair anywhere in this file. +// - `② one assertion per route` — the three rotating routes measured +// separately, because they are three different jobs. Two echo the new token +// in the body; `twoFactor.disable` echoes only `{ status: true }` and is the +// only one whose credential arrives in a response HEADER. +// - `③ the negative control` — the routes that DO NOT rotate must leave the +// stored credential byte-identical. `updateUser` is the decisive leg: it +// stages a session cookie to carry the updated user WITHOUT rotating, so +// `bearer()` emits a `set-auth-token` for it too. An implementation that read +// that header in the shared `fetch` wrapper instead of on the rotating routes +// would rewrite the stored credential on an ordinary write — and every +// assertion in ① and ② would stay green. + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { createHmac } from 'node:crypto'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm'; +import { AuthManager } from '@objectstack/plugin-auth'; +import * as identityObjects from '@objectstack/platform-objects/identity'; +import { ObjectStackClient } from './index'; + +const SECRET = 'test-secret-at-least-32-chars-long!!'; +const PASSWORD = 'S3cure!Passw0rd-16534'; +const NEW_PASSWORD = 'S3cure!Passw0rd-16534-rotated'; +const ORIGIN = 'http://localhost:3000'; + +/** + * The identity objects this arrangement stands up — the user, credential, + * session, verification and two-factor rows better-auth's ObjectQL adapter + * reads and writes on the routes under test, plus every sibling the boot path + * touches (`AuthManager` resolves an OIDC resource row on startup, so a + * hand-picked subset fails at `sys_oauth_resource` before the first request). + * + * Read out of `@objectstack/platform-objects/identity` by shape rather than + * transcribed as a list: plugin-auth's own `authIdentityObjects` is + * package-private, and a hand-copied list here would be a second declaration + * of the same set, drifting silently the day the plugin registers one more. + */ +const IDENTITY_OBJECTS = Object.values( + identityObjects as unknown as Record, +).filter( + (o): o is Record => + !!o && + typeof o === 'object' && + typeof (o as { name?: unknown }).name === 'string' && + typeof (o as { fields?: unknown }).fields === 'object', +); + +// ── RFC 6238 TOTP ────────────────────────────────────────────────────────── +// Hand-rolled for the reason `two-factor-rotated-token-echo.test.ts` 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), which the `otpauth://` URI `enable` answers with states itself. + +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'); +} + +// ── the arrangement ──────────────────────────────────────────────────────── + +/** + * The credential the client is currently presenting. + * + * `token` is private and it STAYS private: reading it through a cast is the + * test's business, and publishing an accessor would be a new export on a card + * whose whole point is that no public surface changes. + */ +const storedToken = (client: ObjectStackClient): string | undefined => + (client as unknown as { token?: string }).token; + +const engines: ObjectQL[] = []; + +const makeEngine = async (): Promise => { + const engine = new ObjectQL(); + engines.push(engine); + engine.registerDriver(new SqliteWasmDriver({ filename: ':memory:' }) as never, true); + await engine.init(); + for (const object of IDENTITY_OBJECTS) { + engine.registry.registerObject(object as never, '@objectstack/plugin-auth'); + } + await engine.syncSchemas(); + return engine; +}; + +/** + * A client whose only stand-in is the transport: everything above this call is + * the SDK's real request-building path and everything below it is better-auth's + * real pipeline. + */ +const newClient = (manager: AuthManager, token?: string): ObjectStackClient => + new ObjectStackClient({ + baseUrl: ORIGIN, + ...(token ? { token } : {}), + fetch: (input: RequestInfo | URL, init?: RequestInit) => + manager.handleRequest(new Request(String(input), init)), + }); + +/** A real `AuthManager` plus a client whose socket IS that manager. */ +const arrange = async () => { + const engine = await makeEngine(); + const manager = new AuthManager({ + secret: SECRET, + baseUrl: ORIGIN, + dataEngine: engine, + plugins: { twoFactor: true }, + } as never); + + return { engine, manager, client: newClient(manager) }; +}; + +let emailSeq = 0; +const nextEmail = () => `rotation-${++emailSeq}-${Date.now()}@example.com`; + +/** A signed-in bearer client, and the address it signed up with. */ +const signedIn = async () => { + const { engine, manager, client } = await arrange(); + const email = nextEmail(); + await client.auth.register({ email, password: PASSWORD, name: 'Rotating User' }); + const token = storedToken(client); + expect(token, 'register stored no bearer token — the premise of this suite is gone').toBeTruthy(); + return { engine, manager, client, email, token: String(token) }; +}; + +/** + * Enrol the signed-in client in TOTP, stopping just BEFORE the rotating + * `verifyTotp` call. Returns the TOTP secret and the backup codes `enable` + * minted (the negative control needs one). + */ +const enrolTotp = async (client: ObjectStackClient) => { + const { totpURI, backupCodes } = await client.auth.twoFactor.enable({ password: PASSWORD }); + expect(totpURI, 'two-factor/enable answered no otpauth URI').toBeTruthy(); + const uriSecret = new URL(String(totpURI).replace('otpauth://', 'https://')).searchParams.get( + 'secret', + ); + expect(uriSecret, 'no secret in the otpauth URI').toBeTruthy(); + return { secret: base32Decode(String(uriSecret)), backupCodes: backupCodes ?? [] }; +}; + +/** + * WHO does a bearer credential resolve to, asked through the exact seam the + * framework's data routes use — `runtime/src/security/resolve-session-principal.ts` + * calls literally this. + * + * `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, which is + * exactly how the defect read in the field. + */ +const principalFor = async ( + manager: AuthManager, + token: string | undefined, +): Promise => { + const auth = (await manager.getAuthInstance()) as unknown as { + api: { getSession(a: { headers: Headers }): Promise }; + }; + const session = (await auth.api + .getSession({ headers: new Headers({ authorization: `Bearer ${token}` }) }) + .catch(() => null)) as { user?: { id?: string }; session?: { userId?: string } } | null; + const id = session?.user?.id ?? session?.session?.userId; + return typeof id === 'string' && id.length > 0 ? id : null; +}; + +const principalForStoredToken = (manager: AuthManager, client: ObjectStackClient) => + principalFor(manager, storedToken(client)); + +/** The `sys_user` id for an address, read at driver level below the adapter. */ +const userIdFor = async (engine: ObjectQL, email: string): Promise => { + const driver = ( + engine as unknown as { getDriver(o: string): { find(o: string, q: unknown): Promise } } + ).getDriver('sys_user'); + const found = await driver.find('sys_user', { where: {} }); + const rows = (Array.isArray(found) ? found : [found]).filter(Boolean) as Record[]; + const row = rows.find((r) => r.email === email); + if (!row) throw new Error(`no sys_user row for ${email}`); + return String(row.id); +}; + +/** + * The SIGNED `.` spelling of an unsigned session token — what + * `bearer()` puts in `set-auth-token`, obtained the way `bearer()` produces it: + * from a response that stages a session cookie. `/update-user` stages one + * WITHOUT rotating, which is exactly the property this helper needs. + */ +const signedCredentialFor = async ( + manager: AuthManager, + unsignedToken: string, +): Promise => { + const res = await manager.handleRequest( + new Request(`${ORIGIN}/api/v1/auth/update-user`, { + method: 'POST', + headers: { 'Content-Type': 'application/json', authorization: `Bearer ${unsignedToken}` }, + body: JSON.stringify({ name: 'Signed-Credential Probe' }), + }), + ); + const signed = res.headers.get('set-auth-token'); + expect( + signed, + 'update-user emitted no set-auth-token to read the signed spelling from', + ).toBeTruthy(); + return String(signed); +}; + +beforeEach(() => { + vi.spyOn(console, 'warn').mockImplementation(() => {}); + vi.spyOn(console, 'error').mockImplementation(() => {}); +}); +afterEach(async () => { + vi.restoreAllMocks(); + while (engines.length) { + const e = engines.pop(); + try { + await (e as unknown as { destroy?(): Promise })?.destroy?.(); + } catch { + /* noop */ + } + } +}); + +// ─────────────────────────────────────────────────────────────────────────── +describe("#16534 ① the card's own probe, with the manual re-set deleted", () => { + it('login → enable → verifyTotp → disable → deleteUser, no hand-repaired credential', async () => { + const { engine, manager, client, email } = await signedIn(); + const userId = await userIdFor(engine, email); + + // ── the probe's FIRST step, spelled the way the card spells it. The card's + // sequence opens on `login`, not on the registration that had to precede + // it, so this opens on `login` too — a real second sign-in whose token + // the SDK stores, which is the line the card measured verbatim: + // `client.auth.login({ email, password }) -> RESOLVED { token, user }`. + const session = await client.auth.login({ email, password: PASSWORD }); + expect(session?.data?.token, 'login echoed no token').toBeTruthy(); + const afterLogin = String(storedToken(client)); + expect(afterLogin, 'login did not store the token it was handed').toBe(session.data?.token); + + // The premise. Without it a green run could not tell "the rotation is + // followed now" from "the bearer seam never worked here". + expect(await principalForStoredToken(manager, client)).toBe(userId); + + const { secret } = await enrolTotp(client); + + // ── rotation #1 — the enrolment lane echoes the live token in the body. + const verified = await client.auth.twoFactor.verifyTotp({ code: totp(secret) }); + expect(verified.token, 'verify-totp echoed no token').toBeTruthy(); + const afterVerify = String(storedToken(client)); + expect(afterVerify).not.toBe(afterLogin); + // + // ⭐ THE ACCEPTANCE CRITERION. The card's probe carried a line right here + // reading `(the probe re-set client.token by hand here to continue)`. + // There is no such line, and the sequence continues. + // + // ── rotation #2 — `disable` echoes `{ status: true }` and NOTHING else; + // its credential is in the `set-auth-token` response header. + const receipt = await client.auth.twoFactor.disable({ password: PASSWORD }); + expect(receipt).toEqual({ status: true }); + const afterDisable = String(storedToken(client)); + expect(afterDisable).not.toBe(afterVerify); + + // The very next call — the one the card measured as `401 UNAUTHORIZED`. + // `delete-user` is booked `disabled` in `auth-route-ledger.ts`, so it + // refuses either way; WHICH refusal it is, is the whole finding. Against + // the unfixed SDK it was 401, a dead credential; the route's own refusal + // is not 401. + const rejection = await client.auth + .deleteUser({ password: PASSWORD }) + .then(() => null) + .catch((e: { httpStatus?: number; code?: string }) => e); + expect(rejection, 'delete-user resolved; this probe assumes the route refuses').not.toBeNull(); + expect( + rejection?.httpStatus, + `delete-user refused with ${rejection?.httpStatus} ${rejection?.code ?? ''} — a 401 means the stored credential is dead`, + ).not.toBe(401); + + // And the positive half, stated directly rather than inferred from a + // status code: after the whole sequence the client is still holding a + // credential that resolves to the same principal. + expect(await principalForStoredToken(manager, client)).toBe(userId); + }, 60_000); +}); + +// ─────────────────────────────────────────────────────────────────────────── +describe('#16534 ② one assertion per rotating route — all three', () => { + it('changePassword({ revokeOtherSessions: true }) — the body echoes the new token', async () => { + const { engine, manager, client, email } = await signedIn(); + const userId = await userIdFor(engine, email); + const before = String(storedToken(client)); + + const result = await client.auth.changePassword({ + currentPassword: PASSWORD, + newPassword: NEW_PASSWORD, + revokeOtherSessions: true, + }); + + expect(result.token, 'change-password echoed no rotated token').toBeTruthy(); + expect(storedToken(client)).toBe(result.token); + expect(storedToken(client)).not.toBe(before); + // The stored value is a live credential, not merely a different string. + expect(await principalForStoredToken(manager, client)).toBe(userId); + // …and the one it replaced is genuinely gone. + expect(await principalFor(manager, before)).toBeNull(); + }, 60_000); + + it('changePassword WITHOUT revokeOtherSessions rotates nothing and stores nothing', async () => { + // The other half of the same route: `token` is `null` there, and a client + // that adopted `null` would sign itself out on an ordinary password change. + const { manager, client } = await signedIn(); + const before = String(storedToken(client)); + + const result = await client.auth.changePassword({ + currentPassword: PASSWORD, + newPassword: NEW_PASSWORD, + }); + + expect(result.token).toBeNull(); + expect(storedToken(client)).toBe(before); + expect(await principalForStoredToken(manager, client)).not.toBeNull(); + }, 60_000); + + it('twoFactor.verifyTotp on the enrolment lane — the body echoes the LIVE token', async () => { + const { engine, manager, client, email } = await signedIn(); + const userId = await userIdFor(engine, email); + const before = String(storedToken(client)); + const { secret } = await enrolTotp(client); + + const result = await client.auth.twoFactor.verifyTotp({ code: totp(secret) }); + + expect(storedToken(client)).toBe(result.token); + expect(storedToken(client)).not.toBe(before); + expect(await principalForStoredToken(manager, client)).toBe(userId); + // The row behind the replaced value was deleted, so asserting only "the + // stored token changed" would not have been enough. + expect(await principalFor(manager, before)).toBeNull(); + }, 60_000); + + it('twoFactor.disable — the credential arrives ONLY in the `set-auth-token` header', async () => { + // The route triage singled out: it answers `{ status: true }`, so an + // implementation reading only response BODIES drops it — and this is the + // assertion that says so. + const { engine, manager, client, email } = await signedIn(); + const userId = await userIdFor(engine, email); + const { secret } = await enrolTotp(client); + await client.auth.twoFactor.verifyTotp({ code: totp(secret) }); + const before = String(storedToken(client)); + + const receipt = await client.auth.twoFactor.disable({ password: PASSWORD }); + + // The body really does carry nothing else — pinned, because it is the + // premise of the whole header read. + expect(receipt).toEqual({ status: true }); + expect(storedToken(client)).not.toBe(before); + expect(await principalForStoredToken(manager, client)).toBe(userId); + expect(await principalFor(manager, before)).toBeNull(); + }, 60_000); +}); + +// ─────────────────────────────────────────────────────────────────────────── +describe('#16534 ③ the negative control — a NON-rotating route changes nothing', () => { + it('ordinary reads and writes leave the stored credential byte-identical', async () => { + // ⭐ `updateUser` is the decisive leg. It stages a session cookie to carry + // the updated user WITHOUT rotating the session, so `bearer()`'s + // after-hook emits a `set-auth-token` for it — carrying the SIGNED + // `.` spelling of the session the client already holds. An + // implementation that read that header in the shared `fetch` wrapper + // rather than on the rotating routes rewrites the stored credential + // here, on an ordinary write, with every assertion in ① and ② still + // green. + const { manager, client } = await signedIn(); + const before = String(storedToken(client)); + + await client.auth.me(); + expect(storedToken(client), 'auth.me() moved the stored credential').toBe(before); + + await client.auth.sessions.list(); + expect(storedToken(client), 'sessions.list() moved the stored credential').toBe(before); + + await client.auth.updateUser({ name: 'Renamed User' }); + expect(storedToken(client), 'updateUser() moved the stored credential').toBe(before); + + // Still the same live session at the end of it — the invariant is "did not + // move", not "was emptied". + expect(await principalForStoredToken(manager, client)).not.toBeNull(); + }, 60_000); + + it("verifyBackupCode's already-logged-in lane leaves the stored credential byte-identical", async () => { + // `/two-factor/verify-backup-code` shares `AuthTwoFactorVerificationResult` + // with `verifyTotp` — the same declared `token` member — and does NOT + // rotate: the vendor's `verifyTwoFactor` echoes the session it resolved at + // entry. "Store the token from every result of this type" is therefore the + // most available wrong move, and this is the assertion that refuses it. + // + // The client here deliberately holds the SIGNED credential better-auth + // handed out in `set-auth-token` — the spelling the bearer plugin tells + // clients to store, and the one this SDK itself ends up on after + // `twoFactor.disable`. Against the UNSIGNED echo this route answers, the + // two are different bytes, so a store here is visible rather than a + // coincidental no-op. + const { engine, manager, client, email } = await signedIn(); + const userId = await userIdFor(engine, email); + const { secret, backupCodes } = await enrolTotp(client); + await client.auth.twoFactor.verifyTotp({ code: totp(secret) }); + expect(backupCodes.length, 'enable minted no backup codes').toBeGreaterThan(0); + + const signed = await signedCredentialFor(manager, String(storedToken(client))); + const bearerClient = newClient(manager, signed); + expect(await principalForStoredToken(manager, bearerClient)).toBe(userId); + + const result = await bearerClient.auth.twoFactor.verifyBackupCode({ code: backupCodes[0] }); + + // The premise of this leg: the echo really is a different string from what + // the client is holding. Without it the equality below could pass for the + // wrong reason. + expect(result.token).not.toBe(signed); + expect(storedToken(bearerClient), 'verifyBackupCode moved the stored credential').toBe(signed); + expect(await principalForStoredToken(manager, bearerClient)).toBe(userId); + }, 60_000); +}); diff --git a/packages/client/src/client-url-conformance.test.ts b/packages/client/src/client-url-conformance.test.ts index a8c4144878..1e21171a8d 100644 --- a/packages/client/src/client-url-conformance.test.ts +++ b/packages/client/src/client-url-conformance.test.ts @@ -277,6 +277,11 @@ const NON_HTTP: Record = { 'getRoute': 'pure route-table lookup', 'unwrapResponse': 'pure envelope unwrap', 'isFilterAST': 'pure type predicate', + // [#16534] Local credential state: it writes `this.token` from a value the + // three rotating auth routes have ALREADY received, and issues nothing of its + // own. Those three routes are swept on their own rows, so parking this helper + // here drops no call out of coverage. + 'adoptRotatedSessionToken': 'local state', 'environment': 'constructs a ScopedEnvironmentClient; its methods are swept separately', 'setProjectId': 'local state', 'getProjectId': 'local state', diff --git a/packages/client/src/index.ts b/packages/client/src/index.ts index a69454eac1..1be591b657 100644 --- a/packages/client/src/index.ts +++ b/packages/client/src/index.ts @@ -1113,8 +1113,12 @@ export interface AuthPasswordChangeResult { * ⚠️ SECRET — an unsigned session token. When `revokeOtherSessions: true` * made the server rotate the caller's session this is the NEW session's * token (every other session is gone and the cookie the caller held is - * dead); `null` otherwise. A bearer-mode caller has to store it itself — - * this SDK does not. + * dead); `null` otherwise. + * + * A bearer-mode caller no longer has to store it by hand: `changePassword` + * adopts a non-null value into the client's own credential before it + * resolves, the way `login()` adopts the token it is handed. The field is + * unchanged and still echoed, for a caller that keeps its own store. */ token: string | null; /** The caller, as better-auth's session held it when the write ran. */ @@ -1153,6 +1157,11 @@ export interface AuthTwoFactorVerificationResult { * `two-factor-rotated-token-echo` repairs the vendor's stale echo). * Through this SDK `verifyBackupCode` cannot send `disableSession`, so * the token is always present. + * + * `verifyTotp` adopts it into the client's own credential; `verifyBackupCode` + * does NOT, and the asymmetry is the wire fact rather than an omission — + * `/two-factor/verify-backup-code` never rotates, so what it echoes is the + * session the caller is already presenting. */ token: string; /** @@ -1438,6 +1447,18 @@ const DEFAULT_DATA_PREFIX = '/data'; */ const DEFAULT_META_PREFIX = '/meta'; +/** + * The response header better-auth's `bearer()` plugin puts a freshly installed + * session token in — the SIGNED `.` form, emitted on every response + * that stages a session cookie, and added to `Access-Control-Expose-Headers` by + * the plugin itself so a cross-origin caller can read it. + * + * Read on exactly one route (`twoFactor.disable`), for the reason + * {@link ObjectStackClient.adoptRotatedSessionToken} states. Not exported: it + * names a vendor wire detail, not a capability this SDK offers. + */ +const SET_AUTH_TOKEN_HEADER = 'set-auth-token'; + export class ObjectStackClient { private baseUrl: string; private token?: string; @@ -4205,8 +4226,10 @@ export class ObjectStackClient { * better-auth: POST /change-password. * Set `revokeOtherSessions: true` to invalidate every other session * after the change — the server then ROTATES the caller's session too and - * answers the new token in `token`; this SDK does not store it, so a - * bearer-mode caller must. + * answers the new token in `token`, and this SDK ADOPTS it (#16534), so a + * bearer-mode caller stays signed in across the change. Without + * `revokeOtherSessions` nothing rotates, the field is `null`, and the + * stored credential is left exactly as it was. */ changePassword: async (req: { currentPassword: string; @@ -4218,7 +4241,9 @@ export class ObjectStackClient { method: 'POST', body: JSON.stringify(req), }); - return res.json(); + const result = (await res.json()) as AuthPasswordChangeResult; + this.adoptRotatedSessionToken(result?.token); + return result; }, /** @@ -4414,8 +4439,11 @@ export class ObjectStackClient { * this browser for the configured trust period. * * On the enrolment lane the server rotates the session and answers the - * LIVE token in `token`; this SDK does not store it — a bearer-mode - * caller must, or its next call answers 401. + * LIVE token in `token`; this SDK ADOPTS it (#16534), so a bearer-mode + * caller stays signed in through enrolment instead of meeting a 401 on + * its next call. On the sign-in-challenge lane the same field carries + * the session the challenge just completed, and adopting it is how the + * SDK finishes signing in. */ verifyTotp: async (req: { code: string; trustDevice?: boolean }): Promise => { const route = this.getRoute('auth'); @@ -4423,16 +4451,21 @@ export class ObjectStackClient { method: 'POST', body: JSON.stringify(req), }); - return res.json(); + const result = (await res.json()) as AuthTwoFactorVerificationResult; + this.adoptRotatedSessionToken(result?.token); + return result; }, /** * Disable 2FA for the current user. Requires the password again. * * ⚠️ The server ROTATES the caller's session on success and echoes only - * the receipt (the new token rides the `Set-Cookie` and the bearer - * plugin's `set-auth-token` header, neither of which this SDK reads), so - * a bearer-mode caller's stored token is dead after this call. + * the receipt — the new token rides the `Set-Cookie` and the bearer + * plugin's `set-auth-token` header. This SDK READS that header (#16534) + * and adopts the rotated session, which is the only route in the family + * where the credential is not in the body at all. A cookie-only + * deployment sends no such header; there is then nothing to adopt and + * the stored credential is left as it was. */ disable: async (req: { password: string }): Promise => { const route = this.getRoute('auth'); @@ -4440,6 +4473,7 @@ export class ObjectStackClient { method: 'POST', body: JSON.stringify(req), }); + this.adoptRotatedSessionToken(res.headers.get(SET_AUTH_TOKEN_HEADER)); return res.json(); }, @@ -6674,6 +6708,41 @@ export class ObjectStackClient { return body as T; } + /** + * Adopt a session token the server rotated this client onto mid-request. + * + * Three better-auth routes ROTATE the caller's session on success: they mint + * a new session, install it in `Set-Cookie` (and, through `bearer()`, in the + * `set-auth-token` response header), and DELETE the row the caller was + * presenting — `changePassword({ revokeOtherSessions: true })`, the enrolment + * lane of `twoFactor.verifyTotp`, and `twoFactor.disable`. A browser carries + * the cookie across on its own; a bearer caller — this SDK's own mode — kept + * presenting the DELETED session's token, so its very next call answered + * `401 UNAUTHORIZED` (#16534). + * + * ⚠️ Called from those three routes ONLY, never from the shared `fetch` + * wrapper, and the narrowness is the design rather than an implementation + * detail. `set-auth-token` rides EVERY response that stages a session cookie, + * rotation or not — `POST /update-user` stages one to carry the updated user + * — and it carries the SIGNED `.` spelling while every JSON + * `token` echo carries the UNSIGNED one. A wrapper-level read would therefore + * rewrite `this.token` into a different spelling of the SAME session on + * ordinary traffic: a stored credential that churns on writes that rotated + * nothing. Storing only where the server actually rotated keeps the stored + * value equal to the credential the caller was last granted. + * + * For the same reason `verifyBackupCode` does not call this: its lane never + * rotates, so its `token` echo is the session the caller already holds. + * + * `login()` / `register()` / `refreshToken()` keep their own assignments: + * those read a normalized `{ data: { token } }` envelope this SDK builds, and + * they establish a session rather than follow a rotation. + */ + private adoptRotatedSessionToken(token: string | null | undefined): void { + if (typeof token !== 'string' || token.length === 0) return; + this.token = token; + } + private async fetch(url: string, options: RequestInit = {}): Promise { this.logger.debug('HTTP request', { method: options.method || 'GET', diff --git a/packages/client/tsconfig.json b/packages/client/tsconfig.json index b8cf584353..26477822d1 100644 --- a/packages/client/tsconfig.json +++ b/packages/client/tsconfig.json @@ -36,7 +36,24 @@ // the last `pnpm build` of three other packages. Each publishes a single // `"."` entry point, so one bare-name rule each — no star, per the // paragraph above. + // [#16534] Same rule, same reason, for the two producers + // `src/auth-rotated-session-token.test.ts` drives: `@objectstack/plugin-auth` + // (the real `AuthManager`, whose three routes rotate the caller's session) + // and the identity object definitions that pipeline stores its rows in. + // That suite's claim is "the SDK keeps the session the SERVER just handed + // it", so its verdict has to be about the server's SOURCE; through + // `exports` it would be about the last `pnpm build` of two other packages, + // which `check:type-source-resolution` reports as a NEW dist-resolved type + // import. + // + // `@objectstack/platform-objects` is the one entry here spelled as a + // SUBPATH: that package publishes eleven of them and this suite imports one + // (`./identity`), so the rule names that subpath exactly. Still no star — + // per the paragraph above, a bare-name star would fold all eleven onto a + // single target and type-check green against the wrong module. "paths": { + "@objectstack/plugin-auth": ["../plugins/plugin-auth/src/index.ts"], + "@objectstack/platform-objects/identity": ["../platform-objects/src/identity/index.ts"], "@objectstack/metadata-core": ["../metadata-core/src/index.ts"], "@objectstack/metadata-protocol": ["../metadata-protocol/src/index.ts"], "@objectstack/rest": ["../rest/src/index.ts"], diff --git a/packages/client/vitest.config.ts b/packages/client/vitest.config.ts index b6aea0964b..bb022d8571 100644 --- a/packages/client/vitest.config.ts +++ b/packages/client/vitest.config.ts @@ -78,6 +78,23 @@ export default defineConfig({ find: /^@objectstack\/service-automation$/, replacement: path.resolve(__dirname, '../services/service-automation/src/index.ts'), }, + // [#16534] `auth-rotated-session-token.test.ts` drives the SDK's + // credential bookkeeping against the REAL better-auth pipeline, so it + // takes VALUE imports on the server that rotates the session + // (`AuthManager`) and on the identity object definitions that pipeline + // stores its rows in. Same reason as every entry above — a unit pin is a + // verdict about the SOURCE in this checkout, and `check:test-source-alias` + // dictates exactly this remedy because its `KNOWN_UNALIASED_TEST_IMPORTS` + // registry is ⛔ SHRINK-ONLY. Anchored (`^…$`, array form) so neither + // entry can swallow a subpath specifier and resolve it THROUGH a file. + { + find: /^@objectstack\/plugin-auth$/, + replacement: path.resolve(__dirname, '../plugins/plugin-auth/src/index.ts'), + }, + { + find: /^@objectstack\/platform-objects\/identity$/, + replacement: path.resolve(__dirname, '../platform-objects/src/identity/index.ts'), + }, ], }, }); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index c5fee5e654..b98543fd58 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -665,6 +665,12 @@ importers: '@objectstack/objectql': specifier: workspace:* version: link:../objectql + '@objectstack/platform-objects': + specifier: workspace:* + version: link:../platform-objects + '@objectstack/plugin-auth': + specifier: workspace:* + version: link:../plugins/plugin-auth '@objectstack/plugin-hono-server': specifier: workspace:* version: link:../plugins/plugin-hono-server