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

Simplify browser integration including cross-platform proxy code #4680

Merged
merged 1 commit into from
May 14, 2020

Conversation

droidmonkey
Copy link
Member

@droidmonkey droidmonkey commented Apr 30, 2020

  • Only allow use of the proxy application when communicating with browser extension

  • Complete refactor of Browser Integration classes

    • Moved browser service entry point to BrowserService class instead of NativeMessagingHost. BrowserService now coordinates the communication to/from clients.
    • Moved settings page definition out of MainWindow
    • Decoupled BrowserService from DatabaseTabWidget
    • Reduced complexity of various functions and cleaned the ABI (public vs private).
    • Eliminated BrowserClients class, moved functionality into the BrowserService
    • Renamed HostInstaller to NativeMessageInstaller and renamed NativeMessageHost to BrowserHost.

Fix #4121

Testing strategy

Updated the browser tests. The main purpose of this refactor is to eliminate platform-specific code for browser integration in the proxy and KeePassXC. The secondary purpose of the refactor is to attempt to address crashes and freezes that happen when integration is enabled. The most likely culprit was mishandling of mutex's.

Type of change

  • ✅ Bug fix (non-breaking change that fixes an issue)
  • ✅ Breaking change (causes existing functionality to change)
  • ✅ Refactor (significant modification to existing code)

@droidmonkey droidmonkey added this to the v2.6.0 milestone Apr 30, 2020
}

// TODO: select database based on this key id
Q_UNUSED(dbid);
Copy link
Member Author

Choose a reason for hiding this comment

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

This seems to be a missing feature between the extension and KeePassXC. The extension is passing which database it is referring to but we always do operations on the current database tab. This functionality needs to be tightened up.

src/browser/BrowserShared.cpp Outdated Show resolved Hide resolved
src/gui/MainWindow.cpp Show resolved Hide resolved
@droidmonkey droidmonkey force-pushed the fix/browser-proxy branch 9 times, most recently from a87c15e to 045bc98 Compare April 30, 2020 22:05
@varjolintu
Copy link
Member

We could also make the support for XDG_CONFIG_HOME.
#4121

src/browser/BrowserShared.cpp Outdated Show resolved Hide resolved
src/browser/BrowserShared.cpp Outdated Show resolved Hide resolved
src/browser/NativeMessageInstaller.cpp Outdated Show resolved Hide resolved
src/proxy/CMakeLists.txt Show resolved Hide resolved
src/proxy/NativeMessagingProxy.cpp Outdated Show resolved Hide resolved
src/proxy/NativeMessagingProxy.cpp Outdated Show resolved Hide resolved
src/proxy/NativeMessagingProxy.cpp Outdated Show resolved Hide resolved
src/proxy/NativeMessagingProxy.cpp Outdated Show resolved Hide resolved
@droidmonkey droidmonkey force-pushed the fix/browser-proxy branch 5 times, most recently from 894f252 to af3a3f1 Compare May 1, 2020 19:39
src/browser/BrowserShared.cpp Outdated Show resolved Hide resolved
src/browser/NativeMessageInstaller.cpp Show resolved Hide resolved
src/browser/NativeMessageInstaller.cpp Outdated Show resolved Hide resolved
src/browser/NativeMessageInstaller.cpp Outdated Show resolved Hide resolved
src/browser/NativeMessageInstaller.cpp Outdated Show resolved Hide resolved
src/browser/NativeMessageInstaller.cpp Outdated Show resolved Hide resolved
src/browser/NativeMessageInstaller.cpp Outdated Show resolved Hide resolved
src/browser/NativeMessageInstaller.cpp Show resolved Hide resolved
src/proxy/NativeMessagingProxy.cpp Outdated Show resolved Hide resolved
@droidmonkey droidmonkey force-pushed the fix/browser-proxy branch 2 times, most recently from 045f968 to aa057e0 Compare May 11, 2020 01:33
@droidmonkey droidmonkey force-pushed the fix/browser-proxy branch 2 times, most recently from e8afe25 to 8117e1a Compare May 13, 2020 11:29
* Removed option to attach KeePassXC to the browser extension. Users must use the proxy application to communicate with KeePassXC.
* Significantly streamlined proxy code. Used same implementation of stdin/stdout interface across all platforms.
* Moved browser service entry point to BrowserService class instead of NativeMessagingHost. BrowserService now coordinates the communication to/from clients.
* Moved settings page definition out of MainWindow
* Decoupled BrowserService from DatabaseTabWidget
* Reduced complexity of various functions and cleaned the ABI (public vs private).
* Eliminated BrowserClients class, moved functionality into the BrowserService
* Renamed HostInstaller to NativeMessageInstaller and renamed NativeMessageHost to BrowserHost.
* Recognize XDG_CONFIG_HOME when installing native message file on Linux. Fix #4121 and fix #4123.
@droidmonkey droidmonkey requested a review from phoerious May 14, 2020 20:34
@droidmonkey droidmonkey merged commit a145bf9 into develop May 14, 2020
@droidmonkey droidmonkey deleted the fix/browser-proxy branch May 14, 2020 21:14
droidmonkey added a commit that referenced this pull request Jul 7, 2020
Added

- Custom Light and Dark themes [#4110, #4769, #4791, #4796, #4892, #4915]
- Compact mode to use classic Group and Entry line height [#4910]
- View menu to quickly switch themes, compact mode, and toggle UI elements [#4910]
- Search for groups and scope search to matched groups [#4705]
- Save Database Backup feature [#4550]
- Sort entries by "natural order" and move lines up/down [#4357]
- Option to launch KeePassXC on system startup/login [#4675]
- Caps Lock warning on password input fields [#3646]
- Add "Size" column to entry view [#4588]
- Browser-like tab experience using Ctrl+[Num] (Alt+[Num] on Linux) [#4063, #4305]
- Password Generator: Define additional characters to choose from [#3876]
- Reports: Database password health check (offline) [#3993]
- Reports: HIBP online service to check for breached passwords [#4438]
- Auto-Type: DateTime placeholders [#4409]
- Browser: Show group name in results sent to browser extension [#4111]
- Browser: Ability to define a custom browser location (macOS and Linux only) [#4148]
- Browser: Ability to change root group UUID and inline edit connection ID [#4315, #4591]
- CLI: `db-info` command [#4231]
- CLI: Use wl-clipboard if xclip is not available (Linux) [#4323]
- CLI: Incorporate xclip into snap builds [#4697]
- SSH Agent: Key file path env substitution, SSH_AUTH_SOCK override, and connection test [#3769, #3801, #4545]
- SSH Agent: Context menu actions to add/remove keys [#4290]

Changed

- Complete replacement of default database icons [#4699]
- Complete replacement of application icons [#4066, #4161, #4203, #4411]
- Complete rewrite of documentation and manpages using Asciidoctor [#4937]
- Complete refactor of config files; separate between local and roaming [#4665]
- Complete refactor of browser integration and proxy code [#4680]
- Complete refactor of hardware key integration (YubiKey and OnlyKey) [#4584, #4843]
- Significantly improve performance when saving and opening databases [#4309, #4833]
- Remove read-only detection for database files [#4508]
- Overhaul of password fields and password generator [#4367]
- Replace instances of "Master Key" with "Database Credentials" [#4929]
- Change settings checkboxes to positive phrasing for consistency [#4715]
- Improve UX of using entry actions (focus fix) [#3893]
- Set expiration time to Now when enabling entry expiration [#4406]
- Always show "New Entry" in context menu [#4617]
- Issue warning before adding large attachments [#4651]
- Improve importing OPVault [#4630]
- Improve AutoOpen capability [#3901, #4752]
- Check for updates every 7 days even while still running [#4752]
- Improve Windows installer UI/UX [#4675]
- Improve config file handling of portable distribution [#4131, #4752]
- macOS: Hide dock icon when application is hidden to tray [#4782]
- Browser: Use unlock dialog to improve UX of opening a locked database [#3698]
- Browser: Improve database and entry settings experience [#4392, #4591]
- Browser: Improve confirm access dialog [#2143, #4660]
- KeeShare: Improve monitoring file changes of shares [#4720]
- CLI: Rename `create` command to `db-create` [#4231]
- CLI: Cleanup `db-create` options (`--set-key-file` and `--set-password`) [#4313]
- CLI: Use stderr for help text and password prompts [#4086, #4623]
- FdoSecrets: Display existing secret service process [#4128]

Fixed

- Fix changing focus around the main window using tab key [#4641]
- Fix search field clearing while still using the application [#4368]
- Improve search help widget displaying on macOS and Linux [#4236]
- Return keyboard focus after editing an entry [#4287]
- Reset database path after failed "Save As" [#4526]
- Use SHA256 Digest for Windows code signing [#4129]
- Improve handling of ccache when building [#4104, #4335]
- macOS: Properly re-hide application window after browser integration and Auto-Type usage [#4909]
- Auto-Type: Fix crash when performing on new entry [#4132]
- Browser: Send legacy HTTP settings to recycle bin [#4589]
- Browser: Fix merging browser keys [#4685]
- CLI: Fix encoding when exporting database [#3921]
- SSH Agent: Improve reliability and underlying code [#3833, #4256, #4549, #4595]
- FdoSecrets: Fix crash when editing settings before service is enabled [#4332]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Browser integration doesn't respect XDG_CONFIG_HOME
3 participants