refactor auth

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);

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

@@ -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<boolean> {
-    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');
-  }
-}

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<any> {
     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();
   }

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<EUserBackend | false> {
     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;
   }
 }

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<EUserBackend> {
+  async validate(
+    username: string,
+    password: string,
+  ): AsyncFailable<EUserBackend> {
+
+    // All this does is call the usersservice authenticate for authentication
     const user = await this.usersService.authenticate(username, password);
     if (HasFailed(user)) {
       throw new UnauthorizedException();

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<boolean> {
+    // 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<Permissions> {
     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>('permissions', context.getHandler()) ??
       this.reflector.get<Permissions>('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;
   }

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<EUserBackend> {