Merge pull request #424 from AikidoSec/patch-set-user

Log warning only once
AikidoSec · Oct 21, 2024 · 7df515d · 7df515d
2 parents b42325d + 4420979
commit 7df515d
Show file tree

Hide file tree

Showing 2 changed files with 78 additions and 51 deletions.
diff --git a/library/agent/context/user.test.ts b/library/agent/context/user.test.ts
@@ -1,13 +1,17 @@
 import * as t from "tap";
-import { type Context, getContext, runWithContext } from "../Context";
+import { wrap } from "../../helpers/wrap";
+import {
+  type Context,
+  getContext,
+  runWithContext,
+  updateContext,
+} from "../Context";
 import { LoggerForTesting } from "../logger/LoggerForTesting";
 import { setUser } from "./user";
 import { createTestAgent } from "../../helpers/createTestAgent";
 
-createTestAgent();
-
-t.test("it does not set user if empty id", async (t) => {
-  const context: Context = {
+function createContext(): Context {
+  return {
     remoteAddress: "::1",
     method: "POST",
     url: "http://localhost:4000",
@@ -21,6 +25,14 @@ t.test("it does not set user if empty id", async (t) => {
     source: "express",
     route: "/posts/:id",
   };
+}
+
+t.beforeEach(() => {
+  createTestAgent();
+});
+
+t.test("it does not set user if empty id", async (t) => {
+  const context = createContext();
 
   runWithContext(context, () => {
     setUser({ id: "" });
@@ -33,20 +45,7 @@ t.test("it does not set user if not inside context", async () => {
 });
 
 t.test("it sets user", async (t) => {
-  const context: Context = {
-    remoteAddress: "::1",
-    method: "POST",
-    url: "http://localhost:4000",
-    query: {},
-    headers: {},
-    body: {
-      myTitle: `-- should be blocked`,
-    },
-    cookies: {},
-    routeParams: {},
-    source: "express",
-    route: "/posts/:id",
-  };
+  const context = createContext();
 
   runWithContext(context, () => {
     setUser({ id: "id" });
@@ -57,20 +56,7 @@ t.test("it sets user", async (t) => {
 });
 
 t.test("it sets user with number as ID", async (t) => {
-  const context: Context = {
-    remoteAddress: "::1",
-    method: "POST",
-    url: "http://localhost:4000",
-    query: {},
-    headers: {},
-    body: {
-      myTitle: `-- should be blocked`,
-    },
-    cookies: {},
-    routeParams: {},
-    source: "express",
-    route: "/posts/:id",
-  };
+  const context = createContext();
 
   runWithContext(context, () => {
     setUser({ id: 1 });
@@ -81,20 +67,7 @@ t.test("it sets user with number as ID", async (t) => {
 });
 
 t.test("it sets user with name", async (t) => {
-  const context: Context = {
-    remoteAddress: "::1",
-    method: "POST",
-    url: "http://localhost:4000",
-    query: {},
-    headers: {},
-    body: {
-      myTitle: `-- should be blocked`,
-    },
-    cookies: {},
-    routeParams: {},
-    source: "express",
-    route: "/posts/:id",
-  };
+  const context = createContext();
 
   runWithContext(context, () => {
     setUser({ id: "id", name: "name" });
@@ -139,3 +112,45 @@ t.test("it logs when setUser has invalid input", async () => {
   ]);
   logger.clear();
 });
+
+t.test(
+  "it does not log warning when setUser is called before middleware",
+  async () => {
+    const logs: string[] = [];
+    wrap(console, "warn", function warn() {
+      return function warn(message: string) {
+        logs.push(message);
+      };
+    });
+
+    const context = createContext();
+    runWithContext(context, () => {
+      setUser({ id: "id" });
+    });
+
+    t.same(logs, []);
+  }
+);
+
+t.test(
+  "it logs warning when setUser is called after middleware (once)",
+  async () => {
+    const logs: string[] = [];
+    wrap(console, "warn", function warn() {
+      return function warn(message: string) {
+        logs.push(message);
+      };
+    });
+
+    const context = createContext();
+    runWithContext(context, () => {
+      updateContext(getContext()!, "executedMiddleware", true);
+      setUser({ id: "id" });
+      setUser({ id: "id" });
+    });
+
+    t.same(logs, [
+      "setUser(...) must be called before the Zen middleware is executed.",
+    ]);
+  }
+);
diff --git a/library/agent/context/user.ts b/library/agent/context/user.ts
@@ -49,10 +49,7 @@ export function setUser(user: unknown) {
   }
 
   if (context.executedMiddleware) {
-    // eslint-disable-next-line no-console
-    console.warn(
-      `setUser(...) must be called before the Zen middleware is executed.`
-    );
+    logWarningSetUserCalledAfterMiddleware();
   }
 
   context.user = validatedUser;
@@ -65,3 +62,18 @@ export function setUser(user: unknown) {
     lastIpAddress: ipAddress,
   });
 }
+
+let loggedWarningSetUserCalledAfterMiddleware = false;
+
+function logWarningSetUserCalledAfterMiddleware() {
+  if (loggedWarningSetUserCalledAfterMiddleware) {
+    return;
+  }
+
+  // eslint-disable-next-line no-console
+  console.warn(
+    `setUser(...) must be called before the Zen middleware is executed.`
+  );
+
+  loggedWarningSetUserCalledAfterMiddleware = true;
+}