From 202c91e7edc2a99eec56436f94f0e552ac4816b5 Mon Sep 17 00:00:00 2001 From: Danny Martini Date: Tue, 4 Jun 2024 13:54:48 +0200 Subject: [PATCH] feat(core): Allow transferring credentials from any project to any team project (#9563) --- .../src/credentials/credentials.controller.ts | 13 + .../src/credentials/credentials.service.ee.ts | 68 +++++ .../transfer-credential.error.ts | 7 + packages/cli/src/requests.ts | 6 + .../cli/src/workflows/workflow.service.ee.ts | 4 +- .../credentials/credentials.api.ee.test.ts | 253 +++++++++++++++++- .../test/integration/shared/db/credentials.ts | 6 + .../workflows/workflows.controller.ee.test.ts | 33 +-- 8 files changed, 371 insertions(+), 19 deletions(-) create mode 100644 packages/cli/src/errors/response-errors/transfer-credential.error.ts diff --git a/packages/cli/src/credentials/credentials.controller.ts b/packages/cli/src/credentials/credentials.controller.ts index 2542c9d60d..2dfa547a93 100644 --- a/packages/cli/src/credentials/credentials.controller.ts +++ b/packages/cli/src/credentials/credentials.controller.ts @@ -28,6 +28,7 @@ import { SharedCredentialsRepository } from '@/databases/repositories/sharedCred import { In } from '@n8n/typeorm'; import { SharedCredentials } from '@/databases/entities/SharedCredentials'; import { ProjectRelationRepository } from '@/databases/repositories/projectRelation.repository'; +import { z } from 'zod'; @RestController('/credentials') export class CredentialsController { @@ -324,4 +325,16 @@ export class CredentialsController { credentialsName: credential.name, }); } + + @Put('/:credentialId/transfer') + @ProjectScope('credential:move') + async transfer(req: CredentialRequest.Transfer) { + const body = z.object({ destinationProjectId: z.string() }).parse(req.body); + + return await this.enterpriseCredentialsService.transferOne( + req.user, + req.params.credentialId, + body.destinationProjectId, + ); + } } diff --git a/packages/cli/src/credentials/credentials.service.ee.ts b/packages/cli/src/credentials/credentials.service.ee.ts index c90a2d0d57..19342810b9 100644 --- a/packages/cli/src/credentials/credentials.service.ee.ts +++ b/packages/cli/src/credentials/credentials.service.ee.ts @@ -8,6 +8,9 @@ import type { ICredentialDataDecryptedObject } from 'n8n-workflow'; import { NotFoundError } from '@/errors/response-errors/not-found.error'; import { OwnershipService } from '@/services/ownership.service'; import { Project } from '@/databases/entities/Project'; +import { ProjectService } from '@/services/project.service'; +import { TransferCredentialError } from '@/errors/response-errors/transfer-credential.error'; +import { SharedCredentials } from '@/databases/entities/SharedCredentials'; @Service() export class EnterpriseCredentialsService { @@ -15,6 +18,7 @@ export class EnterpriseCredentialsService { private readonly sharedCredentialsRepository: SharedCredentialsRepository, private readonly ownershipService: OwnershipService, private readonly credentialsService: CredentialsService, + private readonly projectService: ProjectService, ) {} async shareWithProjects( @@ -91,4 +95,68 @@ export class EnterpriseCredentialsService { return { ...rest }; } + + async transferOne(user: User, credentialId: string, destinationProjectId: string) { + // 1. get credential + const credential = await this.sharedCredentialsRepository.findCredentialForUser( + credentialId, + user, + ['credential:move'], + ); + NotFoundError.isDefinedAndNotNull( + credential, + `Could not find the credential with the id "${credentialId}". Make sure you have the permission to move it.`, + ); + + // 2. get owner-sharing + const ownerSharing = credential.shared.find((s) => s.role === 'credential:owner'); + NotFoundError.isDefinedAndNotNull( + ownerSharing, + `Could not find owner for credential "${credential.id}"`, + ); + + // 3. get source project + const sourceProject = ownerSharing.project; + + // 4. get destination project + const destinationProject = await this.projectService.getProjectWithScope( + user, + destinationProjectId, + ['credential:create'], + ); + NotFoundError.isDefinedAndNotNull( + destinationProject, + `Could not find project with the id "${destinationProjectId}". Make sure you have the permission to create credentials in it.`, + ); + + // 5. checks + if (sourceProject.id === destinationProject.id) { + throw new TransferCredentialError( + "You can't transfer a credential into the project that's already owning it.", + ); + } + if (sourceProject.type !== 'team' && sourceProject.type !== 'personal') { + throw new TransferCredentialError( + 'You can only transfer credentials out of personal or team projects.', + ); + } + if (destinationProject.type !== 'team') { + throw new TransferCredentialError('You can only transfer credentials into team projects.'); + } + + await this.sharedCredentialsRepository.manager.transaction(async (trx) => { + // 6. transfer the credential + // remove all sharings + await trx.remove(credential.shared); + + // create new owner-sharing + await trx.save( + trx.create(SharedCredentials, { + credentialsId: credential.id, + projectId: destinationProject.id, + role: 'credential:owner', + }), + ); + }); + } } diff --git a/packages/cli/src/errors/response-errors/transfer-credential.error.ts b/packages/cli/src/errors/response-errors/transfer-credential.error.ts new file mode 100644 index 0000000000..f10de3104a --- /dev/null +++ b/packages/cli/src/errors/response-errors/transfer-credential.error.ts @@ -0,0 +1,7 @@ +import { ResponseError } from './abstract/response.error'; + +export class TransferCredentialError extends ResponseError { + constructor(message: string) { + super(message, 400, 400); + } +} diff --git a/packages/cli/src/requests.ts b/packages/cli/src/requests.ts index 20679b3d2a..7532d1953e 100644 --- a/packages/cli/src/requests.ts +++ b/packages/cli/src/requests.ts @@ -217,6 +217,12 @@ export declare namespace CredentialRequest { type Test = AuthenticatedRequest<{}, {}, INodeCredentialTestRequest>; type Share = AuthenticatedRequest<{ credentialId: string }, {}, { shareWithIds: string[] }>; + + type Transfer = AuthenticatedRequest< + { credentialId: string }, + {}, + { destinationProjectId: string } + >; } // ---------------------------------- diff --git a/packages/cli/src/workflows/workflow.service.ee.ts b/packages/cli/src/workflows/workflow.service.ee.ts index 62b4acd27b..959fd412b7 100644 --- a/packages/cli/src/workflows/workflow.service.ee.ts +++ b/packages/cli/src/workflows/workflow.service.ee.ts @@ -249,14 +249,14 @@ export class EnterpriseWorkflowService { ]); NotFoundError.isDefinedAndNotNull( workflow, - `Could not find workflow with the id "${workflowId}". Make sure you have the permission to delete it.`, + `Could not find workflow with the id "${workflowId}". Make sure you have the permission to move it.`, ); // 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}`, + `Could not find owner for workflow "${workflow.id}"`, ); // 3. get source project diff --git a/packages/cli/test/integration/credentials/credentials.api.ee.test.ts b/packages/cli/test/integration/credentials/credentials.api.ee.test.ts index 4ace7b86b3..a0330949f8 100644 --- a/packages/cli/test/integration/credentials/credentials.api.ee.test.ts +++ b/packages/cli/test/integration/credentials/credentials.api.ee.test.ts @@ -16,13 +16,22 @@ import type { SaveCredentialFunction } from '../shared/types'; import * as utils from '../shared/utils'; import { affixRoleToSaveCredential, + getCredentialSharings, shareCredentialWithProjects, shareCredentialWithUsers, } from '../shared/db/credentials'; -import { createManyUsers, createUser, createUserShell } from '../shared/db/users'; +import { + createAdmin, + createManyUsers, + createOwner, + createUser, + createUserShell, +} from '../shared/db/users'; import type { SuperAgentTest } from '../shared/types'; import { mockInstance } from '../../shared/mocking'; +import { createTeamProject, linkUserToProject } from '../shared/db/projects'; + const testServer = utils.setupTestServer({ endpointGroups: ['credentials'], enabledFeatures: ['feat:sharing'], @@ -32,6 +41,7 @@ const testServer = utils.setupTestServer({ }); let owner: User; +let admin: User; let ownerPersonalProject: Project; let member: User; let memberPersonalProject: Project; @@ -50,7 +60,8 @@ beforeEach(async () => { projectRepository = Container.get(ProjectRepository); projectService = Container.get(ProjectService); - owner = await createUser({ role: 'global:owner' }); + owner = await createOwner(); + admin = await createAdmin(); ownerPersonalProject = await projectRepository.getPersonalProjectForUserOrFail(owner.id); member = await createUser({ role: 'global:member' }); @@ -648,6 +659,244 @@ describe('PUT /credentials/:id/share', () => { }); }); +describe('PUT /:credentialId/transfer', () => { + test('cannot transfer into the same project', async () => { + const destinationProject = await createTeamProject('Destination Project', member); + + const credential = await saveCredential(randomCredentialPayload(), { + project: destinationProject, + }); + + await testServer + .authAgentFor(member) + .put(`/credentials/${credential.id}/transfer`) + .send({ destinationProjectId: destinationProject.id }) + .expect(400); + }); + + test('cannot transfer into a personal project', async () => { + const credential = await saveCredential(randomCredentialPayload(), { + user: member, + }); + + await testServer + .authAgentFor(member) + .put(`/credentials/${credential.id}/transfer`) + .send({ destinationProjectId: memberPersonalProject.id }) + .expect(400); + }); + + test('cannot transfer somebody elses credential', async () => { + const destinationProject = await createTeamProject('Destination Project', member); + + const credential = await saveCredential(randomCredentialPayload(), { + user: anotherMember, + }); + + await testServer + .authAgentFor(member) + .put(`/credentials/${credential.id}/transfer`) + .send({ destinationProjectId: destinationProject.id }) + .expect(403); + }); + + test("cannot transfer if you're not a member of the destination project", async () => { + const credential = await saveCredential(randomCredentialPayload(), { + user: member, + }); + + const destinationProject = await createTeamProject('Team Project'); + + await testServer + .authAgentFor(member) + .put(`/credentials/${credential.id}/transfer`) + .send({ destinationProjectId: destinationProject.id }) + .expect(404); + }); + + test('project:editors cannot transfer credentials', async () => { + // + // ARRANGE + // + const sourceProject = await createTeamProject('Source Project'); + await linkUserToProject(member, sourceProject, 'project:editor'); + + const credential = await saveCredential(randomCredentialPayload(), { + project: sourceProject, + }); + + const destinationProject = await createTeamProject('Destination Project', member); + + // + // ACT & ASSERT + // + await testServer + .authAgentFor(member) + .put(`/credentials/${credential.id}/transfer`) + .send({ destinationProjectId: destinationProject.id }) + .expect(403); + }); + + test('transferring from a personal project to a team project severs all sharings', async () => { + // + // ARRANGE + // + const credential = await saveCredential(randomCredentialPayload(), { user: member }); + + // these sharings should be deleted by the transfer + await shareCredentialWithUsers(credential, [anotherMember, owner]); + + const destinationProject = await createTeamProject('Destination Project', member); + + // + // ACT + // + const response = await testServer + .authAgentFor(member) + .put(`/credentials/${credential.id}/transfer`) + .send({ destinationProjectId: destinationProject.id }) + .expect(200); + + // + // ASSERT + // + expect(response.body).toEqual({}); + + const allSharings = await getCredentialSharings(credential); + expect(allSharings).toHaveLength(1); + expect(allSharings[0]).toMatchObject({ + projectId: destinationProject.id, + credentialsId: credential.id, + role: 'credential:owner', + }); + }); + + test('can transfer from team to another team project', async () => { + // + // ARRANGE + // + const sourceProject = await createTeamProject('Team Project 1', member); + const credential = await saveCredential(randomCredentialPayload(), { + project: sourceProject, + }); + + const destinationProject = await createTeamProject('Team Project 2', member); + + // + // ACT + // + const response = await testServer + .authAgentFor(member) + .put(`/credentials/${credential.id}/transfer`) + .send({ destinationProjectId: destinationProject.id }) + .expect(200); + + // + // ASSERT + // + expect(response.body).toEqual({}); + + const allSharings = await getCredentialSharings(credential); + expect(allSharings).toHaveLength(1); + expect(allSharings[0]).toMatchObject({ + projectId: destinationProject.id, + credentialsId: credential.id, + role: 'credential:owner', + }); + }); + + test.each([ + ['owners', () => owner], + ['admins', () => admin], + ])( + '%s can always transfer from any personal or team project into any team project', + async (_name, actor) => { + // + // ARRANGE + // + const sourceProject = await createTeamProject('Source Project', member); + const teamCredential = await saveCredential(randomCredentialPayload(), { + project: sourceProject, + }); + + const personalCredential = await saveCredential(randomCredentialPayload(), { user: member }); + + const destinationProject = await createTeamProject('Destination Project', member); + + // + // ACT + // + const response1 = await testServer + .authAgentFor(actor()) + .put(`/credentials/${teamCredential.id}/transfer`) + .send({ destinationProjectId: destinationProject.id }) + .expect(200); + const response2 = await testServer + .authAgentFor(actor()) + .put(`/credentials/${personalCredential.id}/transfer`) + .send({ destinationProjectId: destinationProject.id }) + .expect(200); + + // + // ASSERT + // + expect(response1.body).toEqual({}); + expect(response2.body).toEqual({}); + + { + const allSharings = await getCredentialSharings(teamCredential); + expect(allSharings).toHaveLength(1); + expect(allSharings[0]).toMatchObject({ + projectId: destinationProject.id, + credentialsId: teamCredential.id, + role: 'credential:owner', + }); + } + + { + const allSharings = await getCredentialSharings(personalCredential); + expect(allSharings).toHaveLength(1); + expect(allSharings[0]).toMatchObject({ + projectId: destinationProject.id, + credentialsId: personalCredential.id, + role: 'credential:owner', + }); + } + }, + ); + + test.each([ + ['owners', () => owner], + ['admins', () => admin], + ])('%s cannot transfer into personal projects', async (_name, actor) => { + // + // ARRANGE + // + const sourceProject = await createTeamProject('Source Project', member); + const teamCredential = await saveCredential(randomCredentialPayload(), { + project: sourceProject, + }); + + const personalCredential = await saveCredential(randomCredentialPayload(), { user: member }); + + const destinationProject = anotherMemberPersonalProject; + + // + // ACT & ASSERT + // + await testServer + .authAgentFor(actor()) + .put(`/credentials/${teamCredential.id}/transfer`) + .send({ destinationProjectId: destinationProject.id }) + .expect(400); + await testServer + .authAgentFor(actor()) + .put(`/credentials/${personalCredential.id}/transfer`) + .send({ destinationProjectId: destinationProject.id }) + .expect(400); + }); +}); + function validateMainCredentialData(credential: ListQuery.Credentials.WithOwnedByAndSharedWith) { expect(typeof credential.name).toBe('string'); expect(typeof credential.type).toBe('string'); diff --git a/packages/cli/test/integration/shared/db/credentials.ts b/packages/cli/test/integration/shared/db/credentials.ts index 9464f06bf0..046d27db26 100644 --- a/packages/cli/test/integration/shared/db/credentials.ts +++ b/packages/cli/test/integration/shared/db/credentials.ts @@ -145,3 +145,9 @@ export const getCredentialById = async (id: string) => export async function getAllSharedCredentials() { return await Container.get(SharedCredentialsRepository).find(); } + +export async function getCredentialSharings(credential: CredentialsEntity) { + return await Container.get(SharedCredentialsRepository).findBy({ + credentialsId: credential.id, + }); +} 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 581d9ecaac..d87b143ee0 100644 --- a/packages/cli/test/integration/workflows/workflows.controller.ee.test.ts +++ b/packages/cli/test/integration/workflows/workflows.controller.ee.test.ts @@ -1257,9 +1257,9 @@ describe('PUT /:workflowId/transfer', () => { }); test('cannot transfer into a personal project', async () => { - const destinationProject = await createTeamProject('Team Project', member); + const sourceProject = await createTeamProject('Team Project', member); - const workflow = await createWorkflow({}, destinationProject); + const workflow = await createWorkflow({}, sourceProject); await testServer .authAgentFor(member) @@ -1268,7 +1268,7 @@ describe('PUT /:workflowId/transfer', () => { .expect(400); }); - test('cannot transfer without workflow:move scope for the workflow', async () => { + test('cannot transfer somebody elses workflow', async () => { const destinationProject = await createTeamProject('Team Project', member); const workflow = await createWorkflow({}, anotherMember); @@ -1280,7 +1280,7 @@ describe('PUT /:workflowId/transfer', () => { .expect(403); }); - test('cannot transfer without workflow:create scope in destination project', async () => { + test("cannot transfer if you're not a member of the destination project", async () => { const destinationProject = await createTeamProject('Team Project', anotherMember); const workflow = await createWorkflow({}, member); @@ -1296,13 +1296,14 @@ describe('PUT /:workflowId/transfer', () => { // // ARRANGE // - const sourceProject = await createTeamProject('Team Project 1'); + const sourceProject = await createTeamProject(); await linkUserToProject(member, sourceProject, 'project:editor'); - const destinationProject = await createTeamProject(); - await linkUserToProject(member, destinationProject, 'project:admin'); const workflow = await createWorkflow({}, sourceProject); + const destinationProject = await createTeamProject(); + await linkUserToProject(member, destinationProject, 'project:admin'); + // // ACT & ASSERT // @@ -1319,7 +1320,7 @@ describe('PUT /:workflowId/transfer', () => { // const workflow = await createWorkflow({}, member); - // this sharing should be deleted by the transfer + // these sharings should be deleted by the transfer await shareWorkflowWithUsers(workflow, [anotherMember, owner]); const destinationProject = await createTeamProject('Team Project', member); @@ -1340,7 +1341,7 @@ describe('PUT /:workflowId/transfer', () => { const allSharings = await getWorkflowSharing(workflow); expect(allSharings).toHaveLength(1); - expect(allSharings).not.toContainEqual({ + expect(allSharings[0]).toMatchObject({ projectId: destinationProject.id, workflowId: workflow.id, role: 'workflow:owner', @@ -1352,10 +1353,10 @@ describe('PUT /:workflowId/transfer', () => { // ARRANGE // const sourceProject = await createTeamProject('Team Project 1', member); - const destinationProject = await createTeamProject('Team Project 2', member); - const workflow = await createWorkflow({}, sourceProject); + const destinationProject = await createTeamProject('Team Project 2', member); + // // ACT // @@ -1389,11 +1390,12 @@ describe('PUT /:workflowId/transfer', () => { // ARRANGE // const sourceProject = await createTeamProject('Source Project', member); - const destinationProject = await createTeamProject('Destination Project', member); - const teamWorkflow = await createWorkflow({}, sourceProject); + const personalWorkflow = await createWorkflow({}, member); + const destinationProject = await createTeamProject('Destination Project', member); + // // ACT // @@ -1444,11 +1446,12 @@ describe('PUT /:workflowId/transfer', () => { // ARRANGE // const sourceProject = await createTeamProject('Source Project', member); - const destinationProject = anotherMemberPersonalProject; - const teamWorkflow = await createWorkflow({}, sourceProject); + const personalWorkflow = await createWorkflow({}, member); + const destinationProject = anotherMemberPersonalProject; + // // ACT & ASSERT //