From 6b7928d3a015914ffd3fd57191b50ad5bc2e1eb6 Mon Sep 17 00:00:00 2001 From: Valya Bullions Date: Mon, 22 Jul 2024 14:44:52 +0100 Subject: [PATCH] feat: Allow sharing to and from team projects (no-changelog) --- .../src/credentials/credentials.controller.ts | 22 +- .../src/credentials/credentials.service.ee.ts | 70 ++++-- packages/cli/src/permissions/project-roles.ts | 1 + .../credentials/credentials.api.ee.test.ts | 200 +++++++++++++++++- .../credentials/credentials.api.test.ts | 8 +- 5 files changed, 268 insertions(+), 33 deletions(-) diff --git a/packages/cli/src/credentials/credentials.controller.ts b/packages/cli/src/credentials/credentials.controller.ts index ec592519bb5e0..c282cfbc09c4f 100644 --- a/packages/cli/src/credentials/credentials.controller.ts +++ b/packages/cli/src/credentials/credentials.controller.ts @@ -308,25 +308,22 @@ export class CredentialsController { let newShareeIds: string[] = []; await Db.transaction(async (trx) => { - const currentPersonalProjectIDs = credential.shared + const currentProjectIDs = credential.shared .filter((sc) => sc.role === 'credential:user') .map((sc) => sc.projectId); - const newPersonalProjectIds = shareWithIds; + const newProjectIds = shareWithIds; - const toShare = utils.rightDiff( - [currentPersonalProjectIDs, (id) => id], - [newPersonalProjectIds, (id) => id], - ); + const toShare = utils.rightDiff([currentProjectIDs, (id) => id], [newProjectIds, (id) => id]); const toUnshare = utils.rightDiff( - [newPersonalProjectIds, (id) => id], - [currentPersonalProjectIDs, (id) => id], + [newProjectIds, (id) => id], + [currentProjectIDs, (id) => id], ); const deleteResult = await trx.delete(SharedCredentials, { credentialsId: credentialId, projectId: In(toUnshare), }); - await this.enterpriseCredentialsService.shareWithProjects(credential, toShare, trx); + await this.enterpriseCredentialsService.shareWithProjects(req.user, credential, toShare, trx); if (deleteResult.affected) { amountRemoved = deleteResult.affected; @@ -369,12 +366,17 @@ export class CredentialsController { @Put('/:credentialId/transfer') @ProjectScope('credential:move') async transfer(req: CredentialRequest.Transfer) { - const body = z.object({ destinationProjectId: z.string() }).parse(req.body); + // TODO: make shareWithSource non-optional once the frontend + // has support + const body = z + .object({ destinationProjectId: z.string(), shareWithSource: z.boolean().optional() }) + .parse(req.body); return await this.enterpriseCredentialsService.transferOne( req.user, req.params.credentialId, body.destinationProjectId, + body.shareWithSource ?? false, ); } } diff --git a/packages/cli/src/credentials/credentials.service.ee.ts b/packages/cli/src/credentials/credentials.service.ee.ts index 981ecc5d59fc9..83d4012bc2475 100644 --- a/packages/cli/src/credentials/credentials.service.ee.ts +++ b/packages/cli/src/credentials/credentials.service.ee.ts @@ -12,6 +12,7 @@ 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'; +import { RoleService } from '@/services/role.service'; @Service() export class EnterpriseCredentialsService { @@ -20,9 +21,11 @@ export class EnterpriseCredentialsService { private readonly ownershipService: OwnershipService, private readonly credentialsService: CredentialsService, private readonly projectService: ProjectService, + private readonly roleService: RoleService, ) {} async shareWithProjects( + user: User, credential: CredentialsEntity, shareWithIds: string[], entityManager?: EntityManager, @@ -30,19 +33,35 @@ export class EnterpriseCredentialsService { const em = entityManager ?? this.sharedCredentialsRepository.manager; const projects = await em.find(Project, { - where: { id: In(shareWithIds), type: 'personal' }, + where: [ + { + id: In(shareWithIds), + type: 'team', + // if user can see all projects, don't check project access + // if they don't, find projects they can list + ...(user.hasGlobalScope('project:list') + ? {} + : { + projectRelations: { + userId: user.id, + role: In(this.roleService.rolesWithScope('project', 'project:list')), + }, + }), + }, + { + id: In(shareWithIds), + type: 'personal', + }, + ], }); - const newSharedCredentials = projects - // We filter by role === 'project:personalOwner' above and there should - // always only be one owner. - .map((project) => - this.sharedCredentialsRepository.create({ - credentialsId: credential.id, - role: 'credential:user', - projectId: project.id, - }), - ); + const newSharedCredentials = projects.map((project) => + this.sharedCredentialsRepository.create({ + credentialsId: credential.id, + role: 'credential:user', + projectId: project.id, + }), + ); return await em.save(newSharedCredentials); } @@ -97,7 +116,12 @@ export class EnterpriseCredentialsService { return { ...rest }; } - async transferOne(user: User, credentialId: string, destinationProjectId: string) { + async transferOne( + user: User, + credentialId: string, + destinationProjectId: string, + shareWithSource: boolean, + ) { // 1. get credential const credential = await this.sharedCredentialsRepository.findCredentialForUser( credentialId, @@ -147,8 +171,26 @@ export class EnterpriseCredentialsService { await this.sharedCredentialsRepository.manager.transaction(async (trx) => { // 6. transfer the credential - // remove all sharings - await trx.remove(credential.shared); + + // remove original owner sharing + await trx.remove(ownerSharing); + + // share it back as a user if asked to + if (shareWithSource) { + await trx.save( + trx.create(SharedCredentials, { + credentialsId: credential.id, + projectId: sourceProject.id, + role: 'credential:user', + }), + ); + } + + // remove any previous sharings with the new owner + await trx.delete(SharedCredentials, { + credentialsId: credential.id, + projectId: destinationProjectId, + }); // create new owner-sharing await trx.save( diff --git a/packages/cli/src/permissions/project-roles.ts b/packages/cli/src/permissions/project-roles.ts index 96e4dfff8a475..d05507c47c46c 100644 --- a/packages/cli/src/permissions/project-roles.ts +++ b/packages/cli/src/permissions/project-roles.ts @@ -20,6 +20,7 @@ export const REGULAR_PROJECT_ADMIN_SCOPES: Scope[] = [ 'credential:delete', 'credential:list', 'credential:move', + 'credential:share', 'project:list', 'project:read', 'project:update', 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 df8a562a76768..3d1fd204713a4 100644 --- a/packages/cli/test/integration/credentials/credentials.api.ee.test.ts +++ b/packages/cli/test/integration/credentials/credentials.api.ee.test.ts @@ -48,6 +48,7 @@ let memberPersonalProject: Project; let anotherMember: User; let anotherMemberPersonalProject: Project; let authOwnerAgent: SuperAgentTest; +let authMemberAgent: SuperAgentTest; let authAnotherMemberAgent: SuperAgentTest; let saveCredential: SaveCredentialFunction; const mailer = mockInstance(UserManagementMailer); @@ -73,6 +74,7 @@ beforeEach(async () => { ); authOwnerAgent = testServer.authAgentFor(owner); + authMemberAgent = testServer.authAgentFor(member); authAnotherMemberAgent = testServer.authAgentFor(anotherMember); saveCredential = affixRoleToSaveCredential('credential:owner'); @@ -978,6 +980,128 @@ describe('PUT /credentials/:id/share', () => { config.set('userManagement.emails.mode', 'smtp'); }); + + test('should be able to share from personal project to team project that member has access to', async () => { + const savedCredential = await saveCredential(randomCredentialPayload(), { user: member }); + + const testProject = await createTeamProject(); + await linkUserToProject(member, testProject, 'project:editor'); + + const response = await authMemberAgent + .put(`/credentials/${savedCredential.id}/share`) + .send({ shareWithIds: [testProject.id] }); + + expect(response.statusCode).toBe(200); + expect(response.body.data).toBeUndefined(); + + const shares = await getCredentialSharings(savedCredential); + const testShare = shares.find((s) => s.projectId === testProject.id); + expect(testShare).not.toBeUndefined(); + expect(testShare?.role).toBe('credential:user'); + }); + + test('should be able to share from team project to personal project', async () => { + const testProject = await createTeamProject(undefined, member); + + const savedCredential = await saveCredential(randomCredentialPayload(), { + project: testProject, + }); + + const response = await authMemberAgent + .put(`/credentials/${savedCredential.id}/share`) + .send({ shareWithIds: [anotherMemberPersonalProject.id] }); + + expect(response.statusCode).toBe(200); + expect(response.body.data).toBeUndefined(); + + const shares = await getCredentialSharings(savedCredential); + const testShare = shares.find((s) => s.projectId === anotherMemberPersonalProject.id); + expect(testShare).not.toBeUndefined(); + expect(testShare?.role).toBe('credential:user'); + }); + + test('should be able to share from team project to team project that member has access to', async () => { + const testProject = await createTeamProject(undefined, member); + const testProject2 = await createTeamProject(); + await linkUserToProject(member, testProject2, 'project:editor'); + + const savedCredential = await saveCredential(randomCredentialPayload(), { + project: testProject, + }); + + const response = await authMemberAgent + .put(`/credentials/${savedCredential.id}/share`) + .send({ shareWithIds: [testProject2.id] }); + + expect(response.statusCode).toBe(200); + expect(response.body.data).toBeUndefined(); + + const shares = await getCredentialSharings(savedCredential); + const testShare = shares.find((s) => s.projectId === testProject2.id); + expect(testShare).not.toBeUndefined(); + expect(testShare?.role).toBe('credential:user'); + }); + + test('admins should be able to share from any team project to team project ', async () => { + const testProject = await createTeamProject(); + const testProject2 = await createTeamProject(); + + const savedCredential = await saveCredential(randomCredentialPayload(), { + project: testProject, + }); + + const response = await authOwnerAgent + .put(`/credentials/${savedCredential.id}/share`) + .send({ shareWithIds: [testProject2.id] }); + + expect(response.statusCode).toBe(200); + expect(response.body.data).toBeUndefined(); + + const shares = await getCredentialSharings(savedCredential); + const testShare = shares.find((s) => s.projectId === testProject2.id); + expect(testShare).not.toBeUndefined(); + expect(testShare?.role).toBe('credential:user'); + }); + + test('admins should be able to share from any team project to personal project ', async () => { + const testProject = await createTeamProject(); + + const savedCredential = await saveCredential(randomCredentialPayload(), { + project: testProject, + }); + + const response = await authOwnerAgent + .put(`/credentials/${savedCredential.id}/share`) + .send({ shareWithIds: [memberPersonalProject.id] }); + + expect(response.statusCode).toBe(200); + expect(response.body.data).toBeUndefined(); + + const shares = await getCredentialSharings(savedCredential); + const testShare = shares.find((s) => s.projectId === memberPersonalProject.id); + expect(testShare).not.toBeUndefined(); + expect(testShare?.role).toBe('credential:user'); + }); + + test('admins should be able to share from any personal project to team project ', async () => { + const testProject = await createTeamProject(); + + const savedCredential = await saveCredential(randomCredentialPayload(), { + user: member, + }); + + const response = await authOwnerAgent + .put(`/credentials/${savedCredential.id}/share`) + .send({ shareWithIds: [testProject.id] }); + + expect(response.statusCode).toBe(200); + expect(response.body.data).toBeUndefined(); + + const shares = await getCredentialSharings(savedCredential); + const testShare = shares.find((s) => s.projectId === testProject.id); + expect(testShare).not.toBeUndefined(); + expect(testShare?.role).toBe('credential:user'); + }); }); describe('PUT /:credentialId/transfer', () => { @@ -1058,13 +1182,13 @@ describe('PUT /:credentialId/transfer', () => { .expect(403); }); - test('transferring from a personal project to a team project severs all sharings', async () => { + test('transferring from a personal project to a team project should not sever all sharings', async () => { // // ARRANGE // const credential = await saveCredential(randomCredentialPayload(), { user: member }); - // these sharings should be deleted by the transfer + // these sharings should not be deleted by the transfer await shareCredentialWithUsers(credential, [anotherMember, owner]); const destinationProject = await createTeamProject('Destination Project', member); @@ -1084,12 +1208,72 @@ describe('PUT /:credentialId/transfer', () => { 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', - }); + expect(allSharings).toHaveLength(3); + expect(allSharings).toContainEqual( + expect.objectContaining({ + projectId: destinationProject.id, + credentialsId: credential.id, + role: 'credential:owner', + }), + ); + for (const projectId of [anotherMemberPersonalProject.id, ownerPersonalProject.id]) { + expect(allSharings).toContainEqual( + expect.objectContaining({ + credentialsId: credential.id, + projectId, + role: 'credential:user', + }), + ); + } + }); + + test('transferring should share with original owner when asked', async () => { + // + // ARRANGE + // + const credential = await saveCredential(randomCredentialPayload(), { user: member }); + + // these sharings should not 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, shareWithSource: true }) + .expect(200); + + // + // ASSERT + // + expect(response.body).toEqual({}); + + const allSharings = await getCredentialSharings(credential); + expect(allSharings).toHaveLength(4); + expect(allSharings).toContainEqual( + expect.objectContaining({ + projectId: destinationProject.id, + credentialsId: credential.id, + role: 'credential:owner', + }), + ); + for (const projectId of [ + anotherMemberPersonalProject.id, + ownerPersonalProject.id, + memberPersonalProject.id, + ]) { + expect(allSharings).toContainEqual( + expect.objectContaining({ + credentialsId: credential.id, + projectId, + role: 'credential:user', + }), + ); + } }); test('can transfer from team to another team project', async () => { diff --git a/packages/cli/test/integration/credentials/credentials.api.test.ts b/packages/cli/test/integration/credentials/credentials.api.test.ts index 54de47b16e948..cd66fbe467ef7 100644 --- a/packages/cli/test/integration/credentials/credentials.api.test.ts +++ b/packages/cli/test/integration/credentials/credentials.api.test.ts @@ -142,7 +142,13 @@ describe('GET /credentials', () => { // Team cred expect(cred1.id).toBe(savedCredential1.id); expect(cred1.scopes).toEqual( - ['credential:move', 'credential:read', 'credential:update', 'credential:delete'].sort(), + [ + 'credential:move', + 'credential:read', + 'credential:update', + 'credential:share', + 'credential:delete', + ].sort(), ); // Shared cred