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

A search string is set automatically after database unlock #8626

Closed
varjolintu opened this issue Oct 23, 2022 · 13 comments · Fixed by #8643
Closed

A search string is set automatically after database unlock #8626

varjolintu opened this issue Oct 23, 2022 · 13 comments · Fixed by #8643
Assignees

Comments

@varjolintu
Copy link
Member

varjolintu commented Oct 23, 2022

Overview

When opening a database first time after KeePassXC start, a search string "is:expired-3" is entered automatically to the search field and entries are filtered.

Steps to Reproduce

  1. Start KeePassXC
  2. Open the database
  3. Search field inserts a search string automatically

Expected Behavior

No search string is entered.

Actual Behavior

Search string "is:expired-3" is entered automatically. This only happens after KeePassXC has just started. Reopening any database does not modify the search field.

KeePassXC - 2.7.3

Operating System: macOS/arm64

@varjolintu varjolintu added the bug label Oct 23, 2022
@droidmonkey
Copy link
Member

This is because of the "show expired entries on unlock" setting... but it shouldn't show anything unless the search returns results.

@michaelk83
Copy link

I would say this works as intended, and can be closed. It's just that users aren't used to the new saved searches feature yet.

@varjolintu
Copy link
Member Author

@michaelk83 I haven't enabled any new settings, so it's definitely not working as intended.

@ghost
Copy link

ghost commented Oct 24, 2022

observing the same behaviour as described in the issue. Have not enabled any new settings in prior to ver 2.7.3.

Steps to reproduce:

  1. flatpak update to 2.7.3
  2. run keepassxc
  3. seeing is:expired-3 query string on successful login

Steps to bypass this issue:

  1. Settings > General
  2. Uncheck On database unlock, show entries that ...

Not sure if this checkbox is a part of the new feature from this update. But from an end user standpoint, this behaviour is unexpected and not sure where that query string was coming from since it is never been used for quering before.

@phoerious
Copy link
Member

I think the issue is that the new setting is enabled by default, which I suppose it shouldn't be.

image

@michaelk83
Copy link

I haven't enabled any new settings, so it's definitely not working as intended.

Show expired entries setting was introduced in 2.7.0 (#7290). It's on by default for discoverability and because it's good security practice to update expired passwords. 2.7.2/2.7.3 merely changed it to use a saved search (#8435). The search string you're seeing is because the saved search is being loaded in order to show you the expiring entries (as intended). You should get the same behavior if you manually click the "Expired" saved search in tags pane (well, almost, since #7791 isn't finished yet).

What I meant by "users aren't used to the new feature yet", is that users such as yourself haven't used saved searches before (since it didn't exist), and haven't seen it loading a saved search string. So they don't understand what they are seeing, and aren't expecting it. But that's the same dilemma as when any new feature is introduced: do you leave it off, at the cost of reduced discoverability, or do you turn it on by default, at the cost of some users being surprised? I think this one is beneficial enough to leave it on by default.

Point 3 of #7791 would make this less obtrusive, when it's implemented:

Instead of showing the full report on startup, show only a banner: You have passwords about to expire. [Show me]. "Show me" link opens the Expiring tag.

But in the meantime, if you don't want the expiring entries to be shown on startup, simply disable the setting.

@phoerious
Copy link
Member

phoerious commented Oct 24, 2022

I did notice that the demo database always started with the expired Google entry even before saved searches. I myself don't really use the expiration feature for my personal databases.

@michaelk83
Copy link

All of the confusion caused by this feature will be resolved with #7791:

  • No confusing search string loaded at startup (until the user clicks "Show me").
  • No auto-loading of some filtered list of entries that is not very clear what those are.
  • The banner text would clearly explain what's going on.

So this issue can also be viewed as a duplicate of #7791. Ideally it would've been included with the saved searches introduction in 2.7.2, but time constraints decided otherwise.

@droidmonkey droidmonkey pinned this issue Oct 24, 2022
@droidmonkey droidmonkey self-assigned this Oct 24, 2022
@vphantom
Copy link

FYI, I had not seen this before 2.7.3. Now when I unlock a database, there's a search for is:expired-3 with empty results that I must clear out manually if the setting is on. This is moot since #7791 is coming, but I thought I should point out that I get it even though I have no entries with expiration dates set at all, which doesn't seem to be the intended behavior.

@varjolintu
Copy link
Member Author

After database unlock, if you directly create a new entry, it's title will also be is:expired-3.

droidmonkey added a commit that referenced this issue Oct 28, 2022
* Fixes #8626
* Also remove old feature to set the title of a new entry to the current search text. This only made sense before advanced searching was made available.
@droidmonkey
Copy link
Member

The above is old behavior when searches could only be simple. I removed that in my fix PR.

@faizalsb
Copy link

I think the issue is that the new setting is enabled by default, which I suppose it shouldn't be.

image

This is the solution

@michaelk83
Copy link

This is the solution

If you mean disabling by default, then not really. See #7290 (comment) and #7791. But you're free to disable it yourself if you prefer.

droidmonkey added a commit that referenced this issue Oct 29, 2022
* Fixes #8626
* Also remove old feature to set the title of a new entry to the current search text. This only made sense before advanced searching was made available.
droidmonkey added a commit that referenced this issue Oct 29, 2022
* Fixes #8626
* Also remove old feature to set the title of a new entry to the current search text. This only made sense before advanced searching was made available.
droidmonkey added a commit that referenced this issue Oct 29, 2022
* Fixes #8626
* Also remove old feature to set the title of a new entry to the current search text. This only made sense before advanced searching was made available.
droidmonkey added a commit that referenced this issue Oct 29, 2022
* Fixes #8626
* Also remove old feature to set the title of a new entry to the current search text. This only made sense before advanced searching was made available.
@droidmonkey droidmonkey unpinned this issue Nov 7, 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 a pull request may close this issue.

6 participants