From 17076357433c4365d4c394059f5d2dcb05f1f781 Mon Sep 17 00:00:00 2001 From: bo0tzz Date: Tue, 27 Oct 2026 18:11:18 +0100 Subject: [PATCH] fix(server): require update access on the motion video when linking a live photo (#32606) * fix(server): require update access on the motion video when linking a live photo * chore: allow linking with existing shared link assets --------- Co-authored-by: Jason Rasmussen (cherry picked from commit a7afb4c9e9d2f099f9204a41f62e17b54fca3b17) --- e2e/src/specs/server/api/asset.e2e-spec.ts | 2 +- .../src/services/asset-media.service.spec.ts | 59 +++++++++++++++++++ server/src/services/asset-media.service.ts | 13 ++++ server/src/services/asset.service.spec.ts | 9 ++- server/src/services/asset.service.ts | 1 + server/src/utils/asset.util.ts | 5 +- 6 files changed, 82 insertions(+), 7 deletions(-) diff --git a/e2e/src/specs/server/api/asset.e2e-spec.ts b/e2e/src/specs/server/api/asset.e2e-spec.ts index 6eb0dc412..7a1c99f82 100644 --- a/e2e/src/specs/server/api/asset.e2e-spec.ts +++ b/e2e/src/specs/server/api/asset.e2e-spec.ts @@ -355,7 +355,7 @@ describe('/asset', () => { .set('Authorization', `Bearer ${user1.accessToken}`) .send({ livePhotoVideoId: asset.id }); - expect(body).toEqual(errorDto.badRequest('Live photo video does not belong to the user')); + expect(body).toEqual(errorDto.badRequest('Not found or no asset.update access')); expect(status).toEqual(400); }); diff --git a/server/src/services/asset-media.service.spec.ts b/server/src/services/asset-media.service.spec.ts index 729b64325..8cf06b860 100644 --- a/server/src/services/asset-media.service.spec.ts +++ b/server/src/services/asset-media.service.spec.ts @@ -422,6 +422,7 @@ describe(AssetMediaService.name, () => { .owner(authStub.user1.user) .build(); const asset = AssetFactory.create({ livePhotoVideoId: motionAsset.id }); + mocks.access.asset.checkOwnerAccess.mockResolvedValue(new Set([motionAsset.id])); mocks.asset.getById.mockResolvedValueOnce(getForAsset(motionAsset)); mocks.asset.create.mockResolvedValueOnce(asset); @@ -439,6 +440,7 @@ describe(AssetMediaService.name, () => { it('should hide the linked motion asset', async () => { const motionAsset = AssetFactory.from({ type: AssetType.Video }).owner(authStub.user1.user).build(); const asset = AssetFactory.create(); + mocks.access.asset.checkOwnerAccess.mockResolvedValue(new Set([motionAsset.id])); mocks.asset.getById.mockResolvedValueOnce(getForAsset(motionAsset)); mocks.asset.create.mockResolvedValueOnce(asset); @@ -456,6 +458,63 @@ describe(AssetMediaService.name, () => { }); }); + it('should link a live photo video that is already in the shared link', async () => { + const motionAsset = AssetFactory.from({ type: AssetType.Video }).owner(authStub.user1.user).build(); + const asset = AssetFactory.create({ livePhotoVideoId: motionAsset.id }); + const auth = AuthFactory.from(authStub.user1.user).sharedLink({ allowUpload: true }).build(); + mocks.access.asset.checkSharedLinkAccess.mockResolvedValue(new Set([motionAsset.id])); + mocks.access.asset.checkOwnerAccess.mockResolvedValue(new Set([motionAsset.id])); + mocks.asset.getById.mockResolvedValueOnce(getForAsset(motionAsset)); + mocks.asset.create.mockResolvedValueOnce(asset); + mocks.sharedLink.addAssets.mockResolvedValue([]); + + await expect( + sut.uploadAsset(auth, { ...createDto, livePhotoVideoId: motionAsset.id }, fileStub.livePhotoStill), + ).resolves.toEqual({ + status: AssetMediaStatus.CREATED, + id: asset.id, + }); + + expect(mocks.access.asset.checkSharedLinkAccess).toHaveBeenCalledWith( + auth.sharedLink!.id, + new Set([motionAsset.id]), + ); + expect(mocks.access.asset.checkOwnerAccess).toHaveBeenCalledWith(auth.user.id, new Set([motionAsset.id]), false); + expect(mocks.asset.getById).toHaveBeenCalledWith(motionAsset.id); + expect(mocks.asset.update).toHaveBeenCalledWith({ id: motionAsset.id, visibility: AssetVisibility.Hidden }); + expect(mocks.asset.create).toHaveBeenCalledWith(expect.objectContaining({ livePhotoVideoId: motionAsset.id })); + }); + + it('should not link a live photo video that is not in the shared link', async () => { + const motionAsset = AssetFactory.from({ type: AssetType.Video }).owner(authStub.user1.user).build(); + const auth = AuthFactory.from(authStub.user1.user).sharedLink({ allowUpload: true }).build(); + mocks.access.asset.checkSharedLinkAccess.mockResolvedValue(new Set()); + mocks.access.asset.checkOwnerAccess.mockResolvedValue(new Set([motionAsset.id])); + + await expect( + sut.uploadAsset(auth, { ...createDto, livePhotoVideoId: motionAsset.id }, fileStub.livePhotoStill), + ).rejects.toThrow('Not found or no asset.update permission'); + + expect(mocks.asset.getById).not.toHaveBeenCalled(); + expect(mocks.asset.update).not.toHaveBeenCalled(); + expect(mocks.asset.create).not.toHaveBeenCalled(); + }); + + it('should not link a live photo video in the shared link that is owned by another user', async () => { + const motionAsset = AssetFactory.from({ type: AssetType.Video }).build(); + const auth = AuthFactory.from(authStub.user1.user).sharedLink({ allowUpload: true }).build(); + mocks.access.asset.checkSharedLinkAccess.mockResolvedValue(new Set([motionAsset.id])); + mocks.access.asset.checkOwnerAccess.mockResolvedValue(new Set()); + + await expect( + sut.uploadAsset(auth, { ...createDto, livePhotoVideoId: motionAsset.id }, fileStub.livePhotoStill), + ).rejects.toThrow('Not found or no asset.update permission'); + + expect(mocks.asset.getById).not.toHaveBeenCalled(); + expect(mocks.asset.update).not.toHaveBeenCalled(); + expect(mocks.asset.create).not.toHaveBeenCalled(); + }); + it('should handle a sidecar file', async () => { const asset = AssetFactory.from().file({ type: AssetFileType.Sidecar }).build(); mocks.asset.getById.mockResolvedValueOnce(getForAsset(asset)); diff --git a/server/src/services/asset-media.service.ts b/server/src/services/asset-media.service.ts index 5328d98f4..c17e682e2 100644 --- a/server/src/services/asset-media.service.ts +++ b/server/src/services/asset-media.service.ts @@ -142,6 +142,19 @@ export class AssetMediaService extends BaseService { this.requireQuota(auth, file.size); if (dto.livePhotoVideoId) { + if (auth.sharedLink) { + const ids = new Set([dto.livePhotoVideoId]); + const [assetIds1, assetIds2] = await Promise.all([ + this.accessRepository.asset.checkSharedLinkAccess(auth.sharedLink.id, ids), + this.accessRepository.asset.checkOwnerAccess(auth.user.id, ids, false), + ]); + + if (!assetIds1.intersection(assetIds2).has(dto.livePhotoVideoId)) { + throw new BadRequestException(`Not found or no ${Permission.AssetUpdate} permission`); + } + } else { + await this.requireAccess({ auth, permission: Permission.AssetUpdate, ids: [dto.livePhotoVideoId] }); + } await onBeforeLink( { asset: this.assetRepository, event: this.eventRepository }, { userId: auth.user.id, livePhotoVideoId: dto.livePhotoVideoId }, diff --git a/server/src/services/asset.service.spec.ts b/server/src/services/asset.service.spec.ts index 43fb6cec9..ba984ffba 100755 --- a/server/src/services/asset.service.spec.ts +++ b/server/src/services/asset.service.spec.ts @@ -262,7 +262,9 @@ describe(AssetService.name, () => { const auth = AuthFactory.create(); const motionAsset = AssetFactory.from().owner(auth.user).build(); const asset = AssetFactory.create(); - mocks.access.asset.checkOwnerAccess.mockResolvedValue(new Set([asset.id])); + mocks.access.asset.checkOwnerAccess + .mockResolvedValueOnce(new Set([asset.id])) + .mockResolvedValueOnce(new Set([motionAsset.id])); mocks.asset.getById.mockResolvedValue(getForAsset(asset)); await expect( @@ -315,7 +317,10 @@ describe(AssetService.name, () => { it('should link a live video', async () => { const motionAsset = AssetFactory.create({ type: AssetType.Video, visibility: AssetVisibility.Timeline }); const stillAsset = AssetFactory.create(); - mocks.access.asset.checkOwnerAccess.mockResolvedValue(new Set([stillAsset.id])); + mocks.access.asset.checkOwnerAccess + .mockResolvedValueOnce(new Set([stillAsset.id])) + .mockResolvedValueOnce(new Set([motionAsset.id])) + .mockResolvedValue(new Set([stillAsset.id])); mocks.asset.getById.mockResolvedValueOnce(getForAsset(motionAsset)); mocks.asset.getById.mockResolvedValueOnce(getForAsset(stillAsset)); mocks.asset.update.mockResolvedValue(getForAsset(stillAsset)); diff --git a/server/src/services/asset.service.ts b/server/src/services/asset.service.ts index 60ea58904..022bfeee6 100644 --- a/server/src/services/asset.service.ts +++ b/server/src/services/asset.service.ts @@ -91,6 +91,7 @@ export class AssetService extends BaseService { let previousMotion: { id: string } | null = null; if (rest.livePhotoVideoId) { + await this.requireAccess({ auth, permission: Permission.AssetUpdate, ids: [rest.livePhotoVideoId] }); await onBeforeLink(repos, { userId: auth.user.id, livePhotoVideoId: rest.livePhotoVideoId }); } else if (rest.livePhotoVideoId === null) { const asset = await this.findOrFail(id); diff --git a/server/src/utils/asset.util.ts b/server/src/utils/asset.util.ts index ed63df38e..80b025ad2 100644 --- a/server/src/utils/asset.util.ts +++ b/server/src/utils/asset.util.ts @@ -153,11 +153,8 @@ export const onBeforeLink = async ( if (motionAsset.type !== AssetType.Video) { throw new BadRequestException('Live photo video must be a video'); } - if (motionAsset.ownerId !== userId) { - throw new BadRequestException('Live photo video does not belong to the user'); - } - if (!motionAsset || motionAsset.visibility !== AssetVisibility.Timeline) { + if (motionAsset.visibility !== AssetVisibility.Timeline) { return; }