From 31eac94bc747558d06c2be133b67f71592759b8d Mon Sep 17 00:00:00 2001 From: rubikscraft Date: Mon, 28 Mar 2022 15:43:52 +0200 Subject: [PATCH] refactor backend done --- backend/src/decorators/returns.decorator.ts | 18 +++++++++++++ backend/src/managers/auth/auth.service.ts | 12 ++++++--- .../src/managers/auth/guards/main.guard.ts | 2 +- backend/src/models/requests/imageroute.dto.ts | 2 +- .../multipart.validator.ts | 3 ++- .../permissions.validator.ts | 0 .../api/experiment/experiment.controller.ts | 2 -- .../api/experiment/experiment.module.ts | 3 +++ .../src/routes/api/info/info.controller.ts | 7 +++-- .../src/routes/api/pref/pref.controller.ts | 19 ++++---------- .../src/routes/api/roles/roles.controller.ts | 26 +++++++------------ .../src/routes/api/user/user.controller.ts | 21 ++++++++++----- backend/src/routes/image/imageid.validator.ts | 17 ++++++++++++ .../src/routes/image/imageroute.controller.ts | 22 +++++++--------- backend/src/routes/image/imageroute.module.ts | 2 ++ 15 files changed, 95 insertions(+), 61 deletions(-) create mode 100644 backend/src/decorators/returns.decorator.ts rename backend/src/models/{requests => validators}/multipart.validator.ts (83%) rename backend/src/models/{util => validators}/permissions.validator.ts (100%) create mode 100644 backend/src/routes/image/imageid.validator.ts diff --git a/backend/src/decorators/returns.decorator.ts b/backend/src/decorators/returns.decorator.ts new file mode 100644 index 0000000..d9dcb55 --- /dev/null +++ b/backend/src/decorators/returns.decorator.ts @@ -0,0 +1,18 @@ +import { SetMetadata } from '@nestjs/common'; +import { Newable } from 'picsur-shared/dist/types/newable'; + +// Not yet used, but can be used for outgoing data validation + +type ReturnsMethodDecorator = < + T extends (...args: any) => ReturnType | Promise, +>( + target: Object, + propertyKey: string | symbol, + descriptor: TypedPropertyDescriptor, +) => TypedPropertyDescriptor | void; + +export function Returns( + newable: Newable, +): ReturnsMethodDecorator { + return SetMetadata('returns', newable); +} diff --git a/backend/src/managers/auth/auth.service.ts b/backend/src/managers/auth/auth.service.ts index 5c1bc17..54a7ae7 100644 --- a/backend/src/managers/auth/auth.service.ts +++ b/backend/src/managers/auth/auth.service.ts @@ -2,6 +2,7 @@ import { Injectable, Logger } from '@nestjs/common'; import { JwtService } from '@nestjs/jwt'; import { instanceToPlain, plainToClass } from 'class-transformer'; import { JwtDataDto } from 'picsur-shared/dist/dto/jwt.dto'; +import { AsyncFailable, Fail } from 'picsur-shared/dist/types'; import { strictValidate } from 'picsur-shared/dist/util/validate'; import { EUserBackend } from '../../models/entities/user.entity'; @@ -11,7 +12,7 @@ export class AuthManagerService { constructor(private jwtService: JwtService) {} - async createToken(user: EUserBackend): Promise { + async createToken(user: EUserBackend): AsyncFailable { const jwtData: JwtDataDto = plainToClass(JwtDataDto, { user: { username: user.username, @@ -23,10 +24,13 @@ export class AuthManagerService { // in case of any failures const errors = await strictValidate(jwtData); if (errors.length > 0) { - this.logger.warn(errors); - throw new Error('Invalid jwt token generated'); + return Fail('Invalid JWT: ' + errors); } - return this.jwtService.signAsync(instanceToPlain(jwtData)); + try { + return await this.jwtService.signAsync(instanceToPlain(jwtData)); + } catch (e) { + return Fail("Couldn't create JWT: " + e); + } } } diff --git a/backend/src/managers/auth/guards/main.guard.ts b/backend/src/managers/auth/guards/main.guard.ts index 2ce4538..b262744 100644 --- a/backend/src/managers/auth/guards/main.guard.ts +++ b/backend/src/managers/auth/guards/main.guard.ts @@ -13,7 +13,7 @@ import { strictValidate } from 'picsur-shared/dist/util/validate'; import { UserRolesService } from '../../../collections/userdb/userrolesdb.service'; import { Permissions } from '../../../models/dto/permissions.dto'; import { EUserBackend } from '../../../models/entities/user.entity'; -import { isPermissionsArray } from '../../../models/util/permissions.validator'; +import { isPermissionsArray } from '../../../models/validators/permissions.validator'; // This guard extends both the jwt authenticator and the guest authenticator // The order matters here, because this results in the guest authenticator being used as a fallback diff --git a/backend/src/models/requests/imageroute.dto.ts b/backend/src/models/requests/imageroute.dto.ts index 2a18dd5..d0e7f8b 100644 --- a/backend/src/models/requests/imageroute.dto.ts +++ b/backend/src/models/requests/imageroute.dto.ts @@ -1,5 +1,5 @@ +import { IsMultiPartFile } from '../validators/multipart.validator'; import { MultiPartFileDto } from './multipart.dto'; -import { IsMultiPartFile } from './multipart.validator'; // A validation class for form based file upload of an image export class ImageUploadDto { diff --git a/backend/src/models/requests/multipart.validator.ts b/backend/src/models/validators/multipart.validator.ts similarity index 83% rename from backend/src/models/requests/multipart.validator.ts rename to backend/src/models/validators/multipart.validator.ts index 47dd07a..86ffefc 100644 --- a/backend/src/models/requests/multipart.validator.ts +++ b/backend/src/models/validators/multipart.validator.ts @@ -1,7 +1,8 @@ import { Type } from 'class-transformer'; import { IsDefined, ValidateNested } from 'class-validator'; import { CombinePDecorators } from 'picsur-shared/dist/util/decorator'; -import { MultiPartFieldDto, MultiPartFileDto } from './multipart.dto'; +import { MultiPartFieldDto, MultiPartFileDto } from '../requests/multipart.dto'; + export const IsMultiPartFile = CombinePDecorators( IsDefined(), diff --git a/backend/src/models/util/permissions.validator.ts b/backend/src/models/validators/permissions.validator.ts similarity index 100% rename from backend/src/models/util/permissions.validator.ts rename to backend/src/models/validators/permissions.validator.ts diff --git a/backend/src/routes/api/experiment/experiment.controller.ts b/backend/src/routes/api/experiment/experiment.controller.ts index e99a310..bc0b160 100644 --- a/backend/src/routes/api/experiment/experiment.controller.ts +++ b/backend/src/routes/api/experiment/experiment.controller.ts @@ -1,11 +1,9 @@ import { Controller, Get, Request } from '@nestjs/common'; import AuthFasityRequest from '../../../models/requests/authrequest.dto'; - @Controller('api/experiment') export class ExperimentController { @Get() - // @Guest() async testRoute(@Request() req: AuthFasityRequest) { return { message: req.user, diff --git a/backend/src/routes/api/experiment/experiment.module.ts b/backend/src/routes/api/experiment/experiment.module.ts index 2b88b0d..90ca23e 100644 --- a/backend/src/routes/api/experiment/experiment.module.ts +++ b/backend/src/routes/api/experiment/experiment.module.ts @@ -1,6 +1,9 @@ import { Module } from '@nestjs/common'; import { ExperimentController } from './experiment.controller'; +// This is comletely useless module, but is used for testing +// TODO: remove when out of beta + @Module({ controllers: [ExperimentController] }) diff --git a/backend/src/routes/api/info/info.controller.ts b/backend/src/routes/api/info/info.controller.ts index 309f4be..7fada14 100644 --- a/backend/src/routes/api/info/info.controller.ts +++ b/backend/src/routes/api/info/info.controller.ts @@ -1,6 +1,9 @@ import { Controller, Get } from '@nestjs/common'; import { plainToClass } from 'class-transformer'; -import { AllPermissionsResponse, InfoResponse } from 'picsur-shared/dist/dto/api/info.dto'; +import { + AllPermissionsResponse, + InfoResponse +} from 'picsur-shared/dist/dto/api/info.dto'; import { HostConfigService } from '../../../config/early/host.config.service'; import { NoPermissions } from '../../../decorators/permissions.decorator'; import { PermissionsList } from '../../../models/dto/permissions.dto'; @@ -20,7 +23,7 @@ export class InfoController { } // List all available permissions - @Get('/permissions') + @Get('permissions') async getPermissions(): Promise { const result: AllPermissionsResponse = { Permissions: PermissionsList, diff --git a/backend/src/routes/api/pref/pref.controller.ts b/backend/src/routes/api/pref/pref.controller.ts index 10fd8d7..c2bbc26 100644 --- a/backend/src/routes/api/pref/pref.controller.ts +++ b/backend/src/routes/api/pref/pref.controller.ts @@ -7,12 +7,9 @@ import { Param, Post } from '@nestjs/common'; -import { plainToClass } from 'class-transformer'; import { GetSyspreferenceResponse, - MultipleSysPreferencesResponse, - SysPreferenceBaseResponse, - UpdateSysPreferenceRequest, + MultipleSysPreferencesResponse, UpdateSysPreferenceRequest, UpdateSysPreferenceResponse } from 'picsur-shared/dist/dto/api/pref.dto'; import { HasFailed } from 'picsur-shared/dist/types'; @@ -35,14 +32,10 @@ export class PrefController { throw new InternalServerErrorException('Could not get preferences'); } - const returned: MultipleSysPreferencesResponse = { - preferences: prefs.map((pref) => - plainToClass(SysPreferenceBaseResponse, pref), - ), + return { + preferences: prefs, total: prefs.length, }; - - return plainToClass(MultipleSysPreferencesResponse, returned); } @Get('sys/:key') @@ -55,7 +48,7 @@ export class PrefController { throw new InternalServerErrorException('Could not get preference'); } - return plainToClass(GetSyspreferenceResponse, pref); + return pref; } @Post('sys/:key') @@ -71,12 +64,10 @@ export class PrefController { throw new InternalServerErrorException('Could not set preference'); } - const returned = { + return { key, value: pref.value, type: pref.type, }; - - return plainToClass(UpdateSysPreferenceResponse, returned); } } diff --git a/backend/src/routes/api/roles/roles.controller.ts b/backend/src/routes/api/roles/roles.controller.ts index 67a5afd..33107ff 100644 --- a/backend/src/routes/api/roles/roles.controller.ts +++ b/backend/src/routes/api/roles/roles.controller.ts @@ -6,7 +6,6 @@ import { Logger, Post } from '@nestjs/common'; -import { plainToClass } from 'class-transformer'; import { RoleCreateRequest, RoleCreateResponse, @@ -22,16 +21,14 @@ import { import { HasFailed } from 'picsur-shared/dist/types'; import { RolesService } from '../../../collections/roledb/roledb.service'; import { RequiredPermissions } from '../../../decorators/permissions.decorator'; -import { - Permission -} from '../../../models/dto/permissions.dto'; +import { Permission } from '../../../models/dto/permissions.dto'; import { DefaultRolesList, ImmutableRolesList, SoulBoundRolesList, UndeletableRolesList } from '../../../models/dto/roles.dto'; -import { isPermissionsArray } from '../../../models/util/permissions.validator'; +import { isPermissionsArray } from '../../../models/validators/permissions.validator'; @Controller('api/roles') @RequiredPermissions(Permission.RoleManage) @@ -40,7 +37,7 @@ export class RolesController { constructor(private rolesService: RolesService) {} - @Get('/list') + @Get('list') async getRoles(): Promise { const roles = await this.rolesService.findAll(); if (HasFailed(roles)) { @@ -54,7 +51,7 @@ export class RolesController { }; } - @Post('/info') + @Post('info') async getRole(@Body() body: RoleInfoRequest): Promise { const role = await this.rolesService.findOne(body.name); if (HasFailed(role)) { @@ -65,13 +62,13 @@ export class RolesController { return role; } - @Post('/update') + @Post('update') async updateRole( @Body() body: RoleUpdateRequest, ): Promise { const permissions = body.permissions; if (!isPermissionsArray(permissions)) { - throw new InternalServerErrorException('Invalid permissions array'); + throw new InternalServerErrorException('Invalid permissions'); } const updatedRole = await this.rolesService.setPermissions( @@ -86,7 +83,7 @@ export class RolesController { return updatedRole; } - @Post('/create') + @Post('create') async createRole( @Body() role: RoleCreateRequest, ): Promise { @@ -104,7 +101,7 @@ export class RolesController { return newRole; } - @Post('/delete') + @Post('delete') async deleteRole( @Body() role: RoleDeleteRequest, ): Promise { @@ -117,16 +114,13 @@ export class RolesController { return deletedRole; } - @Get('/special') + @Get('special') async getSpecialRoles(): Promise { - const result: SpecialRolesResponse = { + return { SoulBoundRoles: SoulBoundRolesList, ImmutableRoles: ImmutableRolesList, UndeletableRoles: UndeletableRolesList, DefaultRoles: DefaultRolesList, }; - - return plainToClass(SpecialRolesResponse, result); } - } diff --git a/backend/src/routes/api/user/user.controller.ts b/backend/src/routes/api/user/user.controller.ts index f6b7e2e..33a9095 100644 --- a/backend/src/routes/api/user/user.controller.ts +++ b/backend/src/routes/api/user/user.controller.ts @@ -39,9 +39,13 @@ export class UserController { @Post('login') @UseLocalAuth(Permission.UserLogin) async login(@Request() req: AuthFasityRequest): Promise { - return { - jwt_token: await this.authService.createToken(req.user), - }; + const jwt_token = await this.authService.createToken(req.user); + if (HasFailed(jwt_token)) { + this.logger.warn(jwt_token.getReason()); + throw new InternalServerErrorException('Could not get new token'); + } + + return { jwt_token }; } @Post('register') @@ -71,10 +75,13 @@ export class UserController { throw new InternalServerErrorException('Could not get user'); } - return { - user, - token: await this.authService.createToken(user), - }; + const token = await this.authService.createToken(user); + if (HasFailed(token)) { + this.logger.warn(token.getReason()); + throw new InternalServerErrorException('Could not get new token'); + } + + return { user, token }; } // You can always check your permissions diff --git a/backend/src/routes/image/imageid.validator.ts b/backend/src/routes/image/imageid.validator.ts new file mode 100644 index 0000000..4282433 --- /dev/null +++ b/backend/src/routes/image/imageid.validator.ts @@ -0,0 +1,17 @@ +import { + ArgumentMetadata, + BadRequestException, + Injectable, + PipeTransform +} from '@nestjs/common'; +import { isHash } from 'class-validator'; + +@Injectable() +export class ImageIdValidator implements PipeTransform { + transform(value: string, metadata: ArgumentMetadata): string { + if (isHash(value, 'sha256')) { + return value; + } + throw new BadRequestException('Invalid image id'); + } +} diff --git a/backend/src/routes/image/imageroute.controller.ts b/backend/src/routes/image/imageroute.controller.ts index c5823c0..15360a8 100644 --- a/backend/src/routes/image/imageroute.controller.ts +++ b/backend/src/routes/image/imageroute.controller.ts @@ -1,17 +1,13 @@ import { - BadRequestException, Controller, Get, InternalServerErrorException, Logger, NotFoundException, Param, - Post, - Req, - Res + Post, Res } from '@nestjs/common'; -import { isHash } from 'class-validator'; -import { FastifyReply, FastifyRequest } from 'fastify'; +import { FastifyReply } from 'fastify'; import { ImageMetaResponse } from 'picsur-shared/dist/dto/api/image.dto'; import { HasFailed } from 'picsur-shared/dist/types'; import { MultiPart } from '../../decorators/multipart.decorator'; @@ -19,6 +15,7 @@ import { RequiredPermissions } from '../../decorators/permissions.decorator'; import { ImageManagerService } from '../../managers/imagemanager/imagemanager.service'; import { Permission } from '../../models/dto/permissions.dto'; import { ImageUploadDto } from '../../models/requests/imageroute.dto'; +import { ImageIdValidator } from './imageid.validator'; @Controller('i') @RequiredPermissions(Permission.ImageView) @@ -29,11 +26,11 @@ export class ImageController { @Get(':hash') async getImage( + // Usually passthrough is for manually sending the response, + // But we need it here to set the mime type @Res({ passthrough: true }) res: FastifyReply, - @Param('hash') hash: string, + @Param('hash', ImageIdValidator) hash: string, ): Promise { - if (!isHash(hash, 'sha256')) throw new BadRequestException('Invalid hash'); - const image = await this.imagesService.retrieveComplete(hash); if (HasFailed(image)) { this.logger.warn(image.getReason()); @@ -45,9 +42,9 @@ export class ImageController { } @Get('meta/:hash') - async getImageMeta(@Param('hash') hash: string): Promise { - if (!isHash(hash, 'sha256')) throw new BadRequestException('Invalid hash'); - + async getImageMeta( + @Param('hash', ImageIdValidator) hash: string, + ): Promise { const image = await this.imagesService.retrieveInfo(hash); if (HasFailed(image)) { this.logger.warn(image.getReason()); @@ -60,7 +57,6 @@ export class ImageController { @Post() @RequiredPermissions(Permission.ImageUpload) async uploadImage( - @Req() req: FastifyRequest, @MultiPart(ImageUploadDto) multipart: ImageUploadDto, ): Promise { const fileBuffer = await multipart.image.toBuffer(); diff --git a/backend/src/routes/image/imageroute.module.ts b/backend/src/routes/image/imageroute.module.ts index 0133f5b..0f048db 100644 --- a/backend/src/routes/image/imageroute.module.ts +++ b/backend/src/routes/image/imageroute.module.ts @@ -1,10 +1,12 @@ import { Module } from '@nestjs/common'; import { DecoratorsModule } from '../../decorators/decorators.module'; import { ImageManagerModule } from '../../managers/imagemanager/imagemanager.module'; +import { ImageIdValidator } from './imageid.validator'; import { ImageController } from './imageroute.controller'; @Module({ imports: [ImageManagerModule, DecoratorsModule], + providers: [ImageIdValidator], controllers: [ImageController], }) export class ImageModule {}