mirror/formbricks
1
0
Fork 0
mirror of https://github.com/formbricks/formbricks.git synced 2026-03-17 09:51:14 -05:00
Code Issues Packages Projects Releases 138 Wiki Activity

fix: derive organizationId from resources in server actions to prevent cross-org IDOR (#7409)

Browse Source 
Co-authored-by: Dhruwang <dhruwangjariwala18@gmail.com>
This commit is contained in:
Santosh
2026-03-17 06:36:58 +01:00
committed by GitHub
parent a51a006c26
commit c7faa29437
13 changed files with 97 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/actions.ts
View File

@@ -64,15 +64,17 @@ export const sendEmbedSurveyPreviewEmailAction = authenticatedActionClient
const ZResetSurveyAction = z.object({
surveyId: ZId,
organizationId: ZId,
projectId: ZId,
});
export const resetSurveyAction = authenticatedActionClient.inputSchema(ZResetSurveyAction).action(
withAuditLogging("updated", "survey", async ({ ctx, parsedInput }) => {
const organizationId = await getOrganizationIdFromSurveyId(parsedInput.surveyId);
const projectId = await getProjectIdFromSurveyId(parsedInput.surveyId);
await checkAuthorizationUpdated({
userId: ctx.user.id,
organizationId: parsedInput.organizationId,
organizationId,
access: [
{
type: "organization",
@@ -81,12 +83,12 @@ export const resetSurveyAction = authenticatedActionClient.inputSchema(ZResetSur
{
type: "projectTeam",
minPermission: "readWrite",
projectId: parsedInput.projectId,
projectId,
},
],
});
ctx.auditLoggingCtx.organizationId = parsedInput.organizationId;
ctx.auditLoggingCtx.organizationId = organizationId;
ctx.auditLoggingCtx.surveyId = parsedInput.surveyId;
ctx.auditLoggingCtx.oldObject = null;

3
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/SurveyAnalysisCTA.tsx
View File

@@ -64,7 +64,7 @@ export const SurveyAnalysisCTA = ({
const [isResetModalOpen, setIsResetModalOpen] = useState(false);
const [isResetting, setIsResetting] = useState(false);
const { organizationId, project } = useEnvironment();
const { project } = useEnvironment();
const { refreshSingleUseId } = useSingleUseId(survey, isReadOnly);
const appSetupCompleted = survey.type === "app" && environment.appSetupCompleted;
@@ -128,7 +128,6 @@ export const SurveyAnalysisCTA = ({
setIsResetting(true);
const result = await resetSurveyAction({
surveyId: survey.id,
organizationId: organizationId,
projectId: project.id,
});
if (result?.data) {

68
apps/web/modules/api/v2/lib/tests/utils.test.ts
View File

@@ -217,7 +217,7 @@ describe("utils", () => {
});
describe("logApiError", () => {
test("logs API error details", () => {
test("logs API error details with method and path", () => {
// Mock the withContext method and its returned error method
const errorMock = vi.fn();
const withContextMock = vi.fn().mockReturnValue({
@@ -228,7 +228,7 @@ describe("utils", () => {
const originalWithContext = logger.withContext;
logger.withContext = withContextMock;
const mockRequest = new Request("http://localhost/api/test");
const mockRequest = new Request("http://localhost/api/v2/management/surveys", { method: "POST" });
mockRequest.headers.set("x-request-id", "123");
const error: ApiErrorResponseV2 = {
@@ -238,9 +238,11 @@ describe("utils", () => {
logApiError(mockRequest, error);
// Verify withContext was called with the expected context
// Verify withContext was called with the expected context including method and path
expect(withContextMock).toHaveBeenCalledWith({
correlationId: "123",
method: "POST",
path: "/api/v2/management/surveys",
error,
});
@@ -275,6 +277,8 @@ describe("utils", () => {
// Verify withContext was called with the expected context
expect(withContextMock).toHaveBeenCalledWith({
correlationId: "",
method: "GET",
path: "/api/test",
error,
});
@@ -285,7 +289,7 @@ describe("utils", () => {
logger.withContext = originalWithContext;
});
test("log API error details with SENTRY_DSN set", () => {
test("log API error details with SENTRY_DSN set includes method and path tags", () => {
// Mock the withContext method and its returned error method
const errorMock = vi.fn();
const withContextMock = vi.fn().mockReturnValue({
@@ -295,11 +299,23 @@ describe("utils", () => {
// Mock Sentry's captureException method
vi.mocked(Sentry.captureException).mockImplementation((() => {}) as any);
// Capture the scope mock for tag verification
const scopeSetTagMock = vi.fn();
vi.mocked(Sentry.withScope).mockImplementation((callback: (scope: any) => void) => {
const mockScope = {
setTag: scopeSetTagMock,
setContext: vi.fn(),
setLevel: vi.fn(),
setExtra: vi.fn(),
};
callback(mockScope);
});
// Replace the original withContext with our mock
const originalWithContext = logger.withContext;
logger.withContext = withContextMock;
const mockRequest = new Request("http://localhost/api/test");
const mockRequest = new Request("http://localhost/api/v2/management/surveys", { method: "DELETE" });
mockRequest.headers.set("x-request-id", "123");
const error: ApiErrorResponseV2 = {
@@ -309,20 +325,60 @@ describe("utils", () => {
logApiError(mockRequest, error);
// Verify withContext was called with the expected context
// Verify withContext was called with the expected context including method and path
expect(withContextMock).toHaveBeenCalledWith({
correlationId: "123",
method: "DELETE",
path: "/api/v2/management/surveys",
error,
});
// Verify error was called on the child logger
expect(errorMock).toHaveBeenCalledWith("API V2 Error Details");
// Verify Sentry scope tags include method and path
expect(scopeSetTagMock).toHaveBeenCalledWith("correlationId", "123");
expect(scopeSetTagMock).toHaveBeenCalledWith("method", "DELETE");
expect(scopeSetTagMock).toHaveBeenCalledWith("path", "/api/v2/management/surveys");
// Verify Sentry.captureException was called
expect(Sentry.captureException).toHaveBeenCalled();
// Restore the original method
logger.withContext = originalWithContext;
});
test("does not send to Sentry for non-internal_server_error types", () => {
// Mock the withContext method and its returned error method
const errorMock = vi.fn();
const withContextMock = vi.fn().mockReturnValue({
error: errorMock,
});
vi.mocked(Sentry.captureException).mockClear();
// Replace the original withContext with our mock
const originalWithContext = logger.withContext;
logger.withContext = withContextMock;
const mockRequest = new Request("http://localhost/api/v2/management/surveys");
mockRequest.headers.set("x-request-id", "456");
const error: ApiErrorResponseV2 = {
type: "not_found",
details: [{ field: "survey", issue: "not found" }],
};
logApiError(mockRequest, error);
// Verify Sentry.captureException was NOT called for non-500 errors
expect(Sentry.captureException).not.toHaveBeenCalled();
// But structured logging should still happen
expect(errorMock).toHaveBeenCalledWith("API V2 Error Details");
// Restore the original method
logger.withContext = originalWithContext;
});
});
});

9
apps/web/modules/api/v2/lib/utils-edge.ts
View File

@@ -6,13 +6,18 @@ import { ApiErrorResponseV2 } from "@/modules/api/v2/types/api-error";
export const logApiErrorEdge = (request: Request, error: ApiErrorResponseV2): void => {
const correlationId = request.headers.get("x-request-id") ?? "";
const method = request.method;
const url = new URL(request.url);
const path = url.pathname;
// Send the error to Sentry if the DSN is set and the error type is internal_server_error
// This is useful for tracking down issues without overloading Sentry with errors
if (SENTRY_DSN && IS_PRODUCTION && error.type === "internal_server_error") {
// Use Sentry scope to add correlation ID as a tag for easy filtering
// Use Sentry scope to add correlation ID and request context as tags for easy filtering
Sentry.withScope((scope) => {
scope.setTag("correlationId", correlationId);
scope.setTag("method", method);
scope.setTag("path", path);
scope.setLevel("error");
scope.setExtra("originalError", error);
@@ -24,6 +29,8 @@ export const logApiErrorEdge = (request: Request, error: ApiErrorResponseV2): vo
logger
.withContext({
correlationId,
method,
path,
error,
})
.error("API V2 Error Details");

3
apps/web/modules/ee/contacts/segments/actions.ts
View File

@@ -97,14 +97,13 @@ export const createSegmentAction = authenticatedActionClient.inputSchema(ZSegmen
);
const ZUpdateSegmentAction = z.object({
environmentId: ZId,
segmentId: ZId,
data: ZSegmentUpdateInput,
});
export const updateSegmentAction = authenticatedActionClient.inputSchema(ZUpdateSegmentAction).action(
withAuditLogging("updated", "segment", async ({ ctx, parsedInput }) => {
const organizationId = await getOrganizationIdFromEnvironmentId(parsedInput.environmentId);
const organizationId = await getOrganizationIdFromSegmentId(parsedInput.segmentId);
await checkAuthorizationUpdated({
userId: ctx.user.id,
organizationId,

1
apps/web/modules/ee/contacts/segments/components/segment-settings.tsx
View File

@@ -75,7 +75,6 @@ export function SegmentSettings({
try {
setIsUpdatingSegment(true);
const data = await updateSegmentAction({
environmentId,
segmentId: segment.id,
data: {
title: segment.title,

4
apps/web/modules/ee/contacts/segments/components/targeting-card.tsx
View File

@@ -124,7 +124,7 @@ export function TargetingCard({
};
const handleSaveAsNewSegmentUpdate = async (segmentId: string, data: TSegmentUpdateInput) => {
const updatedSegment = await updateSegmentAction({ segmentId, environmentId, data });
const updatedSegment = await updateSegmentAction({ segmentId, data });
return updatedSegment?.data as TSegment;
};
@@ -136,7 +136,7 @@ export function TargetingCard({
const handleSaveSegment = async (data: TSegmentUpdateInput) => {
try {
if (!segment) throw new Error(t("environments.segments.invalid_segment"));
const result = await updateSegmentAction({ segmentId: segment.id, environmentId, data });
const result = await updateSegmentAction({ segmentId: segment.id, data });
if (result?.serverError) {
toast.error(getFormattedErrorMessage(result));
return;

9
apps/web/modules/ee/quotas/actions.ts
View File

@@ -21,7 +21,6 @@ import { getOrganizationBilling } from "@/modules/survey/lib/survey";
const ZDeleteQuotaAction = z.object({
quotaId: ZId,
surveyId: ZId,
});
const checkQuotasEnabled = async (organizationId: string) => {
@@ -37,7 +36,7 @@ const checkQuotasEnabled = async (organizationId: string) => {
export const deleteQuotaAction = authenticatedActionClient.inputSchema(ZDeleteQuotaAction).action(
withAuditLogging("deleted", "quota", async ({ ctx, parsedInput }) => {
const organizationId = await getOrganizationIdFromSurveyId(parsedInput.surveyId);
const organizationId = await getOrganizationIdFromQuotaId(parsedInput.quotaId);
await checkQuotasEnabled(organizationId);
await checkAuthorizationUpdated({
userId: ctx.user.id,
@@ -49,7 +48,7 @@ export const deleteQuotaAction = authenticatedActionClient.inputSchema(ZDeleteQu
},
{
type: "projectTeam",
projectId: await getProjectIdFromSurveyId(parsedInput.surveyId),
projectId: await getProjectIdFromQuotaId(parsedInput.quotaId),
minPermission: "readWrite",
},
],
@@ -72,7 +71,7 @@ const ZUpdateQuotaAction = z.object({
export const updateQuotaAction = authenticatedActionClient.inputSchema(ZUpdateQuotaAction).action(
withAuditLogging("updated", "quota", async ({ ctx, parsedInput }) => {
const organizationId = await getOrganizationIdFromSurveyId(parsedInput.quota.surveyId);
const organizationId = await getOrganizationIdFromQuotaId(parsedInput.quotaId);
await checkQuotasEnabled(organizationId);
await checkAuthorizationUpdated({
userId: ctx.user.id,
@@ -84,7 +83,7 @@ export const updateQuotaAction = authenticatedActionClient.inputSchema(ZUpdateQu
},
{
type: "projectTeam",
projectId: await getProjectIdFromSurveyId(parsedInput.quota.surveyId),
projectId: await getProjectIdFromQuotaId(parsedInput.quotaId),
minPermission: "readWrite",
},
],

1
apps/web/modules/ee/quotas/components/quotas-card.tsx
View File

@@ -85,7 +85,6 @@ export const QuotasCard = ({
setIsDeletingQuota(true);
const deleteQuotaActionResult = await deleteQuotaAction({
quotaId: quotaId,
surveyId: localSurvey.id,
});
if (deleteQuotaActionResult?.data) {
toast.success(t("environments.surveys.edit.quotas.quota_deleted_successfull_toast"));

15
apps/web/modules/ee/role-management/actions.ts
View File

@@ -10,6 +10,7 @@ import { getUserManagementAccess } from "@/lib/membership/utils";
import { getOrganization } from "@/lib/organization/service";
import { authenticatedActionClient } from "@/lib/utils/action-client";
import { checkAuthorizationUpdated } from "@/lib/utils/action-client/action-client-middleware";
import { getOrganizationIdFromInviteId } from "@/lib/utils/helper";
import { withAuditLogging } from "@/modules/ee/audit-logs/lib/handler";
import { getAccessControlPermission } from "@/modules/ee/license-check/lib/utils";
import { updateInvite } from "@/modules/ee/role-management/lib/invite";
@@ -31,7 +32,6 @@ export const checkRoleManagementPermission = async (organizationId: string) => {
const ZUpdateInviteAction = z.object({
inviteId: ZUuid,
organizationId: ZId,
data: ZInviteUpdateInput,
});
@@ -39,17 +39,16 @@ export type TUpdateInviteAction = z.infer<typeof ZUpdateInviteAction>;
export const updateInviteAction = authenticatedActionClient.inputSchema(ZUpdateInviteAction).action(
withAuditLogging("updated", "invite", async ({ ctx, parsedInput }) => {
const currentUserMembership = await getMembershipByUserIdOrganizationId(
ctx.user.id,
parsedInput.organizationId
);
const organizationId = await getOrganizationIdFromInviteId(parsedInput.inviteId);
const currentUserMembership = await getMembershipByUserIdOrganizationId(ctx.user.id, organizationId);
if (!currentUserMembership) {
throw new AuthenticationError("User not a member of this organization");
}
await checkAuthorizationUpdated({
userId: ctx.user.id,
organizationId: parsedInput.organizationId,
organizationId,
access: [
{
data: parsedInput.data,
@@ -68,9 +67,9 @@ export const updateInviteAction = authenticatedActionClient.inputSchema(ZUpdateI
throw new OperationNotAllowedError("Managers can only invite members");
}
await checkRoleManagementPermission(parsedInput.organizationId);
await checkRoleManagementPermission(organizationId);
ctx.auditLoggingCtx.organizationId = parsedInput.organizationId;
ctx.auditLoggingCtx.organizationId = organizationId;
ctx.auditLoggingCtx.inviteId = parsedInput.inviteId;
ctx.auditLoggingCtx.oldObject = { ...(await getInvite(parsedInput.inviteId)) };

2
apps/web/modules/ee/role-management/components/edit-membership-role.tsx
View File

@@ -65,7 +65,7 @@ export function EditMembershipRole({
}
if (inviteId) {
await updateInviteAction({ inviteId: inviteId, organizationId, data: { role } });
await updateInviteAction({ inviteId: inviteId, data: { role } });
}
} catch (error) {
toast.error(t("common.something_went_wrong_please_try_again"));

7
apps/web/modules/organization/settings/teams/actions.ts
View File

@@ -27,14 +27,15 @@ import { deleteInvite, getInvite, inviteUser, refreshInviteExpiration, resendInv
const ZDeleteInviteAction = z.object({
inviteId: ZUuid,
organizationId: ZId,
});
export const deleteInviteAction = authenticatedActionClient.inputSchema(ZDeleteInviteAction).action(
withAuditLogging("deleted", "invite", async ({ ctx, parsedInput }) => {
const organizationId = await getOrganizationIdFromInviteId(parsedInput.inviteId);
await checkAuthorizationUpdated({
userId: ctx.user.id,
organizationId: parsedInput.organizationId,
organizationId,
access: [
{
type: "organization",
@@ -42,7 +43,7 @@ export const deleteInviteAction = authenticatedActionClient.inputSchema(ZDeleteI
},
],
});
ctx.auditLoggingCtx.organizationId = parsedInput.organizationId;
ctx.auditLoggingCtx.organizationId = organizationId;
ctx.auditLoggingCtx.inviteId = parsedInput.inviteId;
ctx.auditLoggingCtx.oldObject = { ...(await getInvite(parsedInput.inviteId)) };
return await deleteInvite(parsedInput.inviteId);

2
apps/web/modules/organization/settings/teams/components/edit-memberships/member-actions.tsx
View File

@@ -41,7 +41,7 @@ export const MemberActions = ({ organization, member, invite, showDeleteButton }
if (!member && invite) {
// This is an invite
const result = await deleteInviteAction({ inviteId: invite?.id, organizationId: organization.id });
const result = await deleteInviteAction({ inviteId: invite?.id });
if (result?.serverError) {
toast.error(getFormattedErrorMessage(result));
setIsDeleting(false);