Bug 27752

(In reply to comment #0)
> This partially blocks Bug #27669.
> 
> * There's no way to tell whether locations can be set.
>   For instance, on XMPP we set locations in PEP, so we need to
>   do something sensible on Google Talk servers (which don't have
>   PEP). Google Talk users can still retrieve PEP-based Location
>   from non-Google contacts.
> 
>   Proposal A: boolean CanSetLocation property
>   Proposal B: if LocationAccessControlTypes is the empty list,
>       then that means you can't set location
> 
I would go for Proposal A.

> * Clients can't tell when LocationAccessControl has changed.
> 
>   Proposal: LocationAccessControlChanged (u: Type, v: Detail)
> 
+1

> * It's unspecified whether/when LocationAccessControlTypes can change.
> 
>   Proposal: before StatusChanged(CONNECTED, .) it's meaningless;
>       after that it can never change. (But note that this would require
>       Gabble to query whether we have a PEP server before going CONNECTED...)
> 
> * I'm not sure that having separate setters for the access control type
>   and the location is the right model; since geolocation is such
>   sensitive information, we should perhaps set the access control and
>   the location at the same time?
> 
>   In particular, the first SetLocation on a Connection is specified to set the
>   access control to an empty whitelist if possible (though I don't know whether
>   Gabble actually does this); if you want anyone to actually see your location,
>   you'll need to set an access control model.
> 
>   Proposal: SetLocationAndControl(a{sv}, u, v)
+1

I'll spec this up. I prefer proposal A too, FWIW.

Spec meeting agenda.

HTML: http://people.freedesktop.org/~smcv/telepathy-spec-location/spec/org.freedesktop.Telepathy.Connection.Interface.Location.html

gitweb: http://git.collabora.co.uk/?p=user/smcv/telepathy-spec-smcv.git;a=shortlog;h=refs/heads/location

---------------

From the department of reality-compliance:

Someone will need to make sure this can be implemented in Gabble before we merge it. A suitable dummy implementation would be:

* decide the value of CanSetLocation before going CONNECTED and store it in the GabbleConnection (but don't hook it up to D-Bus)

* confirm that "Before publishing location for the first time, if this has not been set by a client, implementations SHOULD set it to be as restrictive as possible" is actually implemented

* confirm that setting both the location and the location access-control type works

* write a dummy implementation of SetLocationAndControl (bonus points if you make SetLocation and Set(Location, AccessControl) be implemented in terms of it!)

Deprecating SetLocation() is unnecessary.

Qt API can have two-and-a-bit methods:

 • setAccessControl(AccessControl foo);
 • setLocation(QVariantMap location);
 • setLocation(QVariantMap location, AccessControl foo).

I've updated the branch based on spec-meeting comments.

(In reply to comment #3)
> HTML:
> http://people.freedesktop.org/~smcv/telepathy-spec-location/spec/org.freedesktop.Telepathy.Connection.Interface.Location.html
> 
> gitweb:
> http://git.collabora.co.uk/?p=user/smcv/telepathy-spec-smcv.git;a=shortlog;h=refs/heads/location

This has bit-rotted rather. I've rebased the branch to bring it up to date with master; CanSetLocation is no longer needed, since wjt added Can_Set.

> Someone will need to make sure this can be implemented in Gabble before we
> merge it. A suitable dummy implementation would be:
> 
> * decide the value of CanSetLocation before going CONNECTED and store it in the
> GabbleConnection (but don't hook it up to D-Bus)

This was eventually done for Can_Set.

> * confirm that "Before publishing location for the first time, if this has not
> been set by a client, implementations SHOULD set it to be as restrictive as
> possible" is actually implemented

To be done.

> * confirm that setting both the location and the location access-control type
> works

To be done.

> * write a dummy implementation of SetLocationAndControl (bonus points if you
> make SetLocation and Set(Location, AccessControl) be implemented in terms of
> it!)

To be done.

This is much messier than I'd realised, if you go beyond Gabble's relatively simple implementation.

(In reply to comment #0)
> * Clients can't tell when LocationAccessControl has changed.

Not actually our bug. In XMPP, you can only tell what the access control model is by doing a network round-trip (for a Data Form... everyone's favourite data structure), and you can't tell when it changed.

Possibly we could redefine LocationAccessControl to mean "what the access control would be if you called SetLocation right now", and introduce a RequestLocationAccessControl which waits for the round-trip, and sets LAC as a side-effect.

> * I'm not sure that having separate setters for the access control type
>   and the location is the right model; since geolocation is such
>   sensitive information, we should perhaps set the access control and
>   the location at the same time?

Unfortunately, there's a race (potentially revealing old or new location to people you didn't intend to receive it) unless you do this:

* set location to nothing
* [1] fetch configuration form
* wait for reply to [1]
* [2] fill in and submit configuration form
* wait for reply to [2], if you care about the possibility that you could reveal your location because it failed
* set location to new value

Since we only implement Location in Gabble, we should probably make the spec follow the underlying protocol.

>   In particular, the first SetLocation on a Connection is specified to set the
>   access control to an empty whitelist if possible (though I don't know whether
>   Gabble actually does this)

Gabble behaves as though XMPP only supported the default "presence" access control model. If another of our resources sets the access control to something more permissive, we'll never know.

(In reply to comment #7)
> (In reply to comment #0)
> > * Clients can't tell when LocationAccessControl has changed.
> 
> Not actually our bug. In XMPP, you can only tell what the access control model
> is by doing a network round-trip (for a Data Form... everyone's favourite data
> structure), and you can't tell when it changed.

... unless the pubsub#notify_config option is set to true, in which case all subscribers to the node (!), including the publisher, will be notified. XEP-0163 is silent on whether pubsub#notify_config has to be supported, or on by default, on PEP servers.

PEP servers are required to support at least the "presence", "open", "roster" and "whitelist" access models (which are our RPACT_Publish, RPACT_Open, RPACT_Roster (but possibly with multiple groups) and RPACT_Whitelist), but MAY support others. Discovering what's allowed takes a round-trip, which we'd have to do before CONNECTED.

Unassigning; I'm not actively working on this and it seems to be full of bees. Someone with a particular interest in Location could pick this up.

Created attachment 57453 [details] [review]
Add rationale for SupportedLocationFeatures, Can_Set

Taken from my earlier branch which added a separate CanSet boolean
property.

---

This is a leftover from the branch I had on this bug earlier; it's useful to merge even if we don't solve anything else here.

-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/telepathy/telepathy-spec/issues/68.