fix: change sessionstate type to prevent unset values

Signed-off-by: Tilman Vatteroth <git@tilmanvatteroth.de>

backend/src/api/private/auth/auth.controller.ts

@@ -26,6 +26,7 @@ import { RegisterDto } from '../../../identity/local/register.dto';
 import { UpdatePasswordDto } from '../../../identity/local/update-password.dto';
 import { SessionGuard } from '../../../identity/session.guard';
 import { ConsoleLoggerService } from '../../../logger/console-logger.service';
+import { SessionState } from '../../../session/session.service';
 import { User } from '../../../users/user.entity';
 import { UsersService } from '../../../users/users.service';
 import { LoginEnabledGuard } from '../../utils/login-enabled.guard';
@@ -34,10 +35,7 @@ import { RegistrationEnabledGuard } from '../../utils/registration-enabled.guard
 import { RequestUser } from '../../utils/request-user.decorator';
 
 type RequestWithSession = Request & {
-  session: {
-    authProvider: string;
-    user: string;
-  };
+  session: SessionState;
 };
 
 @ApiTags('auth')
@@ -65,7 +63,7 @@ export class AuthController {
     );
     // ToDo: Figure out how to rollback user if anything with this calls goes wrong
     await this.identityService.createLocalIdentity(user, registerDto.password);
-    request.session.user = registerDto.username;
+    request.session.username = registerDto.username;
     request.session.authProvider = 'local';
   }
 
@@ -96,7 +94,7 @@ export class AuthController {
     @Body() loginDto: LoginDto,
   ): void {
     // There is no further testing needed as we only get to this point if LocalAuthGuard was successful
-    request.session.user = loginDto.username;
+    request.session.username = loginDto.username;
     request.session.authProvider = 'local';
   }
 
@@ -110,7 +108,7 @@ export class AuthController {
     @Body() loginDto: LdapLoginDto,
   ): void {
     // There is no further testing needed as we only get to this point if LocalAuthGuard was successful
-    request.session.user = loginDto.username;
+    request.session.username = loginDto.username;
     request.session.authProvider = 'ldap';
   }
 

backend/src/api/utils/session-authprovider.decorator.ts

@@ -10,6 +10,8 @@ import {
 } from '@nestjs/common';
 import { Request } from 'express';
 
+import { SessionState } from '../../session/session.service';
+
 /**
  * Extracts the auth provider identifier from a session inside a request
  *
@@ -19,9 +21,7 @@ import { Request } from 'express';
 export const SessionAuthProvider = createParamDecorator(
   (data: unknown, ctx: ExecutionContext) => {
     const request: Request & {
-      session: {
-        authProvider: string;
-      };
+      session: SessionState;
     } = ctx.switchToHttp().getRequest();
     if (!request.session?.authProvider) {
       // We should have an auth provider here, otherwise something is wrong

backend/src/identity/session.guard.ts

@@ -15,14 +15,15 @@ import { GuestAccess } from '../config/guest_access.enum';
 import noteConfiguration, { NoteConfig } from '../config/note.config';
 import { NotInDBError } from '../errors/errors';
 import { ConsoleLoggerService } from '../logger/console-logger.service';
+import { SessionState } from '../session/session.service';
 import { User } from '../users/user.entity';
 import { UsersService } from '../users/users.service';
 
 /**
  * This guard checks if a session is present.
  *
- * If there is a username in `request.session.user` it will try to get this user from the database and put it into `request.user`. See {@link RequestUser}.
- * If there is no `request.session.user`, but any GuestAccess is configured, `request.session.authProvider` is set to `guest` to indicate a guest user.
+ * If there is a username in `request.session.username` it will try to get this user from the database and put it into `request.user`. See {@link RequestUser}.
+ * If there is no `request.session.username`, but any GuestAccess is configured, `request.session.authProvider` is set to `guest` to indicate a guest user.
  *
  * @throws UnauthorizedException
  */
@@ -39,28 +40,25 @@ export class SessionGuard implements CanActivate {
 
   async canActivate(context: ExecutionContext): Promise<boolean> {
     const request: Request & {
-      session?: { user: string; authProvider: string };
+      session?: SessionState;
       user?: User;
     } = context.switchToHttp().getRequest();
-    if (!request.session?.user) {
-      if (this.noteConfig.guestAccess !== GuestAccess.DENY) {
-        if (request.session) {
-          request.session.authProvider = 'guest';
-          return true;
-        }
+    const username = request.session?.username;
+    if (!username) {
+      if (this.noteConfig.guestAccess !== GuestAccess.DENY && request.session) {
+        request.session.authProvider = 'guest';
+        return true;
       }
       this.logger.debug('The user has no session.');
       throw new UnauthorizedException("You're not logged in");
     }
     try {
-      request.user = await this.userService.getUserByUsername(
-        request.session.user,
-      );
+      request.user = await this.userService.getUserByUsername(username);
       return true;
     } catch (e) {
       if (e instanceof NotInDBError) {
         this.logger.debug(
-          `The user '${request.session.user}' does not exist, but has a session.`,
+          `The user '${username}' does not exist, but has a session.`,
         );
         throw new UnauthorizedException("You're not logged in");
       }

backend/src/realtime/websocket/websocket.gateway.ts

@@ -110,6 +110,9 @@ export class WebsocketGateway implements OnGatewayConnection {
     const username = await this.sessionService.fetchUsernameForSessionId(
       sessionId.get(),
     );
+    if (username === undefined) {
+      return null;
+    }
     return await this.userService.getUserByUsername(username);
   }
 }

backend/src/session/session.service.spec.ts

@@ -36,7 +36,7 @@ describe('SessionService', () => {
     jest.resetModules();
     jest.restoreAllMocks();
     const mockedExistingSession = Mock.of<SessionState>({
-      user: mockUsername,
+      username: mockUsername,
     });
     mockedTypeormStore = Mock.of<TypeormStore>({
       connect: jest.fn(() => mockedTypeormStore),

backend/src/session/session.service.ts

@@ -22,7 +22,7 @@ import { HEDGEDOC_SESSION } from '../utils/session';
 
 export interface SessionState {
   cookie: unknown;
-  user: string;
+  username?: string;
   authProvider: string;
 }
 
@@ -58,10 +58,10 @@ export class SessionService {
    * @param sessionId The session id for which the owning user should be found
    * @return A Promise that either resolves with the username or rejects with an error
    */
-  fetchUsernameForSessionId(sessionId: string): Promise<string> {
+  fetchUsernameForSessionId(sessionId: string): Promise<string | undefined> {
     return new Promise((resolve, reject) => {
       this.typeormStore.get(sessionId, (error?: Error, result?: SessionState) =>
-        error || !result ? reject(error) : resolve(result.user),
+        error || !result ? reject(error) : resolve(result.username),
       );
     });
   }