feat: integrate `zxcvbn-ts` for password strength evaluation #308

belhajManel · 2024-12-22T10:46:43Z

What changed?

Resolves #287
Add password strength evaluation during registration and password reset.

2024-12-22-11-17-40.mp4

Why?

Limit account phishing by requiring a minimum password score when creating an account or reseting a password.

How was this change made?

Integrated zxcvbn-ts for password strength estimation
Created a custom usePasswordStrength hook to use in different components

How was this tested?

👀

…in and reset password forms

changeset-bot · 2024-12-22T10:46:47Z

⚠️ No Changeset found

Latest commit: fc44846

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

belhajManel · 2024-12-22T11:08:42Z

src/components/app/PasswordStrength/PasswordStrength.tsx

        <Banner variant="info">
-          <p>
+          <span>


I made this tiny change because it will cause a hydration error

evadecker

Yay! This rocks. 🔥🔥🔥

First: Can we tighten up the margin here between the input and strength meter?

Second: seeing this in action, the messages appearing and disappearing while typing is distracting. I wonder if we can update the PasswordStrength component to:

Display a circle-help icon to the right of the label when suggestions or a warning exists
Open a Popover with the existing banners
Popover should display on the right when there's available space, otherwise on the bottom

Third: Can we update the PasswordStrength component to prevent the label from re-animating if the strength does not change?

CleanShot.2024-12-22.at.10.56.39.mp4

Let me know if you have any thoughts/questions/recommended improvements for the design or implementation!

evadecker · 2024-12-22T15:20:00Z

src/utils/usePasswordStrength.ts

+  zxcvbnOptions.setOptions(options);
+});
+
+export function usePasswordStrength(password: string) {


Love the creation of a hook for this!

evadecker · 2024-12-22T15:22:08Z

src/utils/usePasswordStrength.ts

+const loadOptions = async () => {
+  const zxcvbnCommonPackage = await import(
+    /* webpackChunkName: "zxcvbnCommonPackage" */ "@zxcvbn-ts/language-common"
+  );
+  const zxcvbnEnPackage = await import(
+    /* webpackChunkName: "zxcvbnEnPackage" */ "@zxcvbn-ts/language-en"
+  );
+  return {
+    dictionary: {
+      ...zxcvbnCommonPackage.dictionary,
+      ...zxcvbnEnPackage.dictionary,
+    },
+    translations: zxcvbnEnPackage.translations,
+  };
+};


Thanks for adding lazy-loading!

evadecker · 2024-12-22T15:24:42Z

src/routes/_unauthenticated/signin.tsx

+      <Button
+        type="submit"
+        variant="primary"
+        isDisabled={isSubmitting || (passwordState && passwordState?.score < 3)}


Instead of disabling the button entirely, which can cause accessibility issues, can we instead update our form submission handler to set the error when the score is < 3?

It might also be nice to pull the submission handler out into a handleSubmit function for readability.

evadecker · 2024-12-22T15:25:28Z

src/routes/_unauthenticated/signin.tsx

+              isDisabled={
+                isSubmitting || (passwordState && passwordState.score < 3)
+              }


Let's display an error instead of disabling the button (see other comment)

evadecker · 2024-12-22T16:45:33Z

src/routes/_unauthenticated/signin.tsx

@@ -137,7 +140,20 @@ const SignIn = () => {
              value={password}
              onChange={setPassword}
            />
-            <Button type="submit" isDisabled={isSubmitting} variant="primary">
+            {password && passwordState && (


One more idea:

Can we change the api of PasswordStrength to just accept one prop for password, instead of value and warning and suggestions? And then handle all logic for display inside of the component? That way we don't have to check ?? undefined or nest it within a conditional statement wherever it's used, and we can invoke the hook from within the component itself.

Can we display the strength meter even when the password is empty, but hide the label? To prevent reflowing the page when a user begins typing.

Definitely!
For the 2nd point when the password is empty do we provide the help icon or is it hidden with the label? It can have initial suggestions for the pwd structure but idk if that really helps?

Does the library return suggestions if the password is empty?

Yep it does return 2 suggestions

Yeah, let's try leaving it in even without a label.

belhajManel · 2024-12-22T19:45:19Z

The Popover idea is neat⚡ I'll make the adjustments.

feat: integrate zxcvbn-ts for password strength evaluation in sign-…

fc44846

…in and reset password forms

github-actions bot added dependencies Pull requests that update a dependency file frontend CSS, HTML, and JS changes labels Dec 22, 2024

belhajManel added 2 commits December 22, 2024 11:57

fix: formatting issue

f060290

feat: add zxcvbn dictionaries lazy loading to improve performance

aeb3772

belhajManel commented Dec 22, 2024

View reviewed changes

evadecker reviewed Dec 22, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: integrate `zxcvbn-ts` for password strength evaluation #308

feat: integrate `zxcvbn-ts` for password strength evaluation #308

belhajManel commented Dec 22, 2024

changeset-bot bot commented Dec 22, 2024

belhajManel Dec 22, 2024

evadecker left a comment •

edited

Loading

evadecker Dec 22, 2024

evadecker Dec 22, 2024

evadecker Dec 22, 2024

evadecker Dec 22, 2024

evadecker Dec 22, 2024 •

edited

Loading

belhajManel Dec 22, 2024

evadecker Dec 22, 2024

belhajManel Dec 23, 2024

evadecker Dec 23, 2024

belhajManel commented Dec 22, 2024

feat: integrate zxcvbn-ts for password strength evaluation #308

Are you sure you want to change the base?

feat: integrate zxcvbn-ts for password strength evaluation #308

Conversation

belhajManel commented Dec 22, 2024

What changed?

Why?

How was this change made?

How was this tested?

changeset-bot bot commented Dec 22, 2024

⚠️ No Changeset found

Choose a reason for hiding this comment

evadecker left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evadecker Dec 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

belhajManel commented Dec 22, 2024

feat: integrate `zxcvbn-ts` for password strength evaluation #308

feat: integrate `zxcvbn-ts` for password strength evaluation #308

evadecker left a comment •

edited

Loading

evadecker Dec 22, 2024 •

edited

Loading