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

General database fixes for user management #2710

Merged
merged 13 commits into from
Feb 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
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
14 changes: 3 additions & 11 deletions packages/cli/src/ActiveWorkflowRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -373,17 +373,9 @@ export class ActiveWorkflowRunner {
* @returns {boolean}
* @memberof ActiveWorkflowRunner
*/
async isActive(id: string, userId?: string): Promise<boolean> {
// TODO UM: make userId mandatory?
const queryBuilder = Db.collections.Workflow!.createQueryBuilder('w');
queryBuilder.andWhere('w.id = :id', { id });
if (userId) {
queryBuilder.innerJoin('w.shared', 'shared');
queryBuilder.andWhere('shared.user = :userId', { userId });
}

const workflow = (await queryBuilder.getOne()) as IWorkflowDb;
return workflow?.active;
async isActive(id: string): Promise<boolean> {
const workflow = await Db.collections.Workflow!.findOne(id);
return !!workflow?.active;
}

/**
Expand Down
7 changes: 1 addition & 6 deletions packages/cli/src/Interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,7 @@ export interface IN8nUISettings {
versionNotifications: IVersionNotificationSettings;
instanceId: string;
telemetry: ITelemetrySettings;
personalizationSurvey: IPersonalizationSurvey;
personalizationSurveyEnabled: boolean;
defaultLocale: string;
userManagement: IUserManagementSettings;
workflowTagsDisabled: boolean;
Expand All @@ -445,11 +445,6 @@ export interface IPersonalizationSurveyAnswers {
workArea: string[] | string | null;
}

export interface IPersonalizationSurvey {
answers?: IPersonalizationSurveyAnswers;
shouldShow: boolean;
}

export interface IUserManagementSettings {
enabled: boolean;
showSetupOnFirstLoad?: boolean;
Expand Down
50 changes: 0 additions & 50 deletions packages/cli/src/PersonalizationSurvey.ts

This file was deleted.

11 changes: 3 additions & 8 deletions packages/cli/src/Server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,6 @@ import {
import * as config from '../config';

import * as TagHelpers from './TagHelpers';
import * as PersonalizationSurvey from './PersonalizationSurvey';

import { InternalHooksManager } from './InternalHooksManager';
import { TagEntity } from './databases/entities/TagEntity';
Expand Down Expand Up @@ -309,9 +308,8 @@ class App {
},
instanceId: '',
telemetry: telemetrySettings,
personalizationSurvey: {
shouldShow: false,
},
personalizationSurveyEnabled:
config.get('personalization.enabled') && config.get('diagnostics.enabled'),
defaultLocale: config.get('defaultLocale'),
userManagement: {
enabled:
Expand Down Expand Up @@ -350,9 +348,6 @@ class App {

this.frontendSettings.instanceId = await UserSettings.getInstanceId();

this.frontendSettings.personalizationSurvey =
await PersonalizationSurvey.preparePersonalizationSurvey();

await this.externalHooks.run('frontend.settings', [this.frontendSettings]);

const excludeEndpoints = config.get('security.excludeEndpoints') as string;
Expand Down Expand Up @@ -2520,7 +2515,7 @@ class App {
}

if (executingWorkflowIds.length > 0) {
Object.assign(findOptions.where, { id: !In(executingWorkflowIds) });
Object.assign(findOptions.where, { id: Not(In(executingWorkflowIds)) });
krynble marked this conversation as resolved.
Show resolved Hide resolved
}

const executions = await Db.collections.Execution!.find(findOptions);
Expand Down
9 changes: 8 additions & 1 deletion packages/cli/src/UserManagement/UserManagementHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,13 @@ export function validatePassword(password?: string): string {
* Remove sensitive properties from the user to return to the client.
*/
export function sanitizeUser(user: User): PublicUser {
const { password, resetPasswordToken, createdAt, updatedAt, ...sanitizedUser } = user;
const {
password,
resetPasswordToken,
resetPasswordTokenExpiration,
createdAt,
updatedAt,
...sanitizedUser
} = user;
return sanitizedUser;
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<p>Hi there,</p>
<p>You have been invited to join n8n {{domain}}.</p>
<p>Please click on the following link, or paste it into your browser to complete the process:</p>
<p>Please click on the following link, or paste it into your browser to complete the process. The link is valid for 2 hours.</p>
<p><a href="{{ inviteAcceptUrl }}" target="_blank">{{ inviteAcceptUrl }}</a></p>
<br>
<p>Thanks!</p>
33 changes: 25 additions & 8 deletions packages/cli/src/UserManagement/routes/passwordReset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { v4 as uuid } from 'uuid';
import { URL } from 'url';
import { genSaltSync, hashSync } from 'bcryptjs';
import validator from 'validator';
import { IsNull, Not } from 'typeorm';
import { IsNull, MoreThanOrEqual, Not } from 'typeorm';

import { Db, ResponseHelper } from '../..';
import { N8nApp } from '../Interfaces';
Expand Down Expand Up @@ -45,15 +45,17 @@ export function passwordResetNamespace(this: N8nApp): void {
// User should just be able to reset password if one is already present
const user = await Db.collections.User!.findOne({ email, password: Not(IsNull()) });

if (!user) {
if (!user || !user.password) {
return;
csuermann marked this conversation as resolved.
Show resolved Hide resolved
}

user.resetPasswordToken = uuid();

const { id, firstName, lastName, resetPasswordToken } = user;

await Db.collections.User!.update(id, { resetPasswordToken });
const resetPasswordTokenExpiration = Math.floor(Date.now() / 1000) + 7200;

await Db.collections.User!.update(id, { resetPasswordToken, resetPasswordTokenExpiration });

const baseUrl = getBaseUrl();
const url = new URL('/change-password', baseUrl);
Expand Down Expand Up @@ -82,7 +84,14 @@ export function passwordResetNamespace(this: N8nApp): void {
throw new ResponseHelper.ResponseError('', undefined, 400);
}

const user = await Db.collections.User!.findOne({ resetPasswordToken, id });
// Timestamp is saved in seconds
const currentTimestamp = Math.floor(Date.now() / 1000);

const user = await Db.collections.User!.findOne({
id,
resetPasswordToken,
resetPasswordTokenExpiration: MoreThanOrEqual(currentTimestamp),
});

if (!user) {
throw new ResponseHelper.ResponseError('', undefined, 404);
Expand All @@ -96,23 +105,31 @@ export function passwordResetNamespace(this: N8nApp): void {
this.app.post(
`/${this.restEndpoint}/change-password`,
ResponseHelper.send(async (req: PasswordResetRequest.NewPassword, res: express.Response) => {
const { token: resetPasswordToken, userId, password } = req.body;
const { token: resetPasswordToken, userId: id, password } = req.body;

if (!resetPasswordToken || !userId || !password) {
if (!resetPasswordToken || !id || !password) {
throw new ResponseHelper.ResponseError('Parameter missing', undefined, 400);
}

const validPassword = validatePassword(password);

const user = await Db.collections.User!.findOne({ id: userId, resetPasswordToken });
// Timestamp is saved in seconds
const currentTimestamp = Math.floor(Date.now() / 1000);

const user = await Db.collections.User!.findOne({
id,
resetPasswordToken,
resetPasswordTokenExpiration: MoreThanOrEqual(currentTimestamp),
});

if (!user) {
throw new ResponseHelper.ResponseError('', undefined, 404);
}

await Db.collections.User!.update(userId, {
await Db.collections.User!.update(id, {
password: hashSync(validPassword, genSaltSync(10)),
resetPasswordToken: null,
resetPasswordTokenExpiration: null,
});

await issueCookie(res, user);
Expand Down
10 changes: 9 additions & 1 deletion packages/cli/src/UserManagement/routes/users.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,15 @@ export function usersNamespace(this: N8nApp): void {
this.app.post(
`/${this.restEndpoint}/users`,
ResponseHelper.send(async (req: UserRequest.Invite) => {
if (config.get('userManagement.emails.mode') === '') {
if (!config.get('userManagement.hasOwner')) {
throw new ResponseHelper.ResponseError(
'You must set up your own account before inviting others',
undefined,
400,
);
}

if (!isEmailSetUp) {
throw new ResponseHelper.ResponseError(
'Email sending must be set up in order to invite other users',
undefined,
Expand Down
4 changes: 4 additions & 0 deletions packages/cli/src/databases/entities/User.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,10 @@ export class User {
@Column({ type: String, nullable: true })
resetPasswordToken?: string | null;

// Expiration timestamp saved in seconds
@Column({ type: Number, nullable: true })
resetPasswordTokenExpiration?: number | null;

@Column({
type: resolveDataType('json') as ColumnOptions['type'],
nullable: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,6 @@ export class UpdateWorkflowCredentials1630451444017 implements MigrationInterfac
queryRunner.query(updateQuery, updateParams);
}
});
console.timeEnd(this.name);
}

public async down(queryRunner: QueryRunner): Promise<void> {
Expand Down
Loading