1
0
Fork 0
mirror of https://github.com/immich-app/immich.git synced 2024-12-29 15:11:58 +00:00

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
This commit is contained in:
martin 2024-03-03 00:01:24 +01:00 committed by GitHub
parent f03381a5b1
commit ebe7a14c14
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 28 additions and 14 deletions

View file

@ -41,7 +41,7 @@ describe('/album', () => {
]); ]);
[user1Asset1, user1Asset2] = await Promise.all([ [user1Asset1, user1Asset2] = await Promise.all([
apiUtils.createAsset(user1.accessToken), apiUtils.createAsset(user1.accessToken, { isFavorite: true }),
apiUtils.createAsset(user1.accessToken), apiUtils.createAsset(user1.accessToken),
]); ]);
@ -119,6 +119,17 @@ describe('/album', () => {
expect(body).toEqual(errorDto.badRequest(['assetId must be a UUID'])); 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 () => { it('should not return shared albums with a deleted owner', async () => {
const { status, body } = await request(app) const { status, body } = await request(app)
.get('/album?shared=true') .get('/album?shared=true')

View file

@ -1,6 +1,7 @@
import { AlbumEntity } from '@app/infra/entities'; import { AlbumEntity } from '@app/infra/entities';
import { ApiProperty } from '@nestjs/swagger'; import { ApiProperty } from '@nestjs/swagger';
import { AssetResponseDto, mapAsset } from '../asset'; import { AssetResponseDto, mapAsset } from '../asset';
import { AuthDto } from '../auth/auth.dto';
import { UserResponseDto, mapUser } from '../user'; import { UserResponseDto, mapUser } from '../user';
export class AlbumResponseDto { export class AlbumResponseDto {
@ -24,7 +25,7 @@ export class AlbumResponseDto {
isActivityEnabled!: boolean; isActivityEnabled!: boolean;
} }
export const mapAlbum = (entity: AlbumEntity, withAssets: boolean): AlbumResponseDto => { export const mapAlbum = (entity: AlbumEntity, withAssets: boolean, auth?: AuthDto): AlbumResponseDto => {
const sharedUsers: UserResponseDto[] = []; const sharedUsers: UserResponseDto[] = [];
if (entity.sharedUsers) { if (entity.sharedUsers) {
@ -59,7 +60,7 @@ export const mapAlbum = (entity: AlbumEntity, withAssets: boolean): AlbumRespons
hasSharedLink, hasSharedLink,
startDate, startDate,
endDate, endDate,
assets: (withAssets ? assets : []).map((asset) => mapAsset(asset)), assets: (withAssets ? assets : []).map((asset) => mapAsset(asset, { auth })),
assetCount: entity.assets?.length || 0, assetCount: entity.assets?.length || 0,
isActivityEnabled: entity.isActivityEnabled, isActivityEnabled: entity.isActivityEnabled,
}; };

View file

@ -104,7 +104,7 @@ export class AlbumService {
const [albumMetadataForIds] = await this.albumRepository.getMetadataForIds([album.id]); const [albumMetadataForIds] = await this.albumRepository.getMetadataForIds([album.id]);
return { return {
...mapAlbum(album, withAssets), ...mapAlbum(album, withAssets, auth),
startDate: albumMetadataForIds.startDate, startDate: albumMetadataForIds.startDate,
endDate: albumMetadataForIds.endDate, endDate: albumMetadataForIds.endDate,
assetCount: albumMetadataForIds.assetCount, assetCount: albumMetadataForIds.assetCount,

View file

@ -180,7 +180,7 @@ export class AssetService {
return { return {
title: `${years} year${years > 1 ? 's' : ''} since...`, title: `${years} year${years > 1 ? 's' : ''} since...`,
asset: mapAsset(asset), asset: mapAsset(asset, { auth }),
}; };
}) })
.groupBy((asset) => asset.title) .groupBy((asset) => asset.title)
@ -230,8 +230,8 @@ export class AssetService {
const timeBucketOptions = await this.buildTimeBucketOptions(auth, dto); const timeBucketOptions = await this.buildTimeBucketOptions(auth, dto);
const assets = await this.assetRepository.getTimeBucket(dto.timeBucket, timeBucketOptions); const assets = await this.assetRepository.getTimeBucket(dto.timeBucket, timeBucketOptions);
return !auth.sharedLink || auth.sharedLink?.showExif return !auth.sharedLink || auth.sharedLink?.showExif
? assets.map((asset) => mapAsset(asset, { withStack: true })) ? assets.map((asset) => mapAsset(asset, { withStack: true, auth }))
: assets.map((asset) => mapAsset(asset, { stripMetadata: true })); : assets.map((asset) => mapAsset(asset, { stripMetadata: true, auth }));
} }
async buildTimeBucketOptions(auth: AuthDto, dto: TimeBucketDto): Promise<TimeBucketOptions> { async buildTimeBucketOptions(auth: AuthDto, dto: TimeBucketDto): Promise<TimeBucketOptions> {
@ -261,7 +261,7 @@ export class AssetService {
async getRandom(auth: AuthDto, count: number): Promise<AssetResponseDto[]> { async getRandom(auth: AuthDto, count: number): Promise<AssetResponseDto[]> {
const assets = await this.assetRepository.getRandom(auth.user.id, count); 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) { async getUserAssetsByDeviceId(auth: AuthDto, deviceId: string) {
@ -292,10 +292,10 @@ export class AssetService {
} }
if (auth.sharedLink && !auth.sharedLink.showExif) { 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) { if (auth.sharedLink) {
delete data.owner; delete data.owner;
@ -315,7 +315,7 @@ export class AssetService {
await this.updateMetadata({ id, description, dateTimeOriginal, latitude, longitude }); await this.updateMetadata({ id, description, dateTimeOriginal, latitude, longitude });
const asset = await this.assetRepository.save({ id, ...rest }); const asset = await this.assetRepository.save({ id, ...rest });
return mapAsset(asset); return mapAsset(asset, { auth });
} }
async updateAll(auth: AuthDto, dto: AssetBulkUpdateDto): Promise<void> { async updateAll(auth: AuthDto, dto: AssetBulkUpdateDto): Promise<void> {

View file

@ -1,3 +1,4 @@
import { AuthDto } from '@app/domain/auth/auth.dto';
import { AssetEntity, AssetFaceEntity, AssetType } from '@app/infra/entities'; import { AssetEntity, AssetFaceEntity, AssetType } from '@app/infra/entities';
import { ApiProperty } from '@nestjs/swagger'; import { ApiProperty } from '@nestjs/swagger';
import { PersonWithFacesResponseDto, mapFacesWithoutPerson, mapPerson } from '../../person/person.dto'; import { PersonWithFacesResponseDto, mapFacesWithoutPerson, mapPerson } from '../../person/person.dto';
@ -50,6 +51,7 @@ export class AssetResponseDto extends SanitizedAssetResponseDto {
export type AssetMapOptions = { export type AssetMapOptions = {
stripMetadata?: boolean; stripMetadata?: boolean;
withStack?: boolean; withStack?: boolean;
auth?: AuthDto;
}; };
const peopleWithFaces = (faces: AssetFaceEntity[]): PersonWithFacesResponseDto[] => { const peopleWithFaces = (faces: AssetFaceEntity[]): PersonWithFacesResponseDto[] => {
@ -103,8 +105,8 @@ export function mapAsset(entity: AssetEntity, options: AssetMapOptions = {}): As
fileModifiedAt: entity.fileModifiedAt, fileModifiedAt: entity.fileModifiedAt,
localDateTime: entity.localDateTime, localDateTime: entity.localDateTime,
updatedAt: entity.updatedAt, updatedAt: entity.updatedAt,
isFavorite: entity.isFavorite, isFavorite: options.auth?.user.id === entity.ownerId ? entity.isFavorite : false,
isArchived: entity.isArchived, isArchived: options.auth?.user.id === entity.ownerId ? entity.isArchived : false,
isTrashed: !!entity.deletedAt, isTrashed: !!entity.deletedAt,
duration: entity.duration ?? '0:00:00.00000', duration: entity.duration ?? '0:00:00.00000',
exifInfo: entity.exifInfo ? mapExif(entity.exifInfo) : undefined, exifInfo: entity.exifInfo ? mapExif(entity.exifInfo) : undefined,
@ -117,7 +119,7 @@ export function mapAsset(entity: AssetEntity, options: AssetMapOptions = {}): As
stack: withStack stack: withStack
? entity.stack?.assets ? entity.stack?.assets
.filter((a) => a.id !== entity.stack?.primaryAssetId) .filter((a) => a.id !== entity.stack?.primaryAssetId)
.map((a) => mapAsset(a, { stripMetadata })) .map((a) => mapAsset(a, { stripMetadata, auth: options.auth }))
: undefined, : undefined,
stackCount: entity.stack?.assets?.length ?? null, stackCount: entity.stack?.assets?.length ?? null,
isExternal: entity.isExternal, isExternal: entity.isExternal,