Skip to content

Commit 3ae3260

Browse files
committed
fix(plugin-auth): the organization require-MFA refusal reads in the caller's language, and an admitted turn-on answers the grace period in force (WIP)
Claude-Session: https://claude.ai/code/session_01Utp6Cai3TgWyncLtR5EwF7 Co-authored-by: Claude <noreply@anthropic.com>
1 parent 8fa738f commit 3ae3260

5 files changed

Lines changed: 414 additions & 14 deletions

File tree

‎packages/plugins/plugin-auth/src/auth-manager-password-session-policy.ts‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,17 @@ export interface PasswordSessionPolicyHost {
7171
resolveActor: AuthManager['resolveActor'];
7272
}
7373

74+
/**
75+
* The MFA grace period in force, in whole days (ADR-0069 D3): the configured
76+
* `mfaGracePeriodDays` (the `auth.mfa_grace_period_days` setting), 7 when
77+
* unset, never below 0. The one reading of it: the gate's clock below and the
78+
* organization MFA guard's save notice (`AuthManager#getMfaGracePeriodDays`)
79+
* both ask here, so the number a saver is told is the number the gate holds to.
80+
*/
81+
export function resolveMfaGracePeriodDays(config: { mfaGracePeriodDays?: number }): number {
82+
return Math.max(0, Math.floor(Number(config.mfaGracePeriodDays ?? 7)));
83+
}
84+
7485
/** The body of `AuthManager#assertPasswordComplexity`; the method's docblock there is its contract. */
7586
export async function assertPasswordComplexity(this: Pick<PasswordSessionPolicyHost, 'config'>, password: string): Promise<void> {
7687
if (!this.config.passwordRequireComplexity) return;
@@ -222,7 +233,7 @@ export async function resolveUserAuthGates(
222233
if (mfaRequired && unenrolled && !this.isTwoFactorEnabled()) {
223234
this.noteMfaRequirementWithoutEnrollment();
224235
} else if (mfaRequired && unenrolled) {
225-
const graceDays = Math.max(0, Math.floor(Number(this.config.mfaGracePeriodDays ?? 7)));
236+
const graceDays = resolveMfaGracePeriodDays(this.config);
226237
let requiredAt = u?.mfa_required_at;
227238
if (!requiredAt) {
228239
requiredAt = new Date();

‎packages/plugins/plugin-auth/src/auth-manager.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ import {
124124
recordPasswordHistory,
125125
refreshMustChangeCacheIfStale,
126126
refreshOrgMfaCacheIfStale,
127+
resolveMfaGracePeriodDays,
127128
resolvePasswordChangeUserId,
128129
resolveUserAuthGates,
129130
stampPasswordChangedAt,
@@ -5313,6 +5314,20 @@ export class AuthManager {
53135314
return resolveTwoFactorEnabled(this.config.plugins);
53145315
}
53155316

5317+
/**
5318+
* @internal Package-internal: read by the organization MFA enrollment guard
5319+
* `AuthPlugin` registers; not a supported entry point.
5320+
*
5321+
* [#30411] The MFA grace period in force, in whole days — the
5322+
* `auth.mfa_grace_period_days` setting as the settings pass applied it,
5323+
* {@link resolveMfaGracePeriodDays} over the live config, the same answer
5324+
* the MFA gate's clock reads. Read per call, so a settings change is seen at
5325+
* once.
5326+
*/
5327+
public getMfaGracePeriodDays(): number {
5328+
return resolveMfaGracePeriodDays(this.config);
5329+
}
5330+
53165331
/**
53175332
* [#30201] Called by the MFA gate when a user is under an MFA requirement
53185333
* (the global `mfaRequired` or their organization's `require_mfa`) that it

‎packages/plugins/plugin-auth/src/auth-plugin.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1914,6 +1914,9 @@ export class AuthPlugin implements Plugin {
19141914
registerOrganizationMfaEnrollmentGuard(engine, {
19151915
packageId: 'com.objectstack.plugin-auth.organization-mfa-enrollment-guard',
19161916
isTwoFactorEnabled: () => this.authManager?.isTwoFactorEnabled() ?? false,
1917+
// [#30411] The grace period an admitted turn-on's save notice names.
1918+
// No manager means two-factor reads off above, so nothing is admitted.
1919+
mfaGracePeriodDays: () => this.authManager?.getMfaGracePeriodDays() ?? Number.NaN,
19171920
});
19181921
// A reader's own named list views: the body is judged on every save as
19191922
// a ListView for the row's own object. See personal-view-hooks.ts.

‎packages/plugins/plugin-auth/src/organization-mfa-enrollment-guard.test.ts‎

Lines changed: 238 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,14 @@ import { AuthManager } from './auth-manager';
3232
import { AuthPlugin } from './auth-plugin.js';
3333
import {
3434
ORGANIZATION_MFA_ENROLLMENT_GUARD_PRIORITY,
35+
ORGANIZATION_MFA_REFUSAL_COPY,
36+
ORGANIZATION_MFA_REQUIRED_NOTICE_COPY,
37+
organizationMfaRequiredNotice,
3538
organizationMfaWithoutEnrollmentRefusal,
3639
registerOrganizationMfaEnrollmentGuard,
3740
turnsMfaRequirementOn,
3841
} from './organization-mfa-enrollment-guard.js';
42+
import { resolveMfaGracePeriodDays } from './auth-manager-password-session-policy.js';
3943
import { registerIdentityWriteGuard, registerManagedUpdateWhitelist } from './identity-write-guard.js';
4044
import { managedExtensionEditableFields } from './managed-extension-fields.js';
4145
// The SAME in-memory engine the sibling two-factor suites drive: a new fake
@@ -82,6 +86,7 @@ async function judge(
8286
registerOrganizationMfaEnrollmentGuard(engine, {
8387
packageId: 'test.organization-mfa-enrollment-guard',
8488
isTwoFactorEnabled: () => enrollable,
89+
mfaGracePeriodDays: () => 7,
8590
});
8691
try {
8792
for (const r of engine.registrations.filter((x) => x.event === event)) {
@@ -99,7 +104,11 @@ async function judge(
99104
describe('organization MFA enrollment guard — the rule', () => {
100105
it('registers insert and update hooks on sys_organization alone, after the D2 identity write guard', () => {
101106
const engine = captureEngine();
102-
registerOrganizationMfaEnrollmentGuard(engine, { packageId: 'p', isTwoFactorEnabled: () => false });
107+
registerOrganizationMfaEnrollmentGuard(engine, {
108+
packageId: 'p',
109+
isTwoFactorEnabled: () => false,
110+
mfaGracePeriodDays: () => 7,
111+
});
103112
expect(engine.registrations.map((r) => r.event).sort()).toEqual(['beforeInsert', 'beforeUpdate']);
104113
for (const r of engine.registrations) {
105114
expect(r.options).toEqual({ object: 'sys_organization', priority: 15, packageId: 'p' });
@@ -218,6 +227,20 @@ describe('auth-plugin registers the guard on `kernel:ready` and asks the live ma
218227
// The settings pass turns the plugin on (as `auth.mfa_required` does): the next write is admitted.
219228
(plugin as any).authManager.applyConfigPatch({ plugins: { twoFactor: true } });
220229
await expect(turnOn()).resolves.toBeUndefined();
230+
231+
// [#30411] …and its save notice names the grace period the live manager
232+
// holds, read per write: the settings pass changes it, the next write says so.
233+
const told: string[] = [];
234+
const turnOnTold = () => update.handler({
235+
object: 'sys_organization', input: { id: 'org_1', data: { require_mfa: true } },
236+
session: { userId: 'usr_admin', locale: 'en-US' }, notice: (m: string) => told.push(m),
237+
});
238+
await turnOnTold();
239+
expect(told).toEqual([organizationMfaRequiredNotice(7, 'en')]);
240+
(plugin as any).authManager.applyConfigPatch({ mfaGracePeriodDays: 14 });
241+
await turnOnTold();
242+
expect(told[1]).toBe(organizationMfaRequiredNotice(14, 'en'));
243+
expect(told[1]).toMatch(/has 14 days to enroll/);
221244
});
222245
});
223246

@@ -233,7 +256,10 @@ const SYSTEM = { context: { isSystem: true, positions: [] as string[], permissio
233256
const ADMIN = { context: { userId: 'usr_admin', positions: [] as string[], permissions: [] as string[] } };
234257

235258
/** A real sqlite engine with the real organization object and the production hook composition. */
236-
async function bootOrganizationEngine(enrollable: { value: boolean }): Promise<ObjectQL> {
259+
async function bootOrganizationEngine(
260+
enrollable: { value: boolean },
261+
graceDays: { value: number } = { value: 7 },
262+
): Promise<ObjectQL> {
237263
const engine = new ObjectQL();
238264
engines.push(engine);
239265
engine.registerDriver(
@@ -248,6 +274,7 @@ async function bootOrganizationEngine(enrollable: { value: boolean }): Promise<O
248274
registerOrganizationMfaEnrollmentGuard(engine, {
249275
packageId: 'test.organization-mfa-enrollment-guard',
250276
isTwoFactorEnabled: () => enrollable.value,
277+
mfaGracePeriodDays: () => graceDays.value,
251278
});
252279
await engine.insert('sys_organization', { id: 'org_a', name: 'Acme', slug: 'acme' }, SYSTEM);
253280
await engine.insert('sys_organization', { id: 'org_b', name: 'Bolt', slug: 'bolt' }, SYSTEM);
@@ -343,6 +370,194 @@ describe('organization MFA enrollment guard — on a real engine', () => {
343370
});
344371
});
345372

373+
// ── #30411. What the saver is told, in the saver's language ─────────────────
374+
375+
/** The first sentence of each copy: the language is the contract, so the sentence that carries it is pinned. */
376+
const REFUSAL_FIRST = {
377+
en: 'This organization cannot require multi-factor authentication yet:',
378+
'zh-CN': '本组织暂时无法强制多因素认证:',
379+
} as const;
380+
const NOTICE_FIRST = {
381+
en: 'Multi-factor authentication is now required for this organization.',
382+
'zh-CN': '本组织现已强制多因素认证。',
383+
} as const;
384+
385+
/** Run the guard's hook for `event` with a session locale and a notice listener; the outcome and what it told. */
386+
async function judgeTold(opts: {
387+
enrollable: boolean;
388+
event: 'beforeInsert' | 'beforeUpdate';
389+
data: Record<string, unknown>;
390+
previous?: Record<string, unknown>;
391+
locale?: string;
392+
graceDays?: number;
393+
noNoticeMember?: boolean;
394+
}): Promise<{ error: any; told: string[] }> {
395+
const engine = captureEngine();
396+
registerOrganizationMfaEnrollmentGuard(engine, {
397+
packageId: 'test.organization-mfa-enrollment-guard',
398+
isTwoFactorEnabled: () => opts.enrollable,
399+
mfaGracePeriodDays: () => opts.graceDays ?? 7,
400+
});
401+
const told: string[] = [];
402+
try {
403+
for (const r of engine.registrations.filter((x) => x.event === opts.event)) {
404+
await r.handler({
405+
object: 'sys_organization', event: opts.event, input: { id: 'org_1', data: opts.data },
406+
...(opts.previous ? { previous: opts.previous } : {}),
407+
...(opts.locale ? { session: { userId: 'usr_admin', locale: opts.locale } } : {}),
408+
...(opts.noNoticeMember ? {} : { notice: (m: string) => told.push(m) }),
409+
});
410+
}
411+
return { error: null, told };
412+
} catch (e) {
413+
return { error: e, told };
414+
}
415+
}
416+
417+
describe('#30411 the refusal is in the caller\'s language, with the same field-level shape', () => {
418+
it('zh-CN: the refusal reads in Chinese and still names the env var and the setting key', () => {
419+
const err: any = organizationMfaWithoutEnrollmentRefusal('zh-CN');
420+
expectRequireMfaRefusal(err);
421+
expect(err.message.startsWith(REFUSAL_FIRST['zh-CN'])).toBe(true);
422+
expect(err.message).toMatch(/OS_AUTH_TWO_FACTOR=true/);
423+
expect(err.message).toMatch(/auth\.mfa_required/);
424+
expect(err.fields[0].message).toBe(err.message);
425+
});
426+
427+
it('en: the refusal is the English sentence it was, top level and on the field', () => {
428+
for (const locale of ['en', 'en-US', undefined]) {
429+
const err: any = organizationMfaWithoutEnrollmentRefusal(locale);
430+
expectRequireMfaRefusal(err);
431+
expect(err.message.startsWith(REFUSAL_FIRST.en)).toBe(true);
432+
expect(err.message).toBe(ORGANIZATION_MFA_REFUSAL_COPY.en);
433+
expect(err.fields[0].message).toBe(err.message);
434+
}
435+
});
436+
437+
it.each([
438+
['zh', 'zh-CN'], ['zh-cn', 'zh-CN'], ['zh-Hans-CN', 'zh-CN'], ['ja', 'ja-JP'], ['es-MX', 'es-ES'],
439+
['fr-FR', 'en'], ['', 'en'],
440+
] as const)('the request locale %j reads the %s copy (exact, then by language, else English)', (asked, served) => {
441+
expect((organizationMfaWithoutEnrollmentRefusal(asked) as any).message).toBe(ORGANIZATION_MFA_REFUSAL_COPY[served]);
442+
});
443+
444+
it('every language names what an operator types, and carries no tracker number', () => {
445+
for (const [locale, message] of Object.entries(ORGANIZATION_MFA_REFUSAL_COPY)) {
446+
expect(message, locale).toMatch(/OS_AUTH_TWO_FACTOR=true/);
447+
expect(message, locale).toMatch(/OS_AUTH_TWO_FACTOR .*false|false.*OS_AUTH_TWO_FACTOR/);
448+
expect(message, locale).toMatch(/auth\.mfa_required/);
449+
expect(message, locale).not.toMatch(/#\d/);
450+
expectRequireMfaRefusal(organizationMfaWithoutEnrollmentRefusal(locale));
451+
}
452+
});
453+
454+
it('the hook answers in the session\'s locale, and in English for a write with no caller locale', async () => {
455+
for (const event of ['beforeInsert', 'beforeUpdate'] as const) {
456+
const zh = await judgeTold({ enrollable: false, event, data: { require_mfa: true }, locale: 'zh-CN' });
457+
expectRequireMfaRefusal(zh.error);
458+
expect(zh.error.message.startsWith(REFUSAL_FIRST['zh-CN'])).toBe(true);
459+
expect(zh.told, 'a refused write tells no notice').toEqual([]);
460+
const none = await judgeTold({ enrollable: false, event, data: { require_mfa: true } });
461+
expectRequireMfaRefusal(none.error);
462+
expect(none.error.message.startsWith(REFUSAL_FIRST.en)).toBe(true);
463+
}
464+
});
465+
});
466+
467+
describe('#30411 an admitted turn-on answers the grace period in force as its save notice', () => {
468+
it('turning it on (insert, or update of a row stored off) tells who must enroll and the grace period, once', async () => {
469+
for (const [event, previous] of [
470+
['beforeInsert', undefined],
471+
['beforeUpdate', { require_mfa: false }],
472+
['beforeUpdate', undefined],
473+
] as const) {
474+
const out = await judgeTold({ enrollable: true, event, data: { require_mfa: true }, previous, graceDays: 30 });
475+
expect(out.error).toBeNull();
476+
expect(out.told).toEqual([organizationMfaRequiredNotice(30, 'en')]);
477+
expect(out.told[0].startsWith(NOTICE_FIRST.en)).toBe(true);
478+
expect(out.told[0]).toMatch(/has 30 days to enroll/);
479+
}
480+
});
481+
482+
it('zh-CN: the notice reads in Chinese and names the configured days', async () => {
483+
const out = await judgeTold({
484+
enrollable: true, event: 'beforeUpdate', data: { require_mfa: true }, previous: { require_mfa: false },
485+
locale: 'zh-CN', graceDays: 14,
486+
});
487+
expect(out.told).toHaveLength(1);
488+
expect(out.told[0].startsWith(NOTICE_FIRST['zh-CN'])).toBe(true);
489+
expect(out.told[0]).toMatch(/有 14 天时间完成绑定/);
490+
});
491+
492+
it('a grace period of 0 and of 1 day get their own sentence in every language', () => {
493+
for (const [locale, copy] of Object.entries(ORGANIZATION_MFA_REQUIRED_NOTICE_COPY)) {
494+
expect(copy(0), locale).toMatch(/\b0\b/);
495+
expect(copy(0), locale).not.toBe(copy(1));
496+
expect(copy(1), locale).toMatch(/\b1\b/);
497+
expect(copy(45), locale).toMatch(/\b45\b/);
498+
expect(copy(45), locale).not.toMatch(/#\d/);
499+
}
500+
expect(ORGANIZATION_MFA_REQUIRED_NOTICE_COPY.en(1)).toMatch(/has 1 day to enroll/);
501+
expect(ORGANIZATION_MFA_REQUIRED_NOTICE_COPY.en(0)).toMatch(/blocked from data the next time they use the app/);
502+
});
503+
504+
it('says nothing for a write that turns nothing on', async () => {
505+
for (const event of ['beforeInsert', 'beforeUpdate'] as const) {
506+
for (const data of [{ require_mfa: false }, { timezone: 'Asia/Shanghai' }, { name: 'Acme' }]) {
507+
const out = await judgeTold({ enrollable: true, event, data });
508+
expect(out, `${event} ${JSON.stringify(data)}`).toEqual({ error: null, told: [] });
509+
}
510+
}
511+
// A re-save of a stored `true` (a whole-row door, an import re-run).
512+
const resave = await judgeTold({
513+
enrollable: true, event: 'beforeUpdate', data: { require_mfa: true }, previous: { require_mfa: true },
514+
});
515+
expect(resave).toEqual({ error: null, told: [] });
516+
});
517+
518+
it('a dispatch with no notice member is admitted all the same', async () => {
519+
const out = await judgeTold({ enrollable: true, event: 'beforeInsert', data: { require_mfa: true }, noNoticeMember: true });
520+
expect(out).toEqual({ error: null, told: [] });
521+
});
522+
});
523+
524+
describe('#30411 on a real engine: the caller\'s locale reaches the refusal, the notice reaches the write\'s answer', () => {
525+
const zhAdmin = { context: { ...ADMIN.context, locale: 'zh-CN' } };
526+
527+
it('refuses the zh-CN administrator\'s save in Chinese, envelope unchanged, and the row keeps its value', async () => {
528+
const engine = await bootOrganizationEngine({ value: false });
529+
const err = await caught(engine.update('sys_organization', { id: 'org_a', require_mfa: true }, zhAdmin));
530+
expectRequireMfaRefusal(err);
531+
expect(err.message.startsWith(REFUSAL_FIRST['zh-CN'])).toBe(true);
532+
expect(await storedRequireMfa(engine, 'org_a')).toBe(false);
533+
// Control: the same write with no caller locale reads English.
534+
const en = await caught(engine.update('sys_organization', { id: 'org_a', require_mfa: true }, ADMIN));
535+
expect(en.message.startsWith(REFUSAL_FIRST.en)).toBe(true);
536+
});
537+
538+
it('the admitted save hands its listener the notice, with the grace period read per write', async () => {
539+
const grace = { value: 21 };
540+
const engine = await bootOrganizationEngine({ value: true }, grace);
541+
const told: string[] = [];
542+
await engine.update('sys_organization', { id: 'org_a', require_mfa: true }, {
543+
...zhAdmin, onNotice: (m: string) => told.push(m),
544+
} as any);
545+
expect(await storedRequireMfa(engine, 'org_a')).toBe(true);
546+
expect(told).toEqual([organizationMfaRequiredNotice(21, 'zh-CN')]);
547+
// The setting changes; the next write that turns it on names the new number.
548+
grace.value = 3;
549+
await engine.insert('sys_organization', { id: 'org_c', name: 'Cove', slug: 'cove', require_mfa: true }, {
550+
...ADMIN, onNotice: (m: string) => told.push(m),
551+
} as any);
552+
expect(told[1]).toBe(organizationMfaRequiredNotice(3, 'en'));
553+
// A re-save of the stored `true` answers no notice.
554+
await engine.update('sys_organization', { id: 'org_a', require_mfa: true, timezone: 'UTC' }, {
555+
...ADMIN, onNotice: (m: string) => told.push(m),
556+
} as any);
557+
expect(told).toHaveLength(2);
558+
});
559+
});
560+
346561
// ── 3. End to end ───────────────────────────────────────────────────────────
347562

348563
const SECRET = 'test-secret-at-least-32-chars-long!!';
@@ -366,15 +581,16 @@ const post = (manager: AuthManager, path: string, body: unknown, headers: Record
366581
/**
367582
* A member of an organization that requires MFA, unenrolled, whose grace clock
368583
* started 30 days ago, in a session whose active organization is that one —
369-
* the card's measured state, on a deployment with two-factor on or off.
584+
* the card's measured state, on a deployment with two-factor on or off, and a
585+
* grace period of `graceDays` (7 unless a case says otherwise).
370586
*/
371-
async function arrangeHeldMember(twoFactor: boolean) {
587+
async function arrangeHeldMember(twoFactor: boolean, graceDays: number | undefined = 7) {
372588
const engine: any = createMemoryEngine();
373589
const manager = new AuthManager({
374590
secret: SECRET,
375591
baseUrl: 'http://localhost:3000',
376592
dataEngine: engine,
377-
mfaGracePeriodDays: 7,
593+
...(graceDays === undefined ? {} : { mfaGracePeriodDays: graceDays }),
378594
...(twoFactor ? { plugins: { twoFactor: true } } : {}),
379595
} as any);
380596
const signedUp = await post(manager, '/sign-up/email', { email: EMAIL, password: PASSWORD, name: 'Member' });
@@ -427,3 +643,20 @@ describe('end to end — a member is never held for MFA while the enrollment end
427643
expect(((await enable.clone().json()) as { totpURI?: string }).totpURI).toMatch(/^otpauth:\/\/totp\//);
428644
});
429645
});
646+
647+
describe('#30411 the grace period the notice names is the one the gate holds to', () => {
648+
it.each([
649+
[undefined, 7], [0, 0], [14, 14], [2.9, 2], [-3, 0], [120, 120],
650+
] as const)('resolveMfaGracePeriodDays(%j) = %j, the reading the gate\'s clock always took', (configured, days) => {
651+
expect(resolveMfaGracePeriodDays({ mfaGracePeriodDays: configured as number | undefined })).toBe(days);
652+
});
653+
654+
it.each([
655+
[40, false], [20, true], [undefined, true],
656+
] as const)('grace %j days, clock started 30 days ago: held = %j, and the manager answers the same number', async (graceDays, held) => {
657+
const { manager, userId } = await arrangeHeldMember(true, graceDays);
658+
expect(manager.getMfaGracePeriodDays()).toBe(graceDays ?? 7);
659+
const gates = await manager.resolveUserAuthGates(userId, { organizationId: ORG });
660+
expect(gates.some((g) => g.code === 'MFA_REQUIRED')).toBe(held);
661+
});
662+
});

0 commit comments

Comments
 (0)