refactor collections

backend/src/collections/imagedb/imagedb.service.ts

@@ -4,14 +4,12 @@ import { plainToClass } from 'class-transformer';
 import Crypto from 'crypto';
 import {
   AsyncFailable,
-  Fail,
-  HasFailed,
-  HasSuccess
+  Fail, HasSuccess
 } from 'picsur-shared/dist/types';
 import { Repository } from 'typeorm';
 import { SupportedMime } from '../../models/dto/mimes.dto';
 import { EImageBackend } from '../../models/entities/image.entity';
-import { GetCols } from '../collectionutils';
+import { GetCols } from '../../models/util/collection';
 
 @Injectable()
 export class ImageDBService {
@@ -46,7 +44,9 @@ export class ImageDBService {
   public async findOne<B extends true | undefined = undefined>(
     hash: string,
     getPrivate?: B,
-  ): AsyncFailable<B extends undefined ? EImageBackend : Required<EImageBackend>> {
+  ): AsyncFailable<
+    B extends undefined ? EImageBackend : Required<EImageBackend>
+  > {
     try {
       const found = await this.imageRepository.findOne({
         where: { hash },
@@ -54,21 +54,27 @@ export class ImageDBService {
       });
 
       if (!found) return Fail('Image not found');
-      return found as B extends undefined ? EImageBackend : Required<EImageBackend>;
+      return found as B extends undefined
+        ? EImageBackend
+        : Required<EImageBackend>;
     } catch (e: any) {
       return Fail(e?.message);
     }
   }
 
   public async findMany(
-    startId: number,
-    limit: number,
+    count: number,
+    page: number,
   ): AsyncFailable<EImageBackend[]> {
+    if (count < 1 || page < 0) return Fail('Invalid page');
+    if (count > 100) return Fail('Too many results');
+
     try {
       const found = await this.imageRepository.find({
-        where: { id: { gte: startId } },
-        take: limit,
+        skip: count * page,
+        take: count,
       });
+
       if (found === undefined) return Fail('Images not found');
       return found;
     } catch (e: any) {
@@ -77,18 +83,19 @@ export class ImageDBService {
   }
 
   public async delete(hash: string): AsyncFailable<true> {
-    const image = await this.findOne(hash);
-    if (HasFailed(image)) return image;
-
     try {
-      await this.imageRepository.delete(image);
+      const result = await this.imageRepository.delete({ hash });
+      if (result.affected === 0) return Fail('Image not found');
     } catch (e: any) {
       return Fail(e?.message);
     }
     return true;
   }
 
-  public async deleteAll(): AsyncFailable<true> {
+  public async deleteAll(IAmSure: boolean): AsyncFailable<true> {
+    if (!IAmSure)
+      return Fail('You must confirm that you want to delete all images');
+
     try {
       await this.imageRepository.delete({});
     } catch (e: any) {

backend/src/collections/roledb/roledb.module.ts

@@ -21,6 +21,8 @@ export class RolesModule implements OnModuleInit {
   ) {}
 
   async onModuleInit() {
+    // Nuking roles in dev environment makes testing easier
+    // This ensures that the roles are always started with their default permissions
     if (!this.hostConfig.isProduction()) {
       await this.nukeRoles();
     }
@@ -38,6 +40,7 @@ export class RolesModule implements OnModuleInit {
   }
 
   private async ensureSystemRolesExist() {
+    // The UndeletableRolesList is also the list of systemroles
     for (const systemRole of UndeletableRolesList) {
       this.logger.debug(`Ensuring system role "${systemRole}" exists`);
 
@@ -58,6 +61,9 @@ export class RolesModule implements OnModuleInit {
   }
 
   private async updateImmutableRoles() {
+    // Immutable roles can not be updated via the gui
+    // They therefore do have to be kept up to date from the backend
+    
     for (const immutableRole of ImmutableRolesList) {
       this.logger.debug(
         `Updating permissions for immutable role "${immutableRole}"`,

backend/src/collections/roledb/roledb.service.ts

@@ -7,10 +7,14 @@ import {
   HasFailed,
   HasSuccess
 } from 'picsur-shared/dist/types';
+import { makeUnique } from 'picsur-shared/dist/util/unique';
 import { strictValidate } from 'picsur-shared/dist/util/validate';
 import { In, Repository } from 'typeorm';
 import { Permissions } from '../../models/dto/permissions.dto';
-import { ImmutableRolesList, UndeletableRolesList } from '../../models/dto/roles.dto';
+import {
+  ImmutableRolesList,
+  UndeletableRolesList
+} from '../../models/dto/roles.dto';
 import { ERoleBackend } from '../../models/entities/role.entity';
 
 @Injectable()
@@ -33,12 +37,10 @@ export class RolesService {
     role.permissions = permissions;
 
     try {
-      role = await this.rolesRepository.save(role, { reload: true });
+      return await this.rolesRepository.save(role, { reload: true });
     } catch (e: any) {
       return Fail(e?.message);
     }
-
-    return plainToClass(ERoleBackend, role);
   }
 
   public async delete(
@@ -69,7 +71,7 @@ export class RolesService {
       permissions.push(...foundRole.permissions);
     }
 
-    return [...new Set(...[permissions])];
+    return makeUnique(permissions);
   }
 
   public async addPermissions(
@@ -79,10 +81,10 @@ export class RolesService {
     const roleToModify = await this.resolve(role);
     if (HasFailed(roleToModify)) return roleToModify;
 
-    // This is stupid
-    const newPermissions = [
-      ...new Set([...roleToModify.permissions, ...permissions]),
-    ];
+    const newPermissions = makeUnique([
+      ...roleToModify.permissions,
+      ...permissions,
+    ]);
 
     return this.setPermissions(roleToModify, newPermissions);
   }
@@ -101,9 +103,11 @@ export class RolesService {
     return this.setPermissions(roleToModify, newPermissions);
   }
 
+  // Permission specific validation is done here
   public async setPermissions(
     role: string | ERoleBackend,
     permissions: Permissions,
+    // Extra bypass for internal use
     allowImmutable: boolean = false,
   ): AsyncFailable<ERoleBackend> {
     const roleToModify = await this.resolve(role);
@@ -113,7 +117,7 @@ export class RolesService {
       return Fail('Cannot modify immutable role');
     }
 
-    roleToModify.permissions = [...new Set(permissions)];
+    roleToModify.permissions = makeUnique(permissions);
 
     try {
       return await this.rolesRepository.save(roleToModify);
@@ -129,7 +133,7 @@ export class RolesService {
       });
 
       if (!found) return Fail('Role not found');
-      return found as ERoleBackend;
+      return found;
     } catch (e: any) {
       return Fail(e?.message);
     }
@@ -139,7 +143,7 @@ export class RolesService {
     try {
       const found = await this.rolesRepository.find();
       if (!found) return Fail('No roles found');
-      return found as ERoleBackend[];
+      return found;
     } catch (e: any) {
       return Fail(e?.message);
     }
@@ -149,8 +153,10 @@ export class RolesService {
     return HasSuccess(await this.findOne(username));
   }
 
-  public async nukeSystemRoles(iamsure: boolean = false): AsyncFailable<true> {
-    if (!iamsure) return Fail('Nuke aborted');
+  public async nukeSystemRoles(IAmSure: boolean = false): AsyncFailable<true> {
+    if (!IAmSure)
+      return Fail('You must confirm that you want to delete all roles');
+
     try {
       await this.rolesRepository.delete({
         name: In(UndeletableRolesList),

backend/src/collections/syspreferencesdb/syspreferencedb.service.ts

@@ -1,10 +1,10 @@
 import { Injectable, Logger } from '@nestjs/common';
 import { InjectRepository } from '@nestjs/typeorm';
-import { plainToClass } from 'class-transformer';
 import {
   InternalSysprefRepresentation,
   SysPreference,
-  SysPrefValueType
+  SysPrefValueType,
+  SysPrefValueTypeStrings
 } from 'picsur-shared/dist/dto/syspreferences.dto';
 import {
   AsyncFailable,
@@ -41,6 +41,7 @@ export class SysPreferenceService {
 
     // Set
     try {
+      // Upsert here, because we want to create a new record if it does not exist
       await this.sysPreferenceRepository.upsert(sysPreference, {
         conflictPaths: ['key'],
       });
@@ -70,7 +71,7 @@ export class SysPreferenceService {
     try {
       foundSysPreference = await this.sysPreferenceRepository.findOne(
         { key: validatedKey },
-        { cache: 60000 },
+        { cache: 60000 }, // Enable cache for 1 minute
       );
     } catch (e: any) {
       this.logger.warn(e);
@@ -80,49 +81,46 @@ export class SysPreferenceService {
     // Fallback
     if (!foundSysPreference) {
       return this.saveDefault(validatedKey);
-    } else {
-      foundSysPreference = plainToClass(
-        ESysPreferenceBackend,
-        foundSysPreference,
-      );
+    }
+
+    // Validate
     const errors = await strictValidate(foundSysPreference);
     if (errors.length > 0) {
       this.logger.warn(errors);
       return Fail('Invalid preference');
     }
-    }
 
     // Return
     return this.retrieveConvertedValue(foundSysPreference);
   }
 
   public async getStringPreference(key: string): AsyncFailable<string> {
-    const pref = await this.getPreference(key);
-    if (HasFailed(pref)) return pref;
-    if (pref.type !== 'string') return Fail('Invalid preference type');
-
-    return pref.value as string;
+    return this.getPreferencePinned(key, 'string') as AsyncFailable<string>;
   }
 
   public async getNumberPreference(key: string): AsyncFailable<number> {
-    const pref = await this.getPreference(key);
-    if (HasFailed(pref)) return pref;
-    if (pref.type !== 'number') return Fail('Invalid preference type');
-
-    return pref.value as number;
+    return this.getPreferencePinned(key, 'number') as AsyncFailable<number>;
   }
 
   public async getBooleanPreference(key: string): AsyncFailable<boolean> {
-    const pref = await this.getPreference(key);
-    if (HasFailed(pref)) return pref;
-    if (pref.type !== 'boolean') return Fail('Invalid preference type');
+    return this.getPreferencePinned(key, 'boolean') as AsyncFailable<boolean>;
+  }
 
-    return pref.value as boolean;
+  private async getPreferencePinned(
+    key: string,
+    type: SysPrefValueTypeStrings,
+  ): AsyncFailable<SysPrefValueType> {
+    let pref = await this.getPreference(key);
+    if (HasFailed(pref)) return pref;
+    if (pref.type !== type) return Fail('Invalid preference type');
+
+    return pref.value;
   }
 
   public async getAllPreferences(): AsyncFailable<
     InternalSysprefRepresentation[]
   > {
+    // TODO: We are fetching each value invidually, we should fetch all at once
     let internalSysPrefs = await Promise.all(
       SysPreferenceList.map((key) => this.getPreference(key)),
     );
@@ -132,6 +130,7 @@ export class SysPreferenceService {
 
     return internalSysPrefs as InternalSysprefRepresentation[];
   }
+
   // Private
 
   private async saveDefault(
@@ -140,6 +139,7 @@ export class SysPreferenceService {
     return this.setPreference(key, this.defaultsService.defaults[key]());
   }
 
+  // This converts the raw string representation of the value to the correct type
   private retrieveConvertedValue(
     preference: ESysPreferenceBackend,
   ): Failable<InternalSysprefRepresentation> {
@@ -185,7 +185,7 @@ export class SysPreferenceService {
     verifySysPreference.key = validatedKey;
     verifySysPreference.value = validatedValue;
 
-    // Just to be sure
+    // It should already be valid, but these two validators might go out of sync
     const errors = await strictValidate(verifySysPreference);
     if (errors.length > 0) {
       this.logger.warn(errors);
@@ -196,14 +196,13 @@ export class SysPreferenceService {
   }
 
   private validatePrefKey(key: string): Failable<SysPreference> {
-    if (!SysPreferenceList.includes(key)) {
-      return Fail('Invalid preference key');
-    }
+    if (!SysPreferenceList.includes(key)) return Fail('Invalid preference key');
 
     return key as SysPreference;
   }
 
   private validatePrefValue(
+    // Key is required, because the type of the value depends on the key
     key: SysPreference,
     value: SysPrefValueType,
   ): Failable<string> {

backend/src/collections/syspreferencesdb/syspreferencedefaults.service.ts

@@ -6,6 +6,9 @@ import {
 import { generateRandomString } from 'picsur-shared/dist/util/random';
 import { EnvJwtConfigService } from '../../config/jwt.config.service';
 
+// This specific service is used to store default values for system preferences
+// It needs to be in a service because the values depend on the environment
+
 @Injectable()
 export class SysPreferenceDefaultsService {
   private readonly logger = new Logger('SysPreferenceDefaultsService');
@@ -26,8 +29,8 @@ export class SysPreferenceDefaultsService {
         return generateRandomString(64);
       }
     },
-    [SysPreference.JwtExpiresIn]: () => this.jwtConfigService.getJwtExpiresIn() ?? '7d',
-
+    [SysPreference.JwtExpiresIn]: () =>
+      this.jwtConfigService.getJwtExpiresIn() ?? '7d',
     [SysPreference.TestString]: () => 'test_string',
     [SysPreference.TestNumber]: () => 123,
     [SysPreference.TestBoolean]: () => true,

backend/src/collections/userdb/userdb.module.ts

@@ -28,36 +28,27 @@ export class UsersModule implements OnModuleInit {
   ) {}
 
   async onModuleInit() {
-    await this.ensureGuestExists();
-    await this.ensureAdminExists();
-  }
-
-  private async ensureGuestExists() {
-    const username = 'guest';
-    const password = generateRandomString(128);
-    this.logger.debug(`Ensuring guest user exists`);
-
-    const exists = await this.usersService.exists(username);
-    if (exists) return;
-
-    const newUser = await this.usersService.create(
-      username,
-      password,
+    await this.ensureUserExists(
+      'guest',
+      // Guest should never be able to login
+      // It should be prevented even if you know the password
+      // But to be sure, we set it to a random string
+      generateRandomString(128),
       ['guest'],
-      true,
     );
-    if (HasFailed(newUser)) {
-      this.logger.error(
-        `Failed to create guest user because: ${newUser.getReason()}`,
+    await this.ensureUserExists(
+      'admin',
+      this.authConfigService.getDefaultAdminPassword(),
+      ['user', 'admin'],
     );
-      return;
-    }
   }
 
-  private async ensureAdminExists() {
-    const username = 'admin';
-    const password = this.authConfigService.getDefaultAdminPassword();
-    this.logger.debug(`Ensuring admin user exists`);
+  private async ensureUserExists(
+    username: string,
+    password: string,
+    roles: string[],
+  ) {
+    this.logger.debug(`Ensuring user "${username}" exists`);
 
     const exists = await this.usersService.exists(username);
     if (exists) return;
@@ -65,12 +56,12 @@ export class UsersModule implements OnModuleInit {
     const newUser = await this.usersService.create(
       username,
       password,
-      ['user', 'admin'],
-      true,
+      roles,
+      false,
     );
     if (HasFailed(newUser)) {
       this.logger.error(
-        `Failed to create admin user because: ${newUser.getReason()}`,
+        `Failed to create user "${username}" because: ${newUser.getReason()}`,
       );
       return;
     }

backend/src/collections/userdb/userdb.service.ts

@@ -8,17 +8,22 @@ import {
   HasFailed,
   HasSuccess
 } from 'picsur-shared/dist/types';
+import { makeUnique } from 'picsur-shared/dist/util/unique';
 import { strictValidate } from 'picsur-shared/dist/util/validate';
 import { Repository } from 'typeorm';
 import {
   DefaultRolesList,
   SoulBoundRolesList
 } from '../../models/dto/roles.dto';
-import { ImmutableUsersList, LockedLoginUsersList, UndeletableUsersList } from '../../models/dto/specialusers.dto';
+import {
+  ImmutableUsersList,
+  LockedLoginUsersList,
+  UndeletableUsersList
+} from '../../models/dto/specialusers.dto';
 import { EUserBackend } from '../../models/entities/user.entity';
-import { GetCols } from '../collectionutils';
-import { RolesService } from '../roledb/roledb.service';
+import { GetCols } from '../../models/util/collection';
 
+// TODO: make this a configurable value
 const BCryptStrength = 12;
 
 @Injectable()
@@ -28,7 +33,6 @@ export class UsersService {
   constructor(
     @InjectRepository(EUserBackend)
     private usersRepository: Repository<EUserBackend>,
-    private rolesService: RolesService,
   ) {}
 
   // Creation and deletion
@@ -37,6 +41,7 @@ export class UsersService {
     username: string,
     password: string,
     roles?: string[],
+    // Add option to create "invalid" users, should only be used by system
     byPassRoleCheck?: boolean,
   ): AsyncFailable<EUserBackend> {
     if (await this.exists(username)) return Fail('User already exists');
@@ -48,10 +53,11 @@ export class UsersService {
     user.password = hashedPassword;
     if (byPassRoleCheck) {
       const rolesToAdd = roles ?? [];
-      user.roles = [...new Set([...rolesToAdd])];
+      user.roles = makeUnique(rolesToAdd);
     } else {
+      // Strip soulbound roles and add default roles
       const rolesToAdd = this.filterAddedRoles(roles ?? []);
-      user.roles = [...new Set([...DefaultRolesList, ...rolesToAdd])];
+      user.roles = makeUnique([...DefaultRolesList, ...rolesToAdd]);
     }
 
     try {
@@ -60,10 +66,10 @@ export class UsersService {
       return Fail(e?.message);
     }
 
-    return plainToClass(EUserBackend, user); // Strips unwanted data
+    // Strips unwanted data
+    return plainToClass(EUserBackend, user);
   }
 
-  // Returns user object without id
   public async delete(
     user: string | EUserBackend,
   ): AsyncFailable<EUserBackend> {
@@ -92,6 +98,7 @@ export class UsersService {
 
     if (ImmutableUsersList.includes(userToModify.username)) {
       // Just fail silently
+      this.logger.log("Can't modify system user");
       return userToModify;
     }
 
@@ -99,9 +106,7 @@ export class UsersService {
       SoulBoundRolesList.includes(role),
     );
     const rolesToAdd = this.filterAddedRoles(roles);
-
-    const newRoles = [...new Set([...rolesToKeep, ...rolesToAdd])];
-
+    const newRoles = makeUnique([...rolesToKeep, ...rolesToAdd]);
     userToModify.roles = newRoles;
 
     try {
@@ -115,19 +120,20 @@ export class UsersService {
     user: string | EUserBackend,
     password: string,
   ): AsyncFailable<EUserBackend> {
-    const userToModify = await this.resolve(user);
+    let userToModify = await this.resolve(user);
     if (HasFailed(userToModify)) return userToModify;
 
     const hashedPassword = await bcrypt.hash(password, BCryptStrength);
-
     userToModify.password = hashedPassword;
 
     try {
-      const fullUser = await this.usersRepository.save(userToModify);
-      return plainToClass(EUserBackend, fullUser);
+      userToModify = await this.usersRepository.save(userToModify);
     } catch (e: any) {
       return Fail(e?.message);
     }
+
+    // Strips unwanted data
+    return plainToClass(EUserBackend, userToModify);
   }
 
   // Authentication
@@ -140,7 +146,8 @@ export class UsersService {
     if (HasFailed(user)) return user;
 
     if (LockedLoginUsersList.includes(user.username)) {
-      return Fail('Wrong password');
+      // Error should be kept in backend
+      return Fail('Wrong username');
     }
 
     if (!(await bcrypt.compare(password, user.password)))
@@ -153,6 +160,8 @@ export class UsersService {
 
   public async findOne<B extends true | undefined = undefined>(
     username: string,
+    // Also fetch fields that aren't normally sent to the client
+    // (e.g. hashed password)
     getPrivate?: B,
   ): AsyncFailable<
     B extends undefined ? EUserBackend : Required<EUserBackend>

backend/src/collections/userdb/userrolesdb.service.ts

@@ -1,16 +1,21 @@
 import { Injectable } from '@nestjs/common';
 import { AsyncFailable, HasFailed } from 'picsur-shared/dist/types';
+import { makeUnique } from 'picsur-shared/dist/util/unique';
 import { Permissions } from '../../models/dto/permissions.dto';
 import { EUserBackend } from '../../models/entities/user.entity';
 import { RolesService } from '../roledb/roledb.service';
 import { UsersService } from './userdb.service';
 
+// Move some code here so it doesnt make the userdb service gigantic
+
 @Injectable()
 export class UserRolesService {
-  constructor(private usersService: UsersService, private rolesService: RolesService){}
+  constructor(
+    private usersService: UsersService,
+    private rolesService: RolesService,
+  ) {}
 
   // Permissions and roles
-
   public async getPermissions(
     user: string | EUserBackend,
   ): AsyncFailable<Permissions> {
@@ -27,7 +32,7 @@ export class UserRolesService {
     const userToModify = await this.usersService.resolve(user);
     if (HasFailed(userToModify)) return userToModify;
 
-    const newRoles = [...new Set([...userToModify.roles, ...roles])];
+    const newRoles = makeUnique([...userToModify.roles, ...roles]);
 
     return this.usersService.setRoles(userToModify, newRoles);
   }

backend/src/managers/auth/guards/main.guard.ts

@@ -41,13 +41,13 @@ export class MainAuthGuard extends AuthGuard(['jwt', 'guest']) {
 
     const permissions = this.extractPermissions(context);
     if (HasFailed(permissions)) {
-      this.logger.warn('222' + permissions.getReason());
+      this.logger.warn('Route Permissions: ' + permissions.getReason());
       throw new InternalServerErrorException();
     }
 
     const userPermissions = await this.userRolesService.getPermissions(user);
     if (HasFailed(userPermissions)) {
-      this.logger.warn('111' + userPermissions.getReason());
+      this.logger.warn('User Permissions: ' + userPermissions.getReason());
       throw new InternalServerErrorException();
     }
 

backend/src/managers/demo/demomanager.service.ts

@@ -25,6 +25,6 @@ export class DemoManagerService {
 
   private async executeAsync() {
     this.logger.log('Executing demo cleanup');
-    await this.imagesService.deleteAll();
+    await this.imagesService.deleteAll(true);
   }
 }

shared/src/util/unique.ts

@@ -0,0 +1,8 @@
+export function makeUnique<T>(arr: T[]): T[] {
+  return arr.reduce(function (accum, current) {
+    if (accum.indexOf(current) < 0) {
+      accum.push(current);
+    }
+    return accum;
+  }, [] as T[]);
+}