Compare commits

..

3 Commits

Author SHA1 Message Date
Matti Nannt
d08c93dd6c fix: harden password reset token handling 2026-03-18 07:51:13 +01:00
Anshuman Pandey
f250bc7e88 fix: fixes race between setUserId and trigger (#7498)
Co-authored-by: Dhruwang <dhruwangjariwala18@gmail.com>
2026-03-17 08:57:07 +00:00
Santosh
c7faa29437 fix: derive organizationId from resources in server actions to prevent cross-org IDOR (#7409)
Co-authored-by: Dhruwang <dhruwangjariwala18@gmail.com>
2026-03-17 05:36:58 +00:00
25 changed files with 591 additions and 145 deletions

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;

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) {

View File

@@ -33,14 +33,14 @@ describe("Password Management", () => {
const hashedPassword = await hashPassword(password);
const isValid = await verifyPassword(password, hashedPassword);
expect(isValid).toBe(true);
}, 15000);
});
test("verifyPassword should reject an incorrect password", async () => {
const password = "testPassword123";
const hashedPassword = await hashPassword(password);
const isValid = await verifyPassword("wrongPassword", hashedPassword);
expect(isValid).toBe(false);
}, 15000);
});
});
describe("Organization Access", () => {

View File

@@ -34,7 +34,7 @@ describe("Crypto Utils", () => {
const isValid = await verifySecret(secret, hash);
expect(isValid).toBe(true);
}, 15000);
});
test("should reject wrong secrets", async () => {
const secret = "test-secret-123";
@@ -43,7 +43,7 @@ describe("Crypto Utils", () => {
const isValid = await verifySecret(wrongSecret, hash);
expect(isValid).toBe(false);
}, 15000);
});
test("should generate different hashes for the same secret (due to salt)", async () => {
const secret = "test-secret-123";
@@ -64,7 +64,7 @@ describe("Crypto Utils", () => {
// Verify the cost factor is in the hash
expect(hash).toMatch(/^\$2[aby]\$10\$/);
expect(await verifySecret(secret, hash)).toBe(true);
}, 15000);
});
test("should return false for invalid hash format", async () => {
const secret = "test-secret-123";

View File

@@ -1021,6 +1021,6 @@ describe("updateSurveyDraftAction", () => {
// Expect validation error (skipValidation = false)
await expect(updateSurveyInternal(incompleteSurvey, false)).rejects.toThrow();
}, 15000);
});
});
});

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;
});
});
});

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");

View File

@@ -2,6 +2,7 @@
import { zodResolver } from "@hookform/resolvers/zod";
import { useRouter, useSearchParams } from "next/navigation";
import { useEffect, useState } from "react";
import { SubmitHandler, useForm } from "react-hook-form";
import { toast } from "react-hot-toast";
import { useTranslation } from "react-i18next";
@@ -33,6 +34,7 @@ export const ResetPasswordForm = () => {
const { t } = useTranslation();
const searchParams = useSearchParams();
const router = useRouter();
const [token, setToken] = useState("");
const form = useForm<TPasswordResetForm>({
defaultValues: {
@@ -42,12 +44,24 @@ export const ResetPasswordForm = () => {
resolver: zodResolver(ZPasswordResetForm),
});
useEffect(() => {
const resetToken = searchParams?.get("token");
if (!resetToken) {
return;
}
setToken((currentToken) => currentToken || resetToken);
// Remove the token from the address bar after the page has loaded.
const sanitizedUrl = `${window.location.pathname}${window.location.hash}`;
window.history.replaceState(window.history.state, "", sanitizedUrl);
}, [searchParams]);
const handleSubmit: SubmitHandler<TPasswordResetForm> = async (data) => {
if (data.password !== data.confirmPassword) {
toast.error(t("auth.forgot-password.reset.passwords_do_not_match"));
return;
}
const token = searchParams?.get("token");
if (!token) {
toast.error(t("auth.forgot-password.reset.no_token_provided"));
return;
@@ -94,7 +108,7 @@ export const ResetPasswordForm = () => {
<div>
<Button
type="submit"
disabled={!form.formState.isValid}
disabled={!form.formState.isValid || !token}
className="w-full justify-center"
loading={form.formState.isSubmitting}>
{t("auth.forgot-password.reset_password")}

View File

@@ -159,12 +159,6 @@ describe("organization-billing", () => {
mocks.getCloudPlanFromProduct.mockReturnValue("pro");
mocks.subscriptionsList.mockResolvedValue({ data: [] });
mocks.customersList.mockResolvedValue({ data: [] });
mocks.customersRetrieve.mockResolvedValue({
id: "cus_1",
deleted: false,
invoice_settings: { default_payment_method: null },
default_source: null,
});
mocks.prismaMembershipFindFirst.mockResolvedValue(null);
mocks.productsList.mockResolvedValue({
data: [
@@ -645,64 +639,6 @@ describe("organization-billing", () => {
expect(mocks.cacheDel).toHaveBeenCalledWith(["billing-cache-key"]);
});
test("syncOrganizationBillingFromStripe marks migrated customers with customer-level payment methods", async () => {
mocks.prismaOrganizationBillingFindUnique.mockResolvedValue({
stripeCustomerId: "cus_1",
limits: {
projects: 3,
monthly: {
responses: 1500,
},
},
usageCycleAnchor: new Date(),
stripe: { lastSyncedEventId: null },
});
mocks.subscriptionsList.mockResolvedValue({
data: [
{
id: "sub_1",
status: "active",
default_payment_method: null,
billing_cycle_anchor: 1739923200,
items: {
data: [
{
price: {
metadata: {},
product: { id: "prod_pro", metadata: { formbricks_plan: "pro" } },
recurring: { usage_type: "licensed", interval: "month" },
},
},
],
},
},
],
});
mocks.customersRetrieve.mockResolvedValue({
id: "cus_1",
deleted: false,
invoice_settings: { default_payment_method: "pm_legacy_default" },
default_source: null,
});
mocks.entitlementsList.mockResolvedValue({
data: [],
has_more: false,
});
const result = await syncOrganizationBillingFromStripe("org_1");
expect(result?.stripe?.hasPaymentMethod).toBe(true);
expect(mocks.prismaOrganizationBillingUpdate).toHaveBeenCalledWith(
expect.objectContaining({
data: expect.objectContaining({
stripe: expect.objectContaining({
hasPaymentMethod: true,
}),
}),
})
);
});
test("createPaidPlanCheckoutSession rejects mixed-interval yearly checkout", async () => {
await expect(
createPaidPlanCheckoutSession({

View File

@@ -1107,21 +1107,6 @@ const resolvePendingPlanChange = async (subscription: Stripe.Subscription | null
return null;
};
const resolveHasPaymentMethod = (
subscription: Stripe.Subscription | null,
customer: Stripe.Customer | Stripe.DeletedCustomer
) => {
if (subscription?.default_payment_method != null) {
return true;
}
if (customer.deleted) {
return false;
}
return customer.invoice_settings.default_payment_method != null || customer.default_source != null;
};
export const syncOrganizationBillingFromStripe = async (
organizationId: string,
event?: { id: string; created: number }
@@ -1147,10 +1132,9 @@ export const syncOrganizationBillingFromStripe = async (
return billing;
}
const [subscription, featureLookupKeys, customer] = await Promise.all([
const [subscription, featureLookupKeys] = await Promise.all([
resolveCurrentSubscription(customerId),
listAllActiveEntitlements(customerId),
stripeClient.customers.retrieve(customerId),
]);
const cloudPlan = resolveCloudPlanFromSubscription(subscription);
@@ -1176,7 +1160,7 @@ export const syncOrganizationBillingFromStripe = async (
interval: billingInterval,
subscriptionStatus,
subscriptionId: subscription?.id ?? null,
hasPaymentMethod: resolveHasPaymentMethod(subscription, customer),
hasPaymentMethod: subscription?.default_payment_method != null,
features: featureLookupKeys,
pendingChange,
lastStripeEventCreatedAt: toIsoStringOrNull(incomingEventDate ?? previousEventDate),

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,

View File

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

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;

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",
},
],

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"));

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)) };

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"));

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);

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);

View File

@@ -234,6 +234,15 @@ const nextConfig = {
},
],
},
{
source: "/auth/forgot-password/reset",
headers: [
{
key: "Referrer-Policy",
value: "no-referrer",
},
],
},
{
source: "/js/(.*)",
headers: [

View File

@@ -8,27 +8,20 @@ test.describe("Authentication Security Tests - Vulnerability Prevention", () =>
let csrfToken: string;
let testUser: { email: string; password: string };
test.beforeEach(async ({ request, users }) => {
// Get CSRF token for authentication requests
const csrfResponse = await request.get("/api/auth/csrf");
const csrfData = await csrfResponse.json();
csrfToken = csrfData.csrfToken;
test("should disable referrers on the password reset page", async ({ request }) => {
const response = await request.get("/auth/forgot-password/reset?token=test-token");
// Create a test user for "existing user" scenarios with unique email
const uniqueId = Date.now() + Math.random();
const userName = "Security Test User";
const userEmail = `security-test-${uniqueId}@example.com`;
await users.create({
name: userName,
email: userEmail,
});
testUser = {
email: userEmail,
password: userName, // The fixture uses the name as password
};
expect(response.status()).toBe(200);
expect(response.headers()["referrer-policy"]).toBe("no-referrer");
});
test.describe("DoS Protection - Password Length Limits", () => {
test.beforeEach(async ({ request }) => {
const csrfResponse = await request.get("/api/auth/csrf");
const csrfData = await csrfResponse.json();
csrfToken = csrfData.csrfToken;
});
test("should handle extremely long passwords without crashing", async ({ request }) => {
const email = "nonexistent-dos-test@example.com"; // Use non-existent email for DoS test
const extremelyLongPassword = "A".repeat(50000); // 50,000 characters
@@ -126,6 +119,24 @@ test.describe("Authentication Security Tests - Vulnerability Prevention", () =>
});
test.describe("Timing Attack Prevention - User Enumeration Protection", () => {
test.beforeEach(async ({ request, users }) => {
const csrfResponse = await request.get("/api/auth/csrf");
const csrfData = await csrfResponse.json();
csrfToken = csrfData.csrfToken;
const uniqueId = Date.now() + Math.random();
const userName = "Security Test User";
const userEmail = `security-test-${uniqueId}@example.com`;
await users.create({
name: userName,
email: userEmail,
});
testUser = {
email: userEmail,
password: userName, // The fixture uses the name as password
};
});
test("should not reveal user existence through response timing differences", async ({ request }) => {
// Helper functions for statistical analysis
const calculateMedian = (values: number[]): number => {
@@ -359,6 +370,12 @@ test.describe("Authentication Security Tests - Vulnerability Prevention", () =>
});
test.describe("Security Headers and Response Safety", () => {
test.beforeEach(async ({ request }) => {
const csrfResponse = await request.get("/api/auth/csrf");
const csrfData = await csrfResponse.json();
csrfToken = csrfData.csrfToken;
});
test("should include security headers in responses", async ({ request }) => {
const response = await request.post("/api/auth/callback/credentials", {
data: {

View File

@@ -43,6 +43,17 @@ vi.mock("@/lib/common/utils", () => ({
handleHiddenFields: vi.fn(),
}));
const mockUpdateQueue = {
hasPendingWork: vi.fn().mockReturnValue(false),
waitForPendingWork: vi.fn().mockResolvedValue(true),
};
vi.mock("@/lib/user/update-queue", () => ({
UpdateQueue: {
getInstance: vi.fn(() => mockUpdateQueue),
},
}));
describe("widget-file", () => {
let getInstanceConfigMock: MockInstance<() => Config>;
let getInstanceLoggerMock: MockInstance<() => Logger>;
@@ -249,4 +260,265 @@ describe("widget-file", () => {
widget.removeWidgetContainer();
expect(document.getElementById("formbricks-container")).toBeFalsy();
});
test("renderWidget waits for pending identification before rendering", async () => {
mockUpdateQueue.hasPendingWork.mockReturnValue(true);
mockUpdateQueue.waitForPendingWork.mockResolvedValue(true);
const mockConfigValue = {
get: vi.fn().mockReturnValue({
appUrl: "https://fake.app",
environmentId: "env_123",
environment: {
data: {
project: {
clickOutsideClose: true,
overlay: "none",
placement: "bottomRight",
inAppSurveyBranding: true,
},
},
},
user: {
data: {
userId: "user_abc",
contactId: "contact_abc",
displays: [],
responses: [],
lastDisplayAt: null,
language: "en",
},
},
}),
update: vi.fn(),
};
getInstanceConfigMock.mockReturnValue(mockConfigValue as unknown as Config);
widget.setIsSurveyRunning(false);
// @ts-expect-error -- mock window.formbricksSurveys
window.formbricksSurveys = {
renderSurvey: vi.fn(),
};
vi.useFakeTimers();
await widget.renderWidget({
...mockSurvey,
delay: 0,
} as unknown as TEnvironmentStateSurvey);
expect(mockUpdateQueue.hasPendingWork).toHaveBeenCalled();
expect(mockUpdateQueue.waitForPendingWork).toHaveBeenCalled();
vi.advanceTimersByTime(0);
expect(window.formbricksSurveys.renderSurvey).toHaveBeenCalledWith(
expect.objectContaining({
contactId: "contact_abc",
})
);
vi.useRealTimers();
});
test("renderWidget does not wait when no identification is pending", async () => {
mockUpdateQueue.hasPendingWork.mockReturnValue(false);
const mockConfigValue = {
get: vi.fn().mockReturnValue({
appUrl: "https://fake.app",
environmentId: "env_123",
environment: {
data: {
project: {
clickOutsideClose: true,
overlay: "none",
placement: "bottomRight",
inAppSurveyBranding: true,
},
},
},
user: {
data: {
userId: "user_abc",
contactId: "contact_abc",
displays: [],
responses: [],
lastDisplayAt: null,
language: "en",
},
},
}),
update: vi.fn(),
};
getInstanceConfigMock.mockReturnValue(mockConfigValue as unknown as Config);
widget.setIsSurveyRunning(false);
// @ts-expect-error -- mock window.formbricksSurveys
window.formbricksSurveys = {
renderSurvey: vi.fn(),
};
vi.useFakeTimers();
await widget.renderWidget({
...mockSurvey,
delay: 0,
} as unknown as TEnvironmentStateSurvey);
expect(mockUpdateQueue.hasPendingWork).toHaveBeenCalled();
expect(mockUpdateQueue.waitForPendingWork).not.toHaveBeenCalled();
vi.advanceTimersByTime(0);
expect(window.formbricksSurveys.renderSurvey).toHaveBeenCalled();
vi.useRealTimers();
});
test("renderWidget reads contactId after identification wait completes", async () => {
let callCount = 0;
const mockConfigValue = {
get: vi.fn().mockImplementation(() => {
callCount++;
return {
appUrl: "https://fake.app",
environmentId: "env_123",
environment: {
data: {
project: {
clickOutsideClose: true,
overlay: "none",
placement: "bottomRight",
inAppSurveyBranding: true,
},
},
},
user: {
data: {
// Simulate contactId becoming available after identification
userId: "user_abc",
contactId: callCount > 2 ? "contact_after_identification" : undefined,
displays: [],
responses: [],
lastDisplayAt: null,
language: "en",
},
},
};
}),
update: vi.fn(),
};
getInstanceConfigMock.mockReturnValue(mockConfigValue as unknown as Config);
mockUpdateQueue.hasPendingWork.mockReturnValue(true);
mockUpdateQueue.waitForPendingWork.mockResolvedValue(true);
widget.setIsSurveyRunning(false);
// @ts-expect-error -- mock window.formbricksSurveys
window.formbricksSurveys = {
renderSurvey: vi.fn(),
};
vi.useFakeTimers();
await widget.renderWidget({
...mockSurvey,
delay: 0,
} as unknown as TEnvironmentStateSurvey);
vi.advanceTimersByTime(0);
// The contactId passed to renderSurvey should be read after the wait
expect(window.formbricksSurveys.renderSurvey).toHaveBeenCalledWith(
expect.objectContaining({
contactId: "contact_after_identification",
})
);
vi.useRealTimers();
});
test("renderWidget skips survey when identification fails and survey has segment filters", async () => {
mockUpdateQueue.hasPendingWork.mockReturnValue(true);
mockUpdateQueue.waitForPendingWork.mockResolvedValue(false);
widget.setIsSurveyRunning(false);
// @ts-expect-error -- mock window.formbricksSurveys
window.formbricksSurveys = {
renderSurvey: vi.fn(),
};
await widget.renderWidget({
...mockSurvey,
delay: 0,
segment: { id: "seg_1", filters: [{ type: "attribute", value: "plan" }] },
} as unknown as TEnvironmentStateSurvey);
expect(mockUpdateQueue.waitForPendingWork).toHaveBeenCalled();
expect(mockLogger.debug).toHaveBeenCalledWith(
"User identification failed. Skipping survey with segment filters."
);
expect(window.formbricksSurveys.renderSurvey).not.toHaveBeenCalled();
});
test("renderWidget proceeds when identification fails but survey has no segment filters", async () => {
mockUpdateQueue.hasPendingWork.mockReturnValue(true);
mockUpdateQueue.waitForPendingWork.mockResolvedValue(false);
const mockConfigValue = {
get: vi.fn().mockReturnValue({
appUrl: "https://fake.app",
environmentId: "env_123",
environment: {
data: {
project: {
clickOutsideClose: true,
overlay: "none",
placement: "bottomRight",
inAppSurveyBranding: true,
},
},
},
user: {
data: {
userId: null,
contactId: null,
displays: [],
responses: [],
lastDisplayAt: null,
language: "en",
},
},
}),
update: vi.fn(),
};
getInstanceConfigMock.mockReturnValue(mockConfigValue as unknown as Config);
widget.setIsSurveyRunning(false);
// @ts-expect-error -- mock window.formbricksSurveys
window.formbricksSurveys = {
renderSurvey: vi.fn(),
};
vi.useFakeTimers();
await widget.renderWidget({
...mockSurvey,
delay: 0,
segment: undefined,
} as unknown as TEnvironmentStateSurvey);
expect(mockLogger.debug).toHaveBeenCalledWith(
"User identification failed but survey has no segment filters. Proceeding."
);
vi.advanceTimersByTime(0);
expect(window.formbricksSurveys.renderSurvey).toHaveBeenCalled();
vi.useRealTimers();
});
});

View File

@@ -11,6 +11,7 @@ import {
handleHiddenFields,
shouldDisplayBasedOnPercentage,
} from "@/lib/common/utils";
import { UpdateQueue } from "@/lib/user/update-queue";
import { type TEnvironmentStateSurvey, type TUserState } from "@/types/config";
import { type TTrackProperties } from "@/types/survey";
@@ -60,6 +61,24 @@ export const renderWidget = async (
setIsSurveyRunning(true);
// Wait for pending user identification to complete before rendering
const updateQueue = UpdateQueue.getInstance();
if (updateQueue.hasPendingWork()) {
logger.debug("Waiting for pending user identification before rendering survey");
const identificationSucceeded = await updateQueue.waitForPendingWork();
if (!identificationSucceeded) {
const hasSegmentFilters = Array.isArray(survey.segment?.filters) && survey.segment.filters.length > 0;
if (hasSegmentFilters) {
logger.debug("User identification failed. Skipping survey with segment filters.");
setIsSurveyRunning(false);
return;
}
logger.debug("User identification failed but survey has no segment filters. Proceeding.");
}
}
if (survey.delay) {
logger.debug(`Delaying survey "${survey.name}" by ${survey.delay.toString()} seconds.`);
}

View File

@@ -169,4 +169,104 @@ describe("UpdateQueue", () => {
"Formbricks can't set attributes without a userId! Please set a userId first with the setUserId function"
);
});
test("hasPendingWork returns false when no updates and no flush in flight", () => {
expect(updateQueue.hasPendingWork()).toBe(false);
});
test("hasPendingWork returns true when updates are queued", () => {
updateQueue.updateUserId(mockUserId1);
expect(updateQueue.hasPendingWork()).toBe(true);
});
test("hasPendingWork returns true while processUpdates flush is in flight", () => {
(sendUpdates as Mock).mockReturnValue({
ok: true,
data: { hasWarnings: false },
});
updateQueue.updateUserId(mockUserId1);
// Start processing but don't await — the debounce means the flush is in-flight
void updateQueue.processUpdates();
expect(updateQueue.hasPendingWork()).toBe(true);
});
test("waitForPendingWork returns true immediately when no pending work", async () => {
const result = await updateQueue.waitForPendingWork();
expect(result).toBe(true);
});
test("waitForPendingWork returns true when processUpdates succeeds", async () => {
(sendUpdates as Mock).mockReturnValue({
ok: true,
data: { hasWarnings: false },
});
updateQueue.updateUserId(mockUserId1);
void updateQueue.processUpdates();
const result = await updateQueue.waitForPendingWork();
expect(result).toBe(true);
expect(updateQueue.hasPendingWork()).toBe(false);
expect(sendUpdates).toHaveBeenCalled();
});
test("waitForPendingWork returns false when processUpdates rejects", async () => {
loggerMock.mockReturnValue(mockLogger as unknown as Logger);
(sendUpdates as Mock).mockRejectedValue(new Error("network error"));
updateQueue.updateUserId(mockUserId1);
// eslint-disable-next-line @typescript-eslint/no-empty-function -- intentionally swallowing rejection to avoid unhandled promise
const processPromise = updateQueue.processUpdates().catch(() => {});
const result = await updateQueue.waitForPendingWork();
expect(result).toBe(false);
await processPromise;
});
test("waitForPendingWork returns false when flush hangs past timeout", async () => {
vi.useFakeTimers();
// sendUpdates returns a promise that never resolves, simulating a network hang
// eslint-disable-next-line @typescript-eslint/no-empty-function -- intentionally never-resolving promise
(sendUpdates as Mock).mockReturnValue(new Promise(() => {}));
updateQueue.updateUserId(mockUserId1);
void updateQueue.processUpdates();
const resultPromise = updateQueue.waitForPendingWork();
// Advance past the debounce delay (500ms) so the handler fires and hangs on sendUpdates
await vi.advanceTimersByTimeAsync(500);
// Advance past the pending work timeout (5000ms)
await vi.advanceTimersByTimeAsync(5000);
const result = await resultPromise;
expect(result).toBe(false);
vi.useRealTimers();
});
test("processUpdates reuses pending flush instead of creating orphaned promises", async () => {
(sendUpdates as Mock).mockReturnValue({
ok: true,
data: { hasWarnings: false },
});
updateQueue.updateUserId(mockUserId1);
// First call creates the flush promise
const firstPromise = updateQueue.processUpdates();
// Second call while first is still pending should not create a new flush
updateQueue.updateAttributes({ name: mockAttributes.name });
const secondPromise = updateQueue.processUpdates();
// Both promises should resolve (second is not orphaned)
await Promise.all([firstPromise, secondPromise]);
expect(updateQueue.hasPendingWork()).toBe(false);
});
});

View File

@@ -8,7 +8,9 @@ export class UpdateQueue {
private static instance: UpdateQueue | null = null;
private updates: TUpdates | null = null;
private debounceTimeout: NodeJS.Timeout | null = null;
private pendingFlush: Promise<void> | null = null;
private readonly DEBOUNCE_DELAY = 500;
private readonly PENDING_WORK_TIMEOUT = 5000;
private constructor() {}
@@ -63,17 +65,45 @@ export class UpdateQueue {
return !this.updates;
}
public hasPendingWork(): boolean {
return this.updates !== null || this.pendingFlush !== null;
}
public async waitForPendingWork(): Promise<boolean> {
if (!this.hasPendingWork()) return true;
const flush = this.pendingFlush ?? this.processUpdates();
try {
const succeeded = await Promise.race([
flush.then(() => true as const),
new Promise<false>((resolve) => {
setTimeout(() => {
resolve(false);
}, this.PENDING_WORK_TIMEOUT);
}),
]);
return succeeded;
} catch {
return false;
}
}
public async processUpdates(): Promise<void> {
const logger = Logger.getInstance();
if (!this.updates) {
return;
}
// If a flush is already in flight, reuse it instead of creating a new promise
if (this.pendingFlush) {
return this.pendingFlush;
}
if (this.debounceTimeout) {
clearTimeout(this.debounceTimeout);
}
return new Promise((resolve, reject) => {
const flushPromise = new Promise<void>((resolve, reject) => {
const handler = async (): Promise<void> => {
try {
let currentUpdates = { ...this.updates };
@@ -147,8 +177,10 @@ export class UpdateQueue {
}
this.clearUpdates();
this.pendingFlush = null;
resolve();
} catch (error: unknown) {
this.pendingFlush = null;
logger.error(
`Failed to process updates: ${error instanceof Error ? error.message : "Unknown error"}`
);
@@ -158,5 +190,8 @@ export class UpdateQueue {
this.debounceTimeout = setTimeout(() => void handler(), this.DEBOUNCE_DELAY);
});
this.pendingFlush = flushPromise;
return flushPromise;
}
}