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

Add 2FA suggestions report #7761

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

HexF
Copy link
Contributor

@HexF HexF commented Apr 1, 2022

This PR fixes #4096 , implementing a report which compares all the URLs in a Keepass database against those from the 2fa.directory dataset, informing the user if they can enable 2FA on accounts, and how to go about doing that.

If no network is available (whether that be through no compiled support or no network), the report simply hides the supported, match, and documentation columns.

Filters are available on the right side to switch between showing entries with 2fa enabled, disabled, supported and not supported in any combination.

Double-clicking on a line in the report will edit the related entry in the database, while double-clicking on the documentation will launch it in a browser.

Screenshots

image

image

Testing strategy

Changes were tested manually by adding records with varying 2FA statuses (enabled and disabled), for various different websites known in the 2fa.directory data set.
It was tested that they correctly match, provided the hostnames are equal and that the appropriate level of support is provided.

Type of change

  • ✅ New feature (change that adds functionality

Fixes keepassxreboot#4096

Signed-off-by: Thomas Hobson <thomas@hexf.me>
@codecov-commenter
Copy link

codecov-commenter commented Apr 1, 2022

Codecov Report

Merging #7761 (e5dc3bd) into develop (aca197a) will decrease coverage by 0.21%.
The diff coverage is 16.84%.

@@             Coverage Diff             @@
##           develop    #7761      +/-   ##
===========================================
- Coverage    64.31%   64.10%   -0.21%     
===========================================
  Files          339      341       +2     
  Lines        43450    43646     +196     
===========================================
+ Hits         27943    27979      +36     
- Misses       15507    15667     +160     
Impacted Files Coverage Δ
src/gui/reports/ReportsDialog.h 100.00% <ø> (ø)
src/gui/reports/ReportsWidget2FA.cpp 12.29% <12.29%> (ø)
src/gui/reports/ReportsPage2FA.cpp 57.14% <57.14%> (ø)
src/gui/reports/ReportsDialog.cpp 51.95% <100.00%> (+1.95%) ⬆️
src/core/Entry.cpp 82.79% <0.00%> (+0.31%) ⬆️

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 aca197a...e5dc3bd. Read the comment docs.

@droidmonkey
Copy link
Member

Can we combine this with the password change urls so people can also easily change their password for entries?

@HexF
Copy link
Contributor Author

HexF commented Apr 1, 2022

How do you imagine that would look in terms of the report? I'm struggling to see how to work that into the 2FA report.
What I imagine would be on the edit entry page, a button that will open a dialog like the one in #6323, with a button that will open to password change URLs like defined in #6032

@droidmonkey
Copy link
Member

This is true, nevermind then

@droidmonkey droidmonkey added this to the v2.8.0 milestone Apr 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Suggest TOTP for entries using a web service [$50]
3 participants