Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add password strength meter #1949

Merged
merged 8 commits into from
Oct 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions apps/desktop-e2e/src/features/multisig.feature
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
Feature: Multisig Account Creation

Scenario: User creates a multisig account
Given I have account
| type | label | secretKey | password |
| secret_key | Alice | edsk3QoqBuvdamxouPhin7swCvkQNgq4jP5KZPbwWNnwdZpSpJiEbq | 12345678 |
| type | label | secretKey | password |
| secret_key | Alice | edsk3QoqBuvdamxouPhin7swCvkQNgq4jP5KZPbwWNnwdZpSpJiEbq | Qwerty123123!23vcxz |
And I am on an Accounts page

When I am creating a multisig account
Expand All @@ -25,7 +25,7 @@
| Approvers | Alice,tz1gUNyn3hmnEWqkusWPzxRaon1cs7ndWh7h |
| Min No. of approvals | 1 |

When I sign transaction with password "12345678"
When I sign transaction with password "Qwerty123123!23vcxz"
Then I see "Operation Submitted" modal

And I close modal
Expand Down
16 changes: 8 additions & 8 deletions apps/desktop-e2e/src/features/onboarding.feature
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
Feature: User Onboarding

Scenario: User creates a new seed phrase account
Expand Down Expand Up @@ -33,8 +33,8 @@
When I click "Continue" button
Then I am on "Umami Master Password" onboarding page

When I fill "Password" with "12345678"
And I fill "Confirm Password" with "12345678"
When I fill "Password" with "Qwerty123123!23vcxz"
And I fill "Confirm Password" with "Qwerty123123!23vcxz"
And I click "Submit" button
Then I am on an Accounts page
And I see a toast "Account successfully created!"
Expand Down Expand Up @@ -76,8 +76,8 @@
When I click "Continue" button
Then I am on "Umami Master Password" onboarding page

When I fill "Password" with "12345678"
And I fill "Confirm Password" with "12345678"
When I fill "Password" with "Qwerty123123!23vcxz"
And I fill "Confirm Password" with "Qwerty123123!23vcxz"
And I click "Submit" button
Then I am on an Accounts page
And I see a toast "Account successfully created!"
Expand Down Expand Up @@ -119,8 +119,8 @@
When I click "Continue" button
Then I am on "Umami Master Password" onboarding page

When I fill "Password" with "12345678"
And I fill "Confirm Password" with "12345678"
When I fill "Password" with "Qwerty123123!23vcxz"
And I fill "Confirm Password" with "Qwerty123123!23vcxz"
And I click "Submit" button
Then I am on an Accounts page
And I see a toast "Account successfully created!"
Expand Down Expand Up @@ -158,8 +158,8 @@
When I click "Continue" button
Then I am on "Umami Master Password" onboarding page

When I fill "Password" with "12345678"
And I fill "Confirm Password" with "12345678"
When I fill "Password" with "Qwerty123123!23vcxz"
And I fill "Confirm Password" with "Qwerty123123!23vcxz"
And I click "Submit" button
Then I am on an Accounts page
And I see a toast "Account successfully created!"
Expand Down
12 changes: 6 additions & 6 deletions apps/desktop-e2e/src/features/staking.feature
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
Feature: Staking

# TODO:
Expand All @@ -8,8 +8,8 @@
# stake - unstake - finalize unstake
Scenario: I delegate to a baker
Given I have account
| type | label | secretKey | password |
| secret_key | Alice | edsk3QoqBuvdamxouPhin7swCvkQNgq4jP5KZPbwWNnwdZpSpJiEbq | 12345678 |
| type | label | secretKey | password |
| secret_key | Alice | edsk3QoqBuvdamxouPhin7swCvkQNgq4jP5KZPbwWNnwdZpSpJiEbq | Qwerty123123!23vcxz |
And I am on an Accounts page
When I open account drawer for "Alice"
And I delegate to "baker1"
Expand All @@ -23,8 +23,8 @@

Scenario: I undelegate
Given I have account
| type | label | secretKey | password |
| secret_key | Alice | edsk3QoqBuvdamxouPhin7swCvkQNgq4jP5KZPbwWNnwdZpSpJiEbq | 12345678 |
| type | label | secretKey | password |
| secret_key | Alice | edsk3QoqBuvdamxouPhin7swCvkQNgq4jP5KZPbwWNnwdZpSpJiEbq | Qwerty123123!23vcxz |
And I am on an Accounts page
When I open account drawer for "Alice"
And I delegate to "baker1"
Expand All @@ -44,8 +44,8 @@

Scenario: I stake to a baker
Given I have account
| type | label | secretKey | password |
| secret_key | Alice | edsk3QoqBuvdamxouPhin7swCvkQNgq4jP5KZPbwWNnwdZpSpJiEbq | 12345678 |
| type | label | secretKey | password |
| secret_key | Alice | edsk3QoqBuvdamxouPhin7swCvkQNgq4jP5KZPbwWNnwdZpSpJiEbq | Qwerty123123!23vcxz |
And I am on an Accounts page
Given baker "baker1" sets delegation parameters
| limit-of-staking-over-baking | 5 |
Expand Down
4 changes: 2 additions & 2 deletions apps/desktop-e2e/src/features/updates.feature
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
Feature: Automatic updates

Scenario: My account has been topped-up
Given I have account
| type | label | password | secretKey |
| secret_key | Account 1 | 12345678 | spsk2jm29sHC99HDi64VBpSwEMZRQ7WfHdvQPVMZCkyWyR4spBrtRW |
| type | label | password | secretKey |
| secret_key | Account 1 | Qwerty123123!23vcxz | spsk2jm29sHC99HDi64VBpSwEMZRQ7WfHdvQPVMZCkyWyR4spBrtRW |
And I am on an Accounts page
When "Account 1" is topped-up with "1.123"
And I wait until the next refetch
Expand Down
6 changes: 3 additions & 3 deletions apps/desktop-e2e/src/pages/AccountDrawerPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export class AccountDrawerPage {

await this.page.getByRole("button", { name: "Preview" }).click();

await new SignPage(this.page, "12345678").sign();
await new SignPage(this.page, "Qwerty123123!23vcxz").sign();
}

getRoundButton(buttonName: string): Locator {
Expand All @@ -46,7 +46,7 @@ export class AccountDrawerPage {

await this.page.getByRole("button", { name: "Preview" }).click();

await new SignPage(this.page, "12345678").sign();
await new SignPage(this.page, "Qwerty123123!23vcxz").sign();
}

async stake(amount: number): Promise<void> {
Expand All @@ -59,7 +59,7 @@ export class AccountDrawerPage {

await this.page.getByLabel("Enter Amount").fill(String(amount));
await this.page.getByRole("button", { name: "Preview" }).click();
await new SignPage(this.page, "12345678").sign();
await new SignPage(this.page, "Qwerty123123!23vcxz").sign();
}

async stakedBalance(): Promise<number> {
Expand Down
2 changes: 1 addition & 1 deletion apps/desktop/jest.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,5 @@ export default {
rootDir: "./",
testTimeout: 20000,
bail: false,
setupFilesAfterEnv: ["<rootDir>/src/setupTests.ts"],
setupFilesAfterEnv: ["<rootDir>/src/setupTests.tsx"],
};
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ jest.mock("@umami/state", () => ({
}));

const MOCK_TEZOS_TOOLKIT = {};
const password = "Qwerty123123!23vcxz";
let store: UmamiStore;

beforeEach(() => {
Expand Down Expand Up @@ -124,7 +125,7 @@ describe("<MultisigPendingOperation />", () => {

expect(jest.mocked(estimate)).toHaveBeenCalledWith(operation, MAINNET);

await act(() => user.type(screen.getByTestId("password"), "mockPass"));
await act(() => user.type(screen.getByTestId("password"), password));

const submitButton = screen.getByRole("button", {
name: "Execute transaction",
Expand Down Expand Up @@ -187,7 +188,7 @@ describe("<MultisigPendingOperation />", () => {

expect(jest.mocked(estimate)).toHaveBeenCalledWith(operations, MAINNET);

await act(() => user.type(screen.getByTestId("password"), "mockPass"));
await act(() => user.type(screen.getByTestId("password"), password));

const submitButton = screen.getByRole("button", {
name: "Approve transaction",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ describe("ChangePassword Form", () => {
});
});

it("requires 8 characters", async () => {
it("requires 12 characters", async () => {
render(fixture());

const newPasswordInput = screen.getByTestId("new-password");
Expand All @@ -55,7 +55,7 @@ describe("ChangePassword Form", () => {

await waitFor(() => {
expect(screen.getByTestId("new-password-error")).toHaveTextContent(
"Your password must be at least 8 characters long"
"Password must be at least 12 characters long"
);
});
});
Expand Down Expand Up @@ -104,8 +104,8 @@ describe("ChangePassword Form", () => {
const newPasswordConfirmationInput = screen.getByTestId("new-password-confirmation");

fireEvent.change(currentPasswordInput, { target: { value: "myOldPassword" } });
fireEvent.change(newPasswordInput, { target: { value: "myNewPassword" } });
fireEvent.change(newPasswordConfirmationInput, { target: { value: "myNewPassword" } });
fireEvent.change(newPasswordInput, { target: { value: "myNewPassword123L!" } });
fireEvent.change(newPasswordConfirmationInput, { target: { value: "myNewPassword123L!" } });

await waitFor(() => {
expect(screen.getByRole("button", { name: "Update Password" })).toBeEnabled();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ type ChangePasswordFormValues = {

export const ChangePasswordForm = () => {
const { onClose } = useDynamicModalContext();
const form = useForm<ChangePasswordFormValues>({ mode: "onBlur" });
const form = useForm<ChangePasswordFormValues>({ mode: "all" });
const toast = useToast();
const dispatch = useAppDispatch();
const { handleAsyncAction, isLoading } = useAsyncActionHandler();
Expand Down Expand Up @@ -80,6 +80,7 @@ export const ChangePasswordForm = () => {
<PasswordInput
data-testid="new-password"
inputName="newPassword"
isStrengthCheckEnabled
label="New Password"
placeholder="Enter new password"
required="New password is required"
Expand Down
2 changes: 1 addition & 1 deletion apps/desktop/src/components/FormErrorMessage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { ExclamationIcon } from "../assets/icons";
import colors from "../style/colors";

export const FormErrorMessage = ({ children, ...props }: FormErrorMessageProps) => (
<OriginalFormErrorMessage color={colors.orange} fontSize="12px" {...props}>
<OriginalFormErrorMessage color={colors.orange} fontSize="14px" {...props}>
<Icon as={ExclamationIcon} marginRight="6px" />
{children}
</OriginalFormErrorMessage>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import { noop } from "lodash";
import { EnterAndConfirmPassword } from "./EnterAndConfirmPassword";
import { act, render, screen, userEvent } from "../../../../mocks/testUtils";

const mockPassword = "Qwerty123123!23vcxz";

const fixture = (isLoading: boolean) => (
<EnterAndConfirmPassword isLoading={isLoading} onSubmit={noop} />
);
Expand All @@ -24,17 +26,12 @@ describe("<EnterAndConfirmPassword />", () => {
describe("Form", () => {
test("Working verification", async () => {
render(fixture(false));
await checkPasswords("password", "password", true);
await checkPasswords(mockPassword, mockPassword, true);
});

test("Not matching password", async () => {
render(fixture(false));
await checkPasswords("password", "password1", false);
});

test("Not meeting password policy", async () => {
render(fixture(false));
await checkPasswords("tes", "tes", false);
await checkPasswords(mockPassword, "password1", false);
});

test("Form is loading", () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export const EnterAndConfirmPassword = ({
};

const form = useForm<ConfirmPasswordFormValues>({
mode: "onBlur",
mode: "onChange",
});

const {
Expand All @@ -42,6 +42,7 @@ export const EnterAndConfirmPassword = ({
<PasswordInput
data-testid="password"
inputName="password"
isStrengthCheckEnabled
placeholder="Enter master password"
/>
{errors.password && <FormErrorMessage>{errors.password.message}</FormErrorMessage>}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import { noop } from "lodash";
import { EnterPassword } from "./EnterPassword";
import { act, render, screen, userEvent } from "../../../../mocks/testUtils";

const mockPassword = "Qwerty123123!23vcxz";

const fixture = (isLoading: boolean) => <EnterPassword isLoading={isLoading} onSubmit={noop} />;

const checkPasswords = async (password: string, expected: boolean) => {
Expand All @@ -19,12 +21,7 @@ describe("<EnterPassword />", () => {
describe("Form", () => {
test("Working verification", async () => {
render(fixture(false));
await checkPasswords("password", true);
});

test("Not meeting password policy", async () => {
render(fixture(false));
await checkPasswords("tes", false);
await checkPasswords(mockPassword, true);
});

test("Form is loading", () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export const RestoreSecretKey = ({ goToStep }: { goToStep: (step: OnboardingStep

{isEncrypted && (
<FormControl marginTop="20px" isInvalid={!!errors.password}>
<PasswordInput data-testid="password" inputName="password" minLength={0} />
<PasswordInput data-testid="password" inputName="password" />
{errors.password && <FormErrorMessage>{errors.password.message}</FormErrorMessage>}
</FormControl>
)}
Expand Down
39 changes: 27 additions & 12 deletions apps/desktop/src/components/PasswordInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,22 +6,23 @@ import {
type InputProps,
InputRightElement,
} from "@chakra-ui/react";
import { DEFAULT_MIN_LENGTH, usePasswordValidation } from "@umami/components";
import { useState } from "react";
import { type FieldValues, type Path, type RegisterOptions, useFormContext } from "react-hook-form";

import { EyeIcon, EyeSlashIcon } from "../assets/icons";

const MIN_LENGTH = 8;
import colors from "../style/colors";

// <T extends FieldValues> is needed to be compatible with the useForm's type parameter (FormData)
// <U extends Path<T>> makes sure that we can pass in only valid inputName that exists in FormData
type PasswordInputProps<T extends FieldValues, U extends Path<T>> = {
inputName: U;
label?: string;
placeholder?: string;
isStrengthCheckEnabled?: boolean;
required?: string | boolean;
minLength?: RegisterOptions<T, U>["minLength"];
validate?: RegisterOptions<T, U>["validate"];
validate?: (val: string) => string | boolean;
} & InputProps;

// TODO: add an error message and make it nested under FormControl
Expand All @@ -30,12 +31,32 @@ export const PasswordInput = <T extends FieldValues, U extends Path<T>>({
label = "Password",
placeholder = "Enter your password",
required = "Password is required",
minLength = MIN_LENGTH,
isStrengthCheckEnabled = false,
minLength = DEFAULT_MIN_LENGTH,
validate,
...rest
}: PasswordInputProps<T, U>) => {
const { register } = useFormContext<T>();
const [showPassword, setShowPassword] = useState<boolean>(false);
const { validatePasswordStrength, PasswordStrengthBar } = usePasswordValidation({
color: colors.gray[500],
inputName,
minLength,
});

const handleValidate = (val: string) => {
if (validate) {
const validationResult = validate(val);

if (isStrengthCheckEnabled && validationResult === true) {
return validatePasswordStrength(val);
}

return validationResult;
} else if (isStrengthCheckEnabled) {
return validatePasswordStrength(val);
}
};

return (
<>
Expand All @@ -48,14 +69,7 @@ export const PasswordInput = <T extends FieldValues, U extends Path<T>>({
type={showPassword ? "text" : "password"}
{...register(inputName, {
required,
minLength:
minLength && required
? {
value: minLength,
message: `Your password must be at least ${minLength} characters long`,
}
: undefined,
validate,
validate: handleValidate,
})}
{...rest}
/>
Expand All @@ -69,6 +83,7 @@ export const PasswordInput = <T extends FieldValues, U extends Path<T>>({
</Button>
</InputRightElement>
</InputGroup>
{isStrengthCheckEnabled && PasswordStrengthBar}
</>
);
};
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ export const MasterPasswordModal = ({ onSubmit }: MasterPasswordModalProps) => {
inputName="password"
label="Password"
placeholder="Enter your password"
required="Password is required"
asiia-trilitech marked this conversation as resolved.
Show resolved Hide resolved
/>
</ModalBody>
<ModalFooter>
Expand Down
Loading