Skip to content

Commit

Permalink
fix(core): Only show personal credentials in the personal space (n8n-…
Browse files Browse the repository at this point in the history
  • Loading branch information
despairblue authored Jan 9, 2025
1 parent 980d0bc commit 8a42d55
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 31 deletions.
26 changes: 4 additions & 22 deletions packages/cli/src/credentials/credentials.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);

Expand All @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});

Expand Down

0 comments on commit 8a42d55

Please sign in to comment.