refactor(server): move asset checks to service (#2640)

This commit is contained in:
Jason Rasmussen 2023-06-06 15:17:15 -04:00 committed by GitHub
parent 1e748864c5
commit d1db479727
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 87 additions and 69 deletions

View file

@ -62,6 +62,12 @@ function asStreamableFile({ stream, type, length }: ImmichReadStream) {
return new StreamableFile(stream, { type, length });
}
interface UploadFiles {
assetData: ImmichFile[];
livePhotoData?: ImmichFile[];
sidecarData: ImmichFile[];
}
@ApiTags('Asset')
@Controller('asset')
@Authenticated()
@ -87,8 +93,7 @@ export class AssetController {
})
async uploadFile(
@GetAuthUser() authUser: AuthUserDto,
@UploadedFiles(new ParseFilePipe({ validators: [new FileNotEmptyValidator(['assetData'])] }))
files: { assetData: ImmichFile[]; livePhotoData?: ImmichFile[]; sidecarData: ImmichFile[] },
@UploadedFiles(new ParseFilePipe({ validators: [new FileNotEmptyValidator(['assetData'])] })) files: UploadFiles,
@Body(new ValidationPipe()) dto: CreateAssetDto,
@Response({ passthrough: true }) res: Res,
): Promise<AssetFileUploadResponseDto> {
@ -116,25 +121,19 @@ export class AssetController {
@SharedLinkRoute()
@Get('/download/:id')
@ApiOkResponse({ content: { 'application/octet-stream': { schema: { type: 'string', format: 'binary' } } } })
async downloadFile(
@GetAuthUser() authUser: AuthUserDto,
@Response({ passthrough: true }) res: Res,
@Param() { id }: UUIDParamDto,
) {
downloadFile(@GetAuthUser() authUser: AuthUserDto, @Param() { id }: UUIDParamDto) {
return this.assetService.downloadFile(authUser, id).then(asStreamableFile);
}
@SharedLinkRoute()
@Post('/download-files')
@ApiOkResponse({ content: { 'application/octet-stream': { schema: { type: 'string', format: 'binary' } } } })
async downloadFiles(
downloadFiles(
@GetAuthUser() authUser: AuthUserDto,
@Response({ passthrough: true }) res: Res,
@Body(new ValidationPipe()) dto: DownloadFilesDto,
) {
this.assetService.checkDownloadAccess(authUser);
await this.assetService.checkAssetsAccess(authUser, [...dto.assetIds]);
return this.assetService.downloadFiles(dto).then((download) => handleDownload(download, res));
return this.assetService.downloadFiles(authUser, dto).then((download) => handleDownload(download, res));
}
/**
@ -143,12 +142,11 @@ export class AssetController {
@SharedLinkRoute()
@Get('/download-library')
@ApiOkResponse({ content: { 'application/octet-stream': { schema: { type: 'string', format: 'binary' } } } })
async downloadLibrary(
downloadLibrary(
@GetAuthUser() authUser: AuthUserDto,
@Query(new ValidationPipe({ transform: true })) dto: DownloadDto,
@Response({ passthrough: true }) res: Res,
) {
this.assetService.checkDownloadAccess(authUser);
return this.assetService.downloadLibrary(authUser, dto).then((download) => handleDownload(download, res));
}
@ -156,14 +154,13 @@ export class AssetController {
@Get('/file/:id')
@Header('Cache-Control', 'max-age=31536000')
@ApiOkResponse({ content: { 'application/octet-stream': { schema: { type: 'string', format: 'binary' } } } })
async serveFile(
serveFile(
@GetAuthUser() authUser: AuthUserDto,
@Headers() headers: Record<string, string>,
@Response({ passthrough: true }) res: Res,
@Query(new ValidationPipe({ transform: true })) query: ServeFileDto,
@Param() { id }: UUIDParamDto,
) {
await this.assetService.checkAssetsAccess(authUser, [id]);
return this.assetService.serveFile(authUser, id, query, res, headers);
}
@ -171,55 +168,54 @@ export class AssetController {
@Get('/thumbnail/:id')
@Header('Cache-Control', 'max-age=31536000')
@ApiOkResponse({ content: { 'application/octet-stream': { schema: { type: 'string', format: 'binary' } } } })
async getAssetThumbnail(
getAssetThumbnail(
@GetAuthUser() authUser: AuthUserDto,
@Headers() headers: Record<string, string>,
@Response({ passthrough: true }) res: Res,
@Param() { id }: UUIDParamDto,
@Query(new ValidationPipe({ transform: true })) query: GetAssetThumbnailDto,
) {
await this.assetService.checkAssetsAccess(authUser, [id]);
return this.assetService.getAssetThumbnail(id, query, res, headers);
return this.assetService.getAssetThumbnail(authUser, id, query, res, headers);
}
@Get('/curated-objects')
async getCuratedObjects(@GetAuthUser() authUser: AuthUserDto): Promise<CuratedObjectsResponseDto[]> {
getCuratedObjects(@GetAuthUser() authUser: AuthUserDto): Promise<CuratedObjectsResponseDto[]> {
return this.assetService.getCuratedObject(authUser);
}
@Get('/curated-locations')
async getCuratedLocations(@GetAuthUser() authUser: AuthUserDto): Promise<CuratedLocationsResponseDto[]> {
getCuratedLocations(@GetAuthUser() authUser: AuthUserDto): Promise<CuratedLocationsResponseDto[]> {
return this.assetService.getCuratedLocation(authUser);
}
@Get('/search-terms')
async getAssetSearchTerms(@GetAuthUser() authUser: AuthUserDto): Promise<string[]> {
getAssetSearchTerms(@GetAuthUser() authUser: AuthUserDto): Promise<string[]> {
return this.assetService.getAssetSearchTerm(authUser);
}
@Post('/search')
async searchAsset(
searchAsset(
@GetAuthUser() authUser: AuthUserDto,
@Body(ValidationPipe) searchAssetDto: SearchAssetDto,
@Body(ValidationPipe) dto: SearchAssetDto,
): Promise<AssetResponseDto[]> {
return this.assetService.searchAsset(authUser, searchAssetDto);
return this.assetService.searchAsset(authUser, dto);
}
@Post('/count-by-time-bucket')
async getAssetCountByTimeBucket(
getAssetCountByTimeBucket(
@GetAuthUser() authUser: AuthUserDto,
@Body(ValidationPipe) getAssetCountByTimeGroupDto: GetAssetCountByTimeBucketDto,
@Body(ValidationPipe) dto: GetAssetCountByTimeBucketDto,
): Promise<AssetCountByTimeBucketResponseDto> {
return this.assetService.getAssetCountByTimeBucket(authUser, getAssetCountByTimeGroupDto);
return this.assetService.getAssetCountByTimeBucket(authUser, dto);
}
@Get('/count-by-user-id')
async getAssetCountByUserId(@GetAuthUser() authUser: AuthUserDto): Promise<AssetCountByUserIdResponseDto> {
getAssetCountByUserId(@GetAuthUser() authUser: AuthUserDto): Promise<AssetCountByUserIdResponseDto> {
return this.assetService.getAssetCountByUserId(authUser);
}
@Get('/stat/archive')
async getArchivedAssetCountByUserId(@GetAuthUser() authUser: AuthUserDto): Promise<AssetCountByUserIdResponseDto> {
getArchivedAssetCountByUserId(@GetAuthUser() authUser: AuthUserDto): Promise<AssetCountByUserIdResponseDto> {
return this.assetService.getArchivedAssetCountByUserId(authUser);
}
/**
@ -240,19 +236,19 @@ export class AssetController {
}
@Post('/time-bucket')
async getAssetByTimeBucket(
getAssetByTimeBucket(
@GetAuthUser() authUser: AuthUserDto,
@Body(ValidationPipe) getAssetByTimeBucketDto: GetAssetByTimeBucketDto,
@Body(ValidationPipe) dto: GetAssetByTimeBucketDto,
): Promise<AssetResponseDto[]> {
return await this.assetService.getAssetByTimeBucket(authUser, getAssetByTimeBucketDto);
return this.assetService.getAssetByTimeBucket(authUser, dto);
}
/**
* Get all asset of a device that are in the database, ID only.
*/
@Get('/:deviceId')
async getUserAssetsByDeviceId(@GetAuthUser() authUser: AuthUserDto, @Param() { deviceId }: DeviceIdDto) {
return await this.assetService.getUserAssetsByDeviceId(authUser, deviceId);
getUserAssetsByDeviceId(@GetAuthUser() authUser: AuthUserDto, @Param() { deviceId }: DeviceIdDto) {
return this.assetService.getUserAssetsByDeviceId(authUser, deviceId);
}
/**
@ -260,30 +256,27 @@ export class AssetController {
*/
@SharedLinkRoute()
@Get('/assetById/:id')
async getAssetById(@GetAuthUser() authUser: AuthUserDto, @Param() { id }: UUIDParamDto): Promise<AssetResponseDto> {
await this.assetService.checkAssetsAccess(authUser, [id]);
return await this.assetService.getAssetById(authUser, id);
getAssetById(@GetAuthUser() authUser: AuthUserDto, @Param() { id }: UUIDParamDto): Promise<AssetResponseDto> {
return this.assetService.getAssetById(authUser, id);
}
/**
* Update an asset
*/
@Put('/:id')
async updateAsset(
updateAsset(
@GetAuthUser() authUser: AuthUserDto,
@Param() { id }: UUIDParamDto,
@Body(ValidationPipe) dto: UpdateAssetDto,
): Promise<AssetResponseDto> {
await this.assetService.checkAssetsAccess(authUser, [id], true);
return await this.assetService.updateAsset(authUser, id, dto);
return this.assetService.updateAsset(authUser, id, dto);
}
@Delete('/')
async deleteAsset(
deleteAsset(
@GetAuthUser() authUser: AuthUserDto,
@Body(ValidationPipe) dto: DeleteAssetDto,
): Promise<DeleteAssetResponseDto[]> {
await this.assetService.checkAssetsAccess(authUser, dto.ids, true);
return this.assetService.deleteAll(authUser, dto);
}
@ -293,11 +286,11 @@ export class AssetController {
@SharedLinkRoute()
@Post('/check')
@HttpCode(200)
async checkDuplicateAsset(
checkDuplicateAsset(
@GetAuthUser() authUser: AuthUserDto,
@Body(ValidationPipe) checkDuplicateAssetDto: CheckDuplicateAssetDto,
@Body(ValidationPipe) dto: CheckDuplicateAssetDto,
): Promise<CheckDuplicateAssetResponseDto> {
return await this.assetService.checkDuplicatedAsset(authUser, checkDuplicateAssetDto);
return this.assetService.checkDuplicatedAsset(authUser, dto);
}
/**
@ -305,11 +298,11 @@ export class AssetController {
*/
@Post('/exist')
@HttpCode(200)
async checkExistingAssets(
checkExistingAssets(
@GetAuthUser() authUser: AuthUserDto,
@Body(ValidationPipe) checkExistingAssetsDto: CheckExistingAssetsDto,
@Body(ValidationPipe) dto: CheckExistingAssetsDto,
): Promise<CheckExistingAssetsResponseDto> {
return await this.assetService.checkExistingAssets(authUser, checkExistingAssetsDto);
return this.assetService.checkExistingAssets(authUser, dto);
}
/**
@ -325,28 +318,28 @@ export class AssetController {
}
@Post('/shared-link')
async createAssetsSharedLink(
createAssetsSharedLink(
@GetAuthUser() authUser: AuthUserDto,
@Body(ValidationPipe) dto: CreateAssetsShareLinkDto,
): Promise<SharedLinkResponseDto> {
return await this.assetService.createAssetsSharedLink(authUser, dto);
return this.assetService.createAssetsSharedLink(authUser, dto);
}
@SharedLinkRoute()
@Patch('/shared-link/add')
async addAssetsToSharedLink(
addAssetsToSharedLink(
@GetAuthUser() authUser: AuthUserDto,
@Body(ValidationPipe) dto: AddAssetsDto,
): Promise<SharedLinkResponseDto> {
return await this.assetService.addAssetsToSharedLink(authUser, dto);
return this.assetService.addAssetsToSharedLink(authUser, dto);
}
@SharedLinkRoute()
@Patch('/shared-link/remove')
async removeAssetsFromSharedLink(
removeAssetsFromSharedLink(
@GetAuthUser() authUser: AuthUserDto,
@Body(ValidationPipe) dto: RemoveAssetsDto,
): Promise<SharedLinkResponseDto> {
return await this.assetService.removeAssetsFromSharedLink(authUser, dto);
return this.assetService.removeAssetsFromSharedLink(authUser, dto);
}
}

View file

@ -28,7 +28,7 @@ import {
sharedLinkStub,
} from '@app/domain/../test';
import { CreateAssetsShareLinkDto } from './dto/create-asset-shared-link.dto';
import { BadRequestException, ForbiddenException } from '@nestjs/common';
import { BadRequestException } from '@nestjs/common';
import { when } from 'jest-when';
import { AssetRejectReason, AssetUploadAction } from './response-dto/asset-check-response.dto';
@ -387,6 +387,7 @@ describe('AssetService', () => {
describe('deleteAll', () => {
it('should return failed status when an asset is missing', async () => {
assetRepositoryMock.get.mockResolvedValue(null);
assetRepositoryMock.countByIdAndUser.mockResolvedValue(1);
await expect(sut.deleteAll(authStub.user1, { ids: ['asset1'] })).resolves.toEqual([
{ id: 'asset1', status: 'FAILED' },
@ -398,6 +399,7 @@ describe('AssetService', () => {
it('should return failed status a delete fails', async () => {
assetRepositoryMock.get.mockResolvedValue({ id: 'asset1' } as AssetEntity);
assetRepositoryMock.remove.mockRejectedValue('delete failed');
assetRepositoryMock.countByIdAndUser.mockResolvedValue(1);
await expect(sut.deleteAll(authStub.user1, { ids: ['asset1'] })).resolves.toEqual([
{ id: 'asset1', status: 'FAILED' },
@ -407,6 +409,8 @@ describe('AssetService', () => {
});
it('should delete a live photo', async () => {
assetRepositoryMock.countByIdAndUser.mockResolvedValue(1);
await expect(sut.deleteAll(authStub.user1, { ids: [assetEntityStub.livePhotoStillAsset.id] })).resolves.toEqual([
{ id: assetEntityStub.livePhotoStillAsset.id, status: 'SUCCESS' },
{ id: assetEntityStub.livePhotoMotionAsset.id, status: 'SUCCESS' },
@ -454,6 +458,8 @@ describe('AssetService', () => {
.calledWith(asset2.id)
.mockResolvedValue(asset2 as AssetEntity);
assetRepositoryMock.countByIdAndUser.mockResolvedValue(1);
await expect(sut.deleteAll(authStub.user1, { ids: ['asset1', 'asset2'] })).resolves.toEqual([
{ id: 'asset1', status: 'SUCCESS' },
{ id: 'asset2', status: 'SUCCESS' },
@ -485,15 +491,15 @@ describe('AssetService', () => {
});
});
describe('checkDownloadAccess', () => {
it('should validate download access', async () => {
await sut.checkDownloadAccess(authStub.adminSharedLink);
});
// describe('checkDownloadAccess', () => {
// it('should validate download access', async () => {
// await sut.checkDownloadAccess(authStub.adminSharedLink);
// });
it('should not allow when user is not allowed to download', async () => {
expect(() => sut.checkDownloadAccess(authStub.readonlySharedLink)).toThrow(ForbiddenException);
});
});
// it('should not allow when user is not allowed to download', async () => {
// expect(() => sut.checkDownloadAccess(authStub.readonlySharedLink)).toThrow(ForbiddenException);
// });
// });
describe('downloadFile', () => {
it('should download a single file', async () => {

View file

@ -175,6 +175,8 @@ export class AssetService {
}
public async getAssetById(authUser: AuthUserDto, assetId: string): Promise<AssetResponseDto> {
await this.checkAssetsAccess(authUser, [assetId]);
const allowExif = this.getExifPermission(authUser);
const asset = await this._assetRepository.getById(assetId);
@ -186,6 +188,8 @@ export class AssetService {
}
public async updateAsset(authUser: AuthUserDto, assetId: string, dto: UpdateAssetDto): Promise<AssetResponseDto> {
await this.checkAssetsAccess(authUser, [assetId], true);
const asset = await this._assetRepository.getById(assetId);
if (!asset) {
throw new BadRequestException('Asset not found');
@ -198,13 +202,17 @@ export class AssetService {
return mapAsset(updatedAsset);
}
public async downloadLibrary(user: AuthUserDto, dto: DownloadDto) {
const assets = await this._assetRepository.getAllByUserId(user.id, dto);
public async downloadLibrary(authUser: AuthUserDto, dto: DownloadDto) {
this.checkDownloadAccess(authUser);
const assets = await this._assetRepository.getAllByUserId(authUser.id, dto);
return this.downloadService.downloadArchive(dto.name || `library`, assets);
}
public async downloadFiles(dto: DownloadFilesDto) {
public async downloadFiles(authUser: AuthUserDto, dto: DownloadFilesDto) {
this.checkDownloadAccess(authUser);
await this.checkAssetsAccess(authUser, [...dto.assetIds]);
const assetToDownload = [];
for (const assetId of dto.assetIds) {
@ -239,7 +247,14 @@ export class AssetService {
throw new NotFoundException();
}
async getAssetThumbnail(assetId: string, query: GetAssetThumbnailDto, res: Res, headers: Record<string, string>) {
async getAssetThumbnail(
authUser: AuthUserDto,
assetId: string,
query: GetAssetThumbnailDto,
res: Res,
headers: Record<string, string>,
) {
await this.checkAssetsAccess(authUser, [assetId]);
const asset = await this._assetRepository.get(assetId);
if (!asset) {
throw new NotFoundException('Asset not found');
@ -265,6 +280,8 @@ export class AssetService {
res: Res,
headers: Record<string, string>,
) {
await this.checkAssetsAccess(authUser, [assetId]);
const allowOriginalFile = !!(!authUser.isPublicUser || authUser.isAllowDownload);
const asset = await this._assetRepository.getById(assetId);
@ -346,6 +363,8 @@ export class AssetService {
}
public async deleteAll(authUser: AuthUserDto, dto: DeleteAssetDto): Promise<DeleteAssetResponseDto[]> {
await this.checkAssetsAccess(authUser, dto.ids, true);
const deleteQueue: Array<string | null> = [];
const result: DeleteAssetResponseDto[] = [];
@ -547,7 +566,7 @@ export class AssetService {
return this._assetRepository.getArchivedAssetCountByUserId(authUser.id);
}
async checkAssetsAccess(authUser: AuthUserDto, assetIds: string[], mustBeOwner = false) {
private async checkAssetsAccess(authUser: AuthUserDto, assetIds: string[], mustBeOwner = false) {
for (const assetId of assetIds) {
// Step 1: Check if asset is part of a public shared
if (authUser.sharedLinkId) {
@ -587,7 +606,7 @@ export class AssetService {
}
}
checkDownloadAccess(authUser: AuthUserDto) {
private checkDownloadAccess(authUser: AuthUserDto) {
this.shareCore.checkDownloadAccess(authUser);
}