From c4c5cbd5d056c8b2eb960b794c4b3435f065a9a5 Mon Sep 17 00:00:00 2001
From: Erik Michelson <github@erik.michelson.eu>
Date: Fri, 13 Sep 2024 16:29:26 +0200
Subject: [PATCH] fix(auth/oidc): string "undefined" for missing userinfo
 response fields

The userinfo response endpoint from the OIDC provider should not be
trusted to return what we expect. Fields could be undefined. In that
case HedgeDoc would have written "undefined" into the fields for
profile picture or email address.
This fix checks for fields being undefined and returns a default value
in that case.

Signed-off-by: Erik Michelson <github@erik.michelson.eu>
---
 backend/src/identity/oidc/oidc.service.ts | 49 ++++++++++++++++++-----
 1 file changed, 40 insertions(+), 9 deletions(-)

diff --git a/backend/src/identity/oidc/oidc.service.ts b/backend/src/identity/oidc/oidc.service.ts
index 0a1210518..5fbfc0fbb 100644
--- a/backend/src/identity/oidc/oidc.service.ts
+++ b/backend/src/identity/oidc/oidc.service.ts
@@ -9,7 +9,7 @@ import {
   InternalServerErrorException,
   NotFoundException,
 } from '@nestjs/common';
-import { Client, generators, Issuer } from 'openid-client';
+import { Client, generators, Issuer, UserinfoResponse } from 'openid-client';
 
 import appConfiguration, { AppConfig } from '../../config/app.config';
 import authConfiguration, {
@@ -178,16 +178,31 @@ export class OidcService {
 
     request.session.oidcIdToken = tokenSet.id_token;
     const userInfoResponse = await client.userinfo(tokenSet);
-    const userId = String(
-      userInfoResponse[oidcConfig.userIdField] || userInfoResponse.sub,
+    const userId = OidcService.getResponseFieldValue(
+      userInfoResponse,
+      oidcConfig.userIdField,
+      userInfoResponse.sub,
     );
-    const username = String(
-      userInfoResponse[oidcConfig.userNameField] ||
-        userInfoResponse[oidcConfig.userIdField],
+    const username = OidcService.getResponseFieldValue(
+      userInfoResponse,
+      oidcConfig.userNameField,
+      userId,
     ).toLowerCase() as Lowercase<string>;
-    const displayName = String(userInfoResponse[oidcConfig.displayNameField]);
-    const email = String(userInfoResponse[oidcConfig.emailField]);
-    const photoUrl = String(userInfoResponse[oidcConfig.profilePictureField]);
+    const displayName = OidcService.getResponseFieldValue(
+      userInfoResponse,
+      oidcConfig.displayNameField,
+      username,
+    );
+    const email = OidcService.getResponseFieldValue(
+      userInfoResponse,
+      oidcConfig.emailField,
+      undefined,
+    );
+    const photoUrl = OidcService.getResponseFieldValue(
+      userInfoResponse,
+      oidcConfig.profilePictureField,
+      undefined,
+    );
     const newUserData = {
       username,
       displayName,
@@ -261,4 +276,20 @@ export class OidcService {
     }
     return `${endSessionEndpoint}?post_logout_redirect_uri=${this.appConfig.baseUrl}${idToken ? `&id_token_hint=${idToken}` : ''}`;
   }
+
+  /**
+   * Returns a specific field from the userinfo object or a default value.
+   *
+   * @param {UserinfoResponse} response The response from the OIDC userinfo endpoint
+   * @param {string} field The field to get from the response
+   * @param {string|undefined} defaultValue The default value to return if the value is empty
+   * @returns {string|undefined} The value of the field from the response or the default value
+   */
+  private static getResponseFieldValue<T extends string | undefined>(
+    response: UserinfoResponse,
+    field: string,
+    defaultValue: T,
+  ): string | T {
+    return response[field] ? String(response[field]) : defaultValue;
+  }
 }