diff --git a/backend/src/api/private/auth/auth.controller.ts b/backend/src/api/private/auth/auth.controller.ts index f3da2c92c..f0e659b10 100644 --- a/backend/src/api/private/auth/auth.controller.ts +++ b/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'; } diff --git a/backend/src/api/utils/session-authprovider.decorator.ts b/backend/src/api/utils/session-authprovider.decorator.ts index 6268b7470..19c56a1c6 100644 --- a/backend/src/api/utils/session-authprovider.decorator.ts +++ b/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 diff --git a/backend/src/identity/session.guard.ts b/backend/src/identity/session.guard.ts index a0b6b2ff5..1da0f56f6 100644 --- a/backend/src/identity/session.guard.ts +++ b/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 { 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"); } diff --git a/backend/src/realtime/websocket/websocket.gateway.ts b/backend/src/realtime/websocket/websocket.gateway.ts index 1e05b973a..10c9c225a 100644 --- a/backend/src/realtime/websocket/websocket.gateway.ts +++ b/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); } } diff --git a/backend/src/session/session.service.spec.ts b/backend/src/session/session.service.spec.ts index 7e10aec5e..cf803f43c 100644 --- a/backend/src/session/session.service.spec.ts +++ b/backend/src/session/session.service.spec.ts @@ -36,7 +36,7 @@ describe('SessionService', () => { jest.resetModules(); jest.restoreAllMocks(); const mockedExistingSession = Mock.of({ - user: mockUsername, + username: mockUsername, }); mockedTypeormStore = Mock.of({ connect: jest.fn(() => mockedTypeormStore), diff --git a/backend/src/session/session.service.ts b/backend/src/session/session.service.ts index e79d4dcc2..ca5c6c51f 100644 --- a/backend/src/session/session.service.ts +++ b/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 { + fetchUsernameForSessionId(sessionId: string): Promise { 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), ); }); }