From 8d97b20b3403cb9b14a10abd07cc9324f19254d5 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 20:40:09 +0000 Subject: [PATCH 1/4] fix(plugin-auth)!: refuse an unparseable declared-JSON request body with 400 INVALID_REQUEST instead of reading it as {} Nine body readers in plugin-auth read a body that does not parse as `{}`: the three inline mounts in auth-plugin.ts (admin/oauth2/toggle-disabled, admin/unlock-user, sys-oauth-application/register), the identity import, the four /admin/sso/* bridges and the self-service verification resend. A truncated body was answered as if the client had sent none, and the resend mailed the caller's own address for a body that named another. A package-local reader (request-json-body.ts, not re-exported) now answers a declared application/json body that is not blank and does not parse with the transports' 400 INVALID_REQUEST refusal and message. An empty or whitespace-only body, a body that parses (whatever its Content-Type) and a body that does not declare JSON are read as before. Claude-Session: https://claude.ai/code/session_018VqJPGA1NDNSCZsnecsChP Co-authored-by: Claude --- .../plugin-auth/src/admin-import-users.ts | 9 +- .../plugins/plugin-auth/src/auth-plugin.ts | 28 +- .../plugin-auth/src/register-sso-provider.ts | 40 +- .../src/request-json-body-refusal.test.ts | 347 ++++++++++++++++++ .../plugin-auth/src/request-json-body.ts | 79 ++++ .../src/send-verification-email.test.ts | 17 +- .../src/send-verification-email.ts | 14 +- 7 files changed, 507 insertions(+), 27 deletions(-) create mode 100644 packages/plugins/plugin-auth/src/request-json-body-refusal.test.ts create mode 100644 packages/plugins/plugin-auth/src/request-json-body.ts diff --git a/packages/plugins/plugin-auth/src/admin-import-users.ts b/packages/plugins/plugin-auth/src/admin-import-users.ts index 0e2951c009..7f53836dde 100644 --- a/packages/plugins/plugin-auth/src/admin-import-users.ts +++ b/packages/plugins/plugin-auth/src/admin-import-users.ts @@ -113,6 +113,7 @@ import { prepareImportRequest, runImport } from '@objectstack/rest'; import { generatePlaceholderEmail, isPlaceholderEmail } from './placeholder-email.js'; import { generateTemporaryPassword, normalizePhoneNumber, isLikelyEmail, type AdminActor, type EndpointResult } from './admin-user-endpoints.js'; import { SYS_USER_IMPORT_UPDATE_FIELDS } from './sys-user-writable-fields.js'; +import { readJsonRequestBody } from './request-json-body.js'; import { runImportPasswordReset, type AccountInvitationMarker, @@ -489,11 +490,15 @@ export async function runAdminImportUsers( request: Request, actor: AdminActor, ): Promise { - let body: any = {}; - try { body = await request.json(); } catch { body = {}; } const fail = (status: number, code: string, message: string): EndpointResult => ({ status, body: { success: false, error: { code, message } }, }); + // [#24032] A declared-JSON body that does not parse is refused, never read as + // `{}`, which answered "Provide format…" for a payload the admin did send; + // see request-json-body.ts. + const reading = await readJsonRequestBody(request); + if (!reading.ok) return fail(400, 'INVALID_REQUEST', reading.message); + const body: any = reading.body; // ── Request-level validation ───────────────────────────────────────── // Default policy: `auto` (#3236) — prefer the invite path (no shared secret diff --git a/packages/plugins/plugin-auth/src/auth-plugin.ts b/packages/plugins/plugin-auth/src/auth-plugin.ts index 927ce2a92e..4756ae9329 100644 --- a/packages/plugins/plugin-auth/src/auth-plugin.ts +++ b/packages/plugins/plugin-auth/src/auth-plugin.ts @@ -70,6 +70,7 @@ import { MANAGED_EXTENSION_EDITABLE_FIELDS } from './managed-extension-fields.js import { runSetInitialPassword } from './set-initial-password.js'; import { runRegisterSsoProviderFromForm, runRegisterSamlProviderFromForm, runRequestDomainVerification, runVerifyDomain } from './register-sso-provider.js'; import { runResendVerificationEmail } from './send-verification-email.js'; +import { readJsonRequestBody } from './request-json-body.js'; import type { CounterStore } from './rate-limit-storage.js'; import { authIdentityManifest } from './manifest.js'; import { scheduleLegacySsoSecretMigration } from './sso-client-secret.js'; @@ -2731,8 +2732,14 @@ export class AuthPlugin implements Plugin { // Upstream tracking: https://github.com/better-auth/better-auth rawApp.post(`${basePath}/admin/oauth2/toggle-disabled`, async (c: any) => { try { - let body: any = {}; - try { body = await c.req.json(); } catch { body = {}; } + // [#24032] A declared-JSON body that does not parse is refused, never + // read as `{}` (which answered "client_id is required" for a body that + // carried it); see request-json-body.ts. + const reading = await readJsonRequestBody(c.req.raw); + if (!reading.ok) { + return c.json({ success: false, error: { code: 'INVALID_REQUEST', message: reading.message } }, 400); + } + const body: any = reading.body; const clientId: unknown = body?.client_id; const disabled: unknown = body?.disabled; if (typeof clientId !== 'string' || clientId.length === 0) { @@ -2866,8 +2873,13 @@ export class AuthPlugin implements Plugin { // server-side; mirrors the toggle-disabled route's session+role check. rawApp.post(`${basePath}/admin/unlock-user`, async (c: any) => { try { - let body: any = {}; - try { body = await c.req.json(); } catch { body = {}; } + // [#24032] Same read as toggle-disabled above: a truncated body used to + // answer "userId is required" for a body that carried `userId`. + const reading = await readJsonRequestBody(c.req.raw); + if (!reading.ok) { + return c.json({ success: false, error: { code: 'INVALID_REQUEST', message: reading.message } }, 400); + } + const body: any = reading.body; const userId: unknown = body?.userId ?? body?.user_id; if (typeof userId !== 'string' || userId.length === 0) { return c.json({ success: false, error: { code: 'INVALID_REQUEST', message: 'userId is required' } }, 400); @@ -3419,8 +3431,12 @@ export class AuthPlugin implements Plugin { // just to fix UX. Keeping the wrapper scoped to the self-service flow. rawApp.post(`${basePath}/sys-oauth-application/register`, async (c: any) => { try { - let body: any = {}; - try { body = await c.req.json(); } catch { body = {}; } + // [#24032] Same read as toggle-disabled: refused, never read as `{}`. + const reading = await readJsonRequestBody(c.req.raw); + if (!reading.ok) { + return c.json({ success: false, error: { code: 'INVALID_REQUEST', message: reading.message } }, 400); + } + const body: any = reading.body; const name: unknown = body?.name; const redirectUrlsInput: unknown = body?.redirectURLs; diff --git a/packages/plugins/plugin-auth/src/register-sso-provider.ts b/packages/plugins/plugin-auth/src/register-sso-provider.ts index 5dc817f6c6..4255bb4a92 100644 --- a/packages/plugins/plugin-auth/src/register-sso-provider.ts +++ b/packages/plugins/plugin-auth/src/register-sso-provider.ts @@ -23,6 +23,7 @@ */ import { inProcessRedispatchUrl } from './in-process-redispatch.js'; +import { readJsonRequestBody } from './request-json-body.js'; export interface RegisterSsoFormResult { /** HTTP status to return to the caller. */ @@ -38,6 +39,21 @@ export interface RegisterSsoFormResult { /** A better-auth universal handler: `(request) => Response`. */ export type AuthRequestHandler = (request: Request) => Promise; +/** + * [#24032] The four bridges' body read. A declared-JSON body that does not + * parse is answered `400 INVALID_REQUEST` before the vendor is called; it was + * read as `{}`, which answered "Missing required field(s)" for a form that + * carried every field (see request-json-body.ts). + */ +async function readFormBody(request: Request): Promise<{ ok: true; body: any } | { ok: false; refusal: RegisterSsoFormResult }> { + const reading = await readJsonRequestBody(request); + if (reading.ok) return { ok: true, body: reading.body }; + return { + ok: false, + refusal: { status: 400, body: { success: false, error: { code: 'INVALID_REQUEST', message: reading.message } } }, + }; +} + /** * Resolve the caller's active organization id by re-dispatching a * `/get-session` through the same better-auth handler. Returns `undefined` on @@ -94,12 +110,9 @@ export async function runRegisterSsoProviderFromForm( handle: AuthRequestHandler, request: Request, ): Promise { - let body: any; - try { - body = await request.json(); - } catch { - body = {}; - } + const read = await readFormBody(request); + if (!read.ok) return read.refusal; + const body = read.body; const str = (v: unknown): string => (typeof v === 'string' ? v.trim() : ''); const providerId = str(body?.providerId); const issuer = str(body?.issuer); @@ -246,8 +259,9 @@ export async function runRegisterSamlProviderFromForm( handle: AuthRequestHandler, request: Request, ): Promise { - let body: any; - try { body = await request.json(); } catch { body = {}; } + const read = await readFormBody(request); + if (!read.ok) return read.refusal; + const body = read.body; const str = (v: unknown): string => (typeof v === 'string' ? v.trim() : ''); const providerId = str(body?.providerId); const issuer = str(body?.issuer); @@ -440,8 +454,9 @@ export async function runRequestDomainVerification( request: Request, options: SsoDomainVerificationBridgeOptions = {}, ): Promise { - let body: any; - try { body = await request.json(); } catch { body = {}; } + const read = await readFormBody(request); + if (!read.ok) return read.refusal; + const body = read.body; const str = (v: unknown): string => (typeof v === 'string' ? v.trim() : ''); const providerId = str(body?.providerId); const domain = bareHostname(str(body?.domain)); @@ -497,8 +512,9 @@ export async function runVerifyDomain( request: Request, options: SsoDomainVerificationBridgeOptions = {}, ): Promise { - let body: any; - try { body = await request.json(); } catch { body = {}; } + const read = await readFormBody(request); + if (!read.ok) return read.refusal; + const body = read.body; const str = (v: unknown): string => (typeof v === 'string' ? v.trim() : ''); const providerId = str(body?.providerId); if (!providerId) { diff --git a/packages/plugins/plugin-auth/src/request-json-body-refusal.test.ts b/packages/plugins/plugin-auth/src/request-json-body-refusal.test.ts new file mode 100644 index 0000000000..6f8222eb06 --- /dev/null +++ b/packages/plugins/plugin-auth/src/request-json-body-refusal.test.ts @@ -0,0 +1,347 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * A declared-JSON body that does not parse is REFUSED by this package's own + * routes — `400 INVALID_REQUEST`, the code and message family the HTTP + * transports answer for the same input — never read as `{}` (#24032). + * + * Every route here used to read its body as + * `try { body = await request.json() } catch { body = {} }`, so a truncated + * body was answered as if the client had sent none: `POST /admin/unlock-user` + * with `{"userId": "usr_nobody"` (closing brace missing) answered + * `400 "userId is required"` for a body that carried `userId`, and the one-click + * `send-verification-email` resend mailed the CALLER's own address for a body + * that named another one. + * + * Driven through the plugin's REAL route registration on a real Hono app (the + * `admin-sso-bridge-gate.test.ts` harness), with the auth manager reduced to + * the seams these routes read. One block per route family: + * + * A. the inline mounts in `auth-plugin.ts` — toggle-disabled, unlock-user, + * sys-oauth-application/register; + * B. the identity import (`admin-import-users.ts`); + * C. the four `/admin/sso/*` bridges (`register-sso-provider.ts`); + * D. the self-service resend (`send-verification-email.ts`). + * + * Each refusal asserts the envelope, the code, the message's first sentence + * (the message family IS this card's contract), that the message does not echo + * the body, and that the route did nothing after the read. Each family also + * pins its EMPTY-body answer — no body, a zero-byte body, a whitespace-only + * body — as it is today, since an empty body is not this change. + */ + +import { describe, it, expect, vi, beforeAll, beforeEach } from 'vitest'; +import { Hono } from 'hono'; +import { envelopeViolations } from '@objectstack/spec/api'; +import type { PluginContext } from '@objectstack/core'; +import { AuthPlugin } from './auth-plugin.js'; +// The import route loads this module with a dynamic `import()` on its first +// request, and that first load (`@objectstack/rest`'s import framework among +// it) measured 8.7s inside a 10s clocked test. Loaded here, it is paid during +// collection, and the route's own `import()` reads the module cache. +import './admin-import-users.js'; + +const BASE = '/api/v1/auth'; +const ORIGIN = 'http://localhost:3000'; +const SESSION_EMAIL = 'me@example.test'; +const JSON_TYPE = { 'content-type': 'application/json' }; + +/** The first sentence of the transports' refusal (`plugin-hono-server`'s `request-body.ts`). */ +const PARSE_REFUSAL = 'The request body could not be parsed as JSON (it was sent as application/json).'; +const UNREADABLE_REFUSAL = 'The request body could not be read.'; + +/** Everything a route could DO after reading its body, recorded per request. */ +const acts = { + unlockUser: vi.fn(async (_id: string) => true), + createOAuthClient: vi.fn(async (_input: unknown) => ({ client_id: 'cl_pin' })), + engine: [] as string[], + vendor: [] as string[], + sent: [] as unknown[], +}; + +const engineOp = (op: string) => vi.fn(async (object: string) => { + acts.engine.push(`${op} ${object}`); + return op === 'find' ? [] : op === 'count' ? 0 : null; +}); +const engine = { + insert: engineOp('insert'), + update: engineOp('update'), + delete: engineOp('delete'), + find: engineOp('find'), + findOne: engineOp('findOne'), + count: engineOp('count'), +}; + +const json = (body: unknown, status = 200) => + new Response(JSON.stringify(body), { status, headers: JSON_TYPE }); + +/** better-auth's universal handler: a session for `/get-session`, a recorder for the resend. */ +async function handleRequest(req: Request): Promise { + const path = new URL(req.url).pathname; + acts.vendor.push(path); + if (path.endsWith('/get-session')) return json({ user: { email: SESSION_EMAIL }, session: {} }); + if (path.endsWith('/send-verification-email')) { + acts.sent.push(await req.json()); + return json({ status: true }); + } + return new Response(null, { status: 404 }); +} + +const mockCtx = (): PluginContext => + ({ + registerService: vi.fn(), + getService: vi.fn((name: string) => (name === 'manifest' ? { register: vi.fn() } : undefined)), + getServices: vi.fn(() => new Map()), + hook: vi.fn(), + trigger: vi.fn(), + logger: { info: vi.fn(), error: vi.fn(), warn: vi.fn(), debug: vi.fn() }, + getKernel: vi.fn(), + }) as unknown as PluginContext; + +let app: Hono; + +beforeAll(async () => { + app = new Hono(); + const ctx = mockCtx(); + const plugin = new AuthPlugin({ + secret: 'test-secret-at-least-32-chars-long!!', + plugins: { oidcProvider: false }, + }); + await plugin.init(ctx); + (plugin as unknown as { authManager: unknown }).authManager = { + handleRequest, + getApi: async () => ({ + getSession: async () => ({ user: { id: 'usr_platform_admin', email: SESSION_EMAIL, isPlatformAdmin: true } }), + createOAuthClient: acts.createOAuthClient, + }), + getDataEngine: () => engine, + unlockUser: acts.unlockUser, + isSsoDomainVerificationEnabled: () => true, + isPhoneNumberEnabled: () => false, + isEmailServiceAvailable: () => false, + isPhoneOtpDeliverable: () => false, + getConsoleUrl: () => undefined, + noteMustChangePasswordIssued: () => undefined, + }; + (plugin as unknown as { + registerAuthRoutes(server: unknown, ctx: PluginContext): void; + }).registerAuthRoutes({ getRawApp: () => app, getPort: () => 0 }, ctx); +}); + +beforeEach(() => { + acts.unlockUser.mockClear(); + acts.createOAuthClient.mockClear(); + acts.engine.length = 0; + acts.vendor.length = 0; + acts.sent.length = 0; +}); + +const post = (path: string, init: { headers?: Record; body?: BodyInit }) => + app.request(`${ORIGIN}${BASE}${path}`, { + method: 'POST', + headers: { origin: ORIGIN, ...(init.headers ?? {}) }, + ...(init.body !== undefined ? { body: init.body } : {}), + }); + +/** The three empty spellings a client sends: none at all, zero bytes, whitespace only. */ +const EMPTY_BODIES: Array<[string, { headers?: Record; body?: BodyInit }]> = [ + ['no body and no Content-Type', {}], + ['a zero-byte body sent as application/json', { headers: { ...JSON_TYPE, 'content-length': '0' }, body: '' }], + ['a whitespace-only body sent as application/json', { headers: JSON_TYPE, body: ' \n\t ' }], +]; + +async function expectParseRefusal(res: Response, notEchoed: string, first = PARSE_REFUSAL) { + const body = (await res.json()) as { success: boolean; error: { code: string; message: string } }; + expect(res.status, JSON.stringify(body)).toBe(400); + expect(envelopeViolations(body)).toEqual([]); + expect(body.success).toBe(false); + expect(body.error.code).toBe('INVALID_REQUEST'); + expect(body.error.message.startsWith(first), body.error.message).toBe(true); + expect(body.error.message).not.toContain(notEchoed); +} + +async function expectAnswer(res: Response, status: number, code: string, message: string) { + const body = (await res.json()) as { success: boolean; error?: { code: string; message: string } }; + expect(res.status, JSON.stringify(body)).toBe(status); + expect(body.error?.code).toBe(code); + expect(body.error?.message).toBe(message); +} + +const nothingActed = () => { + expect(acts.unlockUser).not.toHaveBeenCalled(); + expect(acts.createOAuthClient).not.toHaveBeenCalled(); + expect(acts.engine).toEqual([]); + expect(acts.vendor).toEqual([]); + expect(acts.sent).toEqual([]); +}; + +// ── A. the inline mounts in auth-plugin.ts ───────────────────────────────── + +const INLINE: Array<{ path: string; truncated: string; echo: string; empty: string }> = [ + { + path: '/admin/unlock-user', + truncated: '{"userId": "usr_nobody"', + echo: 'usr_nobody', + empty: 'userId is required', + }, + { + path: '/admin/oauth2/toggle-disabled', + truncated: '{"client_id": "cl_pin_absent", "disabled": true', + echo: 'cl_pin_absent', + empty: 'client_id is required', + }, + { + path: '/sys-oauth-application/register', + truncated: '{"name": "Pin App", "redirectURLs": "https://pin.example/cb"', + echo: 'Pin App', + empty: 'name is required', + }, +]; + +describe('A. the inline auth-plugin mounts refuse a declared-JSON body that does not parse', () => { + for (const route of INLINE) { + it(`${route.path}: a truncated body is 400 INVALID_REQUEST, and nothing after the read runs`, async () => { + const res = await post(route.path, { headers: JSON_TYPE, body: route.truncated }); + await expectParseRefusal(res, route.echo); + nothingActed(); + }); + + it(`${route.path}: { not json — the same refusal`, async () => { + await expectParseRefusal(await post(route.path, { headers: JSON_TYPE, body: '{ not json' }), 'not json'); + nothingActed(); + }); + + for (const [label, init] of EMPTY_BODIES) { + it(`${route.path}: CONTROL — ${label} keeps today's answer, 400 "${route.empty}"`, async () => { + await expectAnswer(await post(route.path, init), 400, 'INVALID_REQUEST', route.empty); + nothingActed(); + }); + } + } + + it('/admin/unlock-user: a declared-JSON body that cannot be READ is refused, not read as no body', async () => { + const stream = new ReadableStream({ start(controller) { controller.error(new Error('socket reset')); } }); + const res = await app.request( + new Request(`${ORIGIN}${BASE}/admin/unlock-user`, { + method: 'POST', + headers: { origin: ORIGIN, ...JSON_TYPE }, + body: stream, + duplex: 'half', + } as RequestInit), + ); + await expectParseRefusal(res, 'socket reset', UNREADABLE_REFUSAL); + nothingActed(); + }); + + it('/admin/unlock-user: CONTROL — a well-formed body is read and acted on, as before', async () => { + const res = await post('/admin/unlock-user', { headers: JSON_TYPE, body: '{"userId": "usr_nobody"}' }); + expect(res.status).toBe(200); + expect(await res.json()).toEqual({ success: true, data: { userId: 'usr_nobody' } }); + expect(acts.unlockUser.mock.calls).toEqual([['usr_nobody']]); + }); + + it('/admin/unlock-user: CONTROL — these routes never read the Content-Type to PARSE: valid JSON sent as text/plain is read, as before', async () => { + const res = await post('/admin/unlock-user', { headers: { 'content-type': 'text/plain' }, body: '{"userId": "usr_nobody"}' }); + expect(res.status).toBe(200); + expect(acts.unlockUser.mock.calls).toEqual([['usr_nobody']]); + }); + + it('/admin/unlock-user: CONTROL — a body that does not declare JSON and does not parse is still no body', async () => { + const res = await post('/admin/unlock-user', { headers: { 'content-type': 'text/plain' }, body: '{"userId": "usr_nobody"' }); + await expectAnswer(res, 400, 'INVALID_REQUEST', 'userId is required'); + nothingActed(); + }); +}); + +// ── B. the identity import (admin-import-users.ts) ───────────────────────── + +describe('B. /admin/import-users refuses a declared-JSON body that does not parse', () => { + it('a truncated body is 400 INVALID_REQUEST, and no row is read or written', async () => { + const res = await post('/admin/import-users', { + headers: JSON_TYPE, + body: '{"format": "json", "rows": [{"email": "pin-import@example.test"}]', + }); + await expectParseRefusal(res, 'pin-import@example.test'); + nothingActed(); + }); + + for (const [label, init] of EMPTY_BODIES) { + it(`CONTROL — ${label} keeps today's answer, 400 INVALID_REQUEST naming the payload formats`, async () => { + const res = await post('/admin/import-users', init); + const body = (await res.json()) as { error: { code: string; message: string } }; + expect(res.status, JSON.stringify(body)).toBe(400); + expect(body.error.code).toBe('INVALID_REQUEST'); + expect(body.error.message.startsWith('Provide format:'), body.error.message).toBe(true); + nothingActed(); + }); + } +}); + +// ── C. the four /admin/sso/* bridges (register-sso-provider.ts) ──────────── + +const BRIDGES: Array<{ path: string; truncated: string; empty: string }> = [ + { + path: '/admin/sso/register', + truncated: + '{"providerId": "acme-pin", "issuer": "https://idp.acme.example", "domain": "acme.example", "clientId": "cid", "clientSecret": "csecret"', + empty: 'Missing required field(s): providerId, issuer, domain, clientId, clientSecret', + }, + { + path: '/admin/sso/register-saml', + truncated: + '{"providerId": "acme-pin", "issuer": "https://idp.acme.example/entity", "domain": "acme.example", "entryPoint": "https://idp.acme.example/sso", "cert": "PROBE-CERT"', + empty: 'Missing required field(s): providerId, issuer, domain, entryPoint, cert', + }, + { + path: '/admin/sso/request-domain-verification', + truncated: '{"providerId": "acme-pin"', + empty: 'Missing required field: providerId', + }, + { + path: '/admin/sso/verify-domain', + truncated: '{"providerId": "acme-pin"', + empty: 'Missing required field: providerId', + }, +]; + +describe('C. the /admin/sso/* bridges refuse a declared-JSON body that does not parse', () => { + for (const bridge of BRIDGES) { + it(`${bridge.path}: a truncated body is 400 INVALID_REQUEST, and the vendor is never called`, async () => { + const res = await post(bridge.path, { headers: JSON_TYPE, body: bridge.truncated }); + await expectParseRefusal(res, 'acme-pin'); + nothingActed(); + }); + + for (const [label, init] of EMPTY_BODIES) { + it(`${bridge.path}: CONTROL — ${label} keeps today's answer, 400 "${bridge.empty}"`, async () => { + await expectAnswer(await post(bridge.path, init), 400, 'INVALID_REQUEST', bridge.empty); + nothingActed(); + }); + } + } +}); + +// ── D. the self-service resend (send-verification-email.ts) ──────────────── + +describe('D. /send-verification-email refuses a declared-JSON body that does not parse', () => { + it('a truncated body naming another address is 400 INVALID_REQUEST, and nothing is looked up or sent', async () => { + const res = await post('/send-verification-email', { + headers: { ...JSON_TYPE, cookie: 'better-auth.session_token=pin' }, + body: '{"email": "other@example.test"', + }); + await expectParseRefusal(res, 'other@example.test'); + nothingActed(); + }); + + for (const [label, init] of EMPTY_BODIES) { + it(`CONTROL — ${label} is still the one-click resend to the caller's own address`, async () => { + const res = await post('/send-verification-email', { + ...init, + headers: { ...(init.headers ?? {}), cookie: 'better-auth.session_token=pin' }, + }); + expect(res.status).toBe(200); + expect(await res.json()).toEqual({ status: true }); + expect(acts.sent).toEqual([{ email: SESSION_EMAIL }]); + }); + } +}); diff --git a/packages/plugins/plugin-auth/src/request-json-body.ts b/packages/plugins/plugin-auth/src/request-json-body.ts new file mode 100644 index 0000000000..71516e4a3c --- /dev/null +++ b/packages/plugins/plugin-auth/src/request-json-body.ts @@ -0,0 +1,79 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The JSON body of a request to one of this package's own routes: parsed, + * `{}` when there is none, or a refusal MESSAGE when it was declared as JSON + * and does not parse (#24032). Module-private: `index.ts` does not re-export + * it, and no package entry carries it. + * + * ## The defect + * + * Nine body readers here were spelled + * `try { body = await request.json() } catch { body = {} }` (three of them over + * `c.req.json()`), so a truncated or garbled body was read exactly like no + * body, and the route answered for a request the client never sent. As the + * card measured it, on a fresh `objectstack dev` showcase boot with an admin + * session: `POST /api/v1/auth/admin/unlock-user` with `{"userId": "usr_nobody"` (the + * closing brace missing), sent as `application/json`, answered + * `400 "userId is required"` for a body that carried `userId`. The one-click + * `send-verification-email` resend reads `{}` as "send to my own address", so + * a truncated body naming another address mailed the caller instead. + * + * The HTTP transports (`@objectstack/plugin-hono-server`, `@objectstack/hono`) + * and `@objectstack/cloud-connection`'s own routes refuse the same input with + * the same `code` and the same message words. None of them exports the + * refusal (a shared export would widen a package's public surface), so this + * package carries its own copy of the two messages. + * + * ## What a reading answers + * + * - A body that parses as JSON: that value, as before, whatever its + * `Content-Type`. These routes have never read the media type to PARSE (a + * valid JSON body sent as `text/plain` is read), and they still do not; + * `JSON.parse` over `text()` is what `request.json()` does. + * - A body sent as `application/json` (matched case-insensitively) that is not + * blank and does not parse: `ok: false`, answered `400` with the registered + * code `INVALID_REQUEST` (`ERROR_CODE_LEDGER`) in the declared envelope. The + * message names the parse failure and never echoes the body. + * - A body sent as `application/json` that cannot be READ at all (the stream + * failed): `ok: false`, the same code with the unreadable message. + * - No body, a blank one (empty or whitespace only), or one that does not + * declare JSON and does not parse: `{}`, as before. An empty body is a real + * request here: the `sys_user` resend action fires with none. + * + * It answers a MESSAGE, never a response: each route builds its own `400` + * body, in its own module, beside every other refusal it builds. + */ + +/** `ok: false` is answered `400` with the code `INVALID_REQUEST` and this `message`. */ +export type JsonBodyReading = + | { readonly ok: true; readonly body: any } + | { readonly ok: false; readonly message: string }; + +/** The refusal tail every message ends with: what the route did NOT do. */ +const NOT_EMPTY = 'It was refused rather than read as an empty body'; + +export const MALFORMED_JSON_BODY_MESSAGE = + `The request body could not be parsed as JSON (it was sent as application/json). ${NOT_EMPTY}: ` + + 'send a well-formed JSON document, or no body at all.'; + +export const UNREADABLE_BODY_MESSAGE = `The request body could not be read. ${NOT_EMPTY}.`; + +function declaresJson(request: Request): boolean { + return (request.headers.get('content-type') ?? '').toLowerCase().includes('application/json'); +} + +export async function readJsonRequestBody(request: Request): Promise { + let text: string; + try { + text = await request.text(); + } catch { + return declaresJson(request) ? { ok: false, message: UNREADABLE_BODY_MESSAGE } : { ok: true, body: {} }; + } + try { + return { ok: true, body: JSON.parse(text) }; + } catch { + if (text.trim() === '' || !declaresJson(request)) return { ok: true, body: {} }; + return { ok: false, message: MALFORMED_JSON_BODY_MESSAGE }; + } +} diff --git a/packages/plugins/plugin-auth/src/send-verification-email.test.ts b/packages/plugins/plugin-auth/src/send-verification-email.test.ts index 7ae0808453..9345a1339e 100644 --- a/packages/plugins/plugin-auth/src/send-verification-email.test.ts +++ b/packages/plugins/plugin-auth/src/send-verification-email.test.ts @@ -88,13 +88,26 @@ describe('runResendVerificationEmail', () => { expect(sent).toHaveLength(0); }); - it('tolerates a non-JSON body and falls back to the session email', async () => { + // [#24032] This row used to send `not-json{` as `application/json` and pin + // the session fallback: the very branch that read an unparseable declared + // body as no body. That body is now refused (`request-json-body-refusal.test.ts` + // drives the route). What stays is the undeclared spelling, read as before. + it('a body that does not declare JSON and does not parse still falls back to the session email', async () => { const { handle, sent } = makeHandle({ sessionEmail: 'me@example.test' }); - const res = await runResendVerificationEmail(handle, makeRequest('not-json{')); + const res = await runResendVerificationEmail(handle, makeRequest('not-json{', { 'content-type': 'text/plain' })); expect(res.status).toBe(200); expect(sent[0].body).toEqual({ email: 'me@example.test' }); }); + it('a declared-JSON body that does not parse is refused 400 INVALID_REQUEST, and nothing is looked up or sent', async () => { + const { handle, sent } = makeHandle({ sessionEmail: 'me@example.test' }); + const res = await runResendVerificationEmail(handle, makeRequest('not-json{')); + expect(res.status).toBe(400); + expect((res.body as any).error?.code).toBe('INVALID_REQUEST'); + expect(handle).not.toHaveBeenCalled(); + expect(sent).toHaveLength(0); + }); + it('passes through the native error status/body on failure', async () => { const { handle } = makeHandle({ sessionEmail: 'me@example.test', diff --git a/packages/plugins/plugin-auth/src/send-verification-email.ts b/packages/plugins/plugin-auth/src/send-verification-email.ts index abdc894885..14d37eb37c 100644 --- a/packages/plugins/plugin-auth/src/send-verification-email.ts +++ b/packages/plugins/plugin-auth/src/send-verification-email.ts @@ -31,6 +31,7 @@ import type { AuthRequestHandler } from './register-sso-provider.js'; import { inProcessRedispatchUrl } from './in-process-redispatch.js'; +import { readJsonRequestBody } from './request-json-body.js'; export interface ResendVerificationEmailResult { /** HTTP status to return to the caller. */ @@ -89,12 +90,15 @@ export async function runResendVerificationEmail( handle: AuthRequestHandler, request: Request, ): Promise { - let body: any; - try { - body = await request.json(); - } catch { - body = {}; + // [#24032] A declared-JSON body that does not parse is refused before the + // session is read. Read as `{}`, it was the one-click resend: a truncated + // body naming another address mailed the CALLER's own. An empty body is + // still that resend (see request-json-body.ts). + const reading = await readJsonRequestBody(request); + if (!reading.ok) { + return { status: 400, body: { success: false, error: { code: 'INVALID_REQUEST', message: reading.message } } }; } + const body: any = reading.body; let email = trimStr(body?.email); const callbackURL = trimStr(body?.callbackURL); From dccf68bae164b63726128b35ceb9b4ea33927406 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 20:43:39 +0000 Subject: [PATCH 2/4] chore(changeset): plugin-auth body refusal, an accept-set narrowing shipped as minor Claude-Session: https://claude.ai/code/session_018VqJPGA1NDNSCZsnecsChP Co-authored-by: Claude --- .changeset/24032-plugin-auth-body-refusal.md | 28 ++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 .changeset/24032-plugin-auth-body-refusal.md diff --git a/.changeset/24032-plugin-auth-body-refusal.md b/.changeset/24032-plugin-auth-body-refusal.md new file mode 100644 index 0000000000..9558e5e71e --- /dev/null +++ b/.changeset/24032-plugin-auth-body-refusal.md @@ -0,0 +1,28 @@ +--- +'@objectstack/plugin-auth': minor +--- + +Clause-②: no (narrowing) + +fix(plugin-auth)!: nine of this package's own routes refuse a declared JSON body that does not parse with `400 INVALID_REQUEST`, instead of answering as if no body was sent + + + +**BREAKING** (an accept-set narrowing), shipped as `minor` under the launch-window convention for breaking changes. + +These routes read their bodies as `try { body = await request.json() } catch { body = {} }`, so a truncated body was answered as if the client had sent none: + +- `POST /api/v1/auth/admin/oauth2/toggle-disabled`, `/admin/unlock-user` and `/sys-oauth-application/register`; +- `POST /api/v1/auth/admin/import-users`; +- `POST /api/v1/auth/admin/sso/register`, `/admin/sso/register-saml`, `/admin/sso/request-domain-verification` and `/admin/sso/verify-domain`; +- `POST /api/v1/auth/send-verification-email`. + +`/admin/unlock-user` answered `400 "userId is required"` for a body that carried `userId` and lacked its closing brace. `/send-verification-email` read the same input as the one-click resend, and sent the link to the caller's own address for a body that named another one. + +What changes for an integrator. A body sent as `application/json` (matched case-insensitively) that is not blank and does not parse: FROM the route's answer for an empty body, TO `400` with `{ "success": false, "error": { "code": "INVALID_REQUEST", "message": "The request body could not be parsed as JSON (it was sent as application/json). It was refused rather than read as an empty body: send a well-formed JSON document, or no body at all." } }`, the answer the HTTP transports give the same body. A declared JSON body whose stream cannot be read is refused with the same code and "The request body could not be read. It was refused rather than read as an empty body." Each route refuses where it reads its body: the identity import and the four SSO bridges after their platform-admin gate, the other four before anything else runs. Nothing is looked up, written, sent or forwarded to better-auth. + +The exported bridges six of these routes mount (`runAdminImportUsers`, `runRegisterSsoProviderFromForm`, `runRegisterSamlProviderFromForm`, `runRequestDomainVerification`, `runVerifyDomain` and `runResendVerificationEmail`) answer the same refusal, so a host that mounts them itself refuses it too. + +**Remedy, in one line.** Send a JSON body as well-formed JSON with `Content-Type: application/json`, or send no body. + +**Unchanged.** An empty body (no body at all, a zero-byte one, or whitespace only) is still no body, and every route answers it as before: `/admin/unlock-user` with `400 "userId is required"`, and `/send-verification-email` as the one-click resend to the caller's own address. A body that parses is read as before, whatever its `Content-Type`, and a body that does not declare JSON and does not parse is still read as no body. From e71777cdfddf34d397f5269267427dd8b4eeec09 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 21:39:31 +0000 Subject: [PATCH 3/4] fix(plugin-auth)!: the seven parseJson-spelled auth routes refuse an unparseable declared-JSON body too ban-user, unban-user, set-user-manager, create-user, set-user-password, organization/add-member and set-initial-password read a body that does not parse as {} through a module-private parseJson (or parsed = {}), and answered it with a required-field refusal for a field the body carried. Each reader now goes through the package-local readJsonRequestBody and refuses a declared application/json body that is not blank and does not parse with 400 INVALID_REQUEST. A well-formed non-object body still reads as no fields, and an empty body keeps its answer. The changeset lists the sixteen routes and the eleven exported bridges they mount. Claude-Session: https://claude.ai/code/session_018VqJPGA1NDNSCZsnecsChP Co-authored-by: Claude --- .changeset/24032-plugin-auth-body-refusal.md | 17 ++- .../plugin-auth/src/admin-ban-endpoints.ts | 30 +++-- .../plugin-auth/src/admin-set-user-manager.ts | 26 ++-- .../plugin-auth/src/admin-user-endpoints.ts | 30 +++-- .../src/organization-add-member.ts | 25 ++-- .../src/request-json-body-refusal.test.ts | 111 +++++++++++++++++- .../plugin-auth/src/request-json-body.ts | 8 +- .../plugin-auth/src/set-initial-password.ts | 15 ++- 8 files changed, 211 insertions(+), 51 deletions(-) diff --git a/.changeset/24032-plugin-auth-body-refusal.md b/.changeset/24032-plugin-auth-body-refusal.md index 9558e5e71e..fd1bad8791 100644 --- a/.changeset/24032-plugin-auth-body-refusal.md +++ b/.changeset/24032-plugin-auth-body-refusal.md @@ -4,24 +4,29 @@ Clause-②: no (narrowing) -fix(plugin-auth)!: nine of this package's own routes refuse a declared JSON body that does not parse with `400 INVALID_REQUEST`, instead of answering as if no body was sent +fix(plugin-auth)!: sixteen of this package's own routes refuse a declared JSON body that does not parse with `400 INVALID_REQUEST`, instead of answering as if no body was sent - + **BREAKING** (an accept-set narrowing), shipped as `minor` under the launch-window convention for breaking changes. -These routes read their bodies as `try { body = await request.json() } catch { body = {} }`, so a truncated body was answered as if the client had sent none: +These routes read a body that does not parse as no body, through `try { body = await request.json() } catch { body = {} }` or a module-private `parseJson` that answered `{}` from its `catch`, so a truncated body was answered as if the client had sent none: - `POST /api/v1/auth/admin/oauth2/toggle-disabled`, `/admin/unlock-user` and `/sys-oauth-application/register`; - `POST /api/v1/auth/admin/import-users`; - `POST /api/v1/auth/admin/sso/register`, `/admin/sso/register-saml`, `/admin/sso/request-domain-verification` and `/admin/sso/verify-domain`; -- `POST /api/v1/auth/send-verification-email`. +- `POST /api/v1/auth/send-verification-email`; +- `POST /api/v1/auth/admin/ban-user` and `/admin/unban-user`; +- `POST /api/v1/auth/admin/set-user-manager`; +- `POST /api/v1/auth/admin/create-user` and `/admin/set-user-password`; +- `POST /api/v1/auth/organization/add-member`; +- `POST /api/v1/auth/set-initial-password`. `/admin/unlock-user` answered `400 "userId is required"` for a body that carried `userId` and lacked its closing brace. `/send-verification-email` read the same input as the one-click resend, and sent the link to the caller's own address for a body that named another one. -What changes for an integrator. A body sent as `application/json` (matched case-insensitively) that is not blank and does not parse: FROM the route's answer for an empty body, TO `400` with `{ "success": false, "error": { "code": "INVALID_REQUEST", "message": "The request body could not be parsed as JSON (it was sent as application/json). It was refused rather than read as an empty body: send a well-formed JSON document, or no body at all." } }`, the answer the HTTP transports give the same body. A declared JSON body whose stream cannot be read is refused with the same code and "The request body could not be read. It was refused rather than read as an empty body." Each route refuses where it reads its body: the identity import and the four SSO bridges after their platform-admin gate, the other four before anything else runs. Nothing is looked up, written, sent or forwarded to better-auth. +What changes for an integrator. A body sent as `application/json` (matched case-insensitively) that is not blank and does not parse: FROM the route's answer for an empty body, TO `400` with `{ "success": false, "error": { "code": "INVALID_REQUEST", "message": "The request body could not be parsed as JSON (it was sent as application/json). It was refused rather than read as an empty body: send a well-formed JSON document, or no body at all." } }`, the answer the HTTP transports give the same body. A declared JSON body whose stream cannot be read is refused with the same code and "The request body could not be read. It was refused rather than read as an empty body." Each route refuses where it reads its body. The import, the four SSO bridges, ban/unban, set-user-manager, create-user, set-user-password and add-member read it after their platform-admin gate (and after the admin-plugin or organization-plugin check where the route has one). toggle-disabled, unlock-user, sys-oauth-application/register, the resend and set-initial-password read it before anything else runs. Nothing is looked up, written, sent or forwarded to better-auth. -The exported bridges six of these routes mount (`runAdminImportUsers`, `runRegisterSsoProviderFromForm`, `runRegisterSamlProviderFromForm`, `runRequestDomainVerification`, `runVerifyDomain` and `runResendVerificationEmail`) answer the same refusal, so a host that mounts them itself refuses it too. +The exported bridges eleven of these routes mount (`runAdminImportUsers`, `runRegisterSsoProviderFromForm`, `runRegisterSamlProviderFromForm`, `runRequestDomainVerification`, `runVerifyDomain`, `runResendVerificationEmail`, `runSetInitialPassword`, `runAdminCreateUser`, `runAdminSetUserPassword`, `runAdminBanUser` and `runAdminUnbanUser`) answer the same refusal, so a host that mounts them itself refuses it too. Their signatures and result types are unchanged. **Remedy, in one line.** Send a JSON body as well-formed JSON with `Content-Type: application/json`, or send no body. diff --git a/packages/plugins/plugin-auth/src/admin-ban-endpoints.ts b/packages/plugins/plugin-auth/src/admin-ban-endpoints.ts index caa962d632..7121911dad 100644 --- a/packages/plugins/plugin-auth/src/admin-ban-endpoints.ts +++ b/packages/plugins/plugin-auth/src/admin-ban-endpoints.ts @@ -75,6 +75,7 @@ import { } from './last-local-credential.js'; import type { AdminActor, EndpointResult } from './admin-user-endpoints.js'; import { applyUserBan, applyUserUnban } from './user-ban-write.js'; +import { readJsonRequestBody } from './request-json-body.js'; /** * Minimal better-auth `$context` surface these two routes touch. Mirrors what @@ -103,13 +104,19 @@ const notFound = (): EndpointResult => ({ body: { success: false, error: { code: 'RESOURCE_NOT_FOUND', message: 'User not found' } }, }); -async function parseJson(request: Request): Promise> { - try { - const parsed = await request.json(); - return parsed && typeof parsed === 'object' ? (parsed as Record) : {}; - } catch { - return {}; - } +/** + * [#24032] The body, or the refusal message for a declared-JSON body that does + * not parse: it was read as `{}`, which answered "userId is required" for a + * body that carried `userId` (see request-json-body.ts). A well-formed body + * that is not an object still reads as no fields, as before. + */ +async function parseJson( + request: Request, +): Promise<{ ok: true; body: Record } | { ok: false; message: string }> { + const reading = await readJsonRequestBody(request); + if (!reading.ok) return reading; + const parsed = reading.body; + return { ok: true, body: parsed && typeof parsed === 'object' ? (parsed as Record) : {} }; } /** @@ -128,7 +135,9 @@ export async function runAdminBanUser( actor: AdminActor, request: Request, ): Promise { - const body = await parseJson(request); + const read = await parseJson(request); + if (!read.ok) return invalid(read.message); + const body = read.body; const userId = readUserId(body); if (!userId) return invalid('userId is required'); @@ -191,8 +200,9 @@ export async function runAdminUnbanUser( _actor: AdminActor, request: Request, ): Promise { - const body = await parseJson(request); - const userId = readUserId(body); + const read = await parseJson(request); + if (!read.ok) return invalid(read.message); + const userId = readUserId(read.body); if (!userId) return invalid('userId is required'); const ctx = await deps.getAuthContext(); diff --git a/packages/plugins/plugin-auth/src/admin-set-user-manager.ts b/packages/plugins/plugin-auth/src/admin-set-user-manager.ts index 513ad5d0ac..42365a1809 100644 --- a/packages/plugins/plugin-auth/src/admin-set-user-manager.ts +++ b/packages/plugins/plugin-auth/src/admin-set-user-manager.ts @@ -122,6 +122,7 @@ import { authSystemWriteContext } from './auth-actor-attribution.js'; import type { AdminActor, EndpointResult } from './admin-user-endpoints.js'; +import { readJsonRequestBody } from './request-json-body.js'; /** * Hard cap on the reporting chain this endpoint will create, counted in links @@ -214,13 +215,20 @@ function envelope(refusal: SetUserManagerRefusal): SetUserManagerResult { }; } -async function parseJson(request: Request): Promise> { - try { - const parsed = await request.json(); - return parsed && typeof parsed === 'object' ? (parsed as Record) : {}; - } catch { - return {}; - } +/** + * [#24032] The body, or the refusal message for a declared-JSON body that does + * not parse: it was read as `{}`, which answered "userId is required" for a + * body that carried `userId` (see request-json-body.ts). Refused as the + * `invalid_body` row of the table above. A well-formed body that is not an + * object still reads as no fields, as before. + */ +async function parseJson( + request: Request, +): Promise<{ ok: true; body: Record } | { ok: false; message: string }> { + const reading = await readJsonRequestBody(request); + if (!reading.ok) return reading; + const parsed = reading.body; + return { ok: true, body: parsed && typeof parsed === 'object' ? (parsed as Record) : {} }; } /** @@ -361,7 +369,9 @@ export async function runSetUserManager( actor: AdminActor, request: Request, ): Promise { - const body = await parseJson(request); + const read = await parseJson(request); + if (!read.ok) return envelope(deny(400, 'INVALID_REQUEST', 'invalid_body', read.message)); + const body = read.body; const rawUserId = readId(body, 'userId', 'user_id'); if (typeof rawUserId !== 'string' || rawUserId.length === 0) { diff --git a/packages/plugins/plugin-auth/src/admin-user-endpoints.ts b/packages/plugins/plugin-auth/src/admin-user-endpoints.ts index 965e4614b8..f851ded38f 100644 --- a/packages/plugins/plugin-auth/src/admin-user-endpoints.ts +++ b/packages/plugins/plugin-auth/src/admin-user-endpoints.ts @@ -174,6 +174,7 @@ import { generatePlaceholderEmail } from './placeholder-email.js'; import { revokeSessionsAndApiKeys, type ForcedResetRevocationOutcome } from './forced-reset-revocation.js'; import { reconcileMembership, type MembershipPolicy } from './reconcile-membership.js'; import { resolveDefaultOrgId } from './tenancy-service.js'; +import { readJsonRequestBody } from './request-json-body.js'; const SYSTEM_CTX = { isSystem: true, positions: [], permissions: [] }; @@ -214,13 +215,20 @@ export function generateTemporaryPassword(length = 16): string { // ── Shared helpers ─────────────────────────────────────────────────────── -async function parseJson(request: Request): Promise> { - try { - const parsed = await request.json(); - return parsed && typeof parsed === 'object' ? (parsed as Record) : {}; - } catch { - return {}; - } +/** + * [#24032] The body, or the refusal message for a declared-JSON body that does + * not parse: it was read as `{}`, which answered "userId is required" (or + * "Either email or phoneNumber is required") for a body that carried the field + * (see request-json-body.ts). A well-formed body that is not an object still + * reads as no fields, as before. + */ +async function parseJson( + request: Request, +): Promise<{ ok: true; body: Record } | { ok: false; message: string }> { + const reading = await readJsonRequestBody(request); + if (!reading.ok) return reading; + const parsed = reading.body; + return { ok: true, body: parsed && typeof parsed === 'object' ? (parsed as Record) : {} }; } function badRequest(message: string): EndpointResult { @@ -551,7 +559,9 @@ export async function runAdminCreateUser( request: Request, actor: AdminActor, ): Promise { - const body = await parseJson(request); + const read = await parseJson(request); + if (!read.ok) return badRequest(read.message); + const body = read.body; // #2766 V1.5 — identity: a real email, a phone number, or both. Phone-only // users get a generated placeholder address (better-auth requires a unique @@ -680,7 +690,9 @@ export async function runAdminSetUserPassword( request: Request, actor: AdminActor, ): Promise { - const body = await parseJson(request); + const read = await parseJson(request); + if (!read.ok) return badRequest(read.message); + const body = read.body; const userId = body.userId ?? body.user_id; if (typeof userId !== 'string' || userId.length === 0) { diff --git a/packages/plugins/plugin-auth/src/organization-add-member.ts b/packages/plugins/plugin-auth/src/organization-add-member.ts index 124215441d..470901e7d7 100644 --- a/packages/plugins/plugin-auth/src/organization-add-member.ts +++ b/packages/plugins/plugin-auth/src/organization-add-member.ts @@ -72,6 +72,7 @@ import { inProcessSessionReadInput } from '@objectstack/types'; import { mapAuthApiError, type EndpointResult } from './admin-user-endpoints.js'; +import { readJsonRequestBody } from './request-json-body.js'; /** Minimal better-auth server-api surface this route drives. */ export interface AddMemberCapableApi { @@ -97,13 +98,19 @@ const badRequest = (message: string): EndpointResult => ({ body: { success: false, error: { code: 'INVALID_REQUEST', message } }, }); -async function parseJson(request: Request): Promise> { - try { - const parsed = await request.json(); - return parsed && typeof parsed === 'object' ? (parsed as Record) : {}; - } catch { - return {}; - } +/** + * [#24032] The body, or the refusal message for a declared-JSON body that does + * not parse: it was read as `{}`, which answered "userId is required" for a + * body that carried `userId` (see request-json-body.ts). A well-formed body + * that is not an object still reads as no fields, as before. + */ +async function parseJson( + request: Request, +): Promise<{ ok: true; body: Record } | { ok: false; message: string }> { + const reading = await readJsonRequestBody(request); + if (!reading.ok) return reading; + const parsed = reading.body; + return { ok: true, body: parsed && typeof parsed === 'object' ? (parsed as Record) : {} }; } /** @@ -155,7 +162,9 @@ export async function runOrganizationAddMember( }; } - const body = await parseJson(request); + const read = await parseJson(request); + if (!read.ok) return badRequest(read.message); + const body = read.body; const userId = readString(body, 'userId', 'user_id'); if (!userId) return badRequest('userId is required'); const role = readRole(body); diff --git a/packages/plugins/plugin-auth/src/request-json-body-refusal.test.ts b/packages/plugins/plugin-auth/src/request-json-body-refusal.test.ts index 6f8222eb06..1ad0a50e5f 100644 --- a/packages/plugins/plugin-auth/src/request-json-body-refusal.test.ts +++ b/packages/plugins/plugin-auth/src/request-json-body-refusal.test.ts @@ -21,7 +21,12 @@ * sys-oauth-application/register; * B. the identity import (`admin-import-users.ts`); * C. the four `/admin/sso/*` bridges (`register-sso-provider.ts`); - * D. the self-service resend (`send-verification-email.ts`). + * D. the self-service resend (`send-verification-email.ts`); + * E–I. the routes whose reader was a module-private `parseJson` that answered + * `{}` from its `catch` — ban/unban (`admin-ban-endpoints.ts`), + * set-user-manager, create-user/set-user-password + * (`admin-user-endpoints.ts`), organization/add-member — and + * `set-initial-password` (`parsed = {}`). * * Each refusal asserts the envelope, the code, the message's first sentence * (the message family IS this card's contract), that the message does not echo @@ -38,8 +43,12 @@ import { AuthPlugin } from './auth-plugin.js'; // The import route loads this module with a dynamic `import()` on its first // request, and that first load (`@objectstack/rest`'s import framework among // it) measured 8.7s inside a 10s clocked test. Loaded here, it is paid during -// collection, and the route's own `import()` reads the module cache. +// collection, and the route's own `import()` reads the module cache. The same +// holds for the three modules the E–I routes load on first request. import './admin-import-users.js'; +import './admin-user-endpoints.js'; +import './admin-set-user-manager.js'; +import './organization-add-member.js'; const BASE = '/api/v1/auth'; const ORIGIN = 'http://localhost:3000'; @@ -57,8 +66,15 @@ const acts = { engine: [] as string[], vendor: [] as string[], sent: [] as unknown[], + /** better-auth server-api calls and auth-context reads (the E–I routes' next steps). */ + authApi: [] as string[], }; +const apiCall = (name: string) => vi.fn(async () => { + acts.authApi.push(name); + return {}; +}); + const engineOp = (op: string) => vi.fn(async (object: string) => { acts.engine.push(`${op} ${object}`); return op === 'find' ? [] : op === 'count' ? 0 : null; @@ -113,7 +129,16 @@ beforeAll(async () => { getApi: async () => ({ getSession: async () => ({ user: { id: 'usr_platform_admin', email: SESSION_EMAIL, isPlatformAdmin: true } }), createOAuthClient: acts.createOAuthClient, + // Present so the capability checks admit the E–I routes; recorded if called. + createUser: apiCall('createUser'), + banUser: apiCall('banUser'), + addMember: apiCall('addMember'), + setPassword: apiCall('setPassword'), }), + getAuthContext: async () => { + acts.authApi.push('getAuthContext'); + throw new Error('the auth context is not part of this harness'); + }, getDataEngine: () => engine, unlockUser: acts.unlockUser, isSsoDomainVerificationEnabled: () => true, @@ -134,6 +159,7 @@ beforeEach(() => { acts.engine.length = 0; acts.vendor.length = 0; acts.sent.length = 0; + acts.authApi.length = 0; }); const post = (path: string, init: { headers?: Record; body?: BodyInit }) => @@ -173,6 +199,7 @@ const nothingActed = () => { expect(acts.engine).toEqual([]); expect(acts.vendor).toEqual([]); expect(acts.sent).toEqual([]); + expect(acts.authApi).toEqual([]); }; // ── A. the inline mounts in auth-plugin.ts ───────────────────────────────── @@ -345,3 +372,83 @@ describe('D. /send-verification-email refuses a declared-JSON body that does not }); } }); + +// ── E–I. the second spelling: a reader that answered `{}` from its `catch` ─ + +interface SecondSpellingRoute { + path: string; + truncated: string; + echo: string; + empty: string; + /** `admin-set-user-manager.ts` discriminates its refusals by `error.details.reason`. */ + reason?: string; +} + +const SECOND_SPELLING: Array<{ reader: string; routes: SecondSpellingRoute[] }> = [ + { + reader: 'E. admin-ban-endpoints.ts parseJson', + routes: [ + { path: '/admin/ban-user', truncated: '{"userId": "usr_pin_target", "banReason": "spam"', echo: 'usr_pin_target', empty: 'userId is required' }, + { path: '/admin/unban-user', truncated: '{"userId": "usr_pin_target"', echo: 'usr_pin_target', empty: 'userId is required' }, + ], + }, + { + reader: 'F. admin-set-user-manager.ts parseJson', + routes: [ + { + path: '/admin/set-user-manager', + truncated: '{"userId": "usr_pin_target", "managerId": "usr_pin_boss"', + echo: 'usr_pin_target', + empty: 'userId is required', + reason: 'invalid_body', + }, + ], + }, + { + reader: 'G. admin-user-endpoints.ts parseJson', + routes: [ + { path: '/admin/create-user', truncated: '{"email": "pin-new@example.test", "name": "Pin New"', echo: 'pin-new@example.test', empty: 'Either email or phoneNumber is required' }, + { path: '/admin/set-user-password', truncated: '{"userId": "usr_pin_target", "newPassword": "Pin-Passw0rd!"', echo: 'usr_pin_target', empty: 'userId is required' }, + ], + }, + { + reader: 'H. organization-add-member.ts parseJson', + routes: [ + { path: '/organization/add-member', truncated: '{"userId": "usr_pin_target", "role": "member", "organizationId": "org_pin"', echo: 'usr_pin_target', empty: 'userId is required' }, + ], + }, + { + reader: 'I. set-initial-password.ts (parsed = {})', + routes: [ + { path: '/set-initial-password', truncated: '{"newPassword": "Pin-Passw0rd!"', echo: 'Pin-Passw0rd!', empty: 'newPassword is required' }, + ], + }, +]; + +for (const { reader, routes } of SECOND_SPELLING) { + describe(`${reader} refuses a declared-JSON body that does not parse`, () => { + for (const route of routes) { + it(`${route.path}: a truncated body is 400 INVALID_REQUEST, and nothing after the read runs`, async () => { + const res = await post(route.path, { headers: JSON_TYPE, body: route.truncated }); + if (route.reason) { + const peek = (await res.clone().json()) as { error?: { details?: { reason?: string } } }; + expect(peek.error?.details?.reason).toBe(route.reason); + } + await expectParseRefusal(res, route.echo); + nothingActed(); + }); + + for (const [label, init] of EMPTY_BODIES) { + it(`${route.path}: CONTROL — ${label} keeps today's answer, 400 "${route.empty}"`, async () => { + const res = await post(route.path, init); + if (route.reason) { + const peek = (await res.clone().json()) as { error?: { details?: { reason?: string } } }; + expect(peek.error?.details?.reason).toBe(route.reason); + } + await expectAnswer(res, 400, 'INVALID_REQUEST', route.empty); + nothingActed(); + }); + } + } + }); +} diff --git a/packages/plugins/plugin-auth/src/request-json-body.ts b/packages/plugins/plugin-auth/src/request-json-body.ts index 71516e4a3c..47763441e8 100644 --- a/packages/plugins/plugin-auth/src/request-json-body.ts +++ b/packages/plugins/plugin-auth/src/request-json-body.ts @@ -8,9 +8,11 @@ * * ## The defect * - * Nine body readers here were spelled - * `try { body = await request.json() } catch { body = {} }` (three of them over - * `c.req.json()`), so a truncated or garbled body was read exactly like no + * Fourteen body readers here, serving sixteen routes, read a parse failure as + * `{}`: nine spelled `try { body = await request.json() } catch { body = {} }` + * (three of them over `c.req.json()`), four module-private `parseJson` helpers + * that answered `{}` from their `catch`, and `runSetInitialPassword`'s + * `parsed = {}`. So a truncated or garbled body was read exactly like no * body, and the route answered for a request the client never sent. As the * card measured it, on a fresh `objectstack dev` showcase boot with an admin * session: `POST /api/v1/auth/admin/unlock-user` with `{"userId": "usr_nobody"` (the diff --git a/packages/plugins/plugin-auth/src/set-initial-password.ts b/packages/plugins/plugin-auth/src/set-initial-password.ts index 5904711366..d6a2d2c4b5 100644 --- a/packages/plugins/plugin-auth/src/set-initial-password.ts +++ b/packages/plugins/plugin-auth/src/set-initial-password.ts @@ -22,6 +22,7 @@ */ import { inProcessSessionReadInput } from '@objectstack/types'; +import { readJsonRequestBody } from './request-json-body.js'; /** * Minimal shape of the better-auth server API we depend on. `query` carries @@ -50,12 +51,16 @@ export async function runSetInitialPassword( authApi: SetPasswordCapableApi, request: Request, ): Promise { - let parsed: unknown; - try { - parsed = await request.json(); - } catch { - parsed = {}; + // [#24032] A declared-JSON body that does not parse is refused before + // `setPassword` is reached: it was read as `{}`, which answered + // "newPassword is required" for a body that carried `newPassword` (see + // request-json-body.ts). Same status and code as that answer; only the + // message differs, and a body that parses is read exactly as before. + const reading = await readJsonRequestBody(request); + if (!reading.ok) { + return { status: 400, body: { success: false, error: { code: 'INVALID_REQUEST', message: reading.message } } }; } + const parsed: unknown = reading.body; const newPassword: unknown = (parsed as { newPassword?: unknown } | null)?.newPassword; if (typeof newPassword !== 'string' || newPassword.length === 0) { return { From 7160ac93d7216d3ab9be492350ae7b9fdaf1d2e5 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 21:53:02 +0000 Subject: [PATCH 4/4] test(plugin-auth): set-initial-password's non-JSON row pins the refusal, not just 400 The row asserted only status 400 and INVALID_REQUEST, which the old parsed = {} swallow also answered ("newPassword is required"), so it could not tell a refused body from one read as empty. It now asserts the parse refusal and that setPassword is never reached, and a second row pins the undeclared text/plain spelling, which is still no body. Claude-Session: https://claude.ai/code/session_018VqJPGA1NDNSCZsnecsChP Co-authored-by: Claude --- .../src/set-initial-password.test.ts | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/packages/plugins/plugin-auth/src/set-initial-password.test.ts b/packages/plugins/plugin-auth/src/set-initial-password.test.ts index 234ee8cb16..be2420867b 100644 --- a/packages/plugins/plugin-auth/src/set-initial-password.test.ts +++ b/packages/plugins/plugin-auth/src/set-initial-password.test.ts @@ -25,11 +25,29 @@ describe('runSetInitialPassword', () => { expect(api.setPassword).not.toHaveBeenCalled(); }); - it('rejects a non-JSON body with 400', async () => { + // [#24032] This row asserted only 400 + INVALID_REQUEST, which the old + // `parsed = {}` swallow also answered ("newPassword is required"), so it + // could not tell a refused body from one read as empty. It now pins the + // refusal itself, and the undeclared spelling, which is still no body. + it('refuses a declared-JSON body that does not parse, before setPassword', async () => { const api: SetPasswordCapableApi = { setPassword: vi.fn() }; const res = await runSetInitialPassword(api, makeRequest('not-json{')); expect(res.status).toBe(400); expect(res.body.error?.code).toBe('INVALID_REQUEST'); + expect(res.body.error?.message.startsWith('The request body could not be parsed as JSON')).toBe(true); + expect(api.setPassword).not.toHaveBeenCalled(); + }); + + it('a body that does not declare JSON and does not parse is still no body', async () => { + const api: SetPasswordCapableApi = { setPassword: vi.fn() }; + const req = new Request('https://example.test/api/v1/auth/set-initial-password', { + method: 'POST', + headers: { 'content-type': 'text/plain', cookie: 'better-auth.session_token=abc' }, + body: 'not-json{', + }); + const res = await runSetInitialPassword(api, req); + expect(res.body).toEqual({ success: false, error: { code: 'INVALID_REQUEST', message: 'newPassword is required' } }); + expect(api.setPassword).not.toHaveBeenCalled(); }); it('forwards newPassword + session headers to better-auth and returns 200 on success', async () => {