interfaces: add an interface for gnome-online-accounts D-Bus service

[jhenstridge]

Add an interface exposing the GNOME online accounts system, which provides access to accounts configured through the control center. I've added this as a new gnome-online-accounts-service interface because:

1. the underlying D-Bus service is not designed with confinement in mind: at the AppArmor level, we either expose everything or nothing. So it doesn't make sense to expose this by default through desktop.

2. the existing online-accounts-service interface covers the Ubuntu Online Accounts version 2 API designed for the phone. This service was designed with confinement in mind, so the interface was a candidate for wide auto connection.

So I've put this in its own interface for now. Let me know if you want these rules moved elsewhere.

To test, I've put together this small snap containing the "list-accounts" example program from gnome-online-accounts:

http://people.canonical.com/~jamesh/goa-test_0.1_amd64.snap

With the interface disconnected, goa-test.list-accounts segfaults. After running snap connect goa-test:gnome-online-accounts-service, it should be able to enumerate the registered accounts.

[zyga]

Hey @jhenstridge, thank you for the interface and the test. I will review it now. My suggestion on the test snap is to rename it to test-snapd-gnome-online-accounts, add it to the tree (as source) and work with @mvo5 to upload it to the store for testing.

[zyga]

Looks good in general.

Before merging this, please work on uploading the test snap into the store and adding the source code to tests/lib/snaps. Please also consider changing the summary of the interface as I think it is not accurate at the moment.

[zyga]

I think this allows either to operate as GNOME Online Accounts service or to talk to it, correct?

[zyga]

This is only true on classic. On core this would be confined. If this interface is only for classic then I'd suggest renaming the summary to say something like allows interacting with the GNOME Online Accounts service.

[jdstrand]

FYI, this is only on classic (implicitOnCore is not set). The summary of the language looks fine AFAICT in this context.

[zyga]

implicitOnCore can be left out as it defaults to false.

[jhenstridge]

Thanks for the review comments. I had adapted the interface summary from the existing online-accounts-service interface, which was written with the intention of using it on core.

This interface is pretty much exclusively designed for classic at this point: the API is very much all or nothing. If it was reworked to make it confinement-friendly, we'd probably want to merge it into the desktop interface. While it remains unsafe, it probably makes sense to be reserved for OS and implicit on classic only.

I've also added the source for the test snap to the tree.

[zyga]

+1 once you add a spread test for this.

Please work with @sergiocazzolato to get the test snap in the store.

[zyga]

@jhenstridge tests are failing on

FAIL: basedeclaration_test.go:616: baseDeclSuite.TestConnection
basedeclaration_test.go:649:
c.Check(err, IsNil, comm)
... value *errors.errorString = &errors.errorString{s:"connection denied by slot rule of interface "gnome-online-accounts-service""} ("connection denied by slot rule of interface "gnome-online-accounts-service"")
... gnome-online-accounts-service

[codecov-io]

Codecov Report

Merging #4140 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #4140      +/-   ##
==========================================
+ Coverage   78.04%   78.04%   +<.01%     
==========================================
  Files         450      451       +1     
  Lines       30904    30913       +9     
==========================================
+ Hits        24118    24127       +9     
  Misses       4774     4774              
  Partials     2012     2012

Impacted Files	Coverage Δ
...nterfaces/builtin/gnome_online_accounts_service.go	100% <100%> (ø)

---

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 6257bb7...8a876c6. Read the comment docs.

[mvo5]

Looks good to me, once @jdstrand approves this can go in

[zyga]

@sergiocazzolato can you please confirm if the new test snap is uploaded to the store?

[sergiocazzolato]

@zyga, to upload the snaps to the store, @mvo5 is the right person

[mvo5]

I uploaded the amd64 version of the snap to the store now and shared with Sergio. If more people need access please let me know.

[jdstrand]

Thanks for the PR! :)

One question and a couple of comment suggestions but otherwise LGTM. This interface grants complete access to GOA, so the base declaration denying auto-connection is spot on.

[jdstrand]

The base declaration looks fine.

[zyga]

This is only true on classic. On core this would be confined. If this interface is only for classic then I'd suggest renaming the summary to say something like allows interacting with the GNOME Online Accounts service.

[jdstrand]

FYI, this is only on classic (implicitOnCore is not set). The summary of the language looks fine AFAICT in this context.

[jdstrand]

This also allows 'Set()', is that intended? If so, perhaps # Allow getting/setting properties on accounts.

[jdstrand]

's/Allow access to/Allow use of all/'?

[jdstrand]

It would be nice if this didn't have to be installed over the network, but I guess there is currently no way to create a snap with compiled code in tests/lib/snaps....

[jdstrand]

@jhenstridge - once you decide what to do with my comments, this can go in (based on @mvo5's previous comment). After it is in, I suggest creating a PR against release/2.30 (ideally by Monday) so this can go in this month.

[jdstrand]

The test failures are related:
2017-12-07 15:57:13 Failed tasks: 2
- linode:ubuntu-14.04-64:tests/main/interfaces-gnome-online-accounts-service
- linode:ubuntu-16.04-32:tests/main/interfaces-gnome-online-accounts-service

[jhenstridge]

I'm not exactly sure why the ubuntu-14.04-64 check fails. It is quite possible that the version of gnome-online-accounts in 14.04 isn't protocol compatible with the version in 16.04, so perhaps skipping old releases would make sense.

For the ubuntu-16.04-32 failure, it looks like I built the i386 snap wrong. I used snapcraft cleanbuild --target-arch i386, which claims to handle cross compilation but seemed to produce a 64-bit executable instead.

[jdstrand]

@jhenstridge - thanks! Can you fix up the conflict?

[pedronis]

what would happen to snap on 14.04 that needs the interface? it can it be made compatible with the old service?

[jdstrand]

@pedronis - that is a great observation. In terms of snapd security policy, I suspect there would be no denials since this policy is sufficiently broad.

However your question is very interesting because snapd interfaces are contracts between the slot side and the plugs side. With implicit classic interfaces, the slot side is whatever happens to be running on classic so 14.04 classic distro isn't in a good position to honor the slot side of the contract with series 16 snaps, particularly with fast-moving software like GNOME. Your observation is not limited to gnome-online-accounts, but to really any of the implicit classic interfaces (of course, 14.04 will be able to honor many of them; DBus APIs would be the most problematic).

This was definitely brought up in other contexts-- eg, there is no guarantee that a series 16 snap can work with whatever NetworkManager happens to be running on an arbitrary classic distro.

[jhenstridge]

So if I understand it right, this interface should be no different to any other "implicit on classic" interface that grants access to a D-Bus service (e.g. NetworkManager). If we're not making API guarantees for those interfaces, is it actually a problem for this interface?

[jdstrand]

@jhenstridge - my comments were not meant to be construed as a blocker but to acknowledge the difficulties with implicit interfaces. My approval for this PR remains.