From 14b4cf64abf8299ee1a2481487cb1c2e2297f83d Mon Sep 17 00:00:00 2001 From: portableDD <65081431+portableDD@users.noreply.github.com> Date: Fri, 16 Oct 2026 19:35:37 +0100 Subject: [PATCH] fix(users): normalise emails and remove demo-user fallback --- backend/src/auth/auth.service.spec.ts | 24 +++++++++++ backend/src/auth/dto/login.dto.ts | 3 ++ backend/src/auth/dto/register.dto.ts | 3 ++ .../1760000000000-NormaliseUserEmails.ts | 40 +++++++++++++++++++ backend/src/users/users.controller.spec.ts | 29 ++++++++++++++ backend/src/users/users.controller.ts | 13 ++---- backend/src/users/users.service.ts | 7 +++- .../src/users/utils/normalize-email.spec.ts | 12 ++++++ backend/src/users/utils/normalize-email.ts | 12 ++++++ 9 files changed, 132 insertions(+), 11 deletions(-) create mode 100644 backend/src/migrations/1760000000000-NormaliseUserEmails.ts create mode 100644 backend/src/users/users.controller.spec.ts create mode 100644 backend/src/users/utils/normalize-email.spec.ts create mode 100644 backend/src/users/utils/normalize-email.ts diff --git a/backend/src/auth/auth.service.spec.ts b/backend/src/auth/auth.service.spec.ts index 35bd8ea3..01229d53 100644 --- a/backend/src/auth/auth.service.spec.ts +++ b/backend/src/auth/auth.service.spec.ts @@ -52,6 +52,30 @@ describe('AuthService', () => { jwtService = module.get(JwtService); }); + describe('email normalisation', () => { + it('registers with a lower-cased, trimmed email via UsersService', async () => { + const { RegisterDto } = await import('./dto/register.dto'); + const { plainToInstance } = await import('class-transformer'); + const dto = plainToInstance(RegisterDto, { + firstName: 'J', + lastName: 'D', + email: ' Jane@Corp.COM ', + password: 'Password123!', + }); + expect(dto.email).toBe('jane@corp.com'); + }); + + it('normalises the login email before validation', async () => { + const { LoginDto } = await import('./dto/login.dto'); + const { plainToInstance } = await import('class-transformer'); + const dto = plainToInstance(LoginDto, { + email: 'JANE@corp.com', + password: 'x', + }); + expect(dto.email).toBe('jane@corp.com'); + }); + }); + describe('register', () => { it('should throw ConflictException when email already exists', async () => { jest diff --git a/backend/src/auth/dto/login.dto.ts b/backend/src/auth/dto/login.dto.ts index 7b1fd4eb..8c11ec8e 100644 --- a/backend/src/auth/dto/login.dto.ts +++ b/backend/src/auth/dto/login.dto.ts @@ -1,8 +1,11 @@ import { ApiProperty } from '@nestjs/swagger'; +import { Transform } from 'class-transformer'; import { IsEmail, IsNotEmpty, IsString } from 'class-validator'; +import { normalizeEmailTransform } from '../../users/utils/normalize-email'; export class LoginDto { @ApiProperty({ example: 'user@example.com' }) + @Transform(normalizeEmailTransform) @IsEmail() @IsNotEmpty() email: string; diff --git a/backend/src/auth/dto/register.dto.ts b/backend/src/auth/dto/register.dto.ts index 68909471..5af01b52 100644 --- a/backend/src/auth/dto/register.dto.ts +++ b/backend/src/auth/dto/register.dto.ts @@ -1,5 +1,7 @@ import { ApiProperty } from '@nestjs/swagger'; +import { Transform } from 'class-transformer'; import { IsEmail, IsNotEmpty, IsString, MinLength } from 'class-validator'; +import { normalizeEmailTransform } from '../../users/utils/normalize-email'; export class RegisterDto { @ApiProperty({ example: 'John' }) @@ -13,6 +15,7 @@ export class RegisterDto { lastName: string; @ApiProperty({ example: 'user@example.com' }) + @Transform(normalizeEmailTransform) @IsEmail() @IsNotEmpty() email: string; diff --git a/backend/src/migrations/1760000000000-NormaliseUserEmails.ts b/backend/src/migrations/1760000000000-NormaliseUserEmails.ts new file mode 100644 index 00000000..27a89063 --- /dev/null +++ b/backend/src/migrations/1760000000000-NormaliseUserEmails.ts @@ -0,0 +1,40 @@ +import { MigrationInterface, QueryRunner } from 'typeorm'; + +/** + * Makes user emails case-insensitively unique (#1572). + * + * Existing rows are trimmed and lower-cased. If two accounts differ only by + * case the migration stops and lists them: merging accounts is a data decision + * that must not be made silently. + */ +export class NormaliseUserEmails1760000000000 implements MigrationInterface { + name = 'NormaliseUserEmails1760000000000'; + + public async up(queryRunner: QueryRunner): Promise { + if (!(await queryRunner.hasTable('users'))) return; + + const clashes: { email: string; count: string }[] = await queryRunner.query( + `SELECT lower(btrim(email)) AS email, count(*) AS count + FROM users GROUP BY lower(btrim(email)) HAVING count(*) > 1`, + ); + if (clashes.length) { + throw new Error( + `Cannot normalise user emails; resolve these duplicates first: ${clashes + .map((c) => `${c.email} (x${c.count})`) + .join(', ')}`, + ); + } + + await queryRunner.query( + `UPDATE users SET email = lower(btrim(email)) WHERE email <> lower(btrim(email))`, + ); + await queryRunner.query( + `CREATE UNIQUE INDEX IF NOT EXISTS "IDX_users_email_lower" ON users (lower(email))`, + ); + } + + public async down(queryRunner: QueryRunner): Promise { + // Lower-casing is not reversible; only the new index is dropped. + await queryRunner.query(`DROP INDEX IF EXISTS "IDX_users_email_lower"`); + } +} diff --git a/backend/src/users/users.controller.spec.ts b/backend/src/users/users.controller.spec.ts new file mode 100644 index 00000000..7c09fb59 --- /dev/null +++ b/backend/src/users/users.controller.spec.ts @@ -0,0 +1,29 @@ +import { UnauthorizedException } from '@nestjs/common'; +import { UsersController } from './users.controller'; +import { UsersService } from './users.service'; + +describe('UsersController.getProfile', () => { + const user = { id: 'u-1', email: 'a@b.com' }; + const service = { + findById: jest.fn().mockResolvedValue(user), + sanitize: jest.fn((u) => u), + } as unknown as UsersService; + const controller = new UsersController(service); + + beforeEach(() => jest.clearAllMocks()); + + it('returns the sanitised profile for the authenticated user', async () => { + await expect(controller.getProfile('u-1')).resolves.toEqual(user); + expect(service.findById).toHaveBeenCalledWith('u-1'); + }); + + it.each([undefined, ''])( + 'throws 401 instead of inventing an identity when the user id is %p', + async (id) => { + await expect(controller.getProfile(id)).rejects.toBeInstanceOf( + UnauthorizedException, + ); + expect(service.findById).not.toHaveBeenCalled(); + }, + ); +}); diff --git a/backend/src/users/users.controller.ts b/backend/src/users/users.controller.ts index d92596c0..5f12c17c 100644 --- a/backend/src/users/users.controller.ts +++ b/backend/src/users/users.controller.ts @@ -7,6 +7,7 @@ import { Query, Req, UseGuards, + UnauthorizedException, } from '@nestjs/common'; import { ApiTags, @@ -17,6 +18,7 @@ import { import { UsersService } from './users.service'; import { UserRole } from './entities/user.entity'; import { JwtAuthGuard } from '../auth/guards/jwt-auth.guard'; +import { GetUser } from '../auth/decorators/get-user.decorator'; import { UpdateProfileDto } from './dto/update-profile.dto'; @ApiTags('users') @@ -41,16 +43,9 @@ export class UsersController { @UseGuards(JwtAuthGuard) @ApiOperation({ summary: 'Get current user profile' }) @ApiResponse({ status: 200, description: 'Current user profile' }) - async getProfile(@Req() req: any) { - const userId = req.user?.id || req.user?.sub; + async getProfile(@GetUser('id') userId?: string) { if (!userId) { - return { - id: 'demo-user-id', - email: 'user@example.com', - role: UserRole.ADMIN, - firstName: 'Demo', - lastName: 'User', - }; + throw new UnauthorizedException('Authentication required'); } const user = await this.usersService.findById(userId); return this.usersService.sanitize(user); diff --git a/backend/src/users/users.service.ts b/backend/src/users/users.service.ts index ae2b28f8..0c010985 100644 --- a/backend/src/users/users.service.ts +++ b/backend/src/users/users.service.ts @@ -12,6 +12,7 @@ import * as bcrypt from 'bcryptjs'; import { User, UserRole } from './entities/user.entity'; import { UpdateProfileDto } from './dto/update-profile.dto'; import { AuditLogsService } from '../audit-logs/audit-logs.service'; +import { normalizeEmail } from './utils/normalize-email'; @Injectable() export class UsersService { @@ -61,7 +62,9 @@ export class UsersService { } async findByEmail(email: string): Promise { - return this.userRepository.findOne({ where: { email } }); + return this.userRepository.findOne({ + where: { email: normalizeEmail(email) }, + }); } async create(dto: { @@ -76,7 +79,7 @@ export class UsersService { const passwordHash = await bcrypt.hash(dto.password, 10); const user = this.userRepository.create({ - email: dto.email, + email: normalizeEmail(dto.email), passwordHash, firstName: dto.firstName, lastName: dto.lastName, diff --git a/backend/src/users/utils/normalize-email.spec.ts b/backend/src/users/utils/normalize-email.spec.ts new file mode 100644 index 00000000..771cfa5d --- /dev/null +++ b/backend/src/users/utils/normalize-email.spec.ts @@ -0,0 +1,12 @@ +import { normalizeEmail, normalizeEmailTransform } from './normalize-email'; + +describe('normalizeEmail', () => { + it('trims and lower-cases', () => { + expect(normalizeEmail(' Jane@Corp.COM ')).toBe('jane@corp.com'); + }); + + it('leaves non-strings to the validator', () => { + expect(normalizeEmailTransform({ value: 42 })).toBe(42); + expect(normalizeEmailTransform({ value: undefined })).toBeUndefined(); + }); +}); diff --git a/backend/src/users/utils/normalize-email.ts b/backend/src/users/utils/normalize-email.ts new file mode 100644 index 00000000..3a561eb4 --- /dev/null +++ b/backend/src/users/utils/normalize-email.ts @@ -0,0 +1,12 @@ +/** Canonical form of an email address: trimmed and lower-cased. */ +export function normalizeEmail(email: string): string { + return email.trim().toLowerCase(); +} + +/** + * `class-transformer` helper for DTO fields. Non-strings pass through untouched + * so `@IsEmail()` still reports them. + */ +export function normalizeEmailTransform({ value }: { value: unknown }): unknown { + return typeof value === 'string' ? normalizeEmail(value) : value; +}