|
| 1 | +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | +// |
| 3 | +// #17234 — `auth.login` and `auth.register` annotate their return as |
| 4 | +// `SessionResponse` (`BaseResponseSchema.extend(…)`, so `success` is a REQUIRED |
| 5 | +// boolean) and delivered a body that never carried `success` at all. Two |
| 6 | +// departures were measured on the card; this suite closes one and PINS the |
| 7 | +// other as a measurement rather than letting it be invented away. |
| 8 | +// |
| 9 | +// ## Why the server here is the real one |
| 10 | +// |
| 11 | +// Every claim below is a claim about BYTES BETTER-AUTH WRITES — whether |
| 12 | +// `/sign-in/email` and `/sign-up/email` carry a session anywhere, and which |
| 13 | +// spelling of the session credential reaches `data.token`. A hand-written |
| 14 | +// double would let this suite certify the SDK against a body this file |
| 15 | +// invented, which is exactly how the declared shape and the real one drifted |
| 16 | +// apart in the first place. So the arrangement is a real `AuthManager` |
| 17 | +// (better-auth 1.7.3, organization plugin on by its own default) over a real |
| 18 | +// `ObjectQL` on a real `SqliteWasmDriver`, with an `ObjectStackClient` whose |
| 19 | +// `fetch` hands the `Request` straight to `AuthManager.handleRequest`: |
| 20 | +// everything above that call is the SDK's real request path, everything below |
| 21 | +// it is better-auth's real pipeline. The client's `fetch` also keeps a CLONE of |
| 22 | +// each `Response`, so the wire bytes and the SDK's return value come from ONE |
| 23 | +// call rather than from two that could disagree. |
| 24 | +// |
| 25 | +// ## What each block is for |
| 26 | +// |
| 27 | +// - `① the declared envelope is delivered` — the defect proper. The judge is a |
| 28 | +// PARSE against the declaration, not a key spot-check. |
| 29 | +// - `② the residue is exhaustive` — `SessionResponseSchema` still does not |
| 30 | +// parse, for two reasons that are NOT this card's `success`. Pinned as the |
| 31 | +// complete issue list so a regression on `success` shows up here as an extra |
| 32 | +// issue instead of hiding inside "it already failed". |
| 33 | +// - `③ the instrument can still fail` — the negative control. The same parse, |
| 34 | +// on the same returned value with `success` taken back out, must report |
| 35 | +// `success` again. Without it, a green ① could equally mean the assertion |
| 36 | +// broke. |
| 37 | +// - `④ the credential survives byte-identical` — the regression this fix could |
| 38 | +// most easily have caused. `data.token` is the body's own token, and |
| 39 | +// `client.token` is still armed from it. |
| 40 | +// - `⑤ data.session is not obtainable on these routes` — the card's second |
| 41 | +// departure, left OPEN deliberately. This block is the measurement that says |
| 42 | +// why: no session in the body, none in the headers, and the value only |
| 43 | +// appears on a SECOND call. If better-auth ever starts serving one, this |
| 44 | +// block reddens and #17234 can be closed properly. |
| 45 | +// - `⑥ the raw keys survive the lift` — callers were pushed onto `.user` / |
| 46 | +// `.token` by the very misdeclaration this card fixes. |
| 47 | + |
| 48 | +import { describe, it, expect, afterEach } from 'vitest'; |
| 49 | +import { ObjectQL } from '@objectstack/objectql'; |
| 50 | +import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm'; |
| 51 | +import { AuthManager } from '@objectstack/plugin-auth'; |
| 52 | +import * as identityObjects from '@objectstack/platform-objects/identity'; |
| 53 | +import { BaseResponseSchema, SessionResponseSchema, SessionSchema } from '@objectstack/spec/api'; |
| 54 | +import { ObjectStackClient } from './index'; |
| 55 | + |
| 56 | +const SECRET = 'test-secret-at-least-32-chars-long!!'; |
| 57 | +const ORIGIN = 'http://localhost:3000'; |
| 58 | +const PASSWORD = 'S3cure!Passw0rd-17234'; |
| 59 | + |
| 60 | +/** |
| 61 | + * The identity objects better-auth's ObjectQL adapter reads and writes on the |
| 62 | + * routes under test. Read out of `@objectstack/platform-objects/identity` BY |
| 63 | + * SHAPE rather than transcribed, for the reason |
| 64 | + * `auth-get-session-envelope.test.ts` states: a hand-copied list is a second |
| 65 | + * declaration of the same set, drifting silently the day the plugin registers |
| 66 | + * one more. |
| 67 | + */ |
| 68 | +const IDENTITY_OBJECTS = Object.values( |
| 69 | + identityObjects as unknown as Record<string, unknown>, |
| 70 | +).filter( |
| 71 | + (o): o is Record<string, unknown> => |
| 72 | + !!o && |
| 73 | + typeof o === 'object' && |
| 74 | + typeof (o as { name?: unknown }).name === 'string' && |
| 75 | + typeof (o as { fields?: unknown }).fields === 'object', |
| 76 | +); |
| 77 | + |
| 78 | +const engines: ObjectQL[] = []; |
| 79 | + |
| 80 | +const makeEngine = async (): Promise<ObjectQL> => { |
| 81 | + const engine = new ObjectQL(); |
| 82 | + engines.push(engine); |
| 83 | + engine.registerDriver(new SqliteWasmDriver({ filename: ':memory:' }) as never, true); |
| 84 | + await engine.init(); |
| 85 | + for (const object of IDENTITY_OBJECTS) { |
| 86 | + engine.registry.registerObject(object as never, '@objectstack/plugin-auth'); |
| 87 | + } |
| 88 | + await engine.syncSchemas(); |
| 89 | + return engine; |
| 90 | +}; |
| 91 | + |
| 92 | +let emailSeq = 0; |
| 93 | + |
| 94 | +/** |
| 95 | + * A real manager plus a client factory that records the wire `Response` of |
| 96 | + * every call it makes. |
| 97 | + * |
| 98 | + * ⚠️ One manager per scenario, deliberately: the FIRST sign-up on a fresh |
| 99 | + * environment provisions the owner and the audience posture then closes |
| 100 | + * self-registration (`SELF_REGISTRATION_CLOSED`), so a second `register()` on |
| 101 | + * the same manager is refused before it ever reaches the code under test. |
| 102 | + */ |
| 103 | +const scenario = async () => { |
| 104 | + const engine = await makeEngine(); |
| 105 | + const manager = new AuthManager({ |
| 106 | + secret: SECRET, |
| 107 | + baseUrl: ORIGIN, |
| 108 | + dataEngine: engine, |
| 109 | + } as never); |
| 110 | + const wire: Response[] = []; |
| 111 | + const mkClient = (): ObjectStackClient => |
| 112 | + new ObjectStackClient({ |
| 113 | + baseUrl: ORIGIN, |
| 114 | + fetch: async (input: RequestInfo | URL, init?: RequestInit) => { |
| 115 | + const res = await manager.handleRequest(new Request(String(input), init)); |
| 116 | + // Clone BEFORE the SDK reads it, so both halves of every assertion |
| 117 | + // below come from the same single call. |
| 118 | + wire.push(res.clone()); |
| 119 | + return res; |
| 120 | + }, |
| 121 | + }); |
| 122 | + return { |
| 123 | + manager, |
| 124 | + mkClient, |
| 125 | + email: `envelope-17234-${++emailSeq}-${Date.now()}@example.com`, |
| 126 | + /** The `Response` of the most recent call this scenario's clients made. */ |
| 127 | + lastWire: () => wire[wire.length - 1], |
| 128 | + }; |
| 129 | +}; |
| 130 | + |
| 131 | +/** The credential the client is holding right now. */ |
| 132 | +const storedToken = (client: ObjectStackClient): string | undefined => |
| 133 | + (client as unknown as { token?: string }).token; |
| 134 | + |
| 135 | +/** The raw wire keys the lift keeps alongside `data`. */ |
| 136 | +type RawKeys = { |
| 137 | + success?: unknown; |
| 138 | + redirect?: unknown; |
| 139 | + token?: unknown; |
| 140 | + user?: { id?: string }; |
| 141 | + data?: { token?: unknown; session?: unknown; user?: { id?: string } }; |
| 142 | +}; |
| 143 | + |
| 144 | +/** A registered first user, and both halves of that one call. */ |
| 145 | +const registered = async () => { |
| 146 | + const s = await scenario(); |
| 147 | + const client = s.mkClient(); |
| 148 | + const res = await client.auth.register({ |
| 149 | + email: s.email, |
| 150 | + password: PASSWORD, |
| 151 | + name: 'Envelope 17234', |
| 152 | + }); |
| 153 | + const wire = s.lastWire(); |
| 154 | + return { ...s, client, res, wireRes: wire, wireBody: (await wire.json()) as RawKeys }; |
| 155 | +}; |
| 156 | + |
| 157 | +/** …and a SECOND client that signed that user back in. */ |
| 158 | +const signedIn = async () => { |
| 159 | + const base = await registered(); |
| 160 | + const client = base.mkClient(); |
| 161 | + const res = await client.auth.login({ email: base.email, password: PASSWORD }); |
| 162 | + const wire = base.lastWire(); |
| 163 | + return { ...base, client, res, wireRes: wire, wireBody: (await wire.json()) as RawKeys }; |
| 164 | +}; |
| 165 | + |
| 166 | +/** WHO a credential resolves to, asked through better-auth's own API. */ |
| 167 | +const principalFor = async (manager: AuthManager, token: string | undefined) => { |
| 168 | + const auth = (await manager.getAuthInstance()) as unknown as { |
| 169 | + api: { getSession(a: { headers: Headers }): Promise<unknown> }; |
| 170 | + }; |
| 171 | + const session = (await auth.api |
| 172 | + .getSession({ headers: new Headers({ authorization: `Bearer ${token}` }) }) |
| 173 | + .catch(() => null)) as { user?: { id?: string } } | null; |
| 174 | + return session?.user?.id ?? null; |
| 175 | +}; |
| 176 | + |
| 177 | +afterEach(async () => { |
| 178 | + while (engines.length) { |
| 179 | + const engine = engines.pop(); |
| 180 | + await (engine as unknown as { close?: () => Promise<void> })?.close?.().catch(() => {}); |
| 181 | + } |
| 182 | +}); |
| 183 | + |
| 184 | +describe('[#17234] auth.login / auth.register deliver the SessionResponse envelope they declare', () => { |
| 185 | + describe('① the declared envelope is delivered', () => { |
| 186 | + it('register() parses as the declared envelope, with the payload under `data`', async () => { |
| 187 | + const { res } = await registered(); |
| 188 | + |
| 189 | + // The decisive assertion: the DECLARATION judges the body. On the defect |
| 190 | + // the method returned `{ …raw, data }` with no `success` at all, so this |
| 191 | + // parse was red. |
| 192 | + const envelope = BaseResponseSchema.safeParse(res); |
| 193 | + expect( |
| 194 | + envelope.success, |
| 195 | + `register() did not parse as the declared envelope: ${JSON.stringify(envelope.error?.issues)}`, |
| 196 | + ).toBe(true); |
| 197 | + expect(res.success).toBe(true); |
| 198 | + |
| 199 | + expect(res.data).toBeTruthy(); |
| 200 | + expect(typeof res.data.user?.id).toBe('string'); |
| 201 | + expect(res.data.user?.email).toContain('@'); |
| 202 | + }); |
| 203 | + |
| 204 | + it('login() parses as the declared envelope, with the payload under `data`', async () => { |
| 205 | + const { res } = await signedIn(); |
| 206 | + |
| 207 | + const envelope = BaseResponseSchema.safeParse(res); |
| 208 | + expect( |
| 209 | + envelope.success, |
| 210 | + `login() did not parse as the declared envelope: ${JSON.stringify(envelope.error?.issues)}`, |
| 211 | + ).toBe(true); |
| 212 | + expect(res.success).toBe(true); |
| 213 | + |
| 214 | + expect(res.data).toBeTruthy(); |
| 215 | + expect(typeof res.data.user?.id).toBe('string'); |
| 216 | + expect(res.data.user?.email).toContain('@'); |
| 217 | + }); |
| 218 | + }); |
| 219 | + |
| 220 | + describe('② the residue is exhaustive, and `success` is not in it', () => { |
| 221 | + // Two issues remain on the FULL declared type, and neither is this card's: |
| 222 | + // |
| 223 | + // data.session — block ⑤: these routes serve none. #17234 stays open. |
| 224 | + // data.user.image — `SessionUserSchema.image` is `z.string().optional()`, |
| 225 | + // which does not admit `null`, and better-auth serves |
| 226 | + // `"image": null` for a user who never set one. Filed |
| 227 | + // as #17235, and NOT specific to these two methods. |
| 228 | + // |
| 229 | + // Pinned as the EXHAUSTIVE list rather than as "it still fails": if |
| 230 | + // `success` ever regresses it reappears here as a third issue and these |
| 231 | + // cases redden. It is the residue's tripwire, not an acceptance of it. |
| 232 | + const RESIDUE = ['data.session', 'data.user.image']; |
| 233 | + |
| 234 | + it('register() reports exactly the two issues that are not `success`', async () => { |
| 235 | + const { res } = await registered(); |
| 236 | + const issues = SessionResponseSchema.safeParse(res).error?.issues ?? []; |
| 237 | + expect(issues.map((i) => i.path.join('.'))).toEqual(RESIDUE); |
| 238 | + }); |
| 239 | + |
| 240 | + it('login() reports exactly the two issues that are not `success`', async () => { |
| 241 | + const { res } = await signedIn(); |
| 242 | + const issues = SessionResponseSchema.safeParse(res).error?.issues ?? []; |
| 243 | + expect(issues.map((i) => i.path.join('.'))).toEqual(RESIDUE); |
| 244 | + }); |
| 245 | + }); |
| 246 | + |
| 247 | + describe('③ the instrument can still fail — negative control', () => { |
| 248 | + it('the same parse reports `success` again once it is taken back out', async () => { |
| 249 | + const { res } = await signedIn(); |
| 250 | + |
| 251 | + // Not a fabricated body: the value the method REALLY returned, with the |
| 252 | + // one member this card added removed again. So a green ① cannot be a |
| 253 | + // broken assertion or a schema that stopped checking — this is the same |
| 254 | + // schema, the same call, the same instrument, reporting the defect. |
| 255 | + const { success: _dropped, ...withoutSuccess } = res as unknown as Record<string, unknown> & { |
| 256 | + success?: unknown; |
| 257 | + }; |
| 258 | + const issues = SessionResponseSchema.safeParse(withoutSuccess).error?.issues ?? []; |
| 259 | + expect(issues.map((i) => i.path.join('.'))).toEqual([ |
| 260 | + 'success', |
| 261 | + 'data.session', |
| 262 | + 'data.user.image', |
| 263 | + ]); |
| 264 | + }); |
| 265 | + }); |
| 266 | + |
| 267 | + describe('④ the credential survives the lift byte-identical', () => { |
| 268 | + it('login() keeps the body token at `data.token` and still arms `client.token`', async () => { |
| 269 | + const { manager, client, res, wireBody } = await signedIn(); |
| 270 | + |
| 271 | + // Byte-identity against the WIRE, on the same call — not against a |
| 272 | + // remembered constant. Routing these methods through the shared lift is |
| 273 | + // exactly the change that could have dropped this member (the lift used |
| 274 | + // to build `data: { user, session }` and nothing else), and dropping it |
| 275 | + // would silently stop the auto-set below on the SDK's most-used path. |
| 276 | + expect(typeof wireBody.token).toBe('string'); |
| 277 | + expect(res.data.token).toBe(wireBody.token); |
| 278 | + |
| 279 | + // The auto-set still happens, and it is the same string. |
| 280 | + expect(storedToken(client)).toBe(wireBody.token); |
| 281 | + expect(storedToken(client)).toBe(res.data.token); |
| 282 | + |
| 283 | + // …and it is a WORKING credential, not merely a non-empty string. |
| 284 | + expect(await principalFor(manager, storedToken(client))).toBe(res.data.user?.id); |
| 285 | + // The control that must NOT resolve, so "resolves to the user" is a real |
| 286 | + // reading and not something this arrangement answers for any input. |
| 287 | + expect(await principalFor(manager, 'not-the-session-token-17234')).toBeNull(); |
| 288 | + }); |
| 289 | + |
| 290 | + it('register() keeps the body token at `data.token` and still arms `client.token`', async () => { |
| 291 | + const { client, res, wireBody } = await registered(); |
| 292 | + expect(typeof wireBody.token).toBe('string'); |
| 293 | + expect(res.data.token).toBe(wireBody.token); |
| 294 | + expect(storedToken(client)).toBe(wireBody.token); |
| 295 | + }); |
| 296 | + |
| 297 | + it('the token at `data.token` is the UNSIGNED spelling the body carries', async () => { |
| 298 | + const { res, wireRes, wireBody } = await signedIn(); |
| 299 | + |
| 300 | + // The two spellings of one session credential, measured on ONE response: |
| 301 | + // the body's `token` is UNSIGNED, and `bearer()` publishes the SIGNED |
| 302 | + // `token.signature` form in the `set-auth-token` HEADER. The split is by |
| 303 | + // CARRIER, not by method — which is what makes "never synthesize |
| 304 | + // `data.token` from `session.token`" a rule about inventing a member on a |
| 305 | + // route that served none, rather than about two different credentials. |
| 306 | + const signed = wireRes.headers.get('set-auth-token') ?? ''; |
| 307 | + expect(signed, 'sign-in emitted no set-auth-token — this control cannot fire').toBeTruthy(); |
| 308 | + expect(signed).not.toBe(wireBody.token); |
| 309 | + expect(signed.startsWith(`${String(wireBody.token)}.`)).toBe(true); |
| 310 | + // `data.token` is the body half, untouched. |
| 311 | + expect(res.data.token).toBe(wireBody.token); |
| 312 | + }); |
| 313 | + }); |
| 314 | + |
| 315 | + describe('⑤ `data.session` is not obtainable on these routes — the open half of #17234', () => { |
| 316 | + it('neither route carries a session in its body', async () => { |
| 317 | + const reg = await registered(); |
| 318 | + expect(Object.keys(reg.wireBody as object)).toEqual(['token', 'user']); |
| 319 | + expect(reg.res.data.session).toBeUndefined(); |
| 320 | + |
| 321 | + const log = await signedIn(); |
| 322 | + expect(Object.keys(log.wireBody as object)).toEqual(['redirect', 'token', 'user']); |
| 323 | + expect(log.res.data.session).toBeUndefined(); |
| 324 | + }); |
| 325 | + |
| 326 | + it('nor in any response header — the only credential carrier is a bare token', async () => { |
| 327 | + const { wireRes } = await signedIn(); |
| 328 | + const names = [...wireRes.headers.keys()]; |
| 329 | + // Nothing header-side is named for a session payload… |
| 330 | + expect(names.filter((n) => /session/i.test(n))).toEqual([]); |
| 331 | + // …and the one header that does carry a credential carries a STRING, not |
| 332 | + // a session object: no `id`, no `expiresAt`, nothing `SessionSchema` |
| 333 | + // would accept. So deriving `data.session` from the headers is not an |
| 334 | + // option that was overlooked. |
| 335 | + const signed = wireRes.headers.get('set-auth-token') ?? ''; |
| 336 | + expect(signed).toBeTruthy(); |
| 337 | + expect(signed.trimStart().startsWith('{')).toBe(false); |
| 338 | + expect(SessionSchema.safeParse(signed).success).toBe(false); |
| 339 | + }); |
| 340 | + |
| 341 | + it('the session exists only one NETWORK CALL later, via /get-session', async () => { |
| 342 | + const { client, res } = await signedIn(); |
| 343 | + |
| 344 | + // The positive leg, and the whole reason this card stays open: the value |
| 345 | + // the declared type names is real and reachable — just not on this route. |
| 346 | + // Satisfying `data.session` here would mean either a second round trip |
| 347 | + // inside `login()` (a behaviour change no ruling has authorised) or a |
| 348 | + // fabricated id and expiry (forbidden outright). |
| 349 | + const me = await client.auth.me(); |
| 350 | + const session = SessionSchema.safeParse(me.data.session); |
| 351 | + expect( |
| 352 | + session.success, |
| 353 | + `/get-session did not serve a parseable session: ${JSON.stringify(session.error?.issues)}`, |
| 354 | + ).toBe(true); |
| 355 | + expect(me.data.session?.userId).toBe(res.data.user?.id); |
| 356 | + // …and it really is absent from the sign-in answer, so the two readings |
| 357 | + // above are about one session and not two arrangements. |
| 358 | + expect(res.data.session).toBeUndefined(); |
| 359 | + }); |
| 360 | + }); |
| 361 | + |
| 362 | + describe('⑥ the raw keys survive the lift', () => { |
| 363 | + it('keeps `.user` / `.token` — and login keeps `.redirect` — alongside `data`', async () => { |
| 364 | + const { res } = await signedIn(); |
| 365 | + const raw = res as unknown as RawKeys; |
| 366 | + // Callers were pushed onto the raw keys by the very misdeclaration this |
| 367 | + // card fixes. Buying the declared shape by breaking them would trade one |
| 368 | + // silent breakage for another. |
| 369 | + expect(raw.user?.id).toBe(res.data.user?.id); |
| 370 | + expect(raw.token).toBe(res.data.token); |
| 371 | + expect(raw.redirect).toBe(false); |
| 372 | + }); |
| 373 | + }); |
| 374 | +}); |
0 commit comments