chore: address pr concerns

This commit is contained in:
Tiago Farto
2026-03-31 08:21:56 +00:00
parent e5390daf20
commit 5b05ec87f2
8 changed files with 37 additions and 172 deletions

2
.gitignore vendored
View File

@@ -45,7 +45,7 @@ yarn-error.log*
.direnv
# Playwright
/test-results/
**/test-results/
/playwright-report/
/blob-report/
/playwright/.cache/

View File

@@ -1,113 +0,0 @@
import { afterEach, beforeEach, describe, expect, test, vi } from "vitest";
import { OperationNotAllowedError } from "@formbricks/types/errors";
import { requestPasswordReset } from "@/modules/auth/forgot-password/lib/password-reset-service";
import { resetPasswordAction } from "./actions";
vi.mock("@/app/(app)/environments/[environmentId]/settings/(account)/profile/lib/user", () => ({
getIsEmailUnique: vi.fn(),
verifyUserPassword: vi.fn(),
}));
vi.mock("@/lib/constants", async (importOriginal) => {
const actual = await importOriginal<typeof import("@/lib/constants")>();
return {
...actual,
EMAIL_VERIFICATION_DISABLED: false,
};
});
vi.mock("@/lib/user/service", () => ({
getUser: vi.fn(),
updateUser: vi.fn(),
}));
vi.mock("@/lib/utils/action-client", () => ({
authenticatedActionClient: {
inputSchema: vi.fn().mockReturnThis(),
action: vi.fn((fn) => fn),
},
}));
vi.mock("@/modules/auth/lib/brevo", () => ({
updateBrevoCustomer: vi.fn(),
}));
vi.mock("@/modules/auth/forgot-password/lib/password-reset-service", () => ({
requestPasswordReset: vi.fn(),
}));
vi.mock("@/modules/core/rate-limit/helpers", () => ({
applyRateLimit: vi.fn(),
}));
vi.mock("@/modules/email", () => ({
sendVerificationNewEmail: vi.fn(),
}));
vi.mock("@/modules/core/rate-limit/rate-limit-configs", () => ({
rateLimitConfigs: {
actions: {
emailUpdate: { interval: 3600, allowedPerInterval: 3, namespace: "action:email" },
},
},
}));
vi.mock("@/modules/ee/audit-logs/lib/handler", () => ({
withAuditLogging: vi.fn((_event: string, _object: string, fn: Function) => fn),
}));
describe("profile resetPasswordAction", () => {
const mockCtx = {
user: {
id: "user_123",
email: "user@example.com",
locale: "en-US",
identityProvider: "email",
},
auditLoggingCtx: {
userId: "",
},
};
beforeEach(() => {
vi.resetAllMocks();
});
afterEach(() => {
vi.restoreAllMocks();
});
test("delegates to the shared password reset request service", async () => {
const result = await resetPasswordAction({
ctx: mockCtx,
} as any);
expect(result).toEqual({ success: true });
expect(requestPasswordReset).toHaveBeenCalledWith(mockCtx.user, "profile");
expect(mockCtx.auditLoggingCtx.userId).toBe(mockCtx.user.id);
});
test("surfaces request failures for authenticated users", async () => {
vi.mocked(requestPasswordReset).mockRejectedValue(new Error("SMTP failed"));
await expect(
resetPasswordAction({
ctx: mockCtx,
} as any)
).rejects.toThrow("SMTP failed");
});
test("rejects password reset for non-email identity providers", async () => {
await expect(
resetPasswordAction({
ctx: {
...mockCtx,
user: {
...mockCtx.user,
identityProvider: "google",
},
},
} as any)
).rejects.toThrow(OperationNotAllowedError);
});
});

View File

@@ -10,7 +10,7 @@ import {
getIsEmailUnique,
verifyUserPassword,
} from "@/app/(app)/environments/[environmentId]/settings/(account)/profile/lib/user";
import { EMAIL_VERIFICATION_DISABLED } from "@/lib/constants";
import { EMAIL_VERIFICATION_DISABLED, PASSWORD_RESET_DISABLED } from "@/lib/constants";
import { getUser, updateUser } from "@/lib/user/service";
import { authenticatedActionClient } from "@/lib/utils/action-client";
import { AuthenticatedActionClientCtx } from "@/lib/utils/action-client/types/context";
@@ -86,6 +86,10 @@ export const updateUserAction = authenticatedActionClient.inputSchema(ZUserPerso
export const resetPasswordAction = authenticatedActionClient.action(
withAuditLogging("passwordReset", "user", async ({ ctx }) => {
if (PASSWORD_RESET_DISABLED) {
throw new OperationNotAllowedError("Password reset is disabled");
}
if (ctx.user.identityProvider !== "email") {
throw new OperationNotAllowedError("Password reset is not allowed for this user.");
}

View File

@@ -1,9 +1,6 @@
import { createEnv } from "@t3-oss/env-nextjs";
import { z } from "zod";
const PASSWORD_RESET_TOKEN_LIFETIME_ERROR =
"PASSWORD_RESET_TOKEN_LIFETIME_MINUTES must be an integer between 5 and 120.";
export const env = createEnv({
/*
* Serverside Environment variables, not available on the client.
@@ -65,32 +62,7 @@ export const env = createEnv({
? z.string().optional()
: z.url("REDIS_URL is required for caching, rate limiting, and audit logging"),
PASSWORD_RESET_DISABLED: z.enum(["1", "0"]).optional(),
PASSWORD_RESET_TOKEN_LIFETIME_MINUTES: z
.string()
.optional()
.transform((value, ctx) => {
const rawValue = value ?? "30";
if (!/^\d+$/.test(rawValue)) {
ctx.addIssue({
code: "custom",
message: PASSWORD_RESET_TOKEN_LIFETIME_ERROR,
});
return z.NEVER;
}
const parsedValue = Number.parseInt(rawValue, 10);
if (parsedValue < 5 || parsedValue > 120) {
ctx.addIssue({
code: "custom",
message: PASSWORD_RESET_TOKEN_LIFETIME_ERROR,
});
return z.NEVER;
}
return parsedValue;
}),
PASSWORD_RESET_TOKEN_LIFETIME_MINUTES: z.coerce.number().int().min(5).max(120).optional().default(30),
PRIVACY_URL: z
.url()
.optional()

View File

@@ -5,7 +5,6 @@ import { DatabaseError } from "@formbricks/types/errors";
import {
consumeActiveToken,
deleteByTokenHash,
deleteByUserId,
findByTokenHash,
upsertActiveToken,
} from "./password-reset-token-repository";
@@ -68,17 +67,6 @@ describe("password-reset-token-repository", () => {
});
});
test("deletes by user id", async () => {
vi.mocked(prisma.passwordResetToken.deleteMany).mockResolvedValue({ count: 1 } as any);
const result = await deleteByUserId(userId);
expect(result).toBe(1);
expect(prisma.passwordResetToken.deleteMany).toHaveBeenCalledWith({
where: { userId },
});
});
test("deletes by token hash", async () => {
vi.mocked(prisma.passwordResetToken.deleteMany).mockResolvedValue({ count: 1 } as any);

View File

@@ -80,22 +80,6 @@ export const findByTokenHash = async (
}
};
export const deleteByUserId = async (userId: string, tx?: Prisma.TransactionClient): Promise<number> => {
validateInputs([userId, ZId]);
try {
const result = await getDbClient(tx).passwordResetToken.deleteMany({
where: {
userId,
},
});
return result.count;
} catch (error) {
return handleDatabaseError(error);
}
};
export const deleteByTokenHash = async (
tokenHash: string,
tx?: Prisma.TransactionClient

View File

@@ -2,14 +2,25 @@ import { afterEach, beforeEach, describe, expect, test, vi } from "vitest";
import {
INVALID_PASSWORD_RESET_TOKEN_ERROR_CODE,
InvalidPasswordResetTokenError,
OperationNotAllowedError,
} from "@formbricks/types/errors";
import { completePasswordReset } from "@/modules/auth/forgot-password/lib/password-reset-service";
import { resetPasswordAction } from "./actions";
const constantsState = vi.hoisted(() => ({
passwordResetDisabled: false,
}));
vi.mock("@/modules/auth/forgot-password/lib/password-reset-service", () => ({
completePasswordReset: vi.fn(),
}));
vi.mock("@/lib/constants", () => ({
get PASSWORD_RESET_DISABLED() {
return constantsState.passwordResetDisabled;
},
}));
vi.mock("@/modules/ee/audit-logs/lib/handler", () => ({
withAuditLogging: vi.fn((_event: string, _object: string, fn: Function) => fn),
}));
@@ -37,6 +48,7 @@ describe("resetPasswordAction", () => {
beforeEach(() => {
vi.resetAllMocks();
constantsState.passwordResetDisabled = false;
});
afterEach(() => {
@@ -85,4 +97,16 @@ describe("resetPasswordAction", () => {
} as any)
).rejects.toThrow(INVALID_PASSWORD_RESET_TOKEN_ERROR_CODE);
});
test("rejects reset attempts when password reset is disabled", async () => {
constantsState.passwordResetDisabled = true;
await expect(
resetPasswordAction({
ctx: mockCtx,
parsedInput,
} as any)
).rejects.toThrow(OperationNotAllowedError);
expect(completePasswordReset).not.toHaveBeenCalled();
});
});

View File

@@ -1,7 +1,9 @@
"use server";
import { z } from "zod";
import { OperationNotAllowedError } from "@formbricks/types/errors";
import { ZUserPassword } from "@formbricks/types/user";
import { PASSWORD_RESET_DISABLED } from "@/lib/constants";
import { actionClient } from "@/lib/utils/action-client";
import { completePasswordReset } from "@/modules/auth/forgot-password/lib/password-reset-service";
import { withAuditLogging } from "@/modules/ee/audit-logs/lib/handler";
@@ -13,6 +15,10 @@ const ZResetPasswordAction = z.object({
export const resetPasswordAction = actionClient.inputSchema(ZResetPasswordAction).action(
withAuditLogging("updated", "user", async ({ ctx, parsedInput }) => {
if (PASSWORD_RESET_DISABLED) {
throw new OperationNotAllowedError("Password reset is disabled");
}
const result = await completePasswordReset(parsedInput.token, parsedInput.password);
ctx.auditLoggingCtx.userId = result.userId;