mohiit1502/Automata
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

refactor(core): Remove roleId indirection (no-changelog) (#8413)

Browse Source
This commit is contained in:
कारतोफ्फेलस्क्रिप्ट™
2024-01-24 13:38:57 +01:00
committed by GitHub
parent 1affebd85e
commit d6deceacde
139 changed files with 922 additions and 1684 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
packages/cli/test/unit/PermissionChecker.test.ts
View File

@@ -4,7 +4,6 @@ import type { WorkflowSettings } from 'n8n-workflow';
import { SubworkflowOperationError, Workflow } from 'n8n-workflow';
import config from '@/config';
import type { Role } from '@db/entities/Role';
import { User } from '@db/entities/User';
import { WorkflowRepository } from '@db/repositories/workflow.repository';
import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.repository';
@@ -27,7 +26,6 @@ import * as testDb from '../integration/shared/testDb';
import type { SaveCredentialFunction } from '../integration/shared/types';
import { mockNodeTypesData } from './Helpers';
import { affixRoleToSaveCredential } from '../integration/shared/db/credentials';
import { getCredentialOwnerRole, getWorkflowOwnerRole } from '../integration/shared/db/roles';
import { createOwner, createUser } from '../integration/shared/db/users';
export const toTargetCallErrorMsg = (subworkflowId: string) =>
@@ -71,8 +69,6 @@ export function createSubworkflow({
});
}
let credentialOwnerRole: Role;
let workflowOwnerRole: Role;
let saveCredential: SaveCredentialFunction;
const mockNodeTypes = mockInstance(NodeTypes);
@@ -85,10 +81,7 @@ let permissionChecker: PermissionChecker;
beforeAll(async () => {
await testDb.init();
credentialOwnerRole = await getCredentialOwnerRole();
workflowOwnerRole = await getWorkflowOwnerRole();
saveCredential = affixRoleToSaveCredential(credentialOwnerRole);
saveCredential = affixRoleToSaveCredential('credential:owner');
permissionChecker = Container.get(PermissionChecker);
});
@@ -251,7 +244,7 @@ describe('check()', () => {
await Container.get(SharedWorkflowRepository).save({
workflow: workflowEntity,
user: member,
role: workflowOwnerRole,
role: 'workflow:owner',
});
const workflow = new Workflow(workflowDetails);

4
packages/cli/test/unit/WorkflowRunner.test.ts
View File

@@ -8,7 +8,6 @@ import config from '@/config';
import { mockInstance } from '../shared/mocking';
import * as testDb from '../integration/shared/testDb';
import { setupTestServer } from '../integration/shared/utils';
import { getGlobalOwnerRole } from '../integration/shared/db/roles';
import { createUser } from '../integration/shared/db/users';
import { createWorkflow } from '../integration/shared/db/workflows';
import { createExecution } from '../integration/shared/db/executions';
@@ -25,8 +24,7 @@ const watchers = new Watchers();
const watchedWorkflowExecuteAfter = jest.spyOn(watchers, 'workflowExecuteAfter');
beforeAll(async () => {
const globalOwnerRole = await getGlobalOwnerRole();
owner = await createUser({ globalRole: globalOwnerRole });
owner = await createUser({ role: 'global:owner' });
mockInstance(Push);
Container.set(Push, new Push());

12
packages/cli/test/unit/controllers/me.controller.test.ts
View File

@@ -44,7 +44,7 @@ describe('MeController', () => {
id: '123',
password: 'password',
authIdentities: [],
globalRoleId: '1',
role: 'global:owner',
});
const reqBody = { email: 'valid@email.com', firstName: 'John', lastName: 'Potato' };
const req = mock<MeRequest.UserUpdate>({ user, body: reqBody });
@@ -79,7 +79,7 @@ describe('MeController', () => {
id: '123',
password: 'password',
authIdentities: [],
globalRoleId: '1',
role: 'global:owner',
});
const reqBody = { email: 'valid@email.com', firstName: 'John', lastName: 'Potato' };
const req = mock<MeRequest.UserUpdate>({ user, body: reqBody });
@@ -88,7 +88,7 @@ describe('MeController', () => {
jest.spyOn(jwt, 'sign').mockImplementation(() => 'signed-token');
// Add invalid data to the request payload
Object.assign(reqBody, { id: '0', globalRoleId: '42' });
Object.assign(reqBody, { id: '0', role: '42' });
await controller.updateCurrentUser(req, res);
@@ -99,7 +99,7 @@ describe('MeController', () => {
expect(updatedUser.firstName).toBe(reqBody.firstName);
expect(updatedUser.lastName).toBe(reqBody.lastName);
expect(updatedUser.id).not.toBe('0');
expect(updatedUser.globalRoleId).not.toBe('42');
expect(updatedUser.role).not.toBe('42');
});
it('should throw BadRequestError if beforeUpdate hook throws BadRequestError', async () => {
@@ -107,11 +107,11 @@ describe('MeController', () => {
id: '123',
password: 'password',
authIdentities: [],
globalRoleId: '1',
role: 'global:owner',
});
const reqBody = { email: 'valid@email.com', firstName: 'John', lastName: 'Potato' };
const req = mock<MeRequest.UserUpdate>({ user, body: reqBody });
userService.findOneOrFail.mockResolvedValue(user);
// userService.findOneOrFail.mockResolvedValue(user);
externalHooks.run.mockImplementationOnce(async (hookName) => {
if (hookName === 'user.profile.beforeUpdate') {

2
packages/cli/test/unit/controllers/oAuth1Credential.controller.test.ts
View File

@@ -34,7 +34,7 @@ describe('OAuth1CredentialController', () => {
id: '123',
password: 'password',
authIdentities: [],
globalRoleId: '1',
role: 'global:owner',
});
const credential = mock<CredentialsEntity>({
id: '1',

2
packages/cli/test/unit/controllers/oAuth2Credential.controller.test.ts
View File

@@ -38,7 +38,7 @@ describe('OAuth2CredentialController', () => {
id: '123',
password: 'password',
authIdentities: [],
globalRoleId: '1',
role: 'global:owner',
});
const credential = mock<CredentialsEntity>({
id: '1',

2
packages/cli/test/unit/controllers/owner.controller.test.ts
View File

@@ -76,7 +76,7 @@ describe('OwnerController', () => {
it('should setup the instance owner successfully', async () => {
const user = mock<User>({
id: 'userId',
globalRole: { scope: 'global', name: 'owner' },
role: 'global:owner',
authIdentities: [],
});
const req = mock<OwnerRequest.Post>({

4
packages/cli/test/unit/controllers/translation.controller.test.ts
View File

@@ -1,5 +1,4 @@
import { mock } from 'jest-mock-extended';
import type { ICredentialTypes } from 'n8n-workflow';
import config from '@/config';
import type { TranslationRequest } from '@/controllers/translation.controller';
import {
@@ -7,10 +6,11 @@ import {
CREDENTIAL_TRANSLATIONS_DIR,
} from '@/controllers/translation.controller';
import { BadRequestError } from '@/errors/response-errors/bad-request.error';
import type { CredentialTypes } from '@/CredentialTypes';
describe('TranslationController', () => {
const configGetSpy = jest.spyOn(config, 'getEnv');
const credentialTypes = mock<ICredentialTypes>();
const credentialTypes = mock<CredentialTypes>();
const controller = new TranslationController(credentialTypes);
describe('getCredentialTranslation', () => {

34
packages/cli/test/unit/repositories/role.repository.test.ts
View File

@@ -1,34 +0,0 @@
import { Container } from 'typedi';
import { DataSource, EntityManager } from 'typeorm';
import { mock } from 'jest-mock-extended';
import type { RoleNames, RoleScopes } from '@db/entities/Role';
import { Role } from '@db/entities/Role';
import { RoleRepository } from '@db/repositories/role.repository';
import { mockInstance } from '../../shared/mocking';
import { randomInteger } from '../../integration/shared/random';
describe('RoleRepository', () => {
const entityManager = mockInstance(EntityManager);
const dataSource = mockInstance(DataSource, { manager: entityManager });
dataSource.getMetadata.mockReturnValue(mock());
Object.assign(entityManager, { connection: dataSource });
const roleRepository = Container.get(RoleRepository);
describe('findRole', () => {
test('should return the role when present', async () => {
entityManager.findOne.mockResolvedValueOnce(createRole('global', 'owner'));
const role = await roleRepository.findRole('global', 'owner');
expect(role?.name).toEqual('owner');
expect(role?.scope).toEqual('global');
});
test('should return null otherwise', async () => {
entityManager.findOne.mockResolvedValueOnce(null);
const role = await roleRepository.findRole('global', 'owner');
expect(role).toEqual(null);
});
});
const createRole = (scope: RoleScopes, name: RoleNames) =>
Object.assign(new Role(), { name, scope, id: `${randomInteger()}` });
});

228
packages/cli/test/unit/services/ownership.service.test.ts
View File

@@ -1,181 +1,135 @@
import { OwnershipService } from '@/services/ownership.service';
import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.repository';
import { Role } from '@db/entities/Role';
import { SharedWorkflow } from '@db/entities/SharedWorkflow';
import { User } from '@db/entities/User';
import { RoleService } from '@/services/role.service';
import type { SharedCredentials } from '@db/entities/SharedCredentials';
import { mockInstance } from '../../shared/mocking';
import { WorkflowEntity } from '@/databases/entities/WorkflowEntity';
import { UserRepository } from '@/databases/repositories/user.repository';
import { mock } from 'jest-mock-extended';
import {
mockCredRole,
mockCredential,
mockUser,
mockInstanceOwnerRole,
wfOwnerRole,
} from '../shared/mockObjects';
import { mockCredential, mockUser } from '../shared/mockObjects';
describe('OwnershipService', () => {
const roleService = mockInstance(RoleService);
const userRepository = mockInstance(UserRepository);
const sharedWorkflowRepository = mockInstance(SharedWorkflowRepository);
const ownershipService = new OwnershipService(
mock(),
userRepository,
roleService,
sharedWorkflowRepository,
);
const ownershipService = new OwnershipService(mock(), userRepository, sharedWorkflowRepository);
beforeEach(() => {
jest.clearAllMocks();
});
describe('OwnershipService', () => {
const roleService = mockInstance(RoleService);
const userRepository = mockInstance(UserRepository);
const sharedWorkflowRepository = mockInstance(SharedWorkflowRepository);
describe('getWorkflowOwner()', () => {
test('should retrieve a workflow owner', async () => {
const mockOwner = new User();
const mockNonOwner = new User();
const ownershipService = new OwnershipService(
mock(),
userRepository,
roleService,
sharedWorkflowRepository,
);
const sharedWorkflow = Object.assign(new SharedWorkflow(), {
role: 'workflow:owner',
user: mockOwner,
});
beforeEach(() => {
jest.clearAllMocks();
sharedWorkflowRepository.findOneOrFail.mockResolvedValueOnce(sharedWorkflow);
const returnedOwner = await ownershipService.getWorkflowOwnerCached('some-workflow-id');
expect(returnedOwner).toBe(mockOwner);
expect(returnedOwner).not.toBe(mockNonOwner);
});
describe('getWorkflowOwner()', () => {
test('should retrieve a workflow owner', async () => {
roleService.findWorkflowOwnerRole.mockResolvedValueOnce(wfOwnerRole());
test('should throw if no workflow owner found', async () => {
sharedWorkflowRepository.findOneOrFail.mockRejectedValue(new Error());
const mockOwner = new User();
const mockNonOwner = new User();
await expect(ownershipService.getWorkflowOwnerCached('some-workflow-id')).rejects.toThrow();
});
});
const sharedWorkflow = Object.assign(new SharedWorkflow(), {
role: new Role(),
user: mockOwner,
});
describe('addOwnedByAndSharedWith()', () => {
test('should add `ownedBy` and `sharedWith` to credential', async () => {
const owner = mockUser();
const editor = mockUser();
sharedWorkflowRepository.findOneOrFail.mockResolvedValueOnce(sharedWorkflow);
const credential = mockCredential();
const returnedOwner = await ownershipService.getWorkflowOwnerCached('some-workflow-id');
credential.shared = [
{ role: 'credential:owner', user: owner },
{ role: 'credential:editor', user: editor },
] as SharedCredentials[];
expect(returnedOwner).toBe(mockOwner);
expect(returnedOwner).not.toBe(mockNonOwner);
const { ownedBy, sharedWith } = ownershipService.addOwnedByAndSharedWith(credential);
expect(ownedBy).toStrictEqual({
id: owner.id,
email: owner.email,
firstName: owner.firstName,
lastName: owner.lastName,
});
test('should throw if no workflow owner role found', async () => {
roleService.findWorkflowOwnerRole.mockRejectedValueOnce(new Error());
await expect(ownershipService.getWorkflowOwnerCached('some-workflow-id')).rejects.toThrow();
});
test('should throw if no workflow owner found', async () => {
roleService.findWorkflowOwnerRole.mockResolvedValueOnce(wfOwnerRole());
sharedWorkflowRepository.findOneOrFail.mockRejectedValue(new Error());
await expect(ownershipService.getWorkflowOwnerCached('some-workflow-id')).rejects.toThrow();
});
expect(sharedWith).toStrictEqual([
{
id: editor.id,
email: editor.email,
firstName: editor.firstName,
lastName: editor.lastName,
},
]);
});
describe('addOwnedByAndSharedWith()', () => {
test('should add `ownedBy` and `sharedWith` to credential', async () => {
const owner = mockUser();
const editor = mockUser();
test('should add `ownedBy` and `sharedWith` to workflow', async () => {
const owner = mockUser();
const editor = mockUser();
const credential = mockCredential();
const workflow = new WorkflowEntity();
credential.shared = [
{ role: mockCredRole('owner'), user: owner },
{ role: mockCredRole('editor'), user: editor },
] as SharedCredentials[];
workflow.shared = [
{ role: 'workflow:owner', user: owner },
{ role: 'workflow:editor', user: editor },
] as SharedWorkflow[];
const { ownedBy, sharedWith } = ownershipService.addOwnedByAndSharedWith(credential);
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,
},
]);
expect(ownedBy).toStrictEqual({
id: owner.id,
email: owner.email,
firstName: owner.firstName,
lastName: owner.lastName,
});
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();
const credential = mockCredential();
credential.shared = [{ role: mockCredRole('owner'), user: owner }] as SharedCredentials[];
const { ownedBy, sharedWith } = ownershipService.addOwnedByAndSharedWith(credential);
expect(ownedBy).toStrictEqual({
id: owner.id,
email: owner.email,
firstName: owner.firstName,
lastName: owner.lastName,
});
expect(sharedWith).toHaveLength(0);
});
expect(sharedWith).toStrictEqual([
{
id: editor.id,
email: editor.email,
firstName: editor.firstName,
lastName: editor.lastName,
},
]);
});
describe('getInstanceOwner()', () => {
test('should find owner using global owner role ID', async () => {
const instanceOwnerRole = mockInstanceOwnerRole();
roleService.findGlobalOwnerRole.mockResolvedValue(instanceOwnerRole);
test('should produce an empty sharedWith if no sharee', async () => {
const owner = mockUser();
await ownershipService.getInstanceOwner();
const credential = mockCredential();
expect(userRepository.findOneOrFail).toHaveBeenCalledWith({
where: { globalRoleId: instanceOwnerRole.id },
relations: ['globalRole'],
});
credential.shared = [{ role: 'credential:owner', user: owner }] as SharedCredentials[];
const { ownedBy, sharedWith } = ownershipService.addOwnedByAndSharedWith(credential);
expect(ownedBy).toStrictEqual({
id: owner.id,
email: owner.email,
firstName: owner.firstName,
lastName: owner.lastName,
});
expect(sharedWith).toHaveLength(0);
});
});
describe('getInstanceOwner()', () => {
test('should find owner using global owner role ID', async () => {
await ownershipService.getInstanceOwner();
expect(userRepository.findOneOrFail).toHaveBeenCalledWith({
where: { role: 'global:owner' },
});
});
});

80
packages/cli/test/unit/services/role.service.test.ts
View File

@@ -1,80 +0,0 @@
import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.repository';
import type { RoleNames, RoleScopes } from '@db/entities/Role';
import { Role } from '@db/entities/Role';
import { RoleService } from '@/services/role.service';
import { RoleRepository } from '@db/repositories/role.repository';
import { CacheService } from '@/services/cache/cache.service';
import { SharedWorkflow } from '@db/entities/SharedWorkflow';
import { mockInstance } from '../../shared/mocking';
import { chooseRandomly } from '../../integration/shared/random';
import config from '@/config';
const ROLE_PROPS: Array<{ name: RoleNames; scope: RoleScopes }> = [
{ name: 'owner', scope: 'global' },
{ name: 'member', scope: 'global' },
{ name: 'owner', scope: 'workflow' },
{ name: 'owner', scope: 'credential' },
{ name: 'user', scope: 'credential' },
{ name: 'editor', scope: 'workflow' },
];
export const uppercaseInitial = (str: string) => str[0].toUpperCase() + str.slice(1);
describe('RoleService', () => {
const sharedWorkflowRepository = mockInstance(SharedWorkflowRepository);
const roleRepository = mockInstance(RoleRepository);
const cacheService = mockInstance(CacheService);
const roleService = new RoleService(roleRepository, sharedWorkflowRepository, cacheService);
const userId = '1';
const workflowId = '42';
const { name, scope } = chooseRandomly(ROLE_PROPS);
const display = {
name: uppercaseInitial(name),
scope: uppercaseInitial(scope),
};
beforeEach(() => {
config.load(config.default);
jest.clearAllMocks();
});
[true, false].forEach((cacheEnabled) => {
const tag = ['cache', cacheEnabled ? 'enabled' : 'disabled'].join(' ');
describe(`find${display.scope}${display.name}Role() [${tag}]`, () => {
test(`should return the ${scope} ${name} role if found`, async () => {
config.set('cache.enabled', cacheEnabled);
const role = roleRepository.create({ name, scope });
roleRepository.findRole.mockResolvedValueOnce(role);
const returnedRole = await roleRepository.findRole(scope, name);
expect(returnedRole).toBe(role);
});
});
describe(`findRoleByUserAndWorkflow() [${tag}]`, () => {
test('should return the role if a shared workflow is found', async () => {
config.set('cache.enabled', cacheEnabled);
const sharedWorkflow = Object.assign(new SharedWorkflow(), { role: new Role() });
sharedWorkflowRepository.findOne.mockResolvedValueOnce(sharedWorkflow);
const returnedRole = await roleService.findRoleByUserAndWorkflow(userId, workflowId);
expect(returnedRole).toBe(sharedWorkflow.role);
});
test('should return undefined if no shared workflow is found', async () => {
config.set('cache.enabled', cacheEnabled);
sharedWorkflowRepository.findOne.mockResolvedValueOnce(null);
const returnedRole = await roleService.findRoleByUserAndWorkflow(userId, workflowId);
expect(returnedRole).toBeUndefined();
});
});
});
});

2
packages/cli/test/unit/services/user.service.test.ts
View File

@@ -6,14 +6,12 @@ import { User } from '@db/entities/User';
import { UserRepository } from '@db/repositories/user.repository';
import { UserService } from '@/services/user.service';
import { mockInstance } from '../../shared/mocking';
import { RoleService } from '@/services/role.service';
import { v4 as uuid } from 'uuid';
describe('UserService', () => {
config.set('userManagement.jwtSecret', 'random-secret');
mockInstance(Logger);
mockInstance(RoleService);
const userRepository = mockInstance(UserRepository);
const userService = Container.get(UserService);

22
packages/cli/test/unit/shared/mockObjects.ts
View File

@@ -1,5 +1,4 @@
import { User } from '@db/entities/User';
import { Role } from '@db/entities/Role';
import { CredentialsEntity } from '@db/entities/CredentialsEntity';
import {
@@ -9,20 +8,6 @@ import {
randomName,
} from '../../integration/shared/random';
export const wfOwnerRole = () =>
Object.assign(new Role(), {
scope: 'workflow',
name: 'owner',
id: randomInteger(),
});
export const mockCredRole = (name: 'owner' | 'editor'): Role =>
Object.assign(new Role(), {
scope: 'credentials',
name,
id: randomInteger(),
});
export const mockCredential = (): CredentialsEntity =>
Object.assign(new CredentialsEntity(), randomCredentialPayload());
@@ -33,10 +18,3 @@ export const mockUser = (): User =>
firstName: randomName(),
lastName: randomName(),
});
export const mockInstanceOwnerRole = () =>
Object.assign(new Role(), {
scope: 'global',
name: 'owner',
id: randomInteger(),
});