Merge pull request #13448 from Budibase/fix-oidc-error-logging

Console.error lint rule

.eslintrc.json

@@ -42,6 +42,8 @@
       },
       "rules": {
         "no-unused-vars": "off",
+        "local-rules/no-budibase-imports": "error",
+        "local-rules/no-console-error": "error",
         "@typescript-eslint/no-unused-vars": [
           "error",
           {

eslint-local-rules/index.js

@@ -1,4 +1,25 @@
 module.exports = {
+  "no-console-error": {
+    create: function(context) {
+      return {
+        CallExpression(node) {
+          if (
+            node.callee.type === "MemberExpression" &&
+            node.callee.object.name === "console" &&
+            node.callee.property.name === "error" &&
+            node.arguments.length === 1 &&
+            node.arguments[0].name &&
+            node.arguments[0].name.startsWith("err")
+          ) {
+            context.report({
+              node,
+              message: 'Using console.error(err) on its own is not allowed. Either provide context to the error (console.error(msg, err)) or throw it.',
+            })
+          }
+        },
+      };
+    },
+  },
   "no-budibase-imports": {
     create: function (context) {
       return {

packages/account-portal

@@ -1 +1 @@
-Subproject commit eb7d5da233885c5cffd9c255d3e954d0cd39185e
+Subproject commit c167c331ff9b8161fc18e2ecbaaf1ea5815ba964

packages/backend-core/src/auth/auth.ts

@@ -64,7 +64,6 @@ async function refreshOIDCAccessToken(
     }
     strategy = await oidc.strategyFactory(enrichedConfig, ssoSaveUserNoOp)
   } catch (err) {
-    console.error(err)
     throw new Error("Could not refresh OAuth Token")
   }
 
@@ -99,7 +98,6 @@ async function refreshGoogleAccessToken(
       ssoSaveUserNoOp
     )
   } catch (err: any) {
-    console.error(err)
     throw new Error(
       `Error constructing OIDC refresh strategy: message=${err.message}`
     )

packages/backend-core/src/middleware/authenticated.ts

@@ -138,7 +138,6 @@ export default function (
         } catch (err: any) {
           authenticated = false
           console.error(`Auth Error: ${err.message}`)
-          console.error(err)
           // remove the cookie as the user does not exist anymore
           clearCookie(ctx, Cookie.Auth)
         }
@@ -187,7 +186,6 @@ export default function (
       }
     } catch (err: any) {
       console.error(`Auth Error: ${err.message}`)
-      console.error(err)
       // invalid token, clear the cookie
       if (err?.name === "JsonWebTokenError") {
         clearCookie(ctx, Cookie.Auth)

packages/backend-core/src/middleware/errorHandling.ts

@@ -12,7 +12,7 @@ export async function errorHandling(ctx: any, next: any) {
     if (status >= 400 && status < 500) {
       console.warn(err)
     } else {
-      console.error(err)
+      console.error("Got 400 response code", err)
     }
 
     let error: APIError = {

packages/backend-core/src/middleware/passport/sso/google.ts

@@ -68,7 +68,6 @@ export async function strategyFactory(
       verify
     )
   } catch (err: any) {
-    console.error(err)
     throw new Error(`Error constructing google authentication strategy: ${err}`)
   }
 }

packages/backend-core/src/middleware/passport/sso/oidc.ts

@@ -103,7 +103,6 @@ export async function strategyFactory(
     strategy.name = "oidc"
     return strategy
   } catch (err: any) {
-    console.error(err)
     throw new Error(`Error constructing OIDC authentication strategy - ${err}`)
   }
 }
@@ -142,7 +141,6 @@ export async function fetchStrategyConfig(
       callbackURL: callbackUrl,
     }
   } catch (err) {
-    console.error(err)
     throw new Error(
       `Error constructing OIDC authentication configuration - ${err}`
     )

packages/backend-core/src/migrations/migrations.ts

@@ -26,7 +26,6 @@ export const getMigrationsDoc = async (db: any) => {
     if (err.status && err.status === 404) {
       return { _id: DocumentType.MIGRATIONS }
     } else {
-      console.error(err)
       throw err
     }
   }

packages/server/specs/generate.ts

@@ -76,7 +76,7 @@ function writeFile(output: any, filename: string) {
     console.log(`Wrote spec to ${path}`)
     return path
   } catch (err) {
-    console.error(err)
+    console.error("Error writing spec file", err)
   }
 }
 

packages/server/src/integrations/oracle.ts

@@ -378,7 +378,7 @@ class OracleIntegration extends Sql implements DatasourcePlus {
         try {
           await connection.close()
         } catch (err) {
-          console.error(err)
+          console.error("Error connecting to Oracle", err)
         }
       }
     }

packages/server/src/utilities/fileSystem/filesystem.ts

@@ -43,7 +43,7 @@ export const checkDevelopmentEnvironment = () => {
     error = "Must run via yarn once to generate environment."
   }
   if (error) {
-    console.error(error)
+    console.error("Error during development environment check", error)
     process.exit(-1)
   }
 }

packages/worker/src/api/controllers/global/auth.ts

@@ -35,8 +35,7 @@ async function passportCallback(
   info: { message: string } | null = null
 ) {
   if (err) {
-    console.error("Authentication error")
+    console.error("Authentication error", err)
-    console.error(err)
     console.trace(err)
     return ctx.throw(403, info ? info : "Unauthorized")
   }