diff --git a/.changeset/27862-role-change-names-granted-set.md b/.changeset/27862-role-change-names-granted-set.md new file mode 100644 index 00000000000..eb8d5f40e01 --- /dev/null +++ b/.changeset/27862-role-change-names-granted-set.md @@ -0,0 +1,15 @@ +--- +'@objectstack/plugin-auth': patch +'@objectstack/platform-objects': patch +--- + +fix(plugin-auth): a role change that grants an org-admin permission set says which set it granted (#27862) + +Clause-②: yes (widening: update-member-role's success answer gains outcome 'permission_set_granted' and grantedPermissionSet; the two sys_member actions gain outcomeMessages) +Metadata-protocol: none + +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. + +- **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)." +- **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. +- **Fails toward silence.** A grant read that fails leaves the answer as the route wrote it, without the line. diff --git a/packages/platform-objects/src/apps/translations/en.objects.generated.ts b/packages/platform-objects/src/apps/translations/en.objects.generated.ts index c1248a40c1e..fff816ff235 100644 --- a/packages/platform-objects/src/apps/translations/en.objects.generated.ts +++ b/packages/platform-objects/src/apps/translations/en.objects.generated.ts @@ -694,7 +694,10 @@ export const enObjects: NonNullable = { }, update_member_role: { label: "Change Role", - successMessage: "Member role updated" + successMessage: "Member role updated", + outcomeMessages: { + permission_set_granted: "Member role updated. The new role also granted the permission set ${result.grantedPermissionSet.label}." + } }, remove_member: { label: "Remove Member", @@ -704,7 +707,10 @@ export const enObjects: NonNullable = { transfer_ownership: { label: "Make Owner", 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.", - successMessage: "Member is now an owner" + successMessage: "Member is now an owner", + outcomeMessages: { + permission_set_granted: "Member is now an owner. The new role also granted the permission set ${result.grantedPermissionSet.label}." + } } } }, diff --git a/packages/platform-objects/src/apps/translations/es-ES.objects.generated.ts b/packages/platform-objects/src/apps/translations/es-ES.objects.generated.ts index 7d60c1cfed4..f185d7b0867 100644 --- a/packages/platform-objects/src/apps/translations/es-ES.objects.generated.ts +++ b/packages/platform-objects/src/apps/translations/es-ES.objects.generated.ts @@ -694,7 +694,10 @@ export const esESObjects: NonNullable = { }, update_member_role: { label: "Cambiar rol", - successMessage: "Rol del miembro actualizado" + successMessage: "Rol del miembro actualizado", + outcomeMessages: { + permission_set_granted: "Rol del miembro actualizado. El nuevo rol también concedió el conjunto de permisos ${result.grantedPermissionSet.label}." + } }, remove_member: { label: "Eliminar miembro", @@ -704,7 +707,10 @@ export const esESObjects: NonNullable = { transfer_ownership: { label: "Hacer propietario", 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.", - successMessage: "El miembro ahora es propietario" + successMessage: "El miembro ahora es propietario", + outcomeMessages: { + permission_set_granted: "El miembro ahora es propietario. El nuevo rol también concedió el conjunto de permisos ${result.grantedPermissionSet.label}." + } } } }, diff --git a/packages/platform-objects/src/apps/translations/ja-JP.objects.generated.ts b/packages/platform-objects/src/apps/translations/ja-JP.objects.generated.ts index 6c66a9d97f0..ac96bed3dd1 100644 --- a/packages/platform-objects/src/apps/translations/ja-JP.objects.generated.ts +++ b/packages/platform-objects/src/apps/translations/ja-JP.objects.generated.ts @@ -694,7 +694,10 @@ export const jaJPObjects: NonNullable = { }, update_member_role: { label: "ロールを変更", - successMessage: "メンバーロールを更新しました" + successMessage: "メンバーロールを更新しました", + outcomeMessages: { + permission_set_granted: "メンバーのロールを更新しました。新しいロールにより権限セット ${result.grantedPermissionSet.label} も付与されました。" + } }, remove_member: { label: "メンバーを削除", @@ -704,7 +707,10 @@ export const jaJPObjects: NonNullable = { transfer_ownership: { label: "所有者にする", confirmText: "選択したメンバーをこの組織の所有者にしますか？このメンバーには、組織の削除を含む所有者のすべての権限が付与されます。あなたも引き続き所有者です。退く場合は、自分のメンバーシップで「ロールを変更」を使用してください。", - successMessage: "メンバーが所有者になりました" + successMessage: "メンバーが所有者になりました", + outcomeMessages: { + permission_set_granted: "このメンバーはオーナーになりました。新しいロールにより権限セット ${result.grantedPermissionSet.label} も付与されました。" + } } } }, diff --git a/packages/platform-objects/src/apps/translations/zh-CN.objects.generated.ts b/packages/platform-objects/src/apps/translations/zh-CN.objects.generated.ts index 3b7e6359a9e..2f023867d83 100644 --- a/packages/platform-objects/src/apps/translations/zh-CN.objects.generated.ts +++ b/packages/platform-objects/src/apps/translations/zh-CN.objects.generated.ts @@ -694,7 +694,10 @@ export const zhCNObjects: NonNullable = { }, update_member_role: { label: "更改角色", - successMessage: "成员角色已更新" + successMessage: "成员角色已更新", + outcomeMessages: { + permission_set_granted: "成员角色已更新。新角色还授予了权限集 ${result.grantedPermissionSet.label}。" + } }, remove_member: { label: "移除成员", @@ -704,7 +707,10 @@ export const zhCNObjects: NonNullable = { transfer_ownership: { label: "设为所有者", confirmText: "确定要将所选成员设为该组织的所有者吗？对方将获得所有者的全部权限，包括删除该组织。您仍是所有者；如需卸任，请在您自己的成员记录上使用“更改角色”。", - successMessage: "该成员现已成为所有者" + successMessage: "该成员现已成为所有者", + outcomeMessages: { + permission_set_granted: "该成员现已成为所有者。新角色还授予了权限集 ${result.grantedPermissionSet.label}。" + } } } }, diff --git a/packages/platform-objects/src/identity/sys-member.object.ts b/packages/platform-objects/src/identity/sys-member.object.ts index 920dd2d0681..e4500454da6 100644 --- a/packages/platform-objects/src/identity/sys-member.object.ts +++ b/packages/platform-objects/src/identity/sys-member.object.ts @@ -212,6 +212,14 @@ export const SysMember = ObjectSchema.create({ requiresFeature: 'organization', requiresMembershipReach: 'update_member_role', successMessage: 'Member role updated', + // [#27862] A role change that grants an org-admin permission set says + // which one: update-member-role then answers + // `outcome: 'permission_set_granted'` and `grantedPermissionSet` beside + // the member row (plugin-auth `role-change-granted-set.ts`). + outcomeMessages: { + permission_set_granted: + 'Member role updated. The new role also granted the permission set ${result.grantedPermissionSet.label}.', + }, refreshAfter: true, params: [ { field: 'role', required: true, defaultFromRow: true }, @@ -271,6 +279,11 @@ export const SysMember = ObjectSchema.create({ requiresMembershipReach: 'transfer_ownership', 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.', successMessage: 'Member is now an owner', + // [#27862] The same answer as `update_member_role` (the same route). + outcomeMessages: { + permission_set_granted: + 'Member is now an owner. The new role also granted the permission set ${result.grantedPermissionSet.label}.', + }, refreshAfter: true, }, ], diff --git a/packages/platform-objects/src/identity/transfer-ownership-action-copy.test.ts b/packages/platform-objects/src/identity/transfer-ownership-action-copy.test.ts index 0827576fd1a..331893a19a3 100644 --- a/packages/platform-objects/src/identity/transfer-ownership-action-copy.test.ts +++ b/packages/platform-objects/src/identity/transfer-ownership-action-copy.test.ts @@ -28,6 +28,7 @@ interface CopyAction { label?: string; confirmText?: string; successMessage?: string; + outcomeMessages?: Record; target?: string; bodyExtra?: Record; recordIdParam?: string; @@ -71,7 +72,7 @@ describe('sys_member.transfer_ownership — declared copy', () => { }); it('promises neither a demotion nor a transfer', () => { - for (const text of [action.label, action.confirmText, action.successMessage]) { + for (const text of [action.label, action.confirmText, action.successMessage, ...Object.values(action.outcomeMessages ?? {})]) { expect(text).not.toMatch(/demot|transfer/i); } expect(action.confirmText).toContain('You stay an owner'); @@ -93,6 +94,11 @@ describe('sys_member.transfer_ownership — every locale bundle', () => { it('en is the declared copy', () => { const copy = bundled(enObjects, 'transfer_ownership'); const action = declared('transfer_ownership'); - expect(copy).toEqual({ label: action.label, confirmText: action.confirmText, successMessage: action.successMessage }); + expect(copy).toEqual({ + label: action.label, + confirmText: action.confirmText, + successMessage: action.successMessage, + outcomeMessages: action.outcomeMessages, + }); }); }); diff --git a/packages/plugins/plugin-auth/src/auth-manager-after-hook.ts b/packages/plugins/plugin-auth/src/auth-manager-after-hook.ts index 8c20449115d..299e9674b2e 100644 --- a/packages/plugins/plugin-auth/src/auth-manager-after-hook.ts +++ b/packages/plugins/plugin-auth/src/auth-manager-after-hook.ts @@ -47,6 +47,7 @@ import { rotateCallerBearerOnImpersonation } from './impersonation-bearer-rotati import { stampInvitationDeliveryOutcome } from './invitation-delivery-outcome.js'; import { codeInviteRoleNotFound } from './invite-role-not-found-code.js'; import { readRequestClientAddress } from './request-client-address.js'; +import { stampRoleChangeGrantedSet } from './role-change-granted-set.js'; import { attachSessionToCredentialResponse } from './session-envelope-completion.js'; import { resetVerifiedOnTwoFactorReenrollment } from './two-factor-reenrollment-verified-reset.js'; import { echoInstalledSessionToken } from './two-factor-rotated-token-echo.js'; @@ -64,6 +65,7 @@ export interface AfterHookHost { isEmailDeliveryLive: AuthManager['isEmailDeliveryLive']; recordPasswordHistory: AuthManager['recordPasswordHistory']; recordSignInOutcome: AuthManager['recordSignInOutcome']; + roleChangeGrantDeps: AuthManager['roleChangeGrantDeps']; sendChangeEmailNotice: AuthManager['sendChangeEmailNotice']; stampLastLogin: AuthManager['stampLastLogin']; stampPasswordChangedAt: AuthManager['stampPasswordChangedAt']; @@ -232,6 +234,14 @@ export async function globalAfterHook(this: AfterHookHost, ctx: any) { // `invitation-delivery-outcome.ts`). A refusal passes untouched. await stampInvitationDeliveryOutcome(ctx, () => this.isEmailDeliveryLive()); + // ── #27862: a role change names the permission set it granted ── + // The before hook read the member's org-admin grants; a success that + // left a new one held answers `outcome: 'permission_set_granted'` and + // `grantedPermissionSet: { name, label }` beside the member row (see + // `role-change-granted-set.ts`). A refusal passes untouched. + const roleChangeDeps = this.roleChangeGrantDeps(); + if (roleChangeDeps) await stampRoleChangeGrantedSet(ctx, roleChangeDeps); + // ── #24750: invite-member's unknown-role refusal carries its code ── // better-auth throws it with only a message; the spec promises // `ROLE_NOT_FOUND` (see `invite-role-not-found-code.ts`). It throws diff --git a/packages/plugins/plugin-auth/src/auth-manager-auth-instance.ts b/packages/plugins/plugin-auth/src/auth-manager-auth-instance.ts index 214359a7340..0f142b93fcb 100644 --- a/packages/plugins/plugin-auth/src/auth-manager-auth-instance.ts +++ b/packages/plugins/plugin-auth/src/auth-manager-auth-instance.ts @@ -118,6 +118,7 @@ export interface AuthInstanceHost { resolveAuthSecret: AuthManager['resolveAuthSecret']; resolvePasswordChangeUserId: AuthManager['resolvePasswordChangeUserId']; resolvePasswordHasher: AuthManager['resolvePasswordHasher']; + roleChangeGrantDeps: AuthManager['roleChangeGrantDeps']; sendChangeEmailNotice: AuthManager['sendChangeEmailNotice']; sendExistingAccountSignUpNotice: AuthManager['sendExistingAccountSignUpNotice']; signUpForcedOff: AuthManager['signUpForcedOff']; @@ -706,6 +707,7 @@ export async function createAuthInstance(this: AuthInstanceHost): Promise this.isPlatformAdminUserId(...a), resolveActor: (...a) => this.resolveActor(...a), resolvePasswordChangeUserId: (...a) => this.resolvePasswordChangeUserId(...a), + roleChangeGrantDeps: (...a) => this.roleChangeGrantDeps(...a), validateAudienceAdmission: (...a) => this.validateAudienceAdmission(...a), }; return globalBeforeHook.call(host, ctx, setShouldSkipSessionRefresh); @@ -720,6 +722,7 @@ export async function createAuthInstance(this: AuthInstanceHost): Promise this.isEmailDeliveryLive(...a), recordPasswordHistory: (...a) => this.recordPasswordHistory(...a), recordSignInOutcome: (...a) => this.recordSignInOutcome(...a), + roleChangeGrantDeps: (...a) => this.roleChangeGrantDeps(...a), sendChangeEmailNotice: (...a) => this.sendChangeEmailNotice(...a), stampLastLogin: (...a) => this.stampLastLogin(...a), stampPasswordChangedAt: (...a) => this.stampPasswordChangedAt(...a), diff --git a/packages/plugins/plugin-auth/src/auth-manager-before-hook.ts b/packages/plugins/plugin-auth/src/auth-manager-before-hook.ts index dd1bd1b551d..f1f1fec3362 100644 --- a/packages/plugins/plugin-auth/src/auth-manager-before-hook.ts +++ b/packages/plugins/plugin-auth/src/auth-manager-before-hook.ts @@ -71,6 +71,7 @@ import { } from './last-local-credential.js'; import { ORGANIZATION_DELETE_PATH } from './organization-delete.js'; import { carriesPrincipalSession } from './principal-session-mint.js'; +import { snapshotRoleChangeGrants, UPDATE_MEMBER_ROLE_PATH } from './role-change-granted-set.js'; /** * The `AuthManager` members the function below reads — every one of them, and @@ -98,6 +99,7 @@ export interface BeforeHookHost { isPlatformAdminUserId: AuthManager['isPlatformAdminUserId']; resolveActor: AuthManager['resolveActor']; resolvePasswordChangeUserId: AuthManager['resolvePasswordChangeUserId']; + roleChangeGrantDeps: AuthManager['roleChangeGrantDeps']; validateAudienceAdmission: AuthManager['validateAudienceAdmission']; } @@ -660,6 +662,15 @@ export async function globalBeforeHook( }); } + // ── #27862: read the member's org-admin grants before a role change ── + // The after hook compares them with what the change left held and names + // the set it granted (see `role-change-granted-set.ts`). A read only: it + // gates nothing, and a read that fails stashes nothing. + if (ctx?.path === UPDATE_MEMBER_ROLE_PATH) { + const roleChangeDeps = this.roleChangeGrantDeps(); + if (roleChangeDeps) await snapshotRoleChangeGrants(ctx, roleChangeDeps); + } + // ── ADR-0069 D2: account lockout (gate) ───────────────────── // Reject a sign-in for a locked identity BEFORE better-auth checks // the password — a lock must hold even against the correct password. diff --git a/packages/plugins/plugin-auth/src/auth-manager.ts b/packages/plugins/plugin-auth/src/auth-manager.ts index 2175fb83308..e0462cdddf7 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.ts @@ -58,6 +58,7 @@ import { MCP_OAUTH_SCOPES } from '@objectstack/spec/ai'; import type { RegisteredErrorCode, StandardErrorCode } from '@objectstack/spec/api'; import { createObjectQLAdapterFactory, withSystemReadContext } from './objectql-adapter.js'; import { type ExternalDirectoryDoorDeps } from './external-directory-door.js'; +import { roleChangeGrantDepsFor, type RoleChangeGrantDeps } from './role-change-granted-set.js'; import { isExternalMembershipRole } from './external-membership.js'; import { permissionSetInEffect } from './catalog-set-in-effect.js'; import { recoverInternalFieldsForSystemRead } from './internal-field-readback.js'; @@ -1701,6 +1702,7 @@ export class AuthManager { resolveAuthSecret: (...a) => this.resolveAuthSecret(...a), resolvePasswordChangeUserId: (...a) => this.resolvePasswordChangeUserId(...a), resolvePasswordHasher: (...a) => this.resolvePasswordHasher(...a), + roleChangeGrantDeps: (...a) => this.roleChangeGrantDeps(...a), sendChangeEmailNotice: (...a) => this.sendChangeEmailNotice(...a), sendExistingAccountSignUpNotice: (...a) => this.sendExistingAccountSignUpNotice(...a), signUpForcedOff: (...a) => this.signUpForcedOff(...a), @@ -4784,6 +4786,16 @@ export class AuthManager { }; } + /** + * [#27862] The reads the role-change hooks make to name the set a role change + * granted, bound to this manager's engine under the system read context. With + * no engine there is nothing to read, so no answer names a set. + */ + private roleChangeGrantDeps(): RoleChangeGrantDeps | undefined { + const engine = this.config.dataEngine; + return engine ? roleChangeGrantDepsFor(engine, withSystemReadContext(engine) as any) : undefined; + } + private resolveActor( ctx: any, options?: { failedReadThrows?: boolean }, diff --git a/packages/plugins/plugin-auth/src/role-change-granted-set.test.ts b/packages/plugins/plugin-auth/src/role-change-granted-set.test.ts new file mode 100644 index 00000000000..9578a207635 --- /dev/null +++ b/packages/plugins/plugin-auth/src/role-change-granted-set.test.ts @@ -0,0 +1,243 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #27862 — a role change names the permission set it granted. The card's two +// acceptance rows: +// +// (a) a role change that triggers the org-admin auto-grant answers the +// granted set's name and catalog label beside the member row +// (`outcome: 'permission_set_granted'`); +// (b) a role change that grants nothing (admin → owner keeps the set it +// already held, a demotion revokes it) answers the member row alone, and +// so does a refusal. +// +// The role is changed through the vendor's own route on a real `AuthManager` +// over the installed better-auth. The grant is written by plugin-security's +// own reconciler (`reconcileOrgAdminGrant`), fired after each `sys_member` +// update exactly as the security plugin's middleware fires it, so the rows the +// hooks read are the rows the product writes. + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { ORGANIZATION_ADMIN, ORGANIZATION_ADMIN_NO_BYPASS } from '@objectstack/spec'; +import { reconcileOrgAdminGrant } from '@objectstack/plugin-security'; +import { AuthManager } from './auth-manager'; +import { createMemoryEngine } from './impersonation-bearer-rotation.test'; +import { inviteForAudienceGate } from './audience-gate-test-support'; +import { bindTestSecurityCatalog } from './__tests__/security-catalog.testkit'; +import { + PERMISSION_SET_GRANTED_OUTCOME, + UPDATE_MEMBER_ROLE_PATH, + snapshotRoleChangeGrants, + stampRoleChangeGrantedSet, + type RoleChangeGrantDeps, +} from './role-change-granted-set'; + +const SECRET = 'test-secret-at-least-32-chars-long!!'; +const PASSWORD = 'S3cure!Passw0rd-27862'; +const ORIGIN = 'https://crm.acme.test'; +const BASE = '/api/v1/auth'; +const DEFAULT_ORG = 'org_default'; +const ACME_ORG = 'org_acme'; +const NO_BYPASS_LABEL = 'Organization Administrator (no record bypass)'; + +const singleOrgTenancy = () => + ({ + posture: 'single', + requestedPosture: 'single', + isolationActive: false, + requested: false, + degraded: false, + defaultOrgId: async () => DEFAULT_ORG, + }) as any; + +const cookieFrom = (response: Response): string => + (response.headers.getSetCookie?.() ?? [response.headers.get('set-cookie') ?? '']) + .map((c) => c.split(';')[0]) + .filter(Boolean) + .join('; '); + +/** `Alice Owner` owns `Acme Corp`; Mo is a plain member there. */ +async function stage() { + const engine = createMemoryEngine(); + // The catalog the reconciler checks the set against and the label is read from. + bindTestSecurityCatalog(engine, { + permissions: [ + { name: ORGANIZATION_ADMIN, label: 'Organization Administrator' }, + { name: ORGANIZATION_ADMIN_NO_BYPASS, label: NO_BYPASS_LABEL }, + ], + }); + // plugin-security's sys_member middleware, as it fires: after the write, + // reconcile the pair the row names (the single posture: the no-bypass set). + const update = engine.update.bind(engine); + engine.update = (async (name: string, data: any, options?: any) => { + const result = await update(name, data, options); + if (name === 'sys_member' && result && typeof result === 'object') { + await reconcileOrgAdminGrant(engine, result.user_id, result.organization_id, { posture: 'single' }); + } + return result; + }) as typeof engine.update; + engine.tables.set('sys_organization', [ + { id: DEFAULT_ORG, name: 'Default', slug: 'default' }, + { id: ACME_ORG, name: 'Acme Corp', slug: 'acme' }, + ]); + const manager = new AuthManager({ + secret: SECRET, + baseUrl: ORIGIN, + dataEngine: engine as any, + membershipPolicy: 'auto', + getTenancy: () => singleOrgTenancy(), + } as any); + + const post = (path: string, body: unknown, cookie?: string) => + manager.handleRequest( + new Request(`${ORIGIN}${BASE}${path}`, { + method: 'POST', + headers: { origin: ORIGIN, 'Content-Type': 'application/json', ...(cookie ? { cookie } : {}) }, + body: JSON.stringify(body), + }), + ); + const signUp = async (email: string, name: string) => { + await inviteForAudienceGate(engine, email); + const res = await post('/sign-up/email', { email, password: PASSWORD, name }); + expect(res.status, `sign-up ${email}: ${await res.clone().text()}`).toBe(200); + const user = (engine.tables.get('sys_user') ?? []).find((u: any) => u.email === email); + return { cookie: cookieFrom(res), userId: String(user!.id) }; + }; + const bind = (userId: string, role: string) => + engine.insert('sys_member', { + id: `mem_${userId}_${ACME_ORG}`, + organization_id: ACME_ORG, + user_id: userId, + role, + created_at: new Date(), + }); + + const owner = await signUp('alice.owner@example.com', 'Alice Owner'); + await bind(owner.userId, 'owner'); + const member = await signUp('mo.member@example.com', 'Mo Member'); + await bind(member.userId, 'member'); + const memberRowId = `mem_${member.userId}_${ACME_ORG}`; + + const changeRole = async (role: string, cookie = owner.cookie) => { + const res = await post(UPDATE_MEMBER_ROLE_PATH, { memberId: memberRowId, role, organizationId: ACME_ORG }, cookie); + const text = await res.clone().text(); + return { status: res.status, body: JSON.parse(text), text }; + }; + const grantsOf = (userId: string) => + (engine.tables.get('sys_user_permission_set') ?? []) + .filter((g: any) => g.user_id === userId && g.organization_id === ACME_ORG) + .map((g: any) => g.permission_set) + .sort(); + + return { engine, owner, member, changeRole, grantsOf }; +} + +beforeEach(() => { + vi.spyOn(console, 'info').mockImplementation(() => {}); + vi.spyOn(console, 'warn').mockImplementation(() => {}); + vi.spyOn(console, 'error').mockImplementation(() => {}); + vi.spyOn(console, 'log').mockImplementation(() => {}); +}); +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe('#27862 (a) — a role change that auto-grants names the set', () => { + it('member → admin: the answer names the granted set by name and label, beside the member row', async () => { + const { member, changeRole, grantsOf } = await stage(); + expect(grantsOf(member.userId)).toEqual([]); + const answer = await changeRole('admin'); + expect(answer.status, answer.text).toBe(200); + expect(answer.body.outcome).toBe(PERMISSION_SET_GRANTED_OUTCOME); + expect(answer.body.grantedPermissionSet).toEqual({ name: ORGANIZATION_ADMIN_NO_BYPASS, label: NO_BYPASS_LABEL }); + // The row the answer already carried is untouched. + expect(answer.body.role).toBe('admin'); + expect(answer.body.userId).toBe(member.userId); + // What the line names is what the reconciler wrote, and nothing more. + expect(grantsOf(member.userId)).toEqual([ORGANIZATION_ADMIN_NO_BYPASS]); + }); + + it('member → owner names it too (Make Owner is the same route)', async () => { + const { changeRole } = await stage(); + const answer = await changeRole('owner'); + expect(answer.status, answer.text).toBe(200); + expect(answer.body.outcome).toBe(PERMISSION_SET_GRANTED_OUTCOME); + expect(answer.body.grantedPermissionSet?.name).toBe(ORGANIZATION_ADMIN_NO_BYPASS); + }); +}); + +describe('#27862 (b) — a role change that grants nothing answers the row alone', () => { + it('admin → owner keeps the set it already held: no line', async () => { + const { member, changeRole, grantsOf } = await stage(); + expect((await changeRole('admin')).body.outcome).toBe(PERMISSION_SET_GRANTED_OUTCOME); + const answer = await changeRole('owner'); + expect(answer.status, answer.text).toBe(200); + expect(answer.body).not.toHaveProperty('outcome'); + expect(answer.body).not.toHaveProperty('grantedPermissionSet'); + expect(answer.body.role).toBe('owner'); + expect(grantsOf(member.userId)).toEqual([ORGANIZATION_ADMIN_NO_BYPASS]); + }); + + it('admin → member revokes it: no line', async () => { + const { member, changeRole, grantsOf } = await stage(); + await changeRole('admin'); + const answer = await changeRole('member'); + expect(answer.status, answer.text).toBe(200); + expect(answer.body).not.toHaveProperty('outcome'); + expect(answer.body).not.toHaveProperty('grantedPermissionSet'); + expect(grantsOf(member.userId)).toEqual([]); + }); + + it('a refusal carries no line and grants nothing', async () => { + const { member, changeRole, grantsOf } = await stage(); + const refused = await changeRole('admin', member.cookie); + expect(refused.status, refused.text).toBe(403); + expect(refused.body).not.toHaveProperty('outcome'); + expect(refused.body).not.toHaveProperty('grantedPermissionSet'); + expect(grantsOf(member.userId)).toEqual([]); + }); +}); + +describe('#27862 — the hooks fail toward silence', () => { + const deps = (over: Partial = {}): RoleChangeGrantDeps => ({ + readMemberPair: async () => ({ userId: 'u1', organizationId: 'o1' }), + readGrantedSetNames: async () => [], + readSetLabel: async () => 'Label', + ...over, + }); + const ctxFor = (returned: unknown) => ({ + path: UPDATE_MEMBER_ROLE_PATH, + body: { memberId: 'm1' }, + context: { returned } as Record, + }); + + it('a before-read that throws stashes nothing, so the answer is untouched', async () => { + const ctx = ctxFor({ id: 'm1', role: 'admin' }); + await snapshotRoleChangeGrants(ctx, deps({ readGrantedSetNames: async () => { throw new Error('down'); } })); + await stampRoleChangeGrantedSet(ctx, deps({ readGrantedSetNames: async () => [ORGANIZATION_ADMIN] })); + expect(ctx.context.returned).toEqual({ id: 'm1', role: 'admin' }); + }); + + it('an after-read that throws leaves the answer untouched', async () => { + const ctx = ctxFor({ id: 'm1', role: 'admin' }); + await snapshotRoleChangeGrants(ctx, deps()); + await stampRoleChangeGrantedSet(ctx, deps({ readGrantedSetNames: async () => { throw new Error('down'); } })); + expect(ctx.context.returned).toEqual({ id: 'm1', role: 'admin' }); + }); + + it('a set the catalog gives no label is named by its name', async () => { + const ctx = ctxFor({ id: 'm1', role: 'admin' }); + await snapshotRoleChangeGrants(ctx, deps()); + await stampRoleChangeGrantedSet(ctx, deps({ + readGrantedSetNames: async () => [ORGANIZATION_ADMIN], + readSetLabel: async () => undefined, + })); + expect((ctx.context.returned as any).grantedPermissionSet).toEqual({ name: ORGANIZATION_ADMIN, label: ORGANIZATION_ADMIN }); + }); + + it('another route is never touched', async () => { + const ctx = { ...ctxFor({ id: 'm1' }), path: '/organization/update-member' }; + await snapshotRoleChangeGrants(ctx, deps()); + await stampRoleChangeGrantedSet(ctx, deps({ readGrantedSetNames: async () => [ORGANIZATION_ADMIN] })); + expect(ctx.context.returned).toEqual({ id: 'm1' }); + }); +}); diff --git a/packages/plugins/plugin-auth/src/role-change-granted-set.ts b/packages/plugins/plugin-auth/src/role-change-granted-set.ts new file mode 100644 index 00000000000..ba71c8e10d1 --- /dev/null +++ b/packages/plugins/plugin-auth/src/role-change-granted-set.ts @@ -0,0 +1,173 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #27862 — `POST /organization/update-member-role` says which permission set + * the role change granted. + * + * ## The defect + * + * Raising a member to `admin` or `owner` grants an org-admin permission set as + * a side effect: `plugin-security`'s `sys_member` middleware reconciles the + * pair inside the same request and writes a `sys_user_permission_set` row + * naming `organization_admin` or `organization_admin_no_bypass` (by posture, + * ADR-0105 D4). The route still answered the member row alone, so the console's + * Change Role / Make Owner actions toasted "Member role updated" and the + * administrator later found a set on the user's Permission Sets tab that + * nobody visibly assigned. + * + * ## What this does + * + * A successful answer whose role change left the member holding an org-admin + * set it did not hold before the request gains two top-level members beside + * the row's own columns, the shape #27843 gave invite-member: + * + * - `outcome: 'permission_set_granted'`; + * - `grantedPermissionSet: { name, label }` — the set's catalog name and its + * catalog `label` (the name when the definition carries none). + * + * The console reads it through `ActionSchema.outcomeMessages`, so the copy can + * name `${result.grantedPermissionSet.label}`. A role change that grants + * nothing (a demotion, admin → owner, member → external) answers exactly the + * row, as before. + * + * ## How "granted" is read + * + * Measured, not inferred from the role: the global `before` hook reads the + * target pair's org-admin grants (by the two set names) and stashes them on the + * request's endpoint context; the `after` hook reads them again and names a set + * present after and absent before. ⛔ It decides nothing about what is granted + * and writes nothing: the reconciler in `plugin-security` stays the one writer. + * + * Every read fails toward silence. A read that throws, a member that cannot be + * found, or a stash that did not survive to the `after` hook leaves the answer + * exactly as the vendor wrote it — a missing line, never a wrong one. + */ + +import { securityCatalogReaderOf } from '@objectstack/core'; +import { GRANT_SET_NAME_FIELD, grantSetNameOf } from './grant-set-name.js'; +import { ORGANIZATION_ADMIN_SET_NAMES } from './organization-delete.js'; + +/** better-auth's role-change route (Change Role and Make Owner both call it). */ +export const UPDATE_MEMBER_ROLE_PATH = '/organization/update-member-role'; + +/** The `outcome` an answer carries when its role change granted a set. */ +export const PERMISSION_SET_GRANTED_OUTCOME = 'permission_set_granted'; + +/** The request-context key the `before` hook stashes the pre-change grants under. */ +const STASH_KEY = '__osRoleChangeGrantsBefore'; + +/** The org-admin set names a role change can grant. */ +export const ROLE_CHANGE_GRANTABLE_SETS: readonly string[] = ORGANIZATION_ADMIN_SET_NAMES; + +/** The reads both hooks make. Each may throw; the callers swallow it. */ +export interface RoleChangeGrantDeps { + /** The `sys_member` row's pair, or `undefined` when no row has that id. */ + readMemberPair(memberId: string): Promise<{ userId: string; organizationId: string } | undefined>; + /** The names among {@link ROLE_CHANGE_GRANTABLE_SETS} the pair holds a grant of. */ + readGrantedSetNames(userId: string, organizationId: string): Promise; + /** The set's catalog label, or `undefined` when the catalog names none. */ + readSetLabel(name: string): Promise; +} + +/** A system-context reader of the engine (`withSystemReadContext`). */ +interface SystemReader { + find(object: string, query: Record): Promise; +} + +/** + * The reads bound to an engine: `sys` is its system-context reader, `engine` + * the object the security plugin bound its catalog to. + */ +export function roleChangeGrantDepsFor(engine: object, sys: SystemReader): RoleChangeGrantDeps { + const rowsOf = async (object: string, query: Record): Promise => { + const rows = await sys.find(object, query); + return Array.isArray(rows) ? rows : []; + }; + return { + readMemberPair: async (memberId) => { + const [row] = await rowsOf('sys_member', { where: { id: memberId }, limit: 1 }); + const userId = row?.user_id; + const organizationId = row?.organization_id; + return typeof userId === 'string' && userId && typeof organizationId === 'string' && organizationId + ? { userId, organizationId } + : undefined; + }, + readGrantedSetNames: async (userId, organizationId) => { + const rows = await rowsOf('sys_user_permission_set', { + where: { + user_id: userId, + organization_id: organizationId, + [GRANT_SET_NAME_FIELD]: { $in: [...ROLE_CHANGE_GRANTABLE_SETS] }, + }, + limit: 10, + }); + return [...new Set(rows.map(grantSetNameOf).filter((n): n is string => n !== undefined))]; + }, + readSetLabel: async (name) => { + const entry = await securityCatalogReaderOf(engine)?.resolve('permission', name); + const label = entry?.definition?.label; + return typeof label === 'string' && label.trim() !== '' ? label : undefined; + }, + }; +} + +interface GrantsBefore { + userId: string; + organizationId: string; + names: string[]; +} + +async function isRefusal(returned: unknown): Promise { + try { + const { isAPIError } = await import('better-auth/api'); + return isAPIError(returned); + } catch { + return returned instanceof Error; + } +} + +/** + * `before` hook: stash the target pair's org-admin grants. A no-op on every + * other path. Never throws. + */ +export async function snapshotRoleChangeGrants(ctx: any, deps: RoleChangeGrantDeps): Promise { + try { + if (ctx?.path !== UPDATE_MEMBER_ROLE_PATH || !ctx.context) return; + const memberId = ctx?.body?.memberId; + if (typeof memberId !== 'string' || memberId === '') return; + const pair = await deps.readMemberPair(memberId); + if (!pair) return; + const names = await deps.readGrantedSetNames(pair.userId, pair.organizationId); + const stash: GrantsBefore = { ...pair, names }; + ctx.context[STASH_KEY] = stash; + } catch { + /* no stash: the answer stays exactly as the vendor writes it */ + } +} + +/** + * `after` hook: name the set the role change granted on a successful answer. + * A no-op on every other path, on a refusal, and when nothing new is held. + * Never throws. + */ +export async function stampRoleChangeGrantedSet(ctx: any, deps: RoleChangeGrantDeps): Promise { + try { + if (ctx?.path !== UPDATE_MEMBER_ROLE_PATH) return; + const before = ctx?.context?.[STASH_KEY] as GrantsBefore | undefined; + if (ctx?.context) delete ctx.context[STASH_KEY]; + if (!before) return; + const returned = ctx?.context?.returned; + if (!returned || typeof returned !== 'object' || Array.isArray(returned)) return; + if (await isRefusal(returned)) return; + const after = await deps.readGrantedSetNames(before.userId, before.organizationId); + const granted = after.find((name) => !before.names.includes(name)); + if (!granted) return; + const label = (await deps.readSetLabel(granted)) ?? granted; + Object.assign(returned as Record, { + outcome: PERMISSION_SET_GRANTED_OUTCOME, + grantedPermissionSet: { name: granted, label }, + }); + } catch { + /* leave the answer exactly as the vendor route wrote it */ + } +}