From b03e79ae446003a9db54cf5ed74f79592a844879 Mon Sep 17 00:00:00 2001
From: Iain Sproat <68657+iainsproat@users.noreply.github.com>
Date: Wed, 7 Aug 2024 13:34:49 +0100
Subject: [PATCH] chore(logging): improve logging around passportjs strategies
 (#2593)

- ensures the request logger, containing request details, is used
---
 .../server/modules/auth/strategies/azureAd.ts    | 16 +++++++++++-----
 .../server/modules/auth/strategies/github.ts     |  6 +++++-
 .../server/modules/auth/strategies/google.ts     |  6 +++++-
 packages/server/modules/auth/strategies/oidc.ts  |  5 ++++-
 4 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/packages/server/modules/auth/strategies/azureAd.ts b/packages/server/modules/auth/strategies/azureAd.ts
index f5cdb93d2..274833693 100644
--- a/packages/server/modules/auth/strategies/azureAd.ts
+++ b/packages/server/modules/auth/strategies/azureAd.ts
@@ -83,15 +83,21 @@ const azureAdStrategyBuilder: AuthStrategyBuilder = async (
     passportAuthenticate('azuread-openidconnect'),
     async (req, _res, next) => {
       const serverInfo = await getServerInfo()
+      let logger = req.log.child({
+        authStrategy: 'entraId',
+        serverVersion: serverInfo.version
+      })
 
       try {
         // This is the only strategy that does its own type for req.user - easier to force type cast for now
         // than to refactor everything
         const profile = req.user as Optional<IProfile>
         if (!profile) {
-          throw new Error('No profile provided by Azure AD')
+          throw new Error('No profile provided by Entra ID')
         }
 
+        logger = logger.child({ profileId: profile.oid })
+
         const user = {
           email: profile._json.email,
           name: profile._json.name || profile.displayName
@@ -194,18 +200,18 @@ const azureAdStrategyBuilder: AuthStrategyBuilder = async (
       } catch (err) {
         const e = ensureError(
           err,
-          'Unexpected issue occured while authenticating with Azure AD'
+          'Unexpected issue occured while authenticating with Entra ID'
         )
 
         switch (e.constructor) {
           case UserInputError:
-            req.log.info(
+            logger.info(
               { e },
-              'User input error during Azure AD authentication callback.'
+              'User input error during Entra ID authentication callback.'
             )
             break
           default:
-            req.log.error(e, 'Error during Azure AD authentication callback.')
+            logger.error(e, 'Error during Entra ID authentication callback.')
         }
         return next()
       }
diff --git a/packages/server/modules/auth/strategies/github.ts b/packages/server/modules/auth/strategies/github.ts
index fe218012d..c31a37933 100644
--- a/packages/server/modules/auth/strategies/github.ts
+++ b/packages/server/modules/auth/strategies/github.ts
@@ -11,7 +11,6 @@ import {
   resolveAuthRedirectPathFactory
 } from '@/modules/serverinvites/services/processing'
 import { passportAuthenticate } from '@/modules/auth/services/passportService'
-import { logger } from '@/logging/logging'
 import {
   UserInputError,
   UnverifiedEmailSSOLoginError
@@ -69,6 +68,11 @@ const githubStrategyBuilder: AuthStrategyBuilder = async (
       done: VerifyCallback
     ) => {
       const serverInfo = await getServerInfo()
+      const logger = req.log.child({
+        authStrategy: 'github',
+        profileId: profile.id,
+        serverVersion: serverInfo.version
+      })
 
       try {
         const email = profile.emails?.[0].value
diff --git a/packages/server/modules/auth/strategies/google.ts b/packages/server/modules/auth/strategies/google.ts
index bdc3d7086..db9539bf1 100644
--- a/packages/server/modules/auth/strategies/google.ts
+++ b/packages/server/modules/auth/strategies/google.ts
@@ -9,7 +9,6 @@ import {
   resolveAuthRedirectPathFactory
 } from '@/modules/serverinvites/services/processing'
 import { passportAuthenticate } from '@/modules/auth/services/passportService'
-import { logger } from '@/logging/logging'
 import {
   UserInputError,
   UnverifiedEmailSSOLoginError
@@ -54,6 +53,11 @@ const googleStrategyBuilder: AuthStrategyBuilder = async (
     },
     async (req, _accessToken, _refreshToken, profile, done) => {
       const serverInfo = await getServerInfo()
+      const logger = req.log.child({
+        authStrategy: 'google',
+        profileId: profile.id,
+        serverVersion: serverInfo.version
+      })
 
       try {
         const email = profile.emails?.[0].value
diff --git a/packages/server/modules/auth/strategies/oidc.ts b/packages/server/modules/auth/strategies/oidc.ts
index 7de05e149..c4658f13f 100644
--- a/packages/server/modules/auth/strategies/oidc.ts
+++ b/packages/server/modules/auth/strategies/oidc.ts
@@ -8,7 +8,6 @@ import {
   finalizeInvitedServerRegistrationFactory,
   resolveAuthRedirectPathFactory
 } from '@/modules/serverinvites/services/processing'
-import { logger } from '@/logging/logging'
 import {
   getOidcDiscoveryUrl,
   getOidcClientId,
@@ -60,6 +59,10 @@ const oidcStrategyBuilder: AuthStrategyBuilder = async (
         req.session.userinfo = userinfo
 
         const serverInfo = await getServerInfo()
+        const logger = req.log.child({
+          authStrategy: 'oidc',
+          serverVersion: serverInfo.version
+        })
 
         // TODO: req.session.inviteId doesn't appear to exist, but i'm not removing it to not break things
         const token: Optional<string> =