From 443c8427237283a348bc31a795f53e58d35e4d1c Mon Sep 17 00:00:00 2001 From: Jason Rasmussen Date: Fri, 28 Oct 2022 14:57:52 -0400 Subject: [PATCH] refactor(server): merge auth guards to authentication guard (#877) --- .../src/api-v1/album/album.controller.ts | 5 +-- .../src/api-v1/asset/asset.controller.ts | 5 +-- .../immich/src/api-v1/auth/auth.controller.ts | 6 +-- .../device-info/device-info.controller.ts | 6 +-- .../immich/src/api-v1/job/job.controller.ts | 8 ++-- .../server-info/server-info.controller.ts | 8 ++-- .../immich/src/api-v1/user/user.controller.ts | 15 +++---- .../src/decorators/authenticated.decorator.ts | 16 ++++++++ .../admin-role-guard.middleware.ts | 40 ++++--------------- 9 files changed, 46 insertions(+), 63 deletions(-) create mode 100644 server/apps/immich/src/decorators/authenticated.decorator.ts diff --git a/server/apps/immich/src/api-v1/album/album.controller.ts b/server/apps/immich/src/api-v1/album/album.controller.ts index c58fa97f11..147db25739 100644 --- a/server/apps/immich/src/api-v1/album/album.controller.ts +++ b/server/apps/immich/src/api-v1/album/album.controller.ts @@ -6,7 +6,6 @@ import { Patch, Param, Delete, - UseGuards, ValidationPipe, ParseUUIDPipe, Put, @@ -15,7 +14,7 @@ import { import { ParseMeUUIDPipe } from '../validation/parse-me-uuid-pipe'; import { AlbumService } from './album.service'; import { CreateAlbumDto } from './dto/create-album.dto'; -import { JwtAuthGuard } from '../../modules/immich-jwt/guards/jwt-auth.guard'; +import { Authenticated } from '../../decorators/authenticated.decorator'; import { AuthUserDto, GetAuthUser } from '../../decorators/auth-user.decorator'; import { AddAssetsDto } from './dto/add-assets.dto'; import { AddUsersDto } from './dto/add-users.dto'; @@ -27,7 +26,7 @@ import { AlbumResponseDto } from './response-dto/album-response.dto'; import { AlbumCountResponseDto } from './response-dto/album-count-response.dto'; // TODO might be worth creating a AlbumParamsDto that validates `albumId` instead of using the pipe. -@UseGuards(JwtAuthGuard) +@Authenticated() @ApiBearerAuth() @ApiTags('Album') @Controller('album') diff --git a/server/apps/immich/src/api-v1/asset/asset.controller.ts b/server/apps/immich/src/api-v1/asset/asset.controller.ts index 8f8c16a4ff..8104ae6f04 100644 --- a/server/apps/immich/src/api-v1/asset/asset.controller.ts +++ b/server/apps/immich/src/api-v1/asset/asset.controller.ts @@ -3,7 +3,6 @@ import { Post, UseInterceptors, Body, - UseGuards, Get, Param, ValidationPipe, @@ -17,7 +16,7 @@ import { UploadedFile, Header, } from '@nestjs/common'; -import { JwtAuthGuard } from '../../modules/immich-jwt/guards/jwt-auth.guard'; +import { Authenticated } from '../../decorators/authenticated.decorator'; import { AssetService } from './asset.service'; import { FileInterceptor } from '@nestjs/platform-express'; import { assetUploadOption } from '../../config/asset-upload.config'; @@ -52,7 +51,7 @@ import { AssetCountByUserIdResponseDto } from './response-dto/asset-count-by-use import { CheckExistingAssetsDto } from './dto/check-existing-assets.dto'; import { CheckExistingAssetsResponseDto } from './response-dto/check-existing-assets-response.dto'; -@UseGuards(JwtAuthGuard) +@Authenticated() @ApiBearerAuth() @ApiTags('Asset') @Controller('asset') diff --git a/server/apps/immich/src/api-v1/auth/auth.controller.ts b/server/apps/immich/src/api-v1/auth/auth.controller.ts index 50a18fd066..9fc787bdd3 100644 --- a/server/apps/immich/src/api-v1/auth/auth.controller.ts +++ b/server/apps/immich/src/api-v1/auth/auth.controller.ts @@ -1,7 +1,7 @@ -import { Body, Controller, Post, Res, UseGuards, ValidationPipe, Ip } from '@nestjs/common'; +import { Body, Controller, Post, Res, ValidationPipe, Ip } from '@nestjs/common'; import { ApiBadRequestResponse, ApiBearerAuth, ApiTags } from '@nestjs/swagger'; import { AuthUserDto, GetAuthUser } from '../../decorators/auth-user.decorator'; -import { JwtAuthGuard } from '../../modules/immich-jwt/guards/jwt-auth.guard'; +import { Authenticated } from '../../decorators/authenticated.decorator'; import { AuthService } from './auth.service'; import { LoginCredentialDto } from './dto/login-credential.dto'; import { LoginResponseDto } from './response-dto/login-response.dto'; @@ -42,7 +42,7 @@ export class AuthController { return await this.authService.adminSignUp(signUpCredential); } - @UseGuards(JwtAuthGuard) + @Authenticated() @ApiBearerAuth() @Post('/validateToken') // eslint-disable-next-line @typescript-eslint/no-unused-vars diff --git a/server/apps/immich/src/api-v1/device-info/device-info.controller.ts b/server/apps/immich/src/api-v1/device-info/device-info.controller.ts index 554da4b4bd..f2865d2190 100644 --- a/server/apps/immich/src/api-v1/device-info/device-info.controller.ts +++ b/server/apps/immich/src/api-v1/device-info/device-info.controller.ts @@ -1,13 +1,13 @@ -import { Controller, Post, Body, Patch, UseGuards, ValidationPipe } from '@nestjs/common'; +import { Controller, Post, Body, Patch, ValidationPipe } from '@nestjs/common'; import { ApiBearerAuth, ApiTags } from '@nestjs/swagger'; import { AuthUserDto, GetAuthUser } from '../../decorators/auth-user.decorator'; -import { JwtAuthGuard } from '../../modules/immich-jwt/guards/jwt-auth.guard'; +import { Authenticated } from '../../decorators/authenticated.decorator'; import { DeviceInfoService } from './device-info.service'; import { CreateDeviceInfoDto } from './dto/create-device-info.dto'; import { UpdateDeviceInfoDto } from './dto/update-device-info.dto'; import { DeviceInfoResponseDto } from './response-dto/create-device-info-response.dto'; -@UseGuards(JwtAuthGuard) +@Authenticated() @ApiBearerAuth() @ApiTags('Device Info') @Controller('device-info') diff --git a/server/apps/immich/src/api-v1/job/job.controller.ts b/server/apps/immich/src/api-v1/job/job.controller.ts index 2fbccb7fd8..5766ada3cf 100644 --- a/server/apps/immich/src/api-v1/job/job.controller.ts +++ b/server/apps/immich/src/api-v1/job/job.controller.ts @@ -1,16 +1,14 @@ -import { Controller, Get, Body, UseGuards, ValidationPipe, Put, Param } from '@nestjs/common'; +import { Controller, Get, Body, ValidationPipe, Put, Param } from '@nestjs/common'; import { JobService } from './job.service'; import { ApiBearerAuth, ApiTags } from '@nestjs/swagger'; -import { JwtAuthGuard } from '../../modules/immich-jwt/guards/jwt-auth.guard'; -import { AdminRolesGuard } from '../../middlewares/admin-role-guard.middleware'; +import { Authenticated } from '../../decorators/authenticated.decorator'; import { AllJobStatusResponseDto } from './response-dto/all-job-status-response.dto'; import { GetJobDto } from './dto/get-job.dto'; import { JobStatusResponseDto } from './response-dto/job-status-response.dto'; import { JobCommandDto } from './dto/job-command.dto'; -@UseGuards(JwtAuthGuard) -@UseGuards(AdminRolesGuard) +@Authenticated({ admin: true }) @ApiTags('Job') @ApiBearerAuth() @Controller('jobs') diff --git a/server/apps/immich/src/api-v1/server-info/server-info.controller.ts b/server/apps/immich/src/api-v1/server-info/server-info.controller.ts index 3f22e2d476..9c944ac501 100644 --- a/server/apps/immich/src/api-v1/server-info/server-info.controller.ts +++ b/server/apps/immich/src/api-v1/server-info/server-info.controller.ts @@ -1,4 +1,4 @@ -import { Controller, Get, UseGuards } from '@nestjs/common'; +import { Controller, Get } from '@nestjs/common'; import { ServerInfoService } from './server-info.service'; import { serverVersion } from '../../constants/server_version.constant'; import { ApiTags } from '@nestjs/swagger'; @@ -6,8 +6,7 @@ import { ServerPingResponse } from './response-dto/server-ping-response.dto'; import { ServerVersionReponseDto } from './response-dto/server-version-response.dto'; import { ServerInfoResponseDto } from './response-dto/server-info-response.dto'; import { ServerStatsResponseDto } from './response-dto/server-stats-response.dto'; -import { JwtAuthGuard } from '../../modules/immich-jwt/guards/jwt-auth.guard'; -import { AdminRolesGuard } from '../../middlewares/admin-role-guard.middleware'; +import { Authenticated } from '../../decorators/authenticated.decorator'; @ApiTags('Server Info') @Controller('server-info') @@ -29,8 +28,7 @@ export class ServerInfoController { return serverVersion; } - @UseGuards(JwtAuthGuard) - @UseGuards(AdminRolesGuard) + @Authenticated({ admin: true }) @Get('/stats') async getStats(): Promise { return await this.serverInfoService.getStats(); diff --git a/server/apps/immich/src/api-v1/user/user.controller.ts b/server/apps/immich/src/api-v1/user/user.controller.ts index a534154ad7..aac54c4c54 100644 --- a/server/apps/immich/src/api-v1/user/user.controller.ts +++ b/server/apps/immich/src/api-v1/user/user.controller.ts @@ -4,7 +4,6 @@ import { Post, Body, Param, - UseGuards, ValidationPipe, Put, Query, @@ -14,10 +13,9 @@ import { ParseBoolPipe, } from '@nestjs/common'; import { UserService } from './user.service'; -import { JwtAuthGuard } from '../../modules/immich-jwt/guards/jwt-auth.guard'; +import { Authenticated } from '../../decorators/authenticated.decorator'; import { AuthUserDto, GetAuthUser } from '../../decorators/auth-user.decorator'; import { CreateUserDto } from './dto/create-user.dto'; -import { AdminRolesGuard } from '../../middlewares/admin-role-guard.middleware'; import { UpdateUserDto } from './dto/update-user.dto'; import { FileInterceptor } from '@nestjs/platform-express'; import { profileImageUploadOption } from '../../config/profile-image-upload.config'; @@ -33,7 +31,7 @@ import { CreateProfileImageResponseDto } from './response-dto/create-profile-ima export class UserController { constructor(private readonly userService: UserService) {} - @UseGuards(JwtAuthGuard) + @Authenticated() @ApiBearerAuth() @Get() async getAllUsers( @@ -48,16 +46,15 @@ export class UserController { return await this.userService.getUserById(userId); } - @UseGuards(JwtAuthGuard) + @Authenticated() @ApiBearerAuth() @Get('me') async getMyUserInfo(@GetAuthUser() authUser: AuthUserDto): Promise { return await this.userService.getUserInfo(authUser); } - @UseGuards(JwtAuthGuard) + @Authenticated({ admin: true }) @ApiBearerAuth() - @UseGuards(AdminRolesGuard) @Post() async createUser( @Body(new ValidationPipe({ transform: true })) createUserDto: CreateUserDto, @@ -70,7 +67,7 @@ export class UserController { return await this.userService.getUserCount(); } - @UseGuards(JwtAuthGuard) + @Authenticated() @ApiBearerAuth() @Put() async updateUser( @@ -81,7 +78,7 @@ export class UserController { } @UseInterceptors(FileInterceptor('file', profileImageUploadOption)) - @UseGuards(JwtAuthGuard) + @Authenticated() @ApiBearerAuth() @ApiConsumes('multipart/form-data') @ApiBody({ diff --git a/server/apps/immich/src/decorators/authenticated.decorator.ts b/server/apps/immich/src/decorators/authenticated.decorator.ts new file mode 100644 index 0000000000..1543fc1bb2 --- /dev/null +++ b/server/apps/immich/src/decorators/authenticated.decorator.ts @@ -0,0 +1,16 @@ +import { UseGuards } from '@nestjs/common'; +import { AdminRolesGuard } from '../middlewares/admin-role-guard.middleware'; +import { JwtAuthGuard } from '../modules/immich-jwt/guards/jwt-auth.guard'; + +interface AuthenticatedOptions { + admin?: boolean; +} + +export const Authenticated = (options?: AuthenticatedOptions) => { + const guards: Parameters = [JwtAuthGuard]; + options = options || {}; + if (options.admin) { + guards.push(AdminRolesGuard); + } + return UseGuards(...guards); +}; diff --git a/server/apps/immich/src/middlewares/admin-role-guard.middleware.ts b/server/apps/immich/src/middlewares/admin-role-guard.middleware.ts index 04977c6d68..d555d90af7 100644 --- a/server/apps/immich/src/middlewares/admin-role-guard.middleware.ts +++ b/server/apps/immich/src/middlewares/admin-role-guard.middleware.ts @@ -1,42 +1,18 @@ -import { Injectable, CanActivate, ExecutionContext } from '@nestjs/common'; -import { Reflector } from '@nestjs/core'; -import { InjectRepository } from '@nestjs/typeorm'; -import { Repository } from 'typeorm'; -import { UserEntity } from '@app/database/entities/user.entity'; -import { ImmichJwtService } from '../modules/immich-jwt/immich-jwt.service'; +import { CanActivate, ExecutionContext, Injectable, Logger } from '@nestjs/common'; +import { Request } from 'express'; @Injectable() export class AdminRolesGuard implements CanActivate { - constructor( - private reflector: Reflector, - private jwtService: ImmichJwtService, - @InjectRepository(UserEntity) - private userRepository: Repository, - ) {} + logger = new Logger(AdminRolesGuard.name); async canActivate(context: ExecutionContext): Promise { - const request = context.switchToHttp().getRequest(); - let accessToken = ''; - - if (request.headers['authorization']) { - accessToken = request.headers['authorization'].split(' ')[1]; - } else if (request.cookies['immich_access_token']) { - accessToken = request.cookies['immich_access_token']; - } else { + const request = context.switchToHttp().getRequest(); + const isAdmin = request.user?.isAdmin || false; + if (!isAdmin) { + this.logger.log(`Denied access to admin only route: ${request.path}`); return false; } - const { userId } = await this.jwtService.validateToken(accessToken); - - if (!userId) { - return false; - } - - const user = await this.userRepository.findOne({ where: { id: userId } }); - if (!user) { - return false; - } - - return user.isAdmin; + return true; } }