diff --git a/packages/cli/src/credentials/credentials.service.ts b/packages/cli/src/credentials/credentials.service.ts index e0939beccb..52a1a3e88d 100644 --- a/packages/cli/src/credentials/credentials.service.ts +++ b/packages/cli/src/credentials/credentials.service.ts @@ -20,7 +20,6 @@ import { CREDENTIAL_BLANKING_VALUE } from '@/constants'; import { CredentialTypes } from '@/credential-types'; import { createCredentialsFromCredentialsEntity } from '@/credentials-helper'; import { CredentialsEntity } from '@/databases/entities/credentials-entity'; -import type { ProjectRelation } from '@/databases/entities/project-relation'; import { SharedCredentials } from '@/databases/entities/shared-credentials'; import type { User } from '@/databases/entities/user'; import { CredentialsRepository } from '@/databases/repositories/credentials.repository'; @@ -85,23 +84,6 @@ export class CredentialsService { listQueryOptions.includeData = true; } - let projectRelations: ProjectRelation[] | undefined = undefined; - if (includeScopes) { - projectRelations = await this.projectService.getProjectRelationsForUser(user); - if (listQueryOptions.filter?.projectId && user.hasGlobalScope('credential:list')) { - // Only instance owners and admins have the credential:list scope - // Those users should be able to use _all_ credentials within their workflows. - // TODO: Change this so we filter by `workflowId` in this case. Require a slight FE change - const projectRelation = projectRelations.find( - (relation) => relation.projectId === listQueryOptions.filter?.projectId, - ); - if (projectRelation?.role === 'project:personalOwner') { - // Will not affect team projects as these have admins, not owners. - delete listQueryOptions.filter?.projectId; - } - } - } - if (returnAll) { let credentials = await this.credentialsRepository.findMany(listQueryOptions); @@ -123,9 +105,8 @@ export class CredentialsService { } if (includeScopes) { - credentials = credentials.map((c) => - this.roleService.addScopes(c, user, projectRelations!), - ); + const projectRelations = await this.projectService.getProjectRelationsForUser(user); + credentials = credentials.map((c) => this.roleService.addScopes(c, user, projectRelations)); } if (includeData) { @@ -179,7 +160,8 @@ export class CredentialsService { } if (includeScopes) { - credentials = credentials.map((c) => this.roleService.addScopes(c, user, projectRelations!)); + const projectRelations = await this.projectService.getProjectRelationsForUser(user); + credentials = credentials.map((c) => this.roleService.addScopes(c, user, projectRelations)); } if (includeData) { diff --git a/packages/cli/test/integration/credentials/credentials.api.test.ts b/packages/cli/test/integration/credentials/credentials.api.test.ts index 7bb0a8918a..a988569890 100644 --- a/packages/cli/test/integration/credentials/credentials.api.test.ts +++ b/packages/cli/test/integration/credentials/credentials.api.test.ts @@ -632,25 +632,25 @@ describe('GET /credentials', () => { expect(response.body.data.map((credential) => credential.id)).toContain(memberCredential.id); }); - test('should return all credentials to instance owners when working on their own personal project', async () => { + test('should not ignore the project filter when the request is done by an owner and also includes the scopes', async () => { const ownerCredential = await saveCredential(payload(), { user: owner, role: 'credential:owner', }); - const memberCredential = await saveCredential(payload(), { - user: member, - role: 'credential:owner', - }); + // should not show up + await saveCredential(payload(), { user: member, role: 'credential:owner' }); const response: GetAllResponse = await testServer .authAgentFor(owner) .get('/credentials') - .query(`filter={ "projectId": "${ownerPersonalProject.id}" }&includeScopes=true`) + .query({ + filter: JSON.stringify({ projectId: ownerPersonalProject.id }), + includeScopes: true, + }) .expect(200); - expect(response.body.data).toHaveLength(2); - expect(response.body.data.map((credential) => credential.id)).toContain(ownerCredential.id); - expect(response.body.data.map((credential) => credential.id)).toContain(memberCredential.id); + expect(response.body.data).toHaveLength(1); + expect(response.body.data[0].id).toBe(ownerCredential.id); }); });