Bug 27752 - Deficiencies in Location setting API
Summary: Deficiencies in Location setting API
Status: RESOLVED MOVED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: tp-spec (show other bugs)
Version: git master
Hardware: Other All
: medium normal
Assignee: Telepathy bugs list
QA Contact: Telepathy bugs list
URL: http://cgit.freedesktop.org/~smcv/tel...
Whiteboard: needs work, XMPP is hard
Keywords:
Depends on:
Blocks: 27669
  Show dependency treegraph
 
Reported: 2010-04-20 07:20 UTC by Simon McVittie
Modified: 2019-12-03 20:21 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
Add rationale for SupportedLocationFeatures, Can_Set (2.95 KB, patch)
2012-02-22 05:03 UTC, Simon McVittie
Details | Splinter Review

Description Simon McVittie 2010-04-20 07:20:03 UTC
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

* Clients can't tell when LocationAccessControl has changed.

  Proposal: LocationAccessControlChanged (u: Type, v: Detail)

* 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)
Comment 1 Andre Moreira Magalhaes 2010-04-20 14:14:28 UTC
(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
Comment 2 Simon McVittie 2010-04-21 05:22:50 UTC
I'll spec this up. I prefer proposal A too, FWIW.
Comment 3 Simon McVittie 2010-04-21 05:53:51 UTC
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!)
Comment 4 Will Thompson 2010-04-21 09:26:31 UTC
Deprecating SetLocation() is unnecessary.

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

 • setAccessControl(AccessControl foo);
 • setLocation(QVariantMap location);
 • setLocation(QVariantMap location, AccessControl foo).
Comment 5 Simon McVittie 2010-04-21 12:04:22 UTC
I've updated the branch based on spec-meeting comments.
Comment 6 Simon McVittie 2010-06-08 03:23:19 UTC
(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.
Comment 7 Simon McVittie 2010-08-04 08:03:43 UTC
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.
Comment 8 Simon McVittie 2010-08-04 08:18:20 UTC
(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.
Comment 9 Simon McVittie 2010-08-09 07:44:31 UTC
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.
Comment 10 Simon McVittie 2012-02-22 05:03:29 UTC
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.
Comment 11 GitLab Migration User 2019-12-03 20:21:26 UTC
-- 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.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.