diff --git a/packages/backend/src/middleware/guildAccess.ts b/packages/backend/src/middleware/guildAccess.ts index 36e49e528..caae03526 100644 --- a/packages/backend/src/middleware/guildAccess.ts +++ b/packages/backend/src/middleware/guildAccess.ts @@ -39,11 +39,12 @@ export function requireGuildModuleAccess( next: NextFunction, ): Promise => { try { - if (!req.sessionId) { + const sessionId = req.sessionId ?? req.sessionID + if (!sessionId) { throw AppError.unauthorized() } - const sessionData = await sessionService.getSession(req.sessionId) + const sessionData = await sessionService.getSession(sessionId) if (!sessionData) { throw AppError.unauthorized('Session expired') } @@ -69,6 +70,8 @@ export function requireGuildModuleAccess( } req.guildContext = context + req.sessionId = req.sessionId ?? req.sessionID + req.userId = sessionData.userId next() } catch (error) { next(error) diff --git a/packages/backend/src/routes/guildAutomation.ts b/packages/backend/src/routes/guildAutomation.ts index 1e0d153d4..bd2503518 100644 --- a/packages/backend/src/routes/guildAutomation.ts +++ b/packages/backend/src/routes/guildAutomation.ts @@ -1,5 +1,6 @@ import type { Express, Response } from 'express' -import { requireAuth, type AuthenticatedRequest } from '../middleware/auth' +import type { AuthenticatedRequest } from '../middleware/auth' +import { requireGuildModuleAccess } from '../middleware/guildAccess' import { validateBody, validateParams } from '../middleware/validate' import { writeLimiter } from '../middleware/rateLimit' import { asyncHandler } from '../middleware/asyncHandler' @@ -26,7 +27,7 @@ function requireUserId(req: AuthenticatedRequest): string { export function setupGuildAutomationRoutes(app: Express): void { app.get( '/api/guilds/:guildId/automation/manifest', - requireAuth, + requireGuildModuleAccess('settings', 'view'), validateParams(s.guildIdParam), asyncHandler(async (req: AuthenticatedRequest, res: Response) => { const guildId = p(req.params.guildId) @@ -43,7 +44,7 @@ export function setupGuildAutomationRoutes(app: Express): void { app.put( '/api/guilds/:guildId/automation/manifest', - requireAuth, + requireGuildModuleAccess('settings', 'manage'), writeLimiter, validateParams(s.guildIdParam), validateBody(s.guildAutomationManifestBody), @@ -67,7 +68,7 @@ export function setupGuildAutomationRoutes(app: Express): void { app.post( '/api/guilds/:guildId/automation/capture', - requireAuth, + requireGuildModuleAccess('settings', 'manage'), writeLimiter, validateParams(s.guildIdParam), validateBody(s.guildAutomationManifestBody), @@ -87,7 +88,7 @@ export function setupGuildAutomationRoutes(app: Express): void { app.post( '/api/guilds/:guildId/automation/plan', - requireAuth, + requireGuildModuleAccess('settings', 'manage'), writeLimiter, validateParams(s.guildIdParam), validateBody(s.guildAutomationRunBody), @@ -110,7 +111,7 @@ export function setupGuildAutomationRoutes(app: Express): void { app.post( '/api/guilds/:guildId/automation/apply', - requireAuth, + requireGuildModuleAccess('settings', 'manage'), writeLimiter, validateParams(s.guildIdParam), validateBody(s.guildAutomationRunBody), @@ -121,12 +122,15 @@ export function setupGuildAutomationRoutes(app: Express): void { const actualState = body.actualState ? validateGuildAutomationManifest(body.actualState) : undefined - const result = await guildAutomationService.createApplyRun(guildId, { - actualState, - initiatedBy: userId, - allowProtected: body.allowProtected === true, - runType: 'apply', - }) + const result = await guildAutomationService.createApplyRun( + guildId, + { + actualState, + initiatedBy: userId, + allowProtected: body.allowProtected === true, + runType: 'apply', + }, + ) res.json(result) }), @@ -134,7 +138,7 @@ export function setupGuildAutomationRoutes(app: Express): void { app.post( '/api/guilds/:guildId/automation/reconcile', - requireAuth, + requireGuildModuleAccess('settings', 'manage'), writeLimiter, validateParams(s.guildIdParam), validateBody(s.guildAutomationRunBody), @@ -145,12 +149,15 @@ export function setupGuildAutomationRoutes(app: Express): void { const actualState = body.actualState ? validateGuildAutomationManifest(body.actualState) : undefined - const result = await guildAutomationService.createApplyRun(guildId, { - actualState, - initiatedBy: userId, - allowProtected: body.allowProtected === true, - runType: 'reconcile', - }) + const result = await guildAutomationService.createApplyRun( + guildId, + { + actualState, + initiatedBy: userId, + allowProtected: body.allowProtected === true, + runType: 'reconcile', + }, + ) res.json(result) }), @@ -158,7 +165,7 @@ export function setupGuildAutomationRoutes(app: Express): void { app.get( '/api/guilds/:guildId/automation/status', - requireAuth, + requireGuildModuleAccess('settings', 'view'), validateParams(s.guildIdParam), asyncHandler(async (req: AuthenticatedRequest, res: Response) => { const guildId = p(req.params.guildId) @@ -173,7 +180,7 @@ export function setupGuildAutomationRoutes(app: Express): void { app.post( '/api/guilds/:guildId/automation/cutover', - requireAuth, + requireGuildModuleAccess('settings', 'manage'), writeLimiter, validateParams(s.guildIdParam), validateBody(s.guildAutomationRunBody), @@ -192,7 +199,7 @@ export function setupGuildAutomationRoutes(app: Express): void { app.post( '/api/guilds/:guildId/automation/presets/criativaria/apply', - requireAuth, + requireGuildModuleAccess('settings', 'manage'), writeLimiter, validateParams(s.guildIdParam), asyncHandler(async (req: AuthenticatedRequest, res: Response) => { diff --git a/packages/backend/tests/integration/routes/guildAutomation.test.ts b/packages/backend/tests/integration/routes/guildAutomation.test.ts index 9de17a6fc..934b2eb92 100644 --- a/packages/backend/tests/integration/routes/guildAutomation.test.ts +++ b/packages/backend/tests/integration/routes/guildAutomation.test.ts @@ -4,12 +4,19 @@ import express from 'express' import { setupSessionMiddleware } from '../../../src/middleware/session' import { setupGuildAutomationRoutes } from '../../../src/routes/guildAutomation' import { errorHandler } from '../../../src/middleware/errorHandler' +import { requireGuildModuleAccess } from '../../../src/middleware/guildAccess' import { sessionService } from '../../../src/services/SessionService' +import { guildAccessService } from '../../../src/services/GuildAccessService' import { MOCK_SESSION_DATA } from '../../fixtures/mock-data' jest.mock('../../../src/services/SessionService', () => ({ - sessionService: { - getSession: jest.fn(), + sessionService: { getSession: jest.fn() }, +})) + +jest.mock('../../../src/services/GuildAccessService', () => ({ + guildAccessService: { + resolveGuildContext: jest.fn(), + hasAccess: jest.fn(), }, })) @@ -32,54 +39,97 @@ import { validateGuildAutomationManifest, } from '@lucky/shared/services' +const GUILD_ID = '111111111111111111' +const SESSION_COOKIE = ['sessionId=valid_session_id'] +const AUTOMATION_BASE = `/api/guilds/${GUILD_ID}/automation` + +const MOCK_GUILD_CONTEXT = { + guildId: GUILD_ID, + owner: true, + isAdmin: true, + effectiveAccess: { settings: 'manage' as const }, + roleIds: [], + nickname: null, + canManageRbac: true, +} + +const MOCK_PLAN = { + operations: [], + protectedOperations: [], + summary: { total: 0, safe: 0, protected: 0 }, +} + +const MOCK_MANIFEST = { + guildId: GUILD_ID, + version: 1, + guild: { id: GUILD_ID }, + source: 'manual' as const, +} + describe('Guild Automation Routes', () => { let app: express.Express + let mockedService: jest.Mocked + let mockedGuildAccessService: jest.Mocked + let mockedSessionService: jest.Mocked + + const authed = (app: express.Express) => ({ + get: (path: string) => + request(app).get(path).set('Cookie', SESSION_COOKIE), + put: (path: string) => + request(app).put(path).set('Cookie', SESSION_COOKIE), + post: (path: string) => + request(app).post(path).set('Cookie', SESSION_COOKIE), + head: (path: string) => + request(app).head(path).set('Cookie', SESSION_COOKIE), + }) beforeEach(() => { app = express() app.use(express.json()) setupSessionMiddleware(app) setupGuildAutomationRoutes(app) + + app.get( + '/api/test-route-without-guild-param', + requireGuildModuleAccess('settings', 'view'), + (_req, res) => res.json({ ok: true }), + ) + app.use(errorHandler) jest.clearAllMocks() - const mockedSessionService = sessionService as jest.Mocked< + mockedService = guildAutomationService as jest.Mocked< + typeof guildAutomationService + > + mockedGuildAccessService = guildAccessService as jest.Mocked< + typeof guildAccessService + > + mockedSessionService = sessionService as jest.Mocked< typeof sessionService > + mockedSessionService.getSession.mockResolvedValue(MOCK_SESSION_DATA) + mockedGuildAccessService.resolveGuildContext.mockResolvedValue( + MOCK_GUILD_CONTEXT as any, + ) + mockedGuildAccessService.hasAccess.mockReturnValue(true) }) test('GET manifest returns 404 when not found', async () => { - const mockedService = guildAutomationService as jest.Mocked< - typeof guildAutomationService - > mockedService.getManifest.mockResolvedValue(null) - - await request(app) - .get('/api/guilds/111111111111111111/automation/manifest') - .set('Cookie', ['sessionId=valid_session_id']) - .expect(404) + await authed(app).get(`${AUTOMATION_BASE}/manifest`).expect(404) }) test('PUT manifest saves manifest', async () => { - const mockedService = guildAutomationService as jest.Mocked< - typeof guildAutomationService - > - mockedService.saveManifest.mockResolvedValue({ - guildId: '111111111111111111', + guildId: GUILD_ID, version: 1, updatedAt: new Date(), } as any) - await request(app) - .put('/api/guilds/111111111111111111/automation/manifest') - .set('Cookie', ['sessionId=valid_session_id']) - .send({ - version: 1, - guild: { id: '111111111111111111' }, - source: 'manual', - }) + await authed(app) + .put(`${AUTOMATION_BASE}/manifest`) + .send({ version: 1, guild: { id: GUILD_ID }, source: 'manual' }) .expect(200) expect(validateGuildAutomationManifest).toHaveBeenCalled() @@ -87,75 +137,44 @@ describe('Guild Automation Routes', () => { }) test('POST plan delegates to shared automation service', async () => { - const mockedService = guildAutomationService as jest.Mocked< - typeof guildAutomationService - > - mockedService.createPlan.mockResolvedValue({ runId: 'run-1', - desired: { version: 1, guild: { id: '111111111111111111' } }, - actual: { version: 1, guild: { id: '111111111111111111' } }, - plan: { - operations: [], - protectedOperations: [], - summary: { total: 0, safe: 0, protected: 0 }, - }, + desired: { version: 1, guild: { id: GUILD_ID } }, + actual: { version: 1, guild: { id: GUILD_ID } }, + plan: MOCK_PLAN, } as any) - await request(app) - .post('/api/guilds/111111111111111111/automation/plan') - .set('Cookie', ['sessionId=valid_session_id']) - .send({}) - .expect(200) + await authed(app).post(`${AUTOMATION_BASE}/plan`).send({}).expect(200) - expect(mockedService.createPlan).toHaveBeenCalledWith( - '111111111111111111', - { - actualState: undefined, - initiatedBy: MOCK_SESSION_DATA.userId, - runType: 'plan', - }, - ) + expect(mockedService.createPlan).toHaveBeenCalledWith(GUILD_ID, { + actualState: undefined, + initiatedBy: MOCK_SESSION_DATA.userId, + runType: 'plan', + }) }) test('POST apply delegates with allowProtected option', async () => { - const mockedService = guildAutomationService as jest.Mocked< - typeof guildAutomationService - > - mockedService.createApplyRun.mockResolvedValue({ runId: 'run-2', status: 'completed', blockedByProtected: false, - plan: { - operations: [], - protectedOperations: [], - summary: { total: 0, safe: 0, protected: 0 }, - }, + plan: MOCK_PLAN, } as any) - await request(app) - .post('/api/guilds/111111111111111111/automation/apply') - .set('Cookie', ['sessionId=valid_session_id']) + await authed(app) + .post(`${AUTOMATION_BASE}/apply`) .send({ allowProtected: true }) .expect(200) - expect(mockedService.createApplyRun).toHaveBeenCalledWith( - '111111111111111111', - { - actualState: undefined, - initiatedBy: MOCK_SESSION_DATA.userId, - allowProtected: true, - runType: 'apply', - }, - ) + expect(mockedService.createApplyRun).toHaveBeenCalledWith(GUILD_ID, { + actualState: undefined, + initiatedBy: MOCK_SESSION_DATA.userId, + allowProtected: true, + runType: 'apply', + }) }) test('GET status returns status and recent runs', async () => { - const mockedService = guildAutomationService as jest.Mocked< - typeof guildAutomationService - > - mockedService.getStatus.mockResolvedValue({ manifest: null, latestRun: null, @@ -163,28 +182,19 @@ describe('Guild Automation Routes', () => { }) mockedService.listRuns.mockResolvedValue([] as any) - const response = await request(app) - .get('/api/guilds/111111111111111111/automation/status') - .set('Cookie', ['sessionId=valid_session_id']) + const response = await authed(app) + .get(`${AUTOMATION_BASE}/status`) .expect(200) expect(response.body).toEqual({ - status: { - manifest: null, - latestRun: null, - drifts: [], - }, + status: { manifest: null, latestRun: null, drifts: [] }, runs: [], }) }) test('POST criativaria preset applies manifest and reconcile run', async () => { - const mockedService = guildAutomationService as jest.Mocked< - typeof guildAutomationService - > - mockedService.saveManifest.mockResolvedValue({ - guildId: '111111111111111111', + guildId: GUILD_ID, version: 1, updatedAt: new Date(), } as any) @@ -192,16 +202,11 @@ describe('Guild Automation Routes', () => { runId: 'run-3', status: 'completed', blockedByProtected: false, - plan: { - operations: [], - protectedOperations: [], - summary: { total: 0, safe: 0, protected: 0 }, - }, + plan: MOCK_PLAN, } as any) - const response = await request(app) - .post('/api/guilds/111111111111111111/automation/presets/criativaria/apply') - .set('Cookie', ['sessionId=valid_session_id']) + const response = await authed(app) + .post(`${AUTOMATION_BASE}/presets/criativaria/apply`) .expect(200) expect(response.body).toEqual({ @@ -212,16 +217,12 @@ describe('Guild Automation Routes', () => { runId: 'run-3', status: 'completed', blockedByProtected: false, - plan: { - operations: [], - protectedOperations: [], - summary: { total: 0, safe: 0, protected: 0 }, - }, + plan: MOCK_PLAN, }, }) expect(mockedService.saveManifest).toHaveBeenCalled() expect(mockedService.createApplyRun).toHaveBeenCalledWith( - '111111111111111111', + GUILD_ID, expect.objectContaining({ initiatedBy: MOCK_SESSION_DATA.userId, allowProtected: false, @@ -229,4 +230,63 @@ describe('Guild Automation Routes', () => { }), ) }) + + test('returns 403 for user without settings:manage on write routes', async () => { + mockedGuildAccessService.hasAccess.mockReturnValue(false) + await authed(app) + .put(`${AUTOMATION_BASE}/manifest`) + .send({ version: 1, guild: { id: GUILD_ID } }) + .expect(403) + }) + + test('returns 403 for user without guild access on read routes', async () => { + mockedGuildAccessService.resolveGuildContext.mockResolvedValue(null) + await authed(app).get(`${AUTOMATION_BASE}/manifest`).expect(403) + }) + + test('returns 401 for unauthenticated requests', async () => { + mockedSessionService.getSession.mockResolvedValue(null) + await request(app) + .get(`${AUTOMATION_BASE}/manifest`) + .set('Cookie', ['sessionId=invalid_session_id']) + .expect(401) + }) + + test('returns 401 when no session cookie provided', async () => { + await request(app).get(`${AUTOMATION_BASE}/manifest`).expect(401) + }) + + test('returns 400 when guildId param is missing', async () => { + await request(app).get('/api/guilds//automation/manifest').expect(404) + }) + + test('attaches userId to request from session data', async () => { + mockedService.getManifest.mockResolvedValue(MOCK_MANIFEST as any) + await authed(app).get(`${AUTOMATION_BASE}/manifest`).expect(200) + expect(mockedService.getManifest).toHaveBeenCalledWith(GUILD_ID) + }) + + test('returns 400 when route has no guildId or id parameter', async () => { + await authed(app).get('/api/test-route-without-guild-param').expect(400) + }) + + test('auto mode resolves to manage access for POST requests', async () => { + mockedGuildAccessService.hasAccess.mockReturnValue(false) + await authed(app).post(`${AUTOMATION_BASE}/plan`).send({}).expect(403) + expect(mockedGuildAccessService.hasAccess).toHaveBeenCalledWith( + expect.any(Object), + 'settings', + 'manage', + ) + }) + + test('auto mode resolves to view access for HEAD requests', async () => { + mockedService.getManifest.mockResolvedValue(MOCK_MANIFEST as any) + await authed(app).head(`${AUTOMATION_BASE}/manifest`).expect(200) + expect(mockedGuildAccessService.hasAccess).toHaveBeenCalledWith( + expect.any(Object), + 'settings', + 'view', + ) + }) })