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

Feature : --key-file option for CLI #816

Merged
merged 5 commits into from
Jul 25, 2017

Conversation

louib
Copy link
Member

@louib louib commented Jul 24, 2017

  • Introduced the --key-file option for CLI commands
  • Removed support for --gui-prompt and related code.
  • Moved QCoreApplication instantiation to keepassxc-cli.cpp
  • Removed CompositeKey::readFromLine, which was assuming a password entered at the password prompt was possibly a key file path. We do not need that anymore now that there's an option for that.

Overall, removing the use of a QApplication in the CLI. We only use a QCoreApplication now, which removes the need to branch on the application type, and allows to move some duplicated logic to keepassxc-cli.

How has this been tested?

  • Unit tests
  • Tested all the commands with / without key files
  • Tested the merge command with all key-file, key-file-from and -s combination.

Screenshots (if appropriate):

New documentation for the key-file option:

$ ./src/cli/keepassxc-cli clip
Usage: ./src/cli/keepassxc-cli clip [options] database entry [timeout]
Copy an entry's password to the clipboard.

Options:
  -k, --key-file <path>  Key file of the database.

Arguments:
  database               Path of the database.
  entry                  Path of the entry to clip.
  timeout                Timeout in seconds before clearing the clipboard.

Types of changes

  • ✅ New feature
  • ✅ Breaking change
  • ✅ Refactoring

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.

@louib louib requested review from weslly and TheZ3ro July 24, 2017 00:05
@louib louib changed the title Feature key file Feature : --key-file option for CLI Jul 24, 2017
@droidmonkey
Copy link
Member

Any specific reason why you want to remove the gui prompt option?

@louib
Copy link
Member Author

louib commented Jul 24, 2017

@droidmonkey the main reason is that using the gui prompt requires instantiating a QApplication instead of a QCoreApplication, which has a lot of side effects for a terminal app (weird focus changing in Mac, thread errors messages in Windows, etc). The previous solution was to instantiate a QApplication only when needed, but this would defer the creation of the app to the latest moment, and add a lot of duplicated logic.

@droidmonkey
Copy link
Member

Concur with your explanation, thanks!

@louib
Copy link
Member Author

louib commented Jul 25, 2017

@TheZ3ro This PR addresses #681

Copy link
Contributor

@weslly weslly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good and works fine for me

@louib louib merged commit 1d30283 into keepassxreboot:develop Jul 25, 2017
@louib louib deleted the feature_key_file branch July 25, 2017 17:41
@TheZ3ro
Copy link
Contributor

TheZ3ro commented Jul 26, 2017

@louib updating #681

@phoerious phoerious added this to the v2.3.0 milestone Oct 12, 2017
phoerious added a commit that referenced this pull request Feb 27, 2018
- Add support for KDBX 4.0, Argon2 and ChaCha20 [#148, #1179, #1230, #1494]
- Add SSH Agent feature [#1098, #1450, #1463]
- Add preview panel with details of the selected entry [#879, #1338]
- Add more and configurable columns to entry table and allow copying of values by double click [#1305]
- Add KeePassXC-Browser API as a replacement for KeePassHTTP [#608]
- Deprecate KeePassHTTP [#1392]
- Add support for Steam one-time passwords [#1206]
- Add support for multiple Auto-Type sequences for a single entry [#1390]
- Adjust YubiKey HMAC-SHA1 challenge-response key generation for KDBX 4.0 [#1060]
- Replace qHttp with cURL for website icon downloads [#1460]
- Remove lock file [#1231]
- Add option to create backup file before saving [#1385]
- Ask to save a generated password before closing the entry password generator [#1499]
- Resolve placeholders recursively [#1078]
- Add Auto-Type button to the toolbar [#1056]
- Improve window focus handling for Auto-Type dialogs [#1204, #1490]
- Auto-Type dialog and password generator can now be exited with ESC [#1252, #1412]
- Add optional dark tray icon [#1154]
- Add new "Unsafe saving" option to work around saving problems with file sync services [#1385]
- Add IBus support to AppImage and additional image formats to Windows builds [#1534, #1537]
- Add diceware password generator to CLI [#1406]
- Add --key-file option to CLI [#816, #824]
- Add DBus interface for opening and closing KeePassXC databases [#283]
- Add KDBX compression options to database settings [#1419]
- Discourage use of old fixed-length key files in favor of arbitrary files [#1326, #1327]
- Correct reference resolution in entry fields [#1486]
- Fix window state and recent databases not being remembered on exit [#1453]
- Correct history item generation when configuring TOTP for an entry [#1446]
- Correct multiple TOTP bugs [#1414]
- Automatic saving after every change is now a default [#279]
- Allow creation of new entries during search [#1398]
- Correct menu issues on macOS [#1335]
- Allow compilation on OpenBSD [#1328]
- Improve entry attachments view [#1139, #1298]
- Fix auto lock for Gnome and Xfce [#910, #1249]
- Don't remember key files in file dialogs when the setting is disabled [#1188]
- Improve database merging and conflict resolution [#807, #1165]
- Fix macOS pasteboard issues [#1202]
- Improve startup times on some platforms [#1205]
- Hide the notes field by default [#1124]
- Toggle main window by clicking tray icon with the middle mouse button [#992]
- Fix custom icons not copied over when databases are merged [#1008]
- Allow use of DEL key to delete entries [#914]
- Correct intermittent crash due to stale history items [#1527]
- Sanitize newline characters in title, username and URL fields [#1502]
- Reopen previously opened databases in correct order [#774]
- Use system's zxcvbn library if available [#701]
- Implement various i18n improvements [#690, #875, #1436]
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.

5 participants