Skip to content

Commit 512d4eb

Browse files
committed
fix(plugin-auth): /sso/register 门禁改用唯一那把管理员等级尺 (#5942)
`isOrgOrPlatformAdmin` 的 membership 半边此前手抄了一份判据 (`split(',').map(trim).some(=== 'owner' || === 'admin')`),大小写敏感且只认 字符串。同一个问题在 plugin-auth 内的另一把尺 —— `invitation-role-cap.ts` 的 等级尺(`isOrgAdminGrade`,break-glass ban 守卫在用)—— 会 `.toLowerCase()` 并处理数组拼写。于是 `sys_member.role='Owner'` 被 ban 守卫算作管理员、被 `/sso/register` 门禁算作非管理员,两个方向的错都不出声。 改为直接问 `isOrgAdminGrade(m?.role)`,「哪种 membership 算管理员」在 plugin-auth 内只剩一个答案。 行为变化只有放宽一个方向,且只放宽在此前判错的取值上(大小写非常规值与数组 拼写从误拒变正确放行);无任何收窄 —— 已按 ADR-0108 封闭词表逐值实测。 platform_admin 半边未改动。 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JwwiU9bjhwy2SWj13ho8uv
1 parent dd98cba commit 512d4eb

3 files changed

Lines changed: 251 additions & 13 deletions

File tree

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

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

Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3659,3 +3659,194 @@ describe('getPublicConfig devSeedAdmin (dev-only login hint)', () => {
36593659
expect((manager.getPublicConfig() as any).devSeedAdmin).toBeUndefined();
36603660
});
36613661
});
3662+
3663+
// ---------------------------------------------------------------------------
3664+
// [#5942] `isOrgOrPlatformAdmin` — the ADR-0024 `/sso/register` admin gate's
3665+
// criterion — asks "does this membership administer the org" through the ONE
3666+
// grade ladder (`isOrgAdminGrade`, `invitation-role-cap.ts`), not a hand-copied
3667+
// `role === 'owner' || role === 'admin'`.
3668+
//
3669+
// The hand-copy it replaces did `.split(',').map(trim).some(=== 'owner' ||
3670+
// === 'admin')` — case-SENSITIVE, and blind to the array spelling. The grade
3671+
// ladder additionally `.toLowerCase()`s and joins arrays, so the two answered
3672+
// differently on `Owner` / `ADMIN` / `['owner']`: this gate refused a real
3673+
// administrator (false negative) while the break-glass ban guard
3674+
// (`last-admin-ban-guard.ts`, same ladder) counted the same row AS an
3675+
// administrator. Two spellings of one security question, diverging silently.
3676+
//
3677+
// Direction of the change, measured (see the PR body): every difference is a
3678+
// WIDENING, and only over values the old spelling judged wrongly. There is no
3679+
// value that was admin before and is not admin now — the closed ADR-0108
3680+
// vocabulary (all lowercase) answers identically on both sides, which is why
3681+
// no user could hit this today.
3682+
//
3683+
// NOTE on `' admin '`: it is a regression pin, NOT a before-red case. The
3684+
// hand-copy already trimmed, so it answered `true` before the change too. Only
3685+
// the CASE and ARRAY spellings actually move.
3686+
//
3687+
// The platform-admin half of this method is deliberately untouched (#5942 is
3688+
// scoped to the org ruler); the platform-admin cases below pin that.
3689+
// ---------------------------------------------------------------------------
3690+
describe('isOrgOrPlatformAdmin – one grade ruler for "is this membership an admin" (#5942)', () => {
3691+
const SECRET = 'test-secret-at-least-32-chars-long';
3692+
3693+
/**
3694+
* Read-only engine stub: `members` are the `sys_member` rows, `platformAdmin`
3695+
* controls the org-less `admin_full_access` link. `find` honours the `where`
3696+
* the gate actually passes (`user_id`, and `organization_id` when an active
3697+
* org is set) so the org-scoping half is the product's, not the fixture's.
3698+
*/
3699+
const makeEngine = (opts: { members?: any[]; platformAdmin?: boolean; throws?: boolean } = {}) => ({
3700+
find: vi.fn(async (object: string, query?: any) => {
3701+
if (opts.throws) throw new Error('db down');
3702+
if (object === 'sys_user_permission_set') {
3703+
return opts.platformAdmin
3704+
? [{ user_id: 'u-1', permission_set_id: 'ps-admin', organization_id: null }]
3705+
: [];
3706+
}
3707+
if (object === 'sys_permission_set') return [{ id: 'ps-admin', name: 'admin_full_access' }];
3708+
if (object === 'sys_member') {
3709+
const where = query?.where ?? {};
3710+
return (opts.members ?? []).filter((row) =>
3711+
Object.entries(where).every(([k, v]) => row[k] === v),
3712+
);
3713+
}
3714+
return [];
3715+
}),
3716+
findOne: vi.fn(),
3717+
});
3718+
3719+
/** The gate's criterion, invoked exactly as the `/sso/register` hook does. */
3720+
const judge = async (
3721+
engine: any,
3722+
activeOrgId?: string,
3723+
userId = 'u-1',
3724+
): Promise<boolean> => {
3725+
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
3726+
const manager = new AuthManager({
3727+
secret: SECRET,
3728+
baseUrl: 'http://localhost:3000',
3729+
dataEngine: engine,
3730+
});
3731+
warn.mockRestore();
3732+
return (manager as any).isOrgOrPlatformAdmin(userId, activeOrgId);
3733+
};
3734+
3735+
const memberRow = (role: unknown) => ({
3736+
id: 'm-1',
3737+
user_id: 'u-1',
3738+
organization_id: 'org-1',
3739+
role,
3740+
});
3741+
3742+
// -- (1) the fix itself: values the hand-copy refused, the ladder admits ----
3743+
describe('case-insensitive + array spellings (before: refused, after: admitted)', () => {
3744+
it.each([
3745+
['Owner', 'better-auth owner, capitalized by an import'],
3746+
['ADMIN', 'shout-cased by a hand-written SQL insert'],
3747+
[' Admin ', 'padded AND capitalized'],
3748+
['OWNER', 'shout-cased owner'],
3749+
['member,Owner', 'comma-joined with one capitalized administrative role'],
3750+
])('grades %j as an administrator (%s)', async (role) => {
3751+
expect(await judge(makeEngine({ members: [memberRow(role)] }), 'org-1')).toBe(true);
3752+
});
3753+
3754+
it('grades the ARRAY spelling ["owner"] as an administrator', async () => {
3755+
// The hand-copy read `typeof m.role === 'string' ? m.role : ''`, so any
3756+
// array-valued role graded as nothing at all.
3757+
expect(await judge(makeEngine({ members: [memberRow(['owner'])] }), 'org-1')).toBe(true);
3758+
});
3759+
3760+
it('grades the ARRAY spelling ["member","Admin"] as an administrator', async () => {
3761+
expect(
3762+
await judge(makeEngine({ members: [memberRow(['member', 'Admin'])] }), 'org-1'),
3763+
).toBe(true);
3764+
});
3765+
});
3766+
3767+
// -- (2) regression: the closed ADR-0108 vocabulary answers identically -----
3768+
describe('closed membership vocabulary (ADR-0108) — unchanged by the new ruler', () => {
3769+
it.each([
3770+
['owner', true],
3771+
['admin', true],
3772+
['delegated_admin', false],
3773+
['member', false],
3774+
] as const)('grades the built-in %j as admin=%s', async (role, expected) => {
3775+
expect(await judge(makeEngine({ members: [memberRow(role)] }), 'org-1')).toBe(expected);
3776+
});
3777+
3778+
it.each([
3779+
['owner,member', true],
3780+
['member,admin', true],
3781+
[' admin ', true],
3782+
['member,delegated_admin', false],
3783+
] as const)(
3784+
'grades the comma/whitespace spelling %j as admin=%s (already true before #5942)',
3785+
async (role, expected) => {
3786+
expect(await judge(makeEngine({ members: [memberRow(role)] }), 'org-1')).toBe(expected);
3787+
},
3788+
);
3789+
});
3790+
3791+
// -- (3) non-administrative values still refused (no widening past admin) ---
3792+
describe('fail-closed floor — nothing else is admitted', () => {
3793+
it.each([
3794+
['manager', 'an app-registered name that is not an administrative grade'],
3795+
['administrator', 'a near-miss that is not the vocabulary'],
3796+
['adminx', 'a prefix collision'],
3797+
['', 'an empty role'],
3798+
])('refuses %j (%s)', async (role) => {
3799+
expect(await judge(makeEngine({ members: [memberRow(role)] }), 'org-1')).toBe(false);
3800+
});
3801+
3802+
it.each([
3803+
[null, 'null'],
3804+
[undefined, 'undefined'],
3805+
[42, 'a number'],
3806+
[{ role: 'owner' }, 'an object that merely mentions owner'],
3807+
])('refuses a non-string role (%s: %s)', async (role) => {
3808+
expect(await judge(makeEngine({ members: [memberRow(role)] }), 'org-1')).toBe(false);
3809+
});
3810+
3811+
it('refuses when the user has no membership row at all', async () => {
3812+
expect(await judge(makeEngine({ members: [] }), 'org-1')).toBe(false);
3813+
});
3814+
3815+
it('refuses when the engine read throws (fail CLOSED — ADR-0024)', async () => {
3816+
expect(await judge(makeEngine({ throws: true }), 'org-1')).toBe(false);
3817+
});
3818+
});
3819+
3820+
// -- (4) org scoping and the untouched platform-admin half -----------------
3821+
describe('scoping and the platform-admin half (untouched by #5942)', () => {
3822+
it('judges only the ACTIVE org when one is set', async () => {
3823+
const engine = makeEngine({
3824+
members: [
3825+
{ id: 'm-1', user_id: 'u-1', organization_id: 'org-other', role: 'Owner' },
3826+
{ id: 'm-2', user_id: 'u-1', organization_id: 'org-1', role: 'member' },
3827+
],
3828+
});
3829+
// Administrative elsewhere, plain member here → refused for org-1 …
3830+
expect(await judge(engine, 'org-1')).toBe(false);
3831+
// … and admitted when that other org is the active one.
3832+
expect(await judge(engine, 'org-other')).toBe(true);
3833+
});
3834+
3835+
it('accepts an administrative membership in ANY org when no active org is set', async () => {
3836+
const engine = makeEngine({
3837+
members: [{ id: 'm-1', user_id: 'u-1', organization_id: 'org-other', role: 'ADMIN' }],
3838+
});
3839+
expect(await judge(engine, undefined)).toBe(true);
3840+
});
3841+
3842+
it('still admits a platform admin whose membership is a plain member', async () => {
3843+
const engine = makeEngine({ platformAdmin: true, members: [memberRow('member')] });
3844+
expect(await judge(engine, 'org-1')).toBe(true);
3845+
});
3846+
3847+
it('still refuses a non-platform-admin with no administrative membership', async () => {
3848+
const engine = makeEngine({ platformAdmin: false, members: [memberRow('member')] });
3849+
expect(await judge(engine, 'org-1')).toBe(false);
3850+
});
3851+
});
3852+
});

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

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,11 @@ import { postureEnforcesWall, type TenancyPosture } from '@objectstack/spec/secu
2222
import { MCP_OAUTH_SCOPES } from '@objectstack/spec/ai';
2323
import { createObjectQLAdapterFactory, withSystemReadContext } from './objectql-adapter.js';
2424
import { runWithAuthActorScope, setAuthActorResolver } from './auth-actor-attribution.js';
25-
import { invitationRoleCapFailure, isPlainMemberInvitation } from './invitation-role-cap.js';
25+
import {
26+
invitationRoleCapFailure,
27+
isPlainMemberInvitation,
28+
isOrgAdminGrade,
29+
} from './invitation-role-cap.js';
2630
import { isPlaceholderEmail } from './placeholder-email.js';
2731
import { reconcileMembership, type MembershipPolicy } from './reconcile-membership.js';
2832
import type { TenancyService } from './tenancy-service.js';
@@ -3587,11 +3591,17 @@ export class AuthManager {
35873591
* True when `userId` is a platform admin (a `sys_user_permission_set` row
35883592
* pointing at `admin_full_access` with `organization_id = null`) OR an
35893593
* owner/admin member of `activeOrgId` (any org membership with role
3590-
* owner/admin when no active org is set). Mirrors the role-derivation in
3591-
* `customSession`; reads through `withSystemReadContext` so the lookups are
3592-
* not themselves RLS-scoped to the acting (possibly non-privileged) user.
3593-
* Fails CLOSED (returns false) on any lookup error — this backs a security
3594-
* gate, so an unverifiable actor must never pass.
3594+
* owner/admin when no active org is set). Reads through
3595+
* `withSystemReadContext` so the lookups are not themselves RLS-scoped to the
3596+
* acting (possibly non-privileged) user. Fails CLOSED (returns false) on any
3597+
* lookup error — this backs a security gate, so an unverifiable actor must
3598+
* never pass.
3599+
*
3600+
* [#5942] The membership half asks {@link isOrgAdminGrade} — the single grade
3601+
* ladder in `invitation-role-cap.ts`, shared with the break-glass ban guard —
3602+
* so "which membership is an administrator" has exactly one answer inside
3603+
* plugin-auth. The platform-admin half above is unchanged and still has its
3604+
* own derivations elsewhere (`resolve-authz-context.ts` is authoritative).
35953605
*/
35963606
private async isOrgOrPlatformAdmin(
35973607
userId: string,
@@ -3623,13 +3633,14 @@ export class AuthManager {
36233633
if (activeOrgId) where.organization_id = activeOrgId;
36243634
const members = await sys.find('sys_member', { where, limit: 10 });
36253635
for (const m of (Array.isArray(members) ? members : [])) {
3626-
const raw = typeof m?.role === 'string' ? m.role : '';
3627-
if (
3628-
raw
3629-
.split(',')
3630-
.map((s: string) => s.trim())
3631-
.some((r: string) => r === 'owner' || r === 'admin')
3632-
) {
3636+
// [#5942] The ONE grade ladder answers "does this membership administer
3637+
// the org" — never a hand-copied `role === 'owner' || role === 'admin'`.
3638+
// The copy that used to live here was case-SENSITIVE and string-only, so
3639+
// a `sys_member.role` of `Owner` / `ADMIN` / `['owner']` was refused
3640+
// here while `last-admin-ban-guard.ts` — same question, same ladder —
3641+
// counted that row AS an administrator. Two spellings of one security
3642+
// question cannot disagree if there is only one spelling.
3643+
if (isOrgAdminGrade(m?.role)) {
36333644
return true;
36343645
}
36353646
}

0 commit comments

Comments
 (0)