@ogra I added the file now (http://paste.ubuntu.com/24420390/). There appears to be no filtering happening, am I using this wrong? I still suspect this needs application support but I’m happy to be convinced otherwise.
no, i would expect this to work (and i know we used PK like this a lot on phones, even with just scripts as backends, that had no particular extra integration). perhaps someone from the desktop team can chime in here.
Note that polkit is not a wrapper. It is a mechanism that a service is deliberately coded to use so that the service may make decisions based on polkit’s response. In other words, services must be designed to use polkit: the methods they care to protect all call out to polkit to ask if it is ok, polkit responds yes or no based on the policy defined by the application or overridden by the admin, and then the service makes a decision based on that response (eg, allowing the action or telling the client it was not allowed by polkit). Polkit is used to protect privileged resources that are running on the system bus (see ‘man polkit’) and indeed, there is no org.freedesktop.PolicyKit1 service running on the session bus for session services to use (you can verify in /usr/share/dbus-1/services). xdg-open runs in the user’s session and traditionally anything in the user’s session is trusted. I’m not aware of any polkit integration with xdg-open (and I spot checked the code).
As for the phone, polkit was crippled in that while it was present, it only supported enforcement and didn’t support prompting. I suspect what was used in shell scripts on the phone was pkexec which allows running the command as another user (eg, root) and it is possible to say ‘pkexec /usr/bin/foo’ and ‘pkexec /usr/bin/bar’ and have different policy for authorizing use of foo and bar. See ‘man pkexec’ for details.
2 Likes
Thanks @jdstrand! This is my understanding as well how polkit works. Happy to see this clarified and that we can put the polkit discussion at rest.
I really don’t see what’s wrong with our current approach for XDG stuff. The only thing I have a problem with is the name of the D-Bus path being com.canonical.* instead of being like everything else in using io.snapcraft.*.
I’m extremely annoyed by the cavalier attitude to all the desktop stuff. Yes, I get that snaps weren’t designed for the desktop case, but now that we have desktop apps on there, playing nice with users’ (and developers’) expectations should be something that we should strive for.
It took far too many years to get most (if not all) desktops to roughly conform to XDG specifications, and now that we’re here, we should leverage these things. The Portals concept from Flatpak offers a similar mechanism to what snapd-xdg-open does for desktop integration bits, and I fully expect to see more of this kind of stuff rather than less in the Snappy ecosystem.
Can we just skip all the bickering and go to fixing up snapd-xdg-open? The desktop people all basically agree that it’s sound, I certainly see no problems with the approach, and as far as I can tell, the only objection is that it’s not written in monster-Go to be part of the hydra that is the snapd repository.
Per the discussion above, we were exploring options with the goal of making it easier to implement it evenly and simply across all distributions, because our current approach seems to have failed to do exactly that so far. In different words: the reason this discussion even started is is precisely because the method “all desktop people agree” is still broken despite being available for a long time.
That said, this was an exploration. We haven’t found any mechanisms so far that wouldn’t open up different cans of worms. The feedback we got from several ends saying “let’s please keep the current method” is also part of that process and being heard. So, sure: what do you suggest we do next to fix up the current mechanism? If the idea is integrating the tool into the snapd repository, I’d like to have something slightly more generic. Some “snapd-foo-bar” that may do more than simply opening links in the future, as we don’t want a new tool for every trivial need like this. Suggestions? “snapd-user-helper”?
Quick question, is there any issue apart from packaging? I wasn’t aware of any bugs in that code.
No that I know. snapd-xdg-open as is should work.
I would suggest a two step approach:
- Merge https://github.com/snapcore/snapd/pull/3118 to have snapd-xdg-open directly included in the current form into snapd. With that we can include it directly into the snapd packaging and mix it with the existing autotools/go/systemd infrastructure with no additional cost for other distributions other than having to ship an additional dbus-activation configuration file. In that go we can also change the DBus bus name as @Conan_Kudo suggested from
com.canonical.*toio.snapcraft.* - We rework
snapd-xdg-openitself so it integrates better with the rest of the code. My proposal would be asnapd --userswitch which allows us to start a second snapd instance within a different context which will take care of different things. For this step this would only mean to exactly replicate what snapd-xdg-open did and nothing else. In the future we can add additional xdg DBus interfaces or any other user session related bits in here. This can be either a dbus-activated service or one started by the user session init system which is then abstracted through proper packaging but systemd/upstart should fit almost everything.
@niemeyer If that sounds good to you and also for everyone else I can come with a more detail proposal / PR for 2 once I am through other bits of my work list. 1. is something we can do very easily today and bring snapd-xdg-open back into all distributions with very low cost for packaging and a good integration story.
I’m curious about this. Do you plan to keep the current C code around or really spawn a 2nd copy of snapd that has (perhaps) different APIs (e.g. to expose DBus services).
I would vote for a second snapd instance which doesn’t load/provide anything of what the system instance does. It will load a different mode which provides for now just the dbus endpoint snapd-xdg-open provided before. Over time we can grow that in different other things we may want to introduce for the user session (like the xdg-desktop portals @sitter proposed here). If the implementation is in C or Go, I don’t really care but giving the nature of the existing snapd code base Go sounds the way to go.
Is there a nice Go DBus library we could use?
Sounds good, except perhaps for the idea of merging and pushing snapd-xdg-open as it stands first, for similar reasons to the ones discussed above: this is a tiny application, and should be easy for us to move ahead with this approach. I’d prefer to not invest our time, and that of contributors that need to integrate the tool in each of the distributions, just to then change it all shortly after.
Can we go ahead and just do the trivial application under snapd --user in the first place?
Sure, let me implement that and propose a PR.
2 Likes
A first WIP PR for the snapd --user instance is now available at https://github.com/snapcore/snapd/pull/3260
2 Likes
Reworking the mentioned PR at the moment as I agreed with @niemeyer to add the daemon as snap userd instead of snapd --user as it is really a client of snapd like the snap command and not something which is the user session equivalent of snapd.
i’d like to know when we can move forward with https://github.com/snapcore/core/pull/35 despite being orthogonal @niemeyer asked me to hold back landing it til there was a decision for the other part of the implementation.
given that this is moving forward, can we land this PR to finally get rid of all the PATH hacks it causes in other places ?
I am +1 as the PR doesn’t change anything what we’re doing for snap userd. The only thing we have to change once snap userd lands is to support both service names.
Looks fine to me as well.
1 Like