feat(server,web): remove external path nonsense and make libraries admin-only (#7237)

* remove external path

* open-api

* make sql

* move library settings to admin panel

* Add documentation

* show external libraries only

* fix library list

* make user library settings look good

* fix test

* fix tests

* fix tests

* can pick user for library

* fix tests

* fix e2e

* chore: make sql

* Use unauth exception

* delete user library list

* cleanup

* fix e2e

* fix await lint

* chore: remove unused code

* chore: cleanup

* revert docs

* fix: is admin stuff

* table alignment

---------

Co-authored-by: Jason Rasmussen <jrasm91@gmail.com>
Co-authored-by: Alex Tran <alex.tran1502@gmail.com>

server/src/domain/library/library.dto.ts

@@ -1,59 +1,62 @@
 import { LibraryEntity, LibraryType } from '@app/infra/entities';
 import { ApiProperty } from '@nestjs/swagger';
-import { ArrayMaxSize, ArrayUnique, IsBoolean, IsEnum, IsNotEmpty, IsOptional, IsString } from 'class-validator';
-import { ValidateUUID } from '../domain.util';
+import { ArrayMaxSize, ArrayUnique, IsBoolean, IsEnum, IsNotEmpty, IsString } from 'class-validator';
+import { Optional, ValidateUUID } from '../domain.util';
 
 export class CreateLibraryDto {
   @IsEnum(LibraryType)
   @ApiProperty({ enumName: 'LibraryType', enum: LibraryType })
   type!: LibraryType;
 
+  @ValidateUUID({ optional: true })
+  ownerId?: string;
+
   @IsString()
-  @IsOptional()
+  @Optional()
   @IsNotEmpty()
   name?: string;
 
-  @IsOptional()
+  @Optional()
   @IsBoolean()
   isVisible?: boolean;
 
-  @IsOptional()
+  @Optional()
   @IsString({ each: true })
   @IsNotEmpty({ each: true })
   @ArrayUnique()
   @ArrayMaxSize(128)
   importPaths?: string[];
 
-  @IsOptional()
+  @Optional()
   @IsString({ each: true })
   @IsNotEmpty({ each: true })
   @ArrayUnique()
   @ArrayMaxSize(128)
   exclusionPatterns?: string[];
 
-  @IsOptional()
+  @Optional()
   @IsBoolean()
   isWatched?: boolean;
 }
 
 export class UpdateLibraryDto {
-  @IsOptional()
+  @Optional()
   @IsString()
   @IsNotEmpty()
   name?: string;
 
-  @IsOptional()
+  @Optional()
   @IsBoolean()
   isVisible?: boolean;
 
-  @IsOptional()
+  @Optional()
   @IsString({ each: true })
   @IsNotEmpty({ each: true })
   @ArrayUnique()
   @ArrayMaxSize(128)
   importPaths?: string[];
 
-  @IsOptional()
+  @Optional()
   @IsNotEmpty({ each: true })
   @IsString({ each: true })
   @ArrayUnique()
@@ -68,14 +71,14 @@ export class CrawlOptionsDto {
 }
 
 export class ValidateLibraryDto {
-  @IsOptional()
+  @Optional()
   @IsString({ each: true })
   @IsNotEmpty({ each: true })
   @ArrayUnique()
   @ArrayMaxSize(128)
   importPaths?: string[];
 
-  @IsOptional()
+  @Optional()
   @IsNotEmpty({ each: true })
   @IsString({ each: true })
   @ArrayUnique()
@@ -100,14 +103,21 @@ export class LibrarySearchDto {
 
 export class ScanLibraryDto {
   @IsBoolean()
-  @IsOptional()
+  @Optional()
   refreshModifiedFiles?: boolean;
 
   @IsBoolean()
-  @IsOptional()
+  @Optional()
   refreshAllFiles?: boolean = false;
 }
 
+export class SearchLibraryDto {
+  @IsEnum(LibraryType)
+  @ApiProperty({ enumName: 'LibraryType', enum: LibraryType })
+  @Optional()
+  type?: LibraryType;
+}
+
 export class LibraryResponseDto {
   id!: string;
   ownerId!: string;

server/src/domain/library/library.service.spec.ts

@@ -140,24 +140,6 @@ describe(LibraryService.name, () => {
   });
 
   describe('handleQueueAssetRefresh', () => {
-    it("should not queue assets outside of user's external path", async () => {
-      const mockLibraryJob: ILibraryRefreshJob = {
-        id: libraryStub.externalLibrary1.id,
-        refreshModifiedFiles: false,
-        refreshAllFiles: false,
-      };
-
-      libraryMock.get.mockResolvedValue(libraryStub.externalLibrary1);
-      storageMock.crawl.mockResolvedValue(['/data/user2/photo.jpg']);
-      assetMock.getByLibraryId.mockResolvedValue([]);
-      libraryMock.getOnlineAssetPaths.mockResolvedValue([]);
-      userMock.get.mockResolvedValue(userStub.externalPath1);
-
-      await sut.handleQueueAssetRefresh(mockLibraryJob);
-
-      expect(jobMock.queue.mock.calls).toEqual([]);
-    });
-
     it('should queue new assets', async () => {
       const mockLibraryJob: ILibraryRefreshJob = {
         id: libraryStub.externalLibrary1.id,
@@ -168,8 +150,7 @@ describe(LibraryService.name, () => {
       libraryMock.get.mockResolvedValue(libraryStub.externalLibrary1);
       storageMock.crawl.mockResolvedValue(['/data/user1/photo.jpg']);
       assetMock.getByLibraryId.mockResolvedValue([]);
-      libraryMock.getOnlineAssetPaths.mockResolvedValue([]);
-      userMock.get.mockResolvedValue(userStub.externalPath1);
+      userMock.get.mockResolvedValue(userStub.admin);
 
       await sut.handleQueueAssetRefresh(mockLibraryJob);
 
@@ -196,8 +177,7 @@ describe(LibraryService.name, () => {
       libraryMock.get.mockResolvedValue(libraryStub.externalLibrary1);
       storageMock.crawl.mockResolvedValue(['/data/user1/photo.jpg']);
       assetMock.getByLibraryId.mockResolvedValue([]);
-      libraryMock.getOnlineAssetPaths.mockResolvedValue([]);
-      userMock.get.mockResolvedValue(userStub.externalPath1);
+      userMock.get.mockResolvedValue(userStub.admin);
 
       await sut.handleQueueAssetRefresh(mockLibraryJob);
 
@@ -214,45 +194,6 @@ describe(LibraryService.name, () => {
       ]);
     });
 
-    it("should mark assets outside of the user's external path as offline", async () => {
-      const mockLibraryJob: ILibraryRefreshJob = {
-        id: libraryStub.externalLibrary1.id,
-        refreshModifiedFiles: false,
-        refreshAllFiles: false,
-      };
-
-      libraryMock.get.mockResolvedValue(libraryStub.externalLibrary1);
-      storageMock.crawl.mockResolvedValue(['/data/user1/photo.jpg']);
-      assetMock.getByLibraryId.mockResolvedValue([assetStub.external]);
-      libraryMock.getOnlineAssetPaths.mockResolvedValue([]);
-      userMock.get.mockResolvedValue(userStub.externalPath2);
-
-      await sut.handleQueueAssetRefresh(mockLibraryJob);
-
-      expect(assetMock.updateAll.mock.calls).toEqual([
-        [
-          [assetStub.external.id],
-          {
-            isOffline: true,
-          },
-        ],
-      ]);
-    });
-
-    it('should not scan libraries owned by user without external path', async () => {
-      const mockLibraryJob: ILibraryRefreshJob = {
-        id: libraryStub.externalLibrary1.id,
-        refreshModifiedFiles: false,
-        refreshAllFiles: false,
-      };
-
-      libraryMock.get.mockResolvedValue(libraryStub.externalLibrary1);
-
-      userMock.get.mockResolvedValue(userStub.user1);
-
-      await expect(sut.handleQueueAssetRefresh(mockLibraryJob)).resolves.toBe(false);
-    });
-
     it('should not scan upload libraries', async () => {
       const mockLibraryJob: ILibraryRefreshJob = {
         id: libraryStub.externalLibrary1.id,
@@ -287,7 +228,6 @@ describe(LibraryService.name, () => {
       libraryMock.get.mockResolvedValue(libraryStub.externalLibraryWithImportPaths1);
       storageMock.crawl.mockResolvedValue([]);
       assetMock.getByLibraryId.mockResolvedValue([]);
-      libraryMock.getOnlineAssetPaths.mockResolvedValue([]);
       userMock.get.mockResolvedValue(userStub.externalPathRoot);
 
       await sut.handleQueueAssetRefresh(mockLibraryJob);
@@ -303,7 +243,7 @@ describe(LibraryService.name, () => {
     let mockUser: UserEntity;
 
     beforeEach(() => {
-      mockUser = userStub.externalPath1;
+      mockUser = userStub.admin;
       userMock.get.mockResolvedValue(mockUser);
 
       storageMock.stat.mockResolvedValue({
@@ -780,26 +720,6 @@ describe(LibraryService.name, () => {
     });
   });
 
-  describe('getAllForUser', () => {
-    it('should return all libraries for user', async () => {
-      libraryMock.getAllByUserId.mockResolvedValue([libraryStub.uploadLibrary1, libraryStub.externalLibrary1]);
-      await expect(sut.getAllForUser(authStub.admin)).resolves.toEqual([
-        expect.objectContaining({
-          id: libraryStub.uploadLibrary1.id,
-          name: libraryStub.uploadLibrary1.name,
-          ownerId: libraryStub.uploadLibrary1.ownerId,
-        }),
-        expect.objectContaining({
-          id: libraryStub.externalLibrary1.id,
-          name: libraryStub.externalLibrary1.name,
-          ownerId: libraryStub.externalLibrary1.ownerId,
-        }),
-      ]);
-
-      expect(libraryMock.getAllByUserId).toHaveBeenCalledWith(authStub.admin.user.id);
-    });
-  });
-
   describe('getStatistics', () => {
     it('should return library statistics', async () => {
       libraryMock.getStatistics.mockResolvedValue({ photos: 10, videos: 0, total: 10, usage: 1337 });
@@ -1144,12 +1064,12 @@ describe(LibraryService.name, () => {
       storageMock.checkFileExists.mockResolvedValue(true);
 
       await expect(
-        sut.update(authStub.external1, authStub.external1.user.id, { importPaths: ['/data/user1/foo'] }),
+        sut.update(authStub.admin, authStub.admin.user.id, { importPaths: ['/data/user1/foo'] }),
       ).resolves.toEqual(mapLibrary(libraryStub.externalLibraryWithImportPaths1));
 
       expect(libraryMock.update).toHaveBeenCalledWith(
         expect.objectContaining({
-          id: authStub.external1.user.id,
+          id: authStub.admin.user.id,
         }),
       );
       expect(storageMock.watch).toHaveBeenCalledWith(
@@ -1584,26 +1504,6 @@ describe(LibraryService.name, () => {
       ]);
     });
 
-    it('should error when no external path is set', async () => {
-      await expect(
-        sut.validate(authStub.admin, libraryStub.externalLibrary1.id, { importPaths: ['/photos'] }),
-      ).rejects.toBeInstanceOf(BadRequestException);
-    });
-
-    it('should detect when path is outside external path', async () => {
-      const result = await sut.validate(authStub.external1, libraryStub.externalLibraryWithImportPaths1.id, {
-        importPaths: ['/data/user2'],
-      });
-
-      expect(result.importPaths).toEqual([
-        {
-          importPath: '/data/user2',
-          isValid: false,
-          message: "Not contained in user's external path",
-        },
-      ]);
-    });
-
     it('should detect when path does not exist', async () => {
       storageMock.stat.mockImplementation(() => {
         const error = { code: 'ENOENT' } as any;

server/src/domain/library/library.service.ts

@@ -29,6 +29,7 @@ import {
   LibraryResponseDto,
   LibraryStatsResponseDto,
   ScanLibraryDto,
+  SearchLibraryDto,
   UpdateLibraryDto,
   ValidateLibraryDto,
   ValidateLibraryImportPathResponseDto,
@@ -182,6 +183,7 @@ export class LibraryService extends EventEmitter {
 
   async getStatistics(auth: AuthDto, id: string): Promise<LibraryStatsResponseDto> {
     await this.access.requirePermission(auth, Permission.LIBRARY_READ, id);
+
     return this.repository.getStatistics(id);
   }
 
@@ -189,17 +191,18 @@ export class LibraryService extends EventEmitter {
     return this.repository.getCountForUser(auth.user.id);
   }
 
-  async getAllForUser(auth: AuthDto): Promise<LibraryResponseDto[]> {
-    const libraries = await this.repository.getAllByUserId(auth.user.id);
-    return libraries.map((library) => mapLibrary(library));
-  }
-
   async get(auth: AuthDto, id: string): Promise<LibraryResponseDto> {
     await this.access.requirePermission(auth, Permission.LIBRARY_READ, id);
+
     const library = await this.findOrFail(id);
     return mapLibrary(library);
   }
 
+  async getAll(auth: AuthDto, dto: SearchLibraryDto): Promise<LibraryResponseDto[]> {
+    const libraries = await this.repository.getAll(false, dto.type);
+    return libraries.map((library) => mapLibrary(library));
+  }
+
   async handleQueueCleanup(): Promise<boolean> {
     this.logger.debug('Cleaning up any pending library deletions');
     const pendingDeletion = await this.repository.getAllDeleted();
@@ -234,8 +237,14 @@ export class LibraryService extends EventEmitter {
       }
     }
 
+    let ownerId = auth.user.id;
+
+    if (dto.ownerId) {
+      ownerId = dto.ownerId;
+    }
+
     const library = await this.repository.create({
-      ownerId: auth.user.id,
+      ownerId,
       name: dto.name,
       type: dto.type,
       importPaths: dto.importPaths ?? [],
@@ -300,24 +309,11 @@ export class LibraryService extends EventEmitter {
   public async validate(auth: AuthDto, id: string, dto: ValidateLibraryDto): Promise<ValidateLibraryResponseDto> {
     await this.access.requirePermission(auth, Permission.LIBRARY_UPDATE, id);
 
-    if (!auth.user.externalPath) {
-      throw new BadRequestException('User has no external path set');
-    }
-
     const response = new ValidateLibraryResponseDto();
 
     if (dto.importPaths) {
       response.importPaths = await Promise.all(
         dto.importPaths.map(async (importPath) => {
-          const normalizedPath = path.normalize(importPath);
-
-          if (!this.isInExternalPath(normalizedPath, auth.user.externalPath)) {
-            const validation = new ValidateLibraryImportPathResponseDto();
-            validation.importPath = importPath;
-            validation.message = `Not contained in user's external path`;
-            return validation;
-          }
-
           return await this.validateImportPath(importPath);
         }),
       );
@@ -328,6 +324,7 @@ export class LibraryService extends EventEmitter {
 
   async update(auth: AuthDto, id: string, dto: UpdateLibraryDto): Promise<LibraryResponseDto> {
     await this.access.requirePermission(auth, Permission.LIBRARY_UPDATE, id);
+
     const library = await this.repository.update({ id, ...dto });
 
     if (dto.importPaths) {
@@ -404,7 +401,7 @@ export class LibraryService extends EventEmitter {
         return true;
       } else {
         // File can't be accessed and does not already exist in db
-        throw new BadRequestException("Can't access file", { cause: error });
+        throw new BadRequestException('Cannot access file', { cause: error });
       }
     }
 
@@ -591,12 +588,6 @@ export class LibraryService extends EventEmitter {
       return false;
     }
 
-    const user = await this.userRepository.get(library.ownerId, {});
-    if (!user?.externalPath) {
-      this.logger.warn('User has no external path set, cannot refresh library');
-      return false;
-    }
-
     this.logger.verbose(`Refreshing library: ${job.id}`);
 
     const pathValidation = await Promise.all(
@@ -618,11 +609,7 @@ export class LibraryService extends EventEmitter {
       exclusionPatterns: library.exclusionPatterns,
     });
 
-    const crawledAssetPaths = rawPaths
-      // Normalize file paths. This is important to prevent security issues like path traversal
-      .map((filePath) => path.normalize(filePath))
-      // Filter out paths that are not within the user's external path
-      .filter((assetPath) => this.isInExternalPath(assetPath, user.externalPath)) as string[];
+    const crawledAssetPaths = rawPaths.map((filePath) => path.normalize(filePath));
 
     this.logger.debug(`Found ${crawledAssetPaths.length} asset(s) when crawling import paths ${library.importPaths}`);
     const assetsInLibrary = await this.assetRepository.getByLibraryId([job.id]);