From ebe7a14c142e8056e54e9f5174c5ed75a4cc5f4e Mon Sep 17 00:00:00 2001 From: martin <74269598+martabal@users.noreply.github.com> Date: Sun, 3 Mar 2024 00:01:24 +0100 Subject: [PATCH] fix(server): prevent leaking isFavorite and isArchived info (#7580) * fix: prevent leaking favorites info * add e2e test * fix: e2e test * fix: isArchived * fix: keep old version --- e2e/src/api/specs/album.e2e-spec.ts | 13 ++++++++++++- server/src/domain/album/album-response.dto.ts | 5 +++-- server/src/domain/album/album.service.ts | 2 +- server/src/domain/asset/asset.service.ts | 14 +++++++------- .../asset/response-dto/asset-response.dto.ts | 8 +++++--- 5 files changed, 28 insertions(+), 14 deletions(-) diff --git a/e2e/src/api/specs/album.e2e-spec.ts b/e2e/src/api/specs/album.e2e-spec.ts index 01f23d18db..99a50106ed 100644 --- a/e2e/src/api/specs/album.e2e-spec.ts +++ b/e2e/src/api/specs/album.e2e-spec.ts @@ -41,7 +41,7 @@ describe('/album', () => { ]); [user1Asset1, user1Asset2] = await Promise.all([ - apiUtils.createAsset(user1.accessToken), + apiUtils.createAsset(user1.accessToken, { isFavorite: true }), apiUtils.createAsset(user1.accessToken), ]); @@ -119,6 +119,17 @@ describe('/album', () => { expect(body).toEqual(errorDto.badRequest(['assetId must be a UUID'])); }); + it("should not show other users' favorites", async () => { + const { status, body } = await request(app) + .get(`/album/${user1Albums[0].id}?withoutAssets=false`) + .set('Authorization', `Bearer ${user2.accessToken}`); + expect(status).toEqual(200); + expect(body).toEqual({ + ...user1Albums[0], + assets: [expect.objectContaining({ isFavorite: false })], + }); + }); + it('should not return shared albums with a deleted owner', async () => { const { status, body } = await request(app) .get('/album?shared=true') diff --git a/server/src/domain/album/album-response.dto.ts b/server/src/domain/album/album-response.dto.ts index 1a266abac4..168b385928 100644 --- a/server/src/domain/album/album-response.dto.ts +++ b/server/src/domain/album/album-response.dto.ts @@ -1,6 +1,7 @@ import { AlbumEntity } from '@app/infra/entities'; import { ApiProperty } from '@nestjs/swagger'; import { AssetResponseDto, mapAsset } from '../asset'; +import { AuthDto } from '../auth/auth.dto'; import { UserResponseDto, mapUser } from '../user'; export class AlbumResponseDto { @@ -24,7 +25,7 @@ export class AlbumResponseDto { isActivityEnabled!: boolean; } -export const mapAlbum = (entity: AlbumEntity, withAssets: boolean): AlbumResponseDto => { +export const mapAlbum = (entity: AlbumEntity, withAssets: boolean, auth?: AuthDto): AlbumResponseDto => { const sharedUsers: UserResponseDto[] = []; if (entity.sharedUsers) { @@ -59,7 +60,7 @@ export const mapAlbum = (entity: AlbumEntity, withAssets: boolean): AlbumRespons hasSharedLink, startDate, endDate, - assets: (withAssets ? assets : []).map((asset) => mapAsset(asset)), + assets: (withAssets ? assets : []).map((asset) => mapAsset(asset, { auth })), assetCount: entity.assets?.length || 0, isActivityEnabled: entity.isActivityEnabled, }; diff --git a/server/src/domain/album/album.service.ts b/server/src/domain/album/album.service.ts index b14779a802..9a7b940f77 100644 --- a/server/src/domain/album/album.service.ts +++ b/server/src/domain/album/album.service.ts @@ -104,7 +104,7 @@ export class AlbumService { const [albumMetadataForIds] = await this.albumRepository.getMetadataForIds([album.id]); return { - ...mapAlbum(album, withAssets), + ...mapAlbum(album, withAssets, auth), startDate: albumMetadataForIds.startDate, endDate: albumMetadataForIds.endDate, assetCount: albumMetadataForIds.assetCount, diff --git a/server/src/domain/asset/asset.service.ts b/server/src/domain/asset/asset.service.ts index f328b5dcf6..dd54085060 100644 --- a/server/src/domain/asset/asset.service.ts +++ b/server/src/domain/asset/asset.service.ts @@ -180,7 +180,7 @@ export class AssetService { return { title: `${years} year${years > 1 ? 's' : ''} since...`, - asset: mapAsset(asset), + asset: mapAsset(asset, { auth }), }; }) .groupBy((asset) => asset.title) @@ -230,8 +230,8 @@ export class AssetService { const timeBucketOptions = await this.buildTimeBucketOptions(auth, dto); const assets = await this.assetRepository.getTimeBucket(dto.timeBucket, timeBucketOptions); return !auth.sharedLink || auth.sharedLink?.showExif - ? assets.map((asset) => mapAsset(asset, { withStack: true })) - : assets.map((asset) => mapAsset(asset, { stripMetadata: true })); + ? assets.map((asset) => mapAsset(asset, { withStack: true, auth })) + : assets.map((asset) => mapAsset(asset, { stripMetadata: true, auth })); } async buildTimeBucketOptions(auth: AuthDto, dto: TimeBucketDto): Promise { @@ -261,7 +261,7 @@ export class AssetService { async getRandom(auth: AuthDto, count: number): Promise { const assets = await this.assetRepository.getRandom(auth.user.id, count); - return assets.map((a) => mapAsset(a)); + return assets.map((a) => mapAsset(a, { auth })); } async getUserAssetsByDeviceId(auth: AuthDto, deviceId: string) { @@ -292,10 +292,10 @@ export class AssetService { } if (auth.sharedLink && !auth.sharedLink.showExif) { - return mapAsset(asset, { stripMetadata: true, withStack: true }); + return mapAsset(asset, { stripMetadata: true, withStack: true, auth }); } - const data = mapAsset(asset, { withStack: true }); + const data = mapAsset(asset, { withStack: true, auth }); if (auth.sharedLink) { delete data.owner; @@ -315,7 +315,7 @@ export class AssetService { await this.updateMetadata({ id, description, dateTimeOriginal, latitude, longitude }); const asset = await this.assetRepository.save({ id, ...rest }); - return mapAsset(asset); + return mapAsset(asset, { auth }); } async updateAll(auth: AuthDto, dto: AssetBulkUpdateDto): Promise { diff --git a/server/src/domain/asset/response-dto/asset-response.dto.ts b/server/src/domain/asset/response-dto/asset-response.dto.ts index 4cc0bd6672..2961a9dcc7 100644 --- a/server/src/domain/asset/response-dto/asset-response.dto.ts +++ b/server/src/domain/asset/response-dto/asset-response.dto.ts @@ -1,3 +1,4 @@ +import { AuthDto } from '@app/domain/auth/auth.dto'; import { AssetEntity, AssetFaceEntity, AssetType } from '@app/infra/entities'; import { ApiProperty } from '@nestjs/swagger'; import { PersonWithFacesResponseDto, mapFacesWithoutPerson, mapPerson } from '../../person/person.dto'; @@ -50,6 +51,7 @@ export class AssetResponseDto extends SanitizedAssetResponseDto { export type AssetMapOptions = { stripMetadata?: boolean; withStack?: boolean; + auth?: AuthDto; }; const peopleWithFaces = (faces: AssetFaceEntity[]): PersonWithFacesResponseDto[] => { @@ -103,8 +105,8 @@ export function mapAsset(entity: AssetEntity, options: AssetMapOptions = {}): As fileModifiedAt: entity.fileModifiedAt, localDateTime: entity.localDateTime, updatedAt: entity.updatedAt, - isFavorite: entity.isFavorite, - isArchived: entity.isArchived, + isFavorite: options.auth?.user.id === entity.ownerId ? entity.isFavorite : false, + isArchived: options.auth?.user.id === entity.ownerId ? entity.isArchived : false, isTrashed: !!entity.deletedAt, duration: entity.duration ?? '0:00:00.00000', exifInfo: entity.exifInfo ? mapExif(entity.exifInfo) : undefined, @@ -117,7 +119,7 @@ export function mapAsset(entity: AssetEntity, options: AssetMapOptions = {}): As stack: withStack ? entity.stack?.assets .filter((a) => a.id !== entity.stack?.primaryAssetId) - .map((a) => mapAsset(a, { stripMetadata })) + .map((a) => mapAsset(a, { stripMetadata, auth: options.auth })) : undefined, stackCount: entity.stack?.assets?.length ?? null, isExternal: entity.isExternal,