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

πŸ’»πŸ’» CLI password generation options cleanup #3275

Merged
merged 3 commits into from Aug 31, 2019

Conversation

@louib
Copy link
Member

louib commented Jun 15, 2019

Summary of changes:

  • extract function for creating password generator from options into
    Generate command. This function is now reused in Add and Edit
    commands.
  • Updated manpage with missing password generation options.
  • Updated manpage with missing longer forms of password generation options.
  • Added unit tests for new password generation options in Add and
    Edit.
  • Handle case when -g and -p options are used at the same time.

This PR adds password generation functionalities while reducing
code duplication, but at the cost of 2 small breaking changes:

  • the password generation option for Add and Edit for specifying
    password length is now -L instead of -l, to not clash with the
    -l --lower option.
  • the -u shorthand for the --upper option had to be renamed to -U, to not
    clash with the -u --username option.

@droidmonkey what about the CHANGELOG file, should I update it with the breaking changes now, or do you wait before the releases to do that?

Type of change

  • βœ… Bug fix
  • βœ… Refactor
  • βœ… New feature
  • βœ… Breaking change
  • βœ… Documentation

Testing strategy

existing + new 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.
@louib louib added the plugin: CLI label Jun 15, 2019
@louib louib requested a review from droidmonkey Jun 15, 2019
@louib louib added this to the v2.5.0 milestone Jun 15, 2019
tests/TestCli.cpp Show resolved Hide resolved
@droidmonkey

This comment has been minimized.

Copy link
Member

droidmonkey commented Jun 16, 2019

Please do update the CHANGELOG. You can use a "2.5.0-Beta1" title. I actually like the idea of gradually growing the log with major PR's so I don't miss any important bits later on.

@louib louib force-pushed the louib:add_edit_generate_options branch 2 times, most recently from 5ea3474 to fb6c9ac Jun 16, 2019
@louib

This comment has been minimized.

Copy link
Member Author

louib commented Jun 16, 2019

@droidmonkey I added the 2 breaking changes to the CHANGELOG as well as a line about the new options. Is it clear that πŸ’₯ means a breaking change? πŸ˜†

@louib louib force-pushed the louib:add_edit_generate_options branch 2 times, most recently from fb0bfa4 to c2d4461 Jun 19, 2019
@drone540

This comment has been minimized.

Copy link

drone540 commented Jul 20, 2019

Why not keep the -u option for the --upper password generation, and make the --username option use -U, or vice versa (-U = upper, -u = username). As it stands right now, it's missing a shorthand option if you wanted to do something like -ulnse to use all the groups.

Edit: looks like -u was for username first. so maybe just use -U for --upper

@droidmonkey

This comment has been minimized.

Copy link
Member

droidmonkey commented Jul 21, 2019

-U for upper makes a lot of sense

@louib

This comment has been minimized.

Copy link
Member Author

louib commented Jul 22, 2019

@drone540 @droidmonkey good call, renamed to -U instead of removing the shorthand altogether.

@louib louib force-pushed the louib:add_edit_generate_options branch from a57ce7a to 895d212 Jul 22, 2019
louib added 2 commits Jun 15, 2019
Summary of changes:
* extract function for creating password generator from options into
`Generate` command. This function is now reused in `Add` and `Edit`
commands.
* Updated manpage with missing password generation options.
* Updated manpage with missing longer forms of password generation options.
* Added unit tests for new password generation options in `Add` and
`Edit`.
* Handle case when `-g` and `-p` options are used at the same time.

This PR adds password generation functionalities while reducing
code duplication, but at the cost of 2 small breaking changes:
* the password generation option for `Add` and `Edit` for specifying
password length is now `-L` instead of `-l`, to not clash with the
`-l --lower` option.
* the `-u` shorthand for the `--upper` option has to be removed, to not
clash with the `-u --username` option.
@louib louib force-pushed the louib:add_edit_generate_options branch from 895d212 to c4e901e Aug 19, 2019
@louib louib force-pushed the louib:add_edit_generate_options branch from c4e901e to ef1b16d Aug 19, 2019
@louib

This comment has been minimized.

Copy link
Member Author

louib commented Aug 19, 2019

@phoerious @droidmonkey looks like the MacOS tests are hanging now 😬 😬

@droidmonkey

This comment has been minimized.

Copy link
Member

droidmonkey commented Aug 19, 2019

Please ignore, the macOS server is currently down

@louib louib requested review from keepassxreboot/core-developers and removed request for droidmonkey Aug 20, 2019
@droidmonkey droidmonkey merged commit eb18824 into keepassxreboot:develop Aug 31, 2019
4 checks passed
4 checks passed
CodeFactor No issues found.
Details
MacOS (KeepassXC) TeamCity build finished
Details
Ubuntu Linux (KeepassXC) TeamCity build finished
Details
Windows 10 (KeepassXC) TeamCity build finished
Details
phoerious added a commit that referenced this pull request Oct 26, 2019
Added

- Add 'Paper Backup' aka 'Export to HTML file' to the 'Database' menu [[#3277](#3277)]
- Add statistics panel with information about the database (number of entries, number of unique passwords, etc.) to the Database Settings dialog [[#2034](#2034)]
- Add offline user manual accessible via the 'Help' menu [[#3274](#3274)]
- Add support for importing 1Password OpVault files [[#2292](#2292)]
- Implement Freedesktop.org secret storage DBus protocol so that KeePassXC can be used as a vault service by libsecret [[#2726](#2726)]
- Add support for OnlyKey as an alternative to YubiKeys (requires yubikey-personalization >= 1.20.0) [[#3352](#3352)]
- Add group sorting feature [[#3282](#3282)]
- Add feature to download favicons for all entries at once [[#3169](#3169)]
- Add word case option to passphrase generator [[#3172](#3172)]
- Add support for RFC6238-compliant TOTP hashes [[#2972](#2972)]
- Add UNIX man page for main program [[#3665](#3665)]
- Add 'Monospaced font' option to the notes field [[#3321](#3321)]
- Add support for key files in auto open [[#3504](#3504)]
- Add search field for filtering entries in Auto-Type dialog [[#2955](#2955)]
- Complete usernames based on known usernames from other entries [[#3300](#3300)]
- Parse hyperlinks in the notes field of the entry preview pane [[#3596](#3596)]
- Allow abbreviation of field names in entry search [[#3440](#3440)]
- Allow setting group icons recursively [[#3273](#3273)]
- Add copy context menu for username and password in Auto-Type dialog [[#3038](#3038)]
- Drop to background after copying a password to the clipboard [[#3253](#3253)]
- Add 'Lock databases' entry to tray icon menu [[#2896](#2896)]
- Add option to minimize window after unlocking [[#3439](#3439)]
- Add option to minimize window after opening a URL [[#3302](#3302)]
- Request accessibility permissions for Auto-Type on macOS [[#3624](#3624)]
- Browser: Add initial support for multiple URLs [[#3558](#3558)]
- Browser: Add entry-specific browser integration settings [[#3444](#3444)]
- CLI: Add offline HIBP checker (requires a downloaded HIBP dump) [[#2707](#2707)]
- CLI: Add 'flatten' option to the 'ls' command [[#3276](#3276)]
- CLI: Add password generation options to `Add` and `Edit` commands [[#3275](#3275)]
- CLI: Add XML import [[#3572](#3572)]
- CLI: Add CSV export to the 'export' command [[#3278](#3278)]
- CLI: Add `-y --yubikey` option for YubiKey [[#3416](#3416)]
- CLI: Add `--dry-run` option for merging databases [[#3254](#3254)]
- CLI: Add group commands (mv, mkdir and rmdir) [[#3313](#3313)].
- CLI: Add interactive shell mode command `open` [[#3224](#3224)]

Changed

- Redesign database unlock dialog [ [#3287](#3287)]
- Rework the entry preview panel [ [#3306](#3306)]
- Move notes to General tab on Group Preview Panel [[#3336](#3336)]
- Enable entry actions when editing an entry and cleanup entry context menu  [[#3641](#3641)]
- Improve detection of external database changes  [[#2389](#2389)]
- Warn if user is trying to use a KDBX file as a key file [[#3625](#3625)]
- Add option to disable KeePassHTTP settings migrations prompt [[#3349](#3349), [#3344](#3344)]
- Re-enabled Wayland support (no Auto-Type yet) [[#3520](#3520), [#3341](#3341)]
- Add icon to 'Toggle Window' action in tray icon menu [[3244](#3244)]
- Merge custom data between databases only when necessary [[#3475](#3475)]
- Improve various file-handling related issues when picking files using the system's file dialog [[#3473](#3473)]
- Add 'New Entry' context menu when no entries are selected [[#3671](#3671)]
- Reduce default Argon2 settings from 128 MiB and one thread per CPU core to 64 MiB and two threads to account for lower-spec mobile hardware [ [#3672](#3672)]
- Browser: Remove unused 'Remember' checkbox for HTTP Basic Auth [[#3371](#3371)]
- Browser: Show database name when pairing with a new browser [[#3638](#3638)]
- Browser: Show URL in allow access dialog [[#3639](#3639)]
- CLI: The password length option `-l` for the CLI commands `Add` and `Edit` is now `-L` [[#3275](#3275)]
- CLI: The `-u` shorthand for the `--upper` password generation option has been renamed to `-U` [[#3275](#3275)]
- CLI: Rename command `extract` to `export`. [[#3277](#3277)]

Fixed

- Improve accessibility for assistive technologies [[#3409](#3409)]
- Correctly unlock all databases if `--pw-stdin` is provided [[#2916](#2916)]
- Fix password generator issues with special characters [[#3303](#3303)]
- Fix KeePassXC interrupting shutdown procedure [[#3666](#3666)]
- Fix password visibility toggle button state on unlock dialog [[#3312](#3312)]
- Fix potential data loss if database is reloaded while user is editing an entry [[#3656](#3656)]
- Fix hard-coded background color in search help popup [[#3001](#3001)]
- Fix font choice for password preview [[#3425](#3425)]
- Fix handling of read-only files when autosave is enabled [[#3408](#3408)]
- Handle symlinks correctly when atomic saves are disabled [[#3463](#3463)]
- Enable HighDPI icon scaling on Linux [[#3332](#3332)]
- Make Auto-Type on macOS more robust and remove old Carbon API calls [[#3634](#3634), [[#3347](#3347))]
- Hide Share tab if KeePassXC is compiled without KeeShare support and other minor KeeShare improvements [[#3654](#3654), [[#3291](#3291), [#3029](#3029), [#3031](#3031), [#3236](#3236)]
- Correctly bring window to the front when clicking tray icon on macOS [[#3576](#3576)]
- Correct application shortcut created by MSI Installer on Windows [[#3296](#3296)]
- Fix crash when removing custom data [[#3508](#3508)]
- Fix placeholder resolution in URLs [[#3281](#3281)]
- Fix various inconsistencies and platform-dependent compilation bugs [[#3664](#3664), [#3662](#3662), [#3660](#3660), [#3655](#3655), [#3649](#3649), [#3417](#3417), [#3357](#3357), [#3319](#3319), [#3318](#3318), [#3304](#3304)]
- Browser: Fix potential leaking of entries through the browser integration API if multiple databases are opened [[#3480](#3480)]
- Browser: Fix password entropy calculation [[#3107](#3107)]
- Browser: Fix Windows registry settings for portable installation [[#3603](#3603)]
scoroi added a commit to scoroi/keepassxc that referenced this pull request Nov 10, 2019
Summary of changes:
* Extract function for creating password generator from options into
`Generate` command. This function is now reused in `Add` and `Edit`
commands.
* Updated manpage with missing password generation options.
* Updated manpage with missing longer forms of password generation options.
* Added unit tests for new password generation options in `Add` and
`Edit`.
* Handle case when `-g` and `-p` options are used at the same time.

This PR adds password generation functionalities while reducing
code duplication, but at the cost of 2 small breaking changes:
* The password generation option for `Add` and `Edit` for specifying
password length is now `-L` instead of `-l`, to not clash with the
`-l --lower` option.
* The `-u` shorthand for the `--upper` option has to be removed, to not
clash with the `-u --username` option.
* Add -U variant for uppercase.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
You can’t perform that action at this time.