From acc6aff9ad0a565bd0f7d15c47685ab2b496591a Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 20 May 2026 20:21:30 +0000 Subject: [PATCH] connectors: enforce VIEWER role restriction on connector/tool creation Connector/tool creation and import endpoints only ran JWT auth and license checks, while update/delete paths reject VIEWER users via assertCanWrite/assertCanWriteConnector. This made the read-only role inconsistent: viewers could create connectors and MCP tools. Add an assertCanCreate helper and apply the VIEWER check to: - POST /api/connectors - POST /api/connectors/import-all - POST /api/adapters/:slug/import Add regression tests proving VIEWER gets 403 and EDITOR/ADMIN succeed. https://claude.ai/code/session_01XJiM3Vsk1D98hbYaDiRJJp --- .../src/adapters/adapters.controller.spec.ts | 45 +++++++ .../src/adapters/adapters.controller.ts | 4 + .../connectors/connectors.controller.spec.ts | 114 ++++++++++++++++++ .../src/connectors/connectors.controller.ts | 8 ++ 4 files changed, 171 insertions(+) create mode 100644 packages/backend/src/adapters/adapters.controller.spec.ts create mode 100644 packages/backend/src/connectors/connectors.controller.spec.ts diff --git a/packages/backend/src/adapters/adapters.controller.spec.ts b/packages/backend/src/adapters/adapters.controller.spec.ts new file mode 100644 index 0000000..22d7184 --- /dev/null +++ b/packages/backend/src/adapters/adapters.controller.spec.ts @@ -0,0 +1,45 @@ +import { ForbiddenException } from '@nestjs/common'; +import { AdaptersController } from './adapters.controller'; + +function buildController() { + const adaptersService = { + importAdapter: jest + .fn() + .mockResolvedValue({ connectorId: 'c1', toolsCreated: 3 }), + }; + const licenseGuard = { + checkCanCreateConnector: jest.fn().mockResolvedValue(undefined), + }; + const controller = new AdaptersController( + adaptersService as any, + licenseGuard as any, + ); + return { controller, adaptersService, licenseGuard }; +} + +const req = (role: string) => ({ + user: { sub: 'u1', organizationId: 'org1', role }, +}); + +describe('AdaptersController role enforcement', () => { + describe('POST /api/adapters/:slug/import (importAdapter)', () => { + it('rejects VIEWER before importing the adapter', async () => { + const { controller, adaptersService, licenseGuard } = buildController(); + + await expect( + controller.importAdapter(req('VIEWER'), 'some-slug', {}), + ).rejects.toThrow(ForbiddenException); + + expect(licenseGuard.checkCanCreateConnector).not.toHaveBeenCalled(); + expect(adaptersService.importAdapter).not.toHaveBeenCalled(); + }); + + it.each(['EDITOR', 'ADMIN'])('allows %s to import an adapter', async (role) => { + const { controller, adaptersService } = buildController(); + + await controller.importAdapter(req(role), 'some-slug', {}); + + expect(adaptersService.importAdapter).toHaveBeenCalledTimes(1); + }); + }); +}); diff --git a/packages/backend/src/adapters/adapters.controller.ts b/packages/backend/src/adapters/adapters.controller.ts index 622eace..2a6aea4 100644 --- a/packages/backend/src/adapters/adapters.controller.ts +++ b/packages/backend/src/adapters/adapters.controller.ts @@ -6,6 +6,7 @@ import { Body, Req, UseGuards, + ForbiddenException, } from '@nestjs/common'; import { ApiTags, ApiOperation, ApiBearerAuth } from '@nestjs/swagger'; import { AuthGuard } from '@nestjs/passport'; @@ -62,6 +63,9 @@ export class AdaptersController { @Param('slug') slug: string, @Body() body: { credentials?: Record }, ) { + if (req.user.role === 'VIEWER') { + throw new ForbiddenException('Viewers cannot modify connectors'); + } await this.licenseGuard.checkCanCreateConnector(req.user.sub, req.user.organizationId); const result = await this.adaptersService.importAdapter( slug, diff --git a/packages/backend/src/connectors/connectors.controller.spec.ts b/packages/backend/src/connectors/connectors.controller.spec.ts new file mode 100644 index 0000000..41786a2 --- /dev/null +++ b/packages/backend/src/connectors/connectors.controller.spec.ts @@ -0,0 +1,114 @@ +import { ForbiddenException } from '@nestjs/common'; +import { ConnectorsController } from './connectors.controller'; + +const VALID_ENCRYPTION_KEY = 'a'.repeat(48); + +function buildController(overrides: { + connectorsService?: any; + prisma?: any; + mcpServer?: any; + licenseGuard?: any; +} = {}) { + const connectorsService = overrides.connectorsService ?? { + create: jest.fn().mockResolvedValue({ id: 'c1', type: 'REST' }), + }; + const prisma = overrides.prisma ?? { + connector: { create: jest.fn().mockResolvedValue({ id: 'c1' }) }, + mcpTool: { create: jest.fn() }, + }; + const mcpServer = overrides.mcpServer ?? { + reloadConnectorTools: jest.fn().mockResolvedValue(undefined), + }; + const licenseGuard = overrides.licenseGuard ?? { + checkCanCreateConnector: jest.fn().mockResolvedValue(undefined), + }; + const configService = { get: jest.fn().mockReturnValue(VALID_ENCRYPTION_KEY) }; + + const controller = new ConnectorsController( + connectorsService as any, + {} as any, // openApiParser + {} as any, // wsdlParser + {} as any, // graphqlParser + {} as any, // postmanParser + {} as any, // curlParser + {} as any, // mcpClientEngine + {} as any, // mcpOAuthService + prisma as any, + mcpServer as any, + configService as any, + licenseGuard as any, + ); + + return { controller, connectorsService, prisma, mcpServer, licenseGuard }; +} + +const req = (role: string) => ({ + user: { sub: 'u1', organizationId: 'org1', role }, +}); + +describe('ConnectorsController role enforcement', () => { + describe('POST /api/connectors (create)', () => { + it('rejects VIEWER before creating the connector', async () => { + const { controller, connectorsService, licenseGuard } = buildController(); + + await expect( + controller.create(req('VIEWER'), { + name: 'viewer-created-test', + type: 'REST' as any, + baseUrl: 'https://example.invalid', + }), + ).rejects.toThrow(ForbiddenException); + + expect(licenseGuard.checkCanCreateConnector).not.toHaveBeenCalled(); + expect(connectorsService.create).not.toHaveBeenCalled(); + }); + + it.each(['EDITOR', 'ADMIN'])('allows %s to create a connector', async (role) => { + const { controller, connectorsService } = buildController(); + + await controller.create(req(role), { + name: 'ok', + type: 'REST' as any, + baseUrl: 'https://example.invalid', + }); + + expect(connectorsService.create).toHaveBeenCalledTimes(1); + }); + }); + + describe('POST /api/connectors/import-all (importAll)', () => { + it('rejects VIEWER before importing any connector', async () => { + const { controller, prisma } = buildController(); + + await expect( + controller.importAll(req('VIEWER'), { + connectors: [ + { + name: 'viewer-imported-test', + type: 'REST' as any, + baseUrl: 'https://example.invalid', + }, + ], + }), + ).rejects.toThrow(ForbiddenException); + + expect(prisma.connector.create).not.toHaveBeenCalled(); + }); + + it.each(['EDITOR', 'ADMIN'])('allows %s to import connectors', async (role) => { + const { controller, prisma } = buildController(); + + await controller.importAll(req(role), { + connectors: [ + { + name: 'imported', + type: 'REST' as any, + baseUrl: 'https://example.invalid', + }, + ], + }); + + expect(prisma.connector.create).toHaveBeenCalledTimes(1); + }); + }); +}); diff --git a/packages/backend/src/connectors/connectors.controller.ts b/packages/backend/src/connectors/connectors.controller.ts index 9ab2489..c869a33 100644 --- a/packages/backend/src/connectors/connectors.controller.ts +++ b/packages/backend/src/connectors/connectors.controller.ts @@ -394,6 +394,12 @@ export class ConnectorsController { } } + private assertCanCreate(req: any) { + if (req.user.role === 'VIEWER') { + throw new ForbiddenException('Viewers cannot modify connectors'); + } + } + private assertCanWrite(connector: any, req: any) { this.assertOrgMatch(connector, req); if (req.user.role === 'VIEWER') { @@ -418,6 +424,7 @@ export class ConnectorsController { @Post() @ApiOperation({ summary: 'Create a new connector' }) async create(@Req() req: any, @Body() dto: CreateConnectorDto) { + this.assertCanCreate(req); await this.licenseGuard.checkCanCreateConnector(req.user.sub, req.user.organizationId); const connector = await this.connectorsService.create(req.user.sub, req.user.organizationId, dto); @@ -759,6 +766,7 @@ export class ConnectorsController { 'with duplicate names. Does not import auth credentials.', }) async importAll(@Req() req: any, @Body() body: ImportAllDto) { + this.assertCanCreate(req); if (body.connectors.length === 0) { return { error: 'Provide a "connectors" array with at least one connector' }; }