fix: implement missing password validation (#27634)
* fix: validate password presence and requirements * chore: add error code for invalid passwords * chore: avoid trimming password * chore: add test for password validation * fix: update password to delete user * fix: create user password following protocol * test: simplify deleteMeHandler password validation tests * fix: address coderabbit * fix: modify login params for failing tests * fix: failing e2e tests --------- Co-authored-by: Bandhan Majumder <133476557+bandhan-majumder@users.noreply.github.com>
This commit is contained in:
co-authored by
Bandhan Majumder
parent
a46d5f8dd4
commit
0d164da8dd
@@ -5,20 +5,27 @@ import { test } from "../lib/fixtures";
|
||||
test.afterEach(({ users }) => users.deleteAll());
|
||||
|
||||
test("Can delete user account", async ({ page, users }) => {
|
||||
const deletionPassword = "Deleteme1";
|
||||
const user = await users.create({
|
||||
username: "delete-me",
|
||||
username: "delete-me",
|
||||
password: deletionPassword,
|
||||
});
|
||||
await user.apiLogin();
|
||||
|
||||
await user.apiLogin(undefined, deletionPassword);
|
||||
await page.goto(`/settings/my-account/profile`);
|
||||
await page.waitForLoadState("networkidle");
|
||||
await page.waitForSelector("[data-testid=dashboard-shell]");
|
||||
|
||||
await page.click("[data-testid=delete-account]");
|
||||
const deleteAccountButton = page.locator(
|
||||
'[data-testid="delete-account"]:visible'
|
||||
);
|
||||
await expect(deleteAccountButton).toBeVisible({ timeout: 30_000 });
|
||||
await deleteAccountButton.click();
|
||||
|
||||
expect(user.username).toBeTruthy();
|
||||
|
||||
const $passwordField = page.locator("[data-testid=password]");
|
||||
await $passwordField.fill(String(user.username));
|
||||
await $passwordField.fill(deletionPassword);
|
||||
|
||||
await Promise.all([
|
||||
page.waitForURL((url) => url.pathname === "/auth/logout"),
|
||||
@@ -26,4 +33,4 @@ test("Can delete user account", async ({ page, users }) => {
|
||||
]);
|
||||
|
||||
await expect(page.locator(`[id="modal-title"]`)).toHaveText("You've been logged out");
|
||||
});
|
||||
});
|
||||
@@ -1,13 +1,14 @@
|
||||
import { expect } from "@playwright/test";
|
||||
|
||||
import { test } from "./lib/fixtures";
|
||||
import { submitAndWaitForResponse } from "./lib/testUtils";
|
||||
import { submitAndWaitForResponse, getDefaultPassword } from "./lib/testUtils";
|
||||
|
||||
test.afterEach(({ users }) => users.deleteAll());
|
||||
|
||||
test.describe("Change Password Test", () => {
|
||||
test("change password", async ({ page, users }) => {
|
||||
const pro = await users.create();
|
||||
const currentPassword = getDefaultPassword(String(pro.username))
|
||||
await pro.apiLogin();
|
||||
// Go to http://localhost:3000/settings/security
|
||||
await page.goto("/settings/security/password");
|
||||
@@ -15,10 +16,10 @@ test.describe("Change Password Test", () => {
|
||||
expect(pro.username).toBeTruthy();
|
||||
|
||||
// Fill form
|
||||
await page.locator('[name="oldPassword"]').fill(String(pro.username));
|
||||
await page.locator('[name="oldPassword"]').fill(currentPassword);
|
||||
|
||||
const $newPasswordField = page.locator('[name="newPassword"]');
|
||||
$newPasswordField.fill(`${pro.username}Aa1111`);
|
||||
await $newPasswordField.fill(`${pro.username}Aa1111`);
|
||||
await submitAndWaitForResponse(page, "/api/trpc/auth/changePassword?batch=1", {
|
||||
action: () => page.locator("text=Update").click(),
|
||||
});
|
||||
|
||||
@@ -632,23 +632,23 @@ const createUserFixture = (user: UserWithIncludes, page: Page) => {
|
||||
eventTypes: user.eventTypes,
|
||||
self,
|
||||
apiLogin: async (navigateToUrl?: string, password?: string) =>
|
||||
apiLogin({ ...(await self()), password: password || user.username }, store.page, navigateToUrl),
|
||||
apiLogin({ ...(await self()), password: password ?? `${user.username!.charAt(0).toUpperCase()}${user.username!.slice(1)}1` }, store.page, navigateToUrl),
|
||||
/** Don't forget to close context at the end */
|
||||
apiLoginOnNewBrowser: async (browser: Browser, password?: string) => {
|
||||
const newContext = await browser.newContext();
|
||||
const newPage = await newContext.newPage();
|
||||
await apiLogin({ ...(await self()), password: password || user.username }, newPage);
|
||||
await apiLogin({ ...(await self()), password: password ?? `${user.username!.charAt(0).toUpperCase()}${user.username!.slice(1)}1` }, newPage);
|
||||
// Don't forget to: newContext.close();
|
||||
return [newContext, newPage] as const;
|
||||
},
|
||||
/**
|
||||
* @deprecated use apiLogin instead
|
||||
*/
|
||||
login: async () => login({ ...(await self()), password: user.username }, store.page),
|
||||
loginOnNewBrowser: async (browser: Browser) => {
|
||||
login: async (password?: string) => login({ ...(await self()), password: password ?? `${user.username!.charAt(0).toUpperCase()}${user.username!.slice(1)}1` }, store.page),
|
||||
loginOnNewBrowser: async (browser: Browser, password?: string) => {
|
||||
const newContext = await browser.newContext();
|
||||
const newPage = await newContext.newPage();
|
||||
await login({ ...(await self()), password: user.username }, newPage);
|
||||
await login({ ...(await self()), password: password ?? `${user.username!.charAt(0).toUpperCase()}${user.username!.slice(1)}1` }, newPage);
|
||||
// Don't forget to: newContext.close();
|
||||
return [newContext, newPage] as const;
|
||||
},
|
||||
@@ -826,13 +826,15 @@ const createUser = (
|
||||
: `${opts?.username || "user"}${suffixToMakeUsernameUnique}`;
|
||||
|
||||
const emailDomain = opts?.emailDomain || "example.com";
|
||||
const defaultPassword = opts?.password ?? `${uname.charAt(0).toUpperCase()}${uname.slice(1)}1`;
|
||||
|
||||
return {
|
||||
username: uname,
|
||||
name: opts?.name,
|
||||
email: opts?.email ?? `${uname}@${emailDomain}`,
|
||||
password: {
|
||||
create: {
|
||||
hash: hashPassword(uname),
|
||||
hash: hashPassword(defaultPassword),
|
||||
},
|
||||
},
|
||||
emailVerified: new Date(),
|
||||
|
||||
@@ -637,3 +637,7 @@ export async function cancelBookingFromBookingsList({
|
||||
bookingUid,
|
||||
};
|
||||
}
|
||||
|
||||
export function getDefaultPassword(username: string) {
|
||||
return `${username.charAt(0).toUpperCase()}${username.slice(1)}1`;
|
||||
}
|
||||
@@ -7,6 +7,7 @@ import { totpAuthenticatorCheck } from "@calcom/lib/totp";
|
||||
import { prisma } from "@calcom/prisma";
|
||||
|
||||
import { test } from "./lib/fixtures";
|
||||
import { getDefaultPassword } from "./lib/testUtils";
|
||||
|
||||
test.describe.configure({ mode: "parallel" });
|
||||
|
||||
@@ -23,7 +24,7 @@ test.describe("2FA Tests", async () => {
|
||||
const user = await test.step("Enable 2FA", async () => {
|
||||
const user = await users.create();
|
||||
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
|
||||
const userPassword = user.username!;
|
||||
const userPassword = getDefaultPassword(user.username!);
|
||||
await user.apiLogin();
|
||||
|
||||
// expects the home page for an authorized user
|
||||
@@ -90,7 +91,7 @@ test.describe("2FA Tests", async () => {
|
||||
const user = await test.step("Enable 2FA", async () => {
|
||||
const user = await users.create();
|
||||
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
|
||||
const userPassword = user.username!;
|
||||
const userPassword = getDefaultPassword(user.username!);
|
||||
await user.apiLogin();
|
||||
|
||||
// expects the home page for an authorized user
|
||||
@@ -129,7 +130,7 @@ test.describe("2FA Tests", async () => {
|
||||
|
||||
await test.step("Disable 2FA", async () => {
|
||||
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
|
||||
const userPassword = user.username!;
|
||||
const userPassword = getDefaultPassword(user.username!);
|
||||
|
||||
// expects the home page for an authorized user
|
||||
await page.goto("/settings/security/two-factor-auth");
|
||||
|
||||
@@ -5,6 +5,7 @@ import { OAUTH_ERROR_REASONS } from "@calcom/features/oauth/services/OAuthServic
|
||||
import { generateSecret } from "@calcom/features/oauth/utils/generateSecret";
|
||||
import { WEBAPP_URL } from "@calcom/lib/constants";
|
||||
import { prisma } from "@calcom/prisma";
|
||||
import { getDefaultPassword } from "./lib/testUtils";
|
||||
|
||||
import { test } from "./lib/fixtures";
|
||||
|
||||
@@ -158,7 +159,7 @@ test.describe("OAuth Provider", () => {
|
||||
await expect(page.getByRole("heading", { name: "Cal.diy" })).toBeVisible();
|
||||
await expect(page.getByTestId("login-subtitle")).toBeVisible();
|
||||
await page.locator("#email").fill(user.email);
|
||||
await page.locator("#password").fill(user.username || "");
|
||||
await page.locator("#password").fill(getDefaultPassword(user.username!));
|
||||
await page.locator('[type="submit"]').click();
|
||||
|
||||
await page.waitForSelector('[data-testid="allow-button"]');
|
||||
|
||||
@@ -7,7 +7,7 @@ import type { Messages } from "mailhog";
|
||||
import type { createEmailsFixture } from "./fixtures/emails";
|
||||
import type { createUsersFixture } from "./fixtures/users";
|
||||
import { test } from "./lib/fixtures";
|
||||
import { getEmailsReceivedByUser, submitAndWaitForResponse } from "./lib/testUtils";
|
||||
import { getEmailsReceivedByUser, submitAndWaitForResponse, getDefaultPassword } from "./lib/testUtils";
|
||||
|
||||
const expectInvitationEmailToBeReceived = async ({
|
||||
emails,
|
||||
@@ -73,7 +73,7 @@ test.describe("Update Profile", () => {
|
||||
await page
|
||||
.getByTestId("password")
|
||||
.first()
|
||||
.fill(user?.username ?? "Nameless User");
|
||||
.fill(getDefaultPassword(String(user.username)));
|
||||
|
||||
await submitAndWaitForResponse(page, "/api/trpc/me/updateProfile?batch=1", {
|
||||
action: () => page.getByTestId("profile-update-email-submit-button").first().click(),
|
||||
|
||||
@@ -17,4 +17,5 @@ export enum ErrorCode {
|
||||
RateLimitExceeded = "rate-limit-exceeded",
|
||||
SocialIdentityProviderRequired = "social-identity-provider-required",
|
||||
UserAccountLocked = "user-account-locked",
|
||||
PasswordPolicyViolation = "password-policy-violation",
|
||||
}
|
||||
|
||||
@@ -0,0 +1,63 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
|
||||
import { ErrorCode } from "@calcom/features/auth/lib/ErrorCode";
|
||||
import { HttpError } from "@calcom/lib/http-error";
|
||||
|
||||
import { deleteMeHandler } from "./deleteMe.handler";
|
||||
import type { DeleteMeOptions } from "./deleteMe.handler";
|
||||
|
||||
const makeMockCtx = (role: "USER" | "ADMIN" = "USER") =>
|
||||
({
|
||||
user: {
|
||||
id: 123,
|
||||
name: "Test User",
|
||||
email: "test@example.com",
|
||||
role,
|
||||
},
|
||||
} as DeleteMeOptions["ctx"]);
|
||||
|
||||
describe("deleteMeHandler", () => {
|
||||
describe("Password presence validation", () => {
|
||||
it("should throw error when password is empty", async () => {
|
||||
await expect(
|
||||
deleteMeHandler({
|
||||
ctx: makeMockCtx(),
|
||||
input: { password: "" },
|
||||
})
|
||||
).rejects.toThrow(new HttpError({ statusCode: 400, message: ErrorCode.UserMissingPassword }));
|
||||
});
|
||||
|
||||
it("should throw error when password is only whitespace", async () => {
|
||||
await expect(
|
||||
deleteMeHandler({
|
||||
ctx: makeMockCtx(),
|
||||
input: { password: " " },
|
||||
})
|
||||
).rejects.toThrow(new HttpError({ statusCode: 400, message: ErrorCode.UserMissingPassword }));
|
||||
});
|
||||
});
|
||||
|
||||
describe("Password policy validation", () => {
|
||||
it("should throw error for a weak password (USER role)", async () => {
|
||||
await expect(
|
||||
deleteMeHandler({
|
||||
ctx: makeMockCtx("USER"),
|
||||
input: { password: "weak" },
|
||||
})
|
||||
).rejects.toThrow(
|
||||
new HttpError({ statusCode: 400, message: ErrorCode.PasswordPolicyViolation })
|
||||
);
|
||||
});
|
||||
|
||||
it("should throw error for a weak password (ADMIN role)", async () => {
|
||||
await expect(
|
||||
deleteMeHandler({
|
||||
ctx: makeMockCtx("ADMIN"),
|
||||
input: { password: "weak" },
|
||||
})
|
||||
).rejects.toThrow(
|
||||
new HttpError({ statusCode: 400, message: ErrorCode.PasswordPolicyViolation })
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -2,6 +2,7 @@ import { ErrorCode } from "@calcom/features/auth/lib/ErrorCode";
|
||||
import { verifyPassword } from "@calcom/features/auth/lib/verifyPassword";
|
||||
import { deleteUser } from "@calcom/features/users/lib/deleteUser";
|
||||
import { symmetricDecrypt } from "@calcom/lib/crypto";
|
||||
import { isPasswordValid } from "@calcom/lib/auth/isPasswordValid";
|
||||
import { HttpError } from "@calcom/lib/http-error";
|
||||
import { totpAuthenticatorCheck } from "@calcom/lib/totp";
|
||||
import { prisma } from "@calcom/prisma";
|
||||
@@ -10,7 +11,7 @@ import type { TrpcSessionUser } from "@calcom/trpc/server/types";
|
||||
|
||||
import type { TDeleteMeInputSchema } from "./deleteMe.schema";
|
||||
|
||||
type DeleteMeOptions = {
|
||||
export type DeleteMeOptions = {
|
||||
ctx: {
|
||||
user: NonNullable<TrpcSessionUser>;
|
||||
};
|
||||
@@ -18,7 +19,19 @@ type DeleteMeOptions = {
|
||||
};
|
||||
|
||||
export const deleteMeHandler = async ({ ctx, input }: DeleteMeOptions) => {
|
||||
// TODO: First check password is part of input and meets requirements.
|
||||
if (!input.password || input.password.trim().length === 0) {
|
||||
throw new HttpError({ statusCode: 400, message: ErrorCode.UserMissingPassword });
|
||||
}
|
||||
|
||||
const password = input.password;
|
||||
|
||||
const strict = ctx.user.role !== "USER";
|
||||
if (!isPasswordValid(password, false, strict)) {
|
||||
throw new HttpError({
|
||||
statusCode: 400,
|
||||
message: ErrorCode.PasswordPolicyViolation,
|
||||
});
|
||||
}
|
||||
|
||||
// Check if input.password is correct
|
||||
const user = await prisma.user.findUnique({
|
||||
@@ -47,7 +60,7 @@ export const deleteMeHandler = async ({ ctx, input }: DeleteMeOptions) => {
|
||||
throw new HttpError({ statusCode: 400, message: ErrorCode.UserMissingPassword });
|
||||
}
|
||||
|
||||
const isCorrectPassword = await verifyPassword(input.password, user.password.hash);
|
||||
const isCorrectPassword = await verifyPassword(password, user.password.hash);
|
||||
if (!isCorrectPassword) {
|
||||
throw new HttpError({ statusCode: 403, message: ErrorCode.IncorrectPassword });
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user