Skip to content

Commit 8532b00

Browse files
BHesseldieckBHesseldieck
authored
fix(core): Make email for UM case insensitive (#3078)
* 🚧 lowercasing email * ✅ add tests for case insensitive email * 🐘 add migration to lowercase email * 🚚 rename migration * 🐛 fix package.lock * 🐛 fix double import * 📋 add todo
1 parent d3fecb9 commit 8532b00

File tree

15 files changed

+204
-81
lines changed

15 files changed

+204
-81
lines changed

‎packages/cli/src/UserManagement/email/UserManagementMailer.ts

+1
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
} from './Interfaces';
1313
import { NodeMailer } from './NodeMailer';
1414

15+
// TODO: make function fully async (remove sync functions)
1516
async function getTemplate(configKeyName: string, defaultFilename: string) {
1617
const templateOverride = (await GenericHelpers.getConfigValue(
1718
`userManagement.emails.templates.${configKeyName}`,

‎packages/cli/src/UserManagement/routes/users.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ export function usersNamespace(this: N8nApp): void {
104104
400,
105105
);
106106
}
107-
createUsers[invite.email] = null;
107+
createUsers[invite.email.toLowerCase()] = null;
108108
});
109109

110110
const role = await Db.collections.Role.findOne({ scope: 'global', name: 'member' });

‎packages/cli/src/databases/entities/User.ts

+10-3
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
ManyToOne,
1313
PrimaryGeneratedColumn,
1414
UpdateDateColumn,
15+
BeforeInsert,
1516
} from 'typeorm';
1617
import { IsEmail, IsString, Length } from 'class-validator';
1718
import * as config from '../../../config';
@@ -20,7 +21,7 @@ import { Role } from './Role';
2021
import { SharedWorkflow } from './SharedWorkflow';
2122
import { SharedCredentials } from './SharedCredentials';
2223
import { NoXss } from '../utils/customValidators';
23-
import { answersFormatter } from '../utils/transformers';
24+
import { answersFormatter, lowerCaser } from '../utils/transformers';
2425

2526
export const MIN_PASSWORD_LENGTH = 8;
2627

@@ -62,7 +63,11 @@ export class User {
6263
@PrimaryGeneratedColumn('uuid')
6364
id: string;
6465

65-
@Column({ length: 254, nullable: true })
66+
@Column({
67+
length: 254,
68+
nullable: true,
69+
transformer: lowerCaser,
70+
})
6671
@Index({ unique: true })
6772
@IsEmail()
6873
email: string;
@@ -119,8 +124,10 @@ export class User {
119124
})
120125
updatedAt: Date;
121126

127+
@BeforeInsert()
122128
@BeforeUpdate()
123-
setUpdateDate(): void {
129+
preUpsertHook(): void {
130+
this.email = this.email?.toLowerCase();
124131
this.updatedAt = new Date();
125132
}
126133

‎packages/cli/src/databases/mysqldb/migrations/1648740597343-LowerCaseUserEmail.ts

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import { MigrationInterface, QueryRunner } from 'typeorm';
2+
import config = require('../../../../config');
3+
4+
export class LowerCaseUserEmail1648740597343 implements MigrationInterface {
5+
name = 'LowerCaseUserEmail1648740597343';
6+
7+
public async up(queryRunner: QueryRunner): Promise<void> {
8+
const tablePrefix = config.get('database.tablePrefix');
9+
10+
await queryRunner.query(`
11+
UPDATE ${tablePrefix}user
12+
SET email = LOWER(email);
13+
`);
14+
}
15+
16+
public async down(queryRunner: QueryRunner): Promise<void> {}
17+
}

‎packages/cli/src/databases/mysqldb/migrations/index.ts

+2
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { AddWaitColumnId1626183952959 } from './1626183952959-AddWaitColumn';
1212
import { UpdateWorkflowCredentials1630451444017 } from './1630451444017-UpdateWorkflowCredentials';
1313
import { AddExecutionEntityIndexes1644424784709 } from './1644424784709-AddExecutionEntityIndexes';
1414
import { CreateUserManagement1646992772331 } from './1646992772331-CreateUserManagement';
15+
import { LowerCaseUserEmail1648740597343 } from './1648740597343-LowerCaseUserEmail';
1516

1617
export const mysqlMigrations = [
1718
InitialMigration1588157391238,
@@ -28,4 +29,5 @@ export const mysqlMigrations = [
2829
UpdateWorkflowCredentials1630451444017,
2930
AddExecutionEntityIndexes1644424784709,
3031
CreateUserManagement1646992772331,
32+
LowerCaseUserEmail1648740597343,
3133
];

‎packages/cli/src/databases/postgresdb/migrations/1648740597343-LowerCaseUserEmail.ts

+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
import { MigrationInterface, QueryRunner } from 'typeorm';
2+
import config = require('../../../../config');
3+
4+
export class LowerCaseUserEmail1648740597343 implements MigrationInterface {
5+
name = 'LowerCaseUserEmail1648740597343';
6+
7+
public async up(queryRunner: QueryRunner): Promise<void> {
8+
let tablePrefix = config.get('database.tablePrefix');
9+
const schema = config.get('database.postgresdb.schema');
10+
if (schema) {
11+
tablePrefix = schema + '.' + tablePrefix;
12+
}
13+
14+
await queryRunner.query(`
15+
UPDATE ${tablePrefix}user
16+
SET email = LOWER(email);
17+
`);
18+
}
19+
20+
public async down(queryRunner: QueryRunner): Promise<void> {}
21+
}

‎packages/cli/src/databases/postgresdb/migrations/index.ts

+2
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { UpdateWorkflowCredentials1630419189837 } from './1630419189837-UpdateWo
1010
import { AddExecutionEntityIndexes1644422880309 } from './1644422880309-AddExecutionEntityIndexes';
1111
import { IncreaseTypeVarcharLimit1646834195327 } from './1646834195327-IncreaseTypeVarcharLimit';
1212
import { CreateUserManagement1646992772331 } from './1646992772331-CreateUserManagement';
13+
import { LowerCaseUserEmail1648740597343 } from './1648740597343-LowerCaseUserEmail';
1314

1415
export const postgresMigrations = [
1516
InitialMigration1587669153312,
@@ -24,4 +25,5 @@ export const postgresMigrations = [
2425
AddExecutionEntityIndexes1644422880309,
2526
IncreaseTypeVarcharLimit1646834195327,
2627
CreateUserManagement1646992772331,
28+
LowerCaseUserEmail1648740597343,
2729
];

‎packages/cli/src/databases/sqlite/migrations/1648740597343-LowerCaseUserEmail.ts

+22
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
import { MigrationInterface, QueryRunner } from 'typeorm';
2+
import config = require('../../../../config');
3+
import { logMigrationEnd, logMigrationStart } from '../../utils/migrationHelpers';
4+
5+
export class LowerCaseUserEmail1648740597343 implements MigrationInterface {
6+
name = 'LowerCaseUserEmail1648740597343';
7+
8+
public async up(queryRunner: QueryRunner): Promise<void> {
9+
logMigrationStart(this.name);
10+
11+
const tablePrefix = config.get('database.tablePrefix');
12+
13+
await queryRunner.query(`
14+
UPDATE "${tablePrefix}user"
15+
SET email = LOWER(email);
16+
`);
17+
18+
logMigrationEnd(this.name);
19+
}
20+
21+
public async down(queryRunner: QueryRunner): Promise<void> {}
22+
}

‎packages/cli/src/databases/sqlite/migrations/index.ts

+2
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { AddWaitColumn1621707690587 } from './1621707690587-AddWaitColumn';
1111
import { UpdateWorkflowCredentials1630330987096 } from './1630330987096-UpdateWorkflowCredentials';
1212
import { AddExecutionEntityIndexes1644421939510 } from './1644421939510-AddExecutionEntityIndexes';
1313
import { CreateUserManagement1646992772331 } from './1646992772331-CreateUserManagement';
14+
import { LowerCaseUserEmail1648740597343 } from './1648740597343-LowerCaseUserEmail';
1415

1516
const sqliteMigrations = [
1617
InitialMigration1588102412422,
@@ -24,6 +25,7 @@ const sqliteMigrations = [
2425
UpdateWorkflowCredentials1630330987096,
2526
AddExecutionEntityIndexes1644421939510,
2627
CreateUserManagement1646992772331,
28+
LowerCaseUserEmail1648740597343,
2729
];
2830

2931
export { sqliteMigrations };

‎packages/cli/src/databases/utils/transformers.ts

+7-2
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,13 @@
22
import { IPersonalizationSurveyAnswers } from '../../Interfaces';
33

44
export const idStringifier = {
5-
from: (value: number): string | number => (value ? value.toString() : value),
6-
to: (value: string): number | string => (value ? Number(value) : value),
5+
from: (value: number): string | number => (typeof value === 'number' ? value.toString() : value),
6+
to: (value: string): number | string => (typeof value === 'string' ? Number(value) : value),
7+
};
8+
9+
export const lowerCaser = {
10+
from: (value: string): string => value,
11+
to: (value: string): string => (typeof value === 'string' ? value.toLowerCase() : value),
712
};
813

914
/**

‎packages/cli/test/integration/auth.endpoints.test.ts

+41-33
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import { hashSync, genSaltSync } from 'bcryptjs';
21
import express from 'express';
32
import validator from 'validator';
43
import { v4 as uuid } from 'uuid';
@@ -60,38 +59,47 @@ afterAll(async () => {
6059
test('POST /login should log user in', async () => {
6160
const authlessAgent = utils.createAgent(app);
6261

63-
const response = await authlessAgent.post('/login').send({
64-
email: TEST_USER.email,
65-
password: TEST_USER.password,
66-
});
67-
68-
expect(response.statusCode).toBe(200);
69-
70-
const {
71-
id,
72-
email,
73-
firstName,
74-
lastName,
75-
password,
76-
personalizationAnswers,
77-
globalRole,
78-
resetPasswordToken,
79-
} = response.body.data;
80-
81-
expect(validator.isUUID(id)).toBe(true);
82-
expect(email).toBe(TEST_USER.email);
83-
expect(firstName).toBe(TEST_USER.firstName);
84-
expect(lastName).toBe(TEST_USER.lastName);
85-
expect(password).toBeUndefined();
86-
expect(personalizationAnswers).toBeNull();
87-
expect(password).toBeUndefined();
88-
expect(resetPasswordToken).toBeUndefined();
89-
expect(globalRole).toBeDefined();
90-
expect(globalRole.name).toBe('owner');
91-
expect(globalRole.scope).toBe('global');
92-
93-
const authToken = utils.getAuthToken(response);
94-
expect(authToken).toBeDefined();
62+
await Promise.all(
63+
[
64+
{
65+
email: TEST_USER.email,
66+
password: TEST_USER.password,
67+
},
68+
{
69+
email: TEST_USER.email.toUpperCase(),
70+
password: TEST_USER.password,
71+
},
72+
].map(async (payload) => {
73+
const response = await authlessAgent.post('/login').send(payload);
74+
75+
expect(response.statusCode).toBe(200);
76+
77+
const {
78+
id,
79+
email,
80+
firstName,
81+
lastName,
82+
password,
83+
personalizationAnswers,
84+
globalRole,
85+
resetPasswordToken,
86+
} = response.body.data;
87+
88+
expect(validator.isUUID(id)).toBe(true);
89+
expect(email).toBe(TEST_USER.email);
90+
expect(firstName).toBe(TEST_USER.firstName);
91+
expect(lastName).toBe(TEST_USER.lastName);
92+
expect(password).toBeUndefined();
93+
expect(personalizationAnswers).toBeNull();
94+
expect(resetPasswordToken).toBeUndefined();
95+
expect(globalRole).toBeDefined();
96+
expect(globalRole.name).toBe('owner');
97+
expect(globalRole.scope).toBe('global');
98+
99+
const authToken = utils.getAuthToken(response);
100+
expect(authToken).toBeDefined();
101+
}),
102+
);
95103
});
96104

97105
test('GET /login should receive logged in user', async () => {

‎packages/cli/test/integration/me.api.test.ts

+7-13
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ describe('Owner shell', () => {
9191
} = response.body.data;
9292

9393
expect(validator.isUUID(id)).toBe(true);
94-
expect(email).toBe(validPayload.email);
94+
expect(email).toBe(validPayload.email.toLowerCase());
9595
expect(firstName).toBe(validPayload.firstName);
9696
expect(lastName).toBe(validPayload.lastName);
9797
expect(personalizationAnswers).toBeNull();
@@ -103,7 +103,7 @@ describe('Owner shell', () => {
103103

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

106-
expect(storedOwnerShell.email).toBe(validPayload.email);
106+
expect(storedOwnerShell.email).toBe(validPayload.email.toLowerCase());
107107
expect(storedOwnerShell.firstName).toBe(validPayload.firstName);
108108
expect(storedOwnerShell.lastName).toBe(validPayload.lastName);
109109
}
@@ -245,7 +245,7 @@ describe('Member', () => {
245245
} = response.body.data;
246246

247247
expect(validator.isUUID(id)).toBe(true);
248-
expect(email).toBe(validPayload.email);
248+
expect(email).toBe(validPayload.email.toLowerCase());
249249
expect(firstName).toBe(validPayload.firstName);
250250
expect(lastName).toBe(validPayload.lastName);
251251
expect(personalizationAnswers).toBeNull();
@@ -257,7 +257,7 @@ describe('Member', () => {
257257

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

260-
expect(storedMember.email).toBe(validPayload.email);
260+
expect(storedMember.email).toBe(validPayload.email.toLowerCase());
261261
expect(storedMember.firstName).toBe(validPayload.firstName);
262262
expect(storedMember.lastName).toBe(validPayload.lastName);
263263
}
@@ -400,7 +400,7 @@ describe('Owner', () => {
400400
} = response.body.data;
401401

402402
expect(validator.isUUID(id)).toBe(true);
403-
expect(email).toBe(validPayload.email);
403+
expect(email).toBe(validPayload.email.toLowerCase());
404404
expect(firstName).toBe(validPayload.firstName);
405405
expect(lastName).toBe(validPayload.lastName);
406406
expect(personalizationAnswers).toBeNull();
@@ -412,19 +412,13 @@ describe('Owner', () => {
412412

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

415-
expect(storedOwner.email).toBe(validPayload.email);
415+
expect(storedOwner.email).toBe(validPayload.email.toLowerCase());
416416
expect(storedOwner.firstName).toBe(validPayload.firstName);
417417
expect(storedOwner.lastName).toBe(validPayload.lastName);
418418
}
419419
});
420420
});
421421

422-
const TEST_USER = {
423-
email: randomEmail(),
424-
firstName: randomName(),
425-
lastName: randomName(),
426-
};
427-
428422
const SURVEY = [
429423
'codingSkill',
430424
'companyIndustry',
@@ -444,7 +438,7 @@ const VALID_PATCH_ME_PAYLOADS = [
444438
password: randomValidPassword(),
445439
},
446440
{
447-
email: randomEmail(),
441+
email: randomEmail().toUpperCase(),
448442
firstName: randomName(),
449443
lastName: randomName(),
450444
password: randomValidPassword(),

‎packages/cli/test/integration/owner.api.test.ts

+29
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,12 @@ beforeAll(async () => {
3030
});
3131

3232
beforeEach(async () => {
33+
jest.mock('../../config');
34+
35+
config.set('userManagement.isInstanceOwnerSetUp', false);
36+
});
37+
38+
afterEach(async () => {
3339
await testDb.truncate(['User'], testDbName);
3440
});
3541

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

97+
test('POST /owner should create owner with lowercased email', async () => {
98+
const ownerShell = await testDb.createUserShell(globalOwnerRole);
99+
const authOwnerAgent = utils.createAgent(app, { auth: true, user: ownerShell });
100+
101+
const newOwnerData = {
102+
email: randomEmail().toUpperCase(),
103+
firstName: randomName(),
104+
lastName: randomName(),
105+
password: randomValidPassword(),
106+
};
107+
108+
const response = await authOwnerAgent.post('/owner').send(newOwnerData);
109+
110+
expect(response.statusCode).toBe(200);
111+
112+
const { id, email } = response.body.data;
113+
114+
expect(email).toBe(newOwnerData.email.toLowerCase());
115+
116+
const storedOwner = await Db.collections.User!.findOneOrFail(id);
117+
expect(storedOwner.email).toBe(newOwnerData.email.toLowerCase());
118+
});
119+
91120
test('POST /owner should fail with invalid inputs', async () => {
92121
const ownerShell = await testDb.createUserShell(globalOwnerRole);
93122
const authOwnerAgent = utils.createAgent(app, { auth: true, user: ownerShell });

0 commit comments

Comments
 (0)