keepassium / KeePassium

KeePass-compatible password manager for iOS
https://keepassium.com
Other
1.21k stars 103 forks source link

Randomizer should support both optional and mandatory character sets #207

Closed divarre closed 2 years ago

divarre commented 2 years ago

Description When adding a new entry and using the Randomizer, and “Digits (123…)” is selected in the list of parameters, then I expect digits to be always included in the generated passwords, but the Randomizer sometimes generates passwords without any digits, which does not verify the parameters I selected.

How to reproduce Steps to reproduce the behavior:

  1. Open a database
  2. Tap the “+” in the top right corner and select “Create Entry”
  3. Next to the “Password” field, click “Randomize”
  4. In the Randomizer, enable “Digits (123…)” if it's not already enabled
  5. Tap on the circle arrow icon in the top bar to generate new passwords
  6. Notice that some generated passwords do not contain any digits

Video/gif:

No digits

Expected behavior Newly generated passwords verify all the conditions set in the Randomizer, and they include at least one digit when “Digits (123…) is selected.”

Screenshots

No digits

Also, on the App Store, one of the screenshots of the Randomizer shows no digits despite “Digits (123…)” being selected. This could give the impression there's something wrong before a new user even downloads the app, and it does absolutely not represent the very high quality of the rest of the app :)

No digits app store

Environment:

Additional context The password length does not seem to affect this. Tested and replicated with a password length of 15, 20, 25. It also happened with the previous version.

Thanks!

Thunder33345 commented 2 years ago

Can reproduce, v1.28.99

Seems like the randomizer simply treats checked entries as "include this character set as a possible result", but not as a requirement that it must appear at least once, this can be reproduced with others selections too, if you have a short enough digits(8)

Maybe silently regenerate the result, if at least one of the selected sets is not present could be a possible solution

craigo- commented 2 years ago

I'm guessing that including a "must have" component reduces the entropy of the result.

But yes, there are plenty of examples of systems that require certain character elements to be present.

Suggestion: change the current two-state untick/tick system to a three-state system:

It makes sense to me to make the last option the default.

keepassium commented 2 years ago

Thanks, everyone!

I can confirm that selected character sets are currently treated as "possible", but not "mandatory". So the described behavior is not a bug, it's pretty much by design. That said, password generator came unchanged straight from 2018, so its "by design" is long-due for revision (#78, #86, #160).

I like the three-state suggestion by @craigo-, it sounds flexible enough to cover most of use cases. Stay tuned :)