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

Refactorings from user management walkthrough #2856

Merged
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
79a162a
:zap: Make `getTemplate` async
ivov Feb 21, 2022
2ad6ed8
:zap: Remove query builder from `getCredentials`
ivov Feb 21, 2022
295f21d
:zap: Add save manual executions log message
ivov Feb 21, 2022
95a6295
:rewind: Restore and hide migrations logs
ivov Feb 21, 2022
32d599b
:zap: Centralize ignore paths check
ivov Feb 21, 2022
34a97ce
:shirt: Fix build
ivov Feb 21, 2022
f6861b6
:twisted_rightwards_arrows: Merge parent branch
ivov Feb 22, 2022
c275012
:truck: Rename `hasOwner` to `isInstanceOwnerSetUp`
ivov Feb 22, 2022
0296ab2
:zap: Add `isSetUp` flag to `User`
ivov Feb 22, 2022
c00a560
:zap: Add `isSetUp` to FE interface
ivov Feb 22, 2022
4a29405
:zap: Adjust `isSetUp` checks on FE
ivov Feb 22, 2022
027e0bb
:shirt: Fix build
ivov Feb 22, 2022
df79383
:twisted_rightwards_arrows: Merge parent branch
ivov Feb 23, 2022
0d00aff
:twisted_rightwards_arrows: Merge parent branch
ivov Feb 23, 2022
497440d
:zap: Adjust `isPendingUser()` check
ivov Feb 23, 2022
75514aa
:truck: Shorten helper name
ivov Feb 23, 2022
a1b9338
:twisted_rightwards_arrows: Merge parent branch
ivov Feb 23, 2022
d27ee4d
:zap: Refactor as `isPending` per feedback
ivov Feb 24, 2022
fd84ac4
:twisted_rightwards_arrows: Merge parent branch
ivov Feb 25, 2022
edfb0b2
:pencil2: Update log message
ivov Feb 28, 2022
409fb0f
:zap: Broaden check
ivov Feb 28, 2022
2490716
:fire: Remove unneeded relation
ivov Feb 28, 2022
152d112
:zap: Refactor query
ivov Feb 28, 2022
6ca49b9
:twisted_rightwards_arrows: Merge parent branch
ivov Feb 28, 2022
d460107
:fire: Re-remove logs from migrations
ivov Feb 28, 2022
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: 6 additions & 0 deletions packages/cli/commands/start.ts
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,12 @@ export class Start extends Command {
const editorUrl = GenericHelpers.getBaseUrl();
this.log(`\nEditor is now accessible via:\n${editorUrl}`);

const saveManualExecutions = config.get('executions.saveDataManualExecutions') as boolean;

if (saveManualExecutions) {
this.log('\nManual executions will be visible only for the owner');
}

// Allow to open n8n editor by pressing "o"
if (Boolean(process.stdout.isTTY) && process.stdin.setRawMode) {
process.stdin.setRawMode(true);
Expand Down
5 changes: 4 additions & 1 deletion packages/cli/commands/user-management/reset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,10 @@ export class Reset extends Command {
await Db.collections.User!.delete({ id: Not(owner.id) });
await Db.collections.User!.save(Object.assign(owner, this.defaultUserProps));

await Db.collections.Settings!.update({ key: 'userManagement.hasOwner' }, { value: 'false' });
await Db.collections.Settings!.update(
{ key: 'userManagement.isInstanceOwnerSetUp' },
{ value: 'false' },
);
} catch (error) {
console.error('Error resetting database. See log messages for details.');
if (error instanceof Error) logger.error(error.message);
Expand Down
39 changes: 20 additions & 19 deletions packages/cli/src/CredentialsHelper.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable @typescript-eslint/no-non-null-assertion */
/* eslint-disable no-restricted-syntax */
/* eslint-disable @typescript-eslint/no-unsafe-member-access */
/* eslint-disable @typescript-eslint/no-unsafe-assignment */
Expand Down Expand Up @@ -218,40 +219,40 @@ export class CredentialsHelper extends ICredentialsHelper {
/**
* Returns the credentials instance
*
* @param {INodeCredentialsDetails} nodeCredentials id and name to return instance of
* @param {string} type Type of the credentials to return instance of
* @param {INodeCredentialsDetails} nodeCredential id and name to return instance of
* @param {string} type Type of the credential to return instance of
* @returns {Credentials}
* @memberof CredentialsHelper
*/
async getCredentials(
nodeCredentials: INodeCredentialsDetails,
nodeCredential: INodeCredentialsDetails,
type: string,
userId?: string,
): Promise<Credentials> {
if (!nodeCredentials.id) {
throw new Error(`Credentials "${nodeCredentials.name}" for type "${type}" don't have an ID.`);
if (!nodeCredential.id) {
throw new Error(`Credential "${nodeCredential.name}" of type "${type}" has no ID.`);
}

// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const qb = Db.collections.Credentials!.createQueryBuilder('c');
qb.where('c.id = :id and c.type = :type', { id: nodeCredentials.id, type });
if (userId) {
// TODO UM: implement this.
// qb.
}
const credentials = await qb.getOne();
const credential = userId
? await Db.collections
.SharedCredentials!.findOneOrFail({
relations: ['credentials'],
where: { credentials: { id: nodeCredential.id, type }, user: { id: userId } },
})
.then((shared) => shared.credentials)
: await Db.collections.Credentials!.findOneOrFail({ id: nodeCredential.id, type });

if (!credentials) {
if (!credential) {
throw new Error(
`Credentials with ID "${nodeCredentials.id}" don't exist for type "${type}".`,
`Credential with ID "${nodeCredential.id}" does not exist for type "${type}".`,
);
}

return new Credentials(
{ id: credentials.id.toString(), name: credentials.name },
credentials.type,
credentials.nodesAccess,
credentials.data,
{ id: credential.id.toString(), name: credential.name },
credential.type,
credential.nodesAccess,
credential.data,
);
}

Expand Down
4 changes: 2 additions & 2 deletions packages/cli/src/Server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ class App {
userManagement: {
enabled:
config.get('userManagement.disabled') === false ||
config.get('userManagement.hasOwner') === true,
config.get('userManagement.isInstanceOwnerSetUp') === true,
// showSetupOnFirstLoad: config.get('userManagement.disabled') === false, // && config.get('userManagement.skipOwnerSetup') === true
smtpSetup: config.get('userManagement.emails.mode') === 'smtp',
},
Expand Down Expand Up @@ -1269,7 +1269,7 @@ class App {
throw new ResponseHelper.ResponseError('Workflow tags are disabled');
}
if (
config.get('userManagement.hasOwner') === true &&
config.get('userManagement.isInstanceOwnerSetUp') === true &&
(req.user as User).globalRole.name !== 'owner'
) {
throw new ResponseHelper.ResponseError(
Expand Down
9 changes: 9 additions & 0 deletions packages/cli/src/UserManagement/UserManagementHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,15 @@ export function sanitizeUser(user: User): PublicUser {
return sanitizedUser;
}

/**
* Check if a URL contains an auth-excluded endpoint.
*/
export function isAuthExcluded(url: string, ignoredEndpoints: string[]): boolean {
return !!ignoredEndpoints
.filter(Boolean) // skip empty paths
.find((ignoredEndpoint) => url.includes(ignoredEndpoint));
}

/**
* Check if the endpoint is `POST /users/:id`.
*/
Expand Down
13 changes: 9 additions & 4 deletions packages/cli/src/UserManagement/email/UserManagementMailer.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
/* eslint-disable import/no-cycle */
import { existsSync, readFileSync } from 'fs';
import { IDataObject } from 'n8n-workflow';
import { join as pathJoin } from 'path';
import { GenericHelpers } from '../..';
import config = require('../../../config');
import {
InviteEmailData,
Expand All @@ -10,8 +12,11 @@ import {
} from './Interfaces';
import { NodeMailer } from './NodeMailer';

function getTemplate(configKeyName: string, defaultFilename: string) {
const templateOverride = config.get(`userManagement.emails.templates.${configKeyName}`) as string;
async function getTemplate(configKeyName: string, defaultFilename: string) {
const templateOverride = (await GenericHelpers.getConfigValue(
`userManagement.emails.templates.${configKeyName}`,
)) as string;

let template;
if (templateOverride && existsSync(templateOverride)) {
template = readFileSync(templateOverride, {
Expand Down Expand Up @@ -46,7 +51,7 @@ export class UserManagementMailer {
}

async invite(inviteEmailData: InviteEmailData): Promise<SendEmailResult> {
let template = getTemplate('invite', 'invite.html');
let template = await getTemplate('invite', 'invite.html');
template = replaceStrings(template, inviteEmailData);

// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
Expand All @@ -61,7 +66,7 @@ export class UserManagementMailer {
}

async passwordReset(passwordResetData: PasswordResetData): Promise<SendEmailResult> {
let template = getTemplate('passwordReset', 'passwordReset.html');
let template = await getTemplate('passwordReset', 'passwordReset.html');
template = replaceStrings(template, passwordResetData);

// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
Expand Down
1 change: 1 addition & 0 deletions packages/cli/src/UserManagement/email/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable import/no-cycle */
import { getInstance, UserManagementMailer } from './UserManagementMailer';

export { getInstance, UserManagementMailer };
16 changes: 3 additions & 13 deletions packages/cli/src/UserManagement/routes/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { usersNamespace } from './users';
import { passwordResetNamespace } from './passwordReset';
import { AuthenticatedRequest } from '../../requests';
import { ownerNamespace } from './owner';
import { isAuthenticatedRequest, isPostUsersId } from '../UserManagementHelper';
import { isAuthExcluded, isPostUsersId, isAuthenticatedRequest } from '../UserManagementHelper';

export function addRoutes(this: N8nApp, ignoredEndpoints: string[], restEndpoint: string): void {
// needed for testing; not adding overhead since it directly returns if req.cookies exists
Expand Down Expand Up @@ -65,22 +65,12 @@ export function addRoutes(this: N8nApp, ignoredEndpoints: string[], restEndpoint
isPostUsersId(req, restEndpoint) ||
req.url.startsWith(`/${restEndpoint}/forgot-password`) ||
req.url.startsWith(`/${restEndpoint}/resolve-password-token`) ||
req.url.startsWith(`/${restEndpoint}/change-password`)
req.url.startsWith(`/${restEndpoint}/change-password`) ||
isAuthExcluded(req.url, ignoredEndpoints)
) {
return next();
}

for (let i = 0; i < ignoredEndpoints.length; i++) {
const path = ignoredEndpoints[i];
if (!path) {
// Skip empty paths (they might exist)
// eslint-disable-next-line no-continue
continue;
}
if (req.url.includes(path)) {
return next();
}
}
return passport.authenticate('jwt', { session: false })(req, res, next);
});

Expand Down
10 changes: 5 additions & 5 deletions packages/cli/src/UserManagement/routes/owner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,15 @@ import { sanitizeUser, validatePassword } from '../UserManagementHelper';
export function ownerNamespace(this: N8nApp): void {
/**
* Promote a shell into the owner of the n8n instance,
* and enable `hasOwner` instance setting.
* and enable `isInstanceOwnerSetUp` setting.
*/
this.app.post(
`/${this.restEndpoint}/owner`,
ResponseHelper.send(async (req: OwnerRequest.Post, res: express.Response) => {
const { email, firstName, lastName, password } = req.body;
const { id: userId } = req.user;

if (config.get('userManagement.hasOwner')) {
if (config.get('userManagement.isInstanceOwnerSetUp')) {
Logger.debug(
'Request to claim instance ownership failed because instance owner already exists',
{
Expand Down Expand Up @@ -79,14 +79,14 @@ export function ownerNamespace(this: N8nApp): void {

Logger.info('Owner updated successfully', { userId: req.user.id });

config.set('userManagement.hasOwner', true);
config.set('userManagement.isInstanceOwnerSetUp', true);

await Db.collections.Settings!.update(
{ key: 'userManagement.hasOwner' },
{ key: 'userManagement.isInstanceOwnerSetUp' },
{ value: JSON.stringify(true) },
);

Logger.debug('Setting hasOwner updated successfully', { userId: req.user.id });
Logger.debug('Setting isInstanceOwnerSetUp updated successfully', { userId: req.user.id });

await issueCookie(res, owner);

Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/UserManagement/routes/users.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export function usersNamespace(this: N8nApp): void {
);
}

if (!config.get('userManagement.hasOwner')) {
if (!config.get('userManagement.isInstanceOwnerSetUp')) {
Logger.debug(
'Request to send email invite(s) to user(s) failed because emailing was not set up',
);
Expand Down
11 changes: 11 additions & 0 deletions packages/cli/src/databases/entities/User.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
ManyToOne,
PrimaryGeneratedColumn,
UpdateDateColumn,
AfterLoad,
} from 'typeorm';
import { IsEmail, IsString, Length } from 'class-validator';
import config = require('../../../config');
Expand Down Expand Up @@ -119,4 +120,14 @@ export class User {
setUpdateDate(): void {
this.updatedAt = new Date();
}

/**
* Whether the user is pending setup completion.
*/
isPending: boolean;

@AfterLoad()
computeIsPending(): void {
this.isPending = !!this.password;
}
}
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
import {MigrationInterface, QueryRunner} from "typeorm";
import * as config from '../../../../config';
import { isTestRun } from "../../../../test/integration/shared/utils";

export class AddWaitColumnId1626183952959 implements MigrationInterface {
name = 'AddWaitColumnId1626183952959';

async up(queryRunner: QueryRunner): Promise<void> {
const tablePrefix = config.get('database.tablePrefix');
!isTestRun && console.log(
'\n\nINFO: Started with migration for wait functionality.\n Depending on the number of saved executions, that may take a little bit.\n\n',
);
ivov marked this conversation as resolved.
Show resolved Hide resolved

await queryRunner.query('ALTER TABLE `' + tablePrefix + 'execution_entity` ADD `waitTill` DATETIME NULL');
await queryRunner.query('CREATE INDEX `IDX_' + tablePrefix + 'ca4a71b47f28ac6ea88293a8e2` ON `' + tablePrefix + 'execution_entity` (`waitTill`)');
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { MigrationInterface, QueryRunner } from 'typeorm';
import config = require('../../../../config');
import { isTestRun } from '../../../../test/integration/shared/utils';
import { MigrationHelpers } from '../../MigrationHelpers';

// replacing the credentials in workflows and execution
Expand All @@ -9,6 +10,8 @@ export class UpdateWorkflowCredentials1630451444017 implements MigrationInterfac
name = 'UpdateWorkflowCredentials1630451444017';

public async up(queryRunner: QueryRunner): Promise<void> {
!isTestRun && console.log('Start migration', this.name);
!isTestRun && console.time(this.name);
ivov marked this conversation as resolved.
Show resolved Hide resolved
const tablePrefix = config.get('database.tablePrefix');
const helpers = new MigrationHelpers(queryRunner);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ export class CreateUserManagement1636626154933 implements MigrationInterface {
'INSERT INTO `' +
tablePrefix +
'settings` (`key`, value, loadOnStartup) values ' +
'("userManagement.hasOwner", "false", 1)',
'("userManagement.isInstanceOwnerSetUp", "false", 1)',
);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {MigrationInterface, QueryRunner} from "typeorm";
import * as config from '../../../../config';
import { isTestRun } from "../../../../test/integration/shared/utils";

export class AddwaitTill1626176912946 implements MigrationInterface {
name = 'AddwaitTill1626176912946';
Expand All @@ -12,6 +13,11 @@ export class AddwaitTill1626176912946 implements MigrationInterface {
tablePrefix = schema + '.' + tablePrefix;
}

!isTestRun &&
console.log(
'\n\nINFO: Started with migration for wait functionality.\n Depending on the number of saved executions, that may take a little bit.\n\n',
);

ivov marked this conversation as resolved.
Show resolved Hide resolved
await queryRunner.query(`ALTER TABLE ${tablePrefix}execution_entity ADD "waitTill" TIMESTAMP`);
await queryRunner.query(`CREATE INDEX IF NOT EXISTS IDX_${tablePrefixPure}ca4a71b47f28ac6ea88293a8e2 ON ${tablePrefix}execution_entity ("waitTill")`);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { MigrationInterface, QueryRunner } from 'typeorm';
import config = require('../../../../config');
import { isTestRun } from '../../../../test/integration/shared/utils';
import { MigrationHelpers } from '../../MigrationHelpers';

// replacing the credentials in workflows and execution
Expand All @@ -9,6 +10,8 @@ export class UpdateWorkflowCredentials1630419189837 implements MigrationInterfac
name = 'UpdateWorkflowCredentials1630419189837';

public async up(queryRunner: QueryRunner): Promise<void> {
!isTestRun && console.log('Start migration', this.name);
!isTestRun && console.time(this.name);
ivov marked this conversation as resolved.
Show resolved Hide resolved
let tablePrefix = config.get('database.tablePrefix');
const schema = config.get('database.postgresdb.schema');
if (schema) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ export class CreateUserManagement1636626154934 implements MigrationInterface {
'INSERT INTO "' +
tablePrefix +
'settings" ("key", "value", "loadOnStartup") values ' +
"('userManagement.hasOwner', 'false', true)",
"('userManagement.isInstanceOwnerSetUp', 'false', true)",
);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
import { MigrationInterface, QueryRunner } from 'typeorm';
import * as config from '../../../../config';
import { isTestRun } from '../../../../test/integration/shared/utils';

export class AddWaitColumn1621707690587 implements MigrationInterface {
name = 'AddWaitColumn1621707690587';

async up(queryRunner: QueryRunner): Promise<void> {
const tablePrefix = config.get('database.tablePrefix');
!isTestRun &&
console.log(
'\n\nINFO: Started with migration for wait functionality.\n Depending on the number of saved executions, that may take a little bit.\n\n',
);
ivov marked this conversation as resolved.
Show resolved Hide resolved

await queryRunner.query(`DROP TABLE IF EXISTS "${tablePrefix}temporary_execution_entity"`);
await queryRunner.query(`CREATE TABLE "${tablePrefix}temporary_execution_entity" ("id" integer PRIMARY KEY AUTOINCREMENT NOT NULL, "data" text NOT NULL, "finished" boolean NOT NULL, "mode" varchar NOT NULL, "retryOf" varchar, "retrySuccessId" varchar, "startedAt" datetime NOT NULL, "stoppedAt" datetime, "workflowData" text NOT NULL, "workflowId" varchar, "waitTill" DATETIME)`, undefined);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { MigrationInterface, QueryRunner } from 'typeorm';
import config = require('../../../../config');
import { isTestRun } from '../../../../test/integration/shared/utils';
import { MigrationHelpers } from '../../MigrationHelpers';

// replacing the credentials in workflows and execution
Expand All @@ -9,6 +10,8 @@ export class UpdateWorkflowCredentials1630330987096 implements MigrationInterfac
name = 'UpdateWorkflowCredentials1630330987096';

public async up(queryRunner: QueryRunner): Promise<void> {
!isTestRun && console.log('Start migration', this.name);
!isTestRun && console.time(this.name);
ivov marked this conversation as resolved.
Show resolved Hide resolved
const tablePrefix = config.get('database.tablePrefix');
const helpers = new MigrationHelpers(queryRunner);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ export class CreateUserManagement1636626154932 implements MigrationInterface {

await queryRunner.query(`
INSERT INTO "${tablePrefix}settings" (key, value, loadOnStartup) values
('userManagement.hasOwner', 'false', true)
('userManagement.isInstanceOwnerSetUp', 'false', true)
`);
}

Expand Down
Loading