Skip to content

Add YubiKey support on CLI.#2710

Closed
louib wants to merge 1 commit intokeepassxreboot:developfrom
louib:cli_yubikey_option
Closed

Add YubiKey support on CLI.#2710
louib wants to merge 1 commit intokeepassxreboot:developfrom
louib:cli_yubikey_option

Conversation

@louib
Copy link
Copy Markdown
Member

@louib louib commented Feb 17, 2019 •
edited
Loading

Add YubiKey support for the CLI with the -y, --yubikey option.

Added the YubiKey::isBlocking function, partly extracted from detect, so that the yubikey detection code could be reused from the GUI and the CLI. Also added more specific error messages by handling more specific yubikey errors.

Type of change

  • ✅ Refactor
  • ✅ New feature

Description and Context

#2451

Testing strategy

Locally + unit tests

Checklist:

  • ✅ I have read the CONTRIBUTING document. [REQUIRED]
  • ✅ My code follows the code style of this project. [REQUIRED]
  • ✅ All new and existing tests passed. [REQUIRED]
  • ✅ I have compiled and verified my code with -DWITH_ASAN=ON. [REQUIRED]
  • ✅ My change requires a change to the documentation, and I have updated it accordingly.
  • ✅ I have added tests to cover my changes.

@droidmonkey
Copy link
Copy Markdown
Member

droidmonkey commented Feb 17, 2019

This is just a note, but there has got to be a better way of handling these common command line options than updating every single file. Perhaps now (or in the --no-password PR) we should introduce a shared function that adds all the common parameters given a parser argument.

@louib
Copy link
Copy Markdown
Member Author

louib commented Feb 17, 2019

@droidmonkey I 💯 agree, and this is the next refactoring I have in mind for the CLI. I'm going to add a DatabaseCommand class to the Command hierarchy to centralize the unlocking code for the commands that deal with a db.

@droidmonkey droidmonkey added this to the v2.4.1 milestone Feb 18, 2019
@louib louib changed the title Add YK support on CLI. Add YubiKey support on CLI. Feb 18, 2019
@louib louib force-pushed the cli_yubikey_option branch from 7de9c86 to 890c378 Compare March 13, 2019 14:57
@droidmonkey droidmonkey modified the milestones: v2.4.1, v2.5.0 Mar 24, 2019
@droidmonkey
Copy link
Copy Markdown
Member

droidmonkey commented Mar 24, 2019

Targeting this for 2.5.0 since it is a pretty large change.

@louib louib force-pushed the cli_yubikey_option branch from 890c378 to 535a24e Compare March 26, 2019 00:08
@louib
Copy link
Copy Markdown
Member Author

louib commented Mar 26, 2019

@droidmonkey alright! I rebased the PR on develop!

@louib louib force-pushed the cli_yubikey_option branch from 535a24e to 928a248 Compare April 8, 2019 17:26
@louib louib force-pushed the cli_yubikey_option branch from 928a248 to fe50f6b Compare May 1, 2019 03:40
@louib
Copy link
Copy Markdown
Member Author

louib commented May 31, 2019

@droidmonkey I'm going to split this PR in 2 as I realize it's quite large to review, and the small yubikey core part of the patch can be beneficial by itself.

@louib louib closed this May 31, 2019
@droidmonkey
Copy link
Copy Markdown
Member

droidmonkey commented May 31, 2019

Sounds good to me!

@louib louib mentioned this pull request Jul 31, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@phoerious phoerious Awaiting requested review from phoerious

@droidmonkey droidmonkey Awaiting requested review from droidmonkey

Assignees

No one assigned

Labels

feature: CLI feature: Hardware Keys

Projects

None yet

Milestone

v2.5.0

Development

Successfully merging this pull request may close these issues.

2 participants

@louib @droidmonkey