flatpak / xdg-desktop-portal

Desktop integration portal
https://flatpak.github.io/xdg-desktop-portal/
GNU Lesser General Public License v2.1
593 stars 192 forks source link

Dynamic Launcher Portal design questions #681

Closed mwleeds closed 2 years ago

mwleeds commented 2 years ago

Context: https://gitlab.gnome.org/Teams/Design/os-mockups/-/issues/144

This issue is for discussing the technical architecture of a new portal that would allow sandboxed applications to install .desktop files and icons used by those desktop files to make launcher entries in the desktop environment's application launcher. The primary use case for the portal is to allow Epiphany or Chromium to install web apps when running under Flatpak (or Snap). However it could have other use cases.

cc @matthiasclasen

Requirements

Open questions

Proposed interface:

org.freedesktop.portal.DynamicLauncher.xml ``` ```
mcatanzaro commented 2 years ago

Depending on the options used by the calling application, the user can edit the name (Chromium would not allow this).

Would be nice to allow editing icon as well, because Epiphany is pretty bad at choosing good icons. This is more important if the portal does not allow editing name/icon after the app is created.

There also needs to be a mode where the portal allows the installation of a .desktop launcher by a sandboxed application without any user interaction, but only if a token (a UUID string) is provided which was previously obtained by an unsandboxed trusted system component. This is necessary so that GNOME Software can install web apps without requiring user interaction beyond clicking the "Install" button. GNOME Software would talk to Epiphany over D-Bus and provide the token acquired from the portal, so that Epiphany can stay in charge of the on-disk format for its web apps.

Ah, that might work. Clever.

Is the interface in the right shape?

Seems OK to me, though I'm sure there will be problems we haven't noticed yet. :D

Is it correct to only have the PrepareInstall() method use the org.freedesktop.portal.Request::Response pattern, since it is the only one that involves waiting on user interaction?

I think so? Sounds reasonable?

The path could be simpler to store along with the generated token in the portal's own storage, whereas it's unclear to me if an fd would necessarily stay open until the Install() method call when it's needed.

Well the portal would keep it open, right?

I agree that it's OK to use a file path if the API is not intended for use by sandboxed applications. That said, if all the other APIs use fds, there's really no harm in doing the same for consistency.

Is the scheme laid out in the description of the Uninstall() method acceptable

I think so, although we will have to change Epiphany's web app prefix, because hyphens are not allowed in D-Bus identifiers, an annoying discrepancy between GApplication vs. D-Bus.

What should the directory holding the icons be called, ~/.local/share/icons_org.gnome.Epiphany.WebApp/? The icons would need to live somewhere not writable to the sandboxed application so they have to be moved somewhere by the portal.

How about create them in the app's $XDG_DATA_DIR/icons, but owned by uid 0 so the app has no write permission?

How do I proceed with designing the org.freedesktop.impl.portal.DynamicLauncher interface? I understand that's what a backend like xdg-desktop-portal-gnome will implement?

Good question....

I have one more question of my own: is the sandboxed Epiphany/Chromium application allowed to know about the existence of web apps? i.e. could about:applications continue to exist, where Epiphany displays installed web apps and allows you to delete them? I don't think that's possible with the current design since there's no way to view desktop files. That's not necessarily a bad thing, since you provide a way for web apps to uninstall themselves. Would just require some UI changes in the browsers.

mcatanzaro commented 2 years ago

In that example the desktop file would exist at the path "~/.local/share/applications/org.gnome.Epiphany.WebApp/pinafore-e9d0e1e4b0a10856aa3b38d9eb4375de4070d043.desktop".

Perhaps we should match flatpak, wherever flatpak installs its exported desktop files for user (not system) applications?

(I suppose the behavior for snap would need to be different.)

mwleeds commented 2 years ago

In that example the desktop file would exist at the path "~/.local/share/applications/org.gnome.Epiphany.WebApp/pinafore-e9d0e1e4b0a10856aa3b38d9eb4375de4070d043.desktop".

Perhaps we should match flatpak, wherever flatpak installs its exported desktop files for user (not system) applications?

(I suppose the behavior for snap would need to be different.)

I guess you mean put the .desktop files in ~/.local/share/flatpak/exports/share/applications but I can think of a few reasons that might not be a good idea: (1) these are not Flatpaks and Flatpak should be able to make the assumption that files in that directory are its responsibility (2) we can't even assume that Flatpak or Snap is installed, as this portal would be used even by unsandboxed Epiphany and (3) the advantage of having the desktop files in a subdirectory of ~/.local/share/applications is we can give Flatpak'd Epiphany read-only access to that directory so it can see its installed web apps, in case we decide to keep the "application manager" feature.

Depending on the options used by the calling application, the user can edit the name (Chromium would not allow this).

Would be nice to allow editing icon as well, because Epiphany is pretty bad at choosing good icons. This is more important if the portal does not allow editing name/icon after the app is created.

Yeah, I'm not sure exactly how much complexity that would entail for the implementation but last time we talked about this you agreed it would make sense to put effort into improving Epiphany's icon selection and that effort might be better spent than on making the portal more complex.

The path could be simpler to store along with the generated token in the portal's own storage, whereas it's unclear to me if an fd would necessarily stay open until the Install() method call when it's needed.

Well the portal would keep it open, right?

I guess. To be honest I don't understand how passing a file descriptor over d-bus works.

I agree that it's OK to use a file path if the API is not intended for use by sandboxed applications. That said, if all the other APIs use fds, there's really no harm in doing the same for consistency.

I guess it's possible to work out what the path is for the file descriptor on the portal side?

Is the scheme laid out in the description of the Uninstall() method acceptable

I think so, although we will have to change Epiphany's web app prefix, because hyphens are not allowed in D-Bus identifiers, an annoying discrepancy between GApplication vs. D-Bus.

I don't understand. There are no hyphens in org.gnome.Epiphany.WebApp, and the hyphen after that is the separator used to identify the prefix and decide what directory to use.

What should the directory holding the icons be called, ~/.local/share/icons_org.gnome.Epiphany.WebApp/? The icons would need to live somewhere not writable to the sandboxed application so they have to be moved somewhere by the portal.

How about create them in the app's $XDG_DATA_DIR/icons, but owned by uid 0 so the app has no write permission?

But xdg-desktop-portal needs to be able to delete it when the app is uninstalled, and x-d-p doesn't run as root?

How do I proceed with designing the org.freedesktop.impl.portal.DynamicLauncher interface? I understand that's what a backend like xdg-desktop-portal-gnome will implement?

Good question....

I have one more question of my own: is the sandboxed Epiphany/Chromium application allowed to know about the existence of web apps? i.e. could about:applications continue to exist, where Epiphany displays installed web apps and allows you to delete them? I don't think that's possible with the current design since there's no way to view desktop files. That's not necessarily a bad thing, since you provide a way for web apps to uninstall themselves. Would just require some UI changes in the browsers.

I think it is possible if we add --filesystem=~/.local/share/applications/org.gnome.Epiphany.WebApp:ro to Epiphany's finish-args?

mcatanzaro commented 2 years ago

I guess you mean put the .desktop files in ~/.local/share/flatpak/exports/share/applications but I can think of a few reasons that might not be a good idea: (1) these are not Flatpaks and Flatpak should be able to make the assumption that files in that directory are its responsibility (2) we can't even assume that Flatpak or Snap is installed, as this portal would be used even by unsandboxed Epiphany and (3) the advantage of having the desktop files in a subdirectory of ~/.local/share/applications is we can give Flatpak'd Epiphany read-only access to that directory so it can see its installed web apps, in case we decide to keep the "application manager" feature.

Hmmm, OK.

Problem is, if Epiphany is sandboxed, it won't have permission to view anything under ~/.local/share/applications on the host, not unless the portal mounts it inside its $XDG_DATA_DIR, or unless we poke a sandbox hole. (And the existing hole to access ~/Downloads and the krb5 sockets are enough to get Epiphany labeled unsafe by GNOME Software, so let's avoid adding more holes.)

Yeah, I'm not sure exactly how much complexity that would entail for the implementation but last time we talked about this you agreed it would make sense to put effort into improving Epiphany's icon selection and that effort might be better spent than on making the portal more complex.

Well let's think about the UI for this. We surely want to allow the user to edit app name and icon (even if we improve icon selection code). That doesn't necessarily have to be implemented by the portal: it could be done before the portal is invoked. But if done in two steps, that means two dialogs the user has to click through, instead of one.

I guess it's possible to work out what the path is for the file descriptor on the portal side?

Hm, I don't think so, maybe only if it uses O_PATH? But in that case, you might as well just pass a path instead. If it's the path you really need, not the file contents, then passing the path makes more sense.

I don't understand. There are no hyphens in org.gnome.Epiphany.WebApp, and the hyphen after that is the separator used to identify the prefix and decide what directory to use.

Sadly that hyphen cannot be used in a D-Bus identifier. i.e. owning "org.gnome.Epiphany.WebApp-facebook" bus name is not possible. We need to either change the hyphen to an underscore, or a period (which seems nicer to me). Yes, that means migration for existing web apps, but this all is going to require a huge migration anyway....

But xdg-desktop-portal needs to be able to delete it when the app is uninstalled, and x-d-p doesn't run as root?

Oh sorry, I mean the portal would create it owned by uid 0 in the sandbox so that it can't be written to from within the sandbox. On the host it would be owned by your user (1000 or whatever).

I think it is possible if we add --filesystem=~/.local/share/applications/org.gnome.Epiphany.WebApp:ro to Epiphany's finish-args?

Yes, that would be possible, but let's avoid doing that.

mcatanzaro commented 2 years ago

Hm, I don't think so, maybe only if it uses O_PATH?

I'm wrong, it can be done using fstat(). But does the portal really need a path, or does it only need the file contents?

mwleeds commented 2 years ago

Hm, I don't think so, maybe only if it uses O_PATH?

I'm wrong, it can be done using fstat(). But does the portal really need a path, or does it only need the file contents?

Assuming we keep using GsRemoteIcon in Software for this, that object has a cached file for the icon, so we wouldn't be writing the icon out specifically for this portal call to a temporary location and expecting it to be gone afterwards. The other consideration is the performance cost of reading in the file data and writing it back out to the final location (which might be able to be avoided with a filesystem move or copy), but we also might be able to read the icon data once and use it for both displaying the icon to the user and writing it out when the dialog is confirmed.

So maybe we do just need the fd to read the contents.

mwleeds commented 2 years ago

I guess you mean put the .desktop files in ~/.local/share/flatpak/exports/share/applications but I can think of a few reasons that might not be a good idea: (1) these are not Flatpaks and Flatpak should be able to make the assumption that files in that directory are its responsibility (2) we can't even assume that Flatpak or Snap is installed, as this portal would be used even by unsandboxed Epiphany and (3) the advantage of having the desktop files in a subdirectory of ~/.local/share/applications is we can give Flatpak'd Epiphany read-only access to that directory so it can see its installed web apps, in case we decide to keep the "application manager" feature.

Hmmm, OK.

Problem is, if Epiphany is sandboxed, it won't have permission to view anything under ~/.local/share/applications on the host, not unless the portal mounts it inside its $XDG_DATA_DIR, or unless we poke a sandbox hole. (And the existing hole to access ~/Downloads and the krb5 sockets are enough to get Epiphany labeled unsafe by GNOME Software, so let's avoid adding more holes.)

Well sandboxed Epiphany also can't read ~/.local/share/flatpak/exports/share/applications right? I think Software could be updated to not consider access to ~/.local/share/applications/org.gnome.Epiphany.WebApp/ unsafe, using a check on that path similar to what the portal would do. Even if we don't have the app manager feature of Epiphany, Epiphany still needs to be able to see its web app desktop files to implement the GetInstalledWebApps method of the org.gnome.Epiphany.WebAppProvider service (on my development branch).

Yeah, I'm not sure exactly how much complexity that would entail for the implementation but last time we talked about this you agreed it would make sense to put effort into improving Epiphany's icon selection and that effort might be better spent than on making the portal more complex.

Well let's think about the UI for this. We surely want to allow the user to edit app name and icon (even if we improve icon selection code). That doesn't necessarily have to be implemented by the portal: it could be done before the portal is invoked. But if done in two steps, that means two dialogs the user has to click through, instead of one.

I would agree one dialog is better than two from a UX perspective. And if we are going to use this portal for both sandboxed and unsandboxed Epiphany for PWA and non-PWA web apps, editing the icon might be important since non-PWAs would be less likely to provide a good icon. I was hoping the MVP for this could not include changing the icon, but I can look into including it.

I don't understand. There are no hyphens in org.gnome.Epiphany.WebApp, and the hyphen after that is the separator used to identify the prefix and decide what directory to use.

Sadly that hyphen cannot be used in a D-Bus identifier. i.e. owning "org.gnome.Epiphany.WebApp-facebook" bus name is not possible. We need to either change the hyphen to an underscore, or a period (which seems nicer to me). Yes, that means migration for existing web apps, but this all is going to require a huge migration anyway....

Hmm, well experimentally with --own-name=org.gnome.Epiphany.* in the finish args and using g_desktop_app_info_new_from_filename and g_app_info_launch I was able to launch org.gnome.Epiphany.WebApp-reddit-27aa67d11878da3049675f218f9822318d27bf67.desktop, so I guess it works somehow?

I think it is possible if we add --filesystem=~/.local/share/applications/org.gnome.Epiphany.WebApp:ro to Epiphany's finish-args?

Yes, that would be possible, but let's avoid doing that.

Are you saying you are okay with losing the app manager functionality?

YaLTeR commented 2 years ago

The primary use case for the portal is to allow Epiphany or Chromium to install web apps when running under Flatpak (or Snap). However it could have other use cases.

I think another very important use-case is Steam, to make launcher entries for games.

sonnyp commented 2 years ago

The portal also needs to allow deleting launchers.

As pointed out in https://github.com/flatpak/xdg-desktop-portal/issues/326#issuecomment-520092221 - the launchers should be deleted when the application is uninstalled - somehow.

ramcq commented 2 years ago

cc @wjt @refi64 as folks who have worked on similar in Chromium

matthiasclasen commented 2 years ago

The portal also needs to allow deleting launchers. For sandboxed applications we could only allow deletion of launchers installed by that application so that the portal doesn't need to do any user interaction.

We should also make sure that installed launchers are automatically cleaned up when the flatpak is uninstalled

matthiasclasen commented 2 years ago

There also needs to be a mode where the portal allows the installation of a .desktop launcher by a sandboxed application without any user interaction, but only if a token (a UUID string) is provided which was previously obtained by an unsandboxed trusted system component. This is necessary so that GNOME Software can install web apps without requiring user interaction beyond clicking the "Install" button. GNOME Software would talk to Epiphany over D-Bus and provide the token acquired from the portal, so that Epiphany can stay in charge of the on-disk format for its web apps.

What on-disk format are we talking about here? My understanding is that the launcher is a desktop file that is entirely under the control of the portal. Right?

mcatanzaro commented 2 years ago

I think Software could be updated to not consider access to ~/.local/share/applications/org.gnome.Epiphany.WebApp/ unsafe, using a check on that path similar to what the portal would do. Even if we don't have the app manager feature of Epiphany, Epiphany still needs to be able to see its web app desktop files to implement the GetInstalledWebApps method of the org.gnome.Epiphany.WebAppProvider service (on my development branch).

I suppose that might make sense. We could do the same for icons, then.

Hmm, well experimentally with --own-name=org.gnome.Epiphany.* in the finish args and using g_desktop_app_info_new_from_filename and g_app_info_launch I was able to launch org.gnome.Epiphany.WebApp-reddit-27aa67d11878da3049675f218f9822318d27bf67.desktop, so I guess it works somehow?

Presumably nothing attempts to own the org.gnome.Epiphany.WebApp-reddit-27aa67d11878da3049675f218f9822318d27bf67 bus name?

I think another very important use-case is Steam, to make launcher entries for games.

If Valve is interested in this, then yes, good point.

Otherwise: I don't see why it would matter? Could this be useful without changes in Steam to use the new portal?

As pointed out in #326 (comment) - the launchers should be deleted when the application is uninstalled - somehow.

Good catch.

What on-disk format are we talking about here? My understanding is that the launcher is a desktop file that is entirely under the control of the portal. Right?

That's the goal, but it's not currently the case. Currently Epiphany expects the desktop file to exist in the web app's data directory (under ~/.local/share/applications there is only a symlink) or the app will fail to start. It also has strict and inconsistent requirements for how everything is named. So part of this is figuring out what changes are required to make things work. Another trick is that sandboxed Epiphany must not be allowed to edit its desktop file or icon to avoid spoofing.

mwleeds commented 2 years ago

The portal also needs to allow deleting launchers. For sandboxed applications we could only allow deletion of launchers installed by that application so that the portal doesn't need to do any user interaction.

We should also make sure that installed launchers are automatically cleaned up when the flatpak is uninstalled

Hmm that's a good point but it's not an easy requirement. I don't think we can assume that the users of the portal are only Flatpaks (or Snaps) because traditionally packaged applications could also use it. For Flatpaks we could have the directory in ~/.local/share/applications/ be named with just the app ID org.gnome.Epiphany rather than something like org.gnome.Epiphany.WebApp to simplify cleaning it up upon uninstallation, and that cleanup could be Flatpak's job. That would also simplify the portal's check to see if an app install or uninstall operation should be allowed, and just means a bit more complexity for the migration from the currently used names of epiphany webapp desktop files.

Presumably something similar is possible for Snap. As for other packaging formats like deb/rpm, my understanding is those do not remove package configuration files unless apt-get remove --purge or similar is used, so I guess the only option is to require such apps to have a script to clean up the desktop files that gets run by the package manager upon uninstall. The only other solution I can think of is to have the portal delete them if the binary in their Exec= line ever disappears, but it's unclear when the portal would check that.

ramcq commented 2 years ago

To do some cleanup, you could provide a hook that Flatpak would run after its operations, like the icon cache update in the export folder. Another idea could be to set the TryExec to the binary path on the host, so that the desktop can just filter out stale entries if the app is uninstalled, so it becomes a bit less important to worry about when they are cleaned up?

mwleeds commented 2 years ago

I think Software could be updated to not consider access to ~/.local/share/applications/org.gnome.Epiphany.WebApp/ unsafe, using a check on that path similar to what the portal would do. Even if we don't have the app manager feature of Epiphany, Epiphany still needs to be able to see its web app desktop files to implement the GetInstalledWebApps method of the org.gnome.Epiphany.WebAppProvider service (on my development branch).

I suppose that might make sense. We could do the same for icons, then.

I don't know what exactly you're suggesting we do with icons? As far as I know the suggestion to put the icons in the web app's own data directory but not writable by the web app is the best suggestion so far.

Hmm, well experimentally with --own-name=org.gnome.Epiphany.* in the finish args and using g_desktop_app_info_new_from_filename and g_app_info_launch I was able to launch org.gnome.Epiphany.WebApp-reddit-27aa67d11878da3049675f218f9822318d27bf67.desktop, so I guess it works somehow?

Presumably nothing attempts to own the org.gnome.Epiphany.WebApp-reddit-27aa67d11878da3049675f218f9822318d27bf67 bus name?

From the --log-session-bus output it looks like something is substituting the hyphens with underscores: B5927: <- :1.24 call org.gtk.Actions.DescribeAll at /org/gnome/Epiphany/WebApp_reddit_27aa67d11878da3049675f218f9822318d27bf67

Honestly I don't see the point in looking into it too deeply since it is working.

What on-disk format are we talking about here? My understanding is that the launcher is a desktop file that is entirely under the control of the portal. Right?

That's the goal, but it's not currently the case. Currently Epiphany expects the desktop file to exist in the web app's data directory (under ~/.local/share/applications there is only a symlink) or the app will fail to start. It also has strict and inconsistent requirements for how everything is named. So part of this is figuring out what changes are required to make things work. Another trick is that sandboxed Epiphany must not be allowed to edit its desktop file or icon to avoid spoofing.

Michael is describing the status quo here. I'll describe what I'm proposing:

mwleeds commented 2 years ago

To do some cleanup, you could provide a hook that Flatpak would run after its operations, like the icon cache update in the export folder.

Yeah, the triggers Flatpak already runs do seem like the right mechanism for this.

Another idea could be to set the TryExec to the binary path on the host, so that the desktop can just filter out stale entries if the app is uninstalled, so it becomes a bit less important to worry about when they are cleaned up?

Good idea. I guess for Flatpaks we could use the wrapper script in e.g. ~/.local/share/flatpak/exports/bin/.

mwleeds commented 2 years ago

Another question is whether the title of the dialog should be configurable by the application. Currently the file chooser and print dialogs allow the sandboxed app to control the dialog title, but those dialogs have a very clear purpose regardless of the title, whereas it might be easier to trick the user into thinking this dynamic launcher portal is doing something else (seems unlikely though). So if we want to trade off a little UX for a little security, we could have the portal use a hard-coded title like "Create Application Launcher" when it's not a web app, and "Create Web Application" when it is, or just always use the former.

mcatanzaro commented 2 years ago

So if we want to trade off a little UX for a little security, we could have the portal use a hard-coded title like "Create Application Launcher" when it's not a web app, and "Create Web Application" when it is, or just always use the former.

"Create Application Launcher" sounds fine to me.

smcv commented 2 years ago

As for other packaging formats like deb/rpm, my understanding is those do not remove package configuration files unless apt-get remove --purge or similar is used

Even during apt purge, system-wide packages like .deb only remove system-wide configuration files. Reaching into individual users' home directories to remove per-user configuration would be considered a layering violation, a potential privilege escalation vulnerability (because the owner of the home directory could e.g. carry out symlink attacks on the package's prerm/postrm script), and potentially very wrong if the home directory is shared with other machines (or other OS installations on the same machine) where the package will remain installed.

mwleeds commented 2 years ago

I suppose we could have x-d-p clean up any stale desktop files when it is installing a new one, and determine staleness by checking if the TryExec binary exists. That would leave the possibility that stale launchers would exist for some time but at least not forever, if the portal is being used.

mcatanzaro commented 2 years ago

I think it's OK for the portal to delete files in the user's home directory because the portal will be running with user permissions (in contrast to rpm/dpkg which run as root) and so it shouldn't be able to delete anything the user could not delete anyway. i.e. there is no privilege boundary: no need to worry about the user tricking the portal.

Are you saying you are okay with losing the app manager functionality?

Missed this. Yes, I'm OK with losing that functionality.

mwleeds commented 2 years ago

I'm having some trouble figuring out how to handle the lifecycle of the token used by this portal. Unlike other portals that use tokens, this involves keeping the token alive until after a D-Bus call from a different process than the one that made the call to generate the token. In the case of the screen cast / remote desktop portal, the tokens are revoked if the connection closes (peer_died_cb in xdg-desktop-portal.c), but it's unclear to me how the connection is kept open in that case or how Software could keep it open for the duration of its call to Epiphany's D-Bus service to install the app.

Normally, a token can be revoked when it's successfully used, but in case of an error, like the gnome-software process crashing before using the token, it would be best to revoke the token in that case as well.

In case the flow is unclear, the steps I'm imagining are:

  1. Software acquires an install token using x-d-p's RequestInstallToken method
  2. Software gives the token to Epiphany over D-Bus.
  3. Epiphany gives the token to x-d-p via the Install method.

Anyone have advice?

mcatanzaro commented 2 years ago

Revoke the token after the Install method is called, or after some timeout elapses (five minutes?) to guard against client bugs?

wjt commented 2 years ago

The DBus connection is to the session bus. software will remain connected to the bus until exit.

mwleeds commented 2 years ago

5 minutes is the timeout used to revoke the token in the print portal implementation in xdg-desktop-portal-gnome so there's some precedent. I was hoping there'd be a cleaner solution but it seems like that's the way forward.

mwleeds commented 2 years ago

Does anyone see an issue with x-d-p putting the icons in the same directory as the desktop files, e.g. ~/.local/share/applications/org.gnome.Epiphany/? This would have the following advantages:

mcatanzaro commented 2 years ago

Does anyone see an issue with x-d-p putting the icons in the same directory as the desktop files, e.g. ~/.local/share/applications/org.gnome.Epiphany/?

Asides from it being really weird? I suppose not. I'm not a huge fan of the idea, though.

The --filesystem finish argument giving the app read access to the desktop files would also give it read access to the icons, which seems desirable.

If we do that, then we could simply give them read access to any other location, right?

Honestly, I'm not sure what the right answer is here, but requiring a --filesystem finish argument smells bad. The whole point of using a portal is to avoid the need to poke sandbox holes. I wonder if we can come up with a solution that doesn't require this. I guess the portal could simply provide some APIs to enumerate the installed applications' names and icons?

This simplifies the process for cleaning up these exports upon app uninstall; Flatpak could just delete ~/.local/share/applications/$APP_ID/ if it exists, if --delete-data was passed to the uninstall command. Since the other persistent data for the exported apps would also be in the parent app's data directory, --delete-data would in theory remove all traces of it.

I don't think it's hard to delete one location for the desktop files and a second location for the icons.

Counterproposal: put the icons and desktop files both under ~/.local/share/xdg-desktop-portal/applications, and just install symlinks in ~/.local/share/applications.

It means avoiding the complexity of trying to make the icon read-only but in the app's data directory, which I don't know how to do.

What if we export the icon over the portal's D-Bus API, instead of trying to make it available via the filesystem?

mwleeds commented 2 years ago

Does anyone see an issue with x-d-p putting the icons in the same directory as the desktop files, e.g. ~/.local/share/applications/org.gnome.Epiphany/?

Asides from it being really weird? I suppose not. I'm not a huge fan of the idea, though.

The --filesystem finish argument giving the app read access to the desktop files would also give it read access to the icons, which seems desirable.

If we do that, then we could simply give them read access to any other location, right?

Sure

Honestly, I'm not sure what the right answer is here, but requiring a --filesystem finish argument smells bad. The whole point of using a portal is to avoid the need to poke sandbox holes. I wonder if we can come up with a solution that doesn't require this. I guess the portal could simply provide some APIs to enumerate the installed applications' names and icons?

This simplifies the process for cleaning up these exports upon app uninstall; Flatpak could just delete ~/.local/share/applications/$APP_ID/ if it exists, if --delete-data was passed to the uninstall command. Since the other persistent data for the exported apps would also be in the parent app's data directory, --delete-data would in theory remove all traces of it.

I don't think it's hard to delete one location for the desktop files and a second location for the icons.

Counterproposal: put the icons and desktop files both under ~/.local/share/xdg-desktop-portal/applications, and just install symlinks in ~/.local/share/applications.

I guess this would have the advantage of making it trivial to distinguish x-d-p dynamic launchers from other desktop files, which seems good. But if it's weird to put the desktop files and icons together, should the icons be in ~/.local/share/xdg-desktop-portal/icons/?

It means avoiding the complexity of trying to make the icon read-only but in the app's data directory, which I don't know how to do.

What if we export the icon over the portal's D-Bus API, instead of trying to make it available via the filesystem?

Assume you mean the icon and the desktop file. We could do that but it does seem like a lot of additional code to write just to give an app access to something we could give it with one finish arg. The --filesystem route also has the advantage that it makes the access clear to the user whereas it's pretty opaque if it's through the portal. Another consideration is that you'd need more API to notify the app about when one of the launchers is removed, whereas if it's a directory the app could monitor it itself (but if we're dropping the app manager feature maybe we don't care).

Also, KDE apps need --filesystem=xdg-config/kdeglobals so dynamic launcher apps needing a --filesystem=xdg-data/... arg seems about the same. But on the other hand x-d-p being solely in charge of the desktop/icon directories does seem like a cleaner design...

Side note, I suppose the existing installed web apps aren't an issue since the feature is unsupported under Flatpak, and I think we don't have to worry about a case where Epiphany was installed as a non-Flatpak and then later installed as a Flatpak.

mwleeds commented 2 years ago

I guess we also need to consider what happens when an app is renamed to a different app ID. Since the portal uses the app ID to decide if new app installs or uninstalls are allowed, and since the content of the desktop files would also need modifying for the new app ID, the desktop/icon files would need to be migrated. The portal is the one that constructs the Exec= and TryExec= lines, and it's the one with write access, so it should probably be the one in charge of migrating them. So I guess either the app or Flatpak needs to let the portal know to do the migration (perhaps getting the old id from the desktop file's X-Flatpak-RenamedFrom), and if the app does it there's a period between when the rename update happens and when the app is next launched when the data would be in a bad state (in case the portal gains a method for enumerating the desktop files for example), but if Flatpak does it, I don't think there's any precedent for Flatpak calling out to x-d-p to accomplish something in a transaction...

mwleeds commented 2 years ago

If we did want to keep the migration purely in Flatpak without calling out to the portal, it wouldn't be terribly complicated if the desktop and icon paths are well-known and the Exec= and TryExec= lines just need a simple substitution.

mcatanzaro commented 2 years ago

The --filesystem route also has the advantage that it makes the access clear to the user whereas it's pretty opaque if it's through the portal.

We might consider a future where any use of --filesystem results in the application being displayed as insecure in GNOME Software. I know we're not close to being there yet, but I think that's what we should be designing for. (Epiphany currently needs --filesystem for the ~/Downloads directory -- which could theoretically become a portal -- and for the Kerberos socket, which we could perhaps allowlist in flatpak itself.)

Side note, I suppose the existing installed web apps aren't an issue since the feature is unsupported under Flatpak, and I think we don't have to worry about a case where Epiphany was installed as a non-Flatpak and then later installed as a Flatpak.

I agree.

mcatanzaro commented 2 years ago

I guess we also need to consider what happens when an app is renamed to a different app ID.

Are you talking about Epiphany itself, not web apps? If Epiphany itself is renamed, then it's just a different app, and no compat is expected.

I cannot think of why the app ID of a web app might change during normal use. It does occasionally change when we determine the current data format is not good enough for whatever reason -- this happened very recently -- but this should hopefully be rare.

wjt commented 2 years ago

At a high level, I don't think a --filesystem= sandbox hole should be required for an app to create launchers – portals are meant to remove the need for such sandbox holes. (I don't know the details of the KDE case but that doesn't seem like something to replicate IMO.)

(There is a little-known feature of the desktop entry specification where (e.g.) /usr/share/applications/foo/bar.desktop is treated as the desktop file called foo-bar.desktop. So if there is something particularly desirable about keeping all exported desktop files in a directory, .local/share/applications/org.gnome.Epiphany/foo.desktop would be treated as org.gnome.Epiphany-foo.desktop. However I think things like GNOME Shell only consider $APP_ID.*.desktop as belonging to $APP_ID, not $APP_ID-*.desktop, and IIRC the LibreOffice components were renamed for this reason.)

I guess we also need to consider what happens when an app is renamed to a different app ID.

Are you talking about Epiphany itself, not web apps? If Epiphany itself is renamed, then it's just a different app, and no compat is expected.

If a Flatpak app is renamed, the old app ID gets a commit with the eol.rebase metadata key set to the new app ID, and libflatpak knows how to replace the old app ID with the new app ID and move/symlink (i forget which) the ~/.var/app/$OLD_ID to ~/.var/app/$NEW_ID if it exists. Compat is expected!


I'm not sure if this idea has been explored but you could imagine the app writing its .desktop files and icons in the normal way to .var/app/$APP_ID/data/{applications,icons} (i.e. $XDG_DATA_HOME/{applications,icons}) and then the portal would rewrite them when asked to. (This is analogous to how Flatpak exports desktop files and icons.)

wjt commented 2 years ago

(What is the rationale for Epiphany needing to be able to read the real ~/.local/share/applications?)

alexlarsson commented 2 years ago

I haven't really followed this discussion in detail, but I'd like to point out that it isn't enough for the flatpak instance to call the per-user portal to update things, because an app may be installed system-wide and the rename should affect all users, not just the current one.

wjt commented 2 years ago

The existing logic in Flatpak to handle renamed apps does the right thing. The old IDs are written to the app's deploy file, as well as its (static) exported .desktop files and the logic to rename existing data kicks in when the app is run (i.e. per user). I think the analogy here would be that any renaming logic would happen when the portal is launched, i.e. at login.

mwleeds commented 2 years ago

So the portal is already launched at login?

The migration for an app rename that happens while the user is logged in would still need to either be done by Flatpak or x-d-p would need a way to trigger the migration when the rename happens.

mwleeds commented 2 years ago

At a high level, I don't think a --filesystem= sandbox hole should be required for an app to create launchers – portals are meant to remove the need for such sandbox holes. (I don't know the details of the KDE case but that doesn't seem like something to replicate IMO.)

Slight nitpick: the proposal was for the read-only sandbox hole to be required for Epiphany to provide its app manager interface and its WebAppProvider D-Bus service, not to create the launchers. We can add portal methods to provide read access to the launchers instead, maybe that's worth the complexity.

(There is a little-known feature of the desktop entry specification where (e.g.) /usr/share/applications/foo/bar.desktop is treated as the desktop file called foo-bar.desktop. So if there is something particularly desirable about keeping all exported desktop files in a directory, .local/share/applications/org.gnome.Epiphany/foo.desktop would be treated as org.gnome.Epiphany-foo.desktop. However I think things like GNOME Shell only consider $APP_ID.*.desktop as belonging to $APP_ID, not $APP_ID-*.desktop, and IIRC the LibreOffice components were renamed for this reason.)

Yeah, that's already the plan, to use org.gnome.Epiphany-webappname.desktop file IDs and have them be in a org.gnome.Epiphany directory. Is there a reason GNOME Shell needs to view the launchers as belonging to the same app?

I guess we also need to consider what happens when an app is renamed to a different app ID.

Are you talking about Epiphany itself, not web apps? If Epiphany itself is renamed, then it's just a different app, and no compat is expected.

If a Flatpak app is renamed, the old app ID gets a commit with the eol.rebase metadata key set to the new app ID, and libflatpak knows how to replace the old app ID with the new app ID and move/symlink (i forget which) the ~/.var/app/$OLD_ID to ~/.var/app/$NEW_ID if it exists. Compat is expected!

I'm not sure if this idea has been explored but you could imagine the app writing its .desktop files and icons in the normal way to .var/app/$APP_ID/data/{applications,icons} (i.e. $XDG_DATA_HOME/{applications,icons}) and then the portal would rewrite them when asked to. (This is analogous to how Flatpak exports desktop files and icons.)

I think it's desirable for the app to not have write access to its launchers to maintain the security properties we want, so I don't see how they could be in the app's data directory or what advantage that would provide?

(What is the rationale for Epiphany needing to be able to read the real ~/.local/share/applications?)

The way this was originally planned, Software calls out to Epiphany to see what web apps it has installed. If we want to avoid Epiphany having read access, the options are:

Personally I think Epiphany's app manager is a nice feature and it would be sad to see it go. It would be possible to implement it using the portal for read access to the launchers, but I'm also a bit worried about this project falling behind schedule now that it's clear the design was not sufficiently well thought through...

wjt commented 2 years ago

At a high level, I don't think a --filesystem= sandbox hole should be required for an app to create launchers – portals are meant to remove the need for such sandbox holes. (I don't know the details of the KDE case but that doesn't seem like something to replicate IMO.)

Slight nitpick: the proposal was for the read-only sandbox hole to be required for Epiphany to provide its app manager interface and its WebAppProvider D-Bus service, not to create the launchers. We can add portal methods to provide read access to the launchers instead, maybe that's worth the complexity.

Would it be sufficient to add a portal method that returns just a list of (not rewritten) desktop file IDs?

(There is a little-known feature of the desktop entry specification where (e.g.) /usr/share/applications/foo/bar.desktop is treated as the desktop file called foo-bar.desktop. So if there is something particularly desirable about keeping all exported desktop files in a directory, .local/share/applications/org.gnome.Epiphany/foo.desktop would be treated as org.gnome.Epiphany-foo.desktop. However I think things like GNOME Shell only consider $APP_ID.*.desktop as belonging to $APP_ID, not $APP_ID-*.desktop, and IIRC the LibreOffice components were renamed for this reason.)

Yeah, that's already the plan, to use org.gnome.Epiphany-webappname.desktop file IDs and have them be in a org.gnome.Epiphany directory.

Got it!

Is there a reason GNOME Shell needs to view the launchers as belonging to the same app?

Actually having typed all that out i went and checked the code and it's more complicated than I remembered.

https://gitlab.gnome.org/GNOME/gnome-shell/blob/cfd03885848046af45b7116a7dfe207c071da845/src/shell-window-tracker.c#L132-215

I guess we also need to consider what happens when an app is renamed to a different app ID.

Are you talking about Epiphany itself, not web apps? If Epiphany itself is renamed, then it's just a different app, and no compat is expected.

If a Flatpak app is renamed, the old app ID gets a commit with the eol.rebase metadata key set to the new app ID, and libflatpak knows how to replace the old app ID with the new app ID and move/symlink (i forget which) the ~/.var/app/$OLD_ID to ~/.var/app/$NEW_ID if it exists. Compat is expected! I'm not sure if this idea has been explored but you could imagine the app writing its .desktop files and icons in the normal way to .var/app/$APP_ID/data/{applications,icons} (i.e. $XDG_DATA_HOME/{applications,icons}) and then the portal would rewrite them when asked to. (This is analogous to how Flatpak exports desktop files and icons.)

I think it's desirable for the app to not have write access to its launchers to maintain the security properties we want, so I don't see how they could be in the app's data directory or what advantage that would provide?

These files would not be used directly and would not be in $XDG_DATA_DIRS on the host. The app would write these files (just in the same way it would if it were not sandboxed), then pass a fd for the .desktop file and icon to the portal. The portal would read from both, sanitise the contents, get user confirmation, then write the sanitised content to the real $XDG_DATA_HOME.

I'm not sure this helps anything.

(What is the rationale for Epiphany needing to be able to read the real ~/.local/share/applications?)

The way this was originally planned, Software calls out to Epiphany to see what web apps it has installed. If we want to avoid Epiphany having read access, the options are:

  • Have Software talk to the portal instead, and find a way to provide the URL and install-date which are both available to Epiphany but not completely trivial to infer from the desktop file.

Add X-WebApp-URL= to the file? install-date could be determined from the file's mtime?

  • Have Epiphany talk to the portal in the implementation of GetInstalledWebApps, so it would be a D-Bus call that leads to another D-Bus call, a pattern we already have elsewhere.

This doesn't seem so bad.

Personally I think Epiphany's app manager is a nice feature and it would be sad to see it go. It would be possible to implement it using the portal for read access to the launchers, but I'm also a bit worried about this project falling behind schedule now that it's clear the design was not sufficiently well thought through...

mcatanzaro commented 2 years ago

Would it be sufficient to add a portal method that returns just a list of (not rewritten) desktop file IDs?

I think it requires: (ID, name, icon).

Is there a reason GNOME Shell needs to view the launchers as belonging to the same app?

I think we're just misunderstanding each other here. GNOME Shell surely wants to see each launcher as a separate app.

mwleeds commented 2 years ago

Would it be sufficient to add a portal method that returns just a list of (not rewritten) desktop file IDs?

I think it requires: (ID, name, icon).

Actually just the ID and name would be enough, if we expect Software to gather the rest of the info from the desktop file, but the icon would be useful in case the app manager (about:applications) is ever re-implemented after being dropped.

Is there a reason GNOME Shell needs to view the launchers as belonging to the same app?

I think we're just misunderstanding each other here. GNOME Shell surely wants to see each launcher as a separate app.

Yeah, I understand that now. Taking that requirement and the code Will linked to, it seems to follow that the desktop file IDs must be prefixed with org.gnome.Epiphany. not org.gnome.Epiphany-, and this will mean gnome-shell will associate them with their own desktop file, and use the launcher-specific icon, rather than returning NULL from get_app_from_window_wmclass(). Epiphany also already sets StartupWMClass= in its .desktop files.

(This scheme does mean apps from org.gnome.Epiphany.Devel will be treated completely separately and only work with Devel Epiphany, but that seems ok. It seems pretty unavoidable, since the desktop ID has to have the main app ID followed by a . as a prefix.)

mwleeds commented 2 years ago

I guess we also need to consider what happens when an app is renamed to a different app ID.

Are you talking about Epiphany itself, not web apps? If Epiphany itself is renamed, then it's just a different app, and no compat is expected.

If a Flatpak app is renamed, the old app ID gets a commit with the eol.rebase metadata key set to the new app ID, and libflatpak knows how to replace the old app ID with the new app ID and move/symlink (i forget which) the ~/.var/app/$OLD_ID to ~/.var/app/$NEW_ID if it exists. Compat is expected! I'm not sure if this idea has been explored but you could imagine the app writing its .desktop files and icons in the normal way to .var/app/$APP_ID/data/{applications,icons} (i.e. $XDG_DATA_HOME/{applications,icons}) and then the portal would rewrite them when asked to. (This is analogous to how Flatpak exports desktop files and icons.)

I think it's desirable for the app to not have write access to its launchers to maintain the security properties we want, so I don't see how they could be in the app's data directory or what advantage that would provide?

These files would not be used directly and would not be in $XDG_DATA_DIRS on the host. The app would write these files (just in the same way it would if it were not sandboxed), then pass a fd for the .desktop file and icon to the portal. The portal would read from both, sanitise the contents, get user confirmation, then write the sanitised content to the real $XDG_DATA_HOME.

I'm not sure this helps anything.

Yeah that doesn't seem to have advantages over the current design, where the app passes the desktop entry as a string and the icon as an fd.

(What is the rationale for Epiphany needing to be able to read the real ~/.local/share/applications?)

The way this was originally planned, Software calls out to Epiphany to see what web apps it has installed. If we want to avoid Epiphany having read access, the options are:

  • Have Software talk to the portal instead, and find a way to provide the URL and install-date which are both available to Epiphany but not completely trivial to infer from the desktop file.

Add X-WebApp-URL= to the file?

Could do that

install-date could be determined from the file's mtime?

Yeah, that is what Epiphany currently does, so it wouldn't be any worse. There's a comment "/ FIXME: this should use TIME_CREATED but it does not seem to be working. /"

  • Have Epiphany talk to the portal in the implementation of GetInstalledWebApps, so it would be a D-Bus call that leads to another D-Bus call, a pattern we already have elsewhere.

This doesn't seem so bad.

Yeah, conceptually apps being the authority on what apps they have installed and providing additional info about them makes sense.

mwleeds commented 2 years ago

I'm hesitant to put the .desktop files in ~/.local/share/flatpak/exports/share since

However ~/.local/share/flatpak/exports/share does have the advantage of already being in $XDG_DATA_DIRS, so if we want to use ~/.local/share/xdg-desktop-portal/applications we have to either put that in $XDG_DATA_DIRS like Flatpak does, or make a sym link in ~/.local/share/applications. The latter seems simpler.

So here's my proposal:

(Of course using g_get_user_data_dir () not hard-coding ~/.local/share)

mwleeds commented 2 years ago

That's all assuming the correct behavior for this portal is to create a launcher for the current user even if the parent app is installed for all users. If we wanted the launchers to be made system-wide in that case, we'd probably need something like the flatpak-system-helper to make it possible.

I wonder if "install this system-wide" should be an option in the portal interface, or if it should be inherited from the location of the parent app.

If the launchers are user-specific it is a divergence of behavior from the default for Flatpak, deb, rpm, etc. Currently the only way for a user to choose between per-user and system-wide in Software is with the Sources drop-down, in case remotes are configured in both installations that provide the same thing.

BrainBlasted commented 2 years ago

For most dynamic launchers, I think the average user would expect them to be per-user. If I install a webapp with e.g. Tangram, I don't expect that to be exposed to every other user on the computer. Is there any particular reason why they should be system-wide?

mwleeds commented 2 years ago

For most dynamic launchers, I think the average user would expect them to be per-user. If I install a webapp with e.g. Tangram, I don't expect that to be exposed to every other user on the computer. Is there any particular reason why they should be system-wide?

The arguments for making them system-wide would be

But maybe you're right, maybe per-user is the expected behavior.

I brought this question up here as well.

mcatanzaro commented 2 years ago

My vote would be for the dynamic launchers to always be per-user. That seems more likely to be what users expect, even if the base app is installed system-wide.

smcv commented 2 years ago

My vote would be for the dynamic launchers to always be per-user. That seems more likely to be what users expect, even if the base app is installed system-wide.

I agree. Even if system-wide availability is required at some point in future, it can be something that apps specifically ask for, in a "version 2" interface - but I don't think anyone is going to want it.

When Steam or Epiphany (GNOME Web) create dynamic launchers at the moment (while installed as native desktop apps from something like .deb or .rpm outside Flatpak), they do so per-user, because they are doing so in response to a per-user request, and they cannot assume that they have sysadmin-level privileges.

For Steam, the dynamic launchers point to games installed in the user's Steam library (typically in ~/.local/share/Steam/steamapps/common, or perhaps somewhere like /srv/big-hdd/steamapps/common). Other Unix uids will typically not even have access to that location, and even if they do, they will typically not have access to the installing user's Steam account(s): each Steam installation is private to one Unix uid, even if it is shared by multiple Steam users.

For Epiphany, dynamic launchers are created by a user asking for a particular website (typically a webapp like Element) to be treated like an app. This is a per-user decision, and it's backed by a special Epiphany profile in ~/.local/share/org.gnome.Epiphany.WebApp-NAME-HEX, which, again, other users do not have access to even if they wanted it.

mwleeds commented 2 years ago

For Epiphany, dynamic launchers are created by a user asking for a particular website (typically a webapp like Element) to be treated like an app. This is a per-user decision, and it's backed by a special Epiphany profile in ~/.local/share/org.gnome.Epiphany.WebApp-NAME-HEX, which, again, other users do not have access to even if they wanted it.

If we wanted the launchers to be system-wide I would assume we'd make Epiphany create the profile directory on launch so each user has their own. But I hear that the consensus is to do per-user launchers and I'm happy to carry on with that requirement. Thanks for the input!