From 17fa3a33a0055fa8c2ee1a7f4e045013f7c0bea4 Mon Sep 17 00:00:00 2001 From: os-zhuang Date: Thu, 11 Jun 2026 18:36:21 +0500 Subject: [PATCH] fix(auth): expose roles[] on session user so admin promotion preserves business roles MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The customSession plugin replaces user.role with the literal 'admin' when the user is a platform admin or active-org admin, which destroyed any stored business roles (e.g. 'manager' in a comma-separated 'admin,manager' value). Consumers matching pending approvers by role: (Console approvals inbox) therefore never saw an admin's role-based todos. The session payload now always carries user.roles: string[] — the stored role string split on commas, with 'admin' appended (deduplicated) on promotion. user.role semantics are unchanged: existing strict user.role === 'admin' checks keep working. Co-Authored-By: Claude Fable 5 --- .../plugin-auth/src/auth-manager.test.ts | 96 +++++++++++++++++++ .../plugins/plugin-auth/src/auth-manager.ts | 19 +++- 2 files changed, 113 insertions(+), 2 deletions(-) diff --git a/packages/plugins/plugin-auth/src/auth-manager.test.ts b/packages/plugins/plugin-auth/src/auth-manager.test.ts index 2504ca900b..70906a3852 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.test.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.test.ts @@ -25,6 +25,10 @@ vi.mock('better-auth/plugins/magic-link', () => ({ magicLink: vi.fn((_opts?: any) => ({ id: 'magic-link' })), })); +vi.mock('better-auth/plugins/custom-session', () => ({ + customSession: vi.fn((fn: any) => ({ id: 'custom-session', _fn: fn })), +})); + import { betterAuth } from 'better-auth'; describe('AuthManager', () => { @@ -1179,4 +1183,96 @@ describe('AuthManager', () => { expect((m as any).generateSecret()).toBe('a-strong-production-secret-value'); }); }); + + describe('customSession – derived role and roles array', () => { + // dataEngine stub: `adminLinks` controls whether the user resolves as a + // platform admin (a sys_user_permission_set row pointing at the + // admin_full_access permission set with no organization scope). + const makeDataEngine = (opts: { platformAdmin: boolean }) => ({ + find: vi.fn(async (object: string) => { + 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' }]; + } + return []; + }), + findOne: vi.fn(), + }); + + const getSessionCallback = async (dataEngine: any) => { + let capturedConfig: any; + (betterAuth as any).mockImplementation((config: any) => { + capturedConfig = config; + return { handler: vi.fn(), api: {} }; + }); + + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const manager = new AuthManager({ + secret: 'test-secret-at-least-32-chars-long', + baseUrl: 'http://localhost:3000', + dataEngine, + }); + await manager.getAuthInstance(); + warnSpy.mockRestore(); + + const plugin = capturedConfig.plugins.find((p: any) => p.id === 'custom-session'); + expect(plugin).toBeDefined(); + return plugin._fn as (input: { user: any; session: any }) => Promise; + }; + + it('returns roles=[] for a regular user with no stored role', async () => { + const callback = await getSessionCallback(makeDataEngine({ platformAdmin: false })); + const result = await callback({ + user: { id: 'u-1', email: 'a@b.com' }, + session: {}, + }); + expect(result.user.role).toBeUndefined(); + expect(result.user.roles).toEqual([]); + }); + + it('splits a stored role string into roles for a non-admin user', async () => { + const callback = await getSessionCallback(makeDataEngine({ platformAdmin: false })); + const result = await callback({ + user: { id: 'u-1', email: 'a@b.com', role: 'manager' }, + session: {}, + }); + // No promotion: `role` keeps its stored value. + expect(result.user.role).toBe('manager'); + expect(result.user.roles).toEqual(['manager']); + }); + + it('keeps stored business roles in roles[] when promoting a platform admin', async () => { + const callback = await getSessionCallback(makeDataEngine({ platformAdmin: true })); + const result = await callback({ + user: { id: 'u-1', email: 'a@b.com', role: 'manager' }, + session: {}, + }); + // Promotion replaces `role` (existing semantics) but the stored + // business role survives in the array. + expect(result.user.role).toBe('admin'); + expect(result.user.roles).toEqual(['manager', 'admin']); + }); + + it('does not duplicate admin in roles[] when the stored role already includes it', async () => { + const callback = await getSessionCallback(makeDataEngine({ platformAdmin: true })); + const result = await callback({ + user: { id: 'u-1', email: 'a@b.com', role: 'admin,manager' }, + session: {}, + }); + expect(result.user.role).toBe('admin'); + expect(result.user.roles).toEqual(['admin', 'manager']); + }); + + it('returns the payload untouched when the user has no id', async () => { + const callback = await getSessionCallback(makeDataEngine({ platformAdmin: false })); + const user = { email: 'anon@b.com' }; + const result = await callback({ user, session: {} }); + expect(result.user).toBe(user); + expect(result.user.roles).toBeUndefined(); + }); + }); }); diff --git a/packages/plugins/plugin-auth/src/auth-manager.ts b/packages/plugins/plugin-auth/src/auth-manager.ts index 736ea105ce..6f58fecc3c 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.ts @@ -947,6 +947,15 @@ export class AuthManager { // field so frontend gating (e.g. AppShell's `isAdmin = user.role === 'admin'`) // works without each consumer having to re-query permission sets. // + // It also returns a `roles: string[]` array: the stored `user.role` + // string split on commas (the admin plugin stores multi-role users as + // e.g. `"admin,manager"`), with `'admin'` appended (deduplicated) when + // the user is promoted below. Consumers that match on individual role + // names (e.g. the Console approvals inbox resolving `role:` + // approvers) must read `roles` — `user.role` is *replaced* by the + // literal `'admin'` on promotion, so business roles such as `manager` + // only survive in the array. + // // Better-auth's `sys_user` table doesn't carry a `role` column. We derive // it from two sources: // @@ -1013,8 +1022,14 @@ export class AuthManager { }; const promote = (await isPlatformAdmin()) || (await isActiveOrgAdmin()); - if (!promote) return { user, session }; - return { user: { ...user, role: 'admin' }, session }; + const storedRole = typeof (user as any).role === 'string' ? (user as any).role : ''; + const roles = storedRole + .split(',') + .map((s: string) => s.trim()) + .filter(Boolean); + if (promote && !roles.includes('admin')) roles.push('admin'); + if (!promote) return { user: { ...user, roles }, session }; + return { user: { ...user, role: 'admin', roles }, session }; })); }