From 9895c7112a42ea28f5488e3d4afe1b651ed4c7aa Mon Sep 17 00:00:00 2001 From: Maurice Date: Sun, 9 Aug 2026 21:57:08 +0200 Subject: [PATCH] Give the admin OAuth-session panel back to the oauth domain The last of AdminService's borrowed SQL. oauth_tokens belongs to oauth/, which already had a user-facing view of the same table; AdminService carried a second one only because the panel route is under /api/admin. Both survive, under names that say which is which. They are not duplicates: the user-facing listOAuthSessions is scoped to one user, the admin one spans all of them and joins the owner username, and the admin revoke kills the client session for whoever owned the token. OauthModule gains its first export. Its header said nothing inside the container injects the domain, which stops being true here. --- server/src/nest/admin/admin.controller.ts | 6 ++- server/src/nest/admin/admin.module.ts | 3 +- server/src/nest/admin/admin.service.ts | 35 ----------------- server/src/nest/oauth/oauth.module.ts | 5 ++- server/src/nest/oauth/oauth.service.ts | 39 +++++++++++++++++++ .../tests/unit/nest/admin.controller.test.ts | 11 +++--- server/tests/unit/nest/admin.service.test.ts | 13 ------- server/tests/unit/nest/oauth.service.test.ts | 22 +++++++++++ 8 files changed, 77 insertions(+), 57 deletions(-) diff --git a/server/src/nest/admin/admin.controller.ts b/server/src/nest/admin/admin.controller.ts index 9dcc70695..3dded6b4f 100644 --- a/server/src/nest/admin/admin.controller.ts +++ b/server/src/nest/admin/admin.controller.ts @@ -4,6 +4,7 @@ import { readEnv } from '../../app-config'; import { AdminService } from './admin.service'; import { TokenService } from '../tokens/token.service'; import { RegistrationInvitesService } from '../auth/registration-invites.service'; +import { OauthService } from '../oauth/oauth.service'; import { AdminUserCreateDto, AdminUserUpdateDto, @@ -65,6 +66,7 @@ export class AdminController { private readonly notifications: NotificationsService, private readonly tokens: TokenService, private readonly invites: RegistrationInvitesService, + private readonly oauth: OauthService, ) {} // ── Users ── @@ -257,11 +259,11 @@ export class AdminController { } @Get('oauth-sessions') - listOAuthSessions() { return { sessions: this.admin.listOAuthSessions() }; } + listOAuthSessions() { return { sessions: this.oauth.listAllOAuthSessions() }; } @Delete('oauth-sessions/:id') revokeOAuthSession(@CurrentUser() user: User, @Param('id') id: string, @Req() req: Request) { - ok(this.admin.revokeOAuthSession(id)); + ok(this.oauth.adminRevokeOAuthSession(id)); this.audit.writeAudit({ userId: user.id, action: 'admin.oauth_session_revoke', resource: String(id), ip: getClientIp(req) }); return { success: true }; } diff --git a/server/src/nest/admin/admin.module.ts b/server/src/nest/admin/admin.module.ts index 42cae6bb8..adba9dc6d 100644 --- a/server/src/nest/admin/admin.module.ts +++ b/server/src/nest/admin/admin.module.ts @@ -1,5 +1,6 @@ import { Module } from '@nestjs/common'; import { TokensModule } from '../tokens/tokens.module'; +import { OauthModule } from '../oauth/oauth.module'; import { AdminController } from './admin.controller'; import { AdminService } from './admin.service'; import { PluginsRuntimeModule } from '../plugins/plugins-runtime.module'; @@ -19,7 +20,7 @@ import { PackingModule } from '../packing/packing.module'; import { PermissionsModule } from '../permissions/permissions.module'; @Module({ - imports: [PluginsRuntimeModule, SettingsModule, AuditModule, AddonsModule, AuthModule, NotificationsModule, PackingModule, PermissionsModule, TokensModule], + imports: [PluginsRuntimeModule, SettingsModule, AuditModule, AddonsModule, AuthModule, NotificationsModule, PackingModule, PermissionsModule, TokensModule, OauthModule], controllers: [AdminController], providers: [AdminService], }) diff --git a/server/src/nest/admin/admin.service.ts b/server/src/nest/admin/admin.service.ts index a83e91765..fd10075f2 100644 --- a/server/src/nest/admin/admin.service.ts +++ b/server/src/nest/admin/admin.service.ts @@ -602,41 +602,6 @@ export class AdminService { }; } - // ── OAuth Sessions ───────────────────────────────────────────────────────── - - listOAuthSessions() { - const rows = this.db.all & { scopes: string }>(` - SELECT ot.id, ot.client_id, oc.name AS client_name, ot.user_id, u.username, - ot.scopes, ot.access_token_expires_at, ot.refresh_token_expires_at, ot.created_at - FROM oauth_tokens ot - JOIN oauth_clients oc ON ot.client_id = oc.client_id - JOIN users u ON u.id = ot.user_id - WHERE ot.revoked_at IS NULL - AND ot.refresh_token_expires_at > CURRENT_TIMESTAMP - ORDER BY ot.created_at DESC - `); - // One malformed row must not 500 the whole admin OAuth-sessions panel. - return rows.map((r) => { - let scopes: unknown; - try { - scopes = JSON.parse(r.scopes); - } catch { - scopes = null; - } - return { ...r, scopes }; - }); - } - - revokeOAuthSession(id: string) { - const row = this.db.get<{ id: number; user_id: number; client_id: string }>( - 'SELECT id, user_id, client_id FROM oauth_tokens WHERE id = ?', id, - ); - if (!row) return { error: 'Session not found', status: 404 }; - this.db.run('UPDATE oauth_tokens SET revoked_at = CURRENT_TIMESTAMP WHERE id = ?', id); - revokeUserSessionsForClient(row.user_id, row.client_id); - return {}; - } - // ── JWT Rotation ─────────────────────────────────────────────────────────── rotateJwtSecret(): { error?: string; status?: number } { diff --git a/server/src/nest/oauth/oauth.module.ts b/server/src/nest/oauth/oauth.module.ts index 60e404913..ddb1f1020 100644 --- a/server/src/nest/oauth/oauth.module.ts +++ b/server/src/nest/oauth/oauth.module.ts @@ -14,11 +14,14 @@ import { AddonsModule } from '../addons/addons.module'; * reaches the domain through oauth.bridge.ts, which is why the pending * authorization codes live in a module, not on the provider. * - * No `exports`: nothing inside the container injects this domain. + * Exports OauthService for AdminController, which serves the admin OAuth-session + * panel under /api/admin. That is the one in-container consumer; everything + * outside the container still goes through oauth.bridge.ts. */ @Module({ imports: [RateLimitModule, AuditModule, AddonsModule], controllers: [OauthPublicController, OauthApiController], providers: [OauthService], + exports: [OauthService], }) export class OauthModule {} diff --git a/server/src/nest/oauth/oauth.service.ts b/server/src/nest/oauth/oauth.service.ts index b28577d1b..064bdda8d 100644 --- a/server/src/nest/oauth/oauth.service.ts +++ b/server/src/nest/oauth/oauth.service.ts @@ -641,4 +641,43 @@ export class OauthService { if (!timingSafeEqualHex(hashToken(clientSecret), client.client_secret_hash)) return null; return client; } + // --------------------------------------------------------------------------- + // Admin view of live sessions + // + // Moved from AdminService, which owned the oauth_tokens SQL only because the + // panel route is under /api/admin. The route keeps its path and its guard. + // --------------------------------------------------------------------------- + + listAllOAuthSessions() { + const rows = this.db.all & { scopes: string }>(` + SELECT ot.id, ot.client_id, oc.name AS client_name, ot.user_id, u.username, + ot.scopes, ot.access_token_expires_at, ot.refresh_token_expires_at, ot.created_at + FROM oauth_tokens ot + JOIN oauth_clients oc ON ot.client_id = oc.client_id + JOIN users u ON u.id = ot.user_id + WHERE ot.revoked_at IS NULL + AND ot.refresh_token_expires_at > CURRENT_TIMESTAMP + ORDER BY ot.created_at DESC + `); + // One malformed row must not 500 the whole admin OAuth-sessions panel. + return rows.map((r) => { + let scopes: unknown; + try { + scopes = JSON.parse(r.scopes); + } catch { + scopes = null; + } + return { ...r, scopes }; + }); + } + + adminRevokeOAuthSession(id: string) { + const row = this.db.get<{ id: number; user_id: number; client_id: string }>( + 'SELECT id, user_id, client_id FROM oauth_tokens WHERE id = ?', id, + ); + if (!row) return { error: 'Session not found', status: 404 }; + this.db.run('UPDATE oauth_tokens SET revoked_at = CURRENT_TIMESTAMP WHERE id = ?', id); + revokeUserSessionsForClient(row.user_id, row.client_id); + return {}; + } } diff --git a/server/tests/unit/nest/admin.controller.test.ts b/server/tests/unit/nest/admin.controller.test.ts index 7d42c4668..e217162ce 100644 --- a/server/tests/unit/nest/admin.controller.test.ts +++ b/server/tests/unit/nest/admin.controller.test.ts @@ -9,6 +9,7 @@ import type { AddonsService } from '../../../src/nest/addons/addons.service'; import { AdminController } from '../../../src/nest/admin/admin.controller'; import type { TokenService } from '../../../src/nest/tokens/token.service'; import type { RegistrationInvitesService } from '../../../src/nest/auth/registration-invites.service'; +import type { OauthService } from '../../../src/nest/oauth/oauth.service'; import type { AdminService } from '../../../src/nest/admin/admin.service'; import type { PluginRuntimeService } from '../../../src/nest/plugins/plugin-runtime.service'; import type { AuditService } from '../../../src/nest/audit/audit.service'; @@ -46,8 +47,8 @@ const addonsStub = () => ({ // The MCP-token routes read TokenService now, not AdminService. Stubbed via a // fourth, optional argument so every existing call site stays as it was. -const adminCtl = (s: AdminService, rt?: PluginRuntimeService, addons: AddonsService = addonsStub(), tokens: Partial = {}, invites: Partial = {}) => - new AdminController(s, addons, rt as unknown as PluginRuntimeService, audit, notifications, tokens as TokenService, invites as RegistrationInvitesService); +const adminCtl = (s: AdminService, rt?: PluginRuntimeService, addons: AddonsService = addonsStub(), tokens: Partial = {}, invites: Partial = {}, oauth: Partial = {}) => + new AdminController(s, addons, rt as unknown as PluginRuntimeService, audit, notifications, tokens as TokenService, invites as RegistrationInvitesService, oauth as OauthService); function thrown(fn: () => unknown): { status: number; body: unknown } { try { fn(); } catch (err) { if (err instanceof NotFoundException) return { status: 404, body: err.getResponse() }; @@ -133,7 +134,7 @@ describe('AdminController addons + sessions + jwt + defaults', () => { }); it('oauth-sessions revoke audits; rotate-jwt maps error', () => { - expect(adminCtl(svc({ revokeOAuthSession: vi.fn().mockReturnValue({}) } as Partial)).revokeOAuthSession(user, '3', req)).toEqual({ success: true }); + expect(adminCtl(svc(), undefined, undefined, {}, {}, { adminRevokeOAuthSession: vi.fn().mockReturnValue({}) }).revokeOAuthSession(user, '3', req)).toEqual({ success: true }); expect(thrown(() => adminCtl(svc({ rotateJwtSecret: vi.fn().mockReturnValue({ error: 'locked', status: 409 }) } as Partial)).rotateJwtSecret(user, req))).toEqual({ status: 409, body: { error: 'locked' } }); expect(adminCtl(svc({ rotateJwtSecret: vi.fn().mockReturnValue({}) } as Partial)).rotateJwtSecret(user, req)).toEqual({ success: true }); }); @@ -158,7 +159,7 @@ describe('AdminController read-only getters', () => { expect(adminCtl(svc(), undefined, undefined, {}, { listInvites: vi.fn().mockReturnValue([{ id: 1 }]) }).listInvites()).toEqual({ invites: [{ id: 1 }] }); expect(adminCtl(svc({ listAddons: vi.fn().mockReturnValue([{ id: 'mcp' }]) } as Partial)).listAddons()).toEqual({ addons: [{ id: 'mcp' }] }); expect(adminCtl(svc(), undefined, undefined, { listAllMcpTokens: vi.fn().mockReturnValue([{ id: 1 }]) }).listMcpTokens()).toEqual({ tokens: [{ id: 1 }] }); - expect(adminCtl(svc({ listOAuthSessions: vi.fn().mockReturnValue([{ id: 1 }]) } as Partial)).listOAuthSessions()).toEqual({ sessions: [{ id: 1 }] }); + expect(adminCtl(svc(), undefined, undefined, {}, {}, { listAllOAuthSessions: vi.fn().mockReturnValue([{ id: 1 }]) }).listOAuthSessions()).toEqual({ sessions: [{ id: 1 }] }); }); @@ -178,7 +179,7 @@ describe('AdminController tokens + sessions', () => { it('mcp token + oauth session deletes return success and map errors', () => { expect(adminCtl(svc(), undefined, undefined, { adminDeleteMcpToken: vi.fn().mockReturnValue({}) }).deleteMcpToken('2')).toEqual({ success: true }); expect(thrown(() => adminCtl(svc(), undefined, undefined, { adminDeleteMcpToken: vi.fn().mockReturnValue({ error: 'no token', status: 404 }) }).deleteMcpToken('9'))).toEqual({ status: 404, body: { error: 'no token' } }); - expect(thrown(() => adminCtl(svc({ revokeOAuthSession: vi.fn().mockReturnValue({ error: 'no session', status: 404 }) } as Partial)).revokeOAuthSession(user, '9', req))).toEqual({ status: 404, body: { error: 'no session' } }); + expect(thrown(() => adminCtl(svc(), undefined, undefined, {}, {}, { adminRevokeOAuthSession: vi.fn().mockReturnValue({ error: 'no session', status: 404 }) }).revokeOAuthSession(user, '9', req))).toEqual({ status: 404, body: { error: 'no session' } }); }); }); diff --git a/server/tests/unit/nest/admin.service.test.ts b/server/tests/unit/nest/admin.service.test.ts index 1b7ae8a3c..3a2edcb75 100644 --- a/server/tests/unit/nest/admin.service.test.ts +++ b/server/tests/unit/nest/admin.service.test.ts @@ -515,17 +515,4 @@ describe('admin quirk fixes (post-fold)', () => { }); - it('ADMIN-SVC-074 — listOAuthSessions survives a row with malformed scopes JSON', () => { - const { user } = createUser(testDb); - testDb.prepare("INSERT INTO oauth_clients (client_id, client_secret_hash, name) VALUES ('c1', 'hash', 'Client')").run(); - testDb.prepare(` - INSERT INTO oauth_tokens (client_id, user_id, access_token_hash, refresh_token_hash, scopes, - access_token_expires_at, refresh_token_expires_at) - VALUES ('c1', ?, 'ahash', 'rhash', 'not-json{', datetime('now', '+1 hour'), datetime('now', '+1 day')) - `).run(user.id); - - const sessions = svc.listOAuthSessions() as any[]; - expect(sessions).toHaveLength(1); - expect(sessions[0].scopes).toBeNull(); - }); }); diff --git a/server/tests/unit/nest/oauth.service.test.ts b/server/tests/unit/nest/oauth.service.test.ts index 5b101caf8..c91cbc7ce 100644 --- a/server/tests/unit/nest/oauth.service.test.ts +++ b/server/tests/unit/nest/oauth.service.test.ts @@ -1181,3 +1181,25 @@ describe('OauthModule', () => { expect(providers).toEqual([Svc]); }); }); + +// --------------------------------------------------------------------------- +// Admin view of live sessions — moved here from admin.service.test.ts with the +// method. Named apart from the user-facing listOAuthSessions because the two +// are different queries: this one spans users and carries the owner username. +// --------------------------------------------------------------------------- + +describe('admin OAuth sessions', () => { + it('ADMIN-SVC-074 — listAllOAuthSessions survives a row with malformed scopes JSON', () => { + const { user } = createUser(testDb); + testDb.prepare("INSERT INTO oauth_clients (client_id, client_secret_hash, name) VALUES ('c1', 'hash', 'Client')").run(); + testDb.prepare(` + INSERT INTO oauth_tokens (client_id, user_id, access_token_hash, refresh_token_hash, scopes, + access_token_expires_at, refresh_token_expires_at) + VALUES ('c1', ?, 'ahash', 'rhash', 'not-json{', datetime('now', '+1 hour'), datetime('now', '+1 day')) + `).run(user.id); + + const sessions = svc.listAllOAuthSessions() as any[]; + expect(sessions).toHaveLength(1); + expect(sessions[0].scopes).toBeNull(); + }); +});