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

Added a Passwort Strength Component for User Registration #19355

Draft
wants to merge 10 commits into
base: dev
Choose a base branch
from

Conversation

Csaern
Copy link

@Csaern Csaern commented Dec 19, 2024

As a project I made a password strength component to visualize the strength of a password used to create an account for the galaxy server. The user gets feedback at the registration site, for the strength of his password and also an estimation time, how long it would take to get cracked.
I also added

  • the eye functionality, to show and hide the input for the password
  • a button for password guidelines, to show users, what components they need to get a strong password, which includes an reference link for an external website.

Password_1
Password_2
Password_Modal

How to test the changes?

(Select all options that apply)

  • I've included appropriate automated tests.
  • This is a refactoring of components with existing test coverage.
  • Instructions for manual testing are as follows:
    1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

  • I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

@nsoranzo
Copy link
Member

Thanks for working on this feature! Two suggestions:

  • Add the "eye" functionality to the password field to allow the uses to view what they are typing if it's safe to do so
  • Replace the Password Guidelines button with a simple help text containing the few rules we actually enforce and a link to general-purpose guidelines like https://www.cisa.gov/secure-our-world/use-strong-passwords

@guerler
Copy link
Contributor

guerler commented Dec 19, 2024

I agree with @nsoranzo, and would additionally suggest that you could add a checkmark to each fulfilled criteria listed in the help section. Thank you for working on this!

@Csaern Csaern force-pushed the password branch 2 times, most recently from ccfe6d5 to 44f4e95 Compare January 14, 2025 11:25
@Csaern
Copy link
Author

Csaern commented Jan 14, 2025

Password Component with check marker
Password Guidelines with Reference

@guerler
Copy link
Contributor

guerler commented Jan 21, 2025

I think this might work better without the guidelines button and modal, since similar information is already conveyed through the strength indicator and the list of criteria.

@ElectronicBlueberry
Copy link
Member

since similar information is already conveyed through the strength indicator and the list of criteria

the list of criteria is from an old screenshot. They were removed, since the only criteria we enforce is a length of at least 6 characters.

@guerler
Copy link
Contributor

guerler commented Jan 21, 2025

Ah interesting. Thanks. I still think that the modal should be removed. Having a button and a modal just to display criteria we don't apply seems to be unnecessary. In my opinion, we should either extend the required criteria list or just show the strength bar with some help text under it. The crack time also seems to be misleading and not informative since we can not look 10-20 years into the future.

@ElectronicBlueberry
Copy link
Member

The time estimation is to give people another metric for the strength bar. Seeing that your password can be cracked within a about minute of computing time can be more effective means to motivate people to use a stronger password, than simply displaying "weak".

Regarding the foresight: The strength estimation could also be off in 10-20 years, but I'm trusting we will update our dependencies before it deviates a significant amount.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants