From 284edd97d6b5cac2fda8415460c9684f4a29cf96 Mon Sep 17 00:00:00 2001 From: Jason Rasmussen Date: Wed, 7 Jun 2023 00:34:42 -0400 Subject: [PATCH] refactor(server): shared link asset access check (#2680) --- .../src/api-v1/asset/asset.service.spec.ts | 6 ++-- .../immich/src/api-v1/asset/asset.service.ts | 6 ++-- .../domain/src/access/access.repository.ts | 1 + .../src/shared-link/shared-link.core.ts | 4 --- .../src/shared-link/shared-link.repository.ts | 1 - .../domain/test/access.repository.mock.ts | 1 + .../test/shared-link.repository.mock.ts | 1 - .../src/repositories/access.repository.ts | 32 +++++++++++++++++-- .../repositories/shared-link.repository.ts | 25 --------------- 9 files changed, 39 insertions(+), 38 deletions(-) diff --git a/server/apps/immich/src/api-v1/asset/asset.service.spec.ts b/server/apps/immich/src/api-v1/asset/asset.service.spec.ts index 9642f1c5f3..b9210d6996 100644 --- a/server/apps/immich/src/api-v1/asset/asset.service.spec.ts +++ b/server/apps/immich/src/api-v1/asset/asset.service.spec.ts @@ -225,7 +225,7 @@ describe('AssetService', () => { assetRepositoryMock.getById.mockResolvedValue(asset1); sharedLinkRepositoryMock.get.mockResolvedValue(null); - sharedLinkRepositoryMock.hasAssetAccess.mockResolvedValue(true); + accessMock.hasSharedLinkAssetAccess.mockResolvedValue(true); await expect(sut.addAssetsToSharedLink(authDto, dto)).rejects.toBeInstanceOf(BadRequestException); @@ -242,7 +242,7 @@ describe('AssetService', () => { assetRepositoryMock.getById.mockResolvedValue(asset1); sharedLinkRepositoryMock.get.mockResolvedValue(sharedLinkStub.valid); - sharedLinkRepositoryMock.hasAssetAccess.mockResolvedValue(true); + accessMock.hasSharedLinkAssetAccess.mockResolvedValue(true); sharedLinkRepositoryMock.update.mockResolvedValue(sharedLinkStub.valid); await expect(sut.addAssetsToSharedLink(authDto, dto)).resolves.toEqual(sharedLinkResponseStub.valid); @@ -260,7 +260,7 @@ describe('AssetService', () => { assetRepositoryMock.getById.mockResolvedValue(asset1); sharedLinkRepositoryMock.get.mockResolvedValue(sharedLinkStub.valid); - sharedLinkRepositoryMock.hasAssetAccess.mockResolvedValue(true); + accessMock.hasSharedLinkAssetAccess.mockResolvedValue(true); sharedLinkRepositoryMock.update.mockResolvedValue(sharedLinkStub.valid); await expect(sut.removeAssetsFromSharedLink(authDto, dto)).resolves.toEqual(sharedLinkResponseStub.valid); diff --git a/server/apps/immich/src/api-v1/asset/asset.service.ts b/server/apps/immich/src/api-v1/asset/asset.service.ts index 6ec4fc7039..15c7a172e6 100644 --- a/server/apps/immich/src/api-v1/asset/asset.service.ts +++ b/server/apps/immich/src/api-v1/asset/asset.service.ts @@ -564,10 +564,12 @@ export class AssetService { } private async checkAssetsAccess(authUser: AuthUserDto, assetIds: string[], mustBeOwner = false) { + const sharedLinkId = authUser.sharedLinkId; + for (const assetId of assetIds) { // Step 1: Check if asset is part of a public shared - if (authUser.sharedLinkId) { - const canAccess = await this.shareCore.hasAssetAccess(authUser.sharedLinkId, assetId); + if (sharedLinkId) { + const canAccess = await this.accessRepository.hasSharedLinkAssetAccess(sharedLinkId, assetId); if (canAccess) { continue; } diff --git a/server/libs/domain/src/access/access.repository.ts b/server/libs/domain/src/access/access.repository.ts index 7af2255d4d..06466e5cd2 100644 --- a/server/libs/domain/src/access/access.repository.ts +++ b/server/libs/domain/src/access/access.repository.ts @@ -3,4 +3,5 @@ export const IAccessRepository = 'IAccessRepository'; export interface IAccessRepository { hasPartnerAccess(userId: string, partnerId: string): Promise; hasPartnerAssetAccess(userId: string, assetId: string): Promise; + hasSharedLinkAssetAccess(userId: string, assetId: string): Promise; } diff --git a/server/libs/domain/src/shared-link/shared-link.core.ts b/server/libs/domain/src/shared-link/shared-link.core.ts index 580a0530f7..c64256d795 100644 --- a/server/libs/domain/src/shared-link/shared-link.core.ts +++ b/server/libs/domain/src/shared-link/shared-link.core.ts @@ -47,10 +47,6 @@ export class SharedLinkCore { return this.repository.update({ ...link, assets: newAssets }); } - async hasAssetAccess(id: string, assetId: string): Promise { - return this.repository.hasAssetAccess(id, assetId); - } - checkDownloadAccess(user: AuthUserDto) { if (user.isPublicUser && !user.isAllowDownload) { throw new ForbiddenException(); diff --git a/server/libs/domain/src/shared-link/shared-link.repository.ts b/server/libs/domain/src/shared-link/shared-link.repository.ts index 467ac29c92..7b6dcb6fef 100644 --- a/server/libs/domain/src/shared-link/shared-link.repository.ts +++ b/server/libs/domain/src/shared-link/shared-link.repository.ts @@ -9,5 +9,4 @@ export interface ISharedLinkRepository { create(entity: Omit): Promise; update(entity: Partial): Promise; remove(entity: SharedLinkEntity): Promise; - hasAssetAccess(id: string, assetId: string): Promise; } diff --git a/server/libs/domain/test/access.repository.mock.ts b/server/libs/domain/test/access.repository.mock.ts index 5f3af78a64..75a91fc3cb 100644 --- a/server/libs/domain/test/access.repository.mock.ts +++ b/server/libs/domain/test/access.repository.mock.ts @@ -4,5 +4,6 @@ export const newAccessRepositoryMock = (): jest.Mocked => { return { hasPartnerAccess: jest.fn(), hasPartnerAssetAccess: jest.fn(), + hasSharedLinkAssetAccess: jest.fn(), }; }; diff --git a/server/libs/domain/test/shared-link.repository.mock.ts b/server/libs/domain/test/shared-link.repository.mock.ts index e5bdbb828e..dae72d9c68 100644 --- a/server/libs/domain/test/shared-link.repository.mock.ts +++ b/server/libs/domain/test/shared-link.repository.mock.ts @@ -8,6 +8,5 @@ export const newSharedLinkRepositoryMock = (): jest.Mocked) {} + constructor( + @InjectRepository(PartnerEntity) private partnerRepository: Repository, + @InjectRepository(SharedLinkEntity) private sharedLinkRepository: Repository, + ) {} hasPartnerAccess(userId: string, partnerId: string): Promise { return this.partnerRepository.exist({ @@ -35,4 +38,29 @@ export class AccessRepository implements IAccessRepository { }, }); } + + async hasSharedLinkAssetAccess(sharedLinkId: string, assetId: string): Promise { + return ( + // album asset + (await this.sharedLinkRepository.exist({ + where: { + id: sharedLinkId, + album: { + assets: { + id: assetId, + }, + }, + }, + })) || + // individual asset + (await this.sharedLinkRepository.exist({ + where: { + id: sharedLinkId, + assets: { + id: assetId, + }, + }, + })) + ); + } } diff --git a/server/libs/infra/src/repositories/shared-link.repository.ts b/server/libs/infra/src/repositories/shared-link.repository.ts index 50db03beee..a295a42fb4 100644 --- a/server/libs/infra/src/repositories/shared-link.repository.ts +++ b/server/libs/infra/src/repositories/shared-link.repository.ts @@ -86,31 +86,6 @@ export class SharedLinkRepository implements ISharedLinkRepository { await this.repository.remove(entity); } - async hasAssetAccess(id: string, assetId: string): Promise { - return ( - // album asset - (await this.repository.exist({ - where: { - id, - album: { - assets: { - id: assetId, - }, - }, - }, - })) || - // individual asset - (await this.repository.exist({ - where: { - id, - assets: { - id: assetId, - }, - }, - })) - ); - } - private async save(entity: Partial): Promise { await this.repository.save(entity); return this.repository.findOneOrFail({ where: { id: entity.id } });