refactor(server): prevent sharing album with owner by filtering out user from albumUsers (#28891)

fix(server): prevent sharing album with owner by filtering out user from albumUsers
This commit is contained in:
Timon
2026-06-07 23:46:26 +02:00
committed by GitHub
parent 9e453440e6
commit 474efd39f8
3 changed files with 24 additions and 19 deletions
+4 -3
View File
@@ -504,13 +504,14 @@ describe('/albums', () => {
});
});
it('should not be able to share album with owner', async () => {
it('should deduplicate owner from albumUsers on create', async () => {
const { status, body } = await request(app)
.post('/albums')
.send({ albumName: 'New album', albumUsers: [{ role: AlbumUserRole.Editor, userId: user1.userId }] })
.set('Authorization', `Bearer ${user1.accessToken}`);
expect(status).toBe(400);
expect(body).toEqual(errorDto.badRequest('Cannot share album with owner'));
expect(status).toBe(201);
expect(body.albumUsers).toHaveLength(1);
expect(body.albumUsers[0]).toMatchObject({ role: AlbumUserRole.Owner, user: { id: user1.userId } });
});
});
+19 -11
View File
@@ -348,17 +348,25 @@ describe(AlbumService.name, () => {
expect(mocks.access.asset.checkOwnerAccess).toHaveBeenCalledWith(owner.id, new Set([assetId, 'asset-2']), false);
});
it('should throw an error if the userId is the ownerId', async () => {
const album = AlbumFactory.create();
const { user: owner } = album.albumUsers.find(({ role }) => role === AlbumUserRole.Owner)!;
mocks.user.get.mockResolvedValue(owner);
await expect(
sut.create(AuthFactory.create(owner), {
albumName: 'Empty album',
albumUsers: [{ userId: owner.id, role: AlbumUserRole.Editor }],
}),
).rejects.toBeInstanceOf(BadRequestException);
expect(mocks.album.create).not.toHaveBeenCalled();
it('should deduplicate owner from albumUsers on create', async () => {
const auth = AuthFactory.create();
const album = AlbumFactory.from().build();
mocks.album.create.mockResolvedValue(getForAlbum(album));
mocks.user.getMetadata.mockResolvedValue([]);
mocks.access.asset.checkOwnerAccess.mockResolvedValue(new Set());
await sut.create(auth, {
albumName: 'Empty album',
albumUsers: [{ userId: auth.user.id, role: AlbumUserRole.Editor }],
});
expect(mocks.user.get).not.toHaveBeenCalled();
expect(mocks.album.create).toHaveBeenCalledWith(
expect.objectContaining({ albumName: 'Empty album' }),
[],
[{ userId: auth.user.id, role: AlbumUserRole.Owner }],
auth.user.id,
);
});
});
+1 -5
View File
@@ -98,7 +98,7 @@ export class AlbumService extends BaseService {
}
async create(auth: AuthDto, dto: CreateAlbumDto): Promise<AlbumResponseDto> {
const albumUsers = dto.albumUsers || [];
const albumUsers = (dto.albumUsers || []).filter(({ userId }) => userId !== auth.user.id);
for (const { userId } of albumUsers) {
const exists = await this.userRepository.get(userId, {});
@@ -106,10 +106,6 @@ export class AlbumService extends BaseService {
this.logger.debug('Album creation failed: user not found');
throw new BadRequestException('Invalid user');
}
if (userId == auth.user.id) {
throw new BadRequestException('Cannot share album with owner');
}
}
const allowedAssetIdsSet = await this.checkAccess({