fix(app-shell): turning on Require MFA on an organization shows the grace period once

.changeset/30565-organization-mfa-notice-once.md

@@ -0,0 +1,12 @@
+---
+'@object-ui/app-shell': patch
+---
+
+fix(app-shell): turning on Require MFA on an organization shows the grace period once, with the number Setup set
+
+Clause-②: no
+Metadata-protocol: none
+
+An accepted save that turns `require_mfa` on for an organization now gets a save notice from the server that names who must enroll and the MFA grace period in force. The console still added its own line under it, which says the same thing with the default "7 days". So the toast told the administrator the grace period twice, and when Setup → Authentication set another number, the two lines disagreed.
+
+The record form now leaves its own line out when the save answer's notice is the server's MFA notice. It recognizes that notice by its opening sentence, in each of the four languages the server writes it in, and not just by a notice being there: the answer carries one notice, the last one a hook or flow on the write set, so another notice on the same save keeps the console line. A save whose answer has no notice (a server that does not send one) shows the console line as before.

ui/packages/app-shell/src/console/AppContent.tsx

@@ -1058,10 +1058,13 @@ export function AppContent({ extraRoutes, extraRoutesNoApp }: AppContentProps =
     // invalidation bus (#2269): the dataSource write already emitted a
     // MutationEvent that the bridge fans out — no manual notify needed.
     const claimed = answerWarningClaim.current?.take(saved);
+    // No console line when the answer's notice is the server's own MFA notice,
+    // which names the grace period in force (objectstack#30565).
     const mfaNotice = organizationMfaSavedNotice({
       objectName: formObjectDef?.name,
       storedRequireMfa,
       saved,
+      answerNotice: claimed?.notice,
       t,
     });
     const feedback = mfaNotice

ui/packages/app-shell/src/console/__tests__/AppContent.recordSaveOneToast-27999.test.tsx

@@ -97,6 +97,8 @@ const dataSourceStub = {
   onConnectionStateChange: () => () => {},
   getConnectionState: () => 'connected',
   getObjectSchema: async () => ({ fields: {} as Record<string, { label?: string }> }),
+  // The organization form reads the row's stored `require_mfa` (objectstack#30295): stored off.
+  findOne: vi.fn(async (_objectName: string, id: string) => ({ id, require_mfa: false })),
 };
 vi.mock('../../providers/AdapterProvider', async (importOriginal) => ({
   ...(await importOriginal<Record<string, unknown>>()),
@@ -113,6 +115,13 @@ const OBJECTS = [
   },
   // The parent the child-form task opens over.
   { name: 'showcase_project', label: 'Project', fields: { name: { type: 'text', label: 'Name' } } },
+  // The organization whose `require_mfa` a save turns on (objectstack#30565).
+  {
+    name: 'sys_organization',
+    label: 'Organization',
+    nameField: 'name',
+    fields: { name: { type: 'text', label: 'Name' }, require_mfa: { type: 'boolean', label: 'Require MFA' } },
+  },
 ];
 
 const APPS = [{ name: 'crm', label: 'CRM', isDefault: true, navigation: [] }];
@@ -211,7 +220,8 @@ async function saveFrom(url: string, event: WriteWarningEvent, saved: Record<str
   // The overlay's claim is opened by an effect of the mounted, open dialog.
   await waitFor(() => expect(handlers.has('crud_success')).toBe(true));
   const schema = modalSchemas[modalSchemas.length - 1];
-  expect(schema.objectName).toBe('showcase_task');
+  // The overlay edits the object the write names.
+  expect(schema.objectName).toBe(event.resource);
   const channel = emitWriteWarning(event, t, dataSourceStub as never, identityLabel, toast);
   await schema.onSuccess(saved);
   await channel;
@@ -326,3 +336,57 @@ describe('AppContent — the overlay save toast carries the hook notice (objects
     expect(toastInfo.mock.calls).toEqual([[NOTICE]]);
   });
 });
+
+// objectstack#30565 — an organization save that turns `require_mfa` on (stored
+// off): the server answers a `notice` naming the grace period in force, so the
+// save toast carries it alone, without the console's own
+// `console.organizationMfa.nowRequired` line, which names the default 7 days.
+// With no notice, or another hook's notice, the console line stays.
+describe('AppContent — turning on Require MFA toasts the server notice once (objectstack#30565)', () => {
+  const ORG_EDIT = '/apps/crm/sys_organization?form=org_1';
+  const ORG_SAVED = { id: 'org_1', name: 'Acme', require_mfa: true };
+  // The server's English notice for a 3-day grace period, verbatim (the same
+  // fixture as `organizationMfaNotices-30295.test.tsx`).
+  const SERVER_NOTICE =
+    'Multi-factor authentication is now required for this organization. Every member who has not set up an ' +
+    'authenticator app has 3 days to enroll one, counted from the first time they use the app while it is ' +
+    'required; after that they are blocked from data until they do. Members who already use an authenticator ' +
+    'app are not affected. The MFA grace period is set in Setup → Authentication.';
+  const OTHER_NOTICE = 'Sent to your manager for approval';
+  const CONSOLE_LINE = createI18n({ defaultLanguage: 'en', detectBrowserLanguage: false }).t(
+    'console.organizationMfa.nowRequired',
+  );
+  const orgUpdate = (notice: string | undefined): WriteWarningEvent => ({
+    operation: 'update',
+    resource: 'sys_organization',
+    id: 'org_1',
+    droppedFields: [],
+    notice,
+  });
+
+  it('the answer carries the server notice: one toast, the server notice alone', async () => {
+    await saveFrom(ORG_EDIT, orgUpdate(SERVER_NOTICE), ORG_SAVED);
+
+    expect(dataSourceStub.findOne).toHaveBeenCalledWith('sys_organization', 'org_1');
+    expect(toastSuccess.mock.calls).toEqual([['Organization "Acme" updated', { description: SERVER_NOTICE }]]);
+    expect(toastWarning).not.toHaveBeenCalled();
+    expect(toastInfo).not.toHaveBeenCalled();
+  });
+
+  it('CONTROL: no notice on the answer: the console line', async () => {
+    expect(CONSOLE_LINE).toContain('7 days');
+    await saveFrom(ORG_EDIT, orgUpdate(undefined), ORG_SAVED);
+
+    expect(toastSuccess.mock.calls).toEqual([['Organization "Acme" updated', { description: CONSOLE_LINE }]]);
+    expect(toastInfo).not.toHaveBeenCalled();
+  });
+
+  it("CONTROL: another hook's notice on the answer: both lines", async () => {
+    await saveFrom(ORG_EDIT, orgUpdate(OTHER_NOTICE), ORG_SAVED);
+
+    expect(toastSuccess.mock.calls).toEqual([
+      ['Organization "Acme" updated', { description: `${OTHER_NOTICE}\n${CONSOLE_LINE}` }],
+    ]);
+    expect(toastInfo).not.toHaveBeenCalled();
+  });
+});

ui/packages/app-shell/src/console/__tests__/organizationMfaNotices-30295.test.tsx

@@ -60,6 +60,57 @@ describe('organizationMfaSavedNotice — after an accepted save', () => {
   });
 });
 
+// objectstack#30565 — the server answers an accepted turn-on with its own save
+// notice (objectstack `plugin-auth`'s `ORGANIZATION_MFA_REQUIRED_NOTICE_COPY`),
+// which names the grace period in force. The console's line would repeat it,
+// with the default 7 days where Setup set another number.
+describe('organizationMfaSavedNotice — the server already said it (objectstack#30565)', () => {
+  const saved = { id: 'org_1', name: 'Acme', require_mfa: true };
+  const turnOn = (answerNotice: string | undefined) =>
+    organizationMfaSavedNotice({ objectName: 'sys_organization', storedRequireMfa: false, saved, answerNotice, t });
+
+  // The server's notice as it answers it, in each language its copy is written in,
+  // for a grace period of N days and of 0 days (two different sentences).
+  const SERVER_NOTICES = {
+    'en, 3 days':
+      'Multi-factor authentication is now required for this organization. Every member who has not set up an ' +
+      'authenticator app has 3 days to enroll one, counted from the first time they use the app while it is ' +
+      'required; after that they are blocked from data until they do. Members who already use an authenticator ' +
+      'app are not affected. The MFA grace period is set in Setup → Authentication.',
+    'en, 0 days':
+      'Multi-factor authentication is now required for this organization. Every member who has not set up an ' +
+      'authenticator app is blocked from data the next time they use the app, until they enroll one: the MFA ' +
+      'grace period is 0 days. Members who already use an authenticator app are not affected. The MFA grace ' +
+      'period is set in Setup → Authentication.',
+    'zh-CN, 3 days':
+      '本组织现已强制多因素认证。尚未绑定身份验证器应用的成员有 3 天时间完成绑定，从要求生效后他们第一次使用应用时起算；' +
+      '期满后，未绑定的成员将无法访问数据，直到完成绑定。已使用身份验证器应用的成员不受影响。MFA 宽限期可在“设置 → 认证”中修改。',
+    'ja-JP, 0 days':
+      'この組織では多要素認証が必須になりました。認証アプリをまだ登録していないメンバーは、次にアプリを利用した時点から' +
+      '登録するまでデータにアクセスできなくなります（MFA 猶予期間は 0 日です）。すでに認証アプリを使っているメンバーには' +
+      '影響しません。MFA 猶予期間は「設定 → 認証」で変更できます。',
+    'es-ES, 3 days':
+      'La autenticación multifactor ahora es obligatoria en esta organización. Cada miembro que no haya configurado ' +
+      'una app de autenticación tiene 3 días para registrarla, a contar desde la primera vez que usa la app mientras ' +
+      'sea obligatoria; después se le bloquea el acceso a los datos hasta que lo haga. Los miembros que ya usan un ' +
+      'autenticador no se ven afectados. El periodo de gracia de MFA se configura en Configuración → Autenticación.',
+  };
+
+  it.each(Object.entries(SERVER_NOTICES))('the answer carries the server notice (%s): no console line', (_case, notice) => {
+    expect(turnOn(notice)).toBeUndefined();
+  });
+
+  it('no notice on the answer (a server that does not send one): the console line, as before', () => {
+    expect(turnOn(undefined)).toBe('console.organizationMfa.nowRequired');
+  });
+
+  it("another hook's notice on the same save: the console line still says who must enroll", () => {
+    // The answer carries ONE notice, the last a hook or flow on the write set,
+    // so a later one replaces the server's MFA notice.
+    expect(turnOn('Sent to your manager for approval')).toBe('console.organizationMfa.nowRequired');
+  });
+});
+
 describe('useStoredRequireMfa — the stored value while the form is open', () => {
   it('an edit reads the row once; a create is off; a closed form or another object reads nothing', async () => {
     const findOne = vi.fn(async () => ({ id: 'org_1', require_mfa: true }));

ui/packages/app-shell/src/console/organizationMfaNotices.ts

@@ -25,7 +25,10 @@ import type { useObjectTranslation } from '@object-ui/i18n';
  *   turned on.
  * - **After an accepted save that turned it on**: who is affected (every
  *   member without an authenticator app) and when the block starts (once the
- *   MFA grace period ends, counted from each member's next use).
+ *   MFA grace period ends, counted from each member's next use). The server
+ *   now says this itself, with the grace period in force, as the save answer's
+ *   `notice` (objectstack#30411); the console's line stands in only when the
+ *   answer does not carry that notice (objectstack#30565).
  *
  * Both are judged only on what the console knows: an unread `features` says
  * nothing before saving, and a save whose stored value was already on says
@@ -57,23 +60,55 @@ export function organizationMfaFormNotice(input: {
   return input.t('console.organizationMfa.needsTwoFactor');
 }
 
+/**
+ * The opening sentence of the notice the server answers an accepted turn-on
+ * with (objectstack `plugin-auth`'s `ORGANIZATION_MFA_REQUIRED_NOTICE_COPY`), in
+ * each of the four languages that copy is written in. Its sentence for N days
+ * and its sentence for 0 days both open with it; a server language the copy
+ * lacks answers the English one.
+ */
+const SERVER_MFA_NOTICE_OPENINGS: readonly string[] = [
+  'Multi-factor authentication is now required for this organization.',
+  '本组织现已强制多因素认证。',
+  'この組織では多要素認証が必須になりました。',
+  'La autenticación multifactor ahora es obligatoria en esta organización.',
+];
+
+/**
+ * True when a save answer's `notice` is the server's own MFA notice. Told by
+ * its opening sentence, not by a notice being there: the answer carries ONE
+ * notice, the last one a hook or flow on the write set, so another notice on
+ * the same save means the server's was replaced, and an older server's answer
+ * can carry another hook's notice. Should the server's wording change, this
+ * answers false and the toast shows both lines, never neither.
+ */
+const isServerMfaNotice = (notice: string | undefined): boolean =>
+  typeof notice === 'string' && SERVER_MFA_NOTICE_OPENINGS.some((opening) => notice.includes(opening));
+
 /**
  * The line an accepted organization save adds to its toast when it turned the
  * requirement on: who must enroll and when the block starts. `undefined` when
  * the saved row does not require MFA, or the stored row already did
  * (`storedRequireMfa === true`); an unread stored value (`undefined`) counts as
  * off, since saying it once too often is cheaper than not saying it.
+ *
+ * `undefined` too when the save answer's notice (`answerNotice`) is the
+ * server's MFA notice: it says all of this with the grace period in force,
+ * where the console's line can only name the default 7 days.
  */
 export function organizationMfaSavedNotice(input: {
   objectName: string | undefined;
   storedRequireMfa: boolean | undefined;
   saved: unknown;
+  /** The save answer's `notice`, as the record form's claim took it. */
+  answerNotice?: string;
   t: Translate;
 }): string | undefined {
   if (input.objectName !== ORGANIZATION_OBJECT) return undefined;
   if (input.storedRequireMfa === true) return undefined;
   const saved = input.saved as Record<string, unknown> | null | undefined;
   if (!saved || !isOn(saved[REQUIRE_MFA_FIELD])) return undefined;
+  if (isServerMfaNotice(input.answerNotice)) return undefined;
   return input.t('console.organizationMfa.nowRequired');
 }