fix(realtime): Allow connections for guest users

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

backend/src/realtime/realtime-note/realtime-note.ts

@@ -52,7 +52,7 @@ export class RealtimeNote extends (EventEmitter as TypedEventEmitterConstructor<
    */
   public addClient(client: WebsocketConnection): void {
     this.clients.add(client);
-    this.logger.debug(`User '${client.getUser().username}' connected`);
+    this.logger.debug(`User '${client.getUsername()}' connected`);
   }
 
   /**
@@ -63,7 +63,7 @@ export class RealtimeNote extends (EventEmitter as TypedEventEmitterConstructor<
   public removeClient(client: WebsocketConnection): void {
     this.clients.delete(client);
     this.logger.debug(
-      `User '${client.getUser().username}' disconnected. ${
+      `User '${client.getUsername()}' disconnected. ${
         this.clients.size
       } clients left.`,
     );

backend/src/realtime/realtime-note/test-utils/mock-connection.ts

@@ -18,5 +18,6 @@ export function mockConnection(synced: boolean): WebsocketConnection {
     isSynced: jest.fn(() => synced),
     send: jest.fn(),
     getUser: jest.fn(() => Mock.of<User>({ username: 'mockedUser' })),
+    getUsername: jest.fn(() => 'mocked user'),
   });
 }

backend/src/realtime/realtime-note/websocket-connection.spec.ts

@@ -192,4 +192,26 @@ describe('websocket connection', () => {
 
     expect(sut.getUser()).toBe(mockedUser);
   });
+
+  it('returns the correct username', () => {
+    const mockedUserWithUsername = Mock.of<User>({ username: 'MockUser' });
+
+    const sut = new WebsocketConnection(
+      mockedWebsocket,
+      mockedUserWithUsername,
+      mockedRealtimeNote,
+    );
+
+    expect(sut.getUsername()).toBe('MockUser');
+  });
+
+  it('returns a fallback if no username has been set', () => {
+    const sut = new WebsocketConnection(
+      mockedWebsocket,
+      mockedUser,
+      mockedRealtimeNote,
+    );
+
+    expect(sut.getUsername()).toBe('Guest');
+  });
 });

backend/src/realtime/realtime-note/websocket-connection.ts

@@ -30,7 +30,7 @@ export class WebsocketConnection {
    */
   constructor(
     websocket: WebSocket,
-    private user: User,
+    private user: User | null,
     realtimeNote: RealtimeNote,
   ) {
     const awareness = realtimeNote.getAwareness();
@@ -94,7 +94,11 @@ export class WebsocketConnection {
     return this.controlledAwarenessIds;
   }
 
-  public getUser(): User {
+  public getUser(): User | null {
     return this.user;
   }
+
+  public getUsername(): string {
+    return this.getUser()?.username ?? 'Guest';
+  }
 }

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

@@ -3,6 +3,7 @@
  *
  * SPDX-License-Identifier: AGPL-3.0-only
  */
+import { Optional } from '@mrdrogdrog/optional';
 import { ConfigModule } from '@nestjs/config';
 import { EventEmitterModule } from '@nestjs/event-emitter';
 import { Test, TestingModule } from '@nestjs/testing';
@@ -64,7 +65,9 @@ describe('Websocket gateway', () => {
   const mockedValidSessionCookie = 'mockedValidSessionCookie';
   const mockedSessionIdWithUser = 'mockedSessionIdWithUser';
   const mockedValidUrl = 'mockedValidUrl';
+  const mockedValidGuestUrl = 'mockedValidGuestUrl';
   const mockedValidNoteId = 'mockedValidNoteId';
+  const mockedValidGuestNoteId = 'mockedValidGuestNoteId';
 
   let sessionExistsForUser = true;
   let noteExistsForNoteId = true;
@@ -151,14 +154,15 @@ describe('Websocket gateway', () => {
     permissionsService = module.get<PermissionsService>(PermissionsService);
 
     jest
-      .spyOn(sessionService, 'extractVerifiedSessionIdFromRequest')
+      .spyOn(sessionService, 'extractSessionIdFromRequest')
-      .mockImplementation((request: IncomingMessage): string => {
+      .mockImplementation(
-        if (request.headers.cookie === mockedValidSessionCookie) {
+        (request: IncomingMessage): Optional<string> =>
-          return mockedSessionIdWithUser;
+          Optional.ofNullable(
-        } else {
+            request.headers?.cookie === mockedValidSessionCookie
-          throw new Error('no valid session cookie found');
+              ? mockedSessionIdWithUser
-        }
+              : null,
-      });
+          ),
+      );
 
     const mockUsername = 'mockUsername';
     jest
@@ -184,26 +188,37 @@ describe('Websocket gateway', () => {
       .mockImplementation((request: IncomingMessage): string => {
         if (request.url === mockedValidUrl) {
           return mockedValidNoteId;
+        } else if (request.url === mockedValidGuestUrl) {
+          return mockedValidGuestNoteId;
         } else {
           throw new Error('no valid note id found');
         }
       });
 
     const mockedNote = Mock.of<Note>({ id: 4711 });
+    const mockedGuestNote = Mock.of<Note>({ id: 1235 });
     jest
       .spyOn(notesService, 'getNoteByIdOrAlias')
-      .mockImplementation((noteId: string) =>
+      .mockImplementation((noteId: string) => {
-        noteExistsForNoteId && noteId === mockedValidNoteId
+        if (noteExistsForNoteId && noteId === mockedValidNoteId) {
-          ? Promise.resolve(mockedNote)
+          return Promise.resolve(mockedNote);
-          : Promise.reject('no note found'),
+        }
-      );
+        if (noteId === mockedValidGuestNoteId) {
+          return Promise.resolve(mockedGuestNote);
+        } else {
+          return Promise.reject('no note found');
+        }
+      });
 
     jest
       .spyOn(permissionsService, 'mayRead')
       .mockImplementation(
         (user: User | null, note: Note): Promise<boolean> =>
           Promise.resolve(
-            user === mockUser && note === mockedNote && userHasReadPermissions,
+            (user === mockUser &&
+              note === mockedNote &&
+              userHasReadPermissions) ||
+              (user === null && note === mockedGuestNote),
           ),
       );
 
@@ -231,6 +246,22 @@ describe('Websocket gateway', () => {
     addClientSpy = jest.spyOn(mockedRealtimeNote, 'addClient');
   });
 
+  it('adds a valid connection request without a session', async () => {
+    const request = Mock.of<IncomingMessage>({
+      socket: {
+        remoteAddress: 'mockHost',
+      },
+      url: mockedValidGuestUrl,
+      headers: {},
+    });
+
+    await expect(
+      gateway.handleConnection(mockedWebsocket, request),
+    ).resolves.not.toThrow();
+    expect(addClientSpy).toHaveBeenCalledWith(mockedWebsocketConnection);
+    expect(mockedWebsocketCloseSpy).not.toHaveBeenCalled();
+  });
+
   it('adds a valid connection request', async () => {
     const request = Mock.of<IncomingMessage>({
       socket: {

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

@@ -52,10 +52,12 @@ export class WebsocketGateway implements OnGatewayConnection {
         extractNoteIdFromRequestUrl(request),
       );
 
+      const username = user?.username ?? 'guest';
+
       if (!(await this.permissionsService.mayRead(user, note))) {
         //TODO: [mrdrogdrog] inform client about reason of disconnect.
         this.logger.log(
-          `Access denied to note '${note.id}' for user '${user.username}'`,
+          `Access denied to note '${note.id}' for user '${username}'`,
           'handleConnection',
         );
         clientSocket.close();
@@ -65,7 +67,7 @@ export class WebsocketGateway implements OnGatewayConnection {
       this.logger.debug(
         `New realtime connection to note '${note.id}' (${
           note.publicId
-        }) by user '${user.username}' from ${
+        }) by user '${username}' from ${
           request.socket.remoteAddress ?? 'unknown'
         }`,
       );
@@ -98,11 +100,15 @@ export class WebsocketGateway implements OnGatewayConnection {
    */
   private async findUserByRequestSession(
     request: IncomingMessage,
-  ): Promise<User> {
+  ): Promise<User | null> {
-    const sessionId =
+    const sessionId = this.sessionService.extractSessionIdFromRequest(request);
-      this.sessionService.extractVerifiedSessionIdFromRequest(request);
+
+    if (sessionId.isEmpty()) {
+      return null;
+    }
+
     const username = await this.sessionService.fetchUsernameForSessionId(
-      sessionId,
+      sessionId.get(),
     );
     return await this.userService.getUserByUsername(username);
   }

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

@@ -129,9 +129,9 @@ describe('SessionService', () => {
       const mockedRequest = Mock.of<IncomingMessage>({
         headers: {},
       });
-      expect(() =>
+      expect(
-        sessionService.extractVerifiedSessionIdFromRequest(mockedRequest),
+        sessionService.extractSessionIdFromRequest(mockedRequest).isEmpty(),
-      ).toThrow('No hedgedoc-session cookie found');
+      ).toBeTruthy();
     });
 
     it("fails if the cookie header isn't valid", () => {
@@ -139,9 +139,9 @@ describe('SessionService', () => {
         headers: { cookie: 'no' },
       });
       mockParseCookieModule(`s:anyValidSessionId.validSignature`);
-      expect(() =>
+      expect(
-        sessionService.extractVerifiedSessionIdFromRequest(mockedRequest),
+        sessionService.extractSessionIdFromRequest(mockedRequest).isEmpty(),
-      ).toThrow('No hedgedoc-session cookie found');
+      ).toBeTruthy();
     });
 
     it("fails if the hedgedoc session cookie isn't marked as signed", () => {
@@ -150,8 +150,10 @@ describe('SessionService', () => {
       });
       mockParseCookieModule('sessionId.validSignature');
       expect(() =>
-        sessionService.extractVerifiedSessionIdFromRequest(mockedRequest),
+        sessionService.extractSessionIdFromRequest(mockedRequest),
-      ).toThrow("cookie doesn't look like a signed cookie");
+      ).toThrow(
+        'cookie "hedgedoc-session" doesn\'t look like a signed session cookie',
+      );
     });
 
     it("fails if the hedgedoc session cookie doesn't contain a session id", () => {
@@ -160,8 +162,10 @@ describe('SessionService', () => {
       });
       mockParseCookieModule('s:.validSignature');
       expect(() =>
-        sessionService.extractVerifiedSessionIdFromRequest(mockedRequest),
+        sessionService.extractSessionIdFromRequest(mockedRequest),
-      ).toThrow("cookie doesn't look like a signed cookie");
+      ).toThrow(
+        'cookie "hedgedoc-session" doesn\'t look like a signed session cookie',
+      );
     });
 
     it("fails if the hedgedoc session cookie doesn't contain a signature", () => {
@@ -170,8 +174,10 @@ describe('SessionService', () => {
       });
       mockParseCookieModule('s:sessionId.');
       expect(() =>
-        sessionService.extractVerifiedSessionIdFromRequest(mockedRequest),
+        sessionService.extractSessionIdFromRequest(mockedRequest),
-      ).toThrow("cookie doesn't look like a signed cookie");
+      ).toThrow(
+        'cookie "hedgedoc-session" doesn\'t look like a signed session cookie',
+      );
     });
 
     it("fails if the hedgedoc session cookie isn't signed correctly", () => {
@@ -180,8 +186,8 @@ describe('SessionService', () => {
       });
       mockParseCookieModule('s:sessionId.invalidSignature');
       expect(() =>
-        sessionService.extractVerifiedSessionIdFromRequest(mockedRequest),
+        sessionService.extractSessionIdFromRequest(mockedRequest),
-      ).toThrow("Signature of hedgedoc-session cookie isn't valid.");
+      ).toThrow('signature of cookie "hedgedoc-session" isn\'t valid.');
     });
 
     it('can extract a session id from a valid request', () => {
@@ -190,7 +196,7 @@ describe('SessionService', () => {
       });
       mockParseCookieModule(`s:${validSessionId}.validSignature`);
       expect(
-        sessionService.extractVerifiedSessionIdFromRequest(mockedRequest),
+        sessionService.extractSessionIdFromRequest(mockedRequest).get(),
       ).toBe(validSessionId);
     });
   });

backend/src/session/session.service.ts

@@ -70,27 +70,37 @@ export class SessionService {
    * Extracts the hedgedoc session cookie from the given {@link IncomingMessage request} and checks if the signature is correct.
    *
    * @param request The http request that contains a session cookie
-   * @return The extracted session id
+   * @return An {@link Optional optional} that either contains the extracted session id or is empty if no session cookie has been found
-   * @throws Error if no session cookie was found
+   * @throws Error if the cookie has been found but the content is malformed
-   * @throws Error if the cookie content is malformed
+   * @throws Error if the cookie has been found but the content isn't signed
-   * @throws Error if the cookie content isn't signed
    */
-  extractVerifiedSessionIdFromRequest(request: IncomingMessage): string {
+  extractSessionIdFromRequest(request: IncomingMessage): Optional<string> {
-    return Optional.ofNullable(request.headers.cookie)
+    return Optional.ofNullable(request.headers?.cookie)
       .map((cookieHeader) => parseCookie(cookieHeader)[HEDGEDOC_SESSION])
-      .orThrow(() => new Error(`No ${HEDGEDOC_SESSION} cookie found`))
+      .map((rawCookie) =>
-      .map((cookie) => SessionService.sessionCookieContentRegex.exec(cookie))
+        this.extractVerifiedSessionIdFromCookieContent(rawCookie),
-      .orThrow(
+      );
-        () =>
+  }
-          new Error(
+
-            `${HEDGEDOC_SESSION} cookie doesn't look like a signed cookie`,
+  /**
-          ),
+   * Parses the given session cookie content and extracts the session id.
-      )
+   *
-      .guard(
+   * @param rawCookie The cookie to parse
-        (cookie) => unsign(cookie[1], this.authConfig.session.secret) !== false,
+   * @return The extracted session id
-        () => new Error(`Signature of ${HEDGEDOC_SESSION} cookie isn't valid.`),
+   * @throws Error if the cookie has been found but the content is malformed
-      )
+   * @throws Error if the cookie has been found but the content isn't signed
-      .map((cookie) => cookie[2])
+   */
-      .get();
+  private extractVerifiedSessionIdFromCookieContent(rawCookie: string): string {
+    const parsedCookie =
+      SessionService.sessionCookieContentRegex.exec(rawCookie);
+    if (parsedCookie === null) {
+      throw new Error(
+        `cookie "${HEDGEDOC_SESSION}" doesn't look like a signed session cookie`,
+      );
+    }
+    if (unsign(parsedCookie[1], this.authConfig.session.secret) === false) {
+      throw new Error(`signature of cookie "${HEDGEDOC_SESSION}" isn't valid.`);
+    }
+    return parsedCookie[2];
   }
 }