diff --git a/packages/cli/src/modules/source-control.ee/__tests__/source-control-status.service.test.ts b/packages/cli/src/modules/source-control.ee/__tests__/source-control-status.service.test.ts index b1ba29a9fd4..a616c9e01d9 100644 --- a/packages/cli/src/modules/source-control.ee/__tests__/source-control-status.service.test.ts +++ b/packages/cli/src/modules/source-control.ee/__tests__/source-control-status.service.test.ts @@ -4,6 +4,7 @@ import { GLOBAL_MEMBER_ROLE, type FolderRepository, type FolderWithWorkflowAndSubFolderCount, + type Project, type TagEntity, type TagRepository, type User, @@ -355,6 +356,41 @@ describe('getStatus', () => { ).rejects.toThrowError(ForbiddenError); }); + it('should throw `ForbiddenError` if direction is push and user has no source control push access', async () => { + // ARRANGE + const user = globalMemberUser; + + // ACT + await expect( + sourceControlStatusService.getStatus(user, { + direction: 'push', + verbose: false, + preferLocalVersion: true, + }), + ).rejects.toThrowError(ForbiddenError); + + expect(gitService.resetBranch).not.toHaveBeenCalled(); + }); + + it('should allow push status for a user with project source control push access', async () => { + // ARRANGE + const user = globalMemberUser; + sourceControlContextFactory.createContext.mockResolvedValueOnce( + new SourceControlContext(user, [mock({ id: 'project-1', type: 'team' })], []), + ); + + // ACT + const result = await sourceControlStatusService.getStatus(user, { + direction: 'push', + verbose: false, + preferLocalVersion: true, + }); + + // ASSERT + expect(result).toEqual([]); + expect(sourceControlContextFactory.createContext).toHaveBeenCalledWith(user); + }); + describe('project', () => { // Mock data for reusable test scenarios const mockProjects: Record = { @@ -626,6 +662,15 @@ describe('getStatus', () => { local: visibleProjects, hiddenLocal: hiddenProjects, }); + sourceControlContextFactory.createContext.mockResolvedValueOnce( + new SourceControlContext( + user, + visibleProjects.map((project) => + mock({ id: project.id, name: project.name, type: 'team' }), + ), + [], + ), + ); // ACT const result = await sourceControlStatusService.getStatus(user, { diff --git a/packages/cli/src/modules/source-control.ee/__tests__/source-control.controller.ee.test.ts b/packages/cli/src/modules/source-control.ee/__tests__/source-control.controller.ee.test.ts index e5820cbe1f0..e9c9ec88ea5 100644 --- a/packages/cli/src/modules/source-control.ee/__tests__/source-control.controller.ee.test.ts +++ b/packages/cli/src/modules/source-control.ee/__tests__/source-control.controller.ee.test.ts @@ -1,22 +1,34 @@ import type { PullWorkFolderRequestDto, PushWorkFolderRequestDto } from '@n8n/api-types'; -import type { AuthenticatedRequest } from '@n8n/db'; +import type { AuthenticatedRequest, Project, User } from '@n8n/db'; import { ControllerRegistryMetadata, type Controller } from '@n8n/decorators'; import { Container } from '@n8n/di'; +import * as permissions from '@n8n/permissions'; import type { Response } from 'express'; import { mock } from 'jest-mock-extended'; import type { EventService } from '@/events/event.service'; +import type { SourceControlContextFactory } from '../source-control-context.factory'; import type { SourceControlPreferencesService } from '../source-control-preferences.service.ee'; import { SourceControlController } from '../source-control.controller.ee'; import type { SourceControlService } from '../source-control.service.ee'; import type { SourceControlRequest } from '../types/requests'; +import { SourceControlContext } from '../types/source-control-context'; import type { SourceControlGetStatus } from '../types/source-control-get-status'; +jest.mock('@n8n/permissions', () => { + const actual = jest.requireActual('@n8n/permissions'); + return { + ...actual, + hasGlobalScope: jest.fn(actual.hasGlobalScope), + }; +}); + describe('SourceControlController', () => { let controller: SourceControlController; let sourceControlService: SourceControlService; let sourceControlPreferencesService: SourceControlPreferencesService; + let sourceControlContextFactory: SourceControlContextFactory; let eventService: EventService; beforeEach(() => { @@ -28,16 +40,22 @@ describe('SourceControlController', () => { } as unknown as SourceControlService; sourceControlPreferencesService = mock(); + sourceControlContextFactory = mock(); eventService = mock(); controller = new SourceControlController( sourceControlService, sourceControlPreferencesService, mock(), + sourceControlContextFactory, eventService, ); }); + afterEach(() => { + jest.clearAllMocks(); + }); + describe('pushWorkfolder', () => { it('should push workfolder with expected parameters', async () => { const req = mock({ @@ -170,28 +188,66 @@ describe('SourceControlController', () => { }); describe('getPreferences', () => { - it('should return preferences with public key', async () => { - const mockPreferences = { - branchName: 'main', - repositoryUrl: 'git@github.com:example/repo.git', - connected: true, - publicKey: '', - }; - const mockPublicKey = 'ssh-rsa AAAAB3NzaC1yc2E...'; + const fullPreferences = { + branchName: 'main', + branchColor: '#ff0000', + branchReadOnly: false, + connected: true, + repositoryUrl: 'git@github.com:example/repo.git', + connectionType: 'ssh' as const, + }; + const buildReq = (user: Partial = {}) => + mock({ user: mock({ id: 'user-1', ...user }) }); + + beforeEach(() => { (sourceControlPreferencesService.getPreferences as jest.Mock).mockReturnValue( - mockPreferences, + fullPreferences, ); + }); + + it('should return full preferences (including public key) for users with sourceControl:manage', async () => { + const mockPublicKey = 'ssh-rsa AAAAB3NzaC1yc2E...'; (sourceControlPreferencesService.getPublicKey as jest.Mock).mockResolvedValue(mockPublicKey); + (permissions.hasGlobalScope as jest.Mock).mockReturnValue(true); - const result = await controller.getPreferences(); + const result = await controller.getPreferences(buildReq()); + + expect(result).toEqual({ ...fullPreferences, publicKey: mockPublicKey }); + expect(sourceControlContextFactory.createContext).not.toHaveBeenCalled(); + }); + + it('should return branch name and color for project admins (has authorized projects)', async () => { + (permissions.hasGlobalScope as jest.Mock).mockReturnValue(false); + const user = mock({ id: 'user-1' }); + (sourceControlContextFactory.createContext as jest.Mock).mockResolvedValue( + new SourceControlContext(user, [mock({ id: 'p1', type: 'team' })], []), + ); + + const result = await controller.getPreferences(mock({ user })); - expect(sourceControlPreferencesService.getPublicKey).toHaveBeenCalled(); - expect(sourceControlPreferencesService.getPreferences).toHaveBeenCalled(); expect(result).toEqual({ - ...mockPreferences, - publicKey: mockPublicKey, + connected: fullPreferences.connected, + branchReadOnly: fullPreferences.branchReadOnly, + branchName: fullPreferences.branchName, + branchColor: fullPreferences.branchColor, }); + expect(sourceControlPreferencesService.getPublicKey).not.toHaveBeenCalled(); + }); + + it('should return only branchReadOnly for users with no source-control access', async () => { + (permissions.hasGlobalScope as jest.Mock).mockReturnValue(false); + const user = mock({ id: 'user-1' }); + (sourceControlContextFactory.createContext as jest.Mock).mockResolvedValue( + new SourceControlContext(user, [], []), + ); + + const result = await controller.getPreferences(mock({ user })); + + expect(result).toEqual({ + branchReadOnly: fullPreferences.branchReadOnly, + }); + expect(sourceControlPreferencesService.getPublicKey).not.toHaveBeenCalled(); }); it('should require authentication', () => { diff --git a/packages/cli/src/modules/source-control.ee/__tests__/source-control.service.test.ts b/packages/cli/src/modules/source-control.ee/__tests__/source-control.service.test.ts index ec53368d8ae..8b28e364d98 100644 --- a/packages/cli/src/modules/source-control.ee/__tests__/source-control.service.test.ts +++ b/packages/cli/src/modules/source-control.ee/__tests__/source-control.service.test.ts @@ -15,6 +15,7 @@ import type { SourceControlGitService } from '../source-control-git.service.ee'; import type { SourceControlImportService } from '../source-control-import.service.ee'; import type { SourceControlContextFactory } from '../source-control-context.factory'; import type { SourceControlScopedService } from '../source-control-scoped.service'; +import { SOURCE_CONTROL_DEFAULT_BRANCH_COLOR } from '../constants'; import { sourceControlFoldersExistCheck } from '../source-control-helper.ee'; import type { ExportResult } from '../types/export-result'; @@ -816,6 +817,8 @@ describe('SourceControlService', () => { connected: true, branchName: 'feature-branch', repositoryUrl: 'https://github.com/test/repo.git', + branchReadOnly: true, + branchColor: '#ff0000', connectionType: 'https' as const, }; preferencesService.getPreferences = jest.fn().mockReturnValue(mockPreferences); @@ -826,6 +829,8 @@ describe('SourceControlService', () => { connected: false, branchName: '', repositoryUrl: '', + branchReadOnly: false, + branchColor: SOURCE_CONTROL_DEFAULT_BRANCH_COLOR, connectionType: 'https', }); expect(result).toEqual(preferencesService.sourceControlPreferences); diff --git a/packages/cli/src/modules/source-control.ee/constants.ts b/packages/cli/src/modules/source-control.ee/constants.ts index 6d7630884a1..cf3d237840b 100644 --- a/packages/cli/src/modules/source-control.ee/constants.ts +++ b/packages/cli/src/modules/source-control.ee/constants.ts @@ -12,6 +12,7 @@ export const SOURCE_CONTROL_OWNERS_EXPORT_FILE = 'workflow_owners.json'; export const SOURCE_CONTROL_SSH_FOLDER = 'ssh'; export const SOURCE_CONTROL_SSH_KEY_NAME = 'key'; export const SOURCE_CONTROL_DEFAULT_BRANCH = 'main'; +export const SOURCE_CONTROL_DEFAULT_BRANCH_COLOR = '#5296D6'; export const SOURCE_CONTROL_ORIGIN = 'origin'; export const SOURCE_CONTROL_README = ` # n8n Source Control diff --git a/packages/cli/src/modules/source-control.ee/source-control-status.service.ee.ts b/packages/cli/src/modules/source-control.ee/source-control-status.service.ee.ts index e01e1ec8a98..f3e5bd10c36 100644 --- a/packages/cli/src/modules/source-control.ee/source-control-status.service.ee.ts +++ b/packages/cli/src/modules/source-control.ee/source-control-status.service.ee.ts @@ -185,6 +185,14 @@ export class SourceControlStatusService { throw new ForbiddenError('You do not have permission to pull from source control'); } + if ( + options.direction === 'push' && + !hasGlobalScope(user, 'sourceControl:push') && + context.authorizedProjects.length === 0 + ) { + throw new ForbiddenError('You do not have permission to push to source control'); + } + await this.resetWorkfolder(); const remoteFolders = diff --git a/packages/cli/src/modules/source-control.ee/source-control.controller.ee.ts b/packages/cli/src/modules/source-control.ee/source-control.controller.ee.ts index 48b22d6af1b..279e145892f 100644 --- a/packages/cli/src/modules/source-control.ee/source-control.controller.ee.ts +++ b/packages/cli/src/modules/source-control.ee/source-control.controller.ee.ts @@ -7,11 +7,13 @@ import { } from '@n8n/api-types'; import { AuthenticatedRequest } from '@n8n/db'; import { Get, Post, Patch, RestController, GlobalScope, Body } from '@n8n/decorators'; +import { hasGlobalScope } from '@n8n/permissions'; import * as express from 'express'; import type { PullResult } from 'simple-git'; import { SOURCE_CONTROL_DEFAULT_BRANCH } from './constants'; import { sourceControlEnabledMiddleware } from './middleware/source-control-enabled-middleware.ee'; +import { SourceControlContextFactory } from './source-control-context.factory'; import { getRepoType } from './source-control-helper.ee'; import { SourceControlPreferencesService } from './source-control-preferences.service.ee'; import { SourceControlScopedService } from './source-control-scoped.service'; @@ -31,14 +33,34 @@ export class SourceControlController { private readonly sourceControlService: SourceControlService, private readonly sourceControlPreferencesService: SourceControlPreferencesService, private readonly sourceControlScopedService: SourceControlScopedService, + private readonly sourceControlContextFactory: SourceControlContextFactory, private readonly eventService: EventService, ) {} @Get('/preferences') - async getPreferences(): Promise { - // returns the settings with the privateKey property redacted - const publicKey = await this.sourceControlPreferencesService.getPublicKey(); - return { ...this.sourceControlPreferencesService.getPreferences(), publicKey }; + async getPreferences(req: AuthenticatedRequest): Promise> { + const preferences = this.sourceControlPreferencesService.getPreferences(); + + if (hasGlobalScope(req.user, 'sourceControl:manage')) { + const publicKey = await this.sourceControlPreferencesService.getPublicKey(); + return { ...preferences, publicKey }; + } + + const publicSubset = { + branchReadOnly: preferences.branchReadOnly, + }; + + const ctx = await this.sourceControlContextFactory.createContext(req.user); + if (ctx.authorizedProjects.length > 0) { + return { + ...publicSubset, + connected: preferences.connected, + branchName: preferences.branchName, + branchColor: preferences.branchColor, + }; + } + + return publicSubset; } @Post('/preferences') @@ -162,6 +184,7 @@ export class SourceControlController { } @Get('/get-branches') + @GlobalScope('sourceControl:manage') async getBranches() { try { return await this.sourceControlService.getBranches(); @@ -240,6 +263,9 @@ export class SourceControlController { ); return result; } catch (error) { + if (error instanceof ForbiddenError) { + throw error; + } throw new BadRequestError((error as { message: string }).message); } } @@ -252,6 +278,9 @@ export class SourceControlController { new SourceControlGetStatus(req.query), ); } catch (error) { + if (error instanceof ForbiddenError) { + throw error; + } throw new BadRequestError((error as { message: string }).message); } } diff --git a/packages/cli/src/modules/source-control.ee/source-control.service.ee.ts b/packages/cli/src/modules/source-control.ee/source-control.service.ee.ts index a43687b1f11..64e2d01cfd6 100644 --- a/packages/cli/src/modules/source-control.ee/source-control.service.ee.ts +++ b/packages/cli/src/modules/source-control.ee/source-control.service.ee.ts @@ -13,6 +13,7 @@ import * as path from 'path'; import type { PushResult } from 'simple-git'; import { + SOURCE_CONTROL_DEFAULT_BRANCH_COLOR, SOURCE_CONTROL_DEFAULT_EMAIL, SOURCE_CONTROL_DEFAULT_NAME, SOURCE_CONTROL_README, @@ -182,6 +183,8 @@ export class SourceControlService { connected: false, branchName: '', repositoryUrl: '', + branchReadOnly: false, + branchColor: SOURCE_CONTROL_DEFAULT_BRANCH_COLOR, connectionType: preferences.connectionType, }); await this.sourceControlExportService.deleteRepositoryFolder(); diff --git a/packages/cli/test/integration/environments/source-control.api.test.ts b/packages/cli/test/integration/environments/source-control.api.test.ts index 076422fe22c..6ce73567b48 100644 --- a/packages/cli/test/integration/environments/source-control.api.test.ts +++ b/packages/cli/test/integration/environments/source-control.api.test.ts @@ -1,10 +1,11 @@ import type { SourceControlledFile } from '@n8n/api-types'; -import { mockInstance } from '@n8n/backend-test-utils'; -import { GLOBAL_OWNER_ROLE, type User } from '@n8n/db'; +import { createTeamProject, mockInstance } from '@n8n/backend-test-utils'; +import { GLOBAL_ADMIN_ROLE, GLOBAL_MEMBER_ROLE, GLOBAL_OWNER_ROLE, type User } from '@n8n/db'; import { Container } from '@n8n/di'; import { SourceControlPreferencesService } from '@/modules/source-control.ee/source-control-preferences.service.ee'; import { SourceControlService } from '@/modules/source-control.ee/source-control.service.ee'; +import { SourceControlStatusService } from '@/modules/source-control.ee/source-control-status.service.ee'; import { Telemetry } from '@/telemetry'; import { createUser } from '../shared/db/users'; @@ -12,7 +13,13 @@ import type { SuperAgentTest } from '../shared/types'; import * as utils from '../shared/utils'; let authOwnerAgent: SuperAgentTest; +let authAdminAgent: SuperAgentTest; +let authMemberAgent: SuperAgentTest; +let authProjectAdminAgent: SuperAgentTest; let owner: User; +let admin: User; +let member: User; +let projectAdmin: User; mockInstance(Telemetry); @@ -23,30 +30,176 @@ const testServer = utils.setupTestServer({ let sourceControlPreferencesService: SourceControlPreferencesService; -beforeAll(async () => { - owner = await createUser({ role: GLOBAL_OWNER_ROLE }); - authOwnerAgent = testServer.authAgentFor(owner); +describe('Source Control API', () => { + beforeAll(async () => { + [owner, admin, member, projectAdmin] = await Promise.all([ + createUser({ role: GLOBAL_OWNER_ROLE }), + createUser({ role: GLOBAL_ADMIN_ROLE }), + createUser({ role: GLOBAL_MEMBER_ROLE }), + createUser({ role: GLOBAL_MEMBER_ROLE }), + ]); - sourceControlPreferencesService = Container.get(SourceControlPreferencesService); - await sourceControlPreferencesService.setPreferences({ - connected: true, - keyGeneratorType: 'rsa', + await createTeamProject('Source Control API Test Project', projectAdmin); + + authOwnerAgent = testServer.authAgentFor(owner); + authAdminAgent = testServer.authAgentFor(admin); + authMemberAgent = testServer.authAgentFor(member); + authProjectAdminAgent = testServer.authAgentFor(projectAdmin); + + sourceControlPreferencesService = Container.get(SourceControlPreferencesService); + await sourceControlPreferencesService.setPreferences({ + connected: true, + repositoryUrl: 'git@github.com:n8n-io/source-control-test.git', + branchName: 'main', + branchColor: '#ff6d5a', + branchReadOnly: false, + keyGeneratorType: 'rsa', + }); }); -}); -describe('GET /sourceControl/preferences', () => { - test('should return Source Control preferences', async () => { - await authOwnerAgent - .get('/source-control/preferences') - .expect(200) - .expect((res) => { - return 'repositoryUrl' in res.body && 'branchName' in res.body; + afterEach(() => { + jest.restoreAllMocks(); + }); + + describe('GET /source-control/preferences', () => { + test.each([ + ['owner', () => authOwnerAgent], + ['admin', () => authAdminAgent], + ])('should return full Source Control preferences to global %s', async (_role, getAgent) => { + const res = await getAgent().get('/source-control/preferences').expect(200); + const data = res.body.data ?? res.body; + + expect(data).toMatchObject({ + repositoryUrl: 'git@github.com:n8n-io/source-control-test.git', + branchName: 'main', + branchColor: '#ff6d5a', + branchReadOnly: false, + connected: true, }); + expect(data.publicKey).toEqual(expect.stringMatching(/^ssh-/)); + }); + + test('should return only branch read-only state to members without source-control access', async () => { + const res = await authMemberAgent.get('/source-control/preferences').expect(200); + const data = res.body.data ?? res.body; + + expect(data).toEqual({ branchReadOnly: false }); + }); + + test('should return safe branch metadata to project admins', async () => { + const res = await authProjectAdminAgent.get('/source-control/preferences').expect(200); + const data = res.body.data ?? res.body; + + expect(data).toEqual({ + connected: true, + branchName: 'main', + branchColor: '#ff6d5a', + branchReadOnly: false, + }); + expect(data.repositoryUrl).toBeUndefined(); + expect(data.publicKey).toBeUndefined(); + }); }); - test('should return repo sync status', async () => { - Container.get(SourceControlService).getStatus = async () => { - return [ + describe('POST /source-control/preferences', () => { + test('should reject members', async () => { + await authMemberAgent + .post('/source-control/preferences') + .send({ repositoryUrl: 'git@github.com:n8n-io/test.git' }) + .expect(403); + }); + }); + + describe('PATCH /source-control/preferences', () => { + test('should reject members', async () => { + await authMemberAgent + .patch('/source-control/preferences') + .send({ branchReadOnly: true }) + .expect(403); + }); + }); + + describe('POST /source-control/disconnect', () => { + test('should reject members', async () => { + await authMemberAgent + .post('/source-control/disconnect') + .send({ keepKeyPair: true }) + .expect(403); + }); + }); + + describe('GET /source-control/get-branches', () => { + test('should reject members', async () => { + await authMemberAgent.get('/source-control/get-branches').expect(403); + }); + }); + + describe('GET /source-control/reset-workfolder', () => { + test('should reject members', async () => { + await authMemberAgent.get('/source-control/reset-workfolder').expect(403); + }); + }); + + describe('POST /source-control/generate-key-pair', () => { + test('should return new rsa key', async () => { + const res = await authOwnerAgent.post('/source-control/generate-key-pair').send().expect(200); + + expect(res.body.data).toHaveProperty('publicKey'); + expect(res.body.data).toHaveProperty('keyGeneratorType'); + expect(res.body.data.keyGeneratorType).toBe('rsa'); + expect(res.body.data.publicKey).toContain('ssh-rsa'); + }); + + test('should reject members', async () => { + await authMemberAgent + .post('/source-control/generate-key-pair') + .send({ keyGeneratorType: 'rsa' }) + .expect(403); + }); + }); + + describe('POST /source-control/pull-workfolder', () => { + test.each([ + ['member', () => authMemberAgent], + ['project admin', () => authProjectAdminAgent], + ])('should reject %s', async (_role, getAgent) => { + await getAgent().post('/source-control/pull-workfolder').send({ force: true }).expect(403); + }); + + test.each([ + ['owner', () => authOwnerAgent], + ['admin', () => authAdminAgent], + ])('should allow global %s', async (_role, getAgent) => { + const statusResult: SourceControlledFile[] = [ + { + id: 'workflow-1', + name: 'Workflow 1', + type: 'workflow', + status: 'modified', + location: 'local', + conflict: false, + file: '/Users/michael/.n8n/git/workflows/workflow-1.json', + updatedAt: '2023-07-14T11:24:41.000Z', + }, + ]; + const pullWorkfolderSpy = jest + .spyOn(Container.get(SourceControlService), 'pullWorkfolder') + .mockResolvedValue({ statusCode: 200, statusResult }); + + const res = await getAgent() + .post('/source-control/pull-workfolder') + .send({ force: true }) + .expect(200); + const data = res.body.data ?? res.body; + + expect(data).toEqual(statusResult); + expect(pullWorkfolderSpy).toHaveBeenCalled(); + }); + }); + + describe('GET /source-control/get-status', () => { + test('should return repo sync status', async () => { + jest.spyOn(Container.get(SourceControlService), 'getStatus').mockResolvedValue([ { id: 'haQetoXq9GxHSkft', name: 'My workflow 6 edit', @@ -57,25 +210,74 @@ describe('GET /sourceControl/preferences', () => { file: '/Users/michael/.n8n/git/workflows/haQetoXq9GxHSkft.json', updatedAt: '2023-07-14T11:24:41.000Z', }, - ] as SourceControlledFile[]; - }; - await authOwnerAgent - .get('/source-control/get-status') - .query({ direction: 'push', preferLocalVersion: 'true', verbose: 'false' }) - .expect(200) - .expect((res) => { - const data: SourceControlledFile[] = res.body.data; - expect(data.length).toBe(1); - expect(data[0].id).toBe('haQetoXq9GxHSkft'); + ] as SourceControlledFile[]); + + await authOwnerAgent + .get('/source-control/get-status') + .query({ direction: 'push', preferLocalVersion: 'true', verbose: 'false' }) + .expect(200) + .expect((res) => { + const data: SourceControlledFile[] = res.body.data; + expect(data.length).toBe(1); + expect(data[0].id).toBe('haQetoXq9GxHSkft'); + }); + }); + + describe('access control', () => { + beforeEach(() => { + jest.spyOn(Container.get(SourceControlService), 'sanityCheck').mockResolvedValue(); }); - }); - test('refreshing key pairsshould return new rsa key', async () => { - const res = await authOwnerAgent.post('/source-control/generate-key-pair').send().expect(200); + test.each([ + ['member', () => authMemberAgent], + ['project admin', () => authProjectAdminAgent], + ])('should reject pull status for %s', async (_role, getAgent) => { + await getAgent() + .get('/source-control/get-status') + .query({ direction: 'pull', preferLocalVersion: 'false', verbose: 'false' }) + .expect(403); + }); - expect(res.body.data).toHaveProperty('publicKey'); - expect(res.body.data).toHaveProperty('keyGeneratorType'); - expect(res.body.data.keyGeneratorType).toBe('rsa'); - expect(res.body.data.publicKey).toContain('ssh-rsa'); + test('should reject push status for members without source-control access', async () => { + await authMemberAgent + .get('/source-control/get-status') + .query({ direction: 'push', preferLocalVersion: 'true', verbose: 'false' }) + .expect(403); + }); + + test('should allow push status for project admins', async () => { + const statusResult: SourceControlledFile[] = [ + { + id: 'workflow-1', + name: 'Workflow 1', + type: 'workflow', + status: 'modified', + location: 'local', + conflict: false, + file: '/Users/michael/.n8n/git/workflows/workflow-1.json', + updatedAt: '2023-07-14T11:24:41.000Z', + }, + ]; + const getStatusSpy = jest + .spyOn(Container.get(SourceControlStatusService), 'getStatus') + .mockResolvedValue(statusResult); + + await authProjectAdminAgent + .get('/source-control/get-status') + .query({ direction: 'push', preferLocalVersion: 'true', verbose: 'false' }) + .expect(200) + .expect((res) => { + expect(res.body.data).toEqual(statusResult); + }); + expect(getStatusSpy).toHaveBeenCalledWith( + expect.objectContaining({ id: projectAdmin.id }), + expect.objectContaining({ + direction: 'push', + preferLocalVersion: true, + verbose: false, + }), + ); + }); + }); }); }); diff --git a/packages/cli/test/integration/environments/source-control.service.test.ts b/packages/cli/test/integration/environments/source-control.service.test.ts index 606abc777be..e1c9cd04f51 100644 --- a/packages/cli/test/integration/environments/source-control.service.test.ts +++ b/packages/cli/test/integration/environments/source-control.service.test.ts @@ -796,26 +796,26 @@ describe('SourceControlService', () => { }); describe('global:member user', () => { - it('should see nothing', async () => { - const result = await service.getStatus(globalMember, { - direction: 'push', - preferLocalVersion: true, - verbose: false, - }); - - expect(result).toBeEmptyArray(); + it('should not be allowed to get push status', async () => { + await expect( + service.getStatus(globalMember, { + direction: 'push', + preferLocalVersion: true, + verbose: false, + }), + ).rejects.toThrow(ForbiddenError); }); }); describe('global:chatUser user', () => { - it('should see nothing', async () => { - const result = await service.getStatus(globalChatUser, { - direction: 'push', - preferLocalVersion: true, - verbose: false, - }); - - expect(result).toBeEmptyArray(); + it('should not be allowed to get push status', async () => { + await expect( + service.getStatus(globalChatUser, { + direction: 'push', + preferLocalVersion: true, + verbose: false, + }), + ).rejects.toThrow(ForbiddenError); }); }); diff --git a/packages/frontend/editor-ui/src/features/integrations/sourceControl.ee/sourceControl.api.ts b/packages/frontend/editor-ui/src/features/integrations/sourceControl.ee/sourceControl.api.ts index cc8eadebe13..332e481a490 100644 --- a/packages/frontend/editor-ui/src/features/integrations/sourceControl.ee/sourceControl.api.ts +++ b/packages/frontend/editor-ui/src/features/integrations/sourceControl.ee/sourceControl.api.ts @@ -7,6 +7,8 @@ import type { import type { IRestApiContext } from '@n8n/rest-api-client'; import type { SourceControlPreferences, + SourceControlProjectPreferences, + SourceControlPublicPreferences, SourceControlStatus, SshKeyTypes, } from './sourceControl.types'; @@ -50,7 +52,9 @@ export const updatePreferences = createPreferencesRequestFn('PATCH'); export const getPreferences = async ( context: IRestApiContext, -): Promise => { +): Promise< + SourceControlPublicPreferences | SourceControlProjectPreferences | SourceControlPreferences +> => { return await makeRestApiRequest(context, 'GET', `${sourceControlApiRoot}/preferences`); }; diff --git a/packages/frontend/editor-ui/src/features/integrations/sourceControl.ee/sourceControl.store.test.ts b/packages/frontend/editor-ui/src/features/integrations/sourceControl.ee/sourceControl.store.test.ts index b04a40cc477..83b2b084a29 100644 --- a/packages/frontend/editor-ui/src/features/integrations/sourceControl.ee/sourceControl.store.test.ts +++ b/packages/frontend/editor-ui/src/features/integrations/sourceControl.ee/sourceControl.store.test.ts @@ -164,10 +164,17 @@ describe('useSourceControlStore', () => { }); describe('disconnect', () => { - it('should call API and reset preferences', async () => { + it('should call API, reset active source control state, and keep reconnect inputs', async () => { sourceControlStore.preferences.connected = true; - sourceControlStore.preferences.repositoryUrl = 'https://github.com/user/repo.git'; + sourceControlStore.preferences.repositoryUrl = 'git@github.com:user/repo.git'; + sourceControlStore.preferences.connectionType = 'ssh'; + sourceControlStore.preferences.keyGeneratorType = 'rsa'; + sourceControlStore.preferences.publicKey = 'ssh-rsa AAAATestKey n8n deploy key'; sourceControlStore.preferences.branchName = 'main'; + sourceControlStore.preferences.currentBranch = 'main'; + sourceControlStore.preferences.branchReadOnly = true; + sourceControlStore.preferences.branchColor = '#ff0000'; + sourceControlStore.preferences.branches = ['main', 'develop']; const mockDisconnect = vi.mocked(vcApi.disconnect); mockDisconnect.mockResolvedValue('Disconnected successfully'); @@ -177,6 +184,14 @@ describe('useSourceControlStore', () => { expect(mockDisconnect).toHaveBeenCalledWith({}, false); expect(sourceControlStore.preferences.connected).toBe(false); expect(sourceControlStore.preferences.branches).toEqual([]); + expect(sourceControlStore.preferences.branchName).toBe(''); + expect(sourceControlStore.preferences.currentBranch).toBe(''); + expect(sourceControlStore.preferences.branchReadOnly).toBe(false); + expect(sourceControlStore.preferences.branchColor).toBe('#5296D6'); + expect(sourceControlStore.preferences.repositoryUrl).toBe('git@github.com:user/repo.git'); + expect(sourceControlStore.preferences.connectionType).toBe('ssh'); + expect(sourceControlStore.preferences.keyGeneratorType).toBe('rsa'); + expect(sourceControlStore.preferences.publicKey).toBe('ssh-rsa AAAATestKey n8n deploy key'); }); }); diff --git a/packages/frontend/editor-ui/src/features/integrations/sourceControl.ee/sourceControl.store.ts b/packages/frontend/editor-ui/src/features/integrations/sourceControl.ee/sourceControl.store.ts index cb3d9755a69..28add3310aa 100644 --- a/packages/frontend/editor-ui/src/features/integrations/sourceControl.ee/sourceControl.store.ts +++ b/packages/frontend/editor-ui/src/features/integrations/sourceControl.ee/sourceControl.store.ts @@ -9,6 +9,8 @@ import type { TupleToUnion } from '@/app/utils/typeHelpers'; import type { SourceControlledFile } from '@n8n/api-types'; import type { AutoPublishMode } from 'n8n-workflow'; +const DEFAULT_BRANCH_COLOR = '#5296D6'; + export const useSourceControlStore = defineStore('sourceControl', () => { const rootStore = useRootStore(); const settingsStore = useSettingsStore(); @@ -27,7 +29,7 @@ export const useSourceControlStore = defineStore('sourceControl', () => { branches: [], repositoryUrl: '', branchReadOnly: false, - branchColor: '#5296D6', + branchColor: DEFAULT_BRANCH_COLOR, connected: false, publicKey: '', keyGeneratorType: 'ed25519', @@ -63,8 +65,8 @@ export const useSourceControlStore = defineStore('sourceControl', () => { const makePreferencesAction = (action: typeof vcApi.savePreferences) => - async (preferences: Partial) => { - const data = await action(rootStore.restApiContext, preferences); + async (preferencesUpdate: Partial) => { + const data = await action(rootStore.restApiContext, preferencesUpdate); setPreferences(data); }; @@ -84,16 +86,27 @@ export const useSourceControlStore = defineStore('sourceControl', () => { const disconnect = async (keepKeyPair: boolean) => { await vcApi.disconnect(rootStore.restApiContext, keepKeyPair); - setPreferences({ connected: false, branches: [] }); + + // Connection related preferences are intentionally ommited here. + // This allows users to disconnect and reconnect when troubleshooting issues. + setPreferences({ + connected: false, + branches: [], + branchName: '', + currentBranch: '', + branchReadOnly: false, + branchColor: DEFAULT_BRANCH_COLOR, + }); }; const generateKeyPair = async (keyGeneratorType?: TupleToUnion) => { await vcApi.generateKeyPair(rootStore.restApiContext, keyGeneratorType); const data = await vcApi.getPreferences(rootStore.restApiContext); // To be removed once the API is updated - preferences.publicKey = data.publicKey; + const publicKey = 'publicKey' in data ? data.publicKey : undefined; + preferences.publicKey = publicKey; - return { publicKey: data.publicKey }; + return { publicKey }; }; const getStatus = async () => { diff --git a/packages/frontend/editor-ui/src/features/integrations/sourceControl.ee/sourceControl.types.ts b/packages/frontend/editor-ui/src/features/integrations/sourceControl.ee/sourceControl.types.ts index 62b5892faf1..70bd07cd892 100644 --- a/packages/frontend/editor-ui/src/features/integrations/sourceControl.ee/sourceControl.types.ts +++ b/packages/frontend/editor-ui/src/features/integrations/sourceControl.ee/sourceControl.types.ts @@ -3,13 +3,23 @@ import type { TupleToUnion } from '@/app/utils/typeHelpers'; export type SshKeyTypes = ['ed25519', 'rsa']; -export type SourceControlPreferences = { - connected: boolean; - repositoryUrl: string; - branchName: string; - branches: string[]; +// The endpoint returns a subset of these fields depending on the caller's access: +// - Any authenticated user: SourceControlPublicPreferences +// - Project admin (sourceControl:push on a team project): SourceControlProjectPreferences +// - Global admin (sourceControl:manage): SourceControlPreferences (full) +export type SourceControlPublicPreferences = { branchReadOnly: boolean; +}; + +export type SourceControlProjectPreferences = SourceControlPublicPreferences & { + connected: boolean; + branchName: string; branchColor: string; +}; + +export type SourceControlPreferences = SourceControlProjectPreferences & { + repositoryUrl: string; + branches: string[]; publicKey?: string; keyGeneratorType?: TupleToUnion; currentBranch?: string;