fix(plugin-auth): /sso/register 门禁改用唯一那把管理员等级尺 (#5942)

.changeset/sso-register-gate-one-admin-grade-ruler.md

@@ -0,0 +1,36 @@
+---
+"@objectstack/plugin-auth": patch
+---
+
+fix(plugin-auth): `/sso/register` 的管理员门禁改用唯一那把等级尺,不再手抄一份大小写敏感的判据 (#5942)
+
+ADR-0024 的 `POST /sso/register` 门禁问的是「这个 membership 是不是本组织的管理员」。
+它此前用的是一份手抄判据:
+
+```ts
+raw.split(',').map((s) => s.trim()).some((r) => r === 'owner' || r === 'admin')
+```
+
+同一个问题在 plugin-auth 内还有另一把尺 —— `invitation-role-cap.ts` 的等级尺
+(`parseOrgRoles()` 会 `.trim().toLowerCase()`,`isOrgAdminGrade()` 据此评级),
+break-glass ban 守卫(`last-admin-ban-guard.ts`,ADR-0024 D5.2)用的就是它。
+两把尺在大小写上不一致:`sys_member.role` 若存成 `Owner` / `ADMIN`,ban 守卫把这一行
+算作**管理员**,而 `/sso/register` 门禁算作**非管理员**。同一条安全路径上的两个答案
+互相矛盾,而且两个方向的错都不出声。
+
+现在门禁改问 `isOrgAdminGrade(m.role)` —— 「哪种 membership 算管理员」在 plugin-auth
+内只剩一个答案,两处自此同尺。
+
+**用户可见的行为变化,只有一个方向:放宽,且只放宽在此前判错的取值上。**
+`sys_member.role` 为大小写非常规值(`Owner` / `ADMIN` / ` Admin `,以及
+`member,Owner` 这类逗号拼写)或数组拼写(`['owner']`)的成员,此前会被
+`/sso/register` **误拒**,现在正确判为管理员并放行。**没有任何收窄**:此前被判为管理员
+的取值,换尺后仍然是管理员(已逐值实测,见 PR)。
+
+ADR-0108 的封闭词表(`owner` / `admin` / `delegated_admin` / `member`)全为小写,UI 与
+better-auth 写入的也是小写,所以正常部署下答案逐值不变 —— 这也是为什么它此前只是一条
+静默分歧,而不是线上故障。要撞上分歧得有一条绕过表单的写入(导入、外部写入、手工 SQL)。
+
+`isOrgOrPlatformAdmin` 名字里的 platform_admin 半边**未改动**,仍由
+`packages/core/src/security/resolve-authz-context.ts` 权威推导;那几处实现的合流是
+另一个决策件。

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

@@ -3659,3 +3659,194 @@ describe('getPublicConfig devSeedAdmin (dev-only login hint)', () => {
     expect((manager.getPublicConfig() as any).devSeedAdmin).toBeUndefined();
   });
 });
+
+// ---------------------------------------------------------------------------
+// [#5942] `isOrgOrPlatformAdmin` — the ADR-0024 `/sso/register` admin gate's
+// criterion — asks "does this membership administer the org" through the ONE
+// grade ladder (`isOrgAdminGrade`, `invitation-role-cap.ts`), not a hand-copied
+// `role === 'owner' || role === 'admin'`.
+//
+// The hand-copy it replaces did `.split(',').map(trim).some(=== 'owner' ||
+// === 'admin')` — case-SENSITIVE, and blind to the array spelling. The grade
+// ladder additionally `.toLowerCase()`s and joins arrays, so the two answered
+// differently on `Owner` / `ADMIN` / `['owner']`: this gate refused a real
+// administrator (false negative) while the break-glass ban guard
+// (`last-admin-ban-guard.ts`, same ladder) counted the same row AS an
+// administrator. Two spellings of one security question, diverging silently.
+//
+// Direction of the change, measured (see the PR body): every difference is a
+// WIDENING, and only over values the old spelling judged wrongly. There is no
+// value that was admin before and is not admin now — the closed ADR-0108
+// vocabulary (all lowercase) answers identically on both sides, which is why
+// no user could hit this today.
+//
+// NOTE on `' admin '`: it is a regression pin, NOT a before-red case. The
+// hand-copy already trimmed, so it answered `true` before the change too. Only
+// the CASE and ARRAY spellings actually move.
+//
+// The platform-admin half of this method is deliberately untouched (#5942 is
+// scoped to the org ruler); the platform-admin cases below pin that.
+// ---------------------------------------------------------------------------
+describe('isOrgOrPlatformAdmin – one grade ruler for "is this membership an admin" (#5942)', () => {
+  const SECRET = 'test-secret-at-least-32-chars-long';
+
+  /**
+   * Read-only engine stub: `members` are the `sys_member` rows, `platformAdmin`
+   * controls the org-less `admin_full_access` link. `find` honours the `where`
+   * the gate actually passes (`user_id`, and `organization_id` when an active
+   * org is set) so the org-scoping half is the product's, not the fixture's.
+   */
+  const makeEngine = (opts: { members?: any[]; platformAdmin?: boolean; throws?: boolean } = {}) => ({
+    find: vi.fn(async (object: string, query?: any) => {
+      if (opts.throws) throw new Error('db down');
+      if (object === 'sys_user_permission_set') {
+        return opts.platformAdmin
+          ? [{ user_id: 'u-1', permission_set_id: 'ps-admin', organization_id: null }]
+          : [];
+      }
+      if (object === 'sys_permission_set') return [{ id: 'ps-admin', name: 'admin_full_access' }];
+      if (object === 'sys_member') {
+        const where = query?.where ?? {};
+        return (opts.members ?? []).filter((row) =>
+          Object.entries(where).every(([k, v]) => row[k] === v),
+        );
+      }
+      return [];
+    }),
+    findOne: vi.fn(),
+  });
+
+  /** The gate's criterion, invoked exactly as the `/sso/register` hook does. */
+  const judge = async (
+    engine: any,
+    activeOrgId?: string,
+    userId = 'u-1',
+  ): Promise<boolean> => {
+    const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
+    const manager = new AuthManager({
+      secret: SECRET,
+      baseUrl: 'http://localhost:3000',
+      dataEngine: engine,
+    });
+    warn.mockRestore();
+    return (manager as any).isOrgOrPlatformAdmin(userId, activeOrgId);
+  };
+
+  const memberRow = (role: unknown) => ({
+    id: 'm-1',
+    user_id: 'u-1',
+    organization_id: 'org-1',
+    role,
+  });
+
+  // -- (1) the fix itself: values the hand-copy refused, the ladder admits ----
+  describe('case-insensitive + array spellings (before: refused, after: admitted)', () => {
+    it.each([
+      ['Owner', 'better-auth owner, capitalized by an import'],
+      ['ADMIN', 'shout-cased by a hand-written SQL insert'],
+      [' Admin ', 'padded AND capitalized'],
+      ['OWNER', 'shout-cased owner'],
+      ['member,Owner', 'comma-joined with one capitalized administrative role'],
+    ])('grades %j as an administrator (%s)', async (role) => {
+      expect(await judge(makeEngine({ members: [memberRow(role)] }), 'org-1')).toBe(true);
+    });
+
+    it('grades the ARRAY spelling ["owner"] as an administrator', async () => {
+      // The hand-copy read `typeof m.role === 'string' ? m.role : ''`, so any
+      // array-valued role graded as nothing at all.
+      expect(await judge(makeEngine({ members: [memberRow(['owner'])] }), 'org-1')).toBe(true);
+    });
+
+    it('grades the ARRAY spelling ["member","Admin"] as an administrator', async () => {
+      expect(
+        await judge(makeEngine({ members: [memberRow(['member', 'Admin'])] }), 'org-1'),
+      ).toBe(true);
+    });
+  });
+
+  // -- (2) regression: the closed ADR-0108 vocabulary answers identically -----
+  describe('closed membership vocabulary (ADR-0108) — unchanged by the new ruler', () => {
+    it.each([
+      ['owner', true],
+      ['admin', true],
+      ['delegated_admin', false],
+      ['member', false],
+    ] as const)('grades the built-in %j as admin=%s', async (role, expected) => {
+      expect(await judge(makeEngine({ members: [memberRow(role)] }), 'org-1')).toBe(expected);
+    });
+
+    it.each([
+      ['owner,member', true],
+      ['member,admin', true],
+      [' admin ', true],
+      ['member,delegated_admin', false],
+    ] as const)(
+      'grades the comma/whitespace spelling %j as admin=%s (already true before #5942)',
+      async (role, expected) => {
+        expect(await judge(makeEngine({ members: [memberRow(role)] }), 'org-1')).toBe(expected);
+      },
+    );
+  });
+
+  // -- (3) non-administrative values still refused (no widening past admin) ---
+  describe('fail-closed floor — nothing else is admitted', () => {
+    it.each([
+      ['manager', 'an app-registered name that is not an administrative grade'],
+      ['administrator', 'a near-miss that is not the vocabulary'],
+      ['adminx', 'a prefix collision'],
+      ['', 'an empty role'],
+    ])('refuses %j (%s)', async (role) => {
+      expect(await judge(makeEngine({ members: [memberRow(role)] }), 'org-1')).toBe(false);
+    });
+
+    it.each([
+      [null, 'null'],
+      [undefined, 'undefined'],
+      [42, 'a number'],
+      [{ role: 'owner' }, 'an object that merely mentions owner'],
+    ])('refuses a non-string role (%s: %s)', async (role) => {
+      expect(await judge(makeEngine({ members: [memberRow(role)] }), 'org-1')).toBe(false);
+    });
+
+    it('refuses when the user has no membership row at all', async () => {
+      expect(await judge(makeEngine({ members: [] }), 'org-1')).toBe(false);
+    });
+
+    it('refuses when the engine read throws (fail CLOSED — ADR-0024)', async () => {
+      expect(await judge(makeEngine({ throws: true }), 'org-1')).toBe(false);
+    });
+  });
+
+  // -- (4) org scoping and the untouched platform-admin half -----------------
+  describe('scoping and the platform-admin half (untouched by #5942)', () => {
+    it('judges only the ACTIVE org when one is set', async () => {
+      const engine = makeEngine({
+        members: [
+          { id: 'm-1', user_id: 'u-1', organization_id: 'org-other', role: 'Owner' },
+          { id: 'm-2', user_id: 'u-1', organization_id: 'org-1', role: 'member' },
+        ],
+      });
+      // Administrative elsewhere, plain member here → refused for org-1 …
+      expect(await judge(engine, 'org-1')).toBe(false);
+      // … and admitted when that other org is the active one.
+      expect(await judge(engine, 'org-other')).toBe(true);
+    });
+
+    it('accepts an administrative membership in ANY org when no active org is set', async () => {
+      const engine = makeEngine({
+        members: [{ id: 'm-1', user_id: 'u-1', organization_id: 'org-other', role: 'ADMIN' }],
+      });
+      expect(await judge(engine, undefined)).toBe(true);
+    });
+
+    it('still admits a platform admin whose membership is a plain member', async () => {
+      const engine = makeEngine({ platformAdmin: true, members: [memberRow('member')] });
+      expect(await judge(engine, 'org-1')).toBe(true);
+    });
+
+    it('still refuses a non-platform-admin with no administrative membership', async () => {
+      const engine = makeEngine({ platformAdmin: false, members: [memberRow('member')] });
+      expect(await judge(engine, 'org-1')).toBe(false);
+    });
+  });
+});

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

@@ -22,7 +22,11 @@ import { postureEnforcesWall, type TenancyPosture } from '@objectstack/spec/secu
 import { MCP_OAUTH_SCOPES } from '@objectstack/spec/ai';
 import { createObjectQLAdapterFactory, withSystemReadContext } from './objectql-adapter.js';
 import { runWithAuthActorScope, setAuthActorResolver } from './auth-actor-attribution.js';
-import { invitationRoleCapFailure, isPlainMemberInvitation } from './invitation-role-cap.js';
+import {
+  invitationRoleCapFailure,
+  isPlainMemberInvitation,
+  isOrgAdminGrade,
+} from './invitation-role-cap.js';
 import { isPlaceholderEmail } from './placeholder-email.js';
 import { reconcileMembership, type MembershipPolicy } from './reconcile-membership.js';
 import type { TenancyService } from './tenancy-service.js';
@@ -3587,11 +3591,17 @@ export class AuthManager {
    * True when `userId` is a platform admin (a `sys_user_permission_set` row
    * pointing at `admin_full_access` with `organization_id = null`) OR an
    * owner/admin member of `activeOrgId` (any org membership with role
-   * owner/admin when no active org is set). Mirrors the role-derivation in
-   * `customSession`; reads through `withSystemReadContext` so the lookups are
-   * not themselves RLS-scoped to the acting (possibly non-privileged) user.
-   * Fails CLOSED (returns false) on any lookup error — this backs a security
-   * gate, so an unverifiable actor must never pass.
+   * owner/admin when no active org is set). Reads through
+   * `withSystemReadContext` so the lookups are not themselves RLS-scoped to the
+   * acting (possibly non-privileged) user. Fails CLOSED (returns false) on any
+   * lookup error — this backs a security gate, so an unverifiable actor must
+   * never pass.
+   *
+   * [#5942] The membership half asks {@link isOrgAdminGrade} — the single grade
+   * ladder in `invitation-role-cap.ts`, shared with the break-glass ban guard —
+   * so "which membership is an administrator" has exactly one answer inside
+   * plugin-auth. The platform-admin half above is unchanged and still has its
+   * own derivations elsewhere (`resolve-authz-context.ts` is authoritative).
    */
   private async isOrgOrPlatformAdmin(
     userId: string,
@@ -3623,13 +3633,14 @@ export class AuthManager {
       if (activeOrgId) where.organization_id = activeOrgId;
       const members = await sys.find('sys_member', { where, limit: 10 });
       for (const m of (Array.isArray(members) ? members : [])) {
-        const raw = typeof m?.role === 'string' ? m.role : '';
-        if (
-          raw
-            .split(',')
-            .map((s: string) => s.trim())
-            .some((r: string) => r === 'owner' || r === 'admin')
-        ) {
+        // [#5942] The ONE grade ladder answers "does this membership administer
+        // the org" — never a hand-copied `role === 'owner' || role === 'admin'`.
+        // The copy that used to live here was case-SENSITIVE and string-only, so
+        // a `sys_member.role` of `Owner` / `ADMIN` / `['owner']` was refused
+        // here while `last-admin-ban-guard.ts` — same question, same ladder —
+        // counted that row AS an administrator. Two spellings of one security
+        // question cannot disagree if there is only one spelling.
+        if (isOrgAdminGrade(m?.role)) {
           return true;
         }
       }