Skip to content

Commit 695cd29

Browse files
os-zhuangclaude
andauthored
fix(auth): expose roles[] on session user so admin promotion preserves business roles (#1729)
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:<name> (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 <noreply@anthropic.com>
1 parent 484bcbb commit 695cd29

2 files changed

Lines changed: 113 additions & 2 deletions

File tree

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

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@ vi.mock('better-auth/plugins/magic-link', () => ({
2525
magicLink: vi.fn((_opts?: any) => ({ id: 'magic-link' })),
2626
}));
2727

28+
vi.mock('better-auth/plugins/custom-session', () => ({
29+
customSession: vi.fn((fn: any) => ({ id: 'custom-session', _fn: fn })),
30+
}));
31+
2832
import { betterAuth } from 'better-auth';
2933

3034
describe('AuthManager', () => {
@@ -1179,4 +1183,96 @@ describe('AuthManager', () => {
11791183
expect((m as any).generateSecret()).toBe('a-strong-production-secret-value');
11801184
});
11811185
});
1186+
1187+
describe('customSession – derived role and roles array', () => {
1188+
// dataEngine stub: `adminLinks` controls whether the user resolves as a
1189+
// platform admin (a sys_user_permission_set row pointing at the
1190+
// admin_full_access permission set with no organization scope).
1191+
const makeDataEngine = (opts: { platformAdmin: boolean }) => ({
1192+
find: vi.fn(async (object: string) => {
1193+
if (object === 'sys_user_permission_set') {
1194+
return opts.platformAdmin
1195+
? [{ user_id: 'u-1', permission_set_id: 'ps-admin', organization_id: null }]
1196+
: [];
1197+
}
1198+
if (object === 'sys_permission_set') {
1199+
return [{ id: 'ps-admin', name: 'admin_full_access' }];
1200+
}
1201+
return [];
1202+
}),
1203+
findOne: vi.fn(),
1204+
});
1205+
1206+
const getSessionCallback = async (dataEngine: any) => {
1207+
let capturedConfig: any;
1208+
(betterAuth as any).mockImplementation((config: any) => {
1209+
capturedConfig = config;
1210+
return { handler: vi.fn(), api: {} };
1211+
});
1212+
1213+
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
1214+
const manager = new AuthManager({
1215+
secret: 'test-secret-at-least-32-chars-long',
1216+
baseUrl: 'http://localhost:3000',
1217+
dataEngine,
1218+
});
1219+
await manager.getAuthInstance();
1220+
warnSpy.mockRestore();
1221+
1222+
const plugin = capturedConfig.plugins.find((p: any) => p.id === 'custom-session');
1223+
expect(plugin).toBeDefined();
1224+
return plugin._fn as (input: { user: any; session: any }) => Promise<any>;
1225+
};
1226+
1227+
it('returns roles=[] for a regular user with no stored role', async () => {
1228+
const callback = await getSessionCallback(makeDataEngine({ platformAdmin: false }));
1229+
const result = await callback({
1230+
user: { id: 'u-1', email: 'a@b.com' },
1231+
session: {},
1232+
});
1233+
expect(result.user.role).toBeUndefined();
1234+
expect(result.user.roles).toEqual([]);
1235+
});
1236+
1237+
it('splits a stored role string into roles for a non-admin user', async () => {
1238+
const callback = await getSessionCallback(makeDataEngine({ platformAdmin: false }));
1239+
const result = await callback({
1240+
user: { id: 'u-1', email: 'a@b.com', role: 'manager' },
1241+
session: {},
1242+
});
1243+
// No promotion: `role` keeps its stored value.
1244+
expect(result.user.role).toBe('manager');
1245+
expect(result.user.roles).toEqual(['manager']);
1246+
});
1247+
1248+
it('keeps stored business roles in roles[] when promoting a platform admin', async () => {
1249+
const callback = await getSessionCallback(makeDataEngine({ platformAdmin: true }));
1250+
const result = await callback({
1251+
user: { id: 'u-1', email: 'a@b.com', role: 'manager' },
1252+
session: {},
1253+
});
1254+
// Promotion replaces `role` (existing semantics) but the stored
1255+
// business role survives in the array.
1256+
expect(result.user.role).toBe('admin');
1257+
expect(result.user.roles).toEqual(['manager', 'admin']);
1258+
});
1259+
1260+
it('does not duplicate admin in roles[] when the stored role already includes it', async () => {
1261+
const callback = await getSessionCallback(makeDataEngine({ platformAdmin: true }));
1262+
const result = await callback({
1263+
user: { id: 'u-1', email: 'a@b.com', role: 'admin,manager' },
1264+
session: {},
1265+
});
1266+
expect(result.user.role).toBe('admin');
1267+
expect(result.user.roles).toEqual(['admin', 'manager']);
1268+
});
1269+
1270+
it('returns the payload untouched when the user has no id', async () => {
1271+
const callback = await getSessionCallback(makeDataEngine({ platformAdmin: false }));
1272+
const user = { email: 'anon@b.com' };
1273+
const result = await callback({ user, session: {} });
1274+
expect(result.user).toBe(user);
1275+
expect(result.user.roles).toBeUndefined();
1276+
});
1277+
});
11821278
});

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

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -947,6 +947,15 @@ export class AuthManager {
947947
// field so frontend gating (e.g. AppShell's `isAdmin = user.role === 'admin'`)
948948
// works without each consumer having to re-query permission sets.
949949
//
950+
// It also returns a `roles: string[]` array: the stored `user.role`
951+
// string split on commas (the admin plugin stores multi-role users as
952+
// e.g. `"admin,manager"`), with `'admin'` appended (deduplicated) when
953+
// the user is promoted below. Consumers that match on individual role
954+
// names (e.g. the Console approvals inbox resolving `role:<name>`
955+
// approvers) must read `roles` — `user.role` is *replaced* by the
956+
// literal `'admin'` on promotion, so business roles such as `manager`
957+
// only survive in the array.
958+
//
950959
// Better-auth's `sys_user` table doesn't carry a `role` column. We derive
951960
// it from two sources:
952961
//
@@ -1013,8 +1022,14 @@ export class AuthManager {
10131022
};
10141023

10151024
const promote = (await isPlatformAdmin()) || (await isActiveOrgAdmin());
1016-
if (!promote) return { user, session };
1017-
return { user: { ...user, role: 'admin' }, session };
1025+
const storedRole = typeof (user as any).role === 'string' ? (user as any).role : '';
1026+
const roles = storedRole
1027+
.split(',')
1028+
.map((s: string) => s.trim())
1029+
.filter(Boolean);
1030+
if (promote && !roles.includes('admin')) roles.push('admin');
1031+
if (!promote) return { user: { ...user, roles }, session };
1032+
return { user: { ...user, role: 'admin', roles }, session };
10181033
}));
10191034
}
10201035

0 commit comments

Comments
 (0)