mirror of
https://github.com/Abdulazizzn/n8n-enterprise-unlocked.git
synced 2025-12-18 02:21:13 +00:00
refactor(core): Move instance owner retrieval to ownership service (no-changelog) (#7980)
## Summary Provide details about your pull request and what it adds, fixes, or changes. Photos and videos are recommended. Continue breaking down `UserManagementHelper.ts` ... #### How to test the change: 1. ... ## Issues fixed Include links to Github issue or Community forum post or **Linear ticket**: > Important in order to close automatically and provide context to reviewers ... ## Review / Merge checklist - [ ] PR title and summary are descriptive. **Remember, the title automatically goes into the changelog. Use `(no-changelog)` otherwise.** ([conventions](https://github.com/n8n-io/n8n/blob/master/.github/pull_request_title_conventions.md)) - [ ] [Docs updated](https://github.com/n8n-io/n8n-docs) or follow-up ticket created. - [ ] Tests included. > A bug is not considered fixed, unless a test is added to prevent it from happening again. A feature is not complete without tests. > > *(internal)* You can use Slack commands to trigger [e2e tests](https://www.notion.so/n8n/How-to-use-Test-Instances-d65f49dfc51f441ea44367fb6f67eb0a?pvs=4#a39f9e5ba64a48b58a71d81c837e8227) or [deploy test instance](https://www.notion.so/n8n/How-to-use-Test-Instances-d65f49dfc51f441ea44367fb6f67eb0a?pvs=4#f6a177d32bde4b57ae2da0b8e454bfce) or [deploy early access version on Cloud](https://www.notion.so/n8n/Cloudbot-3dbe779836004972b7057bc989526998?pvs=4#fef2d36ab02247e1a0f65a74f6fb534e).
This commit is contained in:
@@ -8,7 +8,6 @@ import { MAX_PASSWORD_LENGTH, MIN_PASSWORD_LENGTH } from '@db/entities/User';
|
|||||||
import config from '@/config';
|
import config from '@/config';
|
||||||
import { License } from '@/License';
|
import { License } from '@/License';
|
||||||
import { getWebhookBaseUrl } from '@/WebhookHelpers';
|
import { getWebhookBaseUrl } from '@/WebhookHelpers';
|
||||||
import { RoleService } from '@/services/role.service';
|
|
||||||
import { UserRepository } from '@db/repositories/user.repository';
|
import { UserRepository } from '@db/repositories/user.repository';
|
||||||
import type { Scope } from '@n8n/permissions';
|
import type { Scope } from '@n8n/permissions';
|
||||||
import { BadRequestError } from '@/errors/response-errors/bad-request.error';
|
import { BadRequestError } from '@/errors/response-errors/bad-request.error';
|
||||||
@@ -18,17 +17,6 @@ export function isSharingEnabled(): boolean {
|
|||||||
return Container.get(License).isSharingEnabled();
|
return Container.get(License).isSharingEnabled();
|
||||||
}
|
}
|
||||||
|
|
||||||
export async function getInstanceOwner() {
|
|
||||||
const globalOwnerRole = await Container.get(RoleService).findGlobalOwnerRole();
|
|
||||||
|
|
||||||
return Container.get(UserRepository).findOneOrFail({
|
|
||||||
relations: ['globalRole'],
|
|
||||||
where: {
|
|
||||||
globalRoleId: globalOwnerRole.id,
|
|
||||||
},
|
|
||||||
});
|
|
||||||
}
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Return the n8n instance base URL without trailing slash.
|
* Return the n8n instance base URL without trailing slash.
|
||||||
*/
|
*/
|
||||||
|
|||||||
@@ -7,11 +7,11 @@ import { ApplicationError, ExecutionBaseError } from 'n8n-workflow';
|
|||||||
import { ActiveExecutions } from '@/ActiveExecutions';
|
import { ActiveExecutions } from '@/ActiveExecutions';
|
||||||
import { WorkflowRunner } from '@/WorkflowRunner';
|
import { WorkflowRunner } from '@/WorkflowRunner';
|
||||||
import type { IWorkflowExecutionDataProcess } from '@/Interfaces';
|
import type { IWorkflowExecutionDataProcess } from '@/Interfaces';
|
||||||
import { getInstanceOwner } from '@/UserManagement/UserManagementHelper';
|
|
||||||
import { findCliWorkflowStart, isWorkflowIdValid } from '@/utils';
|
import { findCliWorkflowStart, isWorkflowIdValid } from '@/utils';
|
||||||
import { BaseCommand } from './BaseCommand';
|
import { BaseCommand } from './BaseCommand';
|
||||||
import { Container } from 'typedi';
|
import { Container } from 'typedi';
|
||||||
import { WorkflowRepository } from '@db/repositories/workflow.repository';
|
import { WorkflowRepository } from '@db/repositories/workflow.repository';
|
||||||
|
import { OwnershipService } from '@/services/ownership.service';
|
||||||
|
|
||||||
export class Execute extends BaseCommand {
|
export class Execute extends BaseCommand {
|
||||||
static description = '\nExecutes a given workflow';
|
static description = '\nExecutes a given workflow';
|
||||||
@@ -98,7 +98,7 @@ export class Execute extends BaseCommand {
|
|||||||
|
|
||||||
const startingNode = findCliWorkflowStart(workflowData.nodes);
|
const startingNode = findCliWorkflowStart(workflowData.nodes);
|
||||||
|
|
||||||
const user = await getInstanceOwner();
|
const user = await Container.get(OwnershipService).getInstanceOwner();
|
||||||
const runData: IWorkflowExecutionDataProcess = {
|
const runData: IWorkflowExecutionDataProcess = {
|
||||||
executionMode: 'cli',
|
executionMode: 'cli',
|
||||||
startNodes: [startingNode.name],
|
startNodes: [startingNode.name],
|
||||||
|
|||||||
@@ -12,7 +12,6 @@ import { ActiveExecutions } from '@/ActiveExecutions';
|
|||||||
import { WorkflowRunner } from '@/WorkflowRunner';
|
import { WorkflowRunner } from '@/WorkflowRunner';
|
||||||
import type { IWorkflowDb, IWorkflowExecutionDataProcess } from '@/Interfaces';
|
import type { IWorkflowDb, IWorkflowExecutionDataProcess } from '@/Interfaces';
|
||||||
import type { User } from '@db/entities/User';
|
import type { User } from '@db/entities/User';
|
||||||
import { getInstanceOwner } from '@/UserManagement/UserManagementHelper';
|
|
||||||
import { findCliWorkflowStart } from '@/utils';
|
import { findCliWorkflowStart } from '@/utils';
|
||||||
import { BaseCommand } from './BaseCommand';
|
import { BaseCommand } from './BaseCommand';
|
||||||
import { Container } from 'typedi';
|
import { Container } from 'typedi';
|
||||||
@@ -24,6 +23,7 @@ import type {
|
|||||||
IWorkflowExecutionProgress,
|
IWorkflowExecutionProgress,
|
||||||
} from '../types/commands.types';
|
} from '../types/commands.types';
|
||||||
import { WorkflowRepository } from '@db/repositories/workflow.repository';
|
import { WorkflowRepository } from '@db/repositories/workflow.repository';
|
||||||
|
import { OwnershipService } from '@/services/ownership.service';
|
||||||
|
|
||||||
const re = /\d+/;
|
const re = /\d+/;
|
||||||
|
|
||||||
@@ -276,7 +276,7 @@ export class ExecuteBatch extends BaseCommand {
|
|||||||
ExecuteBatch.githubWorkflow = true;
|
ExecuteBatch.githubWorkflow = true;
|
||||||
}
|
}
|
||||||
|
|
||||||
ExecuteBatch.instanceOwner = await getInstanceOwner();
|
ExecuteBatch.instanceOwner = await Container.get(OwnershipService).getInstanceOwner();
|
||||||
|
|
||||||
const query = Container.get(WorkflowRepository).createQueryBuilder('workflows');
|
const query = Container.get(WorkflowRepository).createQueryBuilder('workflows');
|
||||||
|
|
||||||
|
|||||||
@@ -20,9 +20,9 @@ import {
|
|||||||
} from './constants';
|
} from './constants';
|
||||||
import { sourceControlFoldersExistCheck } from './sourceControlHelper.ee';
|
import { sourceControlFoldersExistCheck } from './sourceControlHelper.ee';
|
||||||
import type { User } from '@db/entities/User';
|
import type { User } from '@db/entities/User';
|
||||||
import { getInstanceOwner } from '../../UserManagement/UserManagementHelper';
|
|
||||||
import { Logger } from '@/Logger';
|
import { Logger } from '@/Logger';
|
||||||
import { ApplicationError } from 'n8n-workflow';
|
import { ApplicationError } from 'n8n-workflow';
|
||||||
|
import { OwnershipService } from '@/services/ownership.service';
|
||||||
|
|
||||||
@Service()
|
@Service()
|
||||||
export class SourceControlGitService {
|
export class SourceControlGitService {
|
||||||
@@ -30,7 +30,10 @@ export class SourceControlGitService {
|
|||||||
|
|
||||||
private gitOptions: Partial<SimpleGitOptions> = {};
|
private gitOptions: Partial<SimpleGitOptions> = {};
|
||||||
|
|
||||||
constructor(private readonly logger: Logger) {}
|
constructor(
|
||||||
|
private readonly logger: Logger,
|
||||||
|
private readonly ownershipService: OwnershipService,
|
||||||
|
) {}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Run pre-checks before initialising git
|
* Run pre-checks before initialising git
|
||||||
@@ -103,8 +106,8 @@ export class SourceControlGitService {
|
|||||||
}
|
}
|
||||||
if (!(await this.hasRemote(sourceControlPreferences.repositoryUrl))) {
|
if (!(await this.hasRemote(sourceControlPreferences.repositoryUrl))) {
|
||||||
if (sourceControlPreferences.connected && sourceControlPreferences.repositoryUrl) {
|
if (sourceControlPreferences.connected && sourceControlPreferences.repositoryUrl) {
|
||||||
const user = await getInstanceOwner();
|
const instanceOwner = await this.ownershipService.getInstanceOwner();
|
||||||
await this.initRepository(sourceControlPreferences, user);
|
await this.initRepository(sourceControlPreferences, instanceOwner);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -3,7 +3,7 @@ import { CacheService } from './cache.service';
|
|||||||
import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.repository';
|
import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.repository';
|
||||||
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 { UserRepository } from '@/databases/repositories/user.repository';
|
||||||
import type { ListQuery } from '@/requests';
|
import type { ListQuery } from '@/requests';
|
||||||
import { ApplicationError } from 'n8n-workflow';
|
import { ApplicationError } from 'n8n-workflow';
|
||||||
|
|
||||||
@@ -11,7 +11,7 @@ import { ApplicationError } from 'n8n-workflow';
|
|||||||
export class OwnershipService {
|
export class OwnershipService {
|
||||||
constructor(
|
constructor(
|
||||||
private cacheService: CacheService,
|
private cacheService: CacheService,
|
||||||
private userService: UserService,
|
private userRepository: UserRepository,
|
||||||
private roleService: RoleService,
|
private roleService: RoleService,
|
||||||
private sharedWorkflowRepository: SharedWorkflowRepository,
|
private sharedWorkflowRepository: SharedWorkflowRepository,
|
||||||
) {}
|
) {}
|
||||||
@@ -20,9 +20,9 @@ export class OwnershipService {
|
|||||||
* Retrieve the user who owns the workflow. Note that workflow ownership is **immutable**.
|
* Retrieve the user who owns the workflow. Note that workflow ownership is **immutable**.
|
||||||
*/
|
*/
|
||||||
async getWorkflowOwnerCached(workflowId: string) {
|
async getWorkflowOwnerCached(workflowId: string) {
|
||||||
const cachedValue = (await this.cacheService.get(`cache:workflow-owner:${workflowId}`)) as User;
|
const cachedValue = await this.cacheService.get<User>(`cache:workflow-owner:${workflowId}`);
|
||||||
|
|
||||||
if (cachedValue) return this.userService.create(cachedValue);
|
if (cachedValue) return this.userRepository.create(cachedValue);
|
||||||
|
|
||||||
const workflowOwnerRole = await this.roleService.findWorkflowOwnerRole();
|
const workflowOwnerRole = await this.roleService.findWorkflowOwnerRole();
|
||||||
|
|
||||||
@@ -67,4 +67,13 @@ export class OwnershipService {
|
|||||||
|
|
||||||
return entity;
|
return entity;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
async getInstanceOwner() {
|
||||||
|
const globalOwnerRole = await this.roleService.findGlobalOwnerRole();
|
||||||
|
|
||||||
|
return this.userRepository.findOneOrFail({
|
||||||
|
where: { globalRoleId: globalOwnerRole.id },
|
||||||
|
relations: ['globalRole'],
|
||||||
|
});
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -2,10 +2,8 @@ import { OwnershipService } from '@/services/ownership.service';
|
|||||||
import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.repository';
|
import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.repository';
|
||||||
import { Role } from '@db/entities/Role';
|
import { Role } from '@db/entities/Role';
|
||||||
import { SharedWorkflow } from '@db/entities/SharedWorkflow';
|
import { SharedWorkflow } from '@db/entities/SharedWorkflow';
|
||||||
import { CacheService } from '@/services/cache.service';
|
|
||||||
import { User } from '@db/entities/User';
|
import { User } from '@db/entities/User';
|
||||||
import { RoleService } from '@/services/role.service';
|
import { RoleService } from '@/services/role.service';
|
||||||
import { UserService } from '@/services/user.service';
|
|
||||||
import { CredentialsEntity } from '@db/entities/CredentialsEntity';
|
import { CredentialsEntity } from '@db/entities/CredentialsEntity';
|
||||||
import type { SharedCredentials } from '@db/entities/SharedCredentials';
|
import type { SharedCredentials } from '@db/entities/SharedCredentials';
|
||||||
import { mockInstance } from '../../shared/mocking';
|
import { mockInstance } from '../../shared/mocking';
|
||||||
@@ -16,6 +14,8 @@ import {
|
|||||||
randomName,
|
randomName,
|
||||||
} from '../../integration/shared/random';
|
} from '../../integration/shared/random';
|
||||||
import { WorkflowEntity } from '@/databases/entities/WorkflowEntity';
|
import { WorkflowEntity } from '@/databases/entities/WorkflowEntity';
|
||||||
|
import { UserRepository } from '@/databases/repositories/user.repository';
|
||||||
|
import { mock } from 'jest-mock-extended';
|
||||||
|
|
||||||
const wfOwnerRole = () =>
|
const wfOwnerRole = () =>
|
||||||
Object.assign(new Role(), {
|
Object.assign(new Role(), {
|
||||||
@@ -31,26 +31,33 @@ const mockCredRole = (name: 'owner' | 'editor'): Role =>
|
|||||||
id: randomInteger(),
|
id: randomInteger(),
|
||||||
});
|
});
|
||||||
|
|
||||||
|
const mockInstanceOwnerRole = () =>
|
||||||
|
Object.assign(new Role(), {
|
||||||
|
scope: 'global',
|
||||||
|
name: 'owner',
|
||||||
|
id: randomInteger(),
|
||||||
|
});
|
||||||
|
|
||||||
const mockCredential = (): CredentialsEntity =>
|
const mockCredential = (): CredentialsEntity =>
|
||||||
Object.assign(new CredentialsEntity(), randomCredentialPayload());
|
Object.assign(new CredentialsEntity(), randomCredentialPayload());
|
||||||
|
|
||||||
const mockUser = (): User =>
|
const mockUser = (attributes?: Partial<User>): User =>
|
||||||
Object.assign(new User(), {
|
Object.assign(new User(), {
|
||||||
id: randomInteger(),
|
id: randomInteger(),
|
||||||
email: randomEmail(),
|
email: randomEmail(),
|
||||||
firstName: randomName(),
|
firstName: randomName(),
|
||||||
lastName: randomName(),
|
lastName: randomName(),
|
||||||
|
...attributes,
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('OwnershipService', () => {
|
describe('OwnershipService', () => {
|
||||||
const cacheService = mockInstance(CacheService);
|
|
||||||
const roleService = mockInstance(RoleService);
|
const roleService = mockInstance(RoleService);
|
||||||
const userService = mockInstance(UserService);
|
const userRepository = mockInstance(UserRepository);
|
||||||
const sharedWorkflowRepository = mockInstance(SharedWorkflowRepository);
|
const sharedWorkflowRepository = mockInstance(SharedWorkflowRepository);
|
||||||
|
|
||||||
const ownershipService = new OwnershipService(
|
const ownershipService = new OwnershipService(
|
||||||
cacheService,
|
mock(),
|
||||||
userService,
|
userRepository,
|
||||||
roleService,
|
roleService,
|
||||||
sharedWorkflowRepository,
|
sharedWorkflowRepository,
|
||||||
);
|
);
|
||||||
@@ -174,4 +181,18 @@ describe('OwnershipService', () => {
|
|||||||
expect(sharedWith).toHaveLength(0);
|
expect(sharedWith).toHaveLength(0);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('getInstanceOwner()', () => {
|
||||||
|
test('should find owner using global owner role ID', async () => {
|
||||||
|
const instanceOwnerRole = mockInstanceOwnerRole();
|
||||||
|
roleService.findGlobalOwnerRole.mockResolvedValue(instanceOwnerRole);
|
||||||
|
|
||||||
|
await ownershipService.getInstanceOwner();
|
||||||
|
|
||||||
|
expect(userRepository.findOneOrFail).toHaveBeenCalledWith({
|
||||||
|
where: { globalRoleId: instanceOwnerRole.id },
|
||||||
|
relations: ['globalRole'],
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user