fix: auth checks in storage management api (#5931)

apps/web/app/api/v1/management/storage/lib/utils.test.ts

@@ -0,0 +1,204 @@
+import { checkForRequiredFields } from "./utils";
+import { describe, test, expect } from "vitest";
+import { responses } from "@/app/lib/api/response";
+import { hasUserEnvironmentAccess } from "@/lib/environment/auth";
+import { hasPermission } from "@/modules/organization/settings/api-keys/lib/utils";
+import { NextRequest } from "next/server";
+import { Session } from "next-auth";
+import { vi } from "vitest";
+import { TAuthenticationApiKey } from "@formbricks/types/auth";
+import { authenticateRequest } from "@/app/api/v1/auth";
+import { checkAuth } from "./utils";
+
+// Create mock response objects
+const mockBadRequestResponse = new Response("Bad Request", { status: 400 });
+const mockNotAuthenticatedResponse = new Response("Not authenticated", { status: 401 });
+const mockUnauthorizedResponse = new Response("Unauthorized", { status: 401 });
+
+vi.mock("@/app/api/v1/auth", () => ({
+    authenticateRequest: vi.fn(),
+}));
+
+vi.mock("@/lib/environment/auth", () => ({
+    hasUserEnvironmentAccess: vi.fn(),
+}));
+
+vi.mock("@/modules/organization/settings/api-keys/lib/utils", () => ({
+    hasPermission: vi.fn(),
+}));
+
+vi.mock("@/app/lib/api/response", () => ({
+    responses: {
+        badRequestResponse: vi.fn(() => mockBadRequestResponse),
+        notAuthenticatedResponse: vi.fn(() => mockNotAuthenticatedResponse),
+        unauthorizedResponse: vi.fn(() => mockUnauthorizedResponse),
+    },
+}));
+
+describe("checkForRequiredFields", () => {
+    test("should return undefined when all required fields are present", () => {
+        const result = checkForRequiredFields("env-123", "image/png", "test-file.png");
+        expect(result).toBeUndefined();
+    });
+
+    test("should return bad request response when environmentId is missing", () => {
+        const result = checkForRequiredFields("", "image/png", "test-file.png");
+        expect(responses.badRequestResponse).toHaveBeenCalledWith("environmentId is required");
+        expect(result).toBe(mockBadRequestResponse);
+    });
+
+    test("should return bad request response when fileType is missing", () => {
+        const result = checkForRequiredFields("env-123", "", "test-file.png");
+        expect(responses.badRequestResponse).toHaveBeenCalledWith("contentType is required");
+        expect(result).toBe(mockBadRequestResponse);
+    });
+
+    test("should return bad request response when encodedFileName is missing", () => {
+        const result = checkForRequiredFields("env-123", "image/png", "");
+        expect(responses.badRequestResponse).toHaveBeenCalledWith("fileName is required");
+        expect(result).toBe(mockBadRequestResponse);
+    });
+
+    test("should return bad request response when environmentId is undefined", () => {
+        const result = checkForRequiredFields(undefined as any, "image/png", "test-file.png");
+        expect(responses.badRequestResponse).toHaveBeenCalledWith("environmentId is required");
+        expect(result).toBe(mockBadRequestResponse);
+    });
+
+    test("should return bad request response when fileType is undefined", () => {
+        const result = checkForRequiredFields("env-123", undefined as any, "test-file.png");
+        expect(responses.badRequestResponse).toHaveBeenCalledWith("contentType is required");
+        expect(result).toBe(mockBadRequestResponse);
+    });
+
+    test("should return bad request response when encodedFileName is undefined", () => {
+        const result = checkForRequiredFields("env-123", "image/png", undefined as any);
+        expect(responses.badRequestResponse).toHaveBeenCalledWith("fileName is required");
+        expect(result).toBe(mockBadRequestResponse);
+    });
+});
+
+describe("checkAuth", () => {
+    const environmentId = "env-123";
+    const mockRequest = new NextRequest("http://localhost:3000/api/test");
+
+    test("returns notAuthenticatedResponse when no session and no authentication", async () => {
+        vi.mocked(authenticateRequest).mockResolvedValue(null);
+
+        const result = await checkAuth(null, environmentId, mockRequest);
+
+        expect(authenticateRequest).toHaveBeenCalledWith(mockRequest);
+        expect(responses.notAuthenticatedResponse).toHaveBeenCalled();
+        expect(result).toBe(mockNotAuthenticatedResponse);
+    });
+
+    test("returns unauthorizedResponse when no session and authentication lacks POST permission", async () => {
+        const mockAuthentication: TAuthenticationApiKey = {
+            type: "apiKey",
+            environmentPermissions: [
+                {
+                    environmentId: "env-123",
+                    permission: "read",
+                    environmentType: "development",
+                    projectId: "project-1",
+                    projectName: "Project 1",
+                },
+            ],
+            hashedApiKey: "hashed-key",
+            apiKeyId: "api-key-id",
+            organizationId: "org-id",
+            organizationAccess: {
+                accessControl: {},
+            },
+        };
+
+        vi.mocked(authenticateRequest).mockResolvedValue(mockAuthentication);
+        vi.mocked(hasPermission).mockReturnValue(false);
+
+        const result = await checkAuth(null, environmentId, mockRequest);
+
+        expect(authenticateRequest).toHaveBeenCalledWith(mockRequest);
+        expect(hasPermission).toHaveBeenCalledWith(mockAuthentication.environmentPermissions, environmentId, "POST");
+        expect(responses.unauthorizedResponse).toHaveBeenCalled();
+        expect(result).toBe(mockUnauthorizedResponse);
+    });
+
+    test("returns undefined when no session and authentication has POST permission", async () => {
+        const mockAuthentication: TAuthenticationApiKey = {
+            type: "apiKey",
+            environmentPermissions: [
+                {
+                    environmentId: "env-123",
+                    permission: "write",
+                    environmentType: "development",
+                    projectId: "project-1",
+                    projectName: "Project 1",
+                },
+            ],
+            hashedApiKey: "hashed-key",
+            apiKeyId: "api-key-id",
+            organizationId: "org-id",
+            organizationAccess: {
+                accessControl: {},
+            },
+        };
+
+        vi.mocked(authenticateRequest).mockResolvedValue(mockAuthentication);
+        vi.mocked(hasPermission).mockReturnValue(true);
+
+        const result = await checkAuth(null, environmentId, mockRequest);
+
+        expect(authenticateRequest).toHaveBeenCalledWith(mockRequest);
+        expect(hasPermission).toHaveBeenCalledWith(mockAuthentication.environmentPermissions, environmentId, "POST");
+        expect(result).toBeUndefined();
+    });
+
+    test("returns unauthorizedResponse when session exists but user lacks environment access", async () => {
+        const mockSession: Session = {
+            user: {
+                id: "user-123",
+            },
+            expires: "2024-12-31T23:59:59.999Z",
+        };
+
+        vi.mocked(hasUserEnvironmentAccess).mockResolvedValue(false);
+
+        const result = await checkAuth(mockSession, environmentId, mockRequest);
+
+        expect(hasUserEnvironmentAccess).toHaveBeenCalledWith("user-123", environmentId);
+        expect(responses.unauthorizedResponse).toHaveBeenCalled();
+        expect(result).toBe(mockUnauthorizedResponse);
+    });
+
+    test("returns undefined when session exists and user has environment access", async () => {
+        const mockSession: Session = {
+            user: {
+                id: "user-123",
+            },
+            expires: "2024-12-31T23:59:59.999Z",
+        };
+
+        vi.mocked(hasUserEnvironmentAccess).mockResolvedValue(true);
+
+        const result = await checkAuth(mockSession, environmentId, mockRequest);
+
+        expect(hasUserEnvironmentAccess).toHaveBeenCalledWith("user-123", environmentId);
+        expect(result).toBeUndefined();
+    });
+
+    test("does not call authenticateRequest when session exists", async () => {
+        const mockSession: Session = {
+            user: {
+                id: "user-123",
+            },
+            expires: "2024-12-31T23:59:59.999Z",
+        };
+
+        vi.mocked(hasUserEnvironmentAccess).mockResolvedValue(true);
+
+        await checkAuth(mockSession, environmentId, mockRequest);
+
+        expect(authenticateRequest).not.toHaveBeenCalled();
+        expect(hasUserEnvironmentAccess).toHaveBeenCalledWith("user-123", environmentId);
+    });
+}); 

apps/web/app/api/v1/management/storage/lib/utils.ts

@@ -0,0 +1,38 @@
+import { authenticateRequest } from "@/app/api/v1/auth";
+import { responses } from "@/app/lib/api/response";
+import { hasUserEnvironmentAccess } from "@/lib/environment/auth";
+import { NextRequest } from "next/server";
+import { Session } from "next-auth";
+import { hasPermission } from "@/modules/organization/settings/api-keys/lib/utils";
+
+
+export const checkForRequiredFields = (environmentId: string, fileType: string, encodedFileName: string): Response | undefined => {
+    if (!environmentId) {
+        return responses.badRequestResponse("environmentId is required");
+    }
+
+    if (!fileType) {
+        return responses.badRequestResponse("contentType is required");
+    }
+
+    if (!encodedFileName) {
+        return responses.badRequestResponse("fileName is required");
+    }
+};
+
+export const checkAuth = async (session: Session | null, environmentId: string, request: NextRequest) => {
+    if (!session) {
+        //check whether its using API key
+        const authentication = await authenticateRequest(request);
+        if (!authentication) return responses.notAuthenticatedResponse();
+
+        if (!hasPermission(authentication.environmentPermissions, environmentId, "POST")) {
+            return responses.unauthorizedResponse();
+        }
+    } else {
+        const isUserAuthorized = await hasUserEnvironmentAccess(session.user.id, environmentId);
+        if (!isUserAuthorized) {
+            return responses.unauthorizedResponse();
+        }
+    }
+};

apps/web/app/api/v1/management/storage/local/route.ts

@@ -4,13 +4,13 @@
 import { responses } from "@/app/lib/api/response";
 import { ENCRYPTION_KEY, UPLOADS_DIR } from "@/lib/constants";
 import { validateLocalSignedUrl } from "@/lib/crypto";
-import { hasUserEnvironmentAccess } from "@/lib/environment/auth";
 import { validateFile } from "@/lib/fileValidation";
 import { putFileToLocalStorage } from "@/lib/storage/service";
 import { authOptions } from "@/modules/auth/lib/authOptions";
 import { getServerSession } from "next-auth";
 import { NextRequest } from "next/server";
 import { logger } from "@formbricks/logger";
+import { checkAuth, checkForRequiredFields } from "@/app/api/v1/management/storage/lib/utils";
 
 export const POST = async (req: NextRequest): Promise<Response> => {
   if (!ENCRYPTION_KEY) {
@@ -27,41 +27,17 @@ export const POST = async (req: NextRequest): Promise<Response> => {
   const signedTimestamp = jsonInput.timestamp as string;
   const environmentId = jsonInput.environmentId as string;
 
-  if (!environmentId) {
-    return responses.badRequestResponse("environmentId is required");
-  }
+  const requiredFieldResponse = checkForRequiredFields(environmentId, fileType, encodedFileName);
+  if (requiredFieldResponse) return requiredFieldResponse;
 
-  if (!fileType) {
-    return responses.badRequestResponse("contentType is required");
-  }
-
-  if (!encodedFileName) {
-    return responses.badRequestResponse("fileName is required");
-  }
-
-  if (!signedSignature) {
-    return responses.unauthorizedResponse();
-  }
-
-  if (!signedUuid) {
-    return responses.unauthorizedResponse();
-  }
-
-  if (!signedTimestamp) {
+  if (!signedSignature || !signedUuid || !signedTimestamp) {
     return responses.unauthorizedResponse();
   }
 
   const session = await getServerSession(authOptions);
 
-  if (!session || !session.user) {
-    return responses.notAuthenticatedResponse();
-  }
-
-  const isUserAuthorized = await hasUserEnvironmentAccess(session.user.id, environmentId);
-
-  if (!isUserAuthorized) {
-    return responses.unauthorizedResponse();
-  }
+  const authResponse = await checkAuth(session, environmentId, req);
+  if (authResponse) return authResponse;
 
   const fileName = decodeURIComponent(encodedFileName);
 

apps/web/app/api/v1/management/storage/route.ts

@@ -1,11 +1,12 @@
 import { responses } from "@/app/lib/api/response";
-import { hasUserEnvironmentAccess } from "@/lib/environment/auth";
 import { validateFile } from "@/lib/fileValidation";
 import { authOptions } from "@/modules/auth/lib/authOptions";
 import { getServerSession } from "next-auth";
 import { NextRequest } from "next/server";
 import { logger } from "@formbricks/logger";
 import { getSignedUrlForPublicFile } from "./lib/getSignedUrl";
+import { checkAuth, checkForRequiredFields } from "@/app/api/v1/management/storage/lib/utils";
+
 
 // api endpoint for uploading public files
 // uploaded files will be public, anyone can access the file
@@ -13,29 +14,26 @@ import { getSignedUrlForPublicFile } from "./lib/getSignedUrl";
 // use this to upload files for a specific resource, e.g. a user profile picture or a survey
 // this api endpoint will return a signed url for uploading the file to s3 and another url for uploading file to the local storage
 
-export const POST = async (req: NextRequest): Promise<Response> => {
+
+export const POST = async (request: NextRequest): Promise<Response> => {
   let storageInput;
 
   try {
-    storageInput = await req.json();
+    storageInput = await request.json();
   } catch (error) {
-    logger.error({ error, url: req.url }, "Error parsing JSON input");
+    logger.error({ error, url: request.url }, "Error parsing JSON input");
     return responses.badRequestResponse("Malformed JSON input, please check your request body");
   }
 
   const { fileName, fileType, environmentId, allowedFileExtensions } = storageInput;
 
-  if (!fileName) {
-    return responses.badRequestResponse("fileName is required");
-  }
+  const requiredFieldResponse = checkForRequiredFields(environmentId, fileType, fileName);
+  if (requiredFieldResponse) return requiredFieldResponse;
+  const session = await getServerSession(authOptions);
 
-  if (!fileType) {
-    return responses.badRequestResponse("fileType is required");
-  }
+  const authResponse = await checkAuth(session, environmentId, request);
+  if (authResponse) return authResponse;
 
-  if (!environmentId) {
-    return responses.badRequestResponse("environmentId is required");
-  }
 
   // Perform server-side file validation first to block dangerous file types
   const fileValidation = validateFile(fileName, fileType);
@@ -53,18 +51,5 @@ export const POST = async (req: NextRequest): Promise<Response> => {
     }
   }
 
-  // auth and upload private file
-  const session = await getServerSession(authOptions);
-
-  if (!session || !session.user) {
-    return responses.notAuthenticatedResponse();
-  }
-
-  const isUserAuthorized = await hasUserEnvironmentAccess(session.user.id, environmentId);
-
-  if (!isUserAuthorized) {
-    return responses.unauthorizedResponse();
-  }
-
   return await getSignedUrlForPublicFile(fileName, environmentId, fileType);
 };