Skip to content

Commit 5954705

Browse files
fix(plugin-auth): a role change that grants an org-admin permission set names the set (#28086)
<!-- claude-projects-attribution: {"github_login":"os-elon-musk"} --> _Requested by **Elon** · [project thread](https://claude.ai/code/project/chan_01WedDzV26nniwxv7jC88B9D?thread=cmsg_01WedDzV26nniwxv7jC88B9D8eFziY3iYNvFsFdazc7W6k)_ Fixes #27862 Clause-②: no Metadata-protocol: none Skills-docs: none — `grep -rn "update-member-role\|permission_set_granted\|grantedPermissionSet\|organization_admin_no_bypass" skills content/docs/guides content/docs/protocol` finds no sentence about what a role change answers (one hit in `skills/objectstack-platform/references/operations.md:201` is about the first-admin `admin_full_access` bootstrap, a different grant). ## 维护者速读 - 改了什么:把成员角色改成管理员或所有者时,接口的返回里现在会说明这次顺带授予了哪个权限集(名称和显示名),控制台的提示也会写出来,比如「成员角色已更新。新角色还授予了权限集 Organization Administrator (no record bypass)。」 - 为什么改:改角色会自动给这个人加上组织管理员权限集,但提示只写「成员角色已更新」,管理员事后在用户的权限集页签上看到一个没人手动分配的权限集,不知道从哪来的。 - 风险与回滚:只加读不加写,授予什么完全不变(授予仍由 plugin-security 的协调器单独完成);读失败时就不显示这句,不会显示错的。回滚即 revert 本 PR。 - 先看:`packages/plugins/plugin-auth/src/role-change-granted-set.ts`(前后两次读授予、比出新增的那个);`auth-manager-before-hook.ts` / `auth-manager-after-hook.ts` 的两处接入;`sys-member.object.ts` 的两条 `outcomeMessages`。 - 两分钟验证:`pnpm --filter @objectstack/plugin-auth exec vitest run --maxWorkers=2 src/role-change-granted-set.test.ts -t 27862`,预期 9 个用例通过。 - 需要你做的:无,席位落地。 ## What changes `POST /api/v1/auth/organization/update-member-role` (the `sys_member` actions Change Role and Make Owner) now says which permission set the role change granted. - **plugin-auth, new `role-change-granted-set.ts`.** The global `before` hook reads the target member's org-admin grants (`sys_user_permission_set` rows naming `organization_admin` or `organization_admin_no_bypass`, by name) and stashes them on the request's endpoint context, the `__osPwChangeUserId` shape. The global `after` hook reads them again on a successful answer; a set held after and not before is named on the answer: `outcome: 'permission_set_granted'` and `grantedPermissionSet: { name, label }`, the label read from the security catalog (the name when the definition carries none). This is the shape #27843 gave invite-member's `outcome`. - **platform-objects.** `update_member_role` and `transfer_ownership` declare `outcomeMessages.permission_set_granted`, naming `${result.grantedPermissionSet.label}`; the generated bundles carry the en leaf and hand-translated zh-CN, ja-JP and es-ES leaves (re-extracted, so no source-hash entry claims a copied leaf). - **Purely additive.** The member row's own columns are untouched; a role change that grants nothing (admin to owner keeps the set already held, a demotion revokes it) and every refusal answer exactly what they answered before. No new error code, no spec key. - **What is granted is unchanged.** `plugin-security` is not in the diff. The new code calls only `find` (the deps object has no write member); the grant is still written by `reconcileOrgAdminGrant` alone. - **Fails toward silence.** A read that throws, a member id that resolves to no row, or a stash that did not reach the `after` hook leaves the answer as the vendor wrote it: a missing line, never a wrong one. ## Tests `packages/plugins/plugin-auth/src/role-change-granted-set.test.ts`: a real `AuthManager` over the installed better-auth, the role changed through the vendor's own route by the organization's owner. The grant is written by plugin-security's own `reconcileOrgAdminGrant`, fired after each `sys_member` update as the security plugin's middleware fires it. - (a) member → admin and member → owner answer `outcome: 'permission_set_granted'` and `grantedPermissionSet: { name: 'organization_admin_no_bypass', label: 'Organization Administrator (no record bypass)' }`, the member row's `role` and `userId` intact, and the grant table holds exactly that one set. - (b) admin → owner (set already held) and admin → member (revoked) answer no `outcome` / `grantedPermissionSet`; a plain member's attempt is refused 403 with neither, and nothing is granted. - Fail-toward-silence units: a before-read that throws, an after-read that throws, a set with no catalog label (named by its name), another route. ## Acceptance notes - The label is the catalog definition's `label` as written (English for the platform sets); the surrounding sentence is translated per locale. - Make Owner's `successMessage` and Change Role's stay as they were for the no-grant case. ## Verification <details><summary>Ablation (two, both reverted and verified by blob == HEAD)</summary> 1. `auth-manager-after-hook.ts`: the `stampRoleChangeGrantedSet` call replaced by `void roleChangeDeps;` → `Tests 3 failed | 6 passed`: both (a) cases and the admin → owner case (its first step asserts the grant line) went red. Restored: `git diff HEAD` empty. 2. `role-change-granted-set.ts`: `after.find((name) => !before.names.includes(name))` → `after.find(() => true)` (ignore the before-snapshot) → `Tests 1 failed | 8 passed`: admin → owner went red. Restored: `ok restored: blob == HEAD (ba71c8e)`. </details> <details><summary>Local gates (head a574295)</summary> - `dispatch-gates --commands`: 65 families derived; all 65 run with exit codes recorded; `--ran` reconciliation: "65 run, 0 NOT-MEASURED (a DERIVED zero — all 65 recorded an exit code and none of them is 3)". - Three families first answered exit 3 (PREREQUISITE NOT MET: `check:i18n` and `check:dual-build-cjs-loads` need built output; `check-plugin-teardown-shape --self-test` needs a commit the shallow clone lacked). Re-run after building the closure / fetching the commit: all exit 0. `check:i18n`: "OK (10 package(s) — all bundles in sync, no undeclared authoring keys)". - After the second commit (a test-file edit) the families that read test files were re-run on head `a574295a`: `check:type-check-coverage`, `check:type-check-debt`, `check:test-source-alias`, `check:undeclared-dep-imports`, `check:cross-package-test-inputs`, `check:engine-double-contract`, `check:tier-file-adoption`, `check-issue-citations`, `check:i18n`: all exit 0. The derived list is identical at both heads. - `pnpm --filter @objectstack/plugin-auth test`: Test Files 205 passed (205), Tests 3904 passed / 10 skipped. `typecheck`: exit 0. - `pnpm --filter @objectstack/platform-objects test` at head: Test Files 100 passed (100), Tests 1432 passed. `typecheck`: exit 0. - `pnpm lint` narrowed (declared): `eslint --no-inline-config` over the 9 changed `.ts` files, exit 0; the repo-wide run is CI's. </details> 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_016WV4czy9koaxQiZdkGdPYY --- _Generated by [Claude Code](https://claude.ai/code/session_016WV4czy9koaxQiZdkGdPYY)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 6410029 commit 5954705

13 files changed

Lines changed: 520 additions & 10 deletions
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
'@objectstack/plugin-auth': patch
3+
'@objectstack/platform-objects': patch
4+
---
5+
6+
fix(plugin-auth): a role change that grants an org-admin permission set says which set it granted (#27862)
7+
8+
Clause-②: yes (widening: update-member-role's success answer gains outcome 'permission_set_granted' and grantedPermissionSet; the two sys_member actions gain outcomeMessages)
9+
Metadata-protocol: none
10+
11+
Raising a member to `admin` or `owner` grants an org-admin permission set as a side effect (`organization_admin`, or `organization_admin_no_bypass` on a wall-less posture). The role change used to answer the member row alone, so the console toasted "Member role updated" and the administrator later found a set on the user that nobody visibly assigned.
12+
13+
- **Now.** When the role change left the member holding an org-admin set they did not hold before it, `POST /api/v1/auth/organization/update-member-role` answers two more members beside the member row: `outcome: 'permission_set_granted'` and `grantedPermissionSet: { name, label }`, the label read from the security catalog. The `sys_member` actions Change Role and Make Owner show it: "Member role updated. The new role also granted the permission set Organization Administrator (no record bypass)."
14+
- **Unchanged.** A role change that grants nothing (admin to owner keeps the set already held, a demotion revokes it) and a refusal answer exactly as before. The member row's own columns are untouched. What is granted does not change: the grant is still written by `plugin-security`'s reconciler alone, and the new hooks only read.
15+
- **Fails toward silence.** A grant read that fails leaves the answer as the route wrote it, without the line.

‎packages/platform-objects/src/apps/translations/en.objects.generated.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -694,7 +694,10 @@ export const enObjects: NonNullable<TranslationData['objects']> = {
694694
},
695695
update_member_role: {
696696
label: "Change Role",
697-
successMessage: "Member role updated"
697+
successMessage: "Member role updated",
698+
outcomeMessages: {
699+
permission_set_granted: "Member role updated. The new role also granted the permission set ${result.grantedPermissionSet.label}."
700+
}
698701
},
699702
remove_member: {
700703
label: "Remove Member",
@@ -704,7 +707,10 @@ export const enObjects: NonNullable<TranslationData['objects']> = {
704707
transfer_ownership: {
705708
label: "Make Owner",
706709
confirmText: "Make the selected member an owner of this organization? They get every owner privilege, including deleting the organization. You stay an owner too; to step down, use Change Role on your own membership.",
707-
successMessage: "Member is now an owner"
710+
successMessage: "Member is now an owner",
711+
outcomeMessages: {
712+
permission_set_granted: "Member is now an owner. The new role also granted the permission set ${result.grantedPermissionSet.label}."
713+
}
708714
}
709715
}
710716
},

‎packages/platform-objects/src/apps/translations/es-ES.objects.generated.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -694,7 +694,10 @@ export const esESObjects: NonNullable<TranslationData['objects']> = {
694694
},
695695
update_member_role: {
696696
label: "Cambiar rol",
697-
successMessage: "Rol del miembro actualizado"
697+
successMessage: "Rol del miembro actualizado",
698+
outcomeMessages: {
699+
permission_set_granted: "Rol del miembro actualizado. El nuevo rol también concedió el conjunto de permisos ${result.grantedPermissionSet.label}."
700+
}
698701
},
699702
remove_member: {
700703
label: "Eliminar miembro",
@@ -704,7 +707,10 @@ export const esESObjects: NonNullable<TranslationData['objects']> = {
704707
transfer_ownership: {
705708
label: "Hacer propietario",
706709
confirmText: "¿Hacer propietario de esta organización al miembro seleccionado? Obtendrá todos los privilegios de propietario, incluido eliminar la organización. Tú sigues siendo propietario; para dejar de serlo, usa «Cambiar rol» en tu propia membresía.",
707-
successMessage: "El miembro ahora es propietario"
710+
successMessage: "El miembro ahora es propietario",
711+
outcomeMessages: {
712+
permission_set_granted: "El miembro ahora es propietario. El nuevo rol también concedió el conjunto de permisos ${result.grantedPermissionSet.label}."
713+
}
708714
}
709715
}
710716
},

‎packages/platform-objects/src/apps/translations/ja-JP.objects.generated.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -694,7 +694,10 @@ export const jaJPObjects: NonNullable<TranslationData['objects']> = {
694694
},
695695
update_member_role: {
696696
label: "ロールを変更",
697-
successMessage: "メンバーロールを更新しました"
697+
successMessage: "メンバーロールを更新しました",
698+
outcomeMessages: {
699+
permission_set_granted: "メンバーのロールを更新しました。新しいロールにより権限セット ${result.grantedPermissionSet.label} も付与されました。"
700+
}
698701
},
699702
remove_member: {
700703
label: "メンバーを削除",
@@ -704,7 +707,10 @@ export const jaJPObjects: NonNullable<TranslationData['objects']> = {
704707
transfer_ownership: {
705708
label: "所有者にする",
706709
confirmText: "選択したメンバーをこの組織の所有者にしますか?このメンバーには、組織の削除を含む所有者のすべての権限が付与されます。あなたも引き続き所有者です。退く場合は、自分のメンバーシップで「ロールを変更」を使用してください。",
707-
successMessage: "メンバーが所有者になりました"
710+
successMessage: "メンバーが所有者になりました",
711+
outcomeMessages: {
712+
permission_set_granted: "このメンバーはオーナーになりました。新しいロールにより権限セット ${result.grantedPermissionSet.label} も付与されました。"
713+
}
708714
}
709715
}
710716
},

‎packages/platform-objects/src/apps/translations/zh-CN.objects.generated.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -694,7 +694,10 @@ export const zhCNObjects: NonNullable<TranslationData['objects']> = {
694694
},
695695
update_member_role: {
696696
label: "更改角色",
697-
successMessage: "成员角色已更新"
697+
successMessage: "成员角色已更新",
698+
outcomeMessages: {
699+
permission_set_granted: "成员角色已更新。新角色还授予了权限集 ${result.grantedPermissionSet.label}。"
700+
}
698701
},
699702
remove_member: {
700703
label: "移除成员",
@@ -704,7 +707,10 @@ export const zhCNObjects: NonNullable<TranslationData['objects']> = {
704707
transfer_ownership: {
705708
label: "设为所有者",
706709
confirmText: "确定要将所选成员设为该组织的所有者吗?对方将获得所有者的全部权限,包括删除该组织。您仍是所有者;如需卸任,请在您自己的成员记录上使用“更改角色”。",
707-
successMessage: "该成员现已成为所有者"
710+
successMessage: "该成员现已成为所有者",
711+
outcomeMessages: {
712+
permission_set_granted: "该成员现已成为所有者。新角色还授予了权限集 ${result.grantedPermissionSet.label}。"
713+
}
708714
}
709715
}
710716
},

‎packages/platform-objects/src/identity/sys-member.object.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,14 @@ export const SysMember = ObjectSchema.create({
212212
requiresFeature: 'organization',
213213
requiresMembershipReach: 'update_member_role',
214214
successMessage: 'Member role updated',
215+
// [#27862] A role change that grants an org-admin permission set says
216+
// which one: update-member-role then answers
217+
// `outcome: 'permission_set_granted'` and `grantedPermissionSet` beside
218+
// the member row (plugin-auth `role-change-granted-set.ts`).
219+
outcomeMessages: {
220+
permission_set_granted:
221+
'Member role updated. The new role also granted the permission set ${result.grantedPermissionSet.label}.',
222+
},
215223
refreshAfter: true,
216224
params: [
217225
{ field: 'role', required: true, defaultFromRow: true },
@@ -271,6 +279,11 @@ export const SysMember = ObjectSchema.create({
271279
requiresMembershipReach: 'transfer_ownership',
272280
confirmText: 'Make the selected member an owner of this organization? They get every owner privilege, including deleting the organization. You stay an owner too; to step down, use Change Role on your own membership.',
273281
successMessage: 'Member is now an owner',
282+
// [#27862] The same answer as `update_member_role` (the same route).
283+
outcomeMessages: {
284+
permission_set_granted:
285+
'Member is now an owner. The new role also granted the permission set ${result.grantedPermissionSet.label}.',
286+
},
274287
refreshAfter: true,
275288
},
276289
],

‎packages/platform-objects/src/identity/transfer-ownership-action-copy.test.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ interface CopyAction {
2828
label?: string;
2929
confirmText?: string;
3030
successMessage?: string;
31+
outcomeMessages?: Record<string, string>;
3132
target?: string;
3233
bodyExtra?: Record<string, unknown>;
3334
recordIdParam?: string;
@@ -71,7 +72,7 @@ describe('sys_member.transfer_ownership — declared copy', () => {
7172
});
7273

7374
it('promises neither a demotion nor a transfer', () => {
74-
for (const text of [action.label, action.confirmText, action.successMessage]) {
75+
for (const text of [action.label, action.confirmText, action.successMessage, ...Object.values(action.outcomeMessages ?? {})]) {
7576
expect(text).not.toMatch(/demot|transfer/i);
7677
}
7778
expect(action.confirmText).toContain('You stay an owner');
@@ -93,6 +94,11 @@ describe('sys_member.transfer_ownership — every locale bundle', () => {
9394
it('en is the declared copy', () => {
9495
const copy = bundled(enObjects, 'transfer_ownership');
9596
const action = declared('transfer_ownership');
96-
expect(copy).toEqual({ label: action.label, confirmText: action.confirmText, successMessage: action.successMessage });
97+
expect(copy).toEqual({
98+
label: action.label,
99+
confirmText: action.confirmText,
100+
successMessage: action.successMessage,
101+
outcomeMessages: action.outcomeMessages,
102+
});
97103
});
98104
});

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ import { rotateCallerBearerOnImpersonation } from './impersonation-bearer-rotati
4747
import { stampInvitationDeliveryOutcome } from './invitation-delivery-outcome.js';
4848
import { codeInviteRoleNotFound } from './invite-role-not-found-code.js';
4949
import { readRequestClientAddress } from './request-client-address.js';
50+
import { stampRoleChangeGrantedSet } from './role-change-granted-set.js';
5051
import { attachSessionToCredentialResponse } from './session-envelope-completion.js';
5152
import { resetVerifiedOnTwoFactorReenrollment } from './two-factor-reenrollment-verified-reset.js';
5253
import { echoInstalledSessionToken } from './two-factor-rotated-token-echo.js';
@@ -64,6 +65,7 @@ export interface AfterHookHost {
6465
isEmailDeliveryLive: AuthManager['isEmailDeliveryLive'];
6566
recordPasswordHistory: AuthManager['recordPasswordHistory'];
6667
recordSignInOutcome: AuthManager['recordSignInOutcome'];
68+
roleChangeGrantDeps: AuthManager['roleChangeGrantDeps'];
6769
sendChangeEmailNotice: AuthManager['sendChangeEmailNotice'];
6870
stampLastLogin: AuthManager['stampLastLogin'];
6971
stampPasswordChangedAt: AuthManager['stampPasswordChangedAt'];
@@ -232,6 +234,14 @@ export async function globalAfterHook(this: AfterHookHost, ctx: any) {
232234
// `invitation-delivery-outcome.ts`). A refusal passes untouched.
233235
await stampInvitationDeliveryOutcome(ctx, () => this.isEmailDeliveryLive());
234236

237+
// ── #27862: a role change names the permission set it granted ──
238+
// The before hook read the member's org-admin grants; a success that
239+
// left a new one held answers `outcome: 'permission_set_granted'` and
240+
// `grantedPermissionSet: { name, label }` beside the member row (see
241+
// `role-change-granted-set.ts`). A refusal passes untouched.
242+
const roleChangeDeps = this.roleChangeGrantDeps();
243+
if (roleChangeDeps) await stampRoleChangeGrantedSet(ctx, roleChangeDeps);
244+
235245
// ── #24750: invite-member's unknown-role refusal carries its code ──
236246
// better-auth throws it with only a message; the spec promises
237247
// `ROLE_NOT_FOUND` (see `invite-role-not-found-code.ts`). It throws

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ export interface AuthInstanceHost {
118118
resolveAuthSecret: AuthManager['resolveAuthSecret'];
119119
resolvePasswordChangeUserId: AuthManager['resolvePasswordChangeUserId'];
120120
resolvePasswordHasher: AuthManager['resolvePasswordHasher'];
121+
roleChangeGrantDeps: AuthManager['roleChangeGrantDeps'];
121122
sendChangeEmailNotice: AuthManager['sendChangeEmailNotice'];
122123
sendExistingAccountSignUpNotice: AuthManager['sendExistingAccountSignUpNotice'];
123124
signUpForcedOff: AuthManager['signUpForcedOff'];
@@ -706,6 +707,7 @@ export async function createAuthInstance(this: AuthInstanceHost): Promise<Auth<a
706707
isPlatformAdminUserId: (...a) => this.isPlatformAdminUserId(...a),
707708
resolveActor: (...a) => this.resolveActor(...a),
708709
resolvePasswordChangeUserId: (...a) => this.resolvePasswordChangeUserId(...a),
710+
roleChangeGrantDeps: (...a) => this.roleChangeGrantDeps(...a),
709711
validateAudienceAdmission: (...a) => this.validateAudienceAdmission(...a),
710712
};
711713
return globalBeforeHook.call(host, ctx, setShouldSkipSessionRefresh);
@@ -720,6 +722,7 @@ export async function createAuthInstance(this: AuthInstanceHost): Promise<Auth<a
720722
isEmailDeliveryLive: (...a) => this.isEmailDeliveryLive(...a),
721723
recordPasswordHistory: (...a) => this.recordPasswordHistory(...a),
722724
recordSignInOutcome: (...a) => this.recordSignInOutcome(...a),
725+
roleChangeGrantDeps: (...a) => this.roleChangeGrantDeps(...a),
723726
sendChangeEmailNotice: (...a) => this.sendChangeEmailNotice(...a),
724727
stampLastLogin: (...a) => this.stampLastLogin(...a),
725728
stampPasswordChangedAt: (...a) => this.stampPasswordChangedAt(...a),

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ import {
7171
} from './last-local-credential.js';
7272
import { ORGANIZATION_DELETE_PATH } from './organization-delete.js';
7373
import { carriesPrincipalSession } from './principal-session-mint.js';
74+
import { snapshotRoleChangeGrants, UPDATE_MEMBER_ROLE_PATH } from './role-change-granted-set.js';
7475

7576
/**
7677
* The `AuthManager` members the function below reads — every one of them, and
@@ -98,6 +99,7 @@ export interface BeforeHookHost {
9899
isPlatformAdminUserId: AuthManager['isPlatformAdminUserId'];
99100
resolveActor: AuthManager['resolveActor'];
100101
resolvePasswordChangeUserId: AuthManager['resolvePasswordChangeUserId'];
102+
roleChangeGrantDeps: AuthManager['roleChangeGrantDeps'];
101103
validateAudienceAdmission: AuthManager['validateAudienceAdmission'];
102104
}
103105

@@ -660,6 +662,15 @@ export async function globalBeforeHook(
660662
});
661663
}
662664

665+
// ── #27862: read the member's org-admin grants before a role change ──
666+
// The after hook compares them with what the change left held and names
667+
// the set it granted (see `role-change-granted-set.ts`). A read only: it
668+
// gates nothing, and a read that fails stashes nothing.
669+
if (ctx?.path === UPDATE_MEMBER_ROLE_PATH) {
670+
const roleChangeDeps = this.roleChangeGrantDeps();
671+
if (roleChangeDeps) await snapshotRoleChangeGrants(ctx, roleChangeDeps);
672+
}
673+
663674
// ── ADR-0069 D2: account lockout (gate) ─────────────────────
664675
// Reject a sign-in for a locked identity BEFORE better-auth checks
665676
// the password — a lock must hold even against the correct password.

0 commit comments

Comments
 (0)