mirror of
https://github.com/n8n-io/n8n.git
synced 2026-09-24 23:22:38 +08:00
fix(core): Add project id on /new and /from-url endpoints to add project scope auth (#21865)
This commit is contained in:
@@ -3,4 +3,5 @@ import { Z } from 'zod-class';
|
||||
|
||||
export class ImportWorkflowFromUrlDto extends Z.class({
|
||||
url: z.string().url(),
|
||||
projectId: z.string(),
|
||||
}) {}
|
||||
|
||||
@@ -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<AuthenticatedRequest>();
|
||||
const res = mock<Response>();
|
||||
const projectService = mock<ProjectService>();
|
||||
|
||||
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);
|
||||
});
|
||||
|
||||
@@ -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, {}>;
|
||||
|
||||
|
||||
@@ -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<IWorkflowResponse>(query.url);
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
@@ -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', () => {
|
||||
|
||||
@@ -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<WorkflowDataUpdate | undefined> {
|
||||
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;
|
||||
|
||||
@@ -581,9 +581,10 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => {
|
||||
return createWorkflowObject(nodes, connections);
|
||||
}
|
||||
|
||||
async function getWorkflowFromUrl(url: string): Promise<IWorkflowDb> {
|
||||
async function getWorkflowFromUrl(url: string, projectId: string): Promise<IWorkflowDb> {
|
||||
return await makeRestApiRequest(rootStore.restApiContext, 'GET', '/workflows/from-url', {
|
||||
url,
|
||||
projectId,
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user