Add password strength indicator to password entry fields #7885

jmdana · 2022-04-13T10:31:28Z

Fixes #7437 (partially, entry edit view only)
Fixes #5220

This change adds a password strength indicator to PasswordEditWidget (I have followed the way it is done in PasswordGeneratorWidget).

I want to contribute some code to the project and thought that this could be a good "first issue". Please be gentle ;)

Screenshots

Testing strategy

Testing was quite straightforward. As far as I know, the screenshots above are the only two places where PasswordEditWidget is used. I went to the windows shown above and tried different combinations of passwords to see how the indicator behaves, trying to replicate exactly what is done in PasswordGeneratorWidget.

Both development and testing were made on Linux only.

Type of change

✅ New feature (change that adds functionality)

Comments

I'm basically doing the same as in PasswordGeneratorWidget so, would it be worth refactoring and making a derived class of QProgressBar? I would say it is but it is up to you, just let me know.

Thanks for the software ;)

droidmonkey · 2022-04-13T10:44:26Z

Great! Can you align the quality text to be under the text edit (shift it right)?

codecov-commenter · 2022-04-13T10:46:03Z

Codecov Report

Merging #7885 (5e30494) into develop (692c95b) will increase coverage by 0.07%.
The diff coverage is 91.04%.

@@             Coverage Diff             @@
##           develop    #7885      +/-   ##
===========================================
+ Coverage    64.31%   64.38%   +0.07%     
===========================================
  Files          339      339              
  Lines        43430    43529      +99     
===========================================
+ Hits         27932    28025      +93     
- Misses       15498    15504       +6

Impacted Files	Coverage Δ
src/autotype/PickcharsDialog.cpp	`0.00% <0.00%> (ø)`
src/gui/OpVaultOpenWidget.cpp	`10.00% <0.00%> (ø)`
src/keeshare/group/EditGroupWidgetKeeShare.cpp	`34.47% <0.00%> (ø)`
src/gui/PasswordGeneratorWidget.cpp	`68.85% <33.33%> (ø)`
src/gui/DatabaseOpenWidget.cpp	`56.27% <75.00%> (ø)`
src/gui/entry/EditEntryWidget.cpp	`72.28% <75.00%> (+0.05%)`	⬆️
src/gui/PasswordWidget.cpp	`89.43% <97.25%> (ø)`
src/gui/KeePass1OpenWidget.cpp	`82.14% <100.00%> (ø)`
src/gui/databasekey/PasswordEditWidget.cpp	`82.14% <100.00%> (+1.01%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 692c95b...5e30494. Read the comment docs.

jmdana · 2022-04-13T11:13:03Z

Great! Can you align the quality text to be under the text edit (shift it right)?

Like this?

That's exactly how it appears in the password generation window.

I could also change the layout from "Form" to "Grid" and have the label and the bar next to each other

michaelk83 · 2022-04-13T11:48:35Z

Fixes #7437

@jmdana This PR doesn't address #7437 (yet?). That issue talks about the entry edit widget and preview panel.

jmdana · 2022-04-13T11:57:12Z

Fixes #7437

@jmdana This PR doesn't address #7437 (yet?). That issue talks about the entry edit widget and preview panel.

True! I don't know what I was thinking.

If I amend the commit and push force, will I mess the pull request?

droidmonkey · 2022-04-13T12:00:37Z

Nope that is the preferred way to update the PR.

For the quality label I was thinking inline with the quality bar, not below it. You can wrap the two elements in a horizontal layout to make it "one" for the form layout.

Really this should be built into the PasswordWidget not the databasekey/PasswordEditWidget. That way you can show the quality no matter where the PasswordWidget is used. Make it an option because you obviously don't need that when unlocking your database.

jmdana · 2022-04-13T12:44:07Z

I agree, that would be the best.

Now, from what I see, PasswordEdit extends QLineEdit. I see that you add the icons in there but, as far as I understand, in order to implement the progress bar as in the screenshots, PasswordEdit would have to extend a Layout instead (so we can include both widgets in the object).

Is that what you mean? Am I missing something?

droidmonkey · 2022-04-13T13:13:24Z

Yes that's exactly what I mean.

jmdana · 2022-04-13T18:18:10Z

Yes that's exactly what I mean.

Ok, I'll give it a try.

jmdana · 2022-04-13T20:55:56Z

The change involves many files so here is a small summary:

PasswordEdit is now PasswordWidget, which encapsulates both the previous text entry and the quality bar. Adding new stuff should be very easy now.
In order to keep other objects working I have created methods which work as an interface between the new widget and the text entry inside. If, in the future, new objects are added to the widget, changes will be transparent to others. It also allows for some customization within the widget.
The textChanged signal, which was used by a few other widgets, is now propagated. Other objects keep using the same definition of the signal (which means they are listening to the widget now, instead of the text entry) and it is the widget who decides how to implement it. Currently, the widget propagates the signal every time it is captured coming from the text entry.
By default, the quality bar is hidden. setQualityVisible(bool b) does the trick.
Currently, the quality bar can be seen in the entry menu, database creation and database password change. The password generator is still using the previous implementation, maybe should be part of another commit?
About the quality label, well, here is what I think:
- When side by side with the quality bar, because the length of the string is variable, sometimes the gap between the label and the bar is too large. If we make the gap variable then the bar starts moving around. Personally, I don't really like how it looks.
- If we move the label to the right we solve the previous problem but I don't like it either. It seems too far away from where the user should be looking at.
- Under the bar takes a lof ot space...
So my solution was to add it as a tooltip. How many people don't understand what a colored-moving bar under a password field means? I think it is quite obvious but maybe I'm wrong. As it is right now, any change should be very easy to do.

What a simple "first issue" :)

jmdana · 2022-04-13T21:07:44Z

It looks like a test is having a bad time compiling (I didn't even look at them to be honest) and there was some issue with the formatting (I probably didn't reformat all the files after the class refactor).

I'll have a look at some point during the next few days.

jmdana · 2022-04-13T21:09:38Z

src/autotype/PickcharsDialog.cpp

@@ -77,7 +77,9 @@ PickcharsDialog::PickcharsDialog(const QString& string, QWidget* parent)
    // Remove last selected character
    auto shortcut = new QShortcut(Qt::Key_Backspace, this);
    connect(shortcut, &QShortcut::activated, this, [this] {
-        auto text = m_ui->selectedChars->text();
+        //        auto text = m_ui->selectedChars->text();


to be removed

jmdana · 2022-04-13T21:09:42Z

src/autotype/PickcharsDialog.cpp

-        auto text = m_ui->selectedChars->text();
+        //        auto text = m_ui->selectedChars->text();
+        auto text = m_ui->selectedChars->getText();
+        //        m_ui->selectedChars->setText(text.left(text.size() - 1));


to be removed

droidmonkey · 2022-04-13T21:41:28Z

Looking nice! Love the compact design

droidmonkey · 2022-05-28T21:18:59Z

Cleaned up the code and improved the visual look of the quality bar. Great job on this one!

Fixes keepassxreboot#7437 (entry edit view only) Fixes keepassxreboot#5220

jmdana · 2022-06-02T13:01:46Z

Cleaned up the code and improved the visual look of the quality bar. Great job on this one!

Thanks!

jmdana force-pushed the feature/passwd_strength branch from 23dabb7 to 402d273 Compare April 13, 2022 20:52

jmdana commented Apr 13, 2022

View reviewed changes

jmdana force-pushed the feature/passwd_strength branch 2 times, most recently from 7baf8fa to 5e30494 Compare April 14, 2022 20:16

jmdana changed the title ~~Add password strength indicator to PasswordEditWidget~~ Add password strength indicator to password entry fields Apr 15, 2022

droidmonkey self-requested a review May 1, 2022 14:15

droidmonkey added user interface new feature pr: backport pending Pull request yet to be backported to a previous release labels May 1, 2022

droidmonkey added this to the v2.7.2 milestone May 1, 2022

droidmonkey force-pushed the feature/passwd_strength branch 2 times, most recently from 4ad252d to 0b93393 Compare May 28, 2022 21:17

droidmonkey approved these changes May 28, 2022

View reviewed changes

droidmonkey force-pushed the feature/passwd_strength branch 2 times, most recently from 26b6822 to a6d3a0a Compare May 30, 2022 15:14

Add password strength indicator to PasswordEditWidget

829095e

Fixes keepassxreboot#7437 (entry edit view only) Fixes keepassxreboot#5220

droidmonkey force-pushed the feature/passwd_strength branch from a6d3a0a to 829095e Compare May 31, 2022 03:32

droidmonkey merged commit a740fe1 into keepassxreboot:develop May 31, 2022

michaelk83 mentioned this pull request Jun 24, 2022

Increase minimum master password length #8190

Closed

droidmonkey added pr: backported Pull request backported to previous release and removed pr: backport pending Pull request yet to be backported to a previous release labels Jun 27, 2022

phoerious added pr: new feature Pull request that adds a new feature and removed new feature labels Nov 22, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add password strength indicator to password entry fields #7885

Add password strength indicator to password entry fields #7885

jmdana commented Apr 13, 2022 •

edited

Loading

droidmonkey commented Apr 13, 2022

codecov-commenter commented Apr 13, 2022 •

edited

Loading

jmdana commented Apr 13, 2022

michaelk83 commented Apr 13, 2022

jmdana commented Apr 13, 2022

droidmonkey commented Apr 13, 2022 •

edited

Loading

jmdana commented Apr 13, 2022

droidmonkey commented Apr 13, 2022

jmdana commented Apr 13, 2022

jmdana commented Apr 13, 2022

jmdana commented Apr 13, 2022

jmdana Apr 13, 2022

jmdana Apr 13, 2022

droidmonkey commented Apr 13, 2022 •

edited

Loading

droidmonkey commented May 28, 2022

jmdana commented Jun 2, 2022

Add password strength indicator to password entry fields #7885

Add password strength indicator to password entry fields #7885

Conversation

jmdana commented Apr 13, 2022 • edited Loading

Screenshots

Testing strategy

Type of change

Comments

droidmonkey commented Apr 13, 2022

codecov-commenter commented Apr 13, 2022 • edited Loading

Codecov Report

jmdana commented Apr 13, 2022

michaelk83 commented Apr 13, 2022

jmdana commented Apr 13, 2022

droidmonkey commented Apr 13, 2022 • edited Loading

jmdana commented Apr 13, 2022

droidmonkey commented Apr 13, 2022

jmdana commented Apr 13, 2022

jmdana commented Apr 13, 2022

jmdana commented Apr 13, 2022

jmdana Apr 13, 2022

Choose a reason for hiding this comment

jmdana Apr 13, 2022

Choose a reason for hiding this comment

droidmonkey commented Apr 13, 2022 • edited Loading

droidmonkey commented May 28, 2022

jmdana commented Jun 2, 2022

jmdana commented Apr 13, 2022 •

edited

Loading

codecov-commenter commented Apr 13, 2022 •

edited

Loading

droidmonkey commented Apr 13, 2022 •

edited

Loading

droidmonkey commented Apr 13, 2022 •

edited

Loading