From 434bcec5cc80f603e16d1c4eda985246caac878b Mon Sep 17 00:00:00 2001
From: Michel Heusschen <59014050+michelheusschen@users.noreply.github.com>
Date: Tue, 30 Jul 2024 01:52:04 +0200
Subject: [PATCH] fix(server): correct person birth date across timezones
 (#11369)

* fix(server): correct person birth date across timezones

* fix test

* update e2e tests

* use Optional decorator
---
 e2e/src/api/specs/person.e2e-spec.ts       | 23 ++++++---
 server/src/dtos/person.dto.ts              | 14 +++---
 server/src/entities/person.entity.ts       |  2 +-
 server/src/services/person.service.spec.ts |  6 +--
 server/src/validation.spec.ts              | 56 ++++++++++++++++++++++
 server/src/validation.ts                   | 47 ++++++++++++++++++
 server/test/fixtures/person.stub.ts        |  2 +-
 7 files changed, 132 insertions(+), 18 deletions(-)
 create mode 100644 server/src/validation.spec.ts

diff --git a/e2e/src/api/specs/person.e2e-spec.ts b/e2e/src/api/specs/person.e2e-spec.ts
index a675f54437..d6ccf8265f 100644
--- a/e2e/src/api/specs/person.e2e-spec.ts
+++ b/e2e/src/api/specs/person.e2e-spec.ts
@@ -6,10 +6,19 @@ import request from 'supertest';
 import { beforeAll, beforeEach, describe, expect, it } from 'vitest';
 
 const invalidBirthday = [
-  { birthDate: 'false', response: 'birthDate must be a date string' },
-  { birthDate: '123567', response: 'birthDate must be a date string' },
-  { birthDate: 123_567, response: 'birthDate must be a date string' },
-  { birthDate: new Date(9999, 0, 0).toISOString(), response: ['Birth date cannot be in the future'] },
+  {
+    birthDate: 'false',
+    response: ['birthDate must be a string in the format yyyy-MM-dd', 'Birth date cannot be in the future'],
+  },
+  {
+    birthDate: '123567',
+    response: ['birthDate must be a string in the format yyyy-MM-dd', 'Birth date cannot be in the future'],
+  },
+  {
+    birthDate: 123_567,
+    response: ['birthDate must be a string in the format yyyy-MM-dd', 'Birth date cannot be in the future'],
+  },
+  { birthDate: '9999-01-01', response: ['Birth date cannot be in the future'] },
 ];
 
 describe('/people', () => {
@@ -185,13 +194,13 @@ describe('/people', () => {
         .set('Authorization', `Bearer ${admin.accessToken}`)
         .send({
           name: 'New Person',
-          birthDate: '1990-01-01T05:00:00.000Z',
+          birthDate: '1990-01-01',
         });
       expect(status).toBe(201);
       expect(body).toMatchObject({
         id: expect.any(String),
         name: 'New Person',
-        birthDate: '1990-01-01T05:00:00.000Z',
+        birthDate: '1990-01-01',
       });
     });
   });
@@ -233,7 +242,7 @@ describe('/people', () => {
       const { status, body } = await request(app)
         .put(`/people/${visiblePerson.id}`)
         .set('Authorization', `Bearer ${admin.accessToken}`)
-        .send({ birthDate: '1990-01-01T05:00:00.000Z' });
+        .send({ birthDate: '1990-01-01' });
       expect(status).toBe(200);
       expect(body).toMatchObject({ birthDate: '1990-01-01' });
     });
diff --git a/server/src/dtos/person.dto.ts b/server/src/dtos/person.dto.ts
index 8d60f383a3..573651c3f3 100644
--- a/server/src/dtos/person.dto.ts
+++ b/server/src/dtos/person.dto.ts
@@ -1,11 +1,11 @@
 import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger';
 import { Type } from 'class-transformer';
-import { IsArray, IsInt, IsNotEmpty, IsString, Max, MaxDate, Min, ValidateNested } from 'class-validator';
+import { IsArray, IsInt, IsNotEmpty, IsString, Max, Min, ValidateNested } from 'class-validator';
 import { PropertyLifecycle } from 'src/decorators';
 import { AuthDto } from 'src/dtos/auth.dto';
 import { AssetFaceEntity } from 'src/entities/asset-face.entity';
 import { PersonEntity } from 'src/entities/person.entity';
-import { Optional, ValidateBoolean, ValidateDate, ValidateUUID } from 'src/validation';
+import { IsDateStringFormat, MaxDateString, Optional, ValidateBoolean, ValidateUUID } from 'src/validation';
 
 export class PersonCreateDto {
   /**
@@ -19,9 +19,11 @@ export class PersonCreateDto {
    * Person date of birth.
    * Note: the mobile app cannot currently set the birth date to null.
    */
-  @MaxDate(() => new Date(), { message: 'Birth date cannot be in the future' })
-  @ValidateDate({ optional: true, nullable: true, format: 'date' })
-  birthDate?: Date | null;
+  @ApiProperty({ format: 'date' })
+  @MaxDateString(() => new Date(), { message: 'Birth date cannot be in the future' })
+  @IsDateStringFormat('yyyy-MM-dd')
+  @Optional({ nullable: true })
+  birthDate?: string | null;
 
   /**
    * Person visibility
@@ -84,7 +86,7 @@ export class PersonResponseDto {
   id!: string;
   name!: string;
   @ApiProperty({ format: 'date' })
-  birthDate!: Date | null;
+  birthDate!: string | null;
   thumbnailPath!: string;
   isHidden!: boolean;
   @PropertyLifecycle({ addedAt: 'v1.107.0' })
diff --git a/server/src/entities/person.entity.ts b/server/src/entities/person.entity.ts
index bc60efcd6e..5efbcbfa0b 100644
--- a/server/src/entities/person.entity.ts
+++ b/server/src/entities/person.entity.ts
@@ -33,7 +33,7 @@ export class PersonEntity {
   name!: string;
 
   @Column({ type: 'date', nullable: true })
-  birthDate!: Date | null;
+  birthDate!: string | null;
 
   @Column({ default: '' })
   thumbnailPath!: string;
diff --git a/server/src/services/person.service.spec.ts b/server/src/services/person.service.spec.ts
index 3d195765b6..8a2e88b276 100644
--- a/server/src/services/person.service.spec.ts
+++ b/server/src/services/person.service.spec.ts
@@ -256,15 +256,15 @@ describe(PersonService.name, () => {
       personMock.getAssets.mockResolvedValue([assetStub.image]);
       accessMock.person.checkOwnerAccess.mockResolvedValue(new Set(['person-1']));
 
-      await expect(sut.update(authStub.admin, 'person-1', { birthDate: new Date('1976-06-30') })).resolves.toEqual({
+      await expect(sut.update(authStub.admin, 'person-1', { birthDate: '1976-06-30' })).resolves.toEqual({
         id: 'person-1',
         name: 'Person 1',
-        birthDate: new Date('1976-06-30'),
+        birthDate: '1976-06-30',
         thumbnailPath: '/path/to/thumbnail.jpg',
         isHidden: false,
         updatedAt: expect.any(Date),
       });
-      expect(personMock.update).toHaveBeenCalledWith({ id: 'person-1', birthDate: new Date('1976-06-30') });
+      expect(personMock.update).toHaveBeenCalledWith({ id: 'person-1', birthDate: '1976-06-30' });
       expect(jobMock.queue).not.toHaveBeenCalled();
       expect(jobMock.queueAll).not.toHaveBeenCalled();
       expect(accessMock.person.checkOwnerAccess).toHaveBeenCalledWith(authStub.admin.user.id, new Set(['person-1']));
diff --git a/server/src/validation.spec.ts b/server/src/validation.spec.ts
new file mode 100644
index 0000000000..d470918107
--- /dev/null
+++ b/server/src/validation.spec.ts
@@ -0,0 +1,56 @@
+import { plainToInstance } from 'class-transformer';
+import { validate } from 'class-validator';
+import { IsDateStringFormat, MaxDateString } from 'src/validation';
+
+describe('Validation', () => {
+  describe('MaxDateString', () => {
+    const maxDate = new Date(2000, 0, 1);
+
+    class MyDto {
+      @MaxDateString(maxDate)
+      date!: string;
+    }
+
+    it('passes when date is before maxDate', async () => {
+      const dto = plainToInstance(MyDto, { date: '1999-12-31' });
+      await expect(validate(dto)).resolves.toHaveLength(0);
+    });
+
+    it('passes when date is equal to maxDate', async () => {
+      const dto = plainToInstance(MyDto, { date: '2000-01-01' });
+      await expect(validate(dto)).resolves.toHaveLength(0);
+    });
+
+    it('fails when date is after maxDate', async () => {
+      const dto = plainToInstance(MyDto, { date: '2010-01-01' });
+      await expect(validate(dto)).resolves.toHaveLength(1);
+    });
+  });
+
+  describe('IsDateStringFormat', () => {
+    class MyDto {
+      @IsDateStringFormat('yyyy-MM-dd')
+      date!: string;
+    }
+
+    it('passes when date is valid', async () => {
+      const dto = plainToInstance(MyDto, { date: '1999-12-31' });
+      await expect(validate(dto)).resolves.toHaveLength(0);
+    });
+
+    it('fails when date has invalid format', async () => {
+      const dto = plainToInstance(MyDto, { date: '2000-01-01T00:00:00Z' });
+      await expect(validate(dto)).resolves.toHaveLength(1);
+    });
+
+    it('fails when empty string', async () => {
+      const dto = plainToInstance(MyDto, { date: '' });
+      await expect(validate(dto)).resolves.toHaveLength(1);
+    });
+
+    it('fails when undefined', async () => {
+      const dto = plainToInstance(MyDto, {});
+      await expect(validate(dto)).resolves.toHaveLength(1);
+    });
+  });
+});
diff --git a/server/src/validation.ts b/server/src/validation.ts
index b50491aaab..063f2150a5 100644
--- a/server/src/validation.ts
+++ b/server/src/validation.ts
@@ -16,11 +16,15 @@ import {
   IsOptional,
   IsString,
   IsUUID,
+  ValidateBy,
   ValidateIf,
   ValidationOptions,
+  buildMessage,
   isDateString,
+  maxDate,
 } from 'class-validator';
 import { CronJob } from 'cron';
+import { DateTime } from 'luxon';
 import sanitize from 'sanitize-filename';
 
 @Injectable()
@@ -165,3 +169,46 @@ export const isValidInteger = (value: number, options: { min?: number; max?: num
   const { min = Number.MIN_SAFE_INTEGER, max = Number.MAX_SAFE_INTEGER } = options;
   return Number.isInteger(value) && value >= min && value <= max;
 };
+
+export function isDateStringFormat(value: unknown, format: string) {
+  if (typeof value !== 'string') {
+    return false;
+  }
+  return DateTime.fromFormat(value, format, { zone: 'utc' }).isValid;
+}
+
+export function IsDateStringFormat(format: string, validationOptions?: ValidationOptions) {
+  return ValidateBy(
+    {
+      name: 'isDateStringFormat',
+      constraints: [format],
+      validator: {
+        validate(value: unknown) {
+          return isDateStringFormat(value, format);
+        },
+        defaultMessage: () => `$property must be a string in the format ${format}`,
+      },
+    },
+    validationOptions,
+  );
+}
+
+export function MaxDateString(date: Date | (() => Date), validationOptions?: ValidationOptions): PropertyDecorator {
+  return ValidateBy(
+    {
+      name: 'maxDateString',
+      constraints: [date],
+      validator: {
+        validate: (value, args) => {
+          const date = DateTime.fromISO(value, { zone: 'utc' }).toJSDate();
+          return maxDate(date, args?.constraints[0]);
+        },
+        defaultMessage: buildMessage(
+          (eachPrefix) => 'maximal allowed date for ' + eachPrefix + '$property is $constraint1',
+          validationOptions,
+        ),
+      },
+    },
+    validationOptions,
+  );
+}
diff --git a/server/test/fixtures/person.stub.ts b/server/test/fixtures/person.stub.ts
index 5e5a2214ed..3584d0486e 100644
--- a/server/test/fixtures/person.stub.ts
+++ b/server/test/fixtures/person.stub.ts
@@ -65,7 +65,7 @@ export const personStub = {
     ownerId: userStub.admin.id,
     owner: userStub.admin,
     name: 'Person 1',
-    birthDate: new Date('1976-06-30'),
+    birthDate: '1976-06-30',
     thumbnailPath: '/path/to/thumbnail.jpg',
     faces: [],
     faceAssetId: null,