mirror of
https://github.com/n8n-io/n8n.git
synced 2025-03-05 20:50:17 -08:00
fix(core): Consolidate ownership and sharing data on workflows and credentials (#7920)
## Summary Ensure `ownedBy` and `sharedWith` are present and uniform for credentials and workflows. Details in story: https://linear.app/n8n/issue/PAY-987
This commit is contained in:
parent
0a745d16e4
commit
38b88b946b
|
@ -82,10 +82,6 @@ export class CredentialsService {
|
||||||
return findManyOptions;
|
return findManyOptions;
|
||||||
}
|
}
|
||||||
|
|
||||||
private static addOwnedByAndSharedWith(credentials: CredentialsEntity[]) {
|
|
||||||
return credentials.map((c) => Container.get(OwnershipService).addOwnedByAndSharedWith(c));
|
|
||||||
}
|
|
||||||
|
|
||||||
static async getMany(
|
static async getMany(
|
||||||
user: User,
|
user: User,
|
||||||
options: { listQueryOptions?: ListQuery.Options; onlyOwn?: boolean } = {},
|
options: { listQueryOptions?: ListQuery.Options; onlyOwn?: boolean } = {},
|
||||||
|
@ -98,7 +94,9 @@ export class CredentialsService {
|
||||||
if (returnAll) {
|
if (returnAll) {
|
||||||
const credentials = await Container.get(CredentialsRepository).find(findManyOptions);
|
const credentials = await Container.get(CredentialsRepository).find(findManyOptions);
|
||||||
|
|
||||||
return isDefaultSelect ? this.addOwnedByAndSharedWith(credentials) : credentials;
|
return isDefaultSelect
|
||||||
|
? credentials.map((c) => Container.get(OwnershipService).addOwnedByAndSharedWith(c))
|
||||||
|
: credentials;
|
||||||
}
|
}
|
||||||
|
|
||||||
const ids = await this.getAccessibleCredentials(user.id);
|
const ids = await this.getAccessibleCredentials(user.id);
|
||||||
|
@ -108,7 +106,9 @@ export class CredentialsService {
|
||||||
where: { ...findManyOptions.where, id: In(ids) }, // only accessible credentials
|
where: { ...findManyOptions.where, id: In(ids) }, // only accessible credentials
|
||||||
});
|
});
|
||||||
|
|
||||||
return isDefaultSelect ? this.addOwnedByAndSharedWith(credentials) : credentials;
|
return isDefaultSelect
|
||||||
|
? credentials.map((c) => Container.get(OwnershipService).addOwnedByAndSharedWith(c))
|
||||||
|
: credentials;
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|
|
@ -158,25 +158,31 @@ export namespace ListQuery {
|
||||||
|
|
||||||
type SharedField = Partial<Pick<WorkflowEntity, 'shared'>>;
|
type SharedField = Partial<Pick<WorkflowEntity, 'shared'>>;
|
||||||
|
|
||||||
type OwnedByField = { ownedBy: Pick<IUser, 'id'> | null };
|
type OwnedByField = { ownedBy: SlimUser | null };
|
||||||
|
|
||||||
export type Plain = BaseFields;
|
export type Plain = BaseFields;
|
||||||
|
|
||||||
export type WithSharing = BaseFields & SharedField;
|
export type WithSharing = BaseFields & SharedField;
|
||||||
|
|
||||||
export type WithOwnership = BaseFields & OwnedByField;
|
export type WithOwnership = BaseFields & OwnedByField;
|
||||||
|
|
||||||
|
type SharedWithField = { sharedWith: SlimUser[] };
|
||||||
|
|
||||||
|
export type WithOwnedByAndSharedWith = BaseFields & OwnedByField & SharedWithField;
|
||||||
|
}
|
||||||
|
|
||||||
|
export namespace Credentials {
|
||||||
|
type OwnedByField = { ownedBy: SlimUser | null };
|
||||||
|
|
||||||
|
type SharedWithField = { sharedWith: SlimUser[] };
|
||||||
|
|
||||||
|
export type WithSharing = CredentialsEntity & Partial<Pick<CredentialsEntity, 'shared'>>;
|
||||||
|
|
||||||
|
export type WithOwnedByAndSharedWith = CredentialsEntity & OwnedByField & SharedWithField;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
export namespace Credentials {
|
type SlimUser = Pick<IUser, 'id' | 'email' | 'firstName' | 'lastName'>;
|
||||||
type SlimUser = Pick<IUser, 'id' | 'email' | 'firstName' | 'lastName'>;
|
|
||||||
|
|
||||||
type OwnedByField = { ownedBy: SlimUser | null };
|
|
||||||
|
|
||||||
type SharedWithField = { sharedWith: SlimUser[] };
|
|
||||||
|
|
||||||
export type WithOwnedByAndSharedWith = CredentialsEntity & OwnedByField & SharedWithField;
|
|
||||||
}
|
|
||||||
|
|
||||||
export function hasSharing(
|
export function hasSharing(
|
||||||
workflows: ListQuery.Workflow.Plain[] | ListQuery.Workflow.WithSharing[],
|
workflows: ListQuery.Workflow.Plain[] | ListQuery.Workflow.WithSharing[],
|
||||||
|
|
|
@ -4,9 +4,7 @@ import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.reposi
|
||||||
import type { User } from '@db/entities/User';
|
import type { User } from '@db/entities/User';
|
||||||
import { RoleService } from './role.service';
|
import { RoleService } from './role.service';
|
||||||
import { UserService } from './user.service';
|
import { UserService } from './user.service';
|
||||||
import type { Credentials, ListQuery } from '@/requests';
|
import type { ListQuery } from '@/requests';
|
||||||
import type { Role } from '@db/entities/Role';
|
|
||||||
import type { CredentialsEntity } from '@db/entities/CredentialsEntity';
|
|
||||||
import { ApplicationError } from 'n8n-workflow';
|
import { ApplicationError } from 'n8n-workflow';
|
||||||
|
|
||||||
@Service()
|
@Service()
|
||||||
|
@ -40,37 +38,33 @@ export class OwnershipService {
|
||||||
return sharedWorkflow.user;
|
return sharedWorkflow.user;
|
||||||
}
|
}
|
||||||
|
|
||||||
addOwnedBy(
|
addOwnedByAndSharedWith(
|
||||||
workflow: ListQuery.Workflow.WithSharing,
|
rawWorkflow: ListQuery.Workflow.WithSharing,
|
||||||
workflowOwnerRole: Role,
|
): ListQuery.Workflow.WithOwnedByAndSharedWith;
|
||||||
): ListQuery.Workflow.WithOwnership {
|
addOwnedByAndSharedWith(
|
||||||
const { shared, ...rest } = workflow;
|
rawCredential: ListQuery.Credentials.WithSharing,
|
||||||
|
): ListQuery.Credentials.WithOwnedByAndSharedWith;
|
||||||
|
addOwnedByAndSharedWith(
|
||||||
|
rawEntity: ListQuery.Workflow.WithSharing | ListQuery.Credentials.WithSharing,
|
||||||
|
): ListQuery.Workflow.WithOwnedByAndSharedWith | ListQuery.Credentials.WithOwnedByAndSharedWith {
|
||||||
|
const { shared, ...rest } = rawEntity;
|
||||||
|
|
||||||
const ownerId = shared?.find((s) => s.roleId.toString() === workflowOwnerRole.id)?.userId;
|
const entity = rest as
|
||||||
|
| ListQuery.Workflow.WithOwnedByAndSharedWith
|
||||||
|
| ListQuery.Credentials.WithOwnedByAndSharedWith;
|
||||||
|
|
||||||
return Object.assign(rest, {
|
Object.assign(entity, { ownedBy: null, sharedWith: [] });
|
||||||
ownedBy: ownerId ? { id: ownerId } : null,
|
|
||||||
});
|
|
||||||
}
|
|
||||||
|
|
||||||
addOwnedByAndSharedWith(_credential: CredentialsEntity): Credentials.WithOwnedByAndSharedWith {
|
|
||||||
const { shared, ...rest } = _credential;
|
|
||||||
|
|
||||||
const credential = rest as Credentials.WithOwnedByAndSharedWith;
|
|
||||||
|
|
||||||
credential.ownedBy = null;
|
|
||||||
credential.sharedWith = [];
|
|
||||||
|
|
||||||
shared?.forEach(({ user, role }) => {
|
shared?.forEach(({ user, role }) => {
|
||||||
const { id, email, firstName, lastName } = user;
|
const { id, email, firstName, lastName } = user;
|
||||||
|
|
||||||
if (role.name === 'owner') {
|
if (role.name === 'owner') {
|
||||||
credential.ownedBy = { id, email, firstName, lastName };
|
entity.ownedBy = { id, email, firstName, lastName };
|
||||||
} else {
|
} else {
|
||||||
credential.sharedWith.push({ id, email, firstName, lastName });
|
entity.sharedWith.push({ id, email, firstName, lastName });
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
return credential;
|
return entity;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -23,7 +23,6 @@ import { TestWebhooks } from '@/TestWebhooks';
|
||||||
import { whereClause } from '@/UserManagement/UserManagementHelper';
|
import { whereClause } from '@/UserManagement/UserManagementHelper';
|
||||||
import { InternalHooks } from '@/InternalHooks';
|
import { InternalHooks } from '@/InternalHooks';
|
||||||
import { WorkflowRepository } from '@db/repositories/workflow.repository';
|
import { WorkflowRepository } from '@db/repositories/workflow.repository';
|
||||||
import { RoleService } from '@/services/role.service';
|
|
||||||
import { OwnershipService } from '@/services/ownership.service';
|
import { OwnershipService } from '@/services/ownership.service';
|
||||||
import { isStringArray, isWorkflowIdValid } from '@/utils';
|
import { isStringArray, isWorkflowIdValid } from '@/utils';
|
||||||
import { WorkflowHistoryService } from './workflowHistory/workflowHistory.service.ee';
|
import { WorkflowHistoryService } from './workflowHistory/workflowHistory.service.ee';
|
||||||
|
@ -150,7 +149,7 @@ export class WorkflowsService {
|
||||||
select.tags = { id: true, name: true };
|
select.tags = { id: true, name: true };
|
||||||
}
|
}
|
||||||
|
|
||||||
if (isOwnedByIncluded) relations.push('shared');
|
if (isOwnedByIncluded) relations.push('shared', 'shared.role', 'shared.user');
|
||||||
|
|
||||||
if (typeof where.name === 'string' && where.name !== '') {
|
if (typeof where.name === 'string' && where.name !== '') {
|
||||||
where.name = Like(`%${where.name}%`);
|
where.name = Like(`%${where.name}%`);
|
||||||
|
@ -178,16 +177,14 @@ export class WorkflowsService {
|
||||||
findManyOptions,
|
findManyOptions,
|
||||||
)) as [ListQuery.Workflow.Plain[] | ListQuery.Workflow.WithSharing[], number];
|
)) as [ListQuery.Workflow.Plain[] | ListQuery.Workflow.WithSharing[], number];
|
||||||
|
|
||||||
if (!hasSharing(workflows)) return { workflows, count };
|
return hasSharing(workflows)
|
||||||
|
? {
|
||||||
const workflowOwnerRole = await Container.get(RoleService).findWorkflowOwnerRole();
|
workflows: workflows.map((w) =>
|
||||||
|
Container.get(OwnershipService).addOwnedByAndSharedWith(w),
|
||||||
return {
|
),
|
||||||
workflows: workflows.map((w) =>
|
count,
|
||||||
Container.get(OwnershipService).addOwnedBy(w, workflowOwnerRole),
|
}
|
||||||
),
|
: { workflows, count };
|
||||||
count,
|
|
||||||
};
|
|
||||||
}
|
}
|
||||||
|
|
||||||
static async update(
|
static async update(
|
||||||
|
|
|
@ -211,7 +211,13 @@ describe('GET /workflows', () => {
|
||||||
updatedAt: any(String),
|
updatedAt: any(String),
|
||||||
tags: [{ id: any(String), name: 'A' }],
|
tags: [{ id: any(String), name: 'A' }],
|
||||||
versionId: any(String),
|
versionId: any(String),
|
||||||
ownedBy: { id: owner.id },
|
ownedBy: {
|
||||||
|
id: owner.id,
|
||||||
|
email: any(String),
|
||||||
|
firstName: any(String),
|
||||||
|
lastName: any(String),
|
||||||
|
},
|
||||||
|
sharedWith: [],
|
||||||
}),
|
}),
|
||||||
objectContaining({
|
objectContaining({
|
||||||
id: any(String),
|
id: any(String),
|
||||||
|
@ -221,7 +227,13 @@ describe('GET /workflows', () => {
|
||||||
updatedAt: any(String),
|
updatedAt: any(String),
|
||||||
tags: [],
|
tags: [],
|
||||||
versionId: any(String),
|
versionId: any(String),
|
||||||
ownedBy: { id: owner.id },
|
ownedBy: {
|
||||||
|
id: owner.id,
|
||||||
|
email: any(String),
|
||||||
|
firstName: any(String),
|
||||||
|
lastName: any(String),
|
||||||
|
},
|
||||||
|
sharedWith: [],
|
||||||
}),
|
}),
|
||||||
]),
|
]),
|
||||||
});
|
});
|
||||||
|
@ -231,7 +243,7 @@ describe('GET /workflows', () => {
|
||||||
);
|
);
|
||||||
|
|
||||||
expect(found.nodes).toBeUndefined();
|
expect(found.nodes).toBeUndefined();
|
||||||
expect(found.sharedWith).toBeUndefined();
|
expect(found.sharedWith).toHaveLength(0);
|
||||||
expect(found.usedCredentials).toBeUndefined();
|
expect(found.usedCredentials).toBeUndefined();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
@ -412,8 +424,26 @@ describe('GET /workflows', () => {
|
||||||
expect(response.body).toEqual({
|
expect(response.body).toEqual({
|
||||||
count: 2,
|
count: 2,
|
||||||
data: arrayContaining([
|
data: arrayContaining([
|
||||||
{ id: any(String), ownedBy: { id: owner.id } },
|
{
|
||||||
{ id: any(String), ownedBy: { id: owner.id } },
|
id: any(String),
|
||||||
|
ownedBy: {
|
||||||
|
id: owner.id,
|
||||||
|
email: any(String),
|
||||||
|
firstName: any(String),
|
||||||
|
lastName: any(String),
|
||||||
|
},
|
||||||
|
sharedWith: [],
|
||||||
|
},
|
||||||
|
{
|
||||||
|
id: any(String),
|
||||||
|
ownedBy: {
|
||||||
|
id: owner.id,
|
||||||
|
email: any(String),
|
||||||
|
firstName: any(String),
|
||||||
|
lastName: any(String),
|
||||||
|
},
|
||||||
|
sharedWith: [],
|
||||||
|
},
|
||||||
]),
|
]),
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
|
@ -15,6 +15,7 @@ import {
|
||||||
randomInteger,
|
randomInteger,
|
||||||
randomName,
|
randomName,
|
||||||
} from '../../integration/shared/random';
|
} from '../../integration/shared/random';
|
||||||
|
import { WorkflowEntity } from '@/databases/entities/WorkflowEntity';
|
||||||
|
|
||||||
const wfOwnerRole = () =>
|
const wfOwnerRole = () =>
|
||||||
Object.assign(new Role(), {
|
Object.assign(new Role(), {
|
||||||
|
@ -94,7 +95,7 @@ describe('OwnershipService', () => {
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('addOwnedByAndSharedWith()', () => {
|
describe('addOwnedByAndSharedWith()', () => {
|
||||||
test('should add ownedBy and sharedWith to credential', async () => {
|
test('should add `ownedBy` and `sharedWith` to credential', async () => {
|
||||||
const owner = mockUser();
|
const owner = mockUser();
|
||||||
const editor = mockUser();
|
const editor = mockUser();
|
||||||
|
|
||||||
|
@ -124,6 +125,36 @@ describe('OwnershipService', () => {
|
||||||
]);
|
]);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('should add `ownedBy` and `sharedWith` to workflow', async () => {
|
||||||
|
const owner = mockUser();
|
||||||
|
const editor = mockUser();
|
||||||
|
|
||||||
|
const workflow = new WorkflowEntity();
|
||||||
|
|
||||||
|
workflow.shared = [
|
||||||
|
{ role: mockCredRole('owner'), user: owner },
|
||||||
|
{ role: mockCredRole('editor'), user: editor },
|
||||||
|
] as SharedWorkflow[];
|
||||||
|
|
||||||
|
const { ownedBy, sharedWith } = ownershipService.addOwnedByAndSharedWith(workflow);
|
||||||
|
|
||||||
|
expect(ownedBy).toStrictEqual({
|
||||||
|
id: owner.id,
|
||||||
|
email: owner.email,
|
||||||
|
firstName: owner.firstName,
|
||||||
|
lastName: owner.lastName,
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(sharedWith).toStrictEqual([
|
||||||
|
{
|
||||||
|
id: editor.id,
|
||||||
|
email: editor.email,
|
||||||
|
firstName: editor.firstName,
|
||||||
|
lastName: editor.lastName,
|
||||||
|
},
|
||||||
|
]);
|
||||||
|
});
|
||||||
|
|
||||||
test('should produce an empty sharedWith if no sharee', async () => {
|
test('should produce an empty sharedWith if no sharee', async () => {
|
||||||
const owner = mockUser();
|
const owner = mockUser();
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue