Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(core): Delete duplicate code across all commands #5452

Merged
merged 2 commits into from
Feb 10, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions packages/cli/src/ActiveWorkflowRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ import { WorkflowRunner } from '@/WorkflowRunner';
import { ExternalHooks } from '@/ExternalHooks';
import { whereClause } from './UserManagement/UserManagementHelper';
import { WorkflowsService } from './workflows/workflows.services';
import { START_NODES } from './constants';

const WEBHOOK_PROD_UNREGISTERED_HINT =
"The workflow must be active for a production URL to run successfully. You can activate the workflow using the toggle in the top-right of the editor. Note that unlike test URL calls, production URL calls aren't shown on the canvas (only in the executions list)";
Expand Down Expand Up @@ -801,10 +802,7 @@ export class ActiveWorkflowRunner {
settings: workflowData.settings,
});

const canBeActivated = workflowInstance.checkIfWorkflowCanBeActivated([
'n8n-nodes-base.start',
'n8n-nodes-base.manualTrigger',
]);
const canBeActivated = workflowInstance.checkIfWorkflowCanBeActivated(START_NODES);
if (!canBeActivated) {
Logger.error(`Unable to activate workflow "${workflowData.name}"`);
throw new Error(
Expand Down
3 changes: 2 additions & 1 deletion packages/cli/src/LoadNodesAndCredentials.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
RESPONSE_ERROR_MESSAGES,
CUSTOM_API_CALL_KEY,
CUSTOM_API_CALL_NAME,
inTest,
} from '@/constants';
import {
persistInstalledPackageData,
Expand Down Expand Up @@ -61,7 +62,7 @@ export class LoadNodesAndCredentialsClass implements INodesAndCredentials {

// @ts-ignore
// eslint-disable-next-line @typescript-eslint/no-unsafe-call
module.constructor._initPaths();
if (!inTest) module.constructor._initPaths();

await this.loadNodesFromBasePackages();
await this.loadNodesFromDownloadedPackages();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { SharedWorkflow } from '@db/entities/SharedWorkflow';
import { isInstanceOwner } from '../users/users.service';
import type { Role } from '@db/entities/Role';
import config from '@/config';
import { START_NODES } from '@/constants';

function insertIf(condition: boolean, elements: string[]): string[] {
return condition ? elements : [];
Expand Down Expand Up @@ -128,7 +129,7 @@ export async function updateWorkflow(
export function hasStartNode(workflow: WorkflowEntity): boolean {
if (!workflow.nodes.length) return false;

const found = workflow.nodes.find((node) => node.type === 'n8n-nodes-base.start');
const found = workflow.nodes.find((node) => START_NODES.includes(node.type));

return Boolean(found);
}
Expand Down
3 changes: 2 additions & 1 deletion packages/cli/src/Queue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import config from '@/config';
import * as ActiveExecutions from '@/ActiveExecutions';
import * as WebhookHelpers from '@/WebhookHelpers';

export type JobId = Bull.JobId;
export type Job = Bull.Job<JobData>;
export type JobQueue = Bull.Queue<JobData>;

Expand Down Expand Up @@ -55,7 +56,7 @@ export class Queue {
return this.jobQueue.add(jobData, jobOptions);
}

async getJob(jobId: Bull.JobId): Promise<Job | null> {
async getJob(jobId: JobId): Promise<Job | null> {
return this.jobQueue.getJob(jobId);
}

Expand Down
6 changes: 3 additions & 3 deletions packages/cli/src/WorkflowRunnerProcess.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,20 +101,20 @@ class WorkflowRunnerProcess {

this.startedAt = new Date();

const userSettings = await UserSettings.prepareUserSettings();

const loadNodesAndCredentials = LoadNodesAndCredentials();
await loadNodesAndCredentials.init();

const nodeTypes = NodeTypes(loadNodesAndCredentials);
const credentialTypes = CredentialTypes(loadNodesAndCredentials);

// Load the credentials overwrites if any exist
CredentialsOverwrites(credentialTypes);

// Load all external hooks
const externalHooks = ExternalHooks();
await externalHooks.init();

const instanceId = (await UserSettings.prepareUserSettings()).instanceId ?? '';
const instanceId = userSettings.instanceId ?? '';
await InternalHooksManager.init(instanceId, nodeTypes);

const binaryDataConfig = config.getEnv('binaryDataManager');
Expand Down
107 changes: 73 additions & 34 deletions packages/cli/src/commands/BaseCommand.ts
Original file line number Diff line number Diff line change
@@ -1,59 +1,98 @@
import { Command } from '@oclif/core';
import { LoggerProxy } from 'n8n-workflow';
import { Command } from '@oclif/command';
import type { INodeTypes } from 'n8n-workflow';
import { LoggerProxy, ErrorReporterProxy as ErrorReporter, sleep } from 'n8n-workflow';
import type { IUserSettings } from 'n8n-core';
import { BinaryDataManager, UserSettings } from 'n8n-core';
import type { Logger } from '@/Logger';
import { getLogger } from '@/Logger';
import { User } from '@db/entities/User';
import config from '@/config';
import * as Db from '@/Db';
import * as CrashJournal from '@/CrashJournal';
import { inTest } from '@/constants';
import { CredentialTypes } from '@/CredentialTypes';
import { CredentialsOverwrites } from '@/CredentialsOverwrites';
import { InternalHooksManager } from '@/InternalHooksManager';
import { initErrorHandling } from '@/ErrorReporting';
import { ExternalHooks } from '@/ExternalHooks';
import { NodeTypes } from '@/NodeTypes';
import type { LoadNodesAndCredentialsClass } from '@/LoadNodesAndCredentials';
import { LoadNodesAndCredentials } from '@/LoadNodesAndCredentials';
import type { IExternalHooksClass } from '@/Interfaces';

export const UM_FIX_INSTRUCTION =
'Please fix the database by running ./packages/cli/bin/n8n user-management:reset';

export abstract class BaseCommand extends Command {
logger: Logger;
protected logger: Logger;

protected externalHooks: IExternalHooksClass;

protected loadNodesAndCredentials: LoadNodesAndCredentialsClass;

protected nodeTypes: INodeTypes;

/**
* Lifecycle methods
*/
protected userSettings: IUserSettings;

async init(): Promise<void> {
this.logger = getLogger();
LoggerProxy.init(this.logger);

await Db.init();
}
await initErrorHandling();

async finally(): Promise<void> {
if (inTest) return;
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
this.stopProcess = this.stopProcess.bind(this);

this.exit();
}
// Make sure the settings exist
this.userSettings = await UserSettings.prepareUserSettings();

this.loadNodesAndCredentials = LoadNodesAndCredentials();
await this.loadNodesAndCredentials.init();
this.nodeTypes = NodeTypes(this.loadNodesAndCredentials);
const credentialTypes = CredentialTypes(this.loadNodesAndCredentials);
CredentialsOverwrites(credentialTypes);

/**
* User Management utils
*/
await InternalHooksManager.init(this.userSettings.instanceId ?? '', this.nodeTypes);

defaultUserProps = {
firstName: null,
lastName: null,
email: null,
password: null,
resetPasswordToken: null,
};
await Db.init().catch(async (error: Error) =>
this.exitWithCrash('There was an error initializing DB', error),
);
}

async getInstanceOwner(): Promise<User> {
const globalRole = await Db.collections.Role.findOneByOrFail({
name: 'owner',
scope: 'global',
});
protected async stopProcess() {
// This needs to be overridden
}

const owner = await Db.collections.User.findOneBy({ globalRoleId: globalRole.id });
protected async initCrashJournal() {
await CrashJournal.init();
}

if (owner) return owner;
protected async exitSuccessFully() {
try {
await CrashJournal.cleanup();
} finally {
process.exit();
}
}

const user = new User();
protected async exitWithCrash(message: string, error: unknown) {
ErrorReporter.error(new Error(message, { cause: error }), { level: 'fatal' });
await sleep(2000);
process.exit(1);
}

Object.assign(user, { ...this.defaultUserProps, globalRole });
protected async initBinaryManager() {
const binaryDataConfig = config.getEnv('binaryDataManager');
await BinaryDataManager.init(binaryDataConfig, true);
}

await Db.collections.User.save(user);
protected async initExternalHooks() {
this.externalHooks = ExternalHooks();
await this.externalHooks.init();
}

return Db.collections.User.findOneByOrFail({ globalRoleId: globalRole.id });
async finally(error: Error | undefined) {
if (inTest || this.id === 'start') return;
if (Db.isInitialized) await Db.connection.destroy();
this.exit(error ? 1 : 0);
}
}
12 changes: 1 addition & 11 deletions packages/cli/src/commands/Interfaces.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ interface IResult {
};
executions: IExecutionResult[];
}

interface IExecutionResult {
workflowId: string;
workflowName: string;
Expand Down Expand Up @@ -53,14 +54,3 @@ declare module 'json-diff' {
}
export function diff(obj1: unknown, obj2: unknown, diffOptions: IDiffOptions): string;
}

type SmtpConfig = {
host: string;
port: number;
secure: boolean;
auth: {
user: string;
pass: string;
};
sender: string;
};
47 changes: 3 additions & 44 deletions packages/cli/src/commands/audit.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,12 @@
import Command, { flags } from '@oclif/command';
import { LoggerProxy } from 'n8n-workflow';
import { UserSettings } from 'n8n-core';
import type { Logger } from '@/Logger';
import { getLogger } from '@/Logger';
import { flags } from '@oclif/command';
import { audit } from '@/audit';
import { RISK_CATEGORIES } from '@/audit/constants';
import { CredentialTypes } from '@/CredentialTypes';
import { NodeTypes } from '@/NodeTypes';
import { LoadNodesAndCredentials } from '@/LoadNodesAndCredentials';
import { InternalHooksManager } from '@/InternalHooksManager';
import config from '@/config';
import * as Db from '@/Db';
import type { Risk } from '@/audit/types';
import { BaseCommand } from './BaseCommand';

export class SecurityAudit extends Command {
export class SecurityAudit extends BaseCommand {
static description = 'Generate a security audit report for this n8n instance';

static examples = [
Expand All @@ -35,11 +28,7 @@ export class SecurityAudit extends Command {
}),
};

logger: Logger;

async run() {
await this.init();

const { flags: auditFlags } = this.parse(SecurityAudit);

const categories =
Expand Down Expand Up @@ -70,38 +59,8 @@ export class SecurityAudit extends Command {
void InternalHooksManager.getInstance().onAuditGeneratedViaCli();
}

async init() {
await Db.init();

this.initLogger();

await this.initInternalHooksManager();
}

initLogger() {
this.logger = getLogger();
LoggerProxy.init(this.logger);
}

async initInternalHooksManager(): Promise<void> {
const loadNodesAndCredentials = LoadNodesAndCredentials();
await loadNodesAndCredentials.init();

const nodeTypes = NodeTypes(loadNodesAndCredentials);
CredentialTypes(loadNodesAndCredentials);

const instanceId = await UserSettings.getInstanceId();
await InternalHooksManager.init(instanceId, nodeTypes);
}

async catch(error: Error) {
this.logger.error('Failed to generate security audit');
this.logger.error(error.message);

this.exit(1);
}

async finally() {
this.exit();
}
}
Loading