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

Adding a best option to clip to copy a password of the best match #4489

Merged

Conversation

lerignoux
Copy link
Contributor

@lerignoux lerignoux commented Mar 25, 2020

if only one matching entry exists

I added an option to do a best effort search when doing a clip.

For instance if I have an entry ****/paypal/account and do a keepassxc-cli clip -b db.kdbx payp it will search for a matching entry and clip it if it's unique. This make the search for a password much faster even if you dont know the exact name/case ...

Type of change

  • ✅ New feature (non-breaking change which adds functionality)

Description and Context

I will use the cli exclusively to copy my passwords but it is difficult to remember the exact name we use for the key and slow to find it back. If you know you have a gmail for instance entry it helps you to clip it even with an approximate title.
on failure to find a single one it will display all matching entries enabling you to specify the right one.
Fixes #4270 that I opened on it

Screenshots

image

Testing strategy

Try to call clip without the -b option, it should keep the previous behavior:

  • find the entry on matching name
  • do not find the entry if not exactly matching an entry
    Call clip -b and some sub string of your entries. it should
  • echo the matching entry and clip the password if a unique entry is found
  • display the list of matching entries otherwise

I didn't see any tests on the CLI, and I doubt adding unit tests on this is relevant.

Checklist:

  • ✅ I have read the CONTRIBUTING document.
  • ✅ My code follows the code style of this project.
  • ✅ All new and existing tests passed.
  • ✅ I have compiled and verified my code with -DWITH_ASAN=ON.
  • ✅ My change requires a change to the documentation, and I have updated it accordingly. -> I didnt found documentation on the cli. Could you confirm that no documentation change is needed ?
  • ✅ I have added tests to cover my changes. -> It seems additional tests are not needed. Please tell me if you see something I could add

lerignoux added a commit to lerignoux/keepassxc that referenced this pull request Mar 29, 2020
The best option copy the password from the best match if only one matching entry exists.
@lerignoux lerignoux force-pushed the lerignoux_best_match_clip branch from 97cf956 to fe8510c Compare March 29, 2020 05:02
@lerignoux lerignoux marked this pull request as ready for review March 29, 2020 05:04
@lerignoux
Copy link
Contributor Author

Hello

I didn't found documentation or tests on the CLI to update. Could you confirm that it's ok ?

lerignoux added a commit to lerignoux/keepassxc that referenced this pull request Mar 29, 2020
The best option copy the password from the best match if only one matching entry exists.
@lerignoux lerignoux force-pushed the lerignoux_best_match_clip branch from 226af2d to d3e7b92 Compare March 29, 2020 05:12
lerignoux added a commit to lerignoux/keepassxc that referenced this pull request Mar 29, 2020
The best option copy the password from the best match if only one matching entry exists.
@lerignoux lerignoux force-pushed the lerignoux_best_match_clip branch from d3e7b92 to 52a2c5f Compare March 29, 2020 05:14
lerignoux added a commit to lerignoux/keepassxc that referenced this pull request Mar 29, 2020
The best option copy the password from the best match if only one matching entry exists.
@lerignoux lerignoux force-pushed the lerignoux_best_match_clip branch from 52a2c5f to a7c2303 Compare March 29, 2020 05:27
lerignoux added a commit to lerignoux/keepassxc that referenced this pull request Mar 29, 2020
The best option copy the password from the best match if only one matching entry exists.
@lerignoux lerignoux force-pushed the lerignoux_best_match_clip branch from a7c2303 to 709a377 Compare March 29, 2020 05:54
@lerignoux lerignoux changed the title WIP Adding a best option to clip to copy a password of the best match Adding a best option to clip to copy a password of the best match Mar 29, 2020
@lerignoux
Copy link
Contributor Author

The MacOS build fails because of ssh connection issue. It does not seem related to my changes.
I do not found how to restart the build

@droidmonkey droidmonkey added this to the v2.6.1 milestone May 23, 2020
lerignoux added a commit to lerignoux/keepassxc that referenced this pull request May 28, 2020
The best option copy the password from the best match if only one matching entry exists.
@lerignoux lerignoux force-pushed the lerignoux_best_match_clip branch 2 times, most recently from e81c180 to 1b79b73 Compare May 28, 2020 02:44
lerignoux added a commit to lerignoux/keepassxc that referenced this pull request May 28, 2020
The best option copy the password from the best match if only one matching entry exists.
@lerignoux lerignoux force-pushed the lerignoux_best_match_clip branch from 1b79b73 to 4ad5943 Compare May 28, 2020 03:04
lerignoux added a commit to lerignoux/keepassxc that referenced this pull request May 28, 2020
The best option copy the password from the best match if only one matching entry exists.
@lerignoux lerignoux force-pushed the lerignoux_best_match_clip branch from 4ad5943 to c737684 Compare May 28, 2020 03:10
lerignoux added a commit to lerignoux/keepassxc that referenced this pull request May 28, 2020
The best option copy the password from the best match if only one matching entry exists.
src/cli/Clip.cpp Outdated Show resolved Hide resolved
lerignoux added a commit to lerignoux/keepassxc that referenced this pull request Jul 12, 2020
The best option copy the password from the best match if only one matching entry exists.
@lerignoux lerignoux force-pushed the lerignoux_best_match_clip branch from c737684 to 752caad Compare July 12, 2020 04:53
@droidmonkey droidmonkey modified the milestones: v2.6.1, v2.7.0 Jul 15, 2020
@droidmonkey droidmonkey requested a review from louib July 15, 2020 03:07
@louib
Copy link
Member

louib commented Jul 15, 2020

Hi @lerignoux ! Thanks for the PR.

There is a manpage for the CLI located in docs/man/ which would need to be updated with the new option.

Tests for the CLI are located in tests/TestCli.cpp and I think it's definitely worth it to add tests for the new option.

Displaying which entry was used is a nice touch 👍

@lerignoux lerignoux force-pushed the lerignoux_best_match_clip branch 2 times, most recently from 83659e6 to d76b019 Compare July 20, 2020 11:51
lerignoux added a commit to lerignoux/keepassxc that referenced this pull request Jul 20, 2020
The best option copy the password from the best match if only one matching entry exists.
@lerignoux
Copy link
Contributor Author

lerignoux commented Jul 25, 2020

Hi @lerignoux ! Thanks for the PR.

There is a manpage for the CLI located in docs/man/ which would need to be updated with the new option.

Tests for the CLI are located in tests/TestCli.cpp and I think it's definitely worth it to add tests for the new option.

Displaying which entry was used is a nice touch +1

Hello

Thank you @louib for the info. I updated the manual and tests.

Though I don't manage to run the cli test on my machine. tests seem to run well but the cli test is not run.
I saw the errors on TeamCity, to fix them I should add an entry to the test Database (to properly test the option) though I am not sure how to open and edit it.
do you know the test DB password ?

@lerignoux lerignoux force-pushed the lerignoux_best_match_clip branch 2 times, most recently from 0b96ca9 to 4099066 Compare July 25, 2020 04:40
@lerignoux
Copy link
Contributor Author

@louib would you have any info about my previous commit ? i would like to add a fixture to the test DB to add a relevant test for this new feature.

Cheers

@louib
Copy link
Member

louib commented Aug 24, 2020

@lerignoux for the CLI tests to run, you need to configure the project with -DWITH_GUI_TESTS=ON (this is because the CLI depends on QT's GUI library, which is something we're trying to fix).

The password of the test database must be somewhere in the tests/TestCli.cpp file, but IIRC it's a

@lerignoux lerignoux force-pushed the lerignoux_best_match_clip branch 4 times, most recently from 09ec0b5 to 29ccefb Compare August 27, 2020 12:25
@lerignoux
Copy link
Contributor Author

Yeah thanks a lot @louib it's much easier when I can run the tests directly :p
Managed to clean everything.

tell me if I can do anything else !

The best option copy the password from the best match if only one matching entry exists.

Adding clip best option documentation

Adding unit tests on the new clip --best option
@droidmonkey droidmonkey force-pushed the lerignoux_best_match_clip branch from 29ccefb to 8de2e12 Compare September 1, 2020 00:28
@droidmonkey
Copy link
Member

This is super slick, love it!

@droidmonkey droidmonkey merged commit f49f62d into keepassxreboot:develop Sep 1, 2020
@phoerious phoerious added pr: new feature Pull request that adds a new feature and removed new feature labels Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: CLI pr: new feature Pull request that adds a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CLI: Clip or Show the best (only) match when not providing the full entry path
5 participants