|
| 1 | +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | + |
| 3 | +/** |
| 4 | + * [#8049] `/auth/change-password` must behave IDENTICALLY on every transport. |
| 5 | + * |
| 6 | + * ## The defect this pins |
| 7 | + * |
| 8 | + * An admin-provisioned user (`mustChangePassword` defaults to true) is gated out |
| 9 | + * of every protected route with `403 PASSWORD_EXPIRED` until they rotate their |
| 10 | + * password. On the COOKIE lane the escape hatch worked. On the BEARER lane — |
| 11 | + * the documented API/agent/CLI lane — `/auth/change-password` answered **200**, |
| 12 | + * the password really rotated, and nothing else happened: `must_change_password` |
| 13 | + * stayed `true`, `password_changed_at` stayed `null`, and the caller stayed |
| 14 | + * locked out of every protected route by a success response. |
| 15 | + * |
| 16 | + * Measured on `origin/main` before the fix, all three of these were true at once |
| 17 | + * on the bearer lane and false on the cookie lane — which is the whole point: |
| 18 | + * |
| 19 | + * must_change_password true password_changed_at null |
| 20 | + * protected read 403 previous_password_hashes null |
| 21 | + * reusing the previous password ACCEPTED (200) |
| 22 | + * |
| 23 | + * ## Why it is `security`, not just a lockout |
| 24 | + * |
| 25 | + * That last line is the half that is easy to under-fix. ONE stash — |
| 26 | + * `ctx.context.__osPwChangeUserId`, set by the before-hook when it resolves the |
| 27 | + * acting user — gates all three behaviours: the `password_changed_at` / |
| 28 | + * `must_change_password` stamp, ADR-0069 D1's password-reuse REJECTION, and the |
| 29 | + * history append. Unresolved principal ⇒ none of them run. So the bearer lane |
| 30 | + * did not merely stay flagged; password history was **neither checked nor |
| 31 | + * recorded** there. A control enforced on one transport and silently absent on |
| 32 | + * the other is worse than one absent on both, because the console and every |
| 33 | + * pre-existing pin exercise the working lane. |
| 34 | + * |
| 35 | + * Hence this file asserts the SAME post-conditions on every lane rather than |
| 36 | + * asserting the bug's absence on one. A fix that cleared the flags but left the |
| 37 | + * reuse control transport-dependent passes a lockout test and fails this one. |
| 38 | + * |
| 39 | + * ## Root cause, for whoever changes the resolver next |
| 40 | + * |
| 41 | + * better-auth's `getHooks` (`api/dispatch.mjs`) pushes `options.hooks.before` |
| 42 | + * — the auth manager's global before-hook — ahead of every PLUGIN before-hook, |
| 43 | + * and `bearer()`'s before-hook is what rewrites `Authorization: Bearer` into a |
| 44 | + * session cookie. A bare `getSessionFromCtx(ctx)` in our hook therefore reads a |
| 45 | + * cookie that does not exist yet on the bearer lane and resolves null, while |
| 46 | + * better-auth's own password write — which runs after the conversion — succeeds. |
| 47 | + * That is the 200-with-nothing-stamped. The resolver now goes through the shared |
| 48 | + * hook-order-independent `resolveActor`, which falls back to explicit token |
| 49 | + * lookup. |
| 50 | + * |
| 51 | + * ## Why BOTH bearer spellings are driven |
| 52 | + * |
| 53 | + * `bearer()` hands clients the SIGNED `<token>.<sig>` in the `set-auth-token` |
| 54 | + * response header (this is what the issue's reproduction used) and accepts both |
| 55 | + * that and the raw `token` from the sign-in body. `sys_session.token` stores the |
| 56 | + * UNSIGNED value. A resolver that looked the credential up verbatim would work |
| 57 | + * for one spelling and silently resolve nothing for the other — the same |
| 58 | + * per-transport asymmetry one level down. Driving both is what keeps that |
| 59 | + * closed; drop the signed lane and half the fix can be reverted invisibly. |
| 60 | + * |
| 61 | + * Harness notes: |
| 62 | + * - `/auth/admin/create-user` 501s unless better-auth's `admin` plugin is on, |
| 63 | + * and `bootStack` exposes no auth-plugin override. `OS_SCIM_ENABLED` is the |
| 64 | + * one env knob that reaches it (`buildPluginList` resolves |
| 65 | + * `admin: pluginConfig.admin ?? scimEffective`), so it must precede |
| 66 | + * `bootStack` — same shape as `admin-identity-audit-trail.dogfood.test.ts`. |
| 67 | + * - `passwordHistoryCount` is 0 (off) by default, which would make every reuse |
| 68 | + * assertion below vacuously green. It is set through `applyConfigPatch`, the |
| 69 | + * same seam the settings service writes, so the reuse control is genuinely |
| 70 | + * armed for all lanes. |
| 71 | + * - Two different error envelopes are asserted, deliberately and distinctly: |
| 72 | + * the gate refusal is the ADR-0112 REST envelope (`{error:{code}}`, 403) |
| 73 | + * raised at the transport seam, while the reuse refusal is better-auth's own |
| 74 | + * `APIError` (`{code}`, 400) surfaced through the proxied `/auth/*` route. |
| 75 | + * They are different outcomes and a test that conflated them could not tell |
| 76 | + * "reuse rejected" from "still locked out". |
| 77 | + */ |
| 78 | + |
| 79 | +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; |
| 80 | +import showcaseStack from '@objectstack/example-showcase'; |
| 81 | +import { bootStack, type VerifyStack } from '@objectstack/verify'; |
| 82 | + |
| 83 | +const SYS = { context: { isSystem: true } }; |
| 84 | + |
| 85 | +/** Depth of the ADR-0069 D1 history ring this file arms. */ |
| 86 | +const HISTORY_COUNT = 3; |
| 87 | + |
| 88 | +const FIRST_PASSWORD = 'BearerLane!First1'; |
| 89 | +const SECOND_PASSWORD = 'BearerLane!Second2'; |
| 90 | + |
| 91 | +/** Collect a response's Set-Cookie values into a single request Cookie header. */ |
| 92 | +function cookieHeader(res: Response): string { |
| 93 | + const jar = res.headers.getSetCookie?.() ?? []; |
| 94 | + return jar.map((c) => c.split(';')[0]).join('; '); |
| 95 | +} |
| 96 | + |
| 97 | +/** |
| 98 | + * One authenticated transport. `credential` is whatever the lane carries after |
| 99 | + * a sign-in; `headers` turns it into the request headers that lane would send. |
| 100 | + */ |
| 101 | +interface Lane { |
| 102 | + readonly name: string; |
| 103 | + /** Pick this lane's credential out of a sign-in response. */ |
| 104 | + credential(res: Response, body: { token?: string }): string; |
| 105 | + /** The auth headers a request on this lane carries. */ |
| 106 | + headers(credential: string): Record<string, string>; |
| 107 | +} |
| 108 | + |
| 109 | +const LANES: Lane[] = [ |
| 110 | + { |
| 111 | + name: 'cookie', |
| 112 | + credential: (res) => cookieHeader(res), |
| 113 | + headers: (c) => ({ Cookie: c }), |
| 114 | + }, |
| 115 | + { |
| 116 | + // The credential the issue's reproduction used: the `set-auth-token` |
| 117 | + // response header, which carries the SIGNED `<token>.<sig>` form. |
| 118 | + name: 'bearer (signed set-auth-token)', |
| 119 | + credential: (res) => res.headers.get('set-auth-token') ?? '', |
| 120 | + headers: (c) => ({ Authorization: `Bearer ${c}` }), |
| 121 | + }, |
| 122 | + { |
| 123 | + // The other accepted spelling: the raw session token from the sign-in body. |
| 124 | + name: 'bearer (raw sign-in token)', |
| 125 | + credential: (_res, body) => body.token ?? '', |
| 126 | + headers: (c) => ({ Authorization: `Bearer ${c}` }), |
| 127 | + }, |
| 128 | +]; |
| 129 | + |
| 130 | +describe('#8049: /auth/change-password clears the force-change flag and enforces reuse on EVERY transport', () => { |
| 131 | + let stack: VerifyStack; |
| 132 | + let ql: any; |
| 133 | + let adminToken: string; |
| 134 | + let priorScim: string | undefined; |
| 135 | + |
| 136 | + beforeAll(async () => { |
| 137 | + priorScim = process.env.OS_SCIM_ENABLED; |
| 138 | + process.env.OS_SCIM_ENABLED = 'true'; |
| 139 | + stack = await bootStack(showcaseStack, {}); |
| 140 | + ql = await stack.kernel.getServiceAsync<any>('objectql'); |
| 141 | + |
| 142 | + // Arm ADR-0069 D1's history ring. Default is 0 (off), under which every |
| 143 | + // reuse assertion in this file would pass without testing anything. |
| 144 | + const auth = await stack.kernel.getServiceAsync<any>('auth'); |
| 145 | + auth.applyConfigPatch({ passwordHistoryCount: HISTORY_COUNT }); |
| 146 | + |
| 147 | + adminToken = await stack.signIn(); |
| 148 | + }, 180_000); |
| 149 | + |
| 150 | + afterAll(async () => { |
| 151 | + await stack?.stop?.(); |
| 152 | + if (priorScim === undefined) delete process.env.OS_SCIM_ENABLED; |
| 153 | + else process.env.OS_SCIM_ENABLED = priorScim; |
| 154 | + }); |
| 155 | + |
| 156 | + /** Sign in through the real route and hand back every lane's credential. */ |
| 157 | + async function signIn(email: string, password: string) { |
| 158 | + const res = await stack.api('/auth/sign-in/email', { |
| 159 | + method: 'POST', |
| 160 | + headers: { 'Content-Type': 'application/json' }, |
| 161 | + body: JSON.stringify({ email, password }), |
| 162 | + }); |
| 163 | + const body = res.status === 200 ? ((await res.clone().json()) as { token?: string }) : {}; |
| 164 | + return { res, body }; |
| 165 | + } |
| 166 | + |
| 167 | + /** The `sys_user` row + its credential account, read with system context. */ |
| 168 | + async function identity(email: string) { |
| 169 | + const user = (await ql.find('sys_user', { where: { email }, limit: 1 }, SYS))[0]; |
| 170 | + const account = ( |
| 171 | + await ql.find( |
| 172 | + 'sys_account', |
| 173 | + { where: { user_id: String(user?.id), provider_id: 'credential' }, limit: 1 }, |
| 174 | + SYS, |
| 175 | + ) |
| 176 | + )[0]; |
| 177 | + const raw = account?.previous_password_hashes; |
| 178 | + let history: string[] = []; |
| 179 | + if (typeof raw === 'string' && raw.trim()) { |
| 180 | + try { |
| 181 | + const parsed = JSON.parse(raw); |
| 182 | + if (Array.isArray(parsed)) history = parsed; |
| 183 | + } catch { |
| 184 | + throw new Error(`previous_password_hashes is not JSON: ${raw}`); |
| 185 | + } |
| 186 | + } |
| 187 | + return { user, history }; |
| 188 | + } |
| 189 | + |
| 190 | + for (const lane of LANES) { |
| 191 | + // eslint-disable-next-line vitest/valid-title |
| 192 | + describe(`lane: ${lane.name}`, () => { |
| 193 | + // One provisioned user per lane — the flags are per-user, so sharing one |
| 194 | + // would let an earlier lane's successful change satisfy a later lane's |
| 195 | + // assertions and hide exactly the asymmetry this file exists to catch. |
| 196 | + const email = `bearer.lane.8049.${lane.name.replace(/[^a-z]/gi, '').toLowerCase()}@example.com`; |
| 197 | + let credential = ''; |
| 198 | + |
| 199 | + it('an admin-provisioned user is gated out with 403 PASSWORD_EXPIRED', async () => { |
| 200 | + const created = await stack.apiAs(adminToken, 'POST', '/auth/admin/create-user', { |
| 201 | + email, |
| 202 | + name: `Bearer Lane ${lane.name}`, |
| 203 | + password: FIRST_PASSWORD, |
| 204 | + }); |
| 205 | + expect(created.status, await created.clone().text()).toBe(200); |
| 206 | + expect((await created.json()).data.mustChangePassword).toBe(true); |
| 207 | + |
| 208 | + const { res, body } = await signIn(email, FIRST_PASSWORD); |
| 209 | + expect(res.status, await res.clone().text()).toBe(200); |
| 210 | + credential = lane.credential(res, body); |
| 211 | + expect(credential, `${lane.name}: sign-in yielded no credential`).toBeTruthy(); |
| 212 | + |
| 213 | + const read = await stack.api('/data/showcase_task?$top=1', { headers: lane.headers(credential) }); |
| 214 | + // The gate refusal — asserted as code AND status, and distinctly from |
| 215 | + // the reuse refusal below (different envelope, different outcome). |
| 216 | + expect(read.status).toBe(403); |
| 217 | + expect((await read.json())?.error?.code).toBe('PASSWORD_EXPIRED'); |
| 218 | + }, 120_000); |
| 219 | + |
| 220 | + it('POST /auth/change-password rotates the password AND clears the force-change flag', async () => { |
| 221 | + const changed = await stack.api('/auth/change-password', { |
| 222 | + method: 'POST', |
| 223 | + headers: { 'Content-Type': 'application/json', ...lane.headers(credential) }, |
| 224 | + body: JSON.stringify({ currentPassword: FIRST_PASSWORD, newPassword: SECOND_PASSWORD }), |
| 225 | + }); |
| 226 | + expect(changed.status, await changed.clone().text()).toBe(200); |
| 227 | + |
| 228 | + // The rotation itself was never the broken half — it landed on every |
| 229 | + // lane, which is why the defect answered 200 and looked fine. |
| 230 | + const stale = await signIn(email, FIRST_PASSWORD); |
| 231 | + expect(stale.res.status).toBe(401); |
| 232 | + expect((await stale.res.json())?.code).toBe('INVALID_EMAIL_OR_PASSWORD'); |
| 233 | + |
| 234 | + // …and the half that did NOT run on the bearer lane. |
| 235 | + const { user } = await identity(email); |
| 236 | + expect(user.must_change_password, `${lane.name}: must_change_password not cleared`).toBe(false); |
| 237 | + expect(user.password_changed_at, `${lane.name}: password_changed_at not stamped`).toBeTruthy(); |
| 238 | + expect(Number.isFinite(new Date(user.password_changed_at as string).getTime())).toBe(true); |
| 239 | + }, 120_000); |
| 240 | + |
| 241 | + it('the caller can then reach protected routes with a fresh session', async () => { |
| 242 | + const { res, body } = await signIn(email, SECOND_PASSWORD); |
| 243 | + expect(res.status, await res.clone().text()).toBe(200); |
| 244 | + credential = lane.credential(res, body); |
| 245 | + |
| 246 | + const read = await stack.api('/data/showcase_task?$top=1', { headers: lane.headers(credential) }); |
| 247 | + expect(read.status, await read.clone().text()).toBe(200); |
| 248 | + }, 120_000); |
| 249 | + |
| 250 | + it('ADR-0069 D1: the change RECORDED history and a reused password is REJECTED', async () => { |
| 251 | + // Recorded — the old hash landed in the bounded ring. On the unfixed |
| 252 | + // bearer lane this column stayed null, so a fixture that only checked |
| 253 | + // the flags would have called the security half fixed. |
| 254 | + const { history } = await identity(email); |
| 255 | + expect(history, `${lane.name}: no password history recorded`).toHaveLength(1); |
| 256 | + |
| 257 | + // …and checked. Reusing the password that was just rotated away must be |
| 258 | + // refused — code AND status, distinct from the 403 gate refusal above. |
| 259 | + const reuse = await stack.api('/auth/change-password', { |
| 260 | + method: 'POST', |
| 261 | + headers: { 'Content-Type': 'application/json', ...lane.headers(credential) }, |
| 262 | + body: JSON.stringify({ currentPassword: SECOND_PASSWORD, newPassword: FIRST_PASSWORD }), |
| 263 | + }); |
| 264 | + expect(reuse.status, await reuse.clone().text()).toBe(400); |
| 265 | + const refusal = await reuse.json(); |
| 266 | + expect(refusal?.code).toBe('PASSWORD_REUSE'); |
| 267 | + expect(String(refusal?.message)).toContain(`last ${HISTORY_COUNT} passwords`); |
| 268 | + |
| 269 | + // The refusal must not have rotated anything: the ring is unchanged and |
| 270 | + // the current password still signs in. |
| 271 | + expect((await identity(email)).history).toHaveLength(1); |
| 272 | + const still = await signIn(email, SECOND_PASSWORD); |
| 273 | + expect(still.res.status).toBe(200); |
| 274 | + }, 120_000); |
| 275 | + }); |
| 276 | + } |
| 277 | +}); |
0 commit comments