feat(web,server): user storage label (#2418)

* feat: user storage label

* chore: open api

* fix: checks

* fix: api update validation and tests

* feat: default admin storage label

* fix: linting

* fix: user create/update dto

* fix: delete library with custom label
This commit is contained in:
Jason Rasmussen
2023-05-21 23:18:10 -04:00
committed by GitHub
parent 0ccb73cf2b
commit 74353193f8
43 changed files with 452 additions and 137 deletions

View File

@@ -306,7 +306,7 @@ describe('AuthService', () => {
expect(userTokenMock.save.mock.calls[0][0]).toMatchObject({
id: 'not_active',
token: 'auth_token',
userId: 'immich_id',
userId: 'user-id',
createdAt: new Date('2021-01-01'),
updatedAt: expect.any(Date),
deviceOS: 'Android',

View File

@@ -122,6 +122,7 @@ export class AuthService {
firstName: dto.firstName,
lastName: dto.lastName,
password: dto.password,
storageLabel: 'admin',
});
return mapAdminSignupResponse(admin);

View File

@@ -17,19 +17,21 @@ const responseDto = {
profileImagePath: '',
shouldChangePassword: false,
updatedAt: '2021-01-01',
storageLabel: 'admin',
},
user1: {
createdAt: '2021-01-01',
deletedAt: undefined,
email: 'immich@test.com',
firstName: 'immich_first_name',
id: 'immich_id',
id: 'user-id',
isAdmin: false,
lastName: 'immich_last_name',
oauthId: '',
profileImagePath: '',
shouldChangePassword: false,
updatedAt: '2021-01-01',
storageLabel: null,
},
};

View File

@@ -4,7 +4,7 @@ import handlebar from 'handlebars';
import * as luxon from 'luxon';
import path from 'node:path';
import sanitize from 'sanitize-filename';
import { IStorageRepository, StorageCore, StorageFolder } from '../storage';
import { IStorageRepository, StorageCore } from '../storage';
import {
ISystemConfigRepository,
supportedDayTokens,
@@ -15,6 +15,7 @@ import {
supportedYearTokens,
} from '../system-config';
import { SystemConfigCore } from '../system-config/system-config.core';
import { MoveAssetMetadata } from './storage-template.service';
export class StorageTemplateCore {
private logger = new Logger(StorageTemplateCore.name);
@@ -33,12 +34,14 @@ export class StorageTemplateCore {
this.configCore.config$.subscribe((config) => this.onConfig(config));
}
public async getTemplatePath(asset: AssetEntity, filename: string): Promise<string> {
public async getTemplatePath(asset: AssetEntity, metadata: MoveAssetMetadata): Promise<string> {
const { storageLabel, filename } = metadata;
try {
const source = asset.originalPath;
const ext = path.extname(source).split('.').pop() as string;
const sanitized = sanitize(path.basename(filename, `.${ext}`));
const rootPath = this.storageCore.getFolderLocation(StorageFolder.LIBRARY, asset.ownerId);
const rootPath = this.storageCore.getLibraryFolder({ id: asset.ownerId, storageLabel });
const storagePath = this.render(this.storageTemplate, asset, sanitized, ext);
const fullPath = path.normalize(path.join(rootPath, storagePath));
let destination = `${fullPath}.${ext}`;

View File

@@ -4,18 +4,22 @@ import {
newAssetRepositoryMock,
newStorageRepositoryMock,
newSystemConfigRepositoryMock,
newUserRepositoryMock,
systemConfigStub,
userEntityStub,
} from '../../test';
import { IAssetRepository } from '../asset';
import { StorageTemplateService } from '../storage-template';
import { IStorageRepository } from '../storage/storage.repository';
import { ISystemConfigRepository } from '../system-config';
import { IUserRepository } from '../user';
describe(StorageTemplateService.name, () => {
let sut: StorageTemplateService;
let assetMock: jest.Mocked<IAssetRepository>;
let configMock: jest.Mocked<ISystemConfigRepository>;
let storageMock: jest.Mocked<IStorageRepository>;
let userMock: jest.Mocked<IUserRepository>;
it('should work', () => {
expect(sut).toBeDefined();
@@ -25,12 +29,15 @@ describe(StorageTemplateService.name, () => {
assetMock = newAssetRepositoryMock();
configMock = newSystemConfigRepositoryMock();
storageMock = newStorageRepositoryMock();
sut = new StorageTemplateService(assetMock, configMock, systemConfigStub.defaults, storageMock);
userMock = newUserRepositoryMock();
sut = new StorageTemplateService(assetMock, configMock, systemConfigStub.defaults, storageMock, userMock);
});
describe('handle template migration', () => {
it('should handle no assets', async () => {
assetMock.getAll.mockResolvedValue([]);
userMock.getList.mockResolvedValue([]);
await sut.handleTemplateMigration();
@@ -40,6 +47,7 @@ describe(StorageTemplateService.name, () => {
it('should handle an asset with a duplicate destination', async () => {
assetMock.getAll.mockResolvedValue([assetEntityStub.image]);
assetMock.save.mockResolvedValue(assetEntityStub.image);
userMock.getList.mockResolvedValue([userEntityStub.user1]);
when(storageMock.checkFileExists)
.calledWith('upload/library/user-id/2023/2023-02-23/asset-id.ext')
@@ -57,6 +65,7 @@ describe(StorageTemplateService.name, () => {
id: assetEntityStub.image.id,
originalPath: 'upload/library/user-id/2023/2023-02-23/asset-id+1.ext',
});
expect(userMock.getList).toHaveBeenCalled();
});
it('should skip when an asset already matches the template', async () => {
@@ -66,6 +75,7 @@ describe(StorageTemplateService.name, () => {
originalPath: 'upload/library/user-id/2023/2023-02-23/asset-id.ext',
},
]);
userMock.getList.mockResolvedValue([userEntityStub.user1]);
await sut.handleTemplateMigration();
@@ -82,6 +92,7 @@ describe(StorageTemplateService.name, () => {
originalPath: 'upload/library/user-id/2023/2023-02-23/asset-id+1.ext',
},
]);
userMock.getList.mockResolvedValue([userEntityStub.user1]);
await sut.handleTemplateMigration();
@@ -94,6 +105,7 @@ describe(StorageTemplateService.name, () => {
it('should move an asset', async () => {
assetMock.getAll.mockResolvedValue([assetEntityStub.image]);
assetMock.save.mockResolvedValue(assetEntityStub.image);
userMock.getList.mockResolvedValue([userEntityStub.user1]);
await sut.handleTemplateMigration();
@@ -108,9 +120,28 @@ describe(StorageTemplateService.name, () => {
});
});
it('should use the user storage label', async () => {
assetMock.getAll.mockResolvedValue([assetEntityStub.image]);
assetMock.save.mockResolvedValue(assetEntityStub.image);
userMock.getList.mockResolvedValue([userEntityStub.storageLabel]);
await sut.handleTemplateMigration();
expect(assetMock.getAll).toHaveBeenCalled();
expect(storageMock.moveFile).toHaveBeenCalledWith(
'/original/path.ext',
'upload/library/label-1/2023/2023-02-23/asset-id.ext',
);
expect(assetMock.save).toHaveBeenCalledWith({
id: assetEntityStub.image.id,
originalPath: 'upload/library/label-1/2023/2023-02-23/asset-id.ext',
});
});
it('should not update the database if the move fails', async () => {
assetMock.getAll.mockResolvedValue([assetEntityStub.image]);
storageMock.moveFile.mockRejectedValue(new Error('Read only system'));
userMock.getList.mockResolvedValue([userEntityStub.user1]);
await sut.handleTemplateMigration();
@@ -125,6 +156,7 @@ describe(StorageTemplateService.name, () => {
it('should move the asset back if the database fails', async () => {
assetMock.getAll.mockResolvedValue([assetEntityStub.image]);
assetMock.save.mockRejectedValue('Connection Error!');
userMock.getList.mockResolvedValue([userEntityStub.user1]);
await sut.handleTemplateMigration();
@@ -143,6 +175,7 @@ describe(StorageTemplateService.name, () => {
it('should handle an error', async () => {
assetMock.getAll.mockResolvedValue([]);
storageMock.removeEmptyDirs.mockRejectedValue(new Error('Read only filesystem'));
userMock.getList.mockResolvedValue([]);
await sut.handleTemplateMigration();
});

View File

@@ -6,8 +6,14 @@ import { getLivePhotoMotionFilename } from '../domain.util';
import { IAssetJob } from '../job';
import { IStorageRepository } from '../storage/storage.repository';
import { INITIAL_SYSTEM_CONFIG, ISystemConfigRepository } from '../system-config';
import { IUserRepository } from '../user/user.repository';
import { StorageTemplateCore } from './storage-template.core';
export interface MoveAssetMetadata {
storageLabel: string | null;
filename: string;
}
@Injectable()
export class StorageTemplateService {
private logger = new Logger(StorageTemplateService.name);
@@ -18,6 +24,7 @@ export class StorageTemplateService {
@Inject(ISystemConfigRepository) configRepository: ISystemConfigRepository,
@Inject(INITIAL_SYSTEM_CONFIG) config: SystemConfig,
@Inject(IStorageRepository) private storageRepository: IStorageRepository,
@Inject(IUserRepository) private userRepository: IUserRepository,
) {
this.core = new StorageTemplateCore(configRepository, config, storageRepository);
}
@@ -26,14 +33,16 @@ export class StorageTemplateService {
const { asset } = data;
try {
const user = await this.userRepository.get(asset.ownerId);
const storageLabel = user?.storageLabel || null;
const filename = asset.originalFileName || asset.id;
await this.moveAsset(asset, filename);
await this.moveAsset(asset, { storageLabel, filename });
// move motion part of live photo
if (asset.livePhotoVideoId) {
const [livePhotoVideo] = await this.assetRepository.getByIds([asset.livePhotoVideoId]);
const motionFilename = getLivePhotoMotionFilename(filename, livePhotoVideo.originalPath);
await this.moveAsset(livePhotoVideo, motionFilename);
await this.moveAsset(livePhotoVideo, { storageLabel, filename: motionFilename });
}
} catch (error: any) {
this.logger.error('Error running single template migration', error);
@@ -44,6 +53,7 @@ export class StorageTemplateService {
try {
console.time('migrating-time');
const assets = await this.assetRepository.getAll();
const users = await this.userRepository.getList();
const livePhotoMap: Record<string, AssetEntity> = {};
@@ -56,8 +66,10 @@ export class StorageTemplateService {
for (const asset of assets) {
const livePhotoParentAsset = livePhotoMap[asset.id];
// TODO: remove livePhoto specific stuff once upload is fixed
const user = users.find((user) => user.id === asset.ownerId);
const storageLabel = user?.storageLabel || null;
const filename = asset.originalFileName || livePhotoParentAsset?.originalFileName || asset.id;
await this.moveAsset(asset, filename);
await this.moveAsset(asset, { storageLabel, filename });
}
this.logger.debug('Cleaning up empty directories...');
@@ -70,8 +82,8 @@ export class StorageTemplateService {
}
// TODO: use asset core (once in domain)
async moveAsset(asset: AssetEntity, originalName: string) {
const destination = await this.core.getTemplatePath(asset, originalName);
async moveAsset(asset: AssetEntity, metadata: MoveAssetMetadata) {
const destination = await this.core.getTemplatePath(asset, metadata);
if (asset.originalPath !== destination) {
const source = asset.originalPath;

View File

@@ -10,7 +10,14 @@ export enum StorageFolder {
}
export class StorageCore {
getFolderLocation(folder: StorageFolder, userId: string) {
getFolderLocation(
folder: StorageFolder.ENCODED_VIDEO | StorageFolder.UPLOAD | StorageFolder.PROFILE | StorageFolder.THUMBNAILS,
userId: string,
) {
return join(APP_MEDIA_LOCATION, folder, userId);
}
getLibraryFolder(user: { storageLabel: string | null; id: string }) {
return join(APP_MEDIA_LOCATION, StorageFolder.LIBRARY, user.storageLabel || user.id);
}
}

View File

@@ -1,24 +1,28 @@
import { ApiProperty } from '@nestjs/swagger';
import { Transform } from 'class-transformer';
import { IsNotEmpty, IsEmail } from 'class-validator';
import { IsEmail, IsNotEmpty, IsOptional, IsString } from 'class-validator';
import { toEmail, toSanitized } from '../../../../../apps/immich/src/utils/transform.util';
export class CreateUserDto {
@IsEmail()
@Transform(({ value }) => value?.toLowerCase())
@ApiProperty({ example: 'testuser@email.com' })
@Transform(toEmail)
email!: string;
@IsNotEmpty()
@ApiProperty({ example: 'password' })
@IsString()
password!: string;
@IsNotEmpty()
@ApiProperty({ example: 'John' })
@IsString()
firstName!: string;
@IsNotEmpty()
@ApiProperty({ example: 'Doe' })
@IsString()
lastName!: string;
@IsOptional()
@IsString()
@Transform(toSanitized)
storageLabel?: string | null;
}
export class CreateAdminDto {

View File

@@ -1,8 +1,34 @@
import { IsBoolean, IsNotEmpty, IsOptional, IsUUID } from 'class-validator';
import { CreateUserDto } from './create-user.dto';
import { ApiProperty, PartialType } from '@nestjs/swagger';
import { ApiProperty } from '@nestjs/swagger';
import { Transform } from 'class-transformer';
import { IsBoolean, IsEmail, IsNotEmpty, IsOptional, IsString, IsUUID } from 'class-validator';
import { toEmail, toSanitized } from '../../../../../apps/immich/src/utils/transform.util';
export class UpdateUserDto {
@IsOptional()
@IsEmail()
@Transform(toEmail)
email?: string;
@IsOptional()
@IsNotEmpty()
@IsString()
password?: string;
@IsOptional()
@IsString()
@IsNotEmpty()
firstName?: string;
@IsOptional()
@IsString()
@IsNotEmpty()
lastName?: string;
@IsOptional()
@IsString()
@Transform(toSanitized)
storageLabel?: string;
export class UpdateUserDto extends PartialType(CreateUserDto) {
@IsNotEmpty()
@IsUUID('4')
@ApiProperty({ format: 'uuid' })

View File

@@ -5,6 +5,7 @@ export class UserResponseDto {
email!: string;
firstName!: string;
lastName!: string;
storageLabel!: string | null;
createdAt!: string;
profileImagePath!: string;
shouldChangePassword!: boolean;
@@ -20,6 +21,7 @@ export function mapUser(entity: UserEntity): UserResponseDto {
email: entity.email,
firstName: entity.firstName,
lastName: entity.lastName,
storageLabel: entity.storageLabel,
createdAt: entity.createdAt,
profileImagePath: entity.profileImagePath,
shouldChangePassword: entity.shouldChangePassword,

View File

@@ -5,7 +5,6 @@ import {
InternalServerErrorException,
Logger,
NotFoundException,
UnauthorizedException,
} from '@nestjs/common';
import { hash } from 'bcrypt';
import { constants, createReadStream, ReadStream } from 'fs';
@@ -28,6 +27,7 @@ export class UserCore {
if (!authUser.isAdmin) {
// Users can never update the isAdmin property.
delete dto.isAdmin;
delete dto.storageLabel;
} else if (dto.isAdmin && authUser.id !== id) {
// Admin cannot create another admin.
throw new BadRequestException('The server already has an admin');
@@ -36,7 +36,14 @@ export class UserCore {
if (dto.email) {
const duplicate = await this.userRepository.getByEmail(dto.email);
if (duplicate && duplicate.id !== id) {
throw new BadRequestException('Email already in user by another account');
throw new BadRequestException('Email already in use by another account');
}
}
if (dto.storageLabel) {
const duplicate = await this.userRepository.getByStorageLabel(dto.storageLabel);
if (duplicate && duplicate.id !== id) {
throw new BadRequestException('Storage label already in use by another account');
}
}
@@ -45,6 +52,10 @@ export class UserCore {
dto.password = await this.cryptoRepository.hashBcrypt(dto.password, SALT_ROUNDS);
}
if (dto.storageLabel === '') {
dto.storageLabel = null;
}
return this.userRepository.update(id, dto);
} catch (e) {
Logger.error(e, 'Failed to update user info');
@@ -106,14 +117,8 @@ export class UserCore {
}
async createProfileImage(authUser: AuthUserDto, filePath: string): Promise<UserEntity> {
// TODO: do we need to do this? Maybe we can trust the authUser
const user = await this.userRepository.get(authUser.id);
if (!user) {
throw new NotFoundException('User not found');
}
try {
return this.userRepository.update(user.id, { profileImagePath: filePath });
return this.userRepository.update(authUser.id, { profileImagePath: filePath });
} catch (e) {
Logger.error(e, 'Create User Profile Image');
throw new InternalServerErrorException('Failed to create new user profile image');
@@ -121,12 +126,7 @@ export class UserCore {
}
async restoreUser(authUser: AuthUserDto, userToRestore: UserEntity): Promise<UserEntity> {
// TODO: do we need to do this? Maybe we can trust the authUser
const requestor = await this.userRepository.get(authUser.id);
if (!requestor) {
throw new UnauthorizedException('Requestor not found');
}
if (!requestor.isAdmin) {
if (!authUser.isAdmin) {
throw new ForbiddenException('Unauthorized');
}
try {
@@ -138,12 +138,7 @@ export class UserCore {
}
async deleteUser(authUser: AuthUserDto, userToDelete: UserEntity): Promise<UserEntity> {
// TODO: do we need to do this? Maybe we can trust the authUser
const requestor = await this.userRepository.get(authUser.id);
if (!requestor) {
throw new UnauthorizedException('Requestor not found');
}
if (!requestor.isAdmin) {
if (!authUser.isAdmin) {
throw new ForbiddenException('Unauthorized');
}

View File

@@ -1,7 +1,7 @@
import { UserEntity } from '@app/infra/entities';
export interface UserListFilter {
excludeId?: string;
withDeleted?: boolean;
}
export interface UserStatsQueryResponse {
@@ -19,6 +19,7 @@ export interface IUserRepository {
get(id: string, withDeleted?: boolean): Promise<UserEntity | null>;
getAdmin(): Promise<UserEntity | null>;
getByEmail(email: string, withPassword?: boolean): Promise<UserEntity | null>;
getByStorageLabel(storageLabel: string): Promise<UserEntity | null>;
getByOAuthId(oauthId: string): Promise<UserEntity | null>;
getDeletedUsers(): Promise<UserEntity[]>;
getList(filter?: UserListFilter): Promise<UserEntity[]>;

View File

@@ -36,7 +36,7 @@ const adminUserAuth: AuthUserDto = Object.freeze({
});
const immichUserAuth: AuthUserDto = Object.freeze({
id: 'immich_id',
id: 'user-id',
email: 'immich@test.com',
isAdmin: false,
});
@@ -55,6 +55,7 @@ const adminUser: UserEntity = Object.freeze({
updatedAt: '2021-01-01',
tags: [],
assets: [],
storageLabel: 'admin',
});
const immichUser: UserEntity = Object.freeze({
@@ -71,6 +72,7 @@ const immichUser: UserEntity = Object.freeze({
updatedAt: '2021-01-01',
tags: [],
assets: [],
storageLabel: null,
});
const updatedImmichUser: UserEntity = Object.freeze({
@@ -87,6 +89,7 @@ const updatedImmichUser: UserEntity = Object.freeze({
updatedAt: '2021-01-01',
tags: [],
assets: [],
storageLabel: null,
});
const adminUserResponse = Object.freeze({
@@ -101,6 +104,7 @@ const adminUserResponse = Object.freeze({
profileImagePath: '',
createdAt: '2021-01-01',
updatedAt: '2021-01-01',
storageLabel: 'admin',
});
describe(UserService.name, () => {
@@ -150,7 +154,7 @@ describe(UserService.name, () => {
const response = await sut.getAllUsers(adminUserAuth, false);
expect(userRepositoryMock.getList).toHaveBeenCalledWith({ excludeId: adminUser.id });
expect(userRepositoryMock.getList).toHaveBeenCalledWith({ withDeleted: true });
expect(response).toEqual([
{
id: adminUserAuth.id,
@@ -164,6 +168,7 @@ describe(UserService.name, () => {
profileImagePath: '',
createdAt: '2021-01-01',
updatedAt: '2021-01-01',
storageLabel: 'admin',
},
]);
});
@@ -231,6 +236,22 @@ describe(UserService.name, () => {
expect(updatedUser.shouldChangePassword).toEqual(true);
});
it('should not set an empty string for storage label', async () => {
userRepositoryMock.update.mockResolvedValue(updatedImmichUser);
await sut.updateUser(adminUserAuth, { id: immichUser.id, storageLabel: '' });
expect(userRepositoryMock.update).toHaveBeenCalledWith(immichUser.id, { id: immichUser.id, storageLabel: null });
});
it('should omit a storage label set by non-admin users', async () => {
userRepositoryMock.update.mockResolvedValue(updatedImmichUser);
await sut.updateUser(immichUserAuth, { id: immichUser.id, storageLabel: 'admin' });
expect(userRepositoryMock.update).toHaveBeenCalledWith(immichUser.id, { id: immichUser.id });
});
it('user can only update its information', async () => {
when(userRepositoryMock.get)
.calledWith('not_immich_auth_user_id', undefined)
@@ -255,7 +276,7 @@ describe(UserService.name, () => {
await sut.updateUser(immichUser, dto);
expect(userRepositoryMock.update).toHaveBeenCalledWith(immichUser.id, {
id: 'immich_id',
id: 'user-id',
email: 'updated@test.com',
});
});
@@ -271,6 +292,17 @@ describe(UserService.name, () => {
expect(userRepositoryMock.update).not.toHaveBeenCalled();
});
it('should not let the admin change the storage label to one already in use', async () => {
const dto = { id: immichUser.id, storageLabel: 'admin' };
userRepositoryMock.get.mockResolvedValue(immichUser);
userRepositoryMock.getByStorageLabel.mockResolvedValue(adminUser);
await expect(sut.updateUser(adminUser, dto)).rejects.toBeInstanceOf(BadRequestException);
expect(userRepositoryMock.update).not.toHaveBeenCalled();
});
it('admin can update any user information', async () => {
const update: UpdateUserDto = {
id: immichUser.id,
@@ -481,6 +513,16 @@ describe(UserService.name, () => {
expect(userRepositoryMock.delete).toHaveBeenCalledWith(user, true);
});
it('should delete the library path for a storage label', async () => {
const user = { id: 'deleted-user', deletedAt: makeDeletedAt(10), storageLabel: 'admin' } as UserEntity;
await sut.handleUserDelete({ user });
const options = { force: true, recursive: true };
expect(storageMock.unlinkDir).toHaveBeenCalledWith('upload/library/admin', options);
});
it('should handle an error', async () => {
const user = { id: 'deleted-user', deletedAt: makeDeletedAt(10) } as UserEntity;

View File

@@ -44,13 +44,8 @@ export class UserService {
}
async getAllUsers(authUser: AuthUserDto, isAll: boolean): Promise<UserResponseDto[]> {
if (isAll) {
const allUsers = await this.userCore.getList();
return allUsers.map(mapUser);
}
const allUserExceptRequestedUser = await this.userCore.getList({ excludeId: authUser.id });
return allUserExceptRequestedUser.map(mapUser);
const users = await this.userCore.getList({ withDeleted: !isAll });
return users.map(mapUser);
}
async getUserById(userId: string, withDeleted = false): Promise<UserResponseDto> {
@@ -165,7 +160,7 @@ export class UserService {
try {
const folders = [
this.storageCore.getFolderLocation(StorageFolder.LIBRARY, user.id),
this.storageCore.getLibraryFolder(user),
this.storageCore.getFolderLocation(StorageFolder.UPLOAD, user.id),
this.storageCore.getFolderLocation(StorageFolder.PROFILE, user.id),
this.storageCore.getFolderLocation(StorageFolder.THUMBNAILS, user.id),