Skip to content

Commit

Permalink
refactor(core): Move all base URLs to UrlService (no-changelog) (#8141)
Browse files Browse the repository at this point in the history
This change kept coming up in #6713, #7773, and #8135. 
So this PR moves the existing code without actually changing anything,
to help get rid of some of the circular dependencies.


## Review / Merge checklist
- [x] PR title and summary are descriptive.
  • Loading branch information
netroy authored Dec 22, 2023
1 parent 517b050 commit baee47a
Show file tree
Hide file tree
Showing 16 changed files with 93 additions and 85 deletions.
16 changes: 0 additions & 16 deletions packages/cli/src/GenericHelpers.ts
Original file line number Diff line number Diff line change
@@ -1,28 +1,12 @@
import type express from 'express';
import { validate } from 'class-validator';
import config from '@/config';
import type { WorkflowEntity } from '@db/entities/WorkflowEntity';
import type { CredentialsEntity } from '@db/entities/CredentialsEntity';
import type { TagEntity } from '@db/entities/TagEntity';
import type { User } from '@db/entities/User';
import type { UserUpdatePayload } from '@/requests';
import { BadRequestError } from './errors/response-errors/bad-request.error';

/**
* Returns the base URL n8n is reachable from
*/
export function getBaseUrl(): string {
const protocol = config.getEnv('protocol');
const host = config.getEnv('host');
const port = config.getEnv('port');
const path = config.getEnv('path');

if ((protocol === 'http' && port === 80) || (protocol === 'https' && port === 443)) {
return `${protocol}://${host}${path}`;
}
return `${protocol}://${host}:${port}${path}`;
}

/**
* Returns the session id if one is set
*/
Expand Down
7 changes: 4 additions & 3 deletions packages/cli/src/PublicApi/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/* eslint-disable @typescript-eslint/naming-convention */
import { Container } from 'typedi';
import type { Router } from 'express';
import express from 'express';
import fs from 'fs/promises';
Expand All @@ -11,11 +12,11 @@ import type { OpenAPIV3 } from 'openapi-types';
import type { JsonObject } from 'swagger-ui-express';

import config from '@/config';
import { getInstanceBaseUrl } from '@/UserManagement/UserManagementHelper';
import { Container } from 'typedi';

import { InternalHooks } from '@/InternalHooks';
import { License } from '@/License';
import { UserRepository } from '@db/repositories/user.repository';
import { UrlService } from '@/services/url.service';

async function createApiRouter(
version: string,
Expand All @@ -29,7 +30,7 @@ async function createApiRouter(
// from the Swagger UI
swaggerDocument.server = [
{
url: `${getInstanceBaseUrl()}/${publicApiEndpoint}/${version}}`,
url: `${Container.get(UrlService).getInstanceBaseUrl()}/${publicApiEndpoint}/${version}}`,
},
];
const apiController = express.Router();
Expand Down
17 changes: 1 addition & 16 deletions packages/cli/src/UserManagement/UserManagementHelper.ts
Original file line number Diff line number Diff line change
@@ -1,30 +1,15 @@
import { In } from 'typeorm';
import { Container } from 'typedi';
import type { Scope } from '@n8n/permissions';

import type { WhereClause } from '@/Interfaces';
import type { User } from '@db/entities/User';
import config from '@/config';
import { License } from '@/License';
import { getWebhookBaseUrl } from '@/WebhookHelpers';
import type { Scope } from '@n8n/permissions';

export function isSharingEnabled(): boolean {
return Container.get(License).isSharingEnabled();
}

/**
* Return the n8n instance base URL without trailing slash.
*/
export function getInstanceBaseUrl(): string {
const n8nBaseUrl = config.getEnv('editorBaseUrl') || getWebhookBaseUrl();

return n8nBaseUrl.endsWith('/') ? n8nBaseUrl.slice(0, n8nBaseUrl.length - 1) : n8nBaseUrl;
}

export function generateUserInviteUrl(inviterId: string, inviteeId: string): string {
return `${getInstanceBaseUrl()}/signup?inviterId=${inviterId}&inviteeId=${inviteeId}`;
}

// return the difference between two arrays
export function rightDiff<T1, T2>(
[arr1, keyExtractor1]: [T1[], (item: T1) => string],
Expand Down
12 changes: 0 additions & 12 deletions packages/cli/src/WebhookHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ import type {
WebhookCORSRequest,
WebhookRequest,
} from '@/Interfaces';
import * as GenericHelpers from '@/GenericHelpers';
import * as ResponseHelper from '@/ResponseHelper';
import * as WorkflowHelpers from '@/WorkflowHelpers';
import { WorkflowRunner } from '@/WorkflowRunner';
Expand Down Expand Up @@ -820,14 +819,3 @@ export async function executeWebhook(
return;
}
}

/**
* Returns the base URL of the webhooks
*/
export function getWebhookBaseUrl() {
let urlBaseWebhook = process.env.WEBHOOK_URL ?? GenericHelpers.getBaseUrl();
if (!urlBaseWebhook.endsWith('/')) {
urlBaseWebhook += '/';
}
return urlBaseWebhook;
}
6 changes: 3 additions & 3 deletions packages/cli/src/WorkflowExecuteAdditionalData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ import type {
} from '@/Interfaces';
import { NodeTypes } from '@/NodeTypes';
import { Push } from '@/push';
import * as WebhookHelpers from '@/WebhookHelpers';
import * as WorkflowHelpers from '@/WorkflowHelpers';
import { findSubworkflowStart, isWorkflowIdValid } from '@/utils';
import { PermissionChecker } from './UserManagement/PermissionChecker';
Expand All @@ -68,6 +67,7 @@ import { Logger } from './Logger';
import { saveExecutionProgress } from './executionLifecycleHooks/saveExecutionProgress';
import { WorkflowStaticDataService } from './workflows/workflowStaticData.service';
import { WorkflowRepository } from './databases/repositories/workflow.repository';
import { UrlService } from './services/url.service';

const ERROR_TRIGGER_TYPE = config.getEnv('nodes.errorTriggerType');

Expand Down Expand Up @@ -133,7 +133,7 @@ export function executeErrorWorkflow(
// Check if there was an error and if so if an errorWorkflow or a trigger is set
let pastExecutionUrl: string | undefined;
if (executionId !== undefined) {
pastExecutionUrl = `${WebhookHelpers.getWebhookBaseUrl()}workflow/${
pastExecutionUrl = `${Container.get(UrlService).getWebhookBaseUrl()}workflow/${
workflowData.id
}/executions/${executionId}`;
}
Expand Down Expand Up @@ -965,7 +965,7 @@ export async function getBase(
currentNodeParameters?: INodeParameters,
executionTimeoutTimestamp?: number,
): Promise<IWorkflowExecuteAdditionalData> {
const urlBaseWebhook = WebhookHelpers.getWebhookBaseUrl();
const urlBaseWebhook = Container.get(UrlService).getWebhookBaseUrl();

const formWaitingBaseUrl = urlBaseWebhook + config.getEnv('endpoints.formWaiting');

Expand Down
6 changes: 3 additions & 3 deletions packages/cli/src/commands/start.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import config from '@/config';

import { ActiveExecutions } from '@/ActiveExecutions';
import { ActiveWorkflowRunner } from '@/ActiveWorkflowRunner';
import * as GenericHelpers from '@/GenericHelpers';
import { Server } from '@/Server';
import { EDITOR_UI_DIST_DIR, LICENSE_FEATURES } from '@/constants';
import { eventBus } from '@/eventbus';
Expand All @@ -27,6 +26,7 @@ import { SingleMainSetup } from '@/services/orchestration/main/SingleMainSetup';
import { OrchestrationHandlerMainService } from '@/services/orchestration/main/orchestration.handler.main.service';
import { PruningService } from '@/services/pruning.service';
import { MultiMainSetup } from '@/services/orchestration/main/MultiMainSetup.ee';
import { UrlService } from '@/services/url.service';
import { SettingsRepository } from '@db/repositories/settings.repository';
import { ExecutionRepository } from '@db/repositories/execution.repository';
import { FeatureNotLicensedError } from '@/errors/feature-not-licensed.error';
Expand Down Expand Up @@ -77,7 +77,7 @@ export class Start extends BaseCommand {
* Opens the UI in browser
*/
private openBrowser() {
const editorUrl = GenericHelpers.getBaseUrl();
const editorUrl = Container.get(UrlService).baseUrl;

// eslint-disable-next-line @typescript-eslint/no-unused-vars
open(editorUrl, { wait: true }).catch((error: Error) => {
Expand Down Expand Up @@ -321,7 +321,7 @@ export class Start extends BaseCommand {
// Start to get active workflows and run their triggers
await this.activeWorkflowRunner.init();

const editorUrl = GenericHelpers.getBaseUrl();
const editorUrl = Container.get(UrlService).baseUrl;
this.log(`\nEditor is now accessible via:\n${editorUrl}`);

// Allow to open n8n editor by pressing "o"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@ import type { User } from '@db/entities/User';
import { CredentialsRepository } from '@db/repositories/credentials.repository';
import { SharedCredentialsRepository } from '@db/repositories/sharedCredentials.repository';
import type { ICredentialsDb } from '@/Interfaces';
import { getInstanceBaseUrl } from '@/UserManagement/UserManagementHelper';
import type { OAuthRequest } from '@/requests';
import { RESPONSE_ERROR_MESSAGES } from '@/constants';
import { CredentialsHelper } from '@/CredentialsHelper';
import * as WorkflowExecuteAdditionalData from '@/WorkflowExecuteAdditionalData';
import { Logger } from '@/Logger';
import { ExternalHooks } from '@/ExternalHooks';
import { UrlService } from '@/services/url.service';
import { BadRequestError } from '@/errors/response-errors/bad-request.error';
import { NotFoundError } from '@/errors/response-errors/not-found.error';

Expand All @@ -27,10 +27,11 @@ export abstract class AbstractOAuthController {
private readonly credentialsHelper: CredentialsHelper,
private readonly credentialsRepository: CredentialsRepository,
private readonly sharedCredentialsRepository: SharedCredentialsRepository,
private readonly urlService: UrlService,
) {}

get baseUrl() {
const restUrl = `${getInstanceBaseUrl()}/${config.getEnv('endpoints.rest')}`;
const restUrl = `${this.urlService.getInstanceBaseUrl()}/${config.getEnv('endpoints.rest')}`;
return `${restUrl}/oauth${this.oauthVersion}-credential`;
}

Expand Down
5 changes: 3 additions & 2 deletions packages/cli/src/controllers/passwordReset.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import { IsNull, Not } from 'typeorm';
import validator from 'validator';

import { Get, Post, RestController } from '@/decorators';
import { getInstanceBaseUrl } from '@/UserManagement/UserManagementHelper';
import { PasswordUtility } from '@/services/password.utility';
import { UserManagementMailer } from '@/UserManagement/email';
import { PasswordResetRequest } from '@/requests';
Expand All @@ -19,6 +18,7 @@ import { MfaService } from '@/Mfa/mfa.service';
import { Logger } from '@/Logger';
import { ExternalHooks } from '@/ExternalHooks';
import { InternalHooks } from '@/InternalHooks';
import { UrlService } from '@/services/url.service';
import { InternalServerError } from '@/errors/response-errors/internal-server.error';
import { BadRequestError } from '@/errors/response-errors/bad-request.error';
import { UnauthorizedError } from '@/errors/response-errors/unauthorized.error';
Expand All @@ -41,6 +41,7 @@ export class PasswordResetController {
private readonly mailer: UserManagementMailer,
private readonly userService: UserService,
private readonly mfaService: MfaService,
private readonly urlService: UrlService,
private readonly license: License,
private readonly passwordUtility: PasswordUtility,
) {}
Expand Down Expand Up @@ -130,7 +131,7 @@ export class PasswordResetController {
firstName,
lastName,
passwordResetUrl: url,
domain: getInstanceBaseUrl(),
domain: this.urlService.getInstanceBaseUrl(),
});
} catch (error) {
void this.internalHooks.onEmailFailed({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { Container, Service } from 'typedi';
import type { Variables } from '@db/entities/Variables';
import { InternalHooks } from '@/InternalHooks';
import { generateNanoId } from '@db/utils/generators';
import { canCreateNewVariable } from './enviromentHelpers';
import { canCreateNewVariable } from './environmentHelpers';
import { CacheService } from '@/services/cache.service';
import { VariablesRepository } from '@db/repositories/variables.repository';
import type { DeepPartial } from 'typeorm';
Expand Down
16 changes: 8 additions & 8 deletions packages/cli/src/services/frontend.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,25 +12,24 @@ import type {
} from 'n8n-workflow';
import { InstanceSettings } from 'n8n-core';

import config from '@/config';
import { LICENSE_FEATURES } from '@/constants';
import { CredentialsOverwrites } from '@/CredentialsOverwrites';
import { CredentialTypes } from '@/CredentialTypes';
import { LoadNodesAndCredentials } from '@/LoadNodesAndCredentials';
import { License } from '@/License';
import { getInstanceBaseUrl } from '@/UserManagement/UserManagementHelper';
import * as WebhookHelpers from '@/WebhookHelpers';
import config from '@/config';
import { getCurrentAuthenticationMethod } from '@/sso/ssoHelpers';
import { getLdapLoginLabel } from '@/Ldap/helpers';
import { getSamlLoginLabel } from '@/sso/saml/samlHelpers';
import { getVariablesLimit } from '@/environments/variables/enviromentHelpers';
import { getVariablesLimit } from '@/environments/variables/environmentHelpers';
import {
getWorkflowHistoryLicensePruneTime,
getWorkflowHistoryPruneTime,
} from '@/workflows/workflowHistory/workflowHistoryHelper.ee';
import { UserManagementMailer } from '@/UserManagement/email';
import type { CommunityPackagesService } from '@/services/communityPackages.service';
import { Logger } from '@/Logger';
import { UrlService } from './url.service';

@Service()
export class FrontendService {
Expand All @@ -46,6 +45,7 @@ export class FrontendService {
private readonly license: License,
private readonly mailer: UserManagementMailer,
private readonly instanceSettings: InstanceSettings,
private readonly urlService: UrlService,
) {
loadNodesAndCredentials.addPostProcessor(async () => this.generateTypes());
void this.generateTypes();
Expand All @@ -61,7 +61,7 @@ export class FrontendService {
}

private initSettings() {
const instanceBaseUrl = getInstanceBaseUrl();
const instanceBaseUrl = this.urlService.getInstanceBaseUrl();
const restEndpoint = config.getEnv('endpoints.rest');

const telemetrySettings: ITelemetrySettings = {
Expand Down Expand Up @@ -93,7 +93,7 @@ export class FrontendService {
maxExecutionTimeout: config.getEnv('executions.maxTimeout'),
workflowCallerPolicyDefaultOption: config.getEnv('workflows.callerPolicyDefaultOption'),
timezone: config.getEnv('generic.timezone'),
urlBaseWebhook: WebhookHelpers.getWebhookBaseUrl(),
urlBaseWebhook: this.urlService.getWebhookBaseUrl(),
urlBaseEditor: instanceBaseUrl,
versionCli: '',
releaseChannel: config.getEnv('generic.releaseChannel'),
Expand Down Expand Up @@ -222,8 +222,8 @@ export class FrontendService {
const restEndpoint = config.getEnv('endpoints.rest');

// Update all urls, in case `WEBHOOK_URL` was updated by `--tunnel`
const instanceBaseUrl = getInstanceBaseUrl();
this.settings.urlBaseWebhook = WebhookHelpers.getWebhookBaseUrl();
const instanceBaseUrl = this.urlService.getInstanceBaseUrl();
this.settings.urlBaseWebhook = this.urlService.getWebhookBaseUrl();
this.settings.urlBaseEditor = instanceBaseUrl;
this.settings.oauthCallbackUrls = {
oauth1: `${instanceBaseUrl}/${restEndpoint}/oauth1-credential/callback`,
Expand Down
40 changes: 40 additions & 0 deletions packages/cli/src/services/url.service.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { Service } from 'typedi';
import config from '@/config';

@Service()
export class UrlService {
/** Returns the base URL n8n is reachable from */
readonly baseUrl: string;

constructor() {
this.baseUrl = this.generateBaseUrl();
}

/** Returns the base URL of the webhooks */
getWebhookBaseUrl() {
let urlBaseWebhook = process.env.WEBHOOK_URL ?? this.baseUrl;
if (!urlBaseWebhook.endsWith('/')) {
urlBaseWebhook += '/';
}
return urlBaseWebhook;
}

/** Return the n8n instance base URL without trailing slash */
getInstanceBaseUrl(): string {
const n8nBaseUrl = config.getEnv('editorBaseUrl') || this.getWebhookBaseUrl();

return n8nBaseUrl.endsWith('/') ? n8nBaseUrl.slice(0, n8nBaseUrl.length - 1) : n8nBaseUrl;
}

private generateBaseUrl(): string {
const protocol = config.getEnv('protocol');
const host = config.getEnv('host');
const port = config.getEnv('port');
const path = config.getEnv('path');

if ((protocol === 'http' && port === 80) || (protocol === 'https' && port === 443)) {
return `${protocol}://${host}${path}`;
}
return `${protocol}://${host}:${port}${path}`;
}
}
Loading

0 comments on commit baee47a

Please sign in to comment.