fix(server): do not expose album members to shared link viewers (#32607)
(cherry picked from commit 380f3e74e622938b6b6c450859df8655be4e21ba)
This commit is contained in:
@@ -6,6 +6,7 @@ import { AlbumUser, AuthSharedLink } from 'src/database.js';
|
||||
import { HistoryBuilder } from 'src/decorators.js';
|
||||
import { BulkIdErrorReasonSchema } from 'src/dtos/asset-ids.response.dto.js';
|
||||
import { MapAsset } from 'src/dtos/asset-response.dto.js';
|
||||
import { AuthDto } from 'src/dtos/auth.dto.js';
|
||||
import { UserResponseSchema, mapUser } from 'src/dtos/user.dto.js';
|
||||
import { AlbumUserRole, AlbumUserRoleSchema, AssetOrder, AssetOrderSchema } from 'src/enum.js';
|
||||
import { asDateTimeString } from 'src/utils/date.js';
|
||||
@@ -229,7 +230,7 @@ export type MapAlbumDto = {
|
||||
order: AssetOrder;
|
||||
};
|
||||
|
||||
export const mapAlbum = (entity: MaybeDehydrated<MapAlbumDto>): AlbumResponseDto => {
|
||||
export const mapAlbum = (entity: MaybeDehydrated<MapAlbumDto>, { auth }: { auth?: AuthDto } = {}): AlbumResponseDto => {
|
||||
const albumUsers: AlbumUserResponseDto[] = [];
|
||||
|
||||
if (entity.albumUsers) {
|
||||
@@ -262,7 +263,7 @@ export const mapAlbum = (entity: MaybeDehydrated<MapAlbumDto>): AlbumResponseDto
|
||||
createdAt: asDateTimeString(entity.createdAt),
|
||||
updatedAt: asDateTimeString(entity.updatedAt),
|
||||
id: entity.id,
|
||||
albumUsers,
|
||||
albumUsers: auth?.sharedLink ? [] : albumUsers,
|
||||
shared: hasSharedUser || hasSharedLink,
|
||||
hasSharedLink,
|
||||
startDate: asDateTimeString(startDate),
|
||||
|
||||
@@ -114,7 +114,7 @@ export function mapSharedLink(sharedLink: SharedLink, { auth }: { auth: AuthDto
|
||||
createdAt: sharedLink.createdAt,
|
||||
expiresAt: sharedLink.expiresAt,
|
||||
assets: assets.map((asset) => mapAsset(asset, { auth })),
|
||||
album: sharedLink.album ? mapAlbum(sharedLink.album) : undefined,
|
||||
album: sharedLink.album ? mapAlbum(sharedLink.album, { auth }) : undefined,
|
||||
allowUpload: sharedLink.allowUpload,
|
||||
allowDownload: sharedLink.allowDownload,
|
||||
showMetadata: sharedLink.showExif,
|
||||
|
||||
@@ -713,10 +713,12 @@ describe(AlbumService.name, () => {
|
||||
]);
|
||||
|
||||
const auth = AuthFactory.from().sharedLink().build();
|
||||
await sut.get(auth, album.id);
|
||||
const result = await sut.get(auth, album.id);
|
||||
|
||||
expect(mocks.album.getById).toHaveBeenCalledWith(album.id, auth.user.id);
|
||||
expect(mocks.access.album.checkSharedLinkAccess).toHaveBeenCalledWith(auth.sharedLink!.id, new Set([album.id]));
|
||||
expect(result).toMatchObject({ albumUsers: [], shared: true, contributorCounts: undefined });
|
||||
expect(mocks.album.getContributorCounts).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should get a shared album via shared with user', async () => {
|
||||
|
||||
@@ -83,8 +83,9 @@ export class AlbumService extends BaseService {
|
||||
const isShared = hasSharedUsers || hasSharedLink;
|
||||
|
||||
return {
|
||||
...(await this.mapWithMetadata(album)),
|
||||
contributorCounts: isShared ? await this.albumRepository.getContributorCounts(album.id) : undefined,
|
||||
...(await this.mapWithMetadata(album, auth)),
|
||||
contributorCounts:
|
||||
isShared && !auth.sharedLink ? await this.albumRepository.getContributorCounts(album.id) : undefined,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -356,8 +357,8 @@ export class AlbumService extends BaseService {
|
||||
return findOrFail(() => this.albumRepository.getById(id, authUserId), 'Album');
|
||||
}
|
||||
|
||||
private async mapWithMetadata(album: MapAlbumDto): Promise<AlbumResponseDto> {
|
||||
private async mapWithMetadata(album: MapAlbumDto, auth?: AuthDto): Promise<AlbumResponseDto> {
|
||||
const [metadata] = await this.albumRepository.getMetadataForIds([album.id]);
|
||||
return { ...mapAlbum(album), ...mapAlbumMetadata(metadata) };
|
||||
return { ...mapAlbum(album, { auth }), ...mapAlbumMetadata(metadata) };
|
||||
}
|
||||
}
|
||||
|
||||
@@ -73,6 +73,20 @@ describe(SharedLinkService.name, () => {
|
||||
expect(mocks.sharedLink.get).toHaveBeenCalledWith(authDto.user.id, authDto.sharedLink?.id);
|
||||
});
|
||||
|
||||
it('should not return album users', async () => {
|
||||
const authDto = authStub.adminSharedLink;
|
||||
const sharedLink = SharedLinkFactory.from()
|
||||
.album({}, (builder) => builder.albumUser())
|
||||
.build();
|
||||
mocks.sharedLink.get.mockResolvedValue(getForSharedLink(sharedLink));
|
||||
mocks.album.getMetadataForIds.mockResolvedValue([]);
|
||||
|
||||
const response = await sut.getMine(authDto, []);
|
||||
|
||||
expect(sharedLink.album?.albumUsers.length).toBeGreaterThan(0);
|
||||
expect(response.album).toMatchObject({ id: sharedLink.album!.id, albumUsers: [], shared: true });
|
||||
});
|
||||
|
||||
it('should throw an error for a request without a shared link auth token', async () => {
|
||||
const authDto = authStub.adminSharedLink;
|
||||
mocks.sharedLink.get.mockResolvedValue(sharedLinkStub.passwordRequired);
|
||||
|
||||
Reference in New Issue
Block a user