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

Support key files with Auto Open feature #3504

Merged
merged 1 commit into from Sep 7, 2019

Conversation

@metaphys
Copy link
Contributor

metaphys commented Sep 3, 2019

Fixes #3495
Summary of changes:

  • DatabaseWidget::processAutoOpen() looks for custom attribute KeyFile in
    the AutoOpen's entry that should contain the path to a keyfile in the same
    format as for the database file
  • requestOpenDatabase and its call are updated to handle this extra parameter
  • New feature (non-breaking change which adds functionality)

Testing strategy

auto opening db with or without keyfile works (only tested on linux)

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]
@droidmonkey

This comment has been minimized.

Copy link
Member

droidmonkey commented Sep 4, 2019

I think you are missing a lot of the changes you meant to include in this PR

@metaphys metaphys force-pushed the metaphys:develop branch from ca87edb to bb674ce Sep 4, 2019
@metaphys

This comment has been minimized.

Copy link
Contributor Author

metaphys commented Sep 4, 2019

Yeah sorry I pushed that late yesterday, it's corrected now.

@metaphys

This comment has been minimized.

Copy link
Contributor Author

metaphys commented Sep 4, 2019

I didn't find where are the docs to edit for auto open, could you point me to it?

@droidmonkey

This comment has been minimized.

Copy link
Member

droidmonkey commented Sep 4, 2019

They are missing and need to be created. You are welcome to do that in this pr

@metaphys

This comment has been minimized.

Copy link
Contributor Author

metaphys commented Sep 4, 2019

Well it seems that I still have trouble with that commit :/

@metaphys metaphys force-pushed the metaphys:develop branch 2 times, most recently from 38133e3 to 6584252 Sep 4, 2019
@metaphys

This comment has been minimized.

Copy link
Contributor Author

metaphys commented Sep 4, 2019

Ok that will do for indentation problems and wrongly removed section.
Where should I add the auto open doc? in docs/QUICKSTART.md?

@droidmonkey

This comment has been minimized.

Copy link
Member

droidmonkey commented Sep 4, 2019

Yes that is good

@metaphys metaphys force-pushed the metaphys:develop branch 4 times, most recently from 7b784da to 591e8fd Sep 4, 2019
@metaphys

This comment has been minimized.

Copy link
Contributor Author

metaphys commented Sep 4, 2019

Ok this should be ready for pulling now. (You probably want to correct my awful #engrish before)

@phurrelmann

This comment has been minimized.

Copy link

phurrelmann commented Sep 5, 2019

Hm, could we align this with the autoopen/autoexec feature as implemented in keepass and keepass2droid?
Plugin KeeAutoExec for Keepass and Keepass2droid implement it the same way:

User name: Must contain the path to the key file, if one should be used. The path can be either absolute or relative to the directory containing KeePass.exe.
Password: The master password for the database to open. If no password is required, leave this field empty.
URL: Path to the database file to open. The path can be either absolute or relative to the directory containing KeePass.exe.

Additionally I already got this working with KeepassXC in the past, but it stopped working (regressed) with 2.4.2 or 2.4.3 (afair).

@metaphys

This comment has been minimized.

Copy link
Contributor Author

metaphys commented Sep 5, 2019

I would rather wait to have more opinions before changing the code.
On one hand I do understand the importance of following other software standards.
On the other hand I find strange to use a field for something different than it's label (and yes this is already bugging me for URL).

Maybe a way to mitigate that would be to have a different editor for entry that are part of a AutoOpen group. This editor would just rename the label of those 2 fields so that the user see then as keyfile path and file path while actually saving them in regular Username and URL fields... would the current implementation allow such a change easily?

@droidmonkey

This comment has been minimized.

Copy link
Member

droidmonkey commented Sep 5, 2019

If anything the url field is more appropriate for a file path

@metaphys

This comment has been minimized.

Copy link
Contributor Author

metaphys commented Sep 5, 2019

Ok so I will move the key file pass to Username as suggested to follow standards. I'll leave the entry editor relabel for another PR

@droidmonkey

This comment has been minimized.

Copy link
Member

droidmonkey commented Sep 5, 2019

We can support either a file path in username field OR URL field. Preference is username for backwards compatibility

@metaphys

This comment has been minimized.

Copy link
Contributor Author

metaphys commented Sep 5, 2019

Well we need 2 files: the key and the database. So we need both field, each for one. If keepass and keepass2droid are already having this with keyfile in Username and DB in URL, I think that it would be a good idea to stick to it as @phurrelmann suggested...

@droidmonkey

This comment has been minimized.

Copy link
Member

droidmonkey commented Sep 5, 2019

Oh oops, forgot the database was in the url field. Forget what I said

@metaphys

This comment has been minimized.

Copy link
Contributor Author

metaphys commented Sep 5, 2019

ok am i ready for pulling, then? I don't know about my push to my local fork. I kind of made mess. I'm not very good at git :/ Do I need to merge those 3 commit into one or would the pulling concatenate them all together?

@metaphys metaphys force-pushed the metaphys:develop branch from 5586e79 to 804a75b Sep 5, 2019
@droidmonkey

This comment has been minimized.

Copy link
Member

droidmonkey commented Sep 5, 2019

Yah I got it from here, thanks!

@droidmonkey droidmonkey added this to the v2.5.0 milestone Sep 6, 2019
@droidmonkey droidmonkey self-requested a review Sep 6, 2019
Fixes #3495

* Look for keyfile in username parameter of the Auto Open entries. If present, pass on to unlock call to the database.
@droidmonkey droidmonkey force-pushed the metaphys:develop branch from 804a75b to 83831bb Sep 6, 2019
@droidmonkey droidmonkey merged commit 72c1783 into keepassxreboot:develop Sep 7, 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)]
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.