revert de-duplication of error handling; will be part of #3073

specklesystems · Sep 25, 2024 · e449a50 · e449a50
1 parent cc83de6
commit e449a50
Show file tree

Hide file tree

Showing 5 changed files with 220 additions and 158 deletions.
diff --git a/packages/server/app.ts b/packages/server/app.ts
@@ -411,8 +411,12 @@ export async function init() {
 
   // Expose prometheus metrics
   app.get('/metrics', async (req, res) => {
-    res.set('Content-Type', prometheusClient.register.contentType)
-    res.end(await prometheusClient.register.metrics())
+    try {
+      res.set('Content-Type', prometheusClient.register.contentType)
+      res.end(await prometheusClient.register.metrics())
+    } catch (ex: unknown) {
+      res.status(500).end(ex instanceof Error ? ex.message : `${ex}`)
+    }
   })
 
   // At the very end adding default error handler middleware

diff --git a/packages/server/modules/auth/rest/index.ts b/packages/server/modules/auth/rest/index.ts
@@ -155,16 +155,21 @@ export default function (app: Express) {
   Ensures a user is logged out by invalidating their token and refresh token.
    */
   app.post('/auth/logout', async (req, res) => {
-    const revokeRefreshToken = revokeRefreshTokenFactory({ db })
+    try {
+      const revokeRefreshToken = revokeRefreshTokenFactory({ db })
 
-    const token = req.body.token
-    const refreshToken = req.body.refreshToken
+      const token = req.body.token
+      const refreshToken = req.body.refreshToken
 
-    if (!token) throw new BadRequestError('Invalid request. No token provided.')
-    await revokeTokenById(token)
+      if (!token) throw new BadRequestError('Invalid request. No token provided.')
+      await revokeTokenById(token)
 
-    if (refreshToken) await revokeRefreshToken({ tokenId: refreshToken })
+      if (refreshToken) await revokeRefreshToken({ tokenId: refreshToken })
 
-    return res.status(200).send({ message: 'You have logged out.' })
+      return res.status(200).send({ message: 'You have logged out.' })
+    } catch (err) {
+      req.log.info({ err }, 'Error while trying to logout.')
+      return res.status(400).send('Something went wrong while trying to logout.')
+    }
   })
 }
diff --git a/packages/server/modules/auth/strategies/azureAd.ts b/packages/server/modules/auth/strategies/azureAd.ts
@@ -21,7 +21,7 @@ import {
 } from '@/modules/shared/helpers/envHelper'
 import type { Request } from 'express'
 import { EnvironmentResourceError } from '@/modules/shared/errors'
-import { Optional } from '@speckle/shared'
+import { ensureError, Optional } from '@speckle/shared'
 import { ServerInviteRecord } from '@/modules/serverinvites/domain/types'
 import {
   FinalizeInvitedServerRegistration,
@@ -91,87 +91,109 @@ const azureAdStrategyBuilderFactory =
       deps.passportAuthenticateHandlerBuilder('azuread-openidconnect'),
       async (req, _res, next) => {
         const serverInfo = await deps.getServerInfo()
+        const logger = req.log.child({
+          authStrategy: 'entraId',
+          serverVersion: serverInfo.version
+        })
 
-        // 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 EnvironmentResourceError('No profile provided by Entra ID')
-        }
+        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 EnvironmentResourceError('No profile provided by Entra ID')
+          }
 
-        const user = {
-          email: profile._json.email,
-          name: profile._json.name || profile.displayName
-        }
+          const user = {
+            email: profile._json.email,
+            name: profile._json.name || profile.displayName
+          }
+
+          const existingUser = await deps.getUserByEmail({ email: user.email })
 
-        const existingUser = await deps.getUserByEmail({ email: user.email })
+          if (existingUser && !existingUser.verified) {
+            throw new UnverifiedEmailSSOLoginError(undefined, {
+              info: {
+                email: user.email
+              }
+            })
+          }
 
-        if (existingUser && !existingUser.verified) {
-          throw new UnverifiedEmailSSOLoginError(undefined, {
-            info: {
-              email: user.email
+          // if there is an existing user, go ahead and log them in (regardless of
+          // whether the server is invite only or not).
+          if (existingUser) {
+            const myUser = await deps.findOrCreateUser({
+              user
+            })
+            // ID is used later for verifying access token
+            req.user = {
+              ...profile,
+              id: myUser.id,
+              email: myUser.email
             }
-          })
-        }
+            return next()
+          }
+
+          // if the server is invite only and we have no invite id, throw.
+          if (serverInfo.inviteOnly && !req.session.token) {
+            throw new UserInputError(
+              'This server is invite only. Please authenticate yourself through a valid invite link.'
+            )
+          }
 
-        // if there is an existing user, go ahead and log them in (regardless of
-        // whether the server is invite only or not).
-        if (existingUser) {
+          // 2. if you have an invite it must be valid, both for invite only and public servers
+          let invite: Optional<ServerInviteRecord> = undefined
+          if (req.session.token) {
+            invite = await deps.validateServerInvite(user.email, req.session.token)
+          }
+
+          // create the user
           const myUser = await deps.findOrCreateUser({
-            user
+            user: {
+              ...user,
+              role: invite
+                ? getResourceTypeRole(invite.resource, ServerInviteResourceType)
+                : undefined,
+              verified: !!invite
+            }
           })
+
           // ID is used later for verifying access token
           req.user = {
             ...profile,
             id: myUser.id,
-            email: myUser.email
+            email: myUser.email,
+            isNewUser: myUser.isNewUser,
+            isInvite: !!invite
           }
-          return next()
-        }
 
-        // if the server is invite only and we have no invite id, throw.
-        if (serverInfo.inviteOnly && !req.session.token) {
-          throw new UserInputError(
-            'This server is invite only. Please authenticate yourself through a valid invite link.'
-          )
-        }
+          req.log = req.log.child({ userId: myUser.id })
 
-        // 2. if you have an invite it must be valid, both for invite only and public servers
-        let invite: Optional<ServerInviteRecord> = undefined
-        if (req.session.token) {
-          invite = await deps.validateServerInvite(user.email, req.session.token)
-        }
+          // use the invite
+          await deps.finalizeInvitedServerRegistration(user.email, myUser.id)
 
-        // create the user
-        const myUser = await deps.findOrCreateUser({
-          user: {
-            ...user,
-            role: invite
-              ? getResourceTypeRole(invite.resource, ServerInviteResourceType)
-              : undefined,
-            verified: !!invite
-          }
-        })
+          // Resolve redirect path
+          req.authRedirectPath = deps.resolveAuthRedirectPath(invite)
 
-        // ID is used later for verifying access token
-        req.user = {
-          ...profile,
-          id: myUser.id,
-          email: myUser.email,
-          isNewUser: myUser.isNewUser,
-          isInvite: !!invite
+          // return to the auth flow
+          return next()
+        } catch (err) {
+          const e = ensureError(
+            err,
+            'Unexpected issue occured while authenticating with Entra ID'
+          )
+          switch (e.constructor) {
+            case UserInputError:
+              logger.info(
+                { e },
+                'User input error during Entra ID authentication callback.'
+              )
+              break
+            default:
+              logger.error(e, 'Error during Entra ID authentication callback.')
+          }
+          return next()
         }
-
-        req.log = req.log.child({ userId: myUser.id })
-
-        // use the invite
-        await deps.finalizeInvitedServerRegistration(user.email, myUser.id)
-
-        // Resolve redirect path
-        req.authRedirectPath = deps.resolveAuthRedirectPath(invite)
-
-        // return to the auth flow
-        return next()
       },
       finalizeAuthMiddleware
     )