mirror of
https://github.com/n8n-io/n8n.git
synced 2025-01-12 05:17:28 -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;
|
||||
}
|
||||
|
||||
private static addOwnedByAndSharedWith(credentials: CredentialsEntity[]) {
|
||||
return credentials.map((c) => Container.get(OwnershipService).addOwnedByAndSharedWith(c));
|
||||
}
|
||||
|
||||
static async getMany(
|
||||
user: User,
|
||||
options: { listQueryOptions?: ListQuery.Options; onlyOwn?: boolean } = {},
|
||||
|
@ -98,7 +94,9 @@ export class CredentialsService {
|
|||
if (returnAll) {
|
||||
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);
|
||||
|
@ -108,7 +106,9 @@ export class CredentialsService {
|
|||
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 OwnedByField = { ownedBy: Pick<IUser, 'id'> | null };
|
||||
type OwnedByField = { ownedBy: SlimUser | null };
|
||||
|
||||
export type Plain = BaseFields;
|
||||
|
||||
export type WithSharing = BaseFields & SharedField;
|
||||
|
||||
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 OwnedByField = { ownedBy: SlimUser | null };
|
||||
|
||||
type SharedWithField = { sharedWith: SlimUser[] };
|
||||
|
||||
export type WithOwnedByAndSharedWith = CredentialsEntity & OwnedByField & SharedWithField;
|
||||
}
|
||||
type SlimUser = Pick<IUser, 'id' | 'email' | 'firstName' | 'lastName'>;
|
||||
|
||||
export function hasSharing(
|
||||
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 { RoleService } from './role.service';
|
||||
import { UserService } from './user.service';
|
||||
import type { Credentials, ListQuery } from '@/requests';
|
||||
import type { Role } from '@db/entities/Role';
|
||||
import type { CredentialsEntity } from '@db/entities/CredentialsEntity';
|
||||
import type { ListQuery } from '@/requests';
|
||||
import { ApplicationError } from 'n8n-workflow';
|
||||
|
||||
@Service()
|
||||
|
@ -40,37 +38,33 @@ export class OwnershipService {
|
|||
return sharedWorkflow.user;
|
||||
}
|
||||
|
||||
addOwnedBy(
|
||||
workflow: ListQuery.Workflow.WithSharing,
|
||||
workflowOwnerRole: Role,
|
||||
): ListQuery.Workflow.WithOwnership {
|
||||
const { shared, ...rest } = workflow;
|
||||
addOwnedByAndSharedWith(
|
||||
rawWorkflow: ListQuery.Workflow.WithSharing,
|
||||
): ListQuery.Workflow.WithOwnedByAndSharedWith;
|
||||
addOwnedByAndSharedWith(
|
||||
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, {
|
||||
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 = [];
|
||||
Object.assign(entity, { ownedBy: null, sharedWith: [] });
|
||||
|
||||
shared?.forEach(({ user, role }) => {
|
||||
const { id, email, firstName, lastName } = user;
|
||||
|
||||
if (role.name === 'owner') {
|
||||
credential.ownedBy = { id, email, firstName, lastName };
|
||||
entity.ownedBy = { id, email, firstName, lastName };
|
||||
} 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 { InternalHooks } from '@/InternalHooks';
|
||||
import { WorkflowRepository } from '@db/repositories/workflow.repository';
|
||||
import { RoleService } from '@/services/role.service';
|
||||
import { OwnershipService } from '@/services/ownership.service';
|
||||
import { isStringArray, isWorkflowIdValid } from '@/utils';
|
||||
import { WorkflowHistoryService } from './workflowHistory/workflowHistory.service.ee';
|
||||
|
@ -150,7 +149,7 @@ export class WorkflowsService {
|
|||
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 !== '') {
|
||||
where.name = Like(`%${where.name}%`);
|
||||
|
@ -178,16 +177,14 @@ export class WorkflowsService {
|
|||
findManyOptions,
|
||||
)) as [ListQuery.Workflow.Plain[] | ListQuery.Workflow.WithSharing[], number];
|
||||
|
||||
if (!hasSharing(workflows)) return { workflows, count };
|
||||
|
||||
const workflowOwnerRole = await Container.get(RoleService).findWorkflowOwnerRole();
|
||||
|
||||
return {
|
||||
workflows: workflows.map((w) =>
|
||||
Container.get(OwnershipService).addOwnedBy(w, workflowOwnerRole),
|
||||
),
|
||||
count,
|
||||
};
|
||||
return hasSharing(workflows)
|
||||
? {
|
||||
workflows: workflows.map((w) =>
|
||||
Container.get(OwnershipService).addOwnedByAndSharedWith(w),
|
||||
),
|
||||
count,
|
||||
}
|
||||
: { workflows, count };
|
||||
}
|
||||
|
||||
static async update(
|
||||
|
|
|
@ -211,7 +211,13 @@ describe('GET /workflows', () => {
|
|||
updatedAt: any(String),
|
||||
tags: [{ id: any(String), name: 'A' }],
|
||||
versionId: any(String),
|
||||
ownedBy: { id: owner.id },
|
||||
ownedBy: {
|
||||
id: owner.id,
|
||||
email: any(String),
|
||||
firstName: any(String),
|
||||
lastName: any(String),
|
||||
},
|
||||
sharedWith: [],
|
||||
}),
|
||||
objectContaining({
|
||||
id: any(String),
|
||||
|
@ -221,7 +227,13 @@ describe('GET /workflows', () => {
|
|||
updatedAt: any(String),
|
||||
tags: [],
|
||||
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.sharedWith).toBeUndefined();
|
||||
expect(found.sharedWith).toHaveLength(0);
|
||||
expect(found.usedCredentials).toBeUndefined();
|
||||
});
|
||||
|
||||
|
@ -412,8 +424,26 @@ describe('GET /workflows', () => {
|
|||
expect(response.body).toEqual({
|
||||
count: 2,
|
||||
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,
|
||||
randomName,
|
||||
} from '../../integration/shared/random';
|
||||
import { WorkflowEntity } from '@/databases/entities/WorkflowEntity';
|
||||
|
||||
const wfOwnerRole = () =>
|
||||
Object.assign(new Role(), {
|
||||
|
@ -94,7 +95,7 @@ describe('OwnershipService', () => {
|
|||
});
|
||||
|
||||
describe('addOwnedByAndSharedWith()', () => {
|
||||
test('should add ownedBy and sharedWith to credential', async () => {
|
||||
test('should add `ownedBy` and `sharedWith` to credential', async () => {
|
||||
const owner = 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 () => {
|
||||
const owner = mockUser();
|
||||
|
||||
|
|
Loading…
Reference in a new issue