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

⚡️ remove email case sensitivity #3078

Merged
merged 9 commits into from
Apr 15, 2022
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
} from './Interfaces';
import { NodeMailer } from './NodeMailer';

// TODO: make function fully async (remove sync functions)
async function getTemplate(configKeyName: string, defaultFilename: string) {
const templateOverride = (await GenericHelpers.getConfigValue(
`userManagement.emails.templates.${configKeyName}`,
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 @@ -104,7 +104,7 @@ export function usersNamespace(this: N8nApp): void {
400,
);
}
createUsers[invite.email] = null;
createUsers[invite.email.toLowerCase()] = null;
});

const role = await Db.collections.Role.findOne({ scope: 'global', name: 'member' });
Expand Down
13 changes: 10 additions & 3 deletions packages/cli/src/databases/entities/User.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
ManyToOne,
PrimaryGeneratedColumn,
UpdateDateColumn,
BeforeInsert,
} from 'typeorm';
import { IsEmail, IsString, Length } from 'class-validator';
import * as config from '../../../config';
Expand All @@ -20,7 +21,7 @@ import { Role } from './Role';
import { SharedWorkflow } from './SharedWorkflow';
import { SharedCredentials } from './SharedCredentials';
import { NoXss } from '../utils/customValidators';
import { answersFormatter } from '../utils/transformers';
import { answersFormatter, lowerCaser } from '../utils/transformers';

export const MIN_PASSWORD_LENGTH = 8;

Expand Down Expand Up @@ -62,7 +63,11 @@ export class User {
@PrimaryGeneratedColumn('uuid')
id: string;

@Column({ length: 254, nullable: true })
@Column({
length: 254,
nullable: true,
transformer: lowerCaser,
})
@Index({ unique: true })
@IsEmail()
email: string;
Expand Down Expand Up @@ -119,8 +124,10 @@ export class User {
})
updatedAt: Date;

@BeforeInsert()
@BeforeUpdate()
setUpdateDate(): void {
preUpsertHook(): void {
this.email = this.email?.toLowerCase();
this.updatedAt = new Date();
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { MigrationInterface, QueryRunner } from 'typeorm';
import config = require('../../../../config');

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

public async up(queryRunner: QueryRunner): Promise<void> {
const tablePrefix = config.get('database.tablePrefix');

await queryRunner.query(`
UPDATE ${tablePrefix}user
SET email = LOWER(email);
`);
}

public async down(queryRunner: QueryRunner): Promise<void> {}
}
2 changes: 2 additions & 0 deletions packages/cli/src/databases/mysqldb/migrations/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { AddWaitColumnId1626183952959 } from './1626183952959-AddWaitColumn';
import { UpdateWorkflowCredentials1630451444017 } from './1630451444017-UpdateWorkflowCredentials';
import { AddExecutionEntityIndexes1644424784709 } from './1644424784709-AddExecutionEntityIndexes';
import { CreateUserManagement1646992772331 } from './1646992772331-CreateUserManagement';
import { LowerCaseUserEmail1648740597343 } from './1648740597343-LowerCaseUserEmail';

export const mysqlMigrations = [
InitialMigration1588157391238,
Expand All @@ -28,4 +29,5 @@ export const mysqlMigrations = [
UpdateWorkflowCredentials1630451444017,
AddExecutionEntityIndexes1644424784709,
CreateUserManagement1646992772331,
LowerCaseUserEmail1648740597343,
];
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { MigrationInterface, QueryRunner } from 'typeorm';
import config = require('../../../../config');

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

public async up(queryRunner: QueryRunner): Promise<void> {
let tablePrefix = config.get('database.tablePrefix');
const schema = config.get('database.postgresdb.schema');
if (schema) {
tablePrefix = schema + '.' + tablePrefix;
}

await queryRunner.query(`
UPDATE ${tablePrefix}user
SET email = LOWER(email);
`);
}

public async down(queryRunner: QueryRunner): Promise<void> {}
}
2 changes: 2 additions & 0 deletions packages/cli/src/databases/postgresdb/migrations/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { UpdateWorkflowCredentials1630419189837 } from './1630419189837-UpdateWo
import { AddExecutionEntityIndexes1644422880309 } from './1644422880309-AddExecutionEntityIndexes';
import { IncreaseTypeVarcharLimit1646834195327 } from './1646834195327-IncreaseTypeVarcharLimit';
import { CreateUserManagement1646992772331 } from './1646992772331-CreateUserManagement';
import { LowerCaseUserEmail1648740597343 } from './1648740597343-LowerCaseUserEmail';

export const postgresMigrations = [
InitialMigration1587669153312,
Expand All @@ -24,4 +25,5 @@ export const postgresMigrations = [
AddExecutionEntityIndexes1644422880309,
IncreaseTypeVarcharLimit1646834195327,
CreateUserManagement1646992772331,
LowerCaseUserEmail1648740597343,
];
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { MigrationInterface, QueryRunner } from 'typeorm';
import config = require('../../../../config');
import { logMigrationEnd, logMigrationStart } from '../../utils/migrationHelpers';

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

public async up(queryRunner: QueryRunner): Promise<void> {
logMigrationStart(this.name);

const tablePrefix = config.get('database.tablePrefix');

await queryRunner.query(`
UPDATE "${tablePrefix}user"
SET email = LOWER(email);
`);

logMigrationEnd(this.name);
}

public async down(queryRunner: QueryRunner): Promise<void> {}
}
2 changes: 2 additions & 0 deletions packages/cli/src/databases/sqlite/migrations/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { AddWaitColumn1621707690587 } from './1621707690587-AddWaitColumn';
import { UpdateWorkflowCredentials1630330987096 } from './1630330987096-UpdateWorkflowCredentials';
import { AddExecutionEntityIndexes1644421939510 } from './1644421939510-AddExecutionEntityIndexes';
import { CreateUserManagement1646992772331 } from './1646992772331-CreateUserManagement';
import { LowerCaseUserEmail1648740597343 } from './1648740597343-LowerCaseUserEmail';

const sqliteMigrations = [
InitialMigration1588102412422,
Expand All @@ -24,6 +25,7 @@ const sqliteMigrations = [
UpdateWorkflowCredentials1630330987096,
AddExecutionEntityIndexes1644421939510,
CreateUserManagement1646992772331,
LowerCaseUserEmail1648740597343,
];

export { sqliteMigrations };
9 changes: 7 additions & 2 deletions packages/cli/src/databases/utils/transformers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,13 @@
import { IPersonalizationSurveyAnswers } from '../../Interfaces';

export const idStringifier = {
from: (value: number): string | number => (value ? value.toString() : value),
to: (value: string): number | string => (value ? Number(value) : value),
from: (value: number): string | number => (typeof value === 'number' ? value.toString() : value),
to: (value: string): number | string => (typeof value === 'string' ? Number(value) : value),
};

export const lowerCaser = {
from: (value: string): string => value,
to: (value: string): string => (typeof value === 'string' ? value.toLowerCase() : value),
BHesseldieck marked this conversation as resolved.
Show resolved Hide resolved
};

/**
Expand Down
74 changes: 41 additions & 33 deletions packages/cli/test/integration/auth.endpoints.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { hashSync, genSaltSync } from 'bcryptjs';
import express from 'express';
import validator from 'validator';
import { v4 as uuid } from 'uuid';
Expand Down Expand Up @@ -60,38 +59,47 @@ afterAll(async () => {
test('POST /login should log user in', async () => {
const authlessAgent = utils.createAgent(app);

const response = await authlessAgent.post('/login').send({
email: TEST_USER.email,
password: TEST_USER.password,
});

expect(response.statusCode).toBe(200);

const {
id,
email,
firstName,
lastName,
password,
personalizationAnswers,
globalRole,
resetPasswordToken,
} = response.body.data;

expect(validator.isUUID(id)).toBe(true);
expect(email).toBe(TEST_USER.email);
expect(firstName).toBe(TEST_USER.firstName);
expect(lastName).toBe(TEST_USER.lastName);
expect(password).toBeUndefined();
expect(personalizationAnswers).toBeNull();
expect(password).toBeUndefined();
expect(resetPasswordToken).toBeUndefined();
expect(globalRole).toBeDefined();
expect(globalRole.name).toBe('owner');
expect(globalRole.scope).toBe('global');

const authToken = utils.getAuthToken(response);
expect(authToken).toBeDefined();
await Promise.all(
[
{
email: TEST_USER.email,
password: TEST_USER.password,
},
{
email: TEST_USER.email.toUpperCase(),
password: TEST_USER.password,
},
].map(async (payload) => {
const response = await authlessAgent.post('/login').send(payload);

expect(response.statusCode).toBe(200);

const {
id,
email,
firstName,
lastName,
password,
personalizationAnswers,
globalRole,
resetPasswordToken,
} = response.body.data;

expect(validator.isUUID(id)).toBe(true);
expect(email).toBe(TEST_USER.email);
expect(firstName).toBe(TEST_USER.firstName);
expect(lastName).toBe(TEST_USER.lastName);
expect(password).toBeUndefined();
expect(personalizationAnswers).toBeNull();
expect(resetPasswordToken).toBeUndefined();
expect(globalRole).toBeDefined();
expect(globalRole.name).toBe('owner');
expect(globalRole.scope).toBe('global');

const authToken = utils.getAuthToken(response);
expect(authToken).toBeDefined();
}),
);
});

test('GET /login should receive logged in user', async () => {
Expand Down
20 changes: 7 additions & 13 deletions packages/cli/test/integration/me.api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ describe('Owner shell', () => {
} = response.body.data;

expect(validator.isUUID(id)).toBe(true);
expect(email).toBe(validPayload.email);
expect(email).toBe(validPayload.email.toLowerCase());
expect(firstName).toBe(validPayload.firstName);
expect(lastName).toBe(validPayload.lastName);
expect(personalizationAnswers).toBeNull();
Expand All @@ -103,7 +103,7 @@ describe('Owner shell', () => {

const storedOwnerShell = await Db.collections.User!.findOneOrFail(id);

expect(storedOwnerShell.email).toBe(validPayload.email);
expect(storedOwnerShell.email).toBe(validPayload.email.toLowerCase());
expect(storedOwnerShell.firstName).toBe(validPayload.firstName);
expect(storedOwnerShell.lastName).toBe(validPayload.lastName);
}
Expand Down Expand Up @@ -245,7 +245,7 @@ describe('Member', () => {
} = response.body.data;

expect(validator.isUUID(id)).toBe(true);
expect(email).toBe(validPayload.email);
expect(email).toBe(validPayload.email.toLowerCase());
expect(firstName).toBe(validPayload.firstName);
expect(lastName).toBe(validPayload.lastName);
expect(personalizationAnswers).toBeNull();
Expand All @@ -257,7 +257,7 @@ describe('Member', () => {

const storedMember = await Db.collections.User!.findOneOrFail(id);

expect(storedMember.email).toBe(validPayload.email);
expect(storedMember.email).toBe(validPayload.email.toLowerCase());
expect(storedMember.firstName).toBe(validPayload.firstName);
expect(storedMember.lastName).toBe(validPayload.lastName);
}
Expand Down Expand Up @@ -400,7 +400,7 @@ describe('Owner', () => {
} = response.body.data;

expect(validator.isUUID(id)).toBe(true);
expect(email).toBe(validPayload.email);
expect(email).toBe(validPayload.email.toLowerCase());
expect(firstName).toBe(validPayload.firstName);
expect(lastName).toBe(validPayload.lastName);
expect(personalizationAnswers).toBeNull();
Expand All @@ -412,19 +412,13 @@ describe('Owner', () => {

const storedOwner = await Db.collections.User!.findOneOrFail(id);

expect(storedOwner.email).toBe(validPayload.email);
expect(storedOwner.email).toBe(validPayload.email.toLowerCase());
expect(storedOwner.firstName).toBe(validPayload.firstName);
expect(storedOwner.lastName).toBe(validPayload.lastName);
}
});
});

const TEST_USER = {
email: randomEmail(),
firstName: randomName(),
lastName: randomName(),
};

const SURVEY = [
'codingSkill',
'companyIndustry',
Expand All @@ -444,7 +438,7 @@ const VALID_PATCH_ME_PAYLOADS = [
password: randomValidPassword(),
},
{
email: randomEmail(),
email: randomEmail().toUpperCase(),
firstName: randomName(),
lastName: randomName(),
password: randomValidPassword(),
Expand Down
29 changes: 29 additions & 0 deletions packages/cli/test/integration/owner.api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@ beforeAll(async () => {
});

beforeEach(async () => {
jest.mock('../../config');

config.set('userManagement.isInstanceOwnerSetUp', false);
});

afterEach(async () => {
await testDb.truncate(['User'], testDbName);
});

Expand Down Expand Up @@ -88,6 +94,29 @@ test('POST /owner should create owner and enable isInstanceOwnerSetUp', async ()
expect(isInstanceOwnerSetUpSetting).toBe(true);
});

test('POST /owner should create owner with lowercased email', async () => {
const ownerShell = await testDb.createUserShell(globalOwnerRole);
const authOwnerAgent = utils.createAgent(app, { auth: true, user: ownerShell });

const newOwnerData = {
email: randomEmail().toUpperCase(),
firstName: randomName(),
lastName: randomName(),
password: randomValidPassword(),
};

const response = await authOwnerAgent.post('/owner').send(newOwnerData);

expect(response.statusCode).toBe(200);

const { id, email } = response.body.data;

expect(email).toBe(newOwnerData.email.toLowerCase());

const storedOwner = await Db.collections.User!.findOneOrFail(id);
expect(storedOwner.email).toBe(newOwnerData.email.toLowerCase());
});

test('POST /owner should fail with invalid inputs', async () => {
const ownerShell = await testDb.createUserShell(globalOwnerRole);
const authOwnerAgent = utils.createAgent(app, { auth: true, user: ownerShell });
Expand Down
Loading