Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(core): Allow transferring workflows from any project to any team project #9534

Merged
merged 12 commits into from
Jun 3, 2024
7 changes: 5 additions & 2 deletions packages/@n8n/permissions/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export type CommunityPackageScope = ResourceScope<
'communityPackage',
'install' | 'uninstall' | 'update' | 'list' | 'manage'
>;
export type CredentialScope = ResourceScope<'credential', DefaultOperations | 'share'>;
export type CredentialScope = ResourceScope<'credential', DefaultOperations | 'share' | 'move'>;
export type ExternalSecretScope = ResourceScope<'externalSecret', 'list' | 'use'>;
export type ExternalSecretProviderScope = ResourceScope<
'externalSecretsProvider',
Expand All @@ -58,7 +58,10 @@ export type TagScope = ResourceScope<'tag'>;
export type UserScope = ResourceScope<'user', DefaultOperations | 'resetPassword' | 'changeRole'>;
export type VariableScope = ResourceScope<'variable'>;
export type WorkersViewScope = ResourceScope<'workersView', 'manage'>;
export type WorkflowScope = ResourceScope<'workflow', DefaultOperations | 'share' | 'execute'>;
export type WorkflowScope = ResourceScope<
'workflow',
DefaultOperations | 'share' | 'execute' | 'move'
>;

export type Scope =
| AuditLogsScope
Expand Down
10 changes: 10 additions & 0 deletions packages/cli/src/errors/response-errors/not-found.error.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,16 @@
import { ResponseError } from './abstract/response.error';

export class NotFoundError extends ResponseError {
static isDefinedAndNotNull<T>(
value: T | undefined | null,
message: string,
hint?: string,
): asserts value is T {
if (value === undefined || value === null) {
throw new NotFoundError(message, hint);
}
}

constructor(message: string, hint: string | undefined = undefined) {
super(message, 404, 404, hint);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { ResponseError } from './abstract/response.error';

export class TransferWorkflowError extends ResponseError {
constructor(message: string) {
super(message, 400, 400);
}
}
2 changes: 2 additions & 0 deletions packages/cli/src/permissions/global-roles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export const GLOBAL_OWNER_SCOPES: Scope[] = [
'credential:delete',
'credential:list',
'credential:share',
'credential:move',
'communityPackage:install',
'communityPackage:uninstall',
'communityPackage:update',
Expand Down Expand Up @@ -68,6 +69,7 @@ export const GLOBAL_OWNER_SCOPES: Scope[] = [
'workflow:list',
'workflow:share',
'workflow:execute',
'workflow:move',
'workersView:manage',
'project:list',
'project:create',
Expand Down
4 changes: 4 additions & 0 deletions packages/cli/src/permissions/project-roles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,13 @@ export const REGULAR_PROJECT_ADMIN_SCOPES: Scope[] = [
'workflow:delete',
'workflow:list',
'workflow:execute',
'workflow:move',
'credential:create',
'credential:read',
'credential:update',
'credential:delete',
'credential:list',
'credential:move',
'project:list',
'project:read',
'project:update',
Expand All @@ -32,12 +34,14 @@ export const PERSONAL_PROJECT_OWNER_SCOPES: Scope[] = [
'workflow:list',
'workflow:execute',
'workflow:share',
'workflow:move',
'credential:create',
'credential:read',
'credential:update',
'credential:delete',
'credential:list',
'credential:share',
'credential:move',
'project:list',
'project:read',
];
Expand Down
2 changes: 2 additions & 0 deletions packages/cli/src/permissions/resource-roles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ export const CREDENTIALS_SHARING_OWNER_SCOPES: Scope[] = [
'credential:update',
'credential:delete',
'credential:share',
'credential:move',
];

export const CREDENTIALS_SHARING_USER_SCOPES: Scope[] = ['credential:read'];
Expand All @@ -15,6 +16,7 @@ export const WORKFLOW_SHARING_OWNER_SCOPES: Scope[] = [
'workflow:delete',
'workflow:execute',
'workflow:share',
'workflow:move',
];

export const WORKFLOW_SHARING_EDITOR_SCOPES: Scope[] = [
Expand Down
6 changes: 6 additions & 0 deletions packages/cli/src/workflows/workflow.request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,5 +54,11 @@ export declare namespace WorkflowRequest {

type Share = AuthenticatedRequest<{ workflowId: string }, {}, { shareWithIds: string[] }>;

type Transfer = AuthenticatedRequest<
{ workflowId: string },
{},
{ destinationProjectId: string }
>;

type FromUrl = AuthenticatedRequest<{}, {}, {}, { url?: string }>;
}
104 changes: 103 additions & 1 deletion packages/cli/src/workflows/workflow.service.ee.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Service } from 'typedi';
import omit from 'lodash/omit';
import { ApplicationError, NodeOperationError } from 'n8n-workflow';
import { ApplicationError, NodeOperationError, WorkflowActivationError } from 'n8n-workflow';

import type { CredentialsEntity } from '@db/entities/CredentialsEntity';
import type { User } from '@db/entities/User';
Expand All @@ -20,6 +20,10 @@ import type {
import { OwnershipService } from '@/services/ownership.service';
import { In, type EntityManager } from '@n8n/typeorm';
import { Project } from '@/databases/entities/Project';
import { ProjectService } from '@/services/project.service';
import { ActiveWorkflowManager } from '@/ActiveWorkflowManager';
import { TransferWorkflowError } from '@/errors/response-errors/transfer-workflow.error';
import { SharedWorkflow } from '@/databases/entities/SharedWorkflow';

@Service()
export class EnterpriseWorkflowService {
Expand All @@ -30,6 +34,8 @@ export class EnterpriseWorkflowService {
private readonly credentialsRepository: CredentialsRepository,
private readonly credentialsService: CredentialsService,
private readonly ownershipService: OwnershipService,
private readonly projectService: ProjectService,
private readonly activeWorkflowManager: ActiveWorkflowManager,
) {}

async shareWithProjects(
Expand Down Expand Up @@ -235,4 +241,100 @@ export class EnterpriseWorkflowService {
);
});
}

async transferOne(user: User, workflowId: string, destinationProjectId: string) {
// 1. get workflow
const workflow = await this.sharedWorkflowRepository.findWorkflowForUser(workflowId, user, [
'workflow:move',
]);
NotFoundError.isDefinedAndNotNull(
workflow,
`Could not find workflow with the id "${workflowId}". Make sure you have the permission to delete it.`,
);
Comment on lines +250 to +253
Copy link
Contributor Author

@despairblue despairblue May 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find type guards / assertion functions easier to read than conditional logic and creating an error.

But I don't feel strongly about it, so it's up for debate.


// 2. get owner-sharing
const ownerSharing = workflow.shared.find((s) => s.role === 'workflow:owner')!;
NotFoundError.isDefinedAndNotNull(
ownerSharing,
`Could not find owner for workflow ${workflow.id}`,
);

// 3. get source project
const sourceProject = ownerSharing.project;

// 4. get destination project
const destinationProject = await this.projectService.getProjectWithScope(
user,
destinationProjectId,
['workflow:create'],
);
NotFoundError.isDefinedAndNotNull(
destinationProject,
`Could not find project with the id "${destinationProjectId}". Make sure you have the permission to create workflows in it.`,
);

// 5. checks
if (sourceProject.id === destinationProject.id) {
throw new TransferWorkflowError(
"You can't transfer a workflow into the project that's already owning it.",
);
}
if (sourceProject.type !== 'team' && sourceProject.type !== 'personal') {
throw new TransferWorkflowError(
'You can only transfer workflows out of personal or team projects.',
);
}
if (destinationProject.type !== 'team') {
throw new TransferWorkflowError('You can only transfer workflows into team projects.');
}

// 6. deactivate workflow if necessary
const wasActive = workflow.active;
if (wasActive) {
await this.activeWorkflowManager.remove(workflowId);
}

// 7. transfer the workflow
await this.workflowRepository.manager.transaction(async (trx) => {
// remove all sharings
await trx.remove(workflow.shared);

// create new owner-sharing
await trx.save(
trx.create(SharedWorkflow, {
workflowId: workflow.id,
projectId: destinationProject.id,
role: 'workflow:owner',
}),
);
});

// 8. try to activate it again if it was active
if (wasActive) {
try {
await this.activeWorkflowManager.add(workflowId, 'update');

return;
} catch (error) {
await this.workflowRepository.updateActiveState(workflowId, false);

// Since the transfer worked we return a 200 but also return the
// activation error as data.
if (error instanceof WorkflowActivationError) {
return {
error: error.toJSON
? error.toJSON()
: {
name: error.name,
message: error.message,
},
};
}

throw error;
}
}

return;
}
}
13 changes: 13 additions & 0 deletions packages/cli/src/workflows/workflows.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import { ApplicationError } from 'n8n-workflow';
import { In, type FindOptionsRelations } from '@n8n/typeorm';
import type { Project } from '@/databases/entities/Project';
import { ProjectRelationRepository } from '@/databases/repositories/projectRelation.repository';
import { z } from 'zod';

@RestController('/workflows')
export class WorkflowsController {
Expand Down Expand Up @@ -460,4 +461,16 @@ export class WorkflowsController {
workflow,
});
}

@Put('/:workflowId/transfer')
@ProjectScope('workflow:move')
async transfer(req: WorkflowRequest.Transfer) {
const body = z.object({ destinationProjectId: z.string() }).parse(req.body);

return await this.enterpriseWorkflowService.transferOne(
req.user,
req.params.workflowId,
body.destinationProjectId,
);
}
}
33 changes: 24 additions & 9 deletions packages/cli/test/integration/credentials/credentials.api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ describe('GET /credentials', () => {
// Team cred
expect(cred1.id).toBe(savedCredential1.id);
expect(cred1.scopes).toEqual(
['credential:read', 'credential:update', 'credential:delete'].sort(),
['credential:move', 'credential:read', 'credential:update', 'credential:delete'].sort(),
);

// Shared cred
Expand All @@ -172,7 +172,13 @@ describe('GET /credentials', () => {
// Shared cred
expect(cred2.id).toBe(savedCredential2.id);
expect(cred2.scopes).toEqual(
['credential:read', 'credential:update', 'credential:delete', 'credential:share'].sort(),
[
'credential:delete',
'credential:move',
'credential:read',
'credential:share',
'credential:update',
].sort(),
);
}

Expand All @@ -191,11 +197,12 @@ describe('GET /credentials', () => {
expect(cred1.scopes).toEqual(
[
'credential:create',
'credential:read',
'credential:update',
'credential:delete',
'credential:list',
'credential:move',
'credential:read',
'credential:share',
'credential:update',
].sort(),
);

Expand All @@ -204,11 +211,12 @@ describe('GET /credentials', () => {
expect(cred2.scopes).toEqual(
[
'credential:create',
'credential:read',
'credential:update',
'credential:delete',
'credential:list',
'credential:move',
'credential:read',
'credential:share',
'credential:update',
].sort(),
);
}
Expand Down Expand Up @@ -576,7 +584,13 @@ describe('POST /credentials', () => {
expect(encryptedData).not.toBe(payload.data);

expect(scopes).toEqual(
['credential:read', 'credential:update', 'credential:delete', 'credential:share'].sort(),
[
'credential:delete',
'credential:move',
'credential:read',
'credential:share',
'credential:update',
].sort(),
);

const credential = await Container.get(CredentialsRepository).findOneByOrFail({ id });
Expand Down Expand Up @@ -819,11 +833,12 @@ describe('PATCH /credentials/:id', () => {
expect(scopes).toEqual(
[
'credential:create',
'credential:read',
'credential:update',
'credential:delete',
'credential:list',
'credential:move',
'credential:read',
'credential:share',
'credential:update',
].sort(),
);

Expand Down
Loading
Loading