diff --git a/backend/src/collections/userdb/userdb.module.ts b/backend/src/collections/userdb/userdb.module.ts index fd0dd62..ce0db0a 100644 --- a/backend/src/collections/userdb/userdb.module.ts +++ b/backend/src/collections/userdb/userdb.module.ts @@ -23,7 +23,6 @@ export class UsersModule implements OnModuleInit { constructor( private usersService: UsersService, - private userRolesService: UserRolesService, private authConfigService: AuthConfigService, ) {} diff --git a/backend/src/main.ts b/backend/src/main.ts index c6bc806..cb59140 100644 --- a/backend/src/main.ts +++ b/backend/src/main.ts @@ -7,7 +7,6 @@ import { import * as multipart from 'fastify-multipart'; import { ValidateOptions } from 'picsur-shared/dist/util/validate'; import { AppModule } from './app.module'; -import { UsersService } from './collections/userdb/userdb.service'; import { UserRolesService } from './collections/userdb/userrolesdb.service'; import { HostConfigService } from './config/early/host.config.service'; import { MainExceptionFilter } from './layers/httpexception/httpexception.filter'; @@ -37,7 +36,6 @@ async function bootstrap() { app.useGlobalGuards( new MainAuthGuard( app.get(Reflector), - app.get(UsersService), app.get(UserRolesService), ), ); diff --git a/backend/src/managers/auth/auth.service.ts b/backend/src/managers/auth/auth.service.ts index 68210e8..5c1bc17 100644 --- a/backend/src/managers/auth/auth.service.ts +++ b/backend/src/managers/auth/auth.service.ts @@ -19,6 +19,8 @@ export class AuthManagerService { }, }); + // Validate to be sure, this makes client experience better + // in case of any failures const errors = await strictValidate(jwtData); if (errors.length > 0) { this.logger.warn(errors); diff --git a/backend/src/managers/auth/guards/admin.guard.ts b/backend/src/managers/auth/guards/admin.guard.ts deleted file mode 100644 index 10e2750..0000000 --- a/backend/src/managers/auth/guards/admin.guard.ts +++ /dev/null @@ -1,31 +0,0 @@ -import { - CanActivate, - ExecutionContext, - Injectable, - Logger -} from '@nestjs/common'; -import { plainToClass } from 'class-transformer'; -import { strictValidate } from 'picsur-shared/dist/util/validate'; -import { EUserBackend } from '../../../models/entities/user.entity'; - -@Injectable() -export class AdminGuard implements CanActivate { - private readonly logger = new Logger('AdminGuard'); - - async canActivate(context: ExecutionContext): Promise { - const request = context.switchToHttp().getRequest(); - - if (!request.user) { - return false; - } - - const user = plainToClass(EUserBackend, request.user); - const errors = await strictValidate(user); - if (errors.length > 0) { - this.logger.warn(errors); - return false; - } - - return user.roles.includes('admin'); - } -} diff --git a/backend/src/managers/auth/guards/guest.strategy.ts b/backend/src/managers/auth/guards/guest.strategy.ts index 212cb9a..659f0e3 100644 --- a/backend/src/managers/auth/guards/guest.strategy.ts +++ b/backend/src/managers/auth/guards/guest.strategy.ts @@ -1,4 +1,4 @@ -import { Injectable, Logger } from '@nestjs/common'; +import { Injectable } from '@nestjs/common'; import { PassportStrategy } from '@nestjs/passport'; import { Request } from 'express'; import { ParamsDictionary } from 'express-serve-static-core'; @@ -15,6 +15,7 @@ type ReqType = Request< >; class GuestPassportStrategy extends Strategy { + // Will be overridden by the nest implementation async validate(req: ReqType): Promise { return undefined; } @@ -30,12 +31,11 @@ export class GuestStrategy extends PassportStrategy( GuestPassportStrategy, 'guest', ) { - private readonly logger = new Logger('GuestStrategy'); - constructor(private guestService: GuestService) { super(); } + // Return the guest user created by the guestservice override async validate(payload: any) { return await this.guestService.getGuestUser(); } diff --git a/backend/src/managers/auth/guards/jwt.strategy.ts b/backend/src/managers/auth/guards/jwt.strategy.ts index 3b9c77c..483a7e1 100644 --- a/backend/src/managers/auth/guards/jwt.strategy.ts +++ b/backend/src/managers/auth/guards/jwt.strategy.ts @@ -14,7 +14,8 @@ import { EUserBackend } from '../../../models/entities/user.entity'; export class JwtStrategy extends PassportStrategy(Strategy, 'jwt') { private readonly logger = new Logger('JwtStrategy'); - constructor(@Inject('JWT_SECRET') private jwtSecret: string) { + constructor(@Inject('JWT_SECRET') jwtSecret: string) { + // This will validate the jwt token itself super({ jwtFromRequest: ExtractJwt.fromAuthHeaderAsBearerToken(), ignoreExpiration: false, @@ -25,13 +26,14 @@ export class JwtStrategy extends PassportStrategy(Strategy, 'jwt') { async validate(payload: any): Promise { const jwt = plainToClass(JwtDataDto, payload); + // This then validates the data inside the jwt token const errors = await strictValidate(jwt); - if (errors.length > 0) { this.logger.warn(errors); return false; } + // And return the user return jwt.user; } } diff --git a/backend/src/managers/auth/guards/localauth.strategy.ts b/backend/src/managers/auth/guards/localauth.strategy.ts index 0b334dc..96b8fa8 100644 --- a/backend/src/managers/auth/guards/localauth.strategy.ts +++ b/backend/src/managers/auth/guards/localauth.strategy.ts @@ -11,7 +11,12 @@ export class LocalAuthStrategy extends PassportStrategy(Strategy, 'local') { super(); } - async validate(username: string, password: string): AsyncFailable { + async validate( + username: string, + password: string, + ): AsyncFailable { + + // All this does is call the usersservice authenticate for authentication const user = await this.usersService.authenticate(username, password); if (HasFailed(user)) { throw new UnauthorizedException(); diff --git a/backend/src/managers/auth/guards/main.guard.ts b/backend/src/managers/auth/guards/main.guard.ts index 4bbf4fc..78bca58 100644 --- a/backend/src/managers/auth/guards/main.guard.ts +++ b/backend/src/managers/auth/guards/main.guard.ts @@ -10,25 +10,28 @@ import { AuthGuard } from '@nestjs/passport'; import { plainToClass } from 'class-transformer'; import { Fail, Failable, HasFailed } from 'picsur-shared/dist/types'; import { strictValidate } from 'picsur-shared/dist/util/validate'; -import { UsersService } from '../../../collections/userdb/userdb.service'; 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'; +// 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 +// This way a user will get his own account when logged in, but received guest permissions when not + @Injectable() export class MainAuthGuard extends AuthGuard(['jwt', 'guest']) { private readonly logger = new Logger('MainAuthGuard'); constructor( private reflector: Reflector, - private usersService: UsersService, private userRolesService: UserRolesService, ) { super(); } override async canActivate(context: ExecutionContext): Promise { + // Sanity check const result = await super.canActivate(context); if (result !== true) { this.logger.error('Main Auth has denied access, this should not happen'); @@ -39,12 +42,14 @@ export class MainAuthGuard extends AuthGuard(['jwt', 'guest']) { context.switchToHttp().getRequest().user, ); + // These are the permissions required to access the route const permissions = this.extractPermissions(context); if (HasFailed(permissions)) { this.logger.warn('Route Permissions: ' + permissions.getReason()); throw new InternalServerErrorException(); } + // These are the permissions the user has const userPermissions = await this.userRolesService.getPermissions(user); if (HasFailed(userPermissions)) { this.logger.warn('User Permissions: ' + userPermissions.getReason()); @@ -58,19 +63,19 @@ export class MainAuthGuard extends AuthGuard(['jwt', 'guest']) { private extractPermissions(context: ExecutionContext): Failable { const handlerName = context.getHandler().name; + // Fall back to class permissions if none on function + // But function has higher priority than class const permissions = this.reflector.get('permissions', context.getHandler()) ?? this.reflector.get('permissions', context.getClass()); - if (permissions === undefined) { + if (permissions === undefined) return Fail( `${handlerName} does not have any permissions defined, denying access`, ); - } - if (!isPermissionsArray(permissions)) { + if (!isPermissionsArray(permissions)) return Fail(`Permissions for ${handlerName} is not a string array`); - } return permissions; } diff --git a/backend/src/managers/auth/guest.service.ts b/backend/src/managers/auth/guest.service.ts index 73fac9e..a2d8181 100644 --- a/backend/src/managers/auth/guest.service.ts +++ b/backend/src/managers/auth/guest.service.ts @@ -9,8 +9,8 @@ export class GuestService { constructor(private usersService: UsersService) { this.fallBackUser = new EUserBackend(); - this.fallBackUser.roles = ['guest']; this.fallBackUser.username = 'guest'; + this.fallBackUser.roles = ['guest']; } public async getGuestUser(): Promise {