Merge pull request #1574 from mauriceboe/fix/plugin-req-user-isadmin

fix(plugins): derive plugin req.user.isAdmin from role, not is_admin
This commit is contained in:
Konstantinos Thermos
2026-07-16 13:53:55 +02:00
committed by GitHub
2 changed files with 16 additions and 4 deletions
@@ -90,7 +90,7 @@ export class PluginsProxyController {
}
// Per-route auth: default-on; `auth:false` routes are public (OAuth cb/webhook).
let user: { id: number; username: string; is_admin?: boolean } | null = null;
let user: { id: number; username: string; role?: 'admin' | 'user' } | null = null;
if (route.auth) {
const token = extractToken(req);
const loaded = token ? verifyJwtAndLoadUser(token) : null;
@@ -121,7 +121,7 @@ export class PluginsProxyController {
// allowlisted, credential-free subset — an authenticated route never
// needs them and must not see even the safe ones.
headers: route.auth === false ? pickInboundHeaders(req.headers as Record<string, unknown>) : {},
user: user ? { id: user.id, username: user.username, isAdmin: !!user.is_admin } : null,
user: user ? { id: user.id, username: user.username, isAdmin: user.role === 'admin' } : null,
},
},
// Bind the authenticated session user as the acting user for any trip
@@ -8,7 +8,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest';
const { pluginsEnabledMock, extractTokenMock, verifyMock } = vi.hoisted(() => ({
pluginsEnabledMock: vi.fn(() => true),
extractTokenMock: vi.fn(() => 'tok'),
verifyMock: vi.fn(() => ({ id: 5, username: 'ada', is_admin: false })),
verifyMock: vi.fn(() => ({ id: 5, username: 'ada', role: 'user' })),
}));
vi.mock('../../../src/nest/plugins/kill-switch', () => ({ pluginsEnabled: pluginsEnabledMock }));
vi.mock('../../../src/middleware/auth', () => ({ extractToken: extractTokenMock, verifyJwtAndLoadUser: verifyMock }));
@@ -44,7 +44,7 @@ function makeRuntime(over: Partial<PluginRuntimeService> = {}): PluginRuntimeSer
beforeEach(() => {
pluginsEnabledMock.mockClear().mockReturnValue(true);
verifyMock.mockClear().mockReturnValue({ id: 5, username: 'ada', is_admin: false });
verifyMock.mockClear().mockReturnValue({ id: 5, username: 'ada', role: 'user' });
extractTokenMock.mockClear().mockReturnValue('tok');
});
@@ -90,6 +90,18 @@ describe('PluginsProxyController', () => {
}), 5);
});
it('maps role:admin to isAdmin:true in the forwarded user view', async () => {
// Regression: the proxy derives isAdmin from the loaded user's `role`, not a
// non-existent `is_admin` field — otherwise every user (admins included) is false.
verifyMock.mockReturnValue({ id: 7, username: 'root', role: 'admin' } as never);
const runtime = makeRuntime();
const res = fakeRes();
await new PluginsProxyController(runtime).proxy('p', fakeReq('GET', '/status'), res as never);
expect(runtime.invoke).toHaveBeenCalledWith('p', 'invoke.route', expect.objectContaining({
req: expect.objectContaining({ user: { id: 7, username: 'root', isAdmin: true } }),
}), 7);
});
it('a public (auth:false) route skips the session check', async () => {
const runtime = makeRuntime({ routesOf: vi.fn(() => [{ i: 1, method: 'POST', path: '/webhook', auth: false }]) } as never);
const res = fakeRes();