From 07ef1b12163b5c33826ac830660c2c3b8a87c699 Mon Sep 17 00:00:00 2001 From: rubikscraft Date: Sun, 27 Mar 2022 23:56:25 +0200 Subject: [PATCH] refactor collections --- .../collections/imagedb/imagedb.service.ts | 37 ++++++----- .../src/collections/roledb/roledb.module.ts | 6 ++ .../src/collections/roledb/roledb.service.ts | 34 ++++++----- .../syspreferencedb.service.ts | 61 +++++++++---------- .../syspreferencedefaults.service.ts | 7 ++- .../src/collections/userdb/userdb.module.ts | 49 ++++++--------- .../src/collections/userdb/userdb.service.ts | 41 ++++++++----- .../collections/userdb/userrolesdb.service.ts | 11 +++- .../src/managers/auth/guards/main.guard.ts | 4 +- .../src/managers/demo/demomanager.service.ts | 2 +- .../util/collection.ts} | 0 shared/src/util/unique.ts | 8 +++ 12 files changed, 147 insertions(+), 113 deletions(-) rename backend/src/{collections/collectionutils.ts => models/util/collection.ts} (100%) create mode 100644 shared/src/util/unique.ts diff --git a/backend/src/collections/imagedb/imagedb.service.ts b/backend/src/collections/imagedb/imagedb.service.ts index 8b9b460..0df0c45 100644 --- a/backend/src/collections/imagedb/imagedb.service.ts +++ b/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( hash: string, getPrivate?: B, - ): AsyncFailable> { + ): AsyncFailable< + B extends undefined ? EImageBackend : Required + > { 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; + return found as B extends undefined + ? EImageBackend + : Required; } catch (e: any) { return Fail(e?.message); } } public async findMany( - startId: number, - limit: number, + count: number, + page: number, ): AsyncFailable { + 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 { - 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 { + public async deleteAll(IAmSure: boolean): AsyncFailable { + if (!IAmSure) + return Fail('You must confirm that you want to delete all images'); + try { await this.imageRepository.delete({}); } catch (e: any) { diff --git a/backend/src/collections/roledb/roledb.module.ts b/backend/src/collections/roledb/roledb.module.ts index b4b0d4b..75671d6 100644 --- a/backend/src/collections/roledb/roledb.module.ts +++ b/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}"`, diff --git a/backend/src/collections/roledb/roledb.service.ts b/backend/src/collections/roledb/roledb.service.ts index b5674d3..6f72f63 100644 --- a/backend/src/collections/roledb/roledb.service.ts +++ b/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 { 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 { - if (!iamsure) return Fail('Nuke aborted'); + public async nukeSystemRoles(IAmSure: boolean = false): AsyncFailable { + if (!IAmSure) + return Fail('You must confirm that you want to delete all roles'); + try { await this.rolesRepository.delete({ name: In(UndeletableRolesList), diff --git a/backend/src/collections/syspreferencesdb/syspreferencedb.service.ts b/backend/src/collections/syspreferencesdb/syspreferencedb.service.ts index d68b9ad..3332ead 100644 --- a/backend/src/collections/syspreferencesdb/syspreferencedb.service.ts +++ b/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,16 +81,13 @@ export class SysPreferenceService { // Fallback if (!foundSysPreference) { return this.saveDefault(validatedKey); - } else { - foundSysPreference = plainToClass( - ESysPreferenceBackend, - foundSysPreference, - ); - const errors = await strictValidate(foundSysPreference); - if (errors.length > 0) { - this.logger.warn(errors); - return Fail('Invalid preference'); - } + } + + // Validate + const errors = await strictValidate(foundSysPreference); + if (errors.length > 0) { + this.logger.warn(errors); + return Fail('Invalid preference'); } // Return @@ -97,32 +95,32 @@ export class SysPreferenceService { } public async getStringPreference(key: string): AsyncFailable { - 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; } public async getNumberPreference(key: string): AsyncFailable { - 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; } public async getBooleanPreference(key: string): AsyncFailable { - 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; + } - return pref.value as boolean; + private async getPreferencePinned( + key: string, + type: SysPrefValueTypeStrings, + ): AsyncFailable { + 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 { @@ -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 { - 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 { diff --git a/backend/src/collections/syspreferencesdb/syspreferencedefaults.service.ts b/backend/src/collections/syspreferencesdb/syspreferencedefaults.service.ts index 44d8f76..9a9a103 100644 --- a/backend/src/collections/syspreferencesdb/syspreferencedefaults.service.ts +++ b/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, diff --git a/backend/src/collections/userdb/userdb.module.ts b/backend/src/collections/userdb/userdb.module.ts index f66af90..c5727f2 100644 --- a/backend/src/collections/userdb/userdb.module.ts +++ b/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()}`, - ); - return; - } + await this.ensureUserExists( + 'admin', + this.authConfigService.getDefaultAdminPassword(), + ['user', 'admin'], + ); } - 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; } diff --git a/backend/src/collections/userdb/userdb.service.ts b/backend/src/collections/userdb/userdb.service.ts index 5d92f9c..e2e9488 100644 --- a/backend/src/collections/userdb/userdb.service.ts +++ b/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, - 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 { 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 { @@ -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 { - 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( 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 diff --git a/backend/src/collections/userdb/userrolesdb.service.ts b/backend/src/collections/userdb/userrolesdb.service.ts index 478f412..144b198 100644 --- a/backend/src/collections/userdb/userrolesdb.service.ts +++ b/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 { @@ -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); } diff --git a/backend/src/managers/auth/guards/main.guard.ts b/backend/src/managers/auth/guards/main.guard.ts index 9a07333..4bbf4fc 100644 --- a/backend/src/managers/auth/guards/main.guard.ts +++ b/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(); } diff --git a/backend/src/managers/demo/demomanager.service.ts b/backend/src/managers/demo/demomanager.service.ts index 498d6cf..78d82e7 100644 --- a/backend/src/managers/demo/demomanager.service.ts +++ b/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); } } diff --git a/backend/src/collections/collectionutils.ts b/backend/src/models/util/collection.ts similarity index 100% rename from backend/src/collections/collectionutils.ts rename to backend/src/models/util/collection.ts diff --git a/shared/src/util/unique.ts b/shared/src/util/unique.ts new file mode 100644 index 0000000..720cc24 --- /dev/null +++ b/shared/src/util/unique.ts @@ -0,0 +1,8 @@ +export function makeUnique(arr: T[]): T[] { + return arr.reduce(function (accum, current) { + if (accum.indexOf(current) < 0) { + accum.push(current); + } + return accum; + }, [] as T[]); +}