From e01bab58a5c39f3f0dea86dfbd91ad7c99dba293 Mon Sep 17 00:00:00 2001 From: Ricardo Espinoza Date: Fri, 2 May 2025 13:08:30 -0400 Subject: [PATCH] feat(core): Add onlySharedWithMe filter to GET `/credentials` endpoint (no-changelog) (#14885) Co-authored-by: Danny Martini --- .../credentials-get-many-request.dto.ts | 2 + .../src/credentials/credentials.controller.ts | 1 + .../src/credentials/credentials.service.ts | 20 +++- .../repositories/credentials.repository.ts | 22 +++- .../credentials/credentials.api.test.ts | 103 +++++++++++++++++- 5 files changed, 143 insertions(+), 5 deletions(-) diff --git a/packages/@n8n/api-types/src/dto/credentials/credentials-get-many-request.dto.ts b/packages/@n8n/api-types/src/dto/credentials/credentials-get-many-request.dto.ts index 47332ca7f9..917320e05b 100644 --- a/packages/@n8n/api-types/src/dto/credentials/credentials-get-many-request.dto.ts +++ b/packages/@n8n/api-types/src/dto/credentials/credentials-get-many-request.dto.ts @@ -19,4 +19,6 @@ export class CredentialsGetManyRequestQuery extends Z.class({ * This switches `includeScopes` to true to be able to check for the scopes */ includeData: booleanFromString.optional(), + + onlySharedWithMe: booleanFromString.optional(), }) {} diff --git a/packages/cli/src/credentials/credentials.controller.ts b/packages/cli/src/credentials/credentials.controller.ts index e7e0a340c1..a4fa263075 100644 --- a/packages/cli/src/credentials/credentials.controller.ts +++ b/packages/cli/src/credentials/credentials.controller.ts @@ -70,6 +70,7 @@ export class CredentialsController { listQueryOptions: req.listQueryOptions, includeScopes: query.includeScopes, includeData: query.includeData, + onlySharedWithMe: query.onlySharedWithMe, }); credentials.forEach((c) => { // @ts-expect-error: This is to emulate the old behavior of removing the shared diff --git a/packages/cli/src/credentials/credentials.service.ts b/packages/cli/src/credentials/credentials.service.ts index a9528fe1e8..411a581e52 100644 --- a/packages/cli/src/credentials/credentials.service.ts +++ b/packages/cli/src/credentials/credentials.service.ts @@ -72,10 +72,12 @@ export class CredentialsService { listQueryOptions = {}, includeScopes = false, includeData = false, + onlySharedWithMe = false, }: { listQueryOptions?: ListQuery.Options & { includeData?: boolean }; includeScopes?: boolean; includeData?: boolean; + onlySharedWithMe?: boolean; } = {}, ) { const returnAll = user.hasGlobalScope('credential:list'); @@ -85,6 +87,14 @@ export class CredentialsService { ? listQueryOptions.filter.projectId : undefined; + if (onlySharedWithMe) { + listQueryOptions.filter = { + ...listQueryOptions.filter, + withRole: 'credential:user', + user, + }; + } + if (includeData) { // We need the scopes to check if we're allowed to include the decrypted // data. @@ -118,7 +128,10 @@ export class CredentialsService { // it's shared to a project, it won't be able to find the home project. // To solve this, we have to get all the relation now, even though // we're deleting them later. - if ((listQueryOptions.filter?.shared as { projectId?: string })?.projectId) { + if ( + (listQueryOptions.filter?.shared as { projectId?: string })?.projectId ?? + onlySharedWithMe + ) { const relations = await this.sharedCredentialsRepository.getAllRelationsForCredentials( credentials.map((c) => c.id), ); @@ -168,7 +181,10 @@ export class CredentialsService { // it's shared to a project, it won't be able to find the home project. // To solve this, we have to get all the relation now, even though // we're deleting them later. - if ((listQueryOptions.filter?.shared as { projectId?: string })?.projectId) { + if ( + (listQueryOptions.filter?.shared as { projectId?: string })?.projectId ?? + onlySharedWithMe + ) { const relations = await this.sharedCredentialsRepository.getAllRelationsForCredentials( credentials.map((c) => c.id), ); diff --git a/packages/cli/src/databases/repositories/credentials.repository.ts b/packages/cli/src/databases/repositories/credentials.repository.ts index f32af0e0df..51d91db4c2 100644 --- a/packages/cli/src/databases/repositories/credentials.repository.ts +++ b/packages/cli/src/databases/repositories/credentials.repository.ts @@ -1,3 +1,4 @@ +import type { User } from '@n8n/db'; import { CredentialsEntity } from '@n8n/db'; import { Service } from '@n8n/di'; import { DataSource, In, Repository, Like } from '@n8n/typeorm'; @@ -19,7 +20,7 @@ export class CredentialsRepository extends Repository { } async findMany( - listQueryOptions?: ListQuery.Options & { includeData?: boolean }, + listQueryOptions?: ListQuery.Options & { includeData?: boolean; user?: User }, credentialIds?: string[], ) { const findManyOptions = this.toFindManyOptions(listQueryOptions); @@ -36,7 +37,7 @@ export class CredentialsRepository extends Repository { type Select = Array; - const defaultRelations = ['shared', 'shared.project']; + const defaultRelations = ['shared', 'shared.project', 'shared.project.projectRelations']; const defaultSelect: Select = ['id', 'name', 'type', 'isManaged', 'createdAt', 'updatedAt']; if (!listQueryOptions) return { select: defaultSelect, relations: defaultRelations }; @@ -99,6 +100,23 @@ export class CredentialsRepository extends Repository { }; delete filter.withRole; } + + if ( + filter.user && + typeof filter.user === 'object' && + 'id' in filter.user && + typeof filter.user.id === 'string' + ) { + filter.shared = { + ...(filter?.shared ? filter.shared : {}), + project: { + projectRelations: { + userId: filter.user.id, + }, + }, + }; + delete filter.user; + } } async getManyByIds(ids: string[], { withSharings } = { withSharings: false }) { diff --git a/packages/cli/test/integration/credentials/credentials.api.test.ts b/packages/cli/test/integration/credentials/credentials.api.test.ts index 5402a922aa..36caa1227b 100644 --- a/packages/cli/test/integration/credentials/credentials.api.test.ts +++ b/packages/cli/test/integration/credentials/credentials.api.test.ts @@ -25,7 +25,7 @@ import { shareCredentialWithUsers, } from '../shared/db/credentials'; import { createTeamProject, linkUserToProject } from '../shared/db/projects'; -import { createManyUsers, createMember, createOwner } from '../shared/db/users'; +import { createAdmin, createManyUsers, createMember, createOwner } from '../shared/db/users'; import { randomCredentialPayload as payload, randomCredentialPayload, @@ -42,6 +42,7 @@ const testServer = setupTestServer({ endpointGroups: ['credentials'] }); let owner: User; let member: User; +let admin: User; let secondMember: User; let ownerPersonalProject: Project; @@ -49,6 +50,7 @@ let memberPersonalProject: Project; let authOwnerAgent: SuperAgentTest; let authMemberAgent: SuperAgentTest; +let authAdminAgent: SuperAgentTest; let projectRepository: ProjectRepository; let sharedCredentialsRepository: SharedCredentialsRepository; @@ -58,6 +60,7 @@ beforeEach(async () => { owner = await createOwner(); member = await createMember(); + admin = await createAdmin(); secondMember = await createMember(); ownerPersonalProject = await Container.get(ProjectRepository).getPersonalProjectForUserOrFail( @@ -69,6 +72,7 @@ beforeEach(async () => { authOwnerAgent = testServer.authAgentFor(owner); authMemberAgent = testServer.authAgentFor(member); + authAdminAgent = testServer.authAgentFor(admin); projectRepository = Container.get(ProjectRepository); sharedCredentialsRepository = Container.get(SharedCredentialsRepository); @@ -1509,6 +1513,103 @@ describe('POST /credentials/test', () => { data: credential.data, }); }); + + test('should return only credentials shared with me when ?onlySharedWithMe=true (owner)', async () => { + await saveCredential(randomCredentialPayload(), { + user: owner, + role: 'credential:owner', + }); + + await saveCredential(randomCredentialPayload(), { + user: owner, + role: 'credential:owner', + }); + + const memberCredential = await saveCredential(randomCredentialPayload(), { + user: member, + role: 'credential:owner', + }); + + await shareCredentialWithUsers(memberCredential, [owner]); + + let response; + + response = await authOwnerAgent.get('/credentials').query({ onlySharedWithMe: true }); + expect(response.statusCode).toBe(200); + expect(response.body.data).toHaveLength(1); + expect(response.body.data[0].id).toBe(memberCredential.id); + expect(response.body.data[0].homeProject).not.toBeNull(); + + response = await authMemberAgent.get('/credentials').query({ onlySharedWithMe: true }); + expect(response.statusCode).toBe(200); + expect(response.body.data).toHaveLength(0); + }); + + test('should return only credentials shared with me when ?onlySharedWithMe=true (admin)', async () => { + await saveCredential(randomCredentialPayload(), { + user: admin, + role: 'credential:owner', + }); + + await saveCredential(randomCredentialPayload(), { + user: admin, + role: 'credential:owner', + }); + + const memberCredential = await saveCredential(randomCredentialPayload(), { + user: member, + role: 'credential:owner', + }); + + await shareCredentialWithUsers(memberCredential, [admin]); + + let response; + + response = await authAdminAgent.get('/credentials').query({ onlySharedWithMe: true }); + expect(response.statusCode).toBe(200); + expect(response.body.data).toHaveLength(1); + expect(response.body.data[0].id).toBe(memberCredential.id); + expect(response.body.data[0].homeProject).not.toBeNull(); + + response = await authMemberAgent.get('/credentials').query({ onlySharedWithMe: true }); + expect(response.statusCode).toBe(200); + expect(response.body.data).toHaveLength(0); + }); + + test('should return only credentials shared with me when ?onlySharedWithMe=true (member)', async () => { + await saveCredential(randomCredentialPayload(), { + user: member, + role: 'credential:owner', + }); + + await saveCredential(randomCredentialPayload(), { + user: member, + role: 'credential:owner', + }); + + const ownerCredential = await saveCredential(randomCredentialPayload(), { + user: owner, + role: 'credential:owner', + }); + + await shareCredentialWithUsers(ownerCredential, [member]); + + let response; + + response = await authMemberAgent.get('/credentials').query({ onlySharedWithMe: true }); + + expect(response.statusCode).toBe(200); + + expect(response.body.data).toHaveLength(1); + expect(response.body.data[0].id).toBe(ownerCredential.id); + expect(response.body.data[0].homeProject).not.toBeNull(); + + response = await authOwnerAgent.get('/credentials').query({ onlySharedWithMe: true }); + + expect(response.statusCode).toBe(200); + + expect(response.body.data).toHaveLength(0); + }); }); const INVALID_PAYLOADS = [