diff --git a/server/src/schema/migrations/1779379200000-DeleteMismatchedAssetFaces.ts b/server/src/schema/migrations/1779379200000-DeleteMismatchedAssetFaces.ts new file mode 100644 index 0000000000..d0387c4038 --- /dev/null +++ b/server/src/schema/migrations/1779379200000-DeleteMismatchedAssetFaces.ts @@ -0,0 +1,16 @@ +import { Kysely, sql } from 'kysely'; + +export async function up(db: Kysely): Promise { + // Delete unauthorized cross-owner asset faces + await sql` + DELETE FROM asset_face + USING person, asset + WHERE asset_face."personId" = person.id + AND asset_face."assetId" = asset.id + AND person."ownerId" != asset."ownerId" + `.execute(db); +} + +export async function down(): Promise { + // Not implemented: the deleted rows were unauthorized cross-owner entries +} diff --git a/server/src/services/person.service.spec.ts b/server/src/services/person.service.spec.ts index 5fa7cb87bd..e6a11786af 100644 --- a/server/src/services/person.service.spec.ts +++ b/server/src/services/person.service.spec.ts @@ -452,6 +452,30 @@ describe(PersonService.name, () => { expect(mocks.person.update).not.toHaveBeenCalled(); expect(mocks.job.queueAll).not.toHaveBeenCalled(); }); + + it('should reject creating a face on an asset the user does not own', async () => { + const auth = AuthFactory.create(); + const asset = AssetFactory.create(); + const person = PersonFactory.create({ faceAssetId: null }); + + mocks.access.asset.checkOwnerAccess.mockResolvedValue(new Set()); + mocks.access.person.checkOwnerAccess.mockResolvedValue(new Set([person.id])); + + await expect( + sut.createFace(auth, { + assetId: asset.id, + personId: person.id, + imageHeight: 500, + imageWidth: 400, + x: 10, + y: 20, + width: 100, + height: 110, + }), + ).rejects.toBeInstanceOf(BadRequestException); + + expect(mocks.person.createAssetFace).not.toHaveBeenCalled(); + }); }); describe('createNewFeaturePhoto', () => { diff --git a/server/src/services/person.service.ts b/server/src/services/person.service.ts index de5767ef87..bb17590795 100644 --- a/server/src/services/person.service.ts +++ b/server/src/services/person.service.ts @@ -625,7 +625,7 @@ export class PersonService extends BaseService { // TODO return a asset face response async createFace(auth: AuthDto, dto: AssetFaceCreateDto): Promise { await Promise.all([ - this.requireAccess({ auth, permission: Permission.AssetRead, ids: [dto.assetId] }), + this.requireAccess({ auth, permission: Permission.AssetUpdate, ids: [dto.assetId] }), this.requireAccess({ auth, permission: Permission.PersonRead, ids: [dto.personId] }), ]);