Skip to content
This repository has been archived by the owner on Mar 23, 2023. It is now read-only.

Commit

Permalink
feat: enhance password validation (#3796)
Browse files Browse the repository at this point in the history
  • Loading branch information
alesmit authored May 25, 2021
1 parent b6a1367 commit 6f2a6d0
Show file tree
Hide file tree
Showing 19 changed files with 213 additions and 91 deletions.
5 changes: 5 additions & 0 deletions custom.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,8 @@ declare module "*.svg" {
const src: string;
export default src;
}

declare module "password-pwnd" {
const pwnd: (value: string) => Promise<number>;
const strong: (value: string) => Promise<1 | 0>;
}
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@
"is-ipfs": "^5.0.0",
"node-hid": "^2.1.1",
"parse-author": "^2.0.0",
"password-pwnd": "^1.0.7",
"pretty-bytes": "^5.6.0",
"querystring": "^0.2.1",
"react": "^16.13.1",
Expand Down
3 changes: 3 additions & 0 deletions src/app/i18n/common/i18n.ts
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,9 @@ export const translations: { [key: string]: any } = {
MIN_RECIPIENTS: "At least one recipient is required",
RECIPIENT_INVALID: "Recipient address is invalid",
PASSWORD_SAME_AS_OLD: "New password cannot be old password",
PASSWORD_WEAK:
"The password must contain at least 1 lowercase character, 1 uppercase character, 1 numeric character, 1 special character and must be 8 characters or longer",
PASSWORD_LEAKED: "Please change your password, it has been found in a previous breach",
},

DATETIME: {
Expand Down
101 changes: 87 additions & 14 deletions src/app/validations/password.test.tsx
Original file line number Diff line number Diff line change
@@ -1,32 +1,105 @@
/* eslint-disable @typescript-eslint/require-await */
import { renderHook } from "@testing-library/react-hooks";
import passwordPwnd from "password-pwnd";
import { useTranslation } from "react-i18next";

import { password } from "./password";

describe("Password Validation", () => {
it("should require a password of 6 characters minimum", () => {
let pwnd: jest.SpyInstance;

beforeEach(() => {
pwnd = jest.spyOn(passwordPwnd, "pwnd").mockImplementation(() => Promise.resolve(0));
});

it("should not be required", async () => {
const { result } = renderHook(() => useTranslation());
const { t } = result.current;
const passwordValidation = password(t);

const passwordRule = password(t);
expect(passwordRule.password("password").minLength).toEqual({
message: t("COMMON.VALIDATION.MIN_LENGTH", {
field: t("SETTINGS.GENERAL.PERSONAL.PASSWORD"),
minLength: 6,
}),
value: 6,
});
expect(await passwordValidation.password().validate("")).toEqual(true);
});

it("should require at least 1 lowercase character", async () => {
const { result } = renderHook(() => useTranslation());
const { t } = result.current;
const passwordValidation = password(t);

expect(await passwordValidation.password().validate("NO_LOWER")).toEqual(t("COMMON.VALIDATION.PASSWORD_WEAK"));
expect(pwnd).not.toHaveBeenCalled();
});

it("should require at least 1 uppercase character", async () => {
const { result } = renderHook(() => useTranslation());
const { t } = result.current;
const passwordValidation = password(t);

expect(await passwordValidation.password().validate("no_upper")).toEqual(t("COMMON.VALIDATION.PASSWORD_WEAK"));
expect(pwnd).not.toHaveBeenCalled();
});

it("should require different password than the old password", () => {
it("should require at least 1 numeric character", async () => {
const { result } = renderHook(() => useTranslation());
const { t } = result.current;
const passwordValidation = password(t);

expect(await passwordValidation.password().validate("NoNumeric")).toEqual(t("COMMON.VALIDATION.PASSWORD_WEAK"));
expect(pwnd).not.toHaveBeenCalled();
});

it("should require at least 1 special character", async () => {
const { result } = renderHook(() => useTranslation());
const { t } = result.current;
const passwordValidation = password(t);

expect(await passwordValidation.password().validate("N0SpecialChar5")).toEqual(
t("COMMON.VALIDATION.PASSWORD_WEAK"),
);
expect(pwnd).not.toHaveBeenCalled();
});

it("should be at least 8 characters long", async () => {
const { result } = renderHook(() => useTranslation());
const { t } = result.current;
const passwordValidation = password(t);

expect(await passwordValidation.password().validate("shortpw")).toEqual(t("COMMON.VALIDATION.PASSWORD_WEAK"));
expect(pwnd).not.toHaveBeenCalled();
});

it("should require the password not to have been leaked", async () => {
const { result } = renderHook(() => useTranslation());
const { t } = result.current;
const passwordValidation = password(t);

pwnd.mockImplementation(() => Promise.resolve(1));
expect(await passwordValidation.password().validate("S3cUr3!Pas#w0rd")).toEqual(
t("COMMON.VALIDATION.PASSWORD_LEAKED"),
);
expect(pwnd).toHaveBeenCalledWith("S3cUr3!Pas#w0rd");

pwnd.mockImplementation(() => Promise.resolve(0));
expect(await passwordValidation.password().validate("S3cUr3!Pas#w0rd")).toEqual(true);
expect(pwnd).toHaveBeenCalledWith("S3cUr3!Pas#w0rd");
});

it("should ignore leaked validation if haveibeenpwned API is unreachable", async () => {
const { result } = renderHook(() => useTranslation());
const { t } = result.current;
const passwordValidation = password(t);

pwnd.mockImplementation(() => Promise.reject());
expect(await passwordValidation.password().validate("S3cUr3!Pas#w0rd")).toEqual(true);
});

it("should require different password than the old password", async () => {
const { result } = renderHook(() => useTranslation());
const { t } = result.current;

const passwordRule = password(t).password("S3cUr3!Pas#w0rd");

const passwordRule = password(t);
const passwordValidation = passwordRule.password("password");
expect(passwordValidation.validate("password")).toEqual(t("COMMON.VALIDATION.PASSWORD_SAME_AS_OLD"));
expect(passwordValidation.validate("new password")).toEqual(true);
expect(await passwordRule.validate("S3cUr3!Pas#w0rd")).toEqual(t("COMMON.VALIDATION.PASSWORD_SAME_AS_OLD"));
expect(await passwordRule.validate("S3cUr3!Pas#w0rd2different")).toEqual(true);
});

it("should match password and confirm password", () => {
Expand Down
30 changes: 22 additions & 8 deletions src/app/validations/password.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,31 @@
import { pwnd, strong } from "password-pwnd";

export const password = (t: any) => ({
password: (currentPassword?: string) => ({
minLength: {
value: 6,
message: t("COMMON.VALIDATION.MIN_LENGTH", {
field: t("SETTINGS.GENERAL.PERSONAL.PASSWORD"),
minLength: 6,
}),
},
validate: (password: string) => {
validate: async (password: string) => {
if (!password) {
return true;
}

if (!!currentPassword && currentPassword === password) {
return t("COMMON.VALIDATION.PASSWORD_SAME_AS_OLD");
}

if (!(await strong(password))) {
return t("COMMON.VALIDATION.PASSWORD_WEAK");
}

try {
const hasBeenLeaked = await pwnd(password);

if (hasBeenLeaked) {
return t("COMMON.VALIDATION.PASSWORD_LEAKED");
}
} catch {
// API might be unreachable, ignore this validation.
return true;
}

return true;
},
}),
Expand Down
4 changes: 2 additions & 2 deletions src/domains/profile/e2e/create-profile-action.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ test("should create a profile with password and navigate to welcome screen", asy
await t.expect(getLocation()).contains("/profiles/create");

await t.typeText(nameInput, "Joe Bloggs");
await t.typeText(Selector("input[name=password]"), "password");
await t.typeText(Selector("input[name=confirmPassword]"), "password");
await t.typeText(Selector("input[name=password]"), "S3cUrePa$sword");
await t.typeText(Selector("input[name=confirmPassword]"), "S3cUrePa$sword");
await t.click('[data-testid="SelectDropdownInput__input"]');
await t.click('[data-testid="select-list__toggle-option-0"]');
await t.click(Selector("input[name=isDarkMode]").parent());
Expand Down
5 changes: 4 additions & 1 deletion src/domains/profile/hooks/use-profile-import.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,10 @@ describe("useProfileImport", () => {
const { result } = renderHook(() => useProfileImport({ env }));

await act(async () => {
const profile = await result.current.importProfile({ file: passwordProtectedDwe, password: "testtest" });
const profile = await result.current.importProfile({
file: passwordProtectedDwe,
password: "S3cUrePa$sword",
});
expect(profile?.name()).toEqual("test");
});
});
Expand Down
16 changes: 8 additions & 8 deletions src/domains/profile/pages/CreateProfile/CreateProfile.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -199,8 +199,8 @@ describe("CreateProfile", () => {
const { asFragment, getAllByTestId, getByTestId } = await renderComponent();

fireEvent.input(getAllByTestId("Input")[0], { target: { value: "test profile 3" } });
fireEvent.input(getAllByTestId("InputPassword")[0], { target: { value: "test password" } });
fireEvent.input(getAllByTestId("InputPassword")[1], { target: { value: "test password" } });
fireEvent.input(getAllByTestId("InputPassword")[0], { target: { value: "S3cUrePa$sword.test" } });
fireEvent.input(getAllByTestId("InputPassword")[1], { target: { value: "S3cUrePa$sword.test" } });

const selectDropdown = getByTestId("SelectDropdownInput__input");

Expand Down Expand Up @@ -229,18 +229,18 @@ describe("CreateProfile", () => {
fireEvent.change(selectDropdown, { target: { value: "BTC" } });
fireEvent.click(getByTestId("select-list__toggle-option-0"));

fireEvent.change(getAllByTestId("InputPassword")[0], { target: { value: "test password" } });
fireEvent.change(getAllByTestId("InputPassword")[1], { target: { value: "wrong" } });
fireEvent.change(getAllByTestId("InputPassword")[0], { target: { value: "S3cUrePa$sword.test" } });
fireEvent.change(getAllByTestId("InputPassword")[1], { target: { value: "S3cUrePa$sword.wrong" } });

await waitFor(() => expect(getByTestId("CreateProfile__submit-button")).toHaveAttribute("disabled"));

fireEvent.input(getAllByTestId("InputPassword")[0], { target: { value: "password" } });
fireEvent.input(getAllByTestId("InputPassword")[1], { target: { value: "password" } });
fireEvent.input(getAllByTestId("InputPassword")[0], { target: { value: "S3cUrePa$sword" } });
fireEvent.input(getAllByTestId("InputPassword")[1], { target: { value: "S3cUrePa$sword" } });

await waitFor(() => expect(getByTestId("CreateProfile__submit-button")).not.toHaveAttribute("disabled"));

fireEvent.input(getAllByTestId("InputPassword")[1], { target: { value: "test password" } });
fireEvent.input(getAllByTestId("InputPassword")[0], { target: { value: "wrong" } });
fireEvent.input(getAllByTestId("InputPassword")[1], { target: { value: "S3cUrePa$sword.test" } });
fireEvent.input(getAllByTestId("InputPassword")[0], { target: { value: "S3cUrePa$sword.wrong" } });

await waitFor(() => expect(getByTestId("CreateProfile__submit-button")).toHaveAttribute("disabled"));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ exports[`CreateProfile should fail password confirmation 1`] = `
</div>
</div>
<fieldset
class="FormField__FormFieldStyled-yeauwd-0 qzrKg flex flex-col space-y-2"
class="FormField__FormFieldStyled-yeauwd-0 cnlnjc flex flex-col space-y-2"
>
<label
class="flex mb-2 text-sm font-semibold transition-colors duration-100 FormLabel text-theme-secondary-text"
Expand All @@ -167,38 +167,23 @@ exports[`CreateProfile should fail password confirmation 1`] = `
</span>
</label>
<div
class="Input__InputWrapperStyled-wu99qs-0 jXtMoQ"
class="Input__InputWrapperStyled-wu99qs-0 jYUFQr"
>
<div
class="relative flex flex-1 h-full"
>
<input
aria-invalid="true"
class="Input__InputStyled-wu99qs-1 ioFtLo p-0 border-none bg-transparent focus:ring-0 no-ligatures w-full"
data-testid="InputPassword"
name="password"
type="password"
/>
</div>
<div
class="flex items-center space-x-3 divide-x divide-theme-secondary-300 dark:divide-theme-secondary-800 text-theme-danger-500"
class="flex items-center space-x-3 divide-x divide-theme-secondary-300 dark:divide-theme-secondary-800 text-theme-primary-300 dark:text-theme-secondary-600"
>
<span
data-errortext="Password should have at least 6 characters"
data-testid="Input__error"
>
<div
class="sc-bdfBwQ iyevwF text-theme-danger-500"
height="20"
width="20"
>
<svg>
alert-warning.svg
</svg>
</div>
</span>
<div
class="pl-3"
class=""
>
<button
class="relative flex items-center justify-center w-full h-full text-2xl focus:outline-none group"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ describe("ImportProfile", () => {
await waitFor(() => expect(getByTestId("modal__inner")).toBeInTheDocument());

await act(async () => {
fireEvent.input(getByTestId("PasswordModal__input"), { target: { value: "testtest" } });
fireEvent.input(getByTestId("PasswordModal__input"), { target: { value: "S3cUrePa$sword" } });
});

// wait for formState.isValid to be updated
Expand Down Expand Up @@ -200,7 +200,7 @@ describe("ImportProfile", () => {
await waitFor(() => expect(getByTestId("modal__inner")).toBeInTheDocument());

await act(async () => {
fireEvent.input(getByTestId("PasswordModal__input"), { target: { value: "testtest" } });
fireEvent.input(getByTestId("PasswordModal__input"), { target: { value: "S3cUrePa$sword" } });
});

act(() => {
Expand Down Expand Up @@ -242,7 +242,7 @@ describe("ImportProfile", () => {
await waitFor(() => expect(getByTestId("modal__inner")).toBeInTheDocument());

await act(async () => {
fireEvent.input(getByTestId("PasswordModal__input"), { target: { value: "testtest" } });
fireEvent.input(getByTestId("PasswordModal__input"), { target: { value: "S3cUrePa$sword" } });
});

act(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ describe("Import Profile - Processing import", () => {
await waitFor(() => expect(getByTestId("modal__inner")).toBeInTheDocument());

act(() => {
fireEvent.input(getByTestId("PasswordModal__input"), { target: { value: "testtest" } });
fireEvent.input(getByTestId("PasswordModal__input"), { target: { value: "S3cUrePa$sword" } });
});

await findByTestId("PasswordModal__submit-button");
Expand All @@ -66,7 +66,7 @@ describe("Import Profile - Processing import", () => {
});

await waitFor(() => expect(() => getByTestId("modal__inner")).toThrow());
await waitFor(() => expect(onPasswordChange).toHaveBeenCalledWith("testtest"));
await waitFor(() => expect(onPasswordChange).toHaveBeenCalledWith("S3cUrePa$sword"));
expect(container).toMatchSnapshot();
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,8 @@ describe("Import Profile - Profile Form Step", () => {
fireEvent.click(container.querySelector("input[name=isDarkMode]"));

await act(async () => {
fireEvent.change(getAllByTestId("InputPassword")[0], { target: { value: "password" } });
fireEvent.change(getAllByTestId("InputPassword")[1], { target: { value: "password" } });
fireEvent.change(getAllByTestId("InputPassword")[0], { target: { value: "S3cUrePa$sword" } });
fireEvent.change(getAllByTestId("InputPassword")[1], { target: { value: "S3cUrePa$sword" } });
});

await act(async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ exports[`Import Profile - Profile Form Step should fail password confirmation 1`
class="flex items-center space-x-3 divide-x divide-theme-secondary-300 dark:divide-theme-secondary-800 text-theme-danger-500"
>
<span
data-errortext="Password should have at least 6 characters"
data-errortext="The password must contain at least 1 lowercase character, 1 uppercase character, 1 numeric character, 1 special character and must be 8 characters or longer"
data-testid="Input__error"
>
<div
Expand Down
Loading

0 comments on commit 6f2a6d0

Please sign in to comment.