From 3cbfecc14a4e0fc4dddca23d3d371d3c3188e23e Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 03:12:38 +0000 Subject: [PATCH 1/3] fix(plugin-auth): refuse /admin/revoke-user-session when the token identifies no record better-auth 1.7.1's handler deletes blindly and answers 200 { success: true } unconditionally; a hooks.before admission gate now answers 404 RESOURCE_NOT_FOUND (ADR-0112) when no session carries the supplied token, after grading the vendor's own permission question first so non-admin callers keep the vendor's 401/403 unchanged. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx --- ...in-revoke-user-session-match-guard.test.ts | 391 ++++++++++++++++++ .../admin-revoke-user-session-match-guard.ts | 204 +++++++++ .../plugins/plugin-auth/src/auth-manager.ts | 83 ++++ 3 files changed, 678 insertions(+) create mode 100644 packages/plugins/plugin-auth/src/admin-revoke-user-session-match-guard.test.ts create mode 100644 packages/plugins/plugin-auth/src/admin-revoke-user-session-match-guard.ts diff --git a/packages/plugins/plugin-auth/src/admin-revoke-user-session-match-guard.test.ts b/packages/plugins/plugin-auth/src/admin-revoke-user-session-match-guard.test.ts new file mode 100644 index 0000000000..3eeb0782cb --- /dev/null +++ b/packages/plugins/plugin-auth/src/admin-revoke-user-session-match-guard.test.ts @@ -0,0 +1,391 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #10069 — `POST /admin/revoke-user-session` must not answer success over a +// delete that dispatched nothing. better-auth 1.7.1's handler calls +// `deleteSession(ctx.body.sessionToken)` blindly and answers +// `200 { success: true }` unconditionally — measured on this exact pipeline +// before the guard existed: a zero-match token AND an already-revoked +// (tombstoned) token both answered `200 {"success":true}`. The guard in +// `admin-revoke-user-session-match-guard.ts` refuses those requests with 404 +// `RESOURCE_NOT_FOUND` (ADR-0112: code AND status, never one alone) — but +// ONLY for callers the vendor's own permission check would admit; everyone +// else keeps the vendor's exact 401/403 and gains no existence oracle. +// +// Real better-auth pipeline throughout, following +// `revoke-session-match-guard.test.ts`: requests go in as `Request` objects +// through `AuthManager.handleRequest`, the cookie is the one better-auth +// minted, and the revoke path is better-auth's own. A stub of our adapter +// would prove nothing — the whole question is what answer leaves the library. + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { assertEngineDeleteDispatch, assertEngineUpdateDispatch } from '@objectstack/objectql'; +import { defaultRoles } from 'better-auth/plugins/admin/access'; +import { AuthManager } from './auth-manager'; +import { + ADMIN_REVOKE_USER_SESSION_NOT_FOUND_CODE, + ADMIN_REVOKE_USER_SESSION_NOT_FOUND_MESSAGE, + ADMIN_REVOKE_USER_SESSION_PERMISSION, + adminMayRevokeUserSessions, + anySessionCarriesToken, +} from './admin-revoke-user-session-match-guard'; + +/** + * In-memory IDataEngine — the `session-tombstone.test.ts` harness, unchanged, + * because these tests assert against the same table through the same library + * and a second, more forgiving fake would be able to disagree with it. Both + * write paths stay pinned to ObjectQL's own dispatch predicates + * ({@link assertEngineDeleteDispatch} / {@link assertEngineUpdateDispatch}). + */ +const createMemoryEngine = () => { + const tables = new Map(); + const rows = (name: string) => { + if (!tables.has(name)) tables.set(name, []); + return tables.get(name)!; + }; + const eq = (a: any, b: any) => + a instanceof Date || b instanceof Date + ? new Date(a as any).getTime() === new Date(b as any).getTime() + : a === b; + const matches = (row: any, where: Record = {}) => + Object.entries(where).every(([k, v]) => { + if (k.startsWith('$')) throw new Error(`fake driver: unsupported operator ${k}`); + const actual = row[k]; + if (v && typeof v === 'object' && !Array.isArray(v) && !(v instanceof Date)) { + if ('$ne' in v) return !eq(actual, v.$ne); + if ('$in' in v) return (v.$in as any[]).some((x) => eq(actual, x)); + if ('$gt' in v) return actual > v.$gt; + if ('$gte' in v) return actual >= v.$gte; + if ('$lt' in v) return actual < v.$lt; + if ('$lte' in v) return actual <= v.$lte; + if ('$regex' in v) return new RegExp(String(v.$regex)).test(String(actual ?? '')); + } + return eq(actual, v); + }); + /** `fields` projection — `id` always survives, as it does in ObjectQL. */ + const project = (row: any, fields?: string[]) => { + if (!Array.isArray(fields) || fields.length === 0) return { ...row }; + const out: any = {}; + for (const f of ['id', ...fields]) if (f in row) out[f] = row[f]; + return out; + }; + let seq = 0; + return { + tables, + async insert(name: string, data: any) { + const row = { id: data.id ?? `row_${++seq}`, ...data }; + rows(name).push(row); + return { ...row }; + }, + async findOne(name: string, q: any = {}) { + const row = rows(name).find((r) => matches(r, q.where)); + return row ? project(row, q.fields) : null; + }, + async find(name: string, q: any = {}) { + let out = rows(name).filter((r) => matches(r, q.where)); + const order = q.orderBy?.[0]; + if (order) { + out = [...out].sort( + (a, b) => (a[order.field] > b[order.field] ? 1 : -1) * (order.order === 'desc' ? -1 : 1), + ); + } + if (q.offset) out = out.slice(q.offset); + if (q.limit) out = out.slice(0, q.limit); + return out.map((r) => project(r, q.fields)); + }, + async count(name: string, q: any = {}) { + return rows(name).filter((r) => matches(r, q.where)).length; + }, + async update(name: string, patch: any, options?: any) { + const dispatch = assertEngineUpdateDispatch(patch, options); + if (dispatch.kind === 'multi') { + const hit = rows(name).filter((r) => matches(r, options?.where)); + for (const row of hit) Object.assign(row, patch); + return hit.length; + } + const row = rows(name).find((r) => r.id === dispatch.id); + if (!row) return null; + Object.assign(row, patch); + return { ...row }; + }, + async delete(name: string, q: any = {}) { + assertEngineDeleteDispatch(q); + const table = rows(name); + const keep = table.filter((r) => !matches(r, q.where)); + tables.set(name, keep); + return table.length - keep.length; + }, + }; +}; + +const SECRET = 'test-secret-at-least-32-chars-long!!'; +const PASSWORD = 'S3cure!Passw0rd-10069'; +const BASE = 'http://localhost:3000/api/v1/auth'; + +const makeManager = (engine: any) => + new AuthManager({ + secret: SECRET, + baseUrl: 'http://localhost:3000', + dataEngine: engine, + // The route under test lives on better-auth's `admin` plugin (opt-in here). + plugins: { admin: true }, + } as any); + +const post = (manager: AuthManager, path: string, cookie?: string, body?: unknown) => + manager.handleRequest( + new Request(`${BASE}/${path}`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + ...(cookie ? { cookie } : {}), + }, + body: JSON.stringify(body ?? {}), + }), + ); + +const signUp = (manager: AuthManager, email: string) => + post(manager, 'sign-up/email', undefined, { email, password: PASSWORD, name: 'AdminRevoke' }); + +const signIn = (manager: AuthManager, email: string) => + post(manager, 'sign-in/email', undefined, { email, password: PASSWORD }); + +const getSession = (manager: AuthManager, cookie: string) => + manager.handleRequest( + new Request('http://localhost:3000/api/v1/auth/get-session', { headers: { cookie } }), + ); + +const cookieFrom = (response: Response): string => + (response.headers.getSetCookie?.() ?? [response.headers.get('set-cookie') ?? '']) + .map((c) => c.split(';')[0]) + .filter(Boolean) + .join('; '); + +/** + * Is this cookie still authenticated? better-auth answers `/get-session` with + * HTTP 200 and a JSON `null` body when the session is gone — NOT a 401 — so a + * status-only assertion would pass against a fully revoked session. + */ +const isAuthenticated = async (manager: AuthManager, cookie: string): Promise => { + const res = await getSession(manager, cookie); + if (res.status !== 200) return false; + const body = await res.json().catch(() => null); + return Boolean((body as any)?.user?.id); +}; + +const userRows = (engine: any) => (engine.tables.get('sys_user') ?? []) as any[]; +const sessionRows = (engine: any) => (engine.tables.get('sys_session') ?? []) as any[]; + +/** + * better-auth's admin plugin authorizes on the `user.role` scalar (vendor + * default `adminRoles: ['admin']`). Written straight onto the seeded row, the + * way `impersonation-bearer-rotation.test.ts` does — sign in AFTER promotion + * so the authoritative session read sees it. + */ +const makeRoleAdmin = (engine: any, email: string) => { + const row = userRows(engine).find((r) => r.email === email); + if (!row) throw new Error(`no sys_user row for ${email}`); + row.role = 'admin'; +}; + +const revoke = (manager: AuthManager, cookie: string | undefined, sessionToken: unknown) => + post(manager, 'admin/revoke-user-session', cookie, { sessionToken }); + +beforeEach(() => { + vi.spyOn(console, 'warn').mockImplementation(() => {}); + vi.spyOn(console, 'error').mockImplementation(() => {}); +}); +afterEach(() => vi.restoreAllMocks()); + +/** One env: a target user with a live session, and a role-admin caller. */ +const seed = async () => { + const engine = createMemoryEngine(); + const manager = makeManager(engine); + const targetCookie = cookieFrom(await signUp(manager, 'target@example.com')); + const targetRow = sessionRows(engine)[0]!; + await signUp(manager, 'admin@example.com'); + makeRoleAdmin(engine, 'admin@example.com'); + const adminCookie = cookieFrom(await signIn(manager, 'admin@example.com')); + return { engine, manager, targetCookie, targetRow, adminCookie }; +}; + +// ─────────────────────────────────────────────────────────────────────────── +describe('#10069 — /admin/revoke-user-session refuses when the token identifies no record', () => { + it('a token matching ZERO rows answers 404 with code AND status — never { success: true }', async () => { + const { engine, manager, targetRow, adminCookie } = await seed(); + + const res = await revoke(manager, adminCookie, 'no-such-token-anywhere'); + const body: any = await res.json(); + + // ADR-0112: code AND status, never one alone. + expect(res.status).toBe(404); + expect(body?.code).toBe(ADMIN_REVOKE_USER_SESSION_NOT_FOUND_CODE); + expect(body?.message).toBe(ADMIN_REVOKE_USER_SESSION_NOT_FOUND_MESSAGE); + // The defect's exact shape must be gone, not merely accompanied. + expect(body?.success).not.toBe(true); + // Nobody's session was touched by the failed revoke. + const target = sessionRows(engine).find((r) => r.id === targetRow.id); + expect(target?.revoked_at).toBeUndefined(); + expect(await isAuthenticated(manager, adminCookie)).toBe(true); + }); + + it("an admitted revoke still succeeds end to end — an admin revoking an arbitrary user's session", async () => { + const { engine, manager, targetCookie, targetRow, adminCookie } = await seed(); + + const res = await revoke(manager, adminCookie, targetRow.token); + const body: any = await res.json(); + + // The vendor's success answer, now true when given. + expect(res.status).toBe(200); + expect(body).toHaveProperty('success', true); + // The revoke really happened, as a #7732 tombstone with reason `admin`. + const row = sessionRows(engine).find((r) => r.id === targetRow.id); + expect(row?.revoke_reason).toBe('admin'); + expect(row?.revoked_at).toBeInstanceOf(Date); + expect(await isAuthenticated(manager, targetCookie)).toBe(false); + expect(await isAuthenticated(manager, adminCookie)).toBe(true); + }); + + it('revoking an ALREADY-REVOKED token answers 404 — a revoked session is not a session (#7732)', async () => { + const { engine, manager, targetRow, adminCookie } = await seed(); + + expect((await revoke(manager, adminCookie, targetRow.token)).status).toBe(200); + const tombstonedAt = sessionRows(engine).find((r) => r.id === targetRow.id)?.revoked_at; + + const again = await revoke(manager, adminCookie, targetRow.token); + const body: any = await again.json(); + expect(again.status).toBe(404); + expect(body?.code).toBe(ADMIN_REVOKE_USER_SESSION_NOT_FOUND_CODE); + // The tombstone itself is untouched by the refused second revoke. + const row = sessionRows(engine).find((r) => r.id === targetRow.id); + expect(row?.revoke_reason).toBe('admin'); + expect(row?.revoked_at).toEqual(tombstonedAt); + }); + + it('an EMPTY-string token answers 404 — previously the quietest false success of all', async () => { + const { manager, adminCookie } = await seed(); + + const res = await revoke(manager, adminCookie, ''); + // better-auth's zod body schema accepts an empty string (`z.string()`), so + // this reaches the guard, matches zero rows, and must refuse. + const body: any = await res.json(); + expect(res.status).toBe(404); + expect(body?.code).toBe(ADMIN_REVOKE_USER_SESSION_NOT_FOUND_CODE); + }); + + it('a NON-ADMIN caller keeps the vendor 403 for missing and live tokens alike — no oracle below the permission line', async () => { + const { engine, manager, targetRow } = await seed(); + const plainCookie = cookieFrom(await signUp(manager, 'plain@example.com')); + + const missing = await revoke(manager, plainCookie, 'no-such-token-anywhere'); + const missingBody = await missing.text(); + const live = await revoke(manager, plainCookie, targetRow.token); + const liveBody = await live.text(); + + // The vendor's own permission refusal, both times, byte-identically: a + // caller below the permission line learns nothing about token existence. + expect(missing.status).toBe(403); + expect(live.status).toBe(403); + expect(missingBody).toBe(liveBody); + expect(JSON.parse(missingBody)?.code).toBe('YOU_ARE_NOT_ALLOWED_TO_REVOKE_USERS_SESSIONS'); + expect(JSON.parse(missingBody)?.code).not.toBe(ADMIN_REVOKE_USER_SESSION_NOT_FOUND_CODE); + // And the live target row is untouched: not deleted, not tombstoned. + const row = sessionRows(engine).find((r) => r.id === targetRow.id); + expect(row).toBeDefined(); + expect(row?.revoked_at).toBeUndefined(); + }); + + it('an UNAUTHENTICATED caller still gets the vendor 401, never this 404', async () => { + const { manager, targetRow } = await seed(); + + // A real token, no credentials: the guard must stay silent and let the + // vendor's adminMiddleware answer the authentication question — a 404 + // here would grade an anonymous probe's existence question. + const res = await revoke(manager, undefined, targetRow.token); + expect(res.status).toBe(401); + const body: any = await res.json().catch(() => null); + expect(body?.code).not.toBe(ADMIN_REVOKE_USER_SESSION_NOT_FOUND_CODE); + }); + + it('a NON-STRING token falls through to the vendor 400 — the body schema owns that refusal', async () => { + const { manager, adminCookie } = await seed(); + + const res = await revoke(manager, adminCookie, 12345); + expect(res.status).toBe(400); + const body: any = await res.json().catch(() => null); + expect(body?.code).not.toBe(ADMIN_REVOKE_USER_SESSION_NOT_FOUND_CODE); + }); +}); + +// ─────────────────────────────────────────────────────────────────────────── +describe('adminMayRevokeUserSessions — the vendor permission question, vendor inputs', () => { + const admin = { id: 'u_1', role: 'admin' }; + const user = { id: 'u_2', role: 'user' }; + + it('admits the default admin role and refuses the default user role (real vendor defaultRoles)', () => { + expect(adminMayRevokeUserSessions(admin, {}, defaultRoles as any)).toBe(true); + expect(adminMayRevokeUserSessions(user, {}, defaultRoles as any)).toBe(false); + // Sanity that the imported vendor role really grants the permission the + // handler demands — the mirror and the vendor share this exact object. + expect( + (defaultRoles as any).admin.authorize(ADMIN_REVOKE_USER_SESSION_PERMISSION)?.success, + ).toBe(true); + }); + + it('splits a comma-separated role list, the way the vendor does', () => { + expect(adminMayRevokeUserSessions({ id: 'u', role: 'user,admin' }, {}, defaultRoles as any)).toBe(true); + expect(adminMayRevokeUserSessions({ id: 'u', role: 'user,editor' }, {}, defaultRoles as any)).toBe(false); + }); + + it('admits an adminUserIds member regardless of role', () => { + expect( + adminMayRevokeUserSessions(user, { adminUserIds: ['u_2'] }, defaultRoles as any), + ).toBe(true); + expect( + adminMayRevokeUserSessions(user, { adminUserIds: ['someone-else'] }, defaultRoles as any), + ).toBe(false); + }); + + it('falls back to options.defaultRole when the user carries no role scalar', () => { + expect( + adminMayRevokeUserSessions({ id: 'u' }, { defaultRole: 'admin' }, defaultRoles as any), + ).toBe(true); + expect(adminMayRevokeUserSessions({ id: 'u' }, {}, defaultRoles as any)).toBe(false); + }); + + it('a custom roles map REPLACES the fallback — an admin the operator did not authorize is refused', () => { + // Drift pin, strict direction: were the live options to carry a roles map + // without session:revoke on `admin`, the mirror must refuse (fall through + // to the vendor, which would refuse the same way). + const roles = { admin: { authorize: () => ({ success: false }) } }; + expect(adminMayRevokeUserSessions(admin, { roles }, defaultRoles as any)).toBe(false); + const granting = { admin: { authorize: () => ({ success: true }) } }; + expect(adminMayRevokeUserSessions(admin, { roles: granting }, {} as any)).toBe(true); + }); + + it('grades an unreadable input as not-permitted (fall through), never as admitted', () => { + expect(adminMayRevokeUserSessions(null, {}, defaultRoles as any)).toBe(false); + expect(adminMayRevokeUserSessions({}, null, defaultRoles as any)).toBe(false); + expect( + adminMayRevokeUserSessions( + admin, + { roles: { admin: { authorize: () => { throw new Error('boom'); } } } }, + defaultRoles as any, + ), + ).toBe(false); + }); +}); + +// ─────────────────────────────────────────────────────────────────────────── +describe('anySessionCarriesToken — the admission predicate, no ownership dimension', () => { + it('admits any found session, whoever owns it', () => { + expect(anySessionCarriesToken({ session: { userId: 'u_1', token: 't' } })).toBe(true); + expect(anySessionCarriesToken({ session: {} })).toBe(true); + }); + + it('refuses null / undefined / shapeless results', () => { + expect(anySessionCarriesToken(null)).toBe(false); + expect(anySessionCarriesToken(undefined)).toBe(false); + expect(anySessionCarriesToken({})).toBe(false); + expect(anySessionCarriesToken({ session: null })).toBe(false); + expect(anySessionCarriesToken({ session: 'not-an-object' })).toBe(false); + }); +}); diff --git a/packages/plugins/plugin-auth/src/admin-revoke-user-session-match-guard.ts b/packages/plugins/plugin-auth/src/admin-revoke-user-session-match-guard.ts new file mode 100644 index 0000000000..7c20241ee7 --- /dev/null +++ b/packages/plugins/plugin-auth/src/admin-revoke-user-session-match-guard.ts @@ -0,0 +1,204 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#10069] `POST /admin/revoke-user-session` — a revoke that identifies NO + * record must not report success. + * + * ## The defect, and where it is minted + * + * Not here: the wrong answer comes out of the pinned vendor. better-auth + * `1.7.1` (the installed line, re-read for this card), + * `dist/plugins/admin/routes.mjs`, `revokeUserSession` runs, after its + * `session: ["revoke"]` permission check: + * + * ```js + * await ctx.context.internalAdapter.deleteSession(ctx.body.sessionToken); + * return ctx.json({ success: true }); + * ``` + * + * There is no match check at all — `deleteSession` on a token matching zero + * rows deletes nothing and the endpoint answers `200 { success: true }` + * unconditionally. Measured behaviourally on this repo's real pipeline + * (AuthManager.handleRequest → better-auth 1.7.1 → ObjectQL adapter) before + * the guard existed: a zero-match token answered `200 {"success":true}`, and + * an ALREADY-REVOKED (tombstoned, #7732) token answered `200 {"success":true}` + * as well. Same "security control no-ops while reporting success" class as the + * `/revoke-session` guard (`revoke-session-match-guard.ts`), on the surface + * where a false success matters most: an administrator revoking someone + * else's session and being told it worked. + * + * ## NOT the sibling's predicate + * + * `/revoke-session` skips on an ownership mismatch, so its guard reproduces an + * ownership predicate ("a session of YOURS carries this token"). This admin + * route deletes blindly — the caller is an admin acting on arbitrary users, so + * there is no ownership dimension. The admission predicate here is simply: + * **does any session carry this token** ({@link anySessionCarriesToken}), + * asked through the same `ctx.context.internalAdapter.findSession` seam the + * sibling uses. Copying the sibling's ownership predicate would wrongly narrow + * an admin's legitimate reach to their own sessions. + * + * ## Permission is graded BEFORE existence — the vendor's own question first + * + * The guard runs in the global `hooks.before`, AHEAD of the vendor's + * `adminMiddleware` and its `hasPermission` check. Refusing 404 on a + * zero-match token without first grading the caller would hand every + * authenticated NON-admin an existence oracle the vendor never gave them + * (guard-404 for a missing token vs vendor-403 for a live one). So the guard + * asks the vendor's own permission question first, with the vendor's own + * inputs, and falls through (to the vendor's 401/403) for any caller the + * vendor would refuse: + * + * - the caller's session is resolved via `getAuthoritativeSessionFromCtx` — + * the exact call the vendor's `adminMiddleware` makes (authoritative on + * purpose: never grade a role off the cookie cache); + * - the permission predicate ({@link adminMayRevokeUserSessions}) is + * better-auth's `has-permission.mjs` `hasPermission`, reproduced line for + * line because the vendor does not export it — but asked with the LIVE + * admin-plugin options read off `ctx.context.options.plugins` (the object + * the vendor itself retains) and with the vendor's own exported + * `defaultRoles` from `better-auth/plugins/admin/access` as the fallback, + * so `adminUserIds` / `defaultRole` / custom `roles` configured on the + * plugin are honoured without a second source of truth. (This repo's + * composition passes none of the three — `auth-manager.ts` constructs + * `admin({ schema })` only — and the integration tests pin both drift + * directions: a mirror gone loose answers this guard's 404 where the + * vendor's 403 belongs, a mirror gone strict resurfaces the vendor's false + * success.) + * + * ## The refusal shape + * + * **404 `RESOURCE_NOT_FOUND`** (ADR-0112: code AND status), for the same three + * reasons recorded in `revoke-session-match-guard.ts`: `res.ok` callers, + * DELETE-like semantics on an unidentifiable resource, and the standard + * catalog member over a synonym extension. + * + * ## The existence-oracle question, RE-DECIDED for this surface + * + * The sibling made zero-match and foreign-token answers byte-identical to + * avoid an existence oracle, because its caller is an arbitrary user. Here the + * refusal is deliberately allowed to reveal "no session carries this token" — + * but ONLY to callers who pass the vendor's own `session: ["revoke"]` + * permission check. That caller class is already entitled to session + * existence knowledge: the same default `admin` role statement grants + * `session: ["list"]`, i.e. `/admin/list-user-sessions` over arbitrary users, + * so the 404 tells an entitled admin nothing they cannot already query + * directly. For everyone else the guard is silent by construction (permission + * graded first), so the unauthenticated and unauthorized surfaces keep the + * vendor's exact refusals (401 / 403) with no existence dimension. There is + * no foreign-vs-missing pair to collapse on this route — any live session is + * legitimately deletable by an entitled admin; only "no session at all" is + * refused. + * + * ## What the guard deliberately does NOT decide + * + * - **Unauthenticated / unresolvable callers fall through** — the vendor's + * `adminMiddleware` owns the 401. + * - **Callers its permission mirror refuses fall through** — the vendor's + * `hasPermission` owns the 403 (including the seeded platform admin whose + * `role` scalar is not `admin`, #9482 — this guard must not change that + * surface's recorded behaviour). + * - **A non-string `sessionToken` falls through** — the endpoint's zod body + * schema answers the 400. + * - **An adapter read failure falls through** — never convert "I could not + * look" into "it does not exist". + * + * ## Interaction with session tombstones (#7732) + * + * This route is in `INTERACTIVE_REVOKE_REASON` with reason `admin`, and + * `hideRevokedSessionRow` makes a tombstoned row invisible to + * `internalAdapter.findSession`. So revoking an ALREADY-REVOKED token now + * answers 404 — consistent with the tombstone module's doctrine ("a revoked + * session is not a session") and with the sibling guard; previously it was a + * silent `{ success: true }` no-op (measured, see above). The admitted path + * still tombstones with reason `admin`, untouched. + * + * ## Scope + * + * The SINGULAR route only. `/admin/revoke-user-sessions` (plural) matches by + * user id and cannot mis-identify a single record — zero sessions to sweep is + * genuinely "nothing to do", exactly the reason the sibling left its own + * plural routes untouched. + */ + +/** The refusal's wire code — standard catalog (ADR-0112), see header. */ +export const ADMIN_REVOKE_USER_SESSION_NOT_FOUND_CODE = 'RESOURCE_NOT_FOUND'; + +/** + * The refusal's message. Deliberately NOT the sibling's "of yours" wording — + * this route has no ownership dimension (see header). + */ +export const ADMIN_REVOKE_USER_SESSION_NOT_FOUND_MESSAGE = + 'No session matches the supplied token.'; + +/** + * The permission the vendor's handler demands — `revokeUserSession` calls + * `hasPermission({ …, permissions: { session: ["revoke"] } })`. + */ +export const ADMIN_REVOKE_USER_SESSION_PERMISSION: Readonly> = { + session: ['revoke'], +}; + +/** The vendor's `AccessControl` role shape, as much of it as the mirror reads. */ +type AuthorizingRole = { + authorize?: (permissions: unknown) => { success?: boolean } | undefined; +}; + +/** + * better-auth's admin-plugin `hasPermission` (dist/plugins/admin/ + * has-permission.mjs), reproduced because the vendor does not export it. The + * inputs are the vendor's own: `user` is the authoritative session's user, + * `adminOptions` is the LIVE options object retained on the mounted admin + * plugin, `fallbackRoles` is the vendor's exported `defaultRoles`. Anything + * unreadable grades as not-permitted, which only ever means "fall through to + * the vendor's own refusal" — never a refusal minted here. + */ +export function adminMayRevokeUserSessions( + user: unknown, + adminOptions: unknown, + fallbackRoles: Record, +): boolean { + const u = (user ?? {}) as { id?: unknown; role?: unknown }; + const opts = (adminOptions ?? {}) as { + adminUserIds?: unknown; + defaultRole?: unknown; + roles?: unknown; + }; + const userId = u.id == null ? '' : String(u.id); + if ( + userId && + Array.isArray(opts.adminUserIds) && + opts.adminUserIds.some((x) => String(x) === userId) + ) { + return true; + } + const roleSource = + (typeof u.role === 'string' && u.role) || + (typeof opts.defaultRole === 'string' && opts.defaultRole) || + 'user'; + const acRoles: Record = + opts.roles && typeof opts.roles === 'object' + ? (opts.roles as Record) + : fallbackRoles; + for (const role of roleSource.split(',')) { + try { + if (acRoles[role]?.authorize?.(ADMIN_REVOKE_USER_SESSION_PERMISSION)?.success) return true; + } catch { + // an authorizer that throws grades as not-permitted — the vendor's own + // call would throw the same way one moment later, on its own path. + } + } + return false; +} + +/** + * The admission predicate: does `found` (the `internalAdapter.findSession` + * result) name ANY session? Shape-tolerant like the sibling's — `findSession` + * answers `{ session, user }` or `null`, and anything without a readable + * `session` is "no match", which is also what the vendor's `deleteSession` + * would have made of it (deletes nothing). + */ +export function anySessionCarriesToken(found: unknown): boolean { + const session = (found as { session?: unknown } | null | undefined)?.session; + return session != null && typeof session === 'object'; +} diff --git a/packages/plugins/plugin-auth/src/auth-manager.ts b/packages/plugins/plugin-auth/src/auth-manager.ts index ca2a27c9a2..8cffac5b87 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.ts @@ -54,6 +54,12 @@ import { REVOKE_SESSION_NOT_FOUND_MESSAGE, revokeTargetsCallerSession, } from './revoke-session-match-guard.js'; +import { + ADMIN_REVOKE_USER_SESSION_NOT_FOUND_CODE, + ADMIN_REVOKE_USER_SESSION_NOT_FOUND_MESSAGE, + adminMayRevokeUserSessions, + anySessionCarriesToken, +} from './admin-revoke-user-session-match-guard.js'; import { reconcileMembership, type MembershipPolicy, @@ -1487,6 +1493,24 @@ export class AuthManager { // fall through — the vendor still performs the revoke itself } + // ── #10069: the ADMIN revoke that identifies NO record must not ── + // report success either. better-auth 1.7.1's `admin/revoke-user- + // session` handler calls `deleteSession(ctx.body.sessionToken)` + // blindly and answers `200 { success: true }` unconditionally — + // measured on this pipeline: zero-match AND already-revoked tokens + // both answered success. NOT the #9714 predicate: no ownership + // dimension here (the caller is an admin acting on arbitrary + // users) — the question is only "does any session carry this + // token", and it is asked AFTER the vendor's own permission + // question so non-admins keep the vendor's 401/403 and gain no + // existence oracle. Before-hook on purpose: an after-hook cannot + // change the status. `admin-revoke-user-session-match-guard.ts` + // carries the full reading. + if (ctx?.path === '/admin/revoke-user-session') { + await this.assertAdminRevokeUserSessionIdentifiesRecord(ctx); + // fall through — the vendor still performs the revoke itself + } + // ── ADR-0024: admin-gate self-service SSO provider registration ── // `@better-auth/sso`'s POST /sso/register only checks org-admin when // `body.organizationId` is present (index.mjs: `if (ctx.body @@ -4512,6 +4536,65 @@ export class AuthManager { } } + /** + * [#10069] `/admin/revoke-user-session` admission gate — refuse (404, + * standard `RESOURCE_NOT_FOUND`) when the supplied token identifies no + * session at all, instead of letting the vendor's unconditional success + * line answer `{ success: true }` over a delete that dispatched nothing. + * + * NOT the #9714 predicate: this route has no ownership dimension — the + * caller is an admin acting on arbitrary users, so the question is only + * "does any session carry this token". And it is asked strictly AFTER the + * vendor's own permission question ({@link adminMayRevokeUserSessions}, + * the vendor's `hasPermission` with the LIVE mounted-plugin options), so + * every caller the vendor would refuse falls through to the vendor's own + * 401/403 and never learns whether the token exists. Everything else the + * guard cannot decide falls through too: a non-string token (the vendor's + * zod body schema answers 400), an adapter read failure (never convert + * "could not look" into "does not exist"). Full reading: + * `admin-revoke-user-session-match-guard.ts`. + */ + private async assertAdminRevokeUserSessionIdentifiesRecord(ctx: any): Promise { + const token = ctx?.body?.sessionToken; + if (typeof token !== 'string') return; // vendor's body schema answers 400 + + let admitted: boolean; + try { + // The vendor's own session resolution — the exact call adminMiddleware + // makes (authoritative: a role graded off the cookie cache could admit + // a demoted caller the vendor is about to refuse). + const { getAuthoritativeSessionFromCtx } = await import('better-auth/api'); + const s: any = await getAuthoritativeSessionFromCtx(ctx).catch(() => null); + // No resolvable caller → the vendor's adminMiddleware issues the 401. + if (!s?.user?.id) return; + + // The vendor's own permission question, with the vendor's own inputs: + // the LIVE admin-plugin options (the object the mounted plugin retains) + // and its exported defaultRoles as the fallback. A caller this refuses + // gets the vendor's 403 — never this guard's 404. + const adminOptions = ((ctx?.context?.options?.plugins ?? []) as any[]).find( + (p: any) => p?.id === 'admin', + )?.options; + const { defaultRoles } = await import('better-auth/plugins/admin/access'); + if (!adminMayRevokeUserSessions(s.user, adminOptions, defaultRoles as any)) return; + + const found = await ctx.context.internalAdapter.findSession(token); + admitted = anySessionCarriesToken(found); + } catch { + // A lookup that did not complete hands the request back to the vendor + // unchanged — an engine hiccup must not invent a "not found". + return; + } + + if (!admitted) { + const { APIError } = await import('better-auth/api'); + throw new APIError('NOT_FOUND', { + message: ADMIN_REVOKE_USER_SESSION_NOT_FOUND_MESSAGE, + code: ADMIN_REVOKE_USER_SESSION_NOT_FOUND_CODE, + }); + } + } + /** * [#3697] The issuer's own better-auth membership role in `orgId` — the * input to the invitation role cap. From 79525ca7dbb1abe4c7e4a318d1cd8ff58f239f1a Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 03:23:53 +0000 Subject: [PATCH 2/3] test(plugin-auth): account for defaultRoles in the parity scanners; changeset Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx --- .changeset/spotty-planes-repeat.md | 5 +++++ .../src/better-auth-schema-parity.test.ts | 12 ++++++++++++ .../plugin-auth/src/managed-extension-fields.test.ts | 12 ++++++++++++ 3 files changed, 29 insertions(+) create mode 100644 .changeset/spotty-planes-repeat.md diff --git a/.changeset/spotty-planes-repeat.md b/.changeset/spotty-planes-repeat.md new file mode 100644 index 0000000000..e008c21613 --- /dev/null +++ b/.changeset/spotty-planes-repeat.md @@ -0,0 +1,5 @@ +--- +'@objectstack/plugin-auth': patch +--- + +`POST /api/v1/auth/admin/revoke-user-session` no longer reports success when it revoked nothing. When the supplied `sessionToken` does not identify any live session — including a session that was already revoked — the endpoint now answers `404` with error code `RESOURCE_NOT_FOUND` (ADR-0112 envelope) instead of `200 { "success": true }` over a delete that removed no record. The refusal is only ever given to callers who pass the admin plugin's own `session: ["revoke"]` permission check; unauthenticated and unauthorized callers keep the previous `401`/`403` answers byte-for-byte, so no session-existence information is exposed below the permission line. A revoke that does identify a live session still answers `200 { "success": true }` and tombstones the session with reason `admin`, unchanged. diff --git a/packages/plugins/plugin-auth/src/better-auth-schema-parity.test.ts b/packages/plugins/plugin-auth/src/better-auth-schema-parity.test.ts index 8c7371dced..786ab78744 100644 --- a/packages/plugins/plugin-auth/src/better-auth-schema-parity.test.ts +++ b/packages/plugins/plugin-auth/src/better-auth-schema-parity.test.ts @@ -270,6 +270,18 @@ const AUTH_MANAGER_PLUGINS: Record unknown } | { skip hasPermission: { skip: 'permission predicate exported by the organization plugin — declares no schema', }, + // [#10069] NOT a plugin factory either — same scanner shape as + // `hasPermission` above. `defaultRoles` is the admin plugin's exported + // role→AccessControl map (`better-auth/plugins/admin/access`); it declares no + // schema, so it contributes no model and no column for this gate to compare. + // `assertAdminRevokeUserSessionIdentifiesRecord` reads it so the + // admin-revoke-user-session gate asks the vendor's own permission question + // (its `hasPermission` fallback roles) rather than keeping a second spelling + // of it. The `stale` assertion below removes this entry's licence the moment + // that import goes away. + defaultRoles: { + skip: 'role→AccessControl map exported by the admin plugin — declares no schema', + }, }; /** The plugin set the auth manager actually assembles (`buildPluginList()`). */ diff --git a/packages/plugins/plugin-auth/src/managed-extension-fields.test.ts b/packages/plugins/plugin-auth/src/managed-extension-fields.test.ts index 5f19b39053..72bd821f66 100644 --- a/packages/plugins/plugin-auth/src/managed-extension-fields.test.ts +++ b/packages/plugins/plugin-auth/src/managed-extension-fields.test.ts @@ -453,6 +453,18 @@ const AUTH_MANAGER_PLUGINS: Record unknown } | { skip hasPermission: { skip: 'permission predicate exported by the organization plugin — declares no schema', }, + // [#10069] NOT a plugin factory either — same scanner shape as + // `hasPermission` above. `defaultRoles` is the admin plugin's exported + // role→AccessControl map (`better-auth/plugins/admin/access`); it declares no + // schema, contributes no model and no column, so there is nothing here for + // the collision loop to compare. `assertAdminRevokeUserSessionIdentifiesRecord` + // reads it so the admin-revoke-user-session gate asks the vendor's own + // permission question (its `hasPermission` fallback roles) rather than keeping + // a second spelling of it. The `stale` assertion below removes this entry's + // licence the moment that import goes away. + defaultRoles: { + skip: 'role→AccessControl map exported by the admin plugin — declares no schema', + }, }; /** The plugin set the auth manager actually assembles (`buildPluginList()`). */ From b293081c2dcb0a8767f9807e16df444ec73f42fd Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 03:29:42 +0000 Subject: [PATCH 3/3] chore: register the new pinned engine double in the contract ledger Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx --- scripts/engine-double-contract.pinned.json | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index 747cbf351e..339b2ec6a4 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -1026,6 +1026,16 @@ "verb": "update", "pinned": 1 }, + { + "file": "packages/plugins/plugin-auth/src/admin-revoke-user-session-match-guard.test.ts", + "verb": "delete", + "pinned": 1 + }, + { + "file": "packages/plugins/plugin-auth/src/admin-revoke-user-session-match-guard.test.ts", + "verb": "update", + "pinned": 1 + }, { "file": "packages/plugins/plugin-auth/src/auth-manager.jwt-eddsa-fallback.test.ts", "verb": "delete",