diff --git a/packages/@n8n/api-types/src/dto/workflows/import-workflow-from-url.dto.ts b/packages/@n8n/api-types/src/dto/workflows/import-workflow-from-url.dto.ts index 91efe304f1d..248c079f649 100644 --- a/packages/@n8n/api-types/src/dto/workflows/import-workflow-from-url.dto.ts +++ b/packages/@n8n/api-types/src/dto/workflows/import-workflow-from-url.dto.ts @@ -3,4 +3,5 @@ import { Z } from 'zod-class'; export class ImportWorkflowFromUrlDto extends Z.class({ url: z.string().url(), + projectId: z.string(), }) {} diff --git a/packages/cli/src/workflows/__tests__/workflows.controller.test.ts b/packages/cli/src/workflows/__tests__/workflows.controller.test.ts index 89247c2bc07..3f501a8c911 100644 --- a/packages/cli/src/workflows/__tests__/workflows.controller.test.ts +++ b/packages/cli/src/workflows/__tests__/workflows.controller.test.ts @@ -5,8 +5,10 @@ import type { Response } from 'express'; import { mock } from 'jest-mock-extended'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; +import { ForbiddenError } from '@/errors/response-errors/forbidden.error'; import { NotFoundError } from '@/errors/response-errors/not-found.error'; import type { ExecutionService } from '@/executions/execution.service'; +import type { ProjectService } from '@/services/project.service.ee'; import { WorkflowsController } from '../workflows.controller'; @@ -17,31 +19,70 @@ describe('WorkflowsController', () => { const axiosMock = axios.get as jest.Mock; const req = mock(); const res = mock(); + const projectService = mock(); + + beforeEach(() => { + controller.projectService = projectService; + jest.clearAllMocks(); + }); describe('getFromUrl', () => { + const projectId = 'project-123'; + describe('should return workflow data', () => { - it('when the URL points to a valid JSON file', async () => { + it('when the URL points to a valid JSON file and user has permissions', async () => { const mockWorkflowData = { nodes: [], connections: {}, }; + projectService.getProjectWithScope.mockResolvedValue({} as any); axiosMock.mockResolvedValue({ data: mockWorkflowData }); - const query: ImportWorkflowFromUrlDto = { url: 'https://example.com/workflow.json' }; + const query: ImportWorkflowFromUrlDto = { + url: 'https://example.com/workflow.json', + projectId, + }; const result = await controller.getFromUrl(req, res, query); expect(result).toEqual(mockWorkflowData); + expect(projectService.getProjectWithScope).toHaveBeenCalledWith(req.user, projectId, [ + 'workflow:create', + ]); expect(axiosMock).toHaveBeenCalledWith(query.url); }); }); + describe('should throw a ForbiddenError', () => { + it('when the user does not have permissions to create workflows in the project', async () => { + projectService.getProjectWithScope.mockResolvedValue(null); + + const query: ImportWorkflowFromUrlDto = { + url: 'https://example.com/workflow.json', + projectId, + }; + + await expect(controller.getFromUrl(req, res, query)).rejects.toThrow(ForbiddenError); + expect(projectService.getProjectWithScope).toHaveBeenCalledWith(req.user, projectId, [ + 'workflow:create', + ]); + expect(axiosMock).not.toHaveBeenCalled(); + }); + }); + describe('should throw a BadRequestError', () => { - const query: ImportWorkflowFromUrlDto = { url: 'https://example.com/invalid.json' }; + beforeEach(() => { + projectService.getProjectWithScope.mockResolvedValue({} as any); + }); it('when the URL does not point to a valid JSON file', async () => { axiosMock.mockRejectedValue(new Error('Network Error')); + const query: ImportWorkflowFromUrlDto = { + url: 'https://example.com/invalid.json', + projectId, + }; + await expect(controller.getFromUrl(req, res, query)).rejects.toThrow(BadRequestError); expect(axiosMock).toHaveBeenCalledWith(query.url); }); @@ -54,6 +95,11 @@ describe('WorkflowsController', () => { axiosMock.mockResolvedValue({ data: invalidWorkflowData }); + const query: ImportWorkflowFromUrlDto = { + url: 'https://example.com/invalid.json', + projectId, + }; + await expect(controller.getFromUrl(req, res, query)).rejects.toThrow(BadRequestError); expect(axiosMock).toHaveBeenCalledWith(query.url); }); @@ -66,6 +112,11 @@ describe('WorkflowsController', () => { axiosMock.mockResolvedValue({ data: incompleteWorkflowData }); + const query: ImportWorkflowFromUrlDto = { + url: 'https://example.com/workflow.json', + projectId, + }; + await expect(controller.getFromUrl(req, res, query)).rejects.toThrow(BadRequestError); expect(axiosMock).toHaveBeenCalledWith(query.url); }); diff --git a/packages/cli/src/workflows/workflow.request.ts b/packages/cli/src/workflows/workflow.request.ts index d5fb12a7949..093b9017f56 100644 --- a/packages/cli/src/workflows/workflow.request.ts +++ b/packages/cli/src/workflows/workflow.request.ts @@ -67,7 +67,7 @@ export declare namespace WorkflowRequest { { forceSave?: string } >; - type NewName = AuthenticatedRequest<{}, {}, {}, { name?: string }>; + type NewName = AuthenticatedRequest<{}, {}, {}, { name?: string; projectId: string }>; type ManualRun = AuthenticatedRequest<{ workflowId: string }, {}, ManualRunPayload, {}>; diff --git a/packages/cli/src/workflows/workflows.controller.ts b/packages/cli/src/workflows/workflows.controller.ts index 07ca1fabdf1..1193cedf196 100644 --- a/packages/cli/src/workflows/workflows.controller.ts +++ b/packages/cli/src/workflows/workflows.controller.ts @@ -33,14 +33,6 @@ import express from 'express'; import { UnexpectedError } from 'n8n-workflow'; import { v4 as uuid } from 'uuid'; -import { WorkflowExecutionService } from './workflow-execution.service'; -import { WorkflowFinderService } from './workflow-finder.service'; -import { WorkflowHistoryService } from './workflow-history/workflow-history.service'; -import { WorkflowRequest } from './workflow.request'; -import { WorkflowService } from './workflow.service'; -import { EnterpriseWorkflowService } from './workflow.service.ee'; -import { CredentialsService } from '../credentials/credentials.service'; - import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { ForbiddenError } from '@/errors/response-errors/forbidden.error'; import { InternalServerError } from '@/errors/response-errors/internal-server.error'; @@ -62,6 +54,14 @@ import * as utils from '@/utils'; import * as WorkflowHelpers from '@/workflow-helpers'; import { userHasScopes } from '@/permissions.ee/check-access'; +import { WorkflowExecutionService } from './workflow-execution.service'; +import { WorkflowFinderService } from './workflow-finder.service'; +import { WorkflowHistoryService } from './workflow-history/workflow-history.service'; +import { WorkflowRequest } from './workflow.request'; +import { WorkflowService } from './workflow.service'; +import { EnterpriseWorkflowService } from './workflow.service.ee'; +import { CredentialsService } from '../credentials/credentials.service'; + @RestController('/workflows') export class WorkflowsController { constructor( @@ -251,6 +251,14 @@ export class WorkflowsController { @Get('/new') async getNewName(req: WorkflowRequest.NewName) { + const projectId = req.query.projectId; + if ( + !(await this.projectService.getProjectWithScope(req.user, projectId, ['workflow:create'])) + ) { + throw new ForbiddenError( + "You don't have the permissions to create a workflow in this project.", + ); + } const requestedName = req.query.name ?? this.globalConfig.workflows.defaultName; const name = await this.namingService.getUniqueWorkflowName(requestedName); @@ -259,10 +267,18 @@ export class WorkflowsController { @Get('/from-url') async getFromUrl( - _req: AuthenticatedRequest, + req: AuthenticatedRequest, _res: express.Response, @Query query: ImportWorkflowFromUrlDto, ) { + const projectId = query.projectId; + if ( + !(await this.projectService.getProjectWithScope(req.user, projectId, ['workflow:create'])) + ) { + throw new ForbiddenError( + "You don't have the permissions to create a workflow in this project.", + ); + } let workflowData: IWorkflowResponse | undefined; try { const { data } = await axios.get(query.url); diff --git a/packages/cli/test/integration/access-control/resource-access-matrix.test.ts b/packages/cli/test/integration/access-control/resource-access-matrix.test.ts index d1980b06d03..94df9574ad7 100644 --- a/packages/cli/test/integration/access-control/resource-access-matrix.test.ts +++ b/packages/cli/test/integration/access-control/resource-access-matrix.test.ts @@ -192,8 +192,8 @@ describe('Resource Access Control Matrix Tests', () => { expect(response.body.data.length).toBeGreaterThan(0); }); - test('GET /workflows/new should return 200', async () => { - await testUserAgent.get('/workflows/new').expect(200); + test('GET /workflows/new should return 403', async () => { + await testUserAgent.get(`/workflows/new?projectId=${teamProject.id}`).expect(403); }); test('GET /workflows/:id should return 200', async () => { @@ -276,7 +276,7 @@ describe('Resource Access Control Matrix Tests', () => { }); test('GET /workflows/new should return 200', async () => { - await testUserAgent.get('/workflows/new').expect(200); + await testUserAgent.get(`/workflows/new?projectId=${teamProject.id}`).expect(200); }); test('GET /workflows/:id should return 200', async () => { diff --git a/packages/cli/test/integration/workflows/workflows.controller.ee.test.ts b/packages/cli/test/integration/workflows/workflows.controller.ee.test.ts index a6886fb1720..dc846c67c77 100644 --- a/packages/cli/test/integration/workflows/workflows.controller.ee.test.ts +++ b/packages/cli/test/integration/workflows/workflows.controller.ee.test.ts @@ -361,11 +361,138 @@ describe('GET /workflows/new', () => { await createWorkflow({ name: 'My workflow' }, owner); await createWorkflow({ name: 'My workflow 7' }, owner); - const response = await authOwnerAgent.get('/workflows/new'); + const response = await authOwnerAgent.get('/workflows/new').query({ + projectId: ownerPersonalProject.id, + }); expect(response.statusCode).toBe(200); expect(response.body.data.name).toEqual('My workflow 8'); }); }); + + test('should return 403 when user does not have workflow:create permission in the project', async () => { + const teamProject = await createTeamProject(); + await linkUserToProject(member, teamProject, 'project:viewer'); + + const response = await authMemberAgent + .get('/workflows/new') + .query({ + projectId: teamProject.id, + }) + .expect(403); + + expect(response.body).toMatchObject({ + message: "You don't have the permissions to create a workflow in this project.", + }); + }); + + test('should return 403 when user is not part of the project', async () => { + const teamProject = await createTeamProject(); + await linkUserToProject(anotherMember, teamProject, 'project:admin'); + + const response = await authMemberAgent + .get('/workflows/new') + .query({ + projectId: teamProject.id, + }) + .expect(403); + + expect(response.body).toMatchObject({ + message: "You don't have the permissions to create a workflow in this project.", + }); + }); + + test('should allow user with workflow:create permission in personal project', async () => { + await createWorkflow({ name: 'My workflow' }, member); + + const response = await authMemberAgent + .get('/workflows/new') + .query({ + projectId: memberPersonalProject.id, + }) + .expect(200); + + expect(response.body.data.name).toEqual('My workflow 2'); + }); + + test('should allow user with workflow:create permission in team project', async () => { + const teamProject = await createTeamProject(); + await linkUserToProject(member, teamProject, 'project:editor'); + + const response = await authMemberAgent + .get('/workflows/new') + .query({ + projectId: teamProject.id, + }) + .expect(200); + + // The naming service generates unique names globally, not per-project + expect(response.body.data.name).toBeDefined(); + expect(typeof response.body.data.name).toBe('string'); + }); + + test('should allow project admin to get new workflow name', async () => { + const teamProject = await createTeamProject(); + await linkUserToProject(member, teamProject, 'project:admin'); + + const response = await authMemberAgent + .get('/workflows/new') + .query({ + projectId: teamProject.id, + }) + .expect(200); + + expect(response.body.data.name).toBeDefined(); + }); + + test('should allow instance owner to get new workflow name for any project', async () => { + const teamProject = await createTeamProject(); + await linkUserToProject(anotherMember, teamProject, 'project:admin'); + + const response = await authOwnerAgent + .get('/workflows/new') + .query({ + projectId: teamProject.id, + }) + .expect(200); + + expect(response.body.data.name).toBeDefined(); + }); +}); + +describe('GET /workflows/from-url', () => { + test('should return 403 when user does not have workflow:create permission in the project', async () => { + const teamProject = await createTeamProject(); + await linkUserToProject(member, teamProject, 'project:viewer'); + + const response = await authMemberAgent + .get('/workflows/from-url') + .query({ + url: 'https://example.com/workflow.json', + projectId: teamProject.id, + }) + .expect(403); + + expect(response.body).toMatchObject({ + message: "You don't have the permissions to create a workflow in this project.", + }); + }); + + test('should return 403 when user is not part of the project', async () => { + const teamProject = await createTeamProject(); + await linkUserToProject(anotherMember, teamProject, 'project:admin'); + + const response = await authMemberAgent + .get('/workflows/from-url') + .query({ + url: 'https://example.com/workflow.json', + projectId: teamProject.id, + }) + .expect(403); + + expect(response.body).toMatchObject({ + message: "You don't have the permissions to create a workflow in this project.", + }); + }); }); describe('GET /workflows/:workflowId', () => { diff --git a/packages/frontend/editor-ui/src/app/composables/useCanvasOperations.ts b/packages/frontend/editor-ui/src/app/composables/useCanvasOperations.ts index 00fd192d4db..0bbd146210b 100644 --- a/packages/frontend/editor-ui/src/app/composables/useCanvasOperations.ts +++ b/packages/frontend/editor-ui/src/app/composables/useCanvasOperations.ts @@ -114,7 +114,7 @@ import { computed, nextTick, ref } from 'vue'; import { useClipboard } from '@/app/composables/useClipboard'; import { useUniqueNodeName } from '@/app/composables/useUniqueNodeName'; import { injectWorkflowState } from '@/app/composables/useWorkflowState'; -import { isPresent } from '@/app/utils/typesUtils'; +import { isPresent, tryToParseNumber } from '@/app/utils/typesUtils'; import { useProjectsStore } from '@/features/collaboration/projects/projects.store'; import type { CanvasLayoutEvent } from '@/features/workflows/canvas/composables/useCanvasLayout'; import { chatEventBus } from '@n8n/chat/event-buses'; @@ -128,7 +128,6 @@ import { useFocusPanelStore } from '@/app/stores/focusPanel.store'; import type { TelemetryNdvSource, TelemetryNdvType } from '@/app/types/telemetry'; import { useRoute, useRouter } from 'vue-router'; import { useTemplatesStore } from '@/features/workflows/templates/templates.store'; -import { tryToParseNumber } from '@/app/utils/typesUtils'; import { isValidNodeConnectionType } from '@/app/utils/typeGuards'; import { useParentFolder } from '@/features/core/folders/composables/useParentFolder'; @@ -2142,9 +2141,16 @@ export function useCanvasOperations() { async function fetchWorkflowDataFromUrl(url: string): Promise { let workflowData: WorkflowDataUpdate; + const projectId = projectsStore.currentProjectId ?? projectsStore.personalProject?.id; + if (!projectId) { + // We should never reach this point because the project should be selected before + throw new Error('No project selected'); + return; + } + canvasStore.startLoading(); try { - workflowData = await workflowsStore.getWorkflowFromUrl(url); + workflowData = await workflowsStore.getWorkflowFromUrl(url, projectId); } catch (error) { toast.showError(error, i18n.baseText('nodeView.showError.getWorkflowDataFromUrl.title')); return; diff --git a/packages/frontend/editor-ui/src/app/stores/workflows.store.ts b/packages/frontend/editor-ui/src/app/stores/workflows.store.ts index 70aff03bf09..307017c84f4 100644 --- a/packages/frontend/editor-ui/src/app/stores/workflows.store.ts +++ b/packages/frontend/editor-ui/src/app/stores/workflows.store.ts @@ -581,9 +581,10 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => { return createWorkflowObject(nodes, connections); } - async function getWorkflowFromUrl(url: string): Promise { + async function getWorkflowFromUrl(url: string, projectId: string): Promise { return await makeRestApiRequest(rootStore.restApiContext, 'GET', '/workflows/from-url', { url, + projectId, }); }