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
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 @@ -105,7 +105,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
12 changes: 9 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 config = require('../../../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,10 @@ export class User {
@PrimaryGeneratedColumn('uuid')
id: string;

@Column({ length: 254 })
@Column({
length: 254,
transformer: lowerCaser,
})
@Index({ unique: true })
@IsEmail()
email: string;
Expand Down Expand Up @@ -119,8 +123,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';
BHesseldieck marked this conversation as resolved.
Show resolved Hide resolved
import config = require('../../../../config');

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

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

await queryRunner.query(`
UPDATE ${tablePrefix}user
SET email = LOWER(email);
`);
}
ivov marked this conversation as resolved.
Show resolved Hide resolved

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 { LowerCaseEmail1648740597343 } from './1648740597343-LowerCaseEmail';

export const mysqlMigrations = [
InitialMigration1588157391238,
Expand All @@ -28,4 +29,5 @@ export const mysqlMigrations = [
UpdateWorkflowCredentials1630451444017,
AddExecutionEntityIndexes1644424784709,
CreateUserManagement1646992772331,
LowerCaseEmail1648740597343,
];
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 LowerCaseEmail1648740597343 implements MigrationInterface {
name = 'LowerCaseEmail1648740597343';

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);
`);
}
BHesseldieck marked this conversation as resolved.
Show resolved Hide resolved

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 { LowerCaseEmail1648740597343 } from './1648740597343-LowerCaseEmail';

export const postgresMigrations = [
InitialMigration1587669153312,
Expand All @@ -24,4 +25,5 @@ export const postgresMigrations = [
AddExecutionEntityIndexes1644422880309,
IncreaseTypeVarcharLimit1646834195327,
CreateUserManagement1646992772331,
LowerCaseEmail1648740597343,
];
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 LowerCaseEmail1648740597343 implements MigrationInterface {
name = 'LowerCaseEmail1648740597343';

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 { LowerCaseEmail1648740597343 } from './1648740597343-LowerCaseEmail';

const sqliteMigrations = [
InitialMigration1588102412422,
Expand All @@ -24,6 +25,7 @@ const sqliteMigrations = [
UpdateWorkflowCredentials1630330987096,
AddExecutionEntityIndexes1644421939510,
CreateUserManagement1646992772331,
LowerCaseEmail1648740597343,
BHesseldieck marked this conversation as resolved.
Show resolved Hide resolved
];

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
76 changes: 42 additions & 34 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 = require('express');
import validator from 'validator';
import { v4 as uuid } from 'uuid';
Expand Down Expand Up @@ -35,7 +34,7 @@ beforeEach(async () => {
email: TEST_USER.email,
firstName: TEST_USER.firstName,
lastName: TEST_USER.lastName,
password: hashSync(TEST_USER.password, genSaltSync(10)),
password: TEST_USER.password,
ivov marked this conversation as resolved.
Show resolved Hide resolved
globalRole: globalOwnerRole,
});

Expand All @@ -58,38 +57,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
25 changes: 8 additions & 17 deletions packages/cli/test/integration/me.endpoints.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { hashSync, genSaltSync } from 'bcryptjs';
import express = require('express');
import validator from 'validator';

Expand Down Expand Up @@ -91,7 +90,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 +102,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 @@ -239,7 +238,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 @@ -251,7 +250,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 All @@ -274,9 +273,7 @@ describe('Member', () => {

test('PATCH /me/password should succeed with valid inputs', async () => {
const memberPassword = randomValidPassword();
const member = await testDb.createUser({
password: hashSync(memberPassword, genSaltSync(10)),
});
const member = await testDb.createUser({ password: memberPassword });
ivov marked this conversation as resolved.
Show resolved Hide resolved
const authMemberAgent = utils.createAgent(app, { auth: true, user: member });

const validPayload = {
Expand Down Expand Up @@ -393,7 +390,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 @@ -405,19 +402,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 @@ -437,7 +428,7 @@ const VALID_PATCH_ME_PAYLOADS = [
password: randomValidPassword(),
},
{
email: randomEmail(),
email: randomEmail().toUpperCase(),
firstName: randomName(),
lastName: randomName(),
password: randomValidPassword(),
Expand Down
23 changes: 23 additions & 0 deletions packages/cli/test/integration/owner.endpoints.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,13 @@ beforeAll(async () => {
});

beforeEach(async () => {
jest.isolateModules(() => {
jest.mock('../../config');
});
BHesseldieck marked this conversation as resolved.
Show resolved Hide resolved

await testDb.createOwnerShell();

config.set('userManagement.isInstanceOwnerSetUp', false);
BHesseldieck marked this conversation as resolved.
Show resolved Hide resolved
});

afterEach(async () => {
Expand Down Expand Up @@ -80,6 +86,23 @@ 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 owner = await Db.collections.User!.findOneOrFail();
const authOwnerAgent = utils.createAgent(app, { auth: true, user: owner });

const payload = Object.assign({}, TEST_USER, { email: TEST_USER.email.toUpperCase() });
const response = await authOwnerAgent.post('/owner').send(payload);

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

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

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

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

test('POST /owner should fail with invalid inputs', async () => {
const owner = await Db.collections.User!.findOneOrFail();
const authOwnerAgent = await utils.createAgent(app, { auth: true, user: owner });
Expand Down
Loading