Upstream Flathub patches #7728

louib · 2022-03-30T01:07:16Z

Upstream the patches from Flathub to enable packaging the app in Flatpak.
Related PR here

Type of change

✅ New feature (change that adds functionality)

louib · 2022-03-30T01:08:23Z

@phoerious @droidmonkey This is patch 1/4 of the patches currently on Flathub.
The only thing I'm not sure about is the new ID variable. I'm thinking about giving it a more explicit name, like REVERSE_DNS_ID maybe.

droidmonkey · 2022-03-30T01:20:38Z

Where is ID used?

louib · 2022-03-30T01:29:22Z

@droidmonkey It will be used in the next patches, to change some filenames depending on the build type.

droidmonkey · 2022-03-30T01:42:13Z

Are you going to put them all into this PR? I would appreciate that.

codecov-commenter · 2022-03-30T01:45:49Z

Codecov Report

Merging #7728 (09d28fc) into develop (31db3c3) will increase coverage by 0.00%.
The diff coverage is 75.00%.

@@           Coverage Diff            @@
##           develop    #7728   +/-   ##
========================================
  Coverage    64.29%   64.29%           
========================================
  Files          339      339           
  Lines        43429    43429           
========================================
+ Hits         27920    27922    +2     
+ Misses       15509    15507    -2

Impacted Files	Coverage Δ
src/browser/BrowserSettingsWidget.cpp	`59.35% <ø> (ø)`
src/browser/BrowserShared.cpp	`0.00% <ø> (ø)`
src/browser/NativeMessageInstaller.cpp	`24.17% <ø> (-1.24%)`	⬇️
src/core/EntryAttachments.cpp	`48.68% <ø> (ø)`
src/gui/Icons.h	`100.00% <ø> (ø)`
src/gui/Icons.cpp	`87.21% <75.00%> (+0.15%)`	⬆️
src/core/Entry.cpp	`82.86% <0.00%> (+0.20%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 31db3c3...09d28fc. Read the comment docs.

louib · 2022-03-30T01:46:09Z

@droidmonkey yeah I can do that

louib · 2022-03-30T02:11:22Z

@varjolintu I added you as a reviewer so that you can have a look at the browser extension stuff 🙏

share/linux/org.keepassxc.KeePassXC.desktop.in

src/browser/BrowserShared.cpp

src/core/EntryAttachments.cpp

louib · 2022-03-30T02:22:58Z

@droidmonkey all the patches from Flathub have been added to this PR.

share/CMakeLists.txt

src/browser/BrowserSettingsWidget.cpp

src/browser/NativeMessageInstaller.cpp

src/browser/NativeMessageInstaller.h

utils/flatpak-command-wrapper.sh

share/linux/org.keepassxc.KeePassXC.appdata.xml

varjolintu · 2022-04-01T07:58:21Z

@louib Tested this quickly in Linux Mint with Firefox, and everything is working with the browser extension.

varjolintu · 2022-04-01T13:50:22Z

@louib Like I said in IRC/Matrix, only Firefox works and creates script files when Browser Integration support is enabled. Maybe this is because of https://github.com/keepassxreboot/keepassxc/blob/develop/src/browser/NativeMessageInstaller.cpp#L223= ? Firefox has always QDir::homePath() for the root, but other browsers rely on QStandardPaths::writableLocation(QStandardPaths::ConfigLocation). Does that path need to be something else when on Flatpak?

varjolintu · 2022-04-03T10:42:55Z

Confirmed that Chromium tries to start the Flatpak instead of keepassxc-proxy. With Firefox everything works fine. The scripts are also written to correct locations.
There's a short moment when I saw the following process in running when starting Chromium or trying to restart the connection:
/usr/bin/flatpak -run --branch=test --arch=x86_64 org.keepassxc.KeePassXC chrome-extension://oboonakemofpalcgghocfoadofidjkkk

Also seeing:
keepassxc chrome-extension://oboonakemofpalcgghocfoadofidjkkk/ instead of keepassxc-proxy that is started when using Firefox.

The wrapper script starts /usr/libexec/flatpak-bwrap --args 37 keepassxc-wrapper chrome-extension://oboonakemofpalcgghocfoadofidjkkk/ briefly but then it vanishes.

Could it be possible that the wrapper scripts has some kind of bug concerning non-Firefox browsers? I'm trying to investigate this further.

EDIT: The script checks against chrome-extension://oboonakemofpalcgghocfoadofidjkkk but the actual command paremeter has an extra / at the end.

EDIT 2: That was it! I modified the script locally and it worked nicely with the extra /.

Enable support for the CMake option: KEEPASSXC_DIST_TYPE=Flatpak. Pre-requisite for other Flatpak specific changes. It also means Flatpak is properly displayed as the distribution type in the app debug tab.

Flatpak isolates application files from the host, but some files must be exported to the host to serve their purpose (ex. desktop entries, icons mime types, etc...). Filenames must however be prefixed by the app id, in reverse-dns notation (org.keepassxc.KeePassXC). If KEEPASSXC_DIST_TYPE=Flatpak is set CMake can now install files with exportable filenames, without affecting other distribution types.

Flatpak browser integration and near feature parity with non-sandboxed distribution types. This is primarily made possible by: 1. The unix socket instead listens at $XDG_RUNTIME_DIR/app/$FLATPAK_ID/ which is host accessible by default. Using the Flatpak permission: --filesystem=xdg-run isn't possible (nor allowed). 2. Including a wrapper script which acts as a workaround to the Flatpak limitation of a single exportable host command per app. Running org.keepassxc.KeePassXC on the host will run this instead of keepassxc directly. There are also some Flatpak specific UX improvements, such as better sandbox compatibility and automatic proxy path detection. Custom locations are disabled because it requires extensive Flatpak knowledge and even then there's isn't any one reliable and easy workaround. What does work - Browser integration, including all supported browsers. - Automatic proxy path detection and updating of manifests. - This works with different Flatpak installations: system, user and custom ones, as long as the path uses safe POSIX portable file names. Limitations and caveats - The browser cannot be sandboxed or it will be unable execute commands in the host namespace, which is currently required to use native messaging; i.e. web browsers cannot be installed as Flatpaks. Note: The Native Messaging Host API would be a lot more sandbox friendly with the addition of D-Bus support, as an alternative to stdio.

Ensure that xdg portals can open attachment files, while simultaneously removing the only critical need for global /tmp access. The path: $XDG_RUNTIME_DIR/app/$FLATPAK_ID is used for flatpak because it exists, respects XDG standards and is host accessible by default. - Fix attachment opening from a flatpak style sandbox - Respect $XDG_RUNTIME_DIR when available - Remove need for needlessly exposing the host /tmp

share/CMakeLists.txt

louib requested review from phoerious and droidmonkey March 30, 2022 01:07

louib force-pushed the flatpak_patch_1 branch from 8aa3a78 to 7d87f3e Compare March 30, 2022 02:05

louib changed the title ~~Add flatpak distribution type~~ Upstream Flathub patches Mar 30, 2022

louib requested a review from varjolintu March 30, 2022 02:10

louib commented Mar 30, 2022

View reviewed changes

share/linux/org.keepassxc.KeePassXC.desktop.in Outdated Show resolved Hide resolved

louib commented Mar 30, 2022

View reviewed changes

src/browser/BrowserShared.cpp Outdated Show resolved Hide resolved

louib commented Mar 30, 2022

View reviewed changes

src/core/EntryAttachments.cpp Outdated Show resolved Hide resolved

phoerious requested changes Mar 30, 2022

View reviewed changes

louib mentioned this pull request Mar 31, 2022

Preparing for v2.7.1 flathub/org.keepassxc.KeePassXC#89

Merged

louib commented Mar 31, 2022

View reviewed changes

share/linux/org.keepassxc.KeePassXC.appdata.xml Show resolved Hide resolved

louib force-pushed the flatpak_patch_1 branch from a41c148 to 809dce5 Compare April 1, 2022 01:38

louib requested a review from phoerious April 1, 2022 01:54

droidmonkey added this to the v2.7.1 milestone Apr 2, 2022

droidmonkey added the distribution: Flatpak label Apr 2, 2022

AsavarTzeth added 4 commits April 3, 2022 11:26

Add flatpak distribution type

24be9a2

Enable support for the CMake option: KEEPASSXC_DIST_TYPE=Flatpak. Pre-requisite for other Flatpak specific changes. It also means Flatpak is properly displayed as the distribution type in the app debug tab.

louib added 14 commits April 3, 2022 11:26

Fix EntryAttachment rebase

7e10ecb

Temp commenting

2a79bc1

Remove import

4bf3d4e

Formatting

75fe52c

Make fn static

1706323

Uncomment the installs

14d3cb4

Remove URL from appdata

26df961

Fix standard Linux build

b869462

Fix build on Linux

ebcfe98

Rename ID -> APP_ID

6d71c12

APP_ICON -> APP_ICON_NAME

aeee4c6

Support cli in flatpak wrapper script

9df55fe

Hardcoding XDG paths in Flatpak sandbox

362afbc

Revert regex operator in flatpak wrapper

c822a4f

louib force-pushed the flatpak_patch_1 branch from effdae4 to c822a4f Compare April 3, 2022 15:26

droidmonkey requested changes Apr 3, 2022

View reviewed changes

share/CMakeLists.txt Outdated Show resolved Hide resolved

Use build dir for building

ba28c88

louib requested a review from droidmonkey April 3, 2022 15:43

droidmonkey approved these changes Apr 3, 2022

View reviewed changes

Use CMAKE_CURRENT_BINARY_DIR

31f1351

varjolintu approved these changes Apr 3, 2022

View reviewed changes

phoerious approved these changes Apr 3, 2022

View reviewed changes

Merge branch 'develop' into flatpak_patch_1

09d28fc

droidmonkey merged commit 7cd824a into develop Apr 4, 2022

droidmonkey deleted the flatpak_patch_1 branch April 4, 2022 23:04

pull bot pushed a commit to nonomal/keepassxc that referenced this pull request Apr 5, 2022

Upstream Flathub patches (keepassxreboot#7728)

65eb3bf

droidmonkey added the pr: backported Pull request backported to previous release label Apr 6, 2022

t-h-e pushed a commit to t-h-e/keepassxc that referenced this pull request Sep 8, 2022

Upstream Flathub patches (keepassxreboot#7728)

0415d6a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upstream Flathub patches #7728

Upstream Flathub patches #7728

louib commented Mar 30, 2022 •

edited

Loading

louib commented Mar 30, 2022

droidmonkey commented Mar 30, 2022

louib commented Mar 30, 2022

droidmonkey commented Mar 30, 2022

codecov-commenter commented Mar 30, 2022 •

edited

Loading

louib commented Mar 30, 2022

louib commented Mar 30, 2022

louib commented Mar 30, 2022

varjolintu commented Apr 1, 2022 •

edited

Loading

varjolintu commented Apr 1, 2022

varjolintu commented Apr 3, 2022 •

edited

Loading

Upstream Flathub patches #7728

Upstream Flathub patches #7728

Conversation

louib commented Mar 30, 2022 • edited Loading

Type of change

louib commented Mar 30, 2022

droidmonkey commented Mar 30, 2022

louib commented Mar 30, 2022

droidmonkey commented Mar 30, 2022

codecov-commenter commented Mar 30, 2022 • edited Loading

Codecov Report

louib commented Mar 30, 2022

louib commented Mar 30, 2022

louib commented Mar 30, 2022

varjolintu commented Apr 1, 2022 • edited Loading

varjolintu commented Apr 1, 2022

varjolintu commented Apr 3, 2022 • edited Loading

louib commented Mar 30, 2022 •

edited

Loading

codecov-commenter commented Mar 30, 2022 •

edited

Loading

varjolintu commented Apr 1, 2022 •

edited

Loading

varjolintu commented Apr 3, 2022 •

edited

Loading