From bd88b079ea998e0e323ad127a29f26b7bd578b3f Mon Sep 17 00:00:00 2001 From: Michel Heusschen <59014050+michelheusschen@users.noreply.github.com> Date: Wed, 10 Jul 2024 13:58:06 +0200 Subject: [PATCH] fix(server): avoid server error for invalid email data type (#10978) * fix(server): avoid server error for invalid email data type * add e2e test * fix e2e --- e2e/src/api/specs/auth.e2e-spec.ts | 6 ++++++ e2e/src/responses.ts | 6 ++++++ server/src/dtos/auth.dto.ts | 3 ++- server/src/dtos/user.dto.spec.ts | 16 ++++++++++++++++ server/src/validation.ts | 9 ++++++--- 5 files changed, 36 insertions(+), 4 deletions(-) diff --git a/e2e/src/api/specs/auth.e2e-spec.ts b/e2e/src/api/specs/auth.e2e-spec.ts index 9174128bb8..e871691ed5 100644 --- a/e2e/src/api/specs/auth.e2e-spec.ts +++ b/e2e/src/api/specs/auth.e2e-spec.ts @@ -100,6 +100,12 @@ describe('/auth/*', () => { expect(status).toBe(400); expect(body).toEqual(errorDto.badRequest()); }); + + it('should reject an invalid email', async () => { + const { status, body } = await request(app).post('/auth/login').send({ email: [], password }); + expect(status).toBe(400); + expect(body).toEqual(errorDto.invalidEmail); + }); } it('should accept a correct password', async () => { diff --git a/e2e/src/responses.ts b/e2e/src/responses.ts index f4e4193f86..80e4f76f4f 100644 --- a/e2e/src/responses.ts +++ b/e2e/src/responses.ts @@ -61,6 +61,12 @@ export const errorDto = { message: 'The server already has an admin', correlationId: expect.any(String), }, + invalidEmail: { + error: 'Bad Request', + statusCode: 400, + message: ['email must be an email'], + correlationId: expect.any(String), + }, }; export const signupResponseDto = { diff --git a/server/src/dtos/auth.dto.ts b/server/src/dtos/auth.dto.ts index 679c944320..6488901fb6 100644 --- a/server/src/dtos/auth.dto.ts +++ b/server/src/dtos/auth.dto.ts @@ -5,6 +5,7 @@ import { APIKeyEntity } from 'src/entities/api-key.entity'; import { SessionEntity } from 'src/entities/session.entity'; import { SharedLinkEntity } from 'src/entities/shared-link.entity'; import { UserEntity } from 'src/entities/user.entity'; +import { toEmail } from 'src/validation'; export enum ImmichCookie { ACCESS_TOKEN = 'immich_access_token', @@ -41,7 +42,7 @@ export class AuthDto { export class LoginCredentialDto { @IsEmail({ require_tld: false }) - @Transform(({ value }) => value?.toLowerCase()) + @Transform(toEmail) @IsNotEmpty() @ApiProperty({ example: 'testuser@email.com' }) email!: string; diff --git a/server/src/dtos/user.dto.spec.ts b/server/src/dtos/user.dto.spec.ts index 683879a310..e6be3b17d1 100644 --- a/server/src/dtos/user.dto.spec.ts +++ b/server/src/dtos/user.dto.spec.ts @@ -38,6 +38,22 @@ describe('create user DTO', () => { expect(errors).toHaveLength(0); }); + it('validates invalid email type', async () => { + let dto = plainToInstance(UserAdminCreateDto, { + email: [], + password: 'some password', + name: 'some name', + }); + expect(await validate(dto)).toHaveLength(1); + + dto = plainToInstance(UserAdminCreateDto, { + email: {}, + password: 'some password', + name: 'some name', + }); + expect(await validate(dto)).toHaveLength(1); + }); + it('should allow emails without a tld', async () => { const someEmail = 'test@test'; diff --git a/server/src/validation.ts b/server/src/validation.ts index 4a8d8db866..b50491aaab 100644 --- a/server/src/validation.ts +++ b/server/src/validation.ts @@ -152,11 +152,14 @@ export function validateCronExpression(expression: string) { return true; } -type IValue = { value: string }; +type IValue = { value: unknown }; -export const toEmail = ({ value }: IValue) => (value ? value.toLowerCase() : value); +export const toEmail = ({ value }: IValue) => (typeof value === 'string' ? value.toLowerCase() : value); -export const toSanitized = ({ value }: IValue) => sanitize((value || '').replaceAll('.', '')); +export const toSanitized = ({ value }: IValue) => { + const input = typeof value === 'string' ? value : ''; + return sanitize(input.replaceAll('.', '')); +}; export const isValidInteger = (value: number, options: { min?: number; max?: number }): value is number => { const { min = Number.MIN_SAFE_INTEGER, max = Number.MAX_SAFE_INTEGER } = options;