github-mirrors/immich
1
0
Fork 0
mirror of https://github.com/immich-app/immich.git synced 2026-05-18 03:10:24 +03:00
Code Issues Packages Projects Releases Wiki Activity

refactor(server)!: sanitize error messages to avoid leaking resource and permission details

Browse Source
This commit is contained in:
timonrieger
2026-04-29 14:45:46 +02:00
parent 65bd0a9320
commit b96421a083
31 changed files with 129 additions and 128 deletions
Download Patch File Download Diff File Expand all files Collapse all files
e2e/src/responses.ts
+4 -4
View File
@@ -10,9 +10,9 @@ export const errorDto = {
forbidden: {
message: expect.any(String),
},
missingPermission: (permission: string) => ({
message: `Missing required permission: ${permission}`,
}),
missingPermission: {
message: 'Access denied',
},
wrongPassword: {
message: 'Wrong password',
},
@@ -29,7 +29,7 @@ export const errorDto = {
message: message ?? expect.anything(),
}),
noPermission: {
message: expect.stringContaining('Not found or no'),
message: 'Access denied',
},
incorrectLogin: {
message: 'Incorrect email or password',
e2e/src/specs/server/api/activity.e2e-spec.ts
+2 -2
View File
@@ -323,8 +323,8 @@ describe('/activities', () => {
.delete(`/activities/${reaction.id}`)
.set('Authorization', `Bearer ${nonOwner.accessToken}`);
expect(status).toBe(400);
expect(body).toEqual(errorDto.badRequest('Not found or no activity.delete access'));
expect(status).toBe(403);
expect(body).toEqual(errorDto.noPermission);
});
it('should let a non-owner remove their own comment', async () => {
e2e/src/specs/server/api/album.e2e-spec.ts
+9 -9
View File
@@ -480,8 +480,8 @@ describe('/albums', () => {
.set('Authorization', `Bearer ${user2.accessToken}`)
.send({ ids: [asset.id] });
expect(status).toBe(400);
expect(body).toEqual(errorDto.badRequest('Not found or no albumAsset.create access'));
expect(status).toBe(403);
expect(body).toEqual(errorDto.noPermission);
});
it('should add duplicate assets only once', async () => {
@@ -526,8 +526,8 @@ describe('/albums', () => {
.set('Authorization', `Bearer ${user2.accessToken}`)
.send({ albumName: 'New album name' });
expect(status).toBe(400);
expect(body).toEqual(errorDto.badRequest('Not found or no album.update access'));
expect(status).toBe(403);
expect(body).toEqual(errorDto.noPermission);
});
it('should be able to update as an editor', async () => {
@@ -553,7 +553,7 @@ describe('/albums', () => {
.set('Authorization', `Bearer ${user2.accessToken}`)
.send({ ids: [user1Asset1.id] });
expect(status).toBe(400);
expect(status).toBe(403);
expect(body).toEqual(errorDto.noPermission);
});
@@ -614,8 +614,8 @@ describe('/albums', () => {
.set('Authorization', `Bearer ${user2.accessToken}`)
.send({ ids: [user1Asset1.id] });
expect(status).toBe(400);
expect(body).toEqual(errorDto.badRequest('Not found or no albumAsset.delete access'));
expect(status).toBe(403);
expect(body).toEqual(errorDto.noPermission);
});
it('should remove duplicate assets only once', async () => {
@@ -728,8 +728,8 @@ describe('/albums', () => {
.set('Authorization', `Bearer ${user2.accessToken}`)
.send({ role: AlbumUserRole.Editor });
expect(status).toBe(400);
expect(body).toEqual(errorDto.badRequest('Not found or no album.share access'));
expect(status).toBe(403);
expect(body).toEqual(errorDto.noPermission);
});
});
});
e2e/src/specs/server/api/api-key.e2e-spec.ts
+1 -1
View File
@@ -28,7 +28,7 @@ describe('/api-keys', () => {
const { secret } = await create(user.accessToken, [Permission.ApiKeyRead]);
const { status, body } = await request(app).post('/api-keys').set('x-api-key', secret).send({ name: 'API Key' });
expect(status).toBe(403);
expect(body).toEqual(errorDto.missingPermission('apiKey.create'));
expect(body).toEqual(errorDto.missingPermission);
});
it('should work with apiKey.create', async () => {
e2e/src/specs/server/api/asset.e2e-spec.ts
+4 -4
View File
@@ -157,7 +157,7 @@ describe('/asset', () => {
const { status, body } = await request(app)
.get(`/assets/${user2Assets[0].id}`)
.set('Authorization', `Bearer ${user1.accessToken}`);
expect(status).toBe(400);
expect(status).toBe(403);
expect(body).toEqual(errorDto.noPermission);
});
@@ -385,7 +385,7 @@ describe('/asset', () => {
.put(`/assets/${user2Assets[0].id}`)
.set('Authorization', `Bearer ${user1.accessToken}`)
.send({});
expect(status).toBe(400);
expect(status).toBe(403);
expect(body).toEqual(errorDto.noPermission);
});
@@ -609,8 +609,8 @@ describe('/asset', () => {
.send({ ids: [uuidDto.notFound] })
.set('Authorization', `Bearer ${admin.accessToken}`);
expect(status).toBe(400);
expect(body).toEqual(errorDto.badRequest('Not found or no asset.delete access'));
expect(status).toBe(403);
expect(body).toEqual(errorDto.noPermission);
});
it('should move an asset to trash', async () => {
e2e/src/specs/server/api/memory.e2e-spec.ts
+5 -5
View File
@@ -46,7 +46,7 @@ describe('/memories', () => {
const { status, body } = await request(app)
.get(`/memories/${userMemory.id}`)
.set('Authorization', `Bearer ${admin.accessToken}`);
expect(status).toBe(400);
expect(status).toBe(403);
expect(body).toEqual(errorDto.noPermission);
});
@@ -65,7 +65,7 @@ describe('/memories', () => {
.put(`/memories/${userMemory.id}`)
.send({ isSaved: true })
.set('Authorization', `Bearer ${admin.accessToken}`);
expect(status).toBe(400);
expect(status).toBe(403);
expect(body).toEqual(errorDto.noPermission);
});
@@ -91,7 +91,7 @@ describe('/memories', () => {
.put(`/memories/${userMemory.id}/assets`)
.send({ ids: [userAsset1.id] })
.set('Authorization', `Bearer ${admin.accessToken}`);
expect(status).toBe(400);
expect(status).toBe(403);
expect(body).toEqual(errorDto.noPermission);
});
@@ -126,7 +126,7 @@ describe('/memories', () => {
.delete(`/memories/${userMemory.id}/assets`)
.send({ ids: [userAsset1.id] })
.set('Authorization', `Bearer ${admin.accessToken}`);
expect(status).toBe(400);
expect(status).toBe(403);
expect(body).toEqual(errorDto.noPermission);
});
@@ -160,7 +160,7 @@ describe('/memories', () => {
const { status, body } = await request(app)
.delete(`/memories/${userMemory.id}`)
.set('Authorization', `Bearer ${admin.accessToken}`);
expect(status).toBe(400);
expect(status).toBe(403);
expect(body).toEqual(errorDto.noPermission);
});
e2e/src/specs/server/api/session.e2e-spec.ts
+2 -2
View File
@@ -52,8 +52,8 @@ describe('/sessions', () => {
const { status, body } = await request(app)
.delete(`/sessions/${uuidDto.notFound}`)
.set('Authorization', `Bearer ${admin.accessToken}`);
expect(status).toBe(400);
expect(body).toEqual(errorDto.badRequest('Not found or no authDevice.delete access'));
expect(status).toBe(403);
expect(body).toEqual(errorDto.noPermission);
});
it('should logout a device', async () => {
e2e/src/specs/server/api/stack.e2e-spec.ts
+1 -1
View File
@@ -61,7 +61,7 @@ describe('/stacks', () => {
.set('Authorization', `Bearer ${user1.accessToken}`)
.send({ assetIds: [asset.id, user2Asset.id] });
expect(status).toBe(400);
expect(status).toBe(403);
expect(body).toEqual(errorDto.noPermission);
});
e2e/src/specs/server/api/tag.e2e-spec.ts
+16 -16
View File
@@ -51,7 +51,7 @@ describe('/tags', () => {
const { secret } = await utils.createApiKey(user.accessToken, [Permission.AssetRead]);
const { status, body } = await request(app).post('/tags').set('x-api-key', secret).send({ name: 'TagA' });
expect(status).toBe(403);
expect(body).toEqual(errorDto.missingPermission('tag.create'));
expect(body).toEqual(errorDto.missingPermission);
});
it('should work with tag.create', async () => {
@@ -127,7 +127,7 @@ describe('/tags', () => {
const { secret } = await utils.createApiKey(user.accessToken, [Permission.AssetRead]);
const { status, body } = await request(app).get('/tags').set('x-api-key', secret);
expect(status).toBe(403);
expect(body).toEqual(errorDto.missingPermission('tag.read'));
expect(body).toEqual(errorDto.missingPermission);
});
it('should start off empty', async () => {
@@ -179,7 +179,7 @@ describe('/tags', () => {
const { secret } = await utils.createApiKey(user.accessToken, [Permission.AssetRead]);
const { status, body } = await request(app).put('/tags').set('x-api-key', secret).send({ name: 'TagA' });
expect(status).toBe(403);
expect(body).toEqual(errorDto.missingPermission('tag.create'));
expect(body).toEqual(errorDto.missingPermission);
});
it('should upsert tags', async () => {
@@ -226,7 +226,7 @@ describe('/tags', () => {
.set('x-api-key', secret)
.send({ assetIds: [], tagIds: [] });
expect(status).toBe(403);
expect(body).toEqual(errorDto.missingPermission('tag.asset'));
expect(body).toEqual(errorDto.missingPermission);
});
it('should skip assets that are not owned by the user', async () => {
@@ -290,7 +290,7 @@ describe('/tags', () => {
const { status, body } = await request(app)
.get(`/tags/${tag.id}`)
.set('Authorization', `Bearer ${admin.accessToken}`);
expect(status).toBe(400);
expect(status).toBe(403);
expect(body).toEqual(errorDto.noPermission);
});
@@ -301,7 +301,7 @@ describe('/tags', () => {
.set('x-api-key', secret)
.send({ assetIds: [], tagIds: [] });
expect(status).toBe(403);
expect(body).toEqual(errorDto.missingPermission('tag.read'));
expect(body).toEqual(errorDto.missingPermission);
});
it('should require a valid uuid', async () => {
@@ -362,7 +362,7 @@ describe('/tags', () => {
.put(`/tags/${tag.id}`)
.send({ color: '#000000' })
.set('Authorization', `Bearer ${user.accessToken}`);
expect(status).toBe(400);
expect(status).toBe(403);
expect(body).toEqual(errorDto.noPermission);
});
@@ -374,7 +374,7 @@ describe('/tags', () => {
.set('x-api-key', secret)
.send({ color: '#000000' });
expect(status).toBe(403);
expect(body).toEqual(errorDto.missingPermission('tag.update'));
expect(body).toEqual(errorDto.missingPermission);
});
it('should update a tag', async () => {
@@ -410,7 +410,7 @@ describe('/tags', () => {
const { status, body } = await request(app)
.delete(`/tags/${tag.id}`)
.set('Authorization', `Bearer ${admin.accessToken}`);
expect(status).toBe(400);
expect(status).toBe(403);
expect(body).toEqual(errorDto.noPermission);
});
@@ -419,7 +419,7 @@ describe('/tags', () => {
const { secret } = await utils.createApiKey(user.accessToken, [Permission.AssetRead]);
const { status, body } = await request(app).delete(`/tags/${tag.id}`).set('x-api-key', secret);
expect(status).toBe(403);
expect(body).toEqual(errorDto.missingPermission('tag.delete'));
expect(body).toEqual(errorDto.missingPermission);
});
it('should require a valid uuid', async () => {
@@ -478,7 +478,7 @@ describe('/tags', () => {
.put(`/tags/${tag.id}/assets`)
.set('Authorization', `Bearer ${admin.accessToken}`)
.send({ ids: [userAsset.id] });
expect(status).toBe(400);
expect(status).toBe(403);
expect(body).toEqual(errorDto.noPermission);
});
@@ -490,7 +490,7 @@ describe('/tags', () => {
.set('x-api-key', secret)
.send({ ids: [userAsset.id] });
expect(status).toBe(403);
expect(body).toEqual(errorDto.missingPermission('tag.asset'));
expect(body).toEqual(errorDto.missingPermission);
});
it('should be able to tag own asset', async () => {
@@ -511,8 +511,8 @@ describe('/tags', () => {
.set('Authorization', `Bearer ${user.accessToken}`)
.send({ ids: [userAsset.id] });
expect(status).toBe(400);
expect(body).toEqual(errorDto.badRequest('Not found or no tag.asset access'));
expect(status).toBe(403);
expect(body).toEqual(errorDto.noPermission);
});
it('should add duplicate assets only once', async () => {
@@ -552,7 +552,7 @@ describe('/tags', () => {
.set('Authorization', `Bearer ${admin.accessToken}`)
.send({ ids: [userAsset.id] });
expect(status).toBe(400);
expect(status).toBe(403);
expect(body).toEqual(errorDto.noPermission);
});
@@ -564,7 +564,7 @@ describe('/tags', () => {
.set('x-api-key', secret)
.send({ ids: [userAsset.id] });
expect(status).toBe(403);
expect(body).toEqual(errorDto.missingPermission('tag.asset'));
expect(body).toEqual(errorDto.missingPermission);
});
it('should be able to remove own asset from own tag', async () => {
server/src/services/activity.service.spec.ts
+4 -4
View File
@@ -1,4 +1,4 @@
import { BadRequestException } from '@nestjs/common';
import { ForbiddenException } from '@nestjs/common';
import { ReactionType } from 'src/dtos/activity.dto';
import { ActivityService } from 'src/services/activity.service';
import { ActivityFactory } from 'test/factories/activity.factory';
@@ -78,7 +78,7 @@ describe(ActivityService.name, () => {
await expect(
sut.create(AuthFactory.create(), { albumId, assetId, type: ReactionType.COMMENT, comment: 'comment' }),
).rejects.toBeInstanceOf(BadRequestException);
).rejects.toBeInstanceOf(ForbiddenException);
});
it('should create a comment', async () => {
@@ -113,7 +113,7 @@ describe(ActivityService.name, () => {
await expect(
sut.create(AuthFactory.create(), { albumId, assetId, type: ReactionType.COMMENT, comment: 'comment' }),
).rejects.toBeInstanceOf(BadRequestException);
).rejects.toBeInstanceOf(ForbiddenException);
});
it('should create a like', async () => {
@@ -145,7 +145,7 @@ describe(ActivityService.name, () => {
describe('delete', () => {
it('should require access', async () => {
await expect(sut.delete(AuthFactory.create(), newUuid())).rejects.toBeInstanceOf(BadRequestException);
await expect(sut.delete(AuthFactory.create(), newUuid())).rejects.toBeInstanceOf(ForbiddenException);
expect(mocks.activity.delete).not.toHaveBeenCalled();
});
server/src/services/album.service.spec.ts
+11 -11
View File
@@ -1,4 +1,4 @@
import { BadRequestException } from '@nestjs/common';
import { BadRequestException, ForbiddenException } from '@nestjs/common';
import { BulkIdErrorReason } from 'src/dtos/asset-ids.response.dto';
import { AlbumUserRole, AssetOrder, UserMetadataKey } from 'src/enum';
import { AlbumService } from 'src/services/album.service';
@@ -323,7 +323,7 @@ describe(AlbumService.name, () => {
sut.update(AuthFactory.create(), 'invalid-id', {
albumName: 'Album',
}),
).rejects.toBeInstanceOf(BadRequestException);
).rejects.toBeInstanceOf(ForbiddenException);
expect(mocks.album.update).not.toHaveBeenCalled();
});
@@ -334,7 +334,7 @@ describe(AlbumService.name, () => {
mocks.access.album.checkOwnerAccess.mockResolvedValue(new Set());
await expect(
sut.update(AuthFactory.create(owner), album.id, { albumName: 'new album name' }),
).rejects.toBeInstanceOf(BadRequestException);
).rejects.toBeInstanceOf(ForbiddenException);
});
it('should require a valid thumbnail asset id', async () => {
@@ -376,7 +376,7 @@ describe(AlbumService.name, () => {
const { user: owner } = album.albumUsers.find(({ role }) => role === AlbumUserRole.Owner)!;
mocks.access.album.checkOwnerAccess.mockResolvedValue(new Set());
await expect(sut.delete(AuthFactory.create(owner), album.id)).rejects.toBeInstanceOf(BadRequestException);
await expect(sut.delete(AuthFactory.create(owner), album.id)).rejects.toBeInstanceOf(ForbiddenException);
expect(mocks.album.delete).not.toHaveBeenCalled();
});
@@ -387,7 +387,7 @@ describe(AlbumService.name, () => {
mocks.album.getById.mockResolvedValue(getForAlbum(album));
mocks.access.album.checkOwnerAccess.mockResolvedValue(new Set());
await expect(sut.delete(AuthFactory.create(owner), album.id)).rejects.toBeInstanceOf(BadRequestException);
await expect(sut.delete(AuthFactory.create(owner), album.id)).rejects.toBeInstanceOf(ForbiddenException);
expect(mocks.album.delete).not.toHaveBeenCalled();
});
@@ -412,7 +412,7 @@ describe(AlbumService.name, () => {
mocks.access.album.checkOwnerAccess.mockResolvedValue(new Set());
await expect(
sut.addUsers(AuthFactory.create(user), album.id, { albumUsers: [{ userId: newUuid() }] }),
).rejects.toBeInstanceOf(BadRequestException);
).rejects.toBeInstanceOf(ForbiddenException);
expect(mocks.album.update).not.toHaveBeenCalled();
});
@@ -512,7 +512,7 @@ describe(AlbumService.name, () => {
mocks.album.getById.mockResolvedValue(getForAlbum(album));
await expect(sut.removeUser(AuthFactory.create(user1), album.id, user2.id)).rejects.toBeInstanceOf(
BadRequestException,
ForbiddenException,
);
expect(mocks.albumUser.delete).not.toHaveBeenCalled();
@@ -655,7 +655,7 @@ describe(AlbumService.name, () => {
it('should throw an error for no access', async () => {
const auth = AuthFactory.create();
await expect(sut.get(auth, 'album-123')).rejects.toBeInstanceOf(BadRequestException);
await expect(sut.get(auth, 'album-123')).rejects.toBeInstanceOf(ForbiddenException);
expect(mocks.access.album.checkOwnerAccess).toHaveBeenCalledWith(auth.user.id, new Set(['album-123']));
expect(mocks.access.album.checkSharedAlbumAccess).toHaveBeenCalledWith(
@@ -764,7 +764,7 @@ describe(AlbumService.name, () => {
await expect(
sut.addAssets(AuthFactory.create(user), album.id, { ids: [asset1.id, asset2.id, asset3.id] }),
).rejects.toBeInstanceOf(BadRequestException);
).rejects.toBeInstanceOf(ForbiddenException);
expect(mocks.album.update).not.toHaveBeenCalled();
});
@@ -833,7 +833,7 @@ describe(AlbumService.name, () => {
mocks.album.getById.mockResolvedValue(getForAlbum(album));
await expect(sut.addAssets(AuthFactory.create(user), album.id, { ids: [asset.id] })).rejects.toBeInstanceOf(
BadRequestException,
ForbiddenException,
);
expect(mocks.access.album.checkOwnerAccess).toHaveBeenCalled();
@@ -847,7 +847,7 @@ describe(AlbumService.name, () => {
await expect(
sut.addAssets(AuthFactory.from().sharedLink({ allowUpload: true }).build(), album.id, { ids: [asset.id] }),
).rejects.toBeInstanceOf(BadRequestException);
).rejects.toBeInstanceOf(ForbiddenException);
expect(mocks.access.album.checkSharedLinkAccess).toHaveBeenCalled();
});
server/src/services/asset-media.service.spec.ts
+4 -3
View File
@@ -1,5 +1,6 @@
import {
BadRequestException,
ForbiddenException,
InternalServerErrorException,
NotFoundException,
UnauthorizedException,
@@ -459,7 +460,7 @@ describe(AssetMediaService.name, () => {
describe('downloadOriginal', () => {
it('should require the asset.download permission', async () => {
await expect(sut.downloadOriginal(authStub.admin, 'asset-1', {})).rejects.toBeInstanceOf(BadRequestException);
await expect(sut.downloadOriginal(authStub.admin, 'asset-1', {})).rejects.toBeInstanceOf(ForbiddenException);
expect(mocks.access.asset.checkOwnerAccess).toHaveBeenCalledWith(
authStub.admin.user.id,
@@ -567,7 +568,7 @@ describe(AssetMediaService.name, () => {
describe('viewThumbnail', () => {
it('should require asset.view permissions', async () => {
await expect(sut.viewThumbnail(authStub.admin, 'id', {})).rejects.toBeInstanceOf(BadRequestException);
await expect(sut.viewThumbnail(authStub.admin, 'id', {})).rejects.toBeInstanceOf(ForbiddenException);
expect(mocks.access.asset.checkOwnerAccess).toHaveBeenCalledWith(userStub.admin.id, new Set(['id']), undefined);
expect(mocks.access.asset.checkAlbumAccess).toHaveBeenCalledWith(userStub.admin.id, new Set(['id']));
@@ -710,7 +711,7 @@ describe(AssetMediaService.name, () => {
describe('playbackVideo', () => {
it('should require asset.view permissions', async () => {
await expect(sut.playbackVideo(authStub.admin, 'id')).rejects.toBeInstanceOf(BadRequestException);
await expect(sut.playbackVideo(authStub.admin, 'id')).rejects.toBeInstanceOf(ForbiddenException);
expect(mocks.access.asset.checkOwnerAccess).toHaveBeenCalledWith(userStub.admin.id, new Set(['id']), undefined);
expect(mocks.access.asset.checkAlbumAccess).toHaveBeenCalledWith(userStub.admin.id, new Set(['id']));
server/src/services/asset-media.service.ts
+2 -2
View File
@@ -216,7 +216,7 @@ export class AssetMediaService extends BaseService {
}
if (!path) {
throw new NotFoundException('Asset media not found');
throw new NotFoundException('Not found');
}
const fileNameBase =
@@ -237,7 +237,7 @@ export class AssetMediaService extends BaseService {
const asset = await this.assetRepository.getForVideo(id);
if (!asset) {
throw new NotFoundException('Asset not found or asset is not a video');
throw new NotFoundException('Not found');
}
const filepath = asset.encodedVideoPath || asset.originalPath;
server/src/services/asset.service.spec.ts
+8 -8
View File
@@ -1,4 +1,4 @@
import { BadRequestException } from '@nestjs/common';
import { BadRequestException, ForbiddenException } from '@nestjs/common';
import { DateTime } from 'luxon';
import { AssetJobName, AssetStatsResponseDto } from 'src/dtos/asset.dto';
import { AssetEditAction } from 'src/dtos/editing.dto';
@@ -136,14 +136,14 @@ describe(AssetService.name, () => {
});
it('should throw an error for no access', async () => {
await expect(sut.get(authStub.admin, AssetFactory.create().id)).rejects.toBeInstanceOf(BadRequestException);
await expect(sut.get(authStub.admin, AssetFactory.create().id)).rejects.toBeInstanceOf(ForbiddenException);
expect(mocks.asset.getById).not.toHaveBeenCalled();
});
it('should throw an error for an invalid shared link', async () => {
await expect(sut.get(authStub.adminSharedLink, AssetFactory.create().id)).rejects.toBeInstanceOf(
BadRequestException,
ForbiddenException,
);
expect(mocks.access.asset.checkOwnerAccess).not.toHaveBeenCalled();
@@ -162,7 +162,7 @@ describe(AssetService.name, () => {
it('should require asset write access for the id', async () => {
await expect(
sut.update(authStub.admin, 'asset-1', { visibility: AssetVisibility.Timeline }),
).rejects.toBeInstanceOf(BadRequestException);
).rejects.toBeInstanceOf(ForbiddenException);
expect(mocks.asset.update).not.toHaveBeenCalled();
});
@@ -357,7 +357,7 @@ describe(AssetService.name, () => {
describe('updateAll', () => {
it('should require asset write access for all ids', async () => {
const auth = AuthFactory.create();
await expect(sut.updateAll(auth, { ids: ['asset-1'] })).rejects.toBeInstanceOf(BadRequestException);
await expect(sut.updateAll(auth, { ids: ['asset-1'] })).rejects.toBeInstanceOf(ForbiddenException);
});
it('should update all assets', async () => {
@@ -461,7 +461,7 @@ describe(AssetService.name, () => {
sut.deleteAll(authStub.user1, {
ids: ['asset-1'],
}),
).rejects.toBeInstanceOf(BadRequestException);
).rejects.toBeInstanceOf(ForbiddenException);
});
it('should force delete a batch of assets', async () => {
@@ -612,7 +612,7 @@ describe(AssetService.name, () => {
it('should require asset read permission', async () => {
mocks.access.asset.checkOwnerAccess.mockResolvedValue(new Set());
await expect(sut.getOcr(authStub.admin, 'asset-1')).rejects.toBeInstanceOf(BadRequestException);
await expect(sut.getOcr(authStub.admin, 'asset-1')).rejects.toBeInstanceOf(ForbiddenException);
expect(mocks.ocr.getByAssetId).not.toHaveBeenCalled();
});
@@ -736,7 +736,7 @@ describe(AssetService.name, () => {
},
],
}),
).rejects.toBeInstanceOf(BadRequestException);
).rejects.toBeInstanceOf(ForbiddenException);
expect(mocks.assetEdit.replaceAll).not.toHaveBeenCalled();
});
server/src/services/auth.service.spec.ts
+2 -2
View File
@@ -598,7 +598,7 @@ describe(AuthService.name, () => {
});
await expect(result).rejects.toBeInstanceOf(ForbiddenException);
await expect(result).rejects.toThrow('Missing required permission: asset.read');
await expect(result).rejects.toThrow('Access denied');
});
it('should default to requiring the all permission when omitted', async () => {
@@ -613,7 +613,7 @@ describe(AuthService.name, () => {
metadata: { adminRoute: false, sharedLinkRoute: false, uri: 'test' },
});
await expect(result).rejects.toBeInstanceOf(ForbiddenException);
await expect(result).rejects.toThrow('Missing required permission: all');
await expect(result).rejects.toThrow('Access denied');
});
it('should not require any permission when metadata is set to `false`', async () => {
server/src/services/auth.service.ts
+1 -1
View File
@@ -238,7 +238,7 @@ export class AuthService extends BaseService {
requestedPermission !== false &&
!isGranted({ requested: [requestedPermission], current: authDto.apiKey.permissions })
) {
throw new ForbiddenException(`Missing required permission: ${requestedPermission}`);
throw new ForbiddenException('Access denied');
}
return authDto;
server/src/services/memory.service.spec.ts
+6 -6
View File
@@ -1,4 +1,4 @@
import { BadRequestException } from '@nestjs/common';
import { BadRequestException, ForbiddenException } from '@nestjs/common';
import { MemoryService } from 'src/services/memory.service';
import { OnThisDayData } from 'src/types';
import { AssetFactory } from 'test/factories/asset.factory';
@@ -53,7 +53,7 @@ describe(MemoryService.name, () => {
describe('get', () => {
it('should throw an error when no access', async () => {
await expect(sut.get(factory.auth(), 'not-found')).rejects.toBeInstanceOf(BadRequestException);
await expect(sut.get(factory.auth(), 'not-found')).rejects.toBeInstanceOf(ForbiddenException);
});
it('should throw an error when the memory is not found', async () => {
@@ -151,7 +151,7 @@ describe(MemoryService.name, () => {
describe('update', () => {
it('should require access', async () => {
await expect(sut.update(factory.auth(), 'not-found', { isSaved: true })).rejects.toBeInstanceOf(
BadRequestException,
ForbiddenException,
);
expect(mocks.memory.update).not.toHaveBeenCalled();
@@ -171,7 +171,7 @@ describe(MemoryService.name, () => {
describe('remove', () => {
it('should require access', async () => {
await expect(sut.remove(factory.auth(), newUuid())).rejects.toBeInstanceOf(BadRequestException);
await expect(sut.remove(factory.auth(), newUuid())).rejects.toBeInstanceOf(ForbiddenException);
expect(mocks.memory.delete).not.toHaveBeenCalled();
});
@@ -193,7 +193,7 @@ describe(MemoryService.name, () => {
const [memoryId, assetId] = newUuids();
await expect(sut.addAssets(factory.auth(), memoryId, { ids: [assetId] })).rejects.toBeInstanceOf(
BadRequestException,
ForbiddenException,
);
expect(mocks.memory.addAssetIds).not.toHaveBeenCalled();
@@ -251,7 +251,7 @@ describe(MemoryService.name, () => {
describe('removeAssets', () => {
it('should require memory access', async () => {
await expect(sut.removeAssets(factory.auth(), 'not-found', { ids: ['asset1'] })).rejects.toBeInstanceOf(
BadRequestException,
ForbiddenException,
);
expect(mocks.memory.removeAssetIds).not.toHaveBeenCalled();
server/src/services/partner.service.spec.ts
+2 -2
View File
@@ -1,4 +1,4 @@
import { BadRequestException } from '@nestjs/common';
import { BadRequestException, ForbiddenException } from '@nestjs/common';
import { PartnerDirection } from 'src/repositories/partner.repository';
import { PartnerService } from 'src/services/partner.service';
import { AuthFactory } from 'test/factories/auth.factory';
@@ -109,7 +109,7 @@ describe(PartnerService.name, () => {
const user2 = UserFactory.create();
const auth = AuthFactory.create();
await expect(sut.update(auth, user2.id, { inTimeline: false })).rejects.toBeInstanceOf(BadRequestException);
await expect(sut.update(auth, user2.id, { inTimeline: false })).rejects.toBeInstanceOf(ForbiddenException);
});
it('should update partner', async () => {
server/src/services/person.service.spec.ts
+11 -11
View File
@@ -1,4 +1,4 @@
import { BadRequestException, NotFoundException } from '@nestjs/common';
import { BadRequestException, ForbiddenException, NotFoundException } from '@nestjs/common';
import { BulkIdErrorReason } from 'src/dtos/asset-ids.response.dto';
import { mapFaces, mapPerson } from 'src/dtos/person.dto';
import { AssetFileType, CacheControl, JobName, JobStatus, SourceType, SystemMetadataKey } from 'src/enum';
@@ -95,7 +95,7 @@ describe(PersonService.name, () => {
const auth = AuthFactory.create();
const person = PersonFactory.create();
mocks.person.getById.mockResolvedValue(person);
await expect(sut.getById(auth, person.id)).rejects.toBeInstanceOf(BadRequestException);
await expect(sut.getById(auth, person.id)).rejects.toBeInstanceOf(ForbiddenException);
expect(mocks.access.person.checkOwnerAccess).toHaveBeenCalledWith(auth.user.id, new Set([person.id]));
});
@@ -124,7 +124,7 @@ describe(PersonService.name, () => {
const person = PersonFactory.create();
mocks.person.getById.mockResolvedValue(person);
await expect(sut.getThumbnail(auth, person.id)).rejects.toBeInstanceOf(BadRequestException);
await expect(sut.getThumbnail(auth, person.id)).rejects.toBeInstanceOf(ForbiddenException);
expect(mocks.storage.createReadStream).not.toHaveBeenCalled();
expect(mocks.access.person.checkOwnerAccess).toHaveBeenCalledWith(auth.user.id, new Set([person.id]));
});
@@ -172,7 +172,7 @@ describe(PersonService.name, () => {
const person = PersonFactory.create();
mocks.person.getById.mockResolvedValue(person);
await expect(sut.update(auth, person.id, { name: 'Person 1' })).rejects.toBeInstanceOf(BadRequestException);
await expect(sut.update(auth, person.id, { name: 'Person 1' })).rejects.toBeInstanceOf(ForbiddenException);
expect(mocks.person.update).not.toHaveBeenCalled();
expect(mocks.access.person.checkOwnerAccess).toHaveBeenCalledWith(auth.user.id, new Set([person.id]));
});
@@ -180,7 +180,7 @@ describe(PersonService.name, () => {
it('should throw an error when personId is invalid', async () => {
mocks.access.person.checkOwnerAccess.mockResolvedValue(new Set());
await expect(sut.update(authStub.admin, 'person-1', { name: 'Person 1' })).rejects.toBeInstanceOf(
BadRequestException,
ForbiddenException,
);
expect(mocks.person.update).not.toHaveBeenCalled();
expect(mocks.access.person.checkOwnerAccess).toHaveBeenCalledWith(authStub.admin.user.id, new Set(['person-1']));
@@ -286,7 +286,7 @@ describe(PersonService.name, () => {
mocks.person.getById.mockResolvedValue(person);
mocks.access.person.checkOwnerAccess.mockResolvedValue(new Set([person.id]));
await expect(sut.update(auth, person.id, { featureFaceAssetId: '-1' })).rejects.toThrow(BadRequestException);
await expect(sut.update(auth, person.id, { featureFaceAssetId: '-1' })).rejects.toThrow(ForbiddenException);
expect(mocks.person.update).not.toHaveBeenCalled();
expect(mocks.access.person.checkOwnerAccess).toHaveBeenCalledWith(auth.user.id, new Set([person.id]));
});
@@ -312,7 +312,7 @@ describe(PersonService.name, () => {
sut.reassignFaces(AuthFactory.create(), 'person-id', {
data: [{ personId: 'asset-face-1', assetId: '' }],
}),
).rejects.toBeInstanceOf(BadRequestException);
).rejects.toBeInstanceOf(ForbiddenException);
expect(mocks.job.queue).not.toHaveBeenCalledWith();
expect(mocks.job.queueAll).not.toHaveBeenCalledWith();
});
@@ -371,7 +371,7 @@ describe(PersonService.name, () => {
mocks.access.asset.checkOwnerAccess.mockResolvedValue(new Set());
mocks.person.getFaces.mockResolvedValue([getForAssetFace(face)]);
await expect(sut.getFacesById(AuthFactory.create(), { id: face.assetId })).rejects.toBeInstanceOf(
BadRequestException,
ForbiddenException,
);
});
});
@@ -507,7 +507,7 @@ describe(PersonService.name, () => {
sut.reassignFacesById(AuthFactory.create(), person.id, {
id: face.id,
}),
).rejects.toBeInstanceOf(BadRequestException);
).rejects.toBeInstanceOf(ForbiddenException);
expect(mocks.job.queue).not.toHaveBeenCalledWith();
expect(mocks.job.queueAll).not.toHaveBeenCalledWith();
@@ -1189,7 +1189,7 @@ describe(PersonService.name, () => {
mocks.person.getById.mockResolvedValueOnce(mergePerson);
await expect(sut.mergePerson(auth, person.id, { ids: [mergePerson.id] })).rejects.toBeInstanceOf(
BadRequestException,
ForbiddenException,
);
expect(mocks.person.reassignFaces).not.toHaveBeenCalled();
@@ -1313,7 +1313,7 @@ describe(PersonService.name, () => {
const person = PersonFactory.create();
mocks.person.getById.mockResolvedValue(person);
await expect(sut.getStatistics(auth, person.id)).rejects.toBeInstanceOf(BadRequestException);
await expect(sut.getStatistics(auth, person.id)).rejects.toBeInstanceOf(ForbiddenException);
expect(mocks.access.person.checkOwnerAccess).toHaveBeenCalledWith(auth.user.id, new Set([person.id]));
});
});
server/src/services/person.service.ts
+2 -2
View File
@@ -59,7 +59,7 @@ export class PersonService extends BaseService {
if (closestPersonId) {
const person = await this.personRepository.getById(closestPersonId);
if (!person?.faceAssetId) {
throw new NotFoundException('Person not found');
throw new NotFoundException('Not found');
}
closestFaceAssetId = person.faceAssetId;
}
@@ -637,7 +637,7 @@ export class PersonService extends BaseService {
]);
if (!asset) {
throw new NotFoundException('Asset not found');
throw new NotFoundException('Not found');
}
const edits = asset.edits || [];
server/src/services/shared-link.service.spec.ts
+2 -2
View File
@@ -123,7 +123,7 @@ describe(SharedLinkService.name, () => {
it('should not allow non-owners to create album shared links', async () => {
await expect(
sut.create(authStub.admin, { type: SharedLinkType.Album, assetIds: [], albumId: 'album-1' }),
).rejects.toBeInstanceOf(BadRequestException);
).rejects.toBeInstanceOf(ForbiddenException);
});
it('should not allow individual shared links with no assets', async () => {
@@ -135,7 +135,7 @@ describe(SharedLinkService.name, () => {
it('should require asset ownership to make an individual shared link', async () => {
await expect(
sut.create(authStub.admin, { type: SharedLinkType.Individual, assetIds: ['asset-1'] }),
).rejects.toBeInstanceOf(BadRequestException);
).rejects.toBeInstanceOf(ForbiddenException);
});
it('should create an album shared link', async () => {
server/src/services/stack.service.spec.ts
+7 -7
View File
@@ -1,4 +1,4 @@
import { BadRequestException } from '@nestjs/common';
import { BadRequestException, ForbiddenException } from '@nestjs/common';
import { StackService } from 'src/services/stack.service';
import { AssetFactory } from 'test/factories/asset.factory';
import { AuthFactory } from 'test/factories/auth.factory';
@@ -43,7 +43,7 @@ describe(StackService.name, () => {
const [primaryAsset, asset] = [AssetFactory.create(), AssetFactory.create()];
await expect(sut.create(auth, { assetIds: [primaryAsset.id, asset.id] })).rejects.toBeInstanceOf(
BadRequestException,
ForbiddenException,
);
expect(mocks.access.asset.checkOwnerAccess).toHaveBeenCalled();
@@ -77,7 +77,7 @@ describe(StackService.name, () => {
describe('get', () => {
it('should require stack.read permissions', async () => {
await expect(sut.get(authStub.admin, 'stack-id')).rejects.toBeInstanceOf(BadRequestException);
await expect(sut.get(authStub.admin, 'stack-id')).rejects.toBeInstanceOf(ForbiddenException);
expect(mocks.access.stack.checkOwnerAccess).toHaveBeenCalled();
expect(mocks.stack.getById).not.toHaveBeenCalled();
@@ -115,7 +115,7 @@ describe(StackService.name, () => {
describe('update', () => {
it('should require stack.update permissions', async () => {
await expect(sut.update(AuthFactory.create(), 'stack-id', {})).rejects.toBeInstanceOf(BadRequestException);
await expect(sut.update(AuthFactory.create(), 'stack-id', {})).rejects.toBeInstanceOf(ForbiddenException);
expect(mocks.stack.getById).not.toHaveBeenCalled();
expect(mocks.stack.update).not.toHaveBeenCalled();
@@ -179,7 +179,7 @@ describe(StackService.name, () => {
describe('delete', () => {
it('should require stack.delete permissions', async () => {
await expect(sut.delete(AuthFactory.create(), 'stack-id')).rejects.toBeInstanceOf(BadRequestException);
await expect(sut.delete(AuthFactory.create(), 'stack-id')).rejects.toBeInstanceOf(ForbiddenException);
expect(mocks.stack.delete).not.toHaveBeenCalled();
expect(mocks.event.emit).not.toHaveBeenCalled();
@@ -203,7 +203,7 @@ describe(StackService.name, () => {
describe('deleteAll', () => {
it('should require stack.delete permissions', async () => {
await expect(sut.deleteAll(authStub.admin, { ids: ['stack-id'] })).rejects.toBeInstanceOf(BadRequestException);
await expect(sut.deleteAll(authStub.admin, { ids: ['stack-id'] })).rejects.toBeInstanceOf(ForbiddenException);
expect(mocks.stack.deleteAll).not.toHaveBeenCalled();
expect(mocks.event.emit).not.toHaveBeenCalled();
@@ -226,7 +226,7 @@ describe(StackService.name, () => {
describe('removeAsset', () => {
it('should require stack.update permissions', async () => {
await expect(sut.removeAsset(authStub.admin, { id: 'stack-id', assetId: 'asset-id' })).rejects.toBeInstanceOf(
BadRequestException,
ForbiddenException,
);
expect(mocks.stack.getForAssetRemoval).not.toHaveBeenCalled();
server/src/services/tag.service.spec.ts
+4 -4
View File
@@ -1,4 +1,4 @@
import { BadRequestException } from '@nestjs/common';
import { BadRequestException, ForbiddenException } from '@nestjs/common';
import { BulkIdErrorReason } from 'src/dtos/asset-ids.response.dto';
import { JobStatus } from 'src/enum';
import { TagService } from 'src/services/tag.service';
@@ -45,7 +45,7 @@ describe(TagService.name, () => {
it('should throw an error for no parent tag access', async () => {
mocks.access.tag.checkOwnerAccess.mockResolvedValue(new Set());
await expect(sut.create(authStub.admin, { name: 'tag', parentId: 'tag-parent' })).rejects.toBeInstanceOf(
BadRequestException,
ForbiddenException,
);
expect(mocks.tag.create).not.toHaveBeenCalled();
});
@@ -105,7 +105,7 @@ describe(TagService.name, () => {
it('should throw an error for no update permission', async () => {
mocks.access.tag.checkOwnerAccess.mockResolvedValue(new Set());
await expect(sut.update(authStub.admin, 'tag-1', { color: '#000000' })).rejects.toBeInstanceOf(
BadRequestException,
ForbiddenException,
);
expect(mocks.tag.update).not.toHaveBeenCalled();
});
@@ -165,7 +165,7 @@ describe(TagService.name, () => {
describe('remove', () => {
it('should throw an error for an invalid id', async () => {
mocks.access.tag.checkOwnerAccess.mockResolvedValue(new Set());
await expect(sut.remove(authStub.admin, 'tag-1')).rejects.toBeInstanceOf(BadRequestException);
await expect(sut.remove(authStub.admin, 'tag-1')).rejects.toBeInstanceOf(ForbiddenException);
expect(mocks.tag.delete).not.toHaveBeenCalled();
});
server/src/services/trash.service.spec.ts
+2 -2
View File
@@ -1,4 +1,4 @@
import { BadRequestException } from '@nestjs/common';
import { ForbiddenException } from '@nestjs/common';
import { JobName, JobStatus } from 'src/enum';
import { TrashService } from 'src/services/trash.service';
import { authStub } from 'test/fixtures/auth.stub';
@@ -29,7 +29,7 @@ describe(TrashService.name, () => {
sut.restoreAssets(authStub.user1, {
ids: ['asset-1'],
}),
).rejects.toBeInstanceOf(BadRequestException);
).rejects.toBeInstanceOf(ForbiddenException);
});
it('should handle an empty list', async () => {
server/src/services/user.service.ts
+1 -1
View File
@@ -136,7 +136,7 @@ export class UserService extends BaseService {
async getProfileImage(id: string): Promise<ImmichFileResponse> {
const user = await this.findOrFail(id, {});
if (!user.profileImagePath) {
throw new NotFoundException('User does not have a profile image');
throw new NotFoundException('Not found');
}
return new ImmichFileResponse({
server/src/utils/access.ts
+2 -2
View File
@@ -1,4 +1,4 @@
import { BadRequestException, UnauthorizedException } from '@nestjs/common';
import { ForbiddenException, UnauthorizedException } from '@nestjs/common';
import { AuthSharedLink } from 'src/database';
import { AuthDto } from 'src/dtos/auth.dto';
import { AlbumUserRole, Permission } from 'src/enum';
@@ -37,7 +37,7 @@ export const requireUploadAccess = (auth: AuthDto | null): AuthDto => {
export const requireAccess = async (access: AccessRepository, request: AccessRequest) => {
const allowedIds = await checkAccess(access, request);
if (!setIsEqual(new Set(request.ids), allowedIds)) {
throw new BadRequestException(`Not found or no ${request.permission} access`);
throw new ForbiddenException('Access denied');
}
};
server/test/medium/responses.ts
+4 -4
View File
@@ -7,9 +7,9 @@ export const errorDto = {
forbidden: {
message: expect.any(String),
},
missingPermission: (permission: string) => ({
message: `Missing required permission: ${permission}`,
}),
missingPermission: {
message: 'Access denied',
},
wrongPassword: {
message: 'Wrong password',
},
@@ -26,7 +26,7 @@ export const errorDto = {
message: message ?? expect.anything(),
}),
noPermission: {
message: expect.stringContaining('Not found or no'),
message: 'Access denied',
},
incorrectLogin: {
message: 'Incorrect email or password',
server/test/medium/specs/services/asset.service.spec.ts
+3 -3
View File
@@ -598,7 +598,7 @@ describe(AssetService.name, () => {
const auth = factory.auth({ user });
const { asset } = await ctx.newAsset({ ownerId: user2.id });
await expect(sut.getOcr(auth, asset.id)).rejects.toThrow('Not found or no asset.read access');
await expect(sut.getOcr(auth, asset.id)).rejects.toThrow('Access denied');
});
it('should work', async () => {
@@ -649,7 +649,7 @@ describe(AssetService.name, () => {
const auth = factory.auth({ user });
const { asset } = await ctx.newAsset({ ownerId: user2.id });
await expect(sut.getOcr(auth, asset.id)).rejects.toThrow('Not found or no asset.read access');
await expect(sut.getOcr(auth, asset.id)).rejects.toThrow('Access denied');
});
it('should work', async () => {
@@ -875,7 +875,7 @@ describe(AssetService.name, () => {
await expect(
sut.editAsset(auth, asset.id, { edits: [{ action: AssetEditAction.Rotate, parameters: { angle: 90 } }] }),
).rejects.toThrow('Not found or no asset.edit.create access');
).rejects.toThrow('Access denied');
});
it('should work', async () => {
server/test/medium/specs/services/person.service.spec.ts
+2 -2
View File
@@ -35,7 +35,7 @@ describe(PersonService.name, () => {
const { sut } = setup();
const auth = factory.auth();
const personId = factory.uuid();
await expect(sut.delete(auth, personId)).rejects.toThrow('Not found or no person.delete access');
await expect(sut.delete(auth, personId)).rejects.toThrow('Access denied');
});
it('should delete the person', async () => {
@@ -60,7 +60,7 @@ describe(PersonService.name, () => {
const { sut } = setup();
const auth = factory.auth();
const personId = factory.uuid();
await expect(sut.deleteAll(auth, { ids: [personId] })).rejects.toThrow('Not found or no person.delete access');
await expect(sut.deleteAll(auth, { ids: [personId] })).rejects.toThrow('Access denied');
});
it('should delete the person', async () => {
server/test/medium/specs/services/timeline.service.spec.ts
+3 -3
View File
@@ -1,4 +1,4 @@
import { BadRequestException } from '@nestjs/common';
import { BadRequestException, ForbiddenException } from '@nestjs/common';
import { Kysely } from 'kysely';
import { AssetVisibility } from 'src/enum';
import { AccessRepository } from 'src/repositories/access.repository';
@@ -90,8 +90,8 @@ describe(TimelineService.name, () => {
const { sut } = setup();
const auth = factory.auth({ sharedLink: {} });
const response = sut.getTimeBuckets(auth, {});
await expect(response).rejects.toBeInstanceOf(BadRequestException);
await expect(response).rejects.toThrow('Not found or no timeline.read access');
await expect(response).rejects.toBeInstanceOf(ForbiddenException);
await expect(response).rejects.toThrow('Access denied');
});
});
server/test/medium/specs/services/workflow.service.spec.ts
+2 -2
View File
@@ -724,7 +724,7 @@ describe(WorkflowService.name, () => {
await sut.delete(auth, workflow.id);
await expect(sut.get(auth, workflow.id)).rejects.toThrow('Not found or no workflow.read access');
await expect(sut.get(auth, workflow.id)).rejects.toThrow('Access denied');
});
it('should delete workflow with filters and actions', async () => {
@@ -743,7 +743,7 @@ describe(WorkflowService.name, () => {
await sut.delete(auth, workflow.id);
await expect(sut.get(auth, workflow.id)).rejects.toThrow('Not found or no workflow.read access');
await expect(sut.get(auth, workflow.id)).rejects.toThrow('Access denied');
});
it('should throw error when deleting non-existent workflow', async () => {