fix(plugin-auth): the admin identity rows on the compliance ledger record the admin's decisions, never a value of a user field (#21174)

.changeset/21174-admin-audit-metadata.md

@@ -0,0 +1,13 @@
+---
+'@objectstack/plugin-auth': patch
+---
+
+fix(plugin-auth): the compliance-ledger rows the admin identity endpoints write record the admin's decisions, never a value of a field of the user (#21174)
+
+Clause-②: no
+
+The admin create-user and set-user-password endpoints each write their own `sys_audit_log` row beside the rows plugin-audit's CRUD mirror writes for the same call. That row's free `metadata` copied values the call had just written into fields of the user. The ledger's read side narrows the mirror's before/after snapshots to what each reader is served, but it cannot narrow free metadata without deriving masking a second time, so a ledger reader the data plane withholds one of those fields from was served its value through the explicit row.
+
+The explicit row now carries only the admin's decisions — which operation ran, whether the password was generated, whether the account's address is a generated placeholder, whether the membership was bound and to which organization — plus its reference to the user (`object_name` and `record_id`). The values the call writes into the user's fields are recorded where they already were: on the mirror's `create` and `update` rows for those same writes, in the snapshot columns the read side narrows per reader. The decision set is a closed type, so a field value no longer compiles into the row.
+
+Migration: a reader that took a user field's value from the explicit row's metadata reads it from the mirror's row for the same write instead (its after-snapshot), served according to the reader's field access. Rows written before this release are stored data and are not rewritten.

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

@@ -704,3 +704,139 @@ describe('runAdminSetUserPassword', () => {
     expect(meta.passwordGenerated).toBe(true);
   });
 });
+
+// ── The ledger row records the admin's decisions, never a field value ──────
+//
+// `sys_audit_log.metadata` is free text. The ledger's read side narrows the
+// before/after snapshot columns to what each reader is served; it cannot
+// narrow `metadata` without mapping decision names back to fields, which would
+// derive masking a second time. So the producer must not put a value it wrote
+// into a field of the user there: that value is on plugin-audit's mirror row
+// for the same write, in a column the read side narrows. These pins hold the
+// row to its decisions and its reference (`object_name` + `record_id`).
+describe('admin ledger rows: decisions and a reference, never a field value of the user', () => {
+  const CREATE_DECISIONS = ['event', 'membershipCreated', 'passwordGenerated', 'placeholderEmail'];
+  const PASSWORD_SET_DECISIONS = ['event', 'passwordGenerated'];
+
+  /** Normalise a key so a camelCase metadata key and a snake_case field name compare equal. */
+  const norm = (k: string) => k.replace(/_/g, '').toLowerCase();
+
+  /** Every field this call wrote on the user, with the value it wrote. */
+  function userFieldWrites(m: ReturnType<typeof makeDeps>): Array<[string, unknown]> {
+    const writes: Array<[string, unknown]> = [];
+    for (const call of m.createUser.mock.calls) {
+      const { data, ...body } = call[0].body as Record<string, any>;
+      for (const [k, v] of Object.entries({ ...body, ...(data ?? {}) })) {
+        if (k !== 'password') writes.push([k, v]);
+      }
+    }
+    for (const [object, doc] of callsOf(m.engineUpdate)) {
+      if (object !== 'sys_user') continue;
+      for (const [k, v] of Object.entries(doc as Record<string, unknown>)) {
+        if (k !== 'id') writes.push([k, v]);
+      }
+    }
+    return writes;
+  }
+
+  /** The one ledger row the call wrote, with its metadata parsed. */
+  function ledgerRow(m: ReturnType<typeof makeDeps>, insert = m.engineCreate) {
+    const rows = callsOf(insert).filter(([object]) => object === 'sys_audit_log');
+    expect(rows).toHaveLength(1);
+    const row = rows[0][1] as Record<string, any>;
+    return { row, metadata: JSON.parse(row.metadata) as Record<string, unknown> };
+  }
+
+  /** No metadata key names a written field, and no metadata value is a written string value. */
+  function expectNoFieldValue(metadata: Record<string, unknown>, writes: Array<[string, unknown]>) {
+    expect(writes.length).toBeGreaterThan(0);
+    const keys = new Set(Object.keys(metadata).map(norm));
+    const blob = JSON.stringify(metadata);
+    for (const [field, value] of writes) {
+      expect(keys.has(norm(field)), `metadata names the written field ${field}`).toBe(false);
+      if (typeof value === 'string' && value.length > 0) {
+        expect(blob.includes(value), `metadata carries the value written to ${field}`).toBe(false);
+      }
+    }
+  }
+
+  it('create-user: the row carries the closed decision set and none of the values written into the user', async () => {
+    const m = makeDeps({ phoneNumberEnabled: () => true });
+    const res = await runAdminCreateUser(
+      m.deps,
+      makeRequest({
+        email: 'Ledger.Subject@Example.com',
+        name: 'Ledger Subject',
+        role: 'ledgerrole',
+        phoneNumber: '+8613811112222',
+        generatePassword: true,
+      }),
+      ACTOR,
+    );
+    expect(res.status).toBe(200);
+    const writes = userFieldWrites(m);
+    // Armed: the call really wrote the identity, the role scalar, the phone
+    // and the must-change-password stamp, so the row had them to copy.
+    expect(writes.map(([k]) => norm(k)).sort()).toEqual(
+      ['email', 'mustchangepassword', 'name', 'phonenumber', 'role'],
+    );
+
+    const { row, metadata } = ledgerRow(m);
+    expect(row.object_name).toBe('sys_user');
+    expect(row.record_id).toBe('user-9');
+    expect(Object.keys(metadata).sort()).toEqual(CREATE_DECISIONS);
+    expect(metadata).toMatchObject({ event: 'user.admin_created', placeholderEmail: false, passwordGenerated: true });
+    expectNoFieldValue(metadata, writes);
+  });
+
+  it('create-user, phone-only: the placeholder decision is recorded, the generated address is not', async () => {
+    const m = makeDeps({ phoneNumberEnabled: () => true });
+    const res = await runAdminCreateUser(
+      m.deps,
+      makeRequest({ phoneNumber: '+8613833334444', generatePassword: true }),
+      ACTOR,
+    );
+    expect(res.status).toBe(200);
+    const { metadata } = ledgerRow(m);
+    expect(Object.keys(metadata).sort()).toEqual(CREATE_DECISIONS);
+    expect(metadata.placeholderEmail).toBe(true);
+    expectNoFieldValue(metadata, userFieldWrites(m));
+  });
+
+  it('create-user, membership bound: the organization rides as a reference beside the decisions', async () => {
+    const m = makeDeps();
+    const engineInsert = vi.fn(async () => ({}));
+    const find = vi.fn(async (object: string) => (object === 'sys_organization' ? [{ id: 'org_only' }] : []));
+    m.deps.getDataEngine = () => ({ update: m.engineUpdate, insert: engineInsert, find });
+    const res = await runAdminCreateUser(
+      m.deps,
+      makeRequest({ email: 'bound.subject@example.com', role: 'boundrole', generatePassword: true }),
+      ACTOR,
+    );
+    expect(res.status).toBe(200);
+    const { metadata } = ledgerRow(m, engineInsert);
+    expect(Object.keys(metadata).sort()).toEqual([...CREATE_DECISIONS, 'organizationId'].sort());
+    expect(metadata).toMatchObject({ organizationId: 'org_only', membershipCreated: true });
+    expectNoFieldValue(metadata, userFieldWrites(m));
+  });
+
+  it('set-user-password: the row carries the closed decision set and not the stamp it wrote', async () => {
+    const m = makeDeps();
+    const res = await runAdminSetUserPassword(
+      m.deps,
+      makeRequest({ userId: 'user-9', generatePassword: true }),
+      ACTOR,
+    );
+    expect(res.status).toBe(200);
+    const writes = userFieldWrites(m);
+    // Armed: the stamp really was written on the user.
+    expect(writes.map(([k]) => norm(k))).toEqual(['mustchangepassword']);
+
+    const { row, metadata } = ledgerRow(m);
+    expect(row.object_name).toBe('sys_user');
+    expect(row.record_id).toBe('user-9');
+    expect(Object.keys(metadata).sort()).toEqual(PASSWORD_SET_DECISIONS);
+    expect(metadata).toMatchObject({ event: 'user.admin_password_set', passwordGenerated: true });
+    expectNoFieldValue(metadata, writes);
+  });
+});

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

@@ -374,6 +374,39 @@ async function bindUserToSoleOrganization(
   };
 }
 
+/**
+ * The admin's decisions, the WHOLE of what {@link writeAdminAudit} puts in a
+ * ledger row's `metadata`. Closed on purpose: each member is either a decision
+ * that no field of the user stores, or a reference to another record.
+ *
+ * - `event` — which administrative operation this is.
+ * - `passwordGenerated` — the system minted the password rather than the admin
+ *   typing one. No field stores it; the credential is on `sys_account`.
+ * - `placeholderEmail` — the admin created a phone-only identity, so the
+ *   address on the account is a generated placeholder. The decision, never
+ *   the address.
+ * - `membershipCreated` — this call bound the membership (ADR-0093 D2).
+ * - `organizationId` — a reference to the organization bound to, a record of
+ *   its own; present only when one was resolved.
+ *
+ * ⛔ Never add a member that copies a value this operation writes into a field
+ * of the user (see {@link writeAdminAudit}): that value is already on the
+ * mirror row for the same write, in the snapshot column the ledger's read
+ * side narrows.
+ */
+type AdminAuditDecisions =
+  | {
+      event: 'user.admin_created';
+      placeholderEmail: boolean;
+      passwordGenerated: boolean;
+      membershipCreated: boolean;
+      organizationId?: string;
+    }
+  | {
+      event: 'user.admin_password_set';
+      passwordGenerated: boolean;
+    };
+
 /**
  * Best-effort explicit audit row for an admin identity operation. Never
  * throws; never includes password material (red line).
@@ -413,17 +446,31 @@ async function bindUserToSoleOrganization(
  *    password was administratively reset. "The hook covers it, drop the
  *    explicit insert" would silently delete that trail.
  * 3. **Disjoint payloads.** The generic row is a field diff / row snapshot;
- *    this one records the admin's DECISIONS (`event`, `passwordGenerated`,
- *    `mustChangePassword`, `placeholderEmail`, `membershipCreated`), none of
- *    which is derivable from the stored row.
+ *    this one records the admin's DECISIONS ({@link AdminAuditDecisions}),
+ *    none of which is stored in a field of the user.
+ *
+ * **No field value of the user rides `metadata`.** The row's reference to the
+ * record is `object_name` + `record_id`; `metadata` carries only the
+ * decisions. A value this operation writes into a field of the user — the
+ * identity it was created with, its legacy role scalar, the must-change-password
+ * flag — is recorded by plugin-audit's mirror row for that same write, whose
+ * before/after snapshots the ledger's read side narrows to what each reader is
+ * served. `metadata` is free text that no read-time narrowing can map back to
+ * a field without deriving masking a second time, so a copy here would serve
+ * the value to a ledger reader the data plane withholds it from. Two guards
+ * hold that: the closed {@link AdminAuditDecisions} type refuses a field value
+ * written as a literal key at compile time (a conditional spread passes
+ * TypeScript's excess-property check, so it does not stop that spelling), and
+ * the pins in `admin-user-endpoints.test.ts` fail on any key outside the
+ * decision set and on any value this call wrote into the user.
  */
 async function writeAdminAudit(
   deps: AdminUserEndpointDeps,
   entry: {
     action: 'create' | 'update';
     actor: AdminActor;
     recordId: string;
-    metadata: Record<string, unknown>;
+    metadata: AdminAuditDecisions;
   },
 ): Promise<void> {
   const engine = deps.getDataEngine();
@@ -463,8 +510,8 @@ async function writeAdminAudit(
         + `${entry.recordId} was NOT written — the operation itself SUCCEEDED and the endpoint `
         + 'answers 200, so nothing looks wrong. plugin-audit is installed (sys_audit_log is '
         + 'registered), so this is a REFUSED write, not an absent plugin. This row carries the '
-        + "admin's decisions (event, passwordGenerated, mustChangePassword, placeholderEmail, "
-        + 'membershipCreated), none of which is derivable from the stored row, and for '
+        + "admin's decisions (event, passwordGenerated, placeholderEmail, membershipCreated), "
+        + 'none of which is stored in a field of the user, and for '
         + '/admin/set-user-password it is the only audit record that exists at all because '
         + "sys_account is in plugin-audit's SKIP_OBJECTS. Nothing retries this write, so the "
         + 'action stays permanently untrailed. Remedy: restore write access to sys_audit_log '
@@ -573,18 +620,17 @@ export async function runAdminCreateUser(
   // `membershipPolicy: 'invite-only'` (ADR-0093 D1) — see the helper.
   const membership = await bindUserToSoleOrganization(deps, userId);
 
+  // The decisions only. The values this call wrote into the user's fields —
+  // the identity, the role scalar, the must-change-password stamp — are on
+  // plugin-audit's mirror rows for those same writes (see `writeAdminAudit`).
   await writeAdminAudit(deps, {
     action: 'create',
     actor,
     recordId: userId,
     metadata: {
       event: 'user.admin_created',
-      email: email.toLowerCase(),
-      ...(phoneNumber ? { phoneNumber } : {}),
-      ...(role ? { role } : {}),
       placeholderEmail: !hasEmail,
       passwordGenerated: resolved.generated,
-      mustChangePassword: stamped,
       ...(membership.organizationId ? { organizationId: membership.organizationId } : {}),
       membershipCreated: membership.membershipCreated,
     },
@@ -679,10 +725,11 @@ export async function runAdminSetUserPassword(
     action: 'update',
     actor,
     recordId: userId,
+    // The must-change-password stamp is a field write on the user, recorded by
+    // plugin-audit's mirror row for it — not copied here (see `writeAdminAudit`).
     metadata: {
       event: 'user.admin_password_set',
       passwordGenerated: resolved.generated,
-      mustChangePassword: mustChangePassword && stamped,
     },
   });
 

packages/qa/dogfood/test/admin-identity-audit-trail.dogfood.test.ts

@@ -140,12 +140,21 @@ describe('#4940: what an admin identity operation leaves in sys_audit_log', () =
 
     // W2 — and the endpoint's own row is a SECOND row on the same record.
     // Kept deliberately: it records the admin's DECISIONS, none of which is
-    // derivable from a field diff of the created row.
+    // stored in a field of the created user.
     const explicit = creates.filter((r) => isExplicit(r, 'user.admin_created'));
     expect(explicit).toHaveLength(1);
     expect(explicit[0].user_id).toBe(adminUserId);
-    expect(String(explicit[0].metadata)).toContain('"mustChangePassword":true');
     expect(String(explicit[0].metadata)).toContain('"passwordGenerated":false');
+    // The must-change-password stamp is a write to a field of the user, so it
+    // rides plugin-audit's own `update` row for that write (the snapshot column
+    // the ledger's read side narrows per reader), never the explicit row's
+    // free metadata, which no read-time narrowing reaches.
+    expect(String(explicit[0].metadata)).not.toContain('mustChangePassword');
+    await waitForRows(
+      async () => (await userAudit(ql, userId)).filter((r) => r.action === 'update' && isGeneric(r)),
+      (rows) => rows.some((r) => String(r.new_value).includes('must_change_password')),
+      "plugin-audit's update row for the must-change-password stamp",
+    );
     // The overlap is exactly two — measured, so a third writer appearing on
     // this path is a finding rather than a silent extra ledger row.
     expect(creates).toHaveLength(2);