Show expired entries on DB unlock

[ba32107]

Fixes #4624. I implemented it the way I described in the above linked issue:

I like the way the original KeePass does this. Whenever the database is unlocked,
it groups together all expired entries into a temporary, "virtual" group, which is displayed by default, so you see all your expired entries. If you navigate away to another group, the virtual group disappears, and the application behaves as normal from that point on.

Screenshots

New option:

After DB unlock, option disabled:

After DB unlock, option enabled:

Testing strategy

Manual testing:

• Created expired entries in many groups (child groups too)
• Verified that all expired entries appear on DB unlock, except the ones where "exclude from database reports" is ticked
• Verified that nothing appears when there are no expired entries
• Verified that nothing appears when the option is disabled

Type of change

• ✅ New feature (change that adds functionality)

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 86.11111% with 5 lines in your changes missing coverage. Please review.

Project coverage is 64.33%. Comparing base (064d621) to head (953319d).
Report is 530 commits behind head on develop.

Files with missing lines	Patch %	Lines
src/gui/ApplicationSettingsWidget.cpp	66.67%	5 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #7290      +/-   ##
===========================================
+ Coverage    64.25%   64.33%   +0.08%     
===========================================
  Files          339      339              
  Lines        43160    43195      +35     
===========================================
+ Hits         27729    27786      +57     
+ Misses       15431    15409      -22     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

---

🚨 Try these New Features:

• Flaky Tests Detection - Detect and resolve failed and flaky tests
• JS Bundle Analysis - Avoid shipping oversized bundles

[droidmonkey]

• Can you post a banner message at the top (similar to the search message bar) that says you are viewing the expired entries?
• What if you want to ignore these (ie, keep them expired)?

[ba32107]

Can you post a banner message at the top (similar to the search message bar) that says you are viewing the expired entries?

Good idea, I pushed a change. The message disappears when you switch groups.

What if you want to ignore these (ie, keep them expired)?

In that case, you can set the "exclude from database reports" flag on the entry, disable the feature, or even make the entry non-expiring. Not sure if there's a better way - I think adding a new entry flag called "warn me about expiring this entry" is overkill.

[droidmonkey]

I agree, if you enable this option then you also need to from your database, since that is why you enabled it! This might also be a good use case for a dynamic tag (once merged in).

[ba32107]

I'm not actually sure why the Translations PR check is failing. Is there anything to do for me on this PR?

[droidmonkey]

We can handle that, but if you can.. run ./release-tool i18n lupdate

[ba32107]

Done!

[ba32107]

I pushed a change that allows expiry warnings x days in advance. See screenshots here.

I was not able to test the auto-unlock functionality described in this comment, because for some reason my custom-built KeePassXC binary wasn't able to connect with my browser extension. I don't understand why - the prod version works just fine. Would be great if someone could double-check that bit!

[droidmonkey]

You can test the pathway by just unlocking a database after locking it manually. You should check the value of DatabaseWidget::m_groupBeforeUnlock to see if the database has been unlocked already in thus session. Or you can introduce a boolean in DatabaseWidget that indicates the expired entries have been shown already.

When a new database is opened or the application first launches then these will be null/false indicating that expired entries should be shown if setting is enabled.

[ba32107]

Requested changes pushed. I consider the PR final - let me know if there's anything else left to do. Thanks!

[droidmonkey]

Made a few edits to clean-up the implementation:

1. Removed expired entry build from Database, kept all logic within DatabaseWidget and Entry
2. Made setting off by default
3. Exclude expired entries that are recycled

[michaelk83]

I think this should be on by default, for three reasons:

1. Discoverability. If on, the new behavior is immediately visible, and people who don't want it can turn it off. Maybe show a hint the first time to point users at the setting. If it's off by default, no one would even think to look for that option.
2. Security. If "expired" passwords are only expired in KPXC, then users may continue to use old passwords that should be replaced, which is less secure. Which brings me to...
3. Convenience. Make users' life easier by reminding them to renew their passwords. I think that making users' life easier should always be the default, except where it comes at the cost of security. But here it would improve security, so its' a win-win.

Also, I think it should default to a few days before expiry (a week?), for similar reasons.

edit: I'd also suggest a column with the dice button to renew the expired password directly from the report page, but that can be left to a separate PR.

[droidmonkey]

Yah I'll move it back. Guaranteed we'll get issues about how that is terrible and needs to be off by default 😅

[lindhe]

I'm excited for this feature! Great initiative and great work! :) 👍

[michaelk83]

Guaranteed we'll get issues about how that is terrible and needs to be off by default

That's why I suggested a hint to point users at the setting. If they can help themselves with a couple clicks, they won't need to post an issue (hopefully).

[droidmonkey]

Adding settings redirects is cumbersome, extra code, and takes up space. I am just joking (am I though?) about the hordes of issues,

[droidmonkey]

Great work on this @ba32107 👏 what's next?

[ba32107]

Haha we'll see. Gotta find some more time to work on open-source stuff :)