refactor: optimize handler flow and unify CORS origin check

api/contact/cors.ts

@@ -1,29 +1,30 @@
 import type { VercelRequest, VercelResponse } from "@vercel/node";
 
-export function isOriginAllowed(
-  origin: string | undefined, 
-  allowedOrigins: string[]
-): boolean {
-  if (allowedOrigins.length === 0) return false;
-  return origin !== undefined && allowedOrigins.includes(origin);
-}
 export function setCorsHeaders(
   req: VercelRequest, 
   res: VercelResponse,
   allowedOrigins: string[]
-): boolean {
+): "preflight" | "forbidden" | "ok" {
   res.setHeader("X-Content-Type-Options", "nosniff");
   const origin = req.headers["origin"];
-
-  if (allowedOrigins.length > 0 && allowedOrigins.includes(origin ?? "")) {
-    res.setHeader("Access-Control-Allow-Origin", origin!);
-    res.setHeader("Access-Control-Allow-Methods", "POST, OPTIONS");
-    res.setHeader("Access-Control-Allow-Headers", "Content-Type");
+  const isAllowed = origin && allowedOrigins.includes(origin);
+
+  if (!isAllowed) {
+    if (req.method === "OPTIONS") {
+      res.status(403).end();
+      return "preflight";
+    }
+    return "forbidden";
   }
 
+  res.setHeader("Access-Control-Allow-Origin", origin);
+  res.setHeader("Access-Control-Allow-Methods", "POST, OPTIONS");
+  res.setHeader("Access-Control-Allow-Headers", "Content-Type");
+
   if (req.method === "OPTIONS") {
     res.status(204).end();
-    return true;
+    return "preflight";
   }
-  return false;
+
+  return "ok";
 }

api/contact/index.ts

@@ -1,24 +1,18 @@
 import type { VercelRequest, VercelResponse } from "@vercel/node";
 import { checkRateLimit } from "@vercel/firewall"
-import { setCorsHeaders, isOriginAllowed } from "./cors.js";
+import { setCorsHeaders } from "./cors.js";
 import { isValidBody } from "./validation.js";
 import { getEmailConfig, sendEmail } from "./email.js";
 import { config } from "./config.js";
 
 export default async (req: VercelRequest, res: VercelResponse): Promise<void> => {
-  if (setCorsHeaders(req, res, config.allowedOrigins)) return;
-
-  if (!isOriginAllowed(req.headers["origin"], config.allowedOrigins)) {
+  const cors = setCorsHeaders(req, res, config.allowedOrigins);
+  if (cors === "preflight") return;
+  if (cors === "forbidden") {
     res.status(403).json({ error: "Forbidden" });
     return;
   }
 
-  const { rateLimited } = await checkRateLimit("contact-form-limit");
-  if (rateLimited) {
-    res.status(429).json({ error: "Too many requests. Please try again later." });
-    return;
-  }
-
   if (req.method !== "POST") {
     res.status(405).json({ error: "Method not allowed" });
     return;
@@ -29,20 +23,26 @@ export default async (req: VercelRequest, res: VercelResponse): Promise<void> =>
     return;
   }
 
+  if(req.body["fax_number"]) {
+    console.warn("Honeypot triggered:", req.headers["x-forwarded-for"] ?? "unknown");
+    res.json({ success: true, message: "Message sent successfully" });
+    return;
+  }
+
   const emailConfig = getEmailConfig(config);
   if (!emailConfig) {
     res.status(503).json({ error: "Service temporarily unavailable" });
     return;
   }
 
-  if(req.body["fax_number"]) {
-    console.warn("Honeypot triggered:", req.headers["x-forwarded-for"] ?? "unknown");
-    res.json({ success: true, message: "Message sent successfully" });
+  if (!isValidBody(req.body)) {
+    res.status(400).json({ error: "Invalid or missing fields" });
     return;
   }
 
-  if (!isValidBody(req.body)) {
-    res.status(400).json({ error: "Invalid or missing fields" });
+  const { rateLimited } = await checkRateLimit("contact-form-limit");
+  if (rateLimited) {
+    res.status(429).json({ error: "Too many requests. Please try again later." });
     return;
   }
 

tests/contact/cors.test.ts

@@ -1,5 +1,5 @@
 import { describe, it, expect, vi } from "vitest";
-import { isOriginAllowed, setCorsHeaders } from "@/api/contact/cors.js";
+import { setCorsHeaders } from "@/api/contact/cors.js";
 import type { VercelRequest, VercelResponse } from "@vercel/node";
 
 const makeReq = (origin?: string, method = "POST") => ({
@@ -17,24 +17,6 @@ const makeRes = () => {
   } as unknown as VercelResponse;
 };
 
-describe("isOriginAllowed", () => {
-  it("should return false when allowedOrigins is empty", () => {
-    expect(isOriginAllowed("https://example.com", [])).toBe(false);
-  });
-  it("should return false when origin is undefined", () => {
-    expect(isOriginAllowed(undefined, ["https://example.com"])).toBe(false);
-  });
-  it("should return false when origin is not in the list", () => {
-    expect(isOriginAllowed("https://other.com", ["https://example.com"])).toBe(false);
-  });
-  it("should return true when origin is in the list", () => {
-    expect(isOriginAllowed("https://example.com", ["https://example.com"])).toBe(true);
-  });
-  it("should return true when origin is one of many in the list", () => {
-    expect(isOriginAllowed("https://b.com", ["https://a.com", "https://b.com"])).toBe(true);
-  });
-});
-
 describe("setCorsHeaders", () => {
   it("should always set X-Content-Type-Options", () => {
     const req = makeReq();
@@ -66,28 +48,40 @@ describe("setCorsHeaders", () => {
     expect(res.setHeader).not.toHaveBeenCalledWith("Access-Control-Allow-Origin", expect.anything());
   });
 
-  it("should return true and end response on OPTIONS", () => {
+  it("should return 'preflight' and 204 on OPTIONS from allowed origin", () => {
     const req = makeReq("https://example.com", "OPTIONS");
     const res = makeRes();
     const result = setCorsHeaders(req, res, ["https://example.com"]);
     expect(res.status).toHaveBeenCalledWith(204);
     expect(res.end).toHaveBeenCalled();
-    expect(result).toBe(true);
+    expect(result).toBe("preflight");
   });
 
-  it("should return true and end response on OPTIONS from a disallowed origin" , () => {
+  it("should return 'preflight' and 403 on OPTIONS from a disallowed origin" , () => {
     const req = makeReq("https://other.com", "OPTIONS");
     const res = makeRes();
     const result = setCorsHeaders(req, res, ["https://example.com"]);
-    expect(res.status).toHaveBeenCalledWith(204);
+    expect(res.status).toHaveBeenCalledWith(403);
     expect(res.setHeader).not.toHaveBeenCalledWith("Access-Control-Allow-Origin", expect.anything());
     expect(res.end).toHaveBeenCalled();
-    expect(result).toBe(true);
+    expect(result).toBe("preflight");
+  });
+
+  it("should return 'forbidden' on non-OPTIONS requests from disallowed origin", () => {
+    const req = makeReq("https://example.com", "POST");
+    const res = makeRes();
+    expect(setCorsHeaders(req, res, ["https://other.com"])).toBe("forbidden");
+  });
+
+  it("should return 'ok' on allowed POST request", () => {
+    const req = makeReq("https://example.com", "POST");
+    const res = makeRes();
+    expect(setCorsHeaders(req, res, ["https://example.com"])).toBe("ok");
   });
 
-  it("should return false on non-OPTIONS requests", () => {
+  it("should return 'forbidden' when allowedOrigins is empty", () => {
     const req = makeReq("https://example.com", "POST");
     const res = makeRes();
-    expect(setCorsHeaders(req, res, ["https://example.com"])).toBe(false);
+    expect(setCorsHeaders(req, res, [])).toBe("forbidden");
   });
 });

tests/contact/index.test.ts

@@ -2,13 +2,13 @@ import { vi, describe, it, expect, beforeEach } from "vitest";
 import type { VercelRequest, VercelResponse } from "@vercel/node";
 
 vi.mock("@vercel/firewall", () => ({ checkRateLimit: vi.fn() }));
-vi.mock("@/api/contact/cors.js", () => ({ setCorsHeaders: vi.fn(), isOriginAllowed: vi.fn() }));
+vi.mock("@/api/contact/cors.js", () => ({ setCorsHeaders: vi.fn() }));
 vi.mock("@/api/contact/validation.js", () => ({ isValidBody: vi.fn() }));
 vi.mock("@/api/contact/email.js", () => ({ getEmailConfig: vi.fn(), sendEmail: vi.fn() }));
 vi.mock("@/api/contact/config.js", () => ({ config: { allowedOrigins: ["https://example.com"] } }));
 
 import { checkRateLimit } from "@vercel/firewall";
-import { setCorsHeaders, isOriginAllowed } from "@/api/contact/cors.js";
+import { setCorsHeaders } from "@/api/contact/cors.js";
 import { isValidBody } from "@/api/contact/validation.js";
 import { getEmailConfig, sendEmail } from "@/api/contact/email.js";
 import handler from "@/api/contact/index.js";
@@ -32,33 +32,31 @@ const makeRes = (): VercelResponse => {
 describe("contact handler (index.ts)", () => {
   beforeEach(() => {
     vi.clearAllMocks();
-    vi.mocked(setCorsHeaders).mockReturnValue(false);
-    vi.mocked(isOriginAllowed).mockReturnValue(true);
+    vi.mocked(setCorsHeaders).mockReturnValue("ok");
     vi.mocked(checkRateLimit).mockResolvedValue({ rateLimited: false } as any);
-    vi.mocked(getEmailConfig).mockReturnValue({ client: {} as any, from: "from@test.com", to: "to@test.com" });
+    vi.mocked(getEmailConfig).mockReturnValue({ client: {} as any, from: "from@test.com", to: ["to@test.com"] });
     vi.mocked(isValidBody).mockReturnValue(true);
     vi.mocked(sendEmail).mockResolvedValue(undefined);
   });
 
-  it("returns early when setCorsHeaders returns true (OPTIONS preflight)", async () => {
-    vi.mocked(setCorsHeaders).mockReturnValue(true);
+  it("returns early when setCorsHeaders returns 'preflight'", async () => {
+    vi.mocked(setCorsHeaders).mockReturnValue("preflight");
     const req = makeReq({ method: "OPTIONS" });
     const res = makeRes();
     await handler(req, res);
     expect(res.status).not.toHaveBeenCalled();
     expect(sendEmail).not.toHaveBeenCalled();
   });
 
-  it("returns 403 when origin is not allowed", async () => {
-    vi.mocked(isOriginAllowed).mockReturnValue(false);
+  it("returns 403 when setCorsHeaders returns 'forbidden'", async () => {
+    vi.mocked(setCorsHeaders).mockReturnValue("forbidden");
     const res = makeRes();
     await handler(makeReq(), res);
     expect(res.status).toHaveBeenCalledWith(403);
     expect(res.json).toHaveBeenCalledWith({ error: "Forbidden" });
   });
 
   it("returns 429 when rate limited", async () => {
-    vi.mocked(isOriginAllowed).mockReturnValue(true);
     vi.mocked(checkRateLimit).mockResolvedValue({ rateLimited: true } as any);
     const res = makeRes();
     await handler(makeReq(), res);