Skip to content

Commit f048e42

Browse files
fix(plugin-auth): Create User with no email answers the field-level VALIDATION_FAILED refusal (#28354)
<!-- claude-projects-attribution: {"github_login":"os-project-manager"} --> _Requested by **Leehom** · [project thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1XWd9jEbhqhcm9KSbo6jLGZH)_ Part of #28289 Clause-②: yes (widening: the refusal gains `error.fields`, one `{ field, code, message }` entry per input that would satisfy it, code `required`; its `error.code` moves from `INVALID_REQUEST` to `VALIDATION_FAILED`, the code the same endpoint's permission-set refusal and the data doors already answer with `fields`) Metadata-protocol: none ## 维护者速读 - 改了什么:设置 → 用户 → 创建用户,邮箱(开启手机号登录时为邮箱与手机号)都不填时,服务端报错从通用的 `INVALID_REQUEST` 改为字段级的 `VALIDATION_FAILED`,并带 `fields`:`email`(及 `phoneNumber`)各一条,`code: required`。英文提示不变。 - 为什么改:通用错误码让界面只能显示英文句子;字段级信封让界面能标到输入框、按自己的语言措辞(triage「不新增错误码」)。 - 风险与回滚:只动这一条拒绝;状态码仍 400;其它拒绝(邮箱格式、手机号、权限集、密码长度)原样不变。读 `INVALID_REQUEST` 判断这条拒绝的客户端会看到新码。回滚即还原本 PR。 - 先看:`packages/plugins/plugin-auth/src/admin-user-endpoints.ts`(`requiredFieldsRefusal` 与缺标识分支)、`.changeset/28289-create-user-email-field-refusal.md`。 - 两分钟验证:`pnpm --filter @objectstack/plugin-auth exec vitest run src/admin-user-endpoints.test.ts`,预期全绿,含两条 `VALIDATION_FAILED` 用例与四条对照。 - 需要你做的:无,席位落地。控制台按字段标注并翻译是 ui 车道的后半(见 Acceptance notes),故本 PR 用 `Part of`。 ## What changes `plugin-auth` `admin-user-endpoints.ts`, `POST /api/v1/auth/admin/create-user`: with neither an email nor a phone number, the refusal is the field-level envelope. - Before: `400` `{ code: 'INVALID_REQUEST', message: 'Email is required' }`, or "Email or phone number is required" where phone sign-in is on. - Now: `400` `{ code: 'VALIDATION_FAILED', message, fields }`. `fields` is `[{ field: 'email', code: 'required', message }]`, plus `{ field: 'phoneNumber', code: 'required', message }` where phone sign-in is on. The English `message` is unchanged. - Built by a new private `requiredFieldsRefusal(message, fields)` beside `badRequest()`. Its entry code `required` is checked against `FieldErrorCode` (the closed ADR-0114 catalog, `@objectstack/spec/api`) with `satisfies`, so no code is invented. The code is written in the entry literal, not passed as a parameter: as a parameter, `check:dispatcher-error-vocabulary` read it as a stamped ADR-0112 code (`objlithelper` shape) and went red; fixed in f2bc72d. - Every other refusal of the endpoint still goes through `badRequest()` and answers `INVALID_REQUEST` as before. ### Which field envelope, measured (the triage named `VALIDATION_ERROR` with `issues[]`) The order said to use the field-level shape that already exists for this door, and not to invent a second one. At `origin/main` b339465: - The spec declares no `issues` list on the error envelope. Its per-field list is `fields` (`EnhancedApiErrorSchema.fields`, entries `FieldErrorSchema`, in `packages/spec/src/api/errors.zod.ts`; ADR-0114 D4 renamed `fieldErrors` to `fields` because `fields` is what the wire carries). On the wire, `issues` is a thrown error's raw Zod issues passed through (`@objectstack/rest` `error-response.ts`, one `batch-routes.ts` refusal, the metadata save door that the console's metadata editors read). It is not the record/form field shape. - This same endpoint already answers that envelope: `grantRefusal()` passes a permission-set refusal through as `VALIDATION_FAILED` with `error.fields` (`EndpointResult.body.error.fields`). `VALIDATION_FAILED` is in the error-code ledger under `@objectstack/plugin-auth`, so this adds no ledger row. - The data doors answer the same pair: `@objectstack/rest` `crud-routes.ts` and `form-routes.ts` (`validationFailure`, `400 VALIDATION_FAILED` with `fields[]`). - The console's field-error reader, `extractFieldErrors` (`ui/packages/react/src/utils/error-message.ts`), reads `error.fields`, `error.details.fields` or `validationErrors`, whatever the code. So the refusal uses `VALIDATION_FAILED` + `error.fields`, the shape of the permission-set refusal one branch below it. ## Tests - `admin-user-endpoints.test.ts`: the #27857 pair (phone sign-in off and on) now asserts `400`, `VALIDATION_FAILED`, the unchanged English message, and `fields` exactly. Four new CONTROL cases pin that the refusals beside it keep `{ code: 'INVALID_REQUEST', message }` with no `fields`: a malformed email, a phone number with phone sign-in off, a malformed phone number, a `permissionSet` that is not a name. A valid Create User is pinned by the suite's existing success cases. - `request-json-body-refusal.test.ts` (the real Hono route): the three empty-body controls for `/admin/create-user` now expect `VALIDATION_FAILED` and `fields: [{ field: 'email', code: 'required', message: 'Email is required' }]`, through new optional `emptyCode` / `emptyFields` keys on the route table. `/admin/set-user-password` and every other route keep `INVALID_REQUEST`. <details><summary>Red first, on the base implementation</summary> `git restore --source=b3394651 -- packages/plugins/plugin-auth/src/admin-user-endpoints.ts` with the new tests, then `vitest run src/admin-user-endpoints.test.ts src/request-json-body-refusal.test.ts`: Test Files 2 failed (2), Tests 5 failed, 165 passed (170). The 5 are the two `VALIDATION_FAILED` cases (`expected 'INVALID_REQUEST' to be 'VALIDATION_FAILED'`) and the three real-route empty-body cases. The four controls stayed green. Restored with `git checkout HEAD --`; `git diff HEAD` empty, blob equal to `HEAD`. </details> <details><summary>Green, and the ablation</summary> - `pnpm --filter @objectstack/plugin-auth test` (head f2bc72d): Test Files 205 passed (205), Tests 3921 passed, 10 skipped (3931). Lock VERDICT command-exit 0. - `pnpm --filter @objectstack/plugin-auth typecheck` (head f2bc72d, after building the package itself, which `tsconfig.examples.json` resolves): exit 0; `check:test-typecheck` OK, the debt ledger unchanged. - Ablation through `scripts/ablation-replace.mjs` on head f2bc72d: the `requiredFieldsRefusal(...)` return replaced by `return badRequest(message);` (anchor 1 to 0, blob `1715c5359286` to `f25398fd46e8`) reds the same 5 cases, 165 passed. Restored by the tool, blob after restore `1715c5359286` equal to `HEAD`, `git diff HEAD` empty. (The same ablation on the first head, 0c1a502, read the same 5.) The subject is the package's own `src/`, imported by relative path, so no build is in the path. - `eslint --no-inline-config packages/plugins/plugin-auth`: exit 0. </details> ## Acceptance notes - **Why `Part of`.** The console does not yet use the new `fields`. Create User is the `create_user` api action on `sys_user`. A refused run reaches `useConsoleActionRuntime`'s `refused()`, then `apiActionFailureMessage`, then `actionErrorDetail`, which reads only `error.message`. `ActionParamDialog` shows that text above the buttons ("ActionResult carries the refusal as text only"). So today the dialog shows the same English sentence as before. The per-field mark and its translation are the ui lane's half: `Seam: spec:EnhancedApiErrorSchema.fields → renderer:ActionParamDialog (ui/packages/app-shell/src/views/ActionParamDialog.tsx; useConsoleActionRuntime.tsx refused() → flowResponse.ts apiActionFailureMessage → actionErrorDetail reads error.message only)`. - **Who words the field message.** `FieldErrorSchema.message` is described as rendered in the caller's locale. This door renders English only, like the rest of plugin-auth's refusals. Two routes for the translated sentence: the dialog words `code: 'required'` from its own language pack (it already words its client-side required check), or the server renders `required` through `renderValidationMessage` (`@objectstack/spec/system`) in the caller's locale, which would need this door to read the request locale and the translated field label. Left to the ui-lane card; the report recommends the first. - **Same family, not changed here.** "A valid email is required" and the two phone-number refusals are also about one input and still answer `INVALID_REQUEST` (pinned as controls, as the order asked). They could take `invalid_email` / `invalid_phone` entries the same way. - **The earlier changeset.** `.changeset/27857-create-user-email-refusal.md` (from #28288, unreleased) says the code `INVALID_REQUEST` is unchanged. This PR does not edit it; its own changeset carries a line that supersedes it. - `Clause-②` is declared as measured. The claim's line said `yes` was expected; `request-json-body-refusal.test.ts` read `INVALID_REQUEST` for this refusal, so the published answer moved and a contract review is owed. ## Verification - Derived gate list, head f2bc72d: `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` named 62 commands; each run with its exit code captured before any pipe. All 62 exit 0, including `check:dual-build-cjs-loads` (113 entry points across 70 packages load) and `check:dispatcher-error-vocabulary` (OK, 56 sites all classified). `--ran` reconciliation: 62 derived, 62 run, 0 NOT-MEASURED, 0 UNRUN. - On the first head, `check:dispatcher-error-vocabulary` went red (`[unclassified-site] … stamps unregistered code 'required' (objlithelper)`), because the helper took the field code as a parameter. f2bc72d writes `code: 'required'` in the entry literal; the gate is green and no ledger row or classification row was added. - `eslint --no-inline-config packages/plugins/plugin-auth`: exit 0. - Overlap: the open pull requests read before opening this one touch no file under `packages/plugins/plugin-auth/src/` by title; #28288, the last change to `admin-user-endpoints.ts`, is merged and is this branch's base. - Not run locally: `test:repo` (CI Test Core), the dogfood battery (the title carries no `!` and `Clause-②` has no `(narrowing)` arm). --- _Generated by [Claude Code](https://claude.ai/code/session_01FEkQ5n7KCBaXqGnnMcHsiD)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 700ec85 commit f048e42

4 files changed

Lines changed: 87 additions & 8 deletions

File tree

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
---
2+
'@objectstack/plugin-auth': patch
3+
---
4+
5+
fix(plugin-auth): Create User with no email answers the field-level refusal, so a client can mark the Email input and word it in its own language (#28289)
6+
7+
Clause-②: yes (widening: the refusal gains `error.fields`, one `{ field, code, message }` entry per input that would satisfy it, code `required`; its `error.code` moves from `INVALID_REQUEST` to `VALIDATION_FAILED`, the code the same endpoint's permission-set refusal and the data doors already answer with `fields`)
8+
Metadata-protocol: none
9+
10+
<!-- adr-0087: not-required (no-migration-prescription) No key of any schema is added, removed, renamed or re-shaped, and no export or type changes: one refusal of `POST /api/v1/auth/admin/create-user` answers an existing code from the plugin's ledger row with the existing `fields` envelope. Nothing an author writes changes, so there is nothing for a migration to rewrite and no tombstone. -->
11+
12+
**BREAKING** wire answer of one refusal at `POST /api/v1/auth/admin/create-user` (`@objectstack/plugin-auth`).
13+
14+
- **Before.** With neither an email nor a phone number, the endpoint answered `400` `{ code: 'INVALID_REQUEST', message: 'Email is required' }` ("Email or phone number is required" where phone sign-in is on). The code is the endpoint's generic one, so a client could only show the English sentence.
15+
- **Now.** The same `400` answers `{ code: 'VALIDATION_FAILED', message, fields }`. `fields` holds `{ field: 'email', code: 'required', message }`, plus `{ field: 'phoneNumber', code: 'required', message }` where phone sign-in is on. The English message is unchanged.
16+
- **Unchanged.** Every other refusal of the endpoint keeps `INVALID_REQUEST` and its message: a malformed email, a phone number without phone sign-in, a malformed phone number, a `permissionSet` that is not a name, the password rules. A valid Create User still succeeds.
17+
- **Supersedes** the line "The status (400) and code (`INVALID_REQUEST`) are unchanged" in this release's earlier Create User changeset: the status is still 400, and the code is now `VALIDATION_FAILED`.
18+
- **Who sees it.** A client that read `INVALID_REQUEST` for this one refusal. The console shows the refusal's message as before; marking the Email input from `fields` is the console's own follow-up.

‎packages/plugins/plugin-auth/src/admin-user-endpoints.test.ts‎

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -269,15 +269,43 @@ describe('runAdminCreateUser', () => {
269269

270270
// #27857: the refusal names what the dialog shows. `phoneNumber` is a body
271271
// key, and the dialog has a Phone Number field only where phone sign-in is
272-
// on. The code and status are the endpoint's contract and do not move.
272+
// on. #28289: it is the field-level refusal, `VALIDATION_FAILED` with one
273+
// `fields[]` entry per input that can satisfy it, code `required`, so a
274+
// client can mark the input and word it in its own language.
273275
for (const phone of [false, true]) {
274276
const expected = phone ? 'Email or phone number is required' : 'Email is required';
275-
it(`phone sign-in ${phone ? 'on' : 'off'}: neither email nor phone answers 400 INVALID_REQUEST "${expected}"`, async () => {
277+
const fields = phone
278+
? [
279+
{ field: 'email', code: 'required', message: expected },
280+
{ field: 'phoneNumber', code: 'required', message: expected },
281+
]
282+
: [{ field: 'email', code: 'required', message: expected }];
283+
it(`phone sign-in ${phone ? 'on' : 'off'}: neither email nor phone answers 400 VALIDATION_FAILED "${expected}" at ${fields.map((f) => f.field).join(' + ')}`, async () => {
276284
const m = makeDeps({ phoneNumberEnabled: () => phone });
277285
const res = await runAdminCreateUser(m.deps, makeRequest({ name: 'x', generatePassword: true }), ACTOR);
278286
expect(res.status).toBe(400);
279-
expect(res.body.error?.code).toBe('INVALID_REQUEST');
287+
expect(res.body.error?.code).toBe('VALIDATION_FAILED');
280288
expect(res.body.error?.message).toBe(expected);
289+
expect(res.body.error?.fields).toEqual(fields);
290+
expect(m.createUser).not.toHaveBeenCalled();
291+
});
292+
}
293+
294+
// #28289 CONTROLS: the refusals beside it keep their answer exactly.
295+
for (const [shape, body, phone, message] of [
296+
['a malformed email', { email: 'not-an-email' }, false, 'A valid email is required'],
297+
['a phone number with phone sign-in off', { phoneNumber: '+8613800000000' }, false,
298+
'Phone numbers require the phoneNumber auth plugin (auth.plugins.phoneNumber)'],
299+
['a malformed phone number', { phoneNumber: 'abc' }, true,
300+
'phoneNumber must be a valid phone number (E.164 recommended, e.g. +8613800000000)'],
301+
['a permissionSet that is not a name', { email: 'a@b.co', permissionSet: 7 }, false,
302+
'permissionSet must be the machine name of a permission set'],
303+
] as const) {
304+
it(`CONTROL: ${shape} still answers 400 INVALID_REQUEST with no fields`, async () => {
305+
const m = makeDeps({ phoneNumberEnabled: () => phone });
306+
const res = await runAdminCreateUser(m.deps, makeRequest({ ...body, generatePassword: true }), ACTOR);
307+
expect(res.status).toBe(400);
308+
expect(res.body.error).toEqual({ code: 'INVALID_REQUEST', message });
281309
expect(m.createUser).not.toHaveBeenCalled();
282310
});
283311
}

‎packages/plugins/plugin-auth/src/admin-user-endpoints.ts‎

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,7 @@ import { cloudManagedRefusal } from './cloud-managed.js';
234234
import { temporaryPasswordLength } from './temporary-password-length.js';
235235
import { isCloudManaged } from '@objectstack/core';
236236
import { resolveThrownHttpError, validationFailureDetails } from '@objectstack/types';
237+
import type { FieldErrorCode } from '@objectstack/spec/api';
237238

238239
const SYSTEM_CTX = { isSystem: true, positions: [], permissions: [] };
239240

@@ -295,6 +296,26 @@ function badRequest(message: string): EndpointResult {
295296
return { status: 400, body: { success: false, error: { code: 'INVALID_REQUEST', message } } };
296297
}
297298

299+
/**
300+
* [#28289] A refusal for inputs left empty: `400 VALIDATION_FAILED` with one
301+
* `fields[]` entry per input, code `required` (the closed ADR-0114 catalog) —
302+
* the envelope the grant refusal passes through and the data doors answer, so
303+
* the console's field-error reader needs no case for this door.
304+
*/
305+
function requiredFieldsRefusal(message: string, fields: string[]): EndpointResult {
306+
return {
307+
status: 400,
308+
body: {
309+
success: false,
310+
error: {
311+
code: 'VALIDATION_FAILED',
312+
message,
313+
fields: fields.map((field) => ({ field, code: 'required' satisfies FieldErrorCode, message })),
314+
},
315+
},
316+
};
317+
}
318+
298319
/**
299320
* Linear-time email plausibility check (local@domain with a dotted domain).
300321
* Deliberately not a regex: the obvious `\S+@\S+\.\S+` is polynomially
@@ -696,9 +717,12 @@ export async function runAdminCreateUser(
696717
if (!hasEmail && !phoneNumber) {
697718
// Named in the dialog's words, not the body's keys: an administrator sees
698719
// an Email field, and a Phone Number field only where phone sign-in is on.
699-
return badRequest(
700-
deps.phoneNumberEnabled?.() ? 'Email or phone number is required' : 'Email is required',
701-
);
720+
// [#28289] Field-level, as the grant refusal below is: one `fields[]`
721+
// entry per input that would satisfy it, so a client marks the input and
722+
// words it in its own language instead of showing this sentence.
723+
const phoneSignIn = deps.phoneNumberEnabled?.() === true;
724+
const message = phoneSignIn ? 'Email or phone number is required' : 'Email is required';
725+
return requiredFieldsRefusal(message, phoneSignIn ? ['email', 'phoneNumber'] : ['email']);
702726
}
703727
const email = hasEmail ? (rawEmail as string).trim() : generatePlaceholderEmail();
704728
const name =

‎packages/plugins/plugin-auth/src/request-json-body-refusal.test.ts‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -380,6 +380,10 @@ interface SecondSpellingRoute {
380380
truncated: string;
381381
echo: string;
382382
empty: string;
383+
/** The empty body's code, where it is not `INVALID_REQUEST` (#28289: Create User's field-level refusal). */
384+
emptyCode?: string;
385+
/** The empty body's `error.fields`, where the refusal is field-level. */
386+
emptyFields?: unknown[];
383387
/** `admin-set-user-manager.ts` discriminates its refusals by `error.details.reason`. */
384388
reason?: string;
385389
}
@@ -407,7 +411,8 @@ const SECOND_SPELLING: Array<{ reader: string; routes: SecondSpellingRoute[] }>
407411
{
408412
reader: 'G. admin-user-endpoints.ts parseJson',
409413
routes: [
410-
{ path: '/admin/create-user', truncated: '{"email": "pin-new@example.test", "name": "Pin New"', echo: 'pin-new@example.test', empty: 'Email is required' },
414+
{ path: '/admin/create-user', truncated: '{"email": "pin-new@example.test", "name": "Pin New"', echo: 'pin-new@example.test', empty: 'Email is required',
415+
emptyCode: 'VALIDATION_FAILED', emptyFields: [{ field: 'email', code: 'required', message: 'Email is required' }] },
411416
{ path: '/admin/set-user-password', truncated: '{"userId": "usr_pin_target", "newPassword": "Pin-Passw0rd!"', echo: 'usr_pin_target', empty: 'userId is required' },
412417
],
413418
},
@@ -445,7 +450,11 @@ for (const { reader, routes } of SECOND_SPELLING) {
445450
const peek = (await res.clone().json()) as { error?: { details?: { reason?: string } } };
446451
expect(peek.error?.details?.reason).toBe(route.reason);
447452
}
448-
await expectAnswer(res, 400, 'INVALID_REQUEST', route.empty);
453+
if (route.emptyFields) {
454+
const peek = (await res.clone().json()) as { error?: { fields?: unknown[] } };
455+
expect(peek.error?.fields).toEqual(route.emptyFields);
456+
}
457+
await expectAnswer(res, 400, route.emptyCode ?? 'INVALID_REQUEST', route.empty);
449458
nothingActed();
450459
});
451460
}

0 commit comments

Comments
 (0)