Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Password Settings should show password strength indicator instead of password length #1857

Closed
bsclifton opened this issue May 21, 2016 · 9 comments
Labels
addressed-with-brave-core Needs confirmation, but this issue may be resolved with Brave Core. design A design change, especially one which needs input from the design team. priority/P5 Cosmetic. Spelling, copy, layout. New features (which should also be part of an initiative). sec-low security suggestion wontfix

Comments

@bsclifton
Copy link
Member

Did you search for similar issues before submitting this one?
Yes

Describe the issue you encountered:
Brave's password manager shows all saved user/passwords for each site using asterisks. However, the actual number of asterisks is the actual length of the real password.

Expected behavior:
The password field should either not be shown OR password should be a fixed # of asterisks (so people standing behind a user don't know the exact number of chars for a password).

  • Platform (Win7, 8, 10? OS X? Linux distro?):
    All versions
  • Brave Version:
    All versions
  • Steps to reproduce:
    1. Make sure you have some user/passwords saved
    2. Go to about:passwords
    3. Notice that the hidden passwords have the same number of asterisks as the actual password.
  • Screenshot if needed:
  • Any related issues:
@bsclifton
Copy link
Member Author

My personal preference would be to not show any asterisks and instead show a strength bar (weak / medium / strong). If that was something we could do, I'm not sure how the UX would look cc: @bradleyrichter

If we wanted to go the extra mile, having a check for each password (against other stored passwords) and flagging passwords which are duplicate as weak or another status like "DANGEROUS" would be ideal.

@darkyen
Copy link

darkyen commented May 23, 2016

@bsclifton How about input[type=password] being prefilled with asterisked "Password" as placeholder. If the field is not-mutated load and submit the password. With a strength bar at the bottom.

@diracdeltas
Copy link
Member

It's showing the length of the encrypted password. I had thought that for AES-GCM that would always be the same as the length of the plaintext password, but I guess not (due to encoding normalization or something?).

Would be easy to switch to fixed-length asterisks. Strength bar is doable too via something like https://github.com/dropbox/zxcvbn

@bsclifton
Copy link
Member Author

bsclifton commented May 23, 2016

@diracdeltas ah- this is good to know. I only checked a few of my PWs and the encrypted version just happened to be the same length as the plain text

That PW strength estimator project you linked to is amazing (how have I not seen that?). For people not wanting to use 1Password, Dashlane, etc showing a strength bar using the logic from that project would be a really nice feature 😄

@bsclifton bsclifton added the design A design change, especially one which needs input from the design team. label May 23, 2016
@diracdeltas
Copy link
Member

diracdeltas commented May 23, 2016

@bsclifton sorry, i misread this bug and thought you meant the opposite of what you actually meant. IMO there is not much point to obfuscating the PW length since we are using a password encryption algorithm that preserves the length (it's actually showing length of encrypted password). i guess it helps very slightly against over-the-shoulder attacks.

changing bug title to be about password strength indicator which would be a nice enhancement regardless

@diracdeltas diracdeltas changed the title Password Settings should not show actual PW length Password Settings should show password strength indicator instead of password length May 23, 2016
@cezaraugusto cezaraugusto added this to the Triage Backlog milestone Nov 8, 2017
@MargarytaChepiga
Copy link
Contributor

If no one is working on it, I would love to give it a try 😄

@diracdeltas
Copy link
Member

@MargarytaChepiga i think this is low priority since we are moving away from using our password manager page. however, if you do want to try this, i would use https://github.com/dropbox/zxcvbn for strength indication

@MargarytaChepiga
Copy link
Contributor

I will work on something else then. Thanks for letting me know @diracdeltas!

@bsclifton bsclifton added the priority/P5 Cosmetic. Spelling, copy, layout. New features (which should also be part of an initiative). label Mar 28, 2018
@bsclifton bsclifton modified the milestones: Triage Backlog, Backlog (Prioritized) Mar 28, 2018
@bsclifton bsclifton removed this from the Backlog (Prioritized) milestone Jul 22, 2018
@bsclifton
Copy link
Member Author

Closing as wontfix, since this won't be an issue with Brave Core (and we won't be able to address before then)

@bsclifton bsclifton added the addressed-with-brave-core Needs confirmation, but this issue may be resolved with Brave Core. label Jul 22, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
addressed-with-brave-core Needs confirmation, but this issue may be resolved with Brave Core. design A design change, especially one which needs input from the design team. priority/P5 Cosmetic. Spelling, copy, layout. New features (which should also be part of an initiative). sec-low security suggestion wontfix
Projects
None yet
Development

No branches or pull requests

6 participants