Bug 24908 (CommPolicy)

Summary: Communication policy (blocking policy) API
Product: Telepathy Reporter: Simon McVittie <smcv>
Component: tp-specAssignee: Eitan Isaacson <eitan.isaacson>
Status: RESOLVED MOVED QA Contact: Telepathy bugs list <telepathy-bugs>
Severity: enhancement    
Priority: medium CC: danielle, devurandom
Version: unspecified   
Hardware: Other   
OS: All   
Whiteboard: draft 1 in 0.21.1
i915 platform: i915 features:
Bug Depends on: 28423    
Bug Blocks: 24894    

Description Simon McVittie 2009-11-04 06:17:39 UTC
Maemo 5 uses various extended interfaces beyond what's in telepathy-spec. One such interface is an API for Skype's ability to block unknown contacts:

http://git.collabora.co.uk/?p=rtcom-telepathy-glib.git;a=blob;f=rtcom-telepathy-glib/Connection_Interface_Skype_CommunicationPolicy.xml

MSN would benefit from similar functionality.

We should incorporate this functionality into the main telepathy-spec.

One useful possibility would be to align the access control model with that of rich presence.
Comment 1 Simon McVittie 2009-11-17 10:29:17 UTC
The Privacy interface exists in telepathy-spec git but does not generate HTML (in all.xml it's annotated as "Never implemented, vague"). Its API is:

interface Connection.Interface.Privacy
    requires Connection

    An interface to support getting and setting privacy modes to configure
    situations such as not being contactable by people who are not on your
    subscribe list. If this interface is not implemented, the default can be
    presumed to be allow-all (as defined in GetPrivacyModes).

    method GetPrivacyMode () -> s
        Return the current privacy mode, which must be one of the values
        returned by GetPrivacyModes.

        out s: (unnamed): A string representing the current privacy mode
        raises Disconnected, NetworkError

    method GetPrivacyModes () -> as
        Returns the privacy modes available on this connection. The following
        well-known names should be used where appropriate:

        allow-all
            any contact may initiate communication
        allow-specified
            only contacts on your 'allow' list may initiate communication
        allow-subscribed
            only contacts on your subscription list may initiate communication

        out as: (unnamed): An array of valid privacy modes for this connection

    signal PrivacyModeChanged (s: Mode)
        Emitted when the privacy mode is changed or the value has been
        initially received from the server.

        s: Mode: The current privacy mode

    method SetPrivacyMode (s: Mode)
        Request that the privacy mode be changed to the given value, which
        must be one of the values returned by GetPrivacyModes. Success is
        indicated by the method returning and the PrivacyModeChanged
        signal being emitted.

        in s: Mode: The desired privacy mode
        raises Disconnected, NetworkError, PermissionDenied, InvalidArgument
Comment 2 Eitan Isaacson 2010-07-07 12:00:37 UTC
I am trying to gauge the general sentiments around this interface.

Simon, what did you mean by aligning the access control model with rich presence? Is this another interface? Isn't rich presence broken up by feature? (Location, etc).

I think I understood Will say in passing that the decoupling of calls/chat is wrong. Should there be one mode for all forms of communications without granularity?
Comment 3 Will Thompson 2010-07-07 16:00:21 UTC
(In reply to comment #2)
> I think I understood Will say in passing that the decoupling of calls/chat is
> wrong. Should there be one mode for all forms of communications without
> granularity?

I just suspect there are protocols where you can't change policy for different modes of communication separately.
Comment 4 Simon McVittie 2010-07-12 05:58:39 UTC
(In reply to comment #2)
> Simon, what did you mean by aligning the access control model with rich
> presence? Is this another interface? Isn't rich presence broken up by feature?
> (Location, etc).

The way Location's access control works is that there's a property of type Rich_Presence_Access_Control, which is a tagged union: a struct (uv) containing a Rich_Presence_Access_Control_Type (an enum), plus a variant for extra info if needed. There's also a property of type Rich_Presence_Access_Control_Type[] listing the enum members that make sense in this implementation.

R_P_A_C is defined by SimplePresence, which doesn't use it itself. The idea is that future Mood, Music, HatColour, etc. interfaces would each have a pair of properties corresponding to the ones for Location. Avatars could also gain this pattern if needed.

My idea was that communications policy could perhaps be controlled by a R_P_A_C as well (the interpretation would be the same, but with "can see this rich presence" replaced by "can communicate (in this way)", and we could add more members to the enum if needed.

Communications blocking via the 'deny' list, or its replacement in the ContactList world, should always take precedence, but this interface is about what to do when people aren't blocked.

I have some proposals for extensions to Rich_Presence_Access_Control, from earlier discussion; I'll follow up with those.
Comment 5 Simon McVittie 2010-07-12 06:03:52 UTC
Some proposed extensions to RPACT from earlier discussion on this topic:

Rich_Presence_Access_Control_Type_Closed = ???
    No contacts can see the extended presence information. The associated
    variant is ignored. Any connection manager supporting Whitelist SHOULD
    support this too, since it is equivalent to an empty whitelist.

    | It's very easy, and it's one of the possible access control modes for
    | things that can be hidden from everyone.

Rich_Presence_Access_Control_Type_Subscribe_List = ???
    All contacts in the user's 'publish' contact list can see the extended
    presence information. The associated variant is ignored.

    | An obvious extension of Publish_List.

Rich_Presence_Access_Control_Type_Subscribe_Or_Publish_List = ???
    All contacts in either the user's 'subscribe' or 'publish' contact list
    can see the extended presence information. The associated variant is
    ignored.

    | I'm told this is one of the possible settings for your avatar in
    | Skype?

Rich_Presence_Access_Control_Type_Not_Understood = ???
    The access control rule is too complex to be represented in the current
    Telepathy API. The associated variant is meaningless. Setting this mode
    is never valid; the connection manager MUST raise an error if this is
    attempted.

    | XEP-0016 Privacy Lists (the terrifying XEP resembling iptables for XMPP)
    | can easily produce access control mechanisms that can't be expressed in
    | a simpler API. We need to be able to at least indicate that fact.

I can think of another potentially useful mode now:

Rich_Presence_Access_Control_Type_Subscribe_Or_Pending = ???
    All contacts in the user's 'subscribe' contact list, or remote-pending
    on that list, can see the extended presence information.
Comment 6 Eitan Isaacson 2010-07-12 15:20:02 UTC
(In reply to comment #5)
> Rich_Presence_Access_Control_Type_Subscribe_Or_Pending = ???
>     All contacts in the user's 'subscribe' contact list, or remote-pending
>     on that list, can see the extended presence information.

If we do that, wouldn't we also need to introduce
Rich_Presence_Access_Control_Type_Publish_Or_Subscribe_Or_Pending?
Comment 7 Eitan Isaacson 2010-07-12 16:12:14 UTC
Since communication policy is not presence related, maybe the prefix of 
Rich_Presence_Access_Control will be misleading?

I am thinking that maybe Connection.Interface.AccessControl needs to be the new home for these types (the enum and struct). And that Connection.Interface.Location/Avatars and future rich presence ifaces reference those types.

AccessControl itself would have properties for communication policy, like
- ChatAccessControl (uv) (Access_Policy) R/W
- ChatAccessControlTypes (au) (Access_Policy_Type_List) RO
- CallAccessControl (uv) (Access_Policy) R/W
- CallAccessControlTypes (au) (Access_Policy_Type_List) RO
- FileTransferAccessConrol (uv) (Access_Policy) R/W
- FileTransferAccessConrolType (au) (Access_Policy_Type_List) RO
- TubeAccessControl (uv) (Access_Policy) R/W
- TubeAccessControlType  (au) (Access_Policy_Type_List) RO

As Will says, some protocols will not let you set these things individually, so maybe setting one of these properties will change the other properties that are grouped with it. There would be a change notification signal, so if you set one property, you will get a signal from other properties that have been affected:

- ChatAccessControlChanged (uv) (Access_Policy)
- CallAccessControlChanged (uv) (Access_Policy)
- FileTransferAccessControlChanged (uv) (Access_Policy)
- TubeAccessControlChanged (uv) (Access_Policy)

A not very well thought out alternative
=======================================

This might be slightly less straightforward, but seems more extendable.

- AccessControlTypes (a{sau})
Property. A mapping of channel interfaces and a list of Access_Policy types they could have.

- AccessControl (a{s(uv)})
Property. A mapping of channel interfaces and Access_Policy values. For example {'org.freedesktop.Telepathy.Channel.Interface.Text' : (Publish_List, None),
'org.freedesktop.Telepathy.Channel.Interface.Call' : (Whitelist, [])}

- SetAccessControl (s, uv)
A method for creating/modifying a policy.

- AccessControlChanged (a{sv})
A signal emitted when the AccessControl property has changed. The mapping provided by the signal would be a subset of the properties that changed.

Another suggestion
==================

Maybe the Access_Policy_Type could be a bitfield-friendly enum? That way the supported access control types could be expressed in a single u as opposed to an au.
Comment 8 Eitan Isaacson 2010-07-18 22:32:19 UTC
I put the second alternative to spec:
http://people.freedesktop.org/~eitani/telepathy-spec-commpolicy/spec/
Comment 9 Will Thompson 2010-07-19 09:54:00 UTC
It does seem a shame to define a pair of new types which are basically extensions of Rich_Presence_Access_Control and Rich_Presence_Access_Control_Type... Otherwise I like the shape of this. Would be nice to have a few more words in the introduction, like:

  This interface supports controlling which contacts is allowed to initiate text chats, incoming calls, etc., as supported by the underlying protocol. The policies supported for different communication methods on this connection are listed in the <tp:member-ref>SupportedPolicies</tp:member-ref> property. The current configuration is held in <tp:member-ref>ActivePolicies</tp:member-ref>; it can be modified using <tp:member-ref>SetPolicy</tp:member-ref>, and changes are signalled by <tp:member-ref>PolicyChanged</tp:member-ref>.

ActivePolicies should say "Changes to this property are signalled by PolicyChanged."
Comment 10 Will Thompson 2010-07-19 10:04:34 UTC
[Commit]ted too early.

I think that, wording aside, it's okay to merge as a first draft. I wonder if the right thing to do might be to define Access_Control[_Type] to be synonyms for Rich_Presence_Access_Control[_Type] or so. The type names *are* really long. :)
Comment 11 Simon McVittie 2010-07-19 10:54:39 UTC
(In reply to comment #9)
> It does seem a shame to define a pair of new types which are basically
> extensions of Rich_Presence_Access_Control and
> Rich_Presence_Access_Control_Type...

I'd like to veto the addition of things that are this similar to RPAC/RPACT, but not identical.

My inclination would be to just use RPAC/RPACT (adding entries as needed), and ignore the slightly incorrect naming.

Alternatively, we could rename RPAC/RPACT, and put compatibility hacks in telepathy-glib and telepathy-qt4. I think telepathy-glib can be made compatible with a few #defines, but telepathy-qt4 would need a compatibility typedef?

Another alternative is to keep RPAC/RPACT in the spec, documented as "the same as AC/ACT, for backwards compatibility only", and ensure that the two are kept exactly in sync.

In any case, it's probably premature to add any extra entries that we don't have a concrete use-case for, apart from Not_Understood and perhaps Closed. Some of the ones I suggested in Comment #5 come from Skype features; Mikhail Zabaluev might be able to tell us which ones are needed there.
Comment 12 Eitan Isaacson 2010-07-19 11:18:23 UTC
(In reply to comment #9)
> It does seem a shame to define a pair of new types which are basically
> extensions of Rich_Presence_Access_Control and
> Rich_Presence_Access_Control_Type... 

I was hoping that these would succeed RPAC and that rich presence ifaces would reference this. Of course that breaks API, so I don't know how to transition.
Comment 13 Eitan Isaacson 2010-07-19 11:23:04 UTC
(In reply to comment #11)
> (In reply to comment #9)
> > It does seem a shame to define a pair of new types which are basically
> > extensions of Rich_Presence_Access_Control and
> > Rich_Presence_Access_Control_Type...
> 
> I'd like to veto the addition of things that are this similar to RPAC/RPACT,
> but not identical.
> 
> My inclination would be to just use RPAC/RPACT (adding entries as needed), and
> ignore the slightly incorrect naming.
> 

Are there any other consumers besides Location? If not, it seems like a good opportunity to transition out of the RPAC naming and not have it persist as a legacy thing forever.

> Alternatively, we could rename RPAC/RPACT, and put compatibility hacks in
> telepathy-glib and telepathy-qt4. I think telepathy-glib can be made compatible
> with a few #defines, but telepathy-qt4 would need a compatibility typedef?
> 
> Another alternative is to keep RPAC/RPACT in the spec, documented as "the same
> as AC/ACT, for backwards compatibility only", and ensure that the two are kept
> exactly in sync.

Both alternatives look like good ideas to me.

> 
> In any case, it's probably premature to add any extra entries that we don't
> have a concrete use-case for, apart from Not_Understood and perhaps Closed.
> Some of the ones I suggested in Comment #5 come from Skype features; Mikhail
> Zabaluev might be able to tell us which ones are needed there.

Which ones specifically?
Comment 14 Eitan Isaacson 2010-07-19 11:24:59 UTC
(In reply to comment #13)
> Both alternatives look like good ideas to me.
> 

I'll reshuffle the enum to match RPAC for easy compatibility.
Comment 15 Eitan Isaacson 2010-07-19 11:40:58 UTC
OK,

1. Added iface intro (thanks Will!) bda5494
2. Reshuffled enum. 50ec50b
3. Documented ActivePoicies cange notification f97c383

It's in my branch (url of this bug), and here:
http://people.freedesktop.org/~eitani/telepathy-spec-commpolicy/spec/
Comment 16 Eitan Isaacson 2010-08-03 09:45:45 UTC
Could I merge this? Keep it drafted?
Comment 17 Simon McVittie 2010-08-03 10:20:53 UTC
I suppose we can merge this if you insist, but since every deletion from the middle of the enum breaks API, it seems sensible to get it into a somewhat stable state first. The shortest path to a stable state, as is often the case, is deleting everything that's new and controversial :-)

+          All contacts in the user's 'subscribe' or 'pending' contact
+          list can communicate with us. The associated variant is ignored.
+        </tp:docstring>
+      </tp:enumvalue>
+      <tp:enumvalue suffix="Subscribe_Or_Pending_Or_Publish_List" value="7">
+        <tp:docstring>
+          All contacts in the user's 'subscribe' or 'pending' or
+          'publish' contact lists can communicate with us. The
+          associated variant is ignored.

There's no such list as 'pending'. There are subscribe, publish and stored lists, each of which have (full) members, local-pending members and remote-pending members; if you're keeping these, please be entirely clear about what they mean.

In any case I'd be inclined to drop any of the "_Or_" versions that we don't know a concrete use for; sorry I brought them up in the first place.

+          The associated variant is a list of contacts (signature 'au',
+          Contact_Handle[]) who can communicate with us.

"can't"? :-)

I don't think we should have Blacklist anyway, though, to reduce confusion with contact blocking, Bug #28423 (or equivalently, the 'deny' list in the pre-ContactList world), unless we really, really need it to be distinct. Do you have a picture of how the two interfaces should interact?

(The difference between blacklist-based CommPolicy and contact blocking is that contact blocking also prevents those contacts from seeing our presence, even if they think they have a valid subscription to us - that's how blocking works on at least Google Talk, and I suspect it works that way in basically every protocol that has it.)

I think we should have a way to deal with the common case that all forms of communication (channel types, here) are equal, and blocking one blocks them all (XMPP privacy lists, as used in practice, do this). Perhaps an immutable-after-CONNECTED flag on the Connection to say that that's how it works, or the empty string as channel type, or something?
Comment 18 Eitan Isaacson 2010-08-03 11:02:56 UTC
(In reply to comment #17)
> I suppose we can merge this if you insist, but since every deletion from the
> middle of the enum breaks API, it seems sensible to get it into a somewhat
> stable state first. The shortest path to a stable state, as is often the case,
> is deleting everything that's new and controversial :-)
> 
> +          All contacts in the user's 'subscribe' or 'pending' contact
> +          list can communicate with us. The associated variant is ignored.
> +        </tp:docstring>
> +      </tp:enumvalue>
> +      <tp:enumvalue suffix="Subscribe_Or_Pending_Or_Publish_List" value="7">
> +        <tp:docstring>
> +          All contacts in the user's 'subscribe' or 'pending' or
> +          'publish' contact lists can communicate with us. The
> +          associated variant is ignored.
> 
> There's no such list as 'pending'. There are subscribe, publish and stored
> lists, each of which have (full) members, local-pending members and
> remote-pending members; if you're keeping these, please be entirely clear about
> what they mean.
> 
> In any case I'd be inclined to drop any of the "_Or_" versions that we don't
> know a concrete use for; sorry I brought them up in the first place.
> 

So this is what I am slating for removal:
* Subscribe_Or_Pending_List: For the reasons you stated above, this is confusing and has no concrete use case yet.
* Subscribe_Or_Pending_Or_Publish_List: ditto
* Subscribe_List: No concrete use-case yet.

Additions I think we should keep:
 * Blacklist: See below.
 * Closed: Seems useful.
 * Not_Understood: Weird XEP-0016 privacy lists are a good use case.

> +          The associated variant is a list of contacts (signature 'au',
> +          Contact_Handle[]) who can communicate with us.
> 
> "can't"? :-)

I'll fix that.

> 
> I don't think we should have Blacklist anyway, though, to reduce confusion with
> contact blocking, Bug #28423 (or equivalently, the 'deny' list in the
> pre-ContactList world), unless we really, really need it to be distinct. Do you
> have a picture of how the two interfaces should interact?
> 
> (The difference between blacklist-based CommPolicy and contact blocking is that
> contact blocking also prevents those contacts from seeing our presence, even if
> they think they have a valid subscription to us - that's how blocking works on
> at least Google Talk, and I suspect it works that way in basically every
> protocol that has it.)
> 

To reduce confusion, I think the scope of this interface (or Comm.I.Blocking) should cover both. I don't think this should be spread out on more than one iface. To include presence blocking, perhaps besides channel interfaces, we should also support having Comm.I.SimplePresence (and other presence ifaces, like Location) as keys in SupportedPolicies and ActivePolicies.

> I think we should have a way to deal with the common case that all forms of
> communication (channel types, here) are equal, and blocking one blocks them all
> (XMPP privacy lists, as used in practice, do this). Perhaps an
> immutable-after-CONNECTED flag on the Connection to say that that's how it
> works, or the empty string as channel type, or something?

Maybe instead of having ifaces as keys (o), we should have iface lists (ao).
That way, if more than one communication method is grouped together (ie. for the protocol the chat policy and call policy are synonymous, or if we include presence, presence blocking and chat blocking are synonymous), we would understand their relationship from the groupings in the SupportedPolicies and ActivePolicies properties. For example:

SupportedPolicies = 
{['org.freedesktop.Telepathy.Channel.Interface.Text', 
  'org.freedesktop.Telepathy.Channel.Interface.Call'] :
 [Publish_List, Whitelist]}

^^ Text and Call are bound to each other and support Publish_List and Whitelist policies.
Comment 19 Simon McVittie 2010-08-04 05:21:36 UTC
That's an interesting idea. Note that because the old 'deny' list exists, TpBaseContactList supports blocking and unblocking contacts, and will have to continue to do so in the medium term. TpBaseContactList is just a "view" - the CM maintains the "model" - so a CM could easily have both TpBaseContactList and some hypothetical TpBlockingMixin as views of the same information.

The access control models in RPAC come from XMPP PEP (asking the server to control access to things it stores on our behalf), whereas blocking and communication policy are XMPP privacy lists (asking the server to control what messages will get to *us*).

SimplePresence is odd because its access control is implicitly the intersection of the publish list (or equivalently, the publish attribute in Bug #21787) with something else (by default, allow-all). In terms of RPAC, it looks like (Publish_But_Not_Blacklist, [alice, bob]), except that that's too unwieldy a format to actually use :-)

MSN might be an interesting real-world use case. As I understand it, you can essentially set your access control to one of:

* my buddies (not sure whether this is Publish, Subscribe or some mixture)
* everyone

but there is also a blacklist, which takes precedence over either.
Comment 20 Simon McVittie 2010-08-04 08:06:48 UTC
Location's Bug #27752 goes into more detail about PEP access control: it's more complicated than we thought.
Comment 21 Eitan Isaacson 2010-08-17 10:57:53 UTC
(In reply to comment #19)
> That's an interesting idea. Note that because the old 'deny' list exists,
> TpBaseContactList supports blocking and unblocking contacts, and will have to
> continue to do so in the medium term. TpBaseContactList is just a "view" - the
> CM maintains the "model" - so a CM could easily have both TpBaseContactList and
> some hypothetical TpBlockingMixin as views of the same information.

Yup.

> 
> The access control models in RPAC come from XMPP PEP (asking the server to
> control access to things it stores on our behalf), whereas blocking and
> communication policy are XMPP privacy lists (asking the server to control what
> messages will get to *us*).

Privacy lists also control outgoing communication (<message-out/>, <presence/>). Whether this is information that is stored on the server or something the client sends directly seems like a protocol-specific detail.

> 
> SimplePresence is odd because its access control is implicitly the intersection
> of the publish list (or equivalently, the publish attribute in Bug #21787) with
> something else (by default, allow-all). In terms of RPAC, it looks like
> (Publish_But_Not_Blacklist, [alice, bob]), except that that's too unwieldy a
> format to actually use :-)

In this proposed API it would look like this:
Conn.I.CommPolicy.SetPolicy("Conn.I.SimplePresence", (Blacklist, [alice, bob])

> 
> MSN might be an interesting real-world use case. As I understand it, you can
> essentially set your access control to one of:
> 
> * my buddies (not sure whether this is Publish, Subscribe or some mixture)
> * everyone

Maybe there needs to be a Everyone access control type as well...

> 
> but there is also a blacklist, which takes precedence over either.

Ugh. It looks like we will be re-inventing privacy lists before this is done..

I am beginning to think that maybe this API should not include presence at all, but only communication. I originally thought we should unify all this under one interface, but it is getting extremely complex. So maybe we need two interfaces that are only complex (as opposed to extremely). I feel like if this were limited to communication, it would be a lot easier to use.
Comment 23 Simon McVittie 2010-09-03 04:03:38 UTC
I like the "shape" of this interface now, but I'd prefer to not merge it as a draft until A_C_T has consecutive values, so that the first merged draft is something we could reasonably undraft.

> Access_Control_Type

The new values (Closed, Not_Understood and Subscribe_Or_Publish_List) are non-contiguous; they should get consecutive values.

Rather than talking about allowing communication or letting contacts see rich presence, I think this should be phrased in terms of "allow [x]", like you did for Whitelist.

If A_C[_T] is meant to replace R_P_A_C[_T], I'd actually prefer it to go straight in to the SimplePresence interface, with a note saying "new interfaces should use this one", and notes in R_P_A_C[_T] saying "Location uses this for historical reasons, new interfaces will use A_C[_T]" and "New values MUST NOT be added to this enum unless they are equal to the corresponding value of Access_Control_Type".

We should still have an Access_Control struct equivalent to R_P_A_C, even if CommunicationPolicy won't actually use it.

I think the addition of Closed (exactly equivalent to an empty whitelist) and Not_Understood is sufficiently non-controversial to put them straight in as stable API, tbh.

If you insist on omitting the associated variant in the CommunicationPolicy interface, the descriptions of access control types should still talk about it:

    Whitelist = 0

    Only allow contacts that are in a certain whitelist.

    Setting this policy only makes sense in the context of an
    Access_Control structure, in which the variant must contain
    a list of _Contact_Handle_ representing the whitelist, with
    signature 'au'.

Do we have a use-case for S_O_P_L (i.e. a form of communication in some protocol that can have this restriction), or should it be shelved for now? I believe the use-case was that it was a possible access control mode for Skype avatars, in which case we could add it if/when we add access control to the Avatars interface :-)

The API you've chosen for SetPolicy etc. excludes the possibility of ever using an access control type that needs a variant (e.g. Whitelist or Group) in CommunicationPolicy.

I'm not sure that that's a good idea - even if we don't need the variant for any of the access control types we support *now*, it doesn't cost us much to have a variant with a dummy value (i.e. replace the Access_Control_Type in SetPolicy, PolicyChanged and ActivePolicies with an Access_Control (uv), but keep Access_Control_Type in SupportedPolicies).

> +  <tp:copyright>Copyright (C) 2010 Collabora Limited</tp:copyright>

Nitpicking: can we have a © instead of ASCII art? :-)
Comment 24 Eitan Isaacson 2010-09-07 16:16:03 UTC
(In reply to comment #23)
> I like the "shape" of this interface now, but I'd prefer to not merge it as a
> draft until A_C_T has consecutive values, so that the first merged draft is
> something we could reasonably undraft.
> 
> > Access_Control_Type
> 
> The new values (Closed, Not_Understood and Subscribe_Or_Publish_List) are
> non-contiguous; they should get consecutive values.

64a1a14

> 
> Rather than talking about allowing communication or letting contacts see rich
> presence, I think this should be phrased in terms of "allow [x]", like you did
> for Whitelist.

a018e2e

> 
> If A_C[_T] is meant to replace R_P_A_C[_T], I'd actually prefer it to go
> straight in to the SimplePresence interface, with a note saying "new interfaces
> should use this one", and notes in R_P_A_C[_T] saying "Location uses this for
> historical reasons, new interfaces will use A_C[_T]" and "New values MUST NOT
> be added to this enum unless they are equal to the corresponding value of
> Access_Control_Type".
> 

64a1a14

> We should still have an Access_Control struct equivalent to R_P_A_C, even if
> CommunicationPolicy won't actually use it.
> 

Added it back, as you suggested below. I wish I remember why I removed it, I think for simplicity. Future proofing and extendability is more important here.

0420594

> I think the addition of Closed (exactly equivalent to an empty whitelist) and
> Not_Understood is sufficiently non-controversial to put them straight in as
> stable API, tbh.

Great, they are in there.

> 
> If you insist on omitting the associated variant in the CommunicationPolicy
> interface, the descriptions of access control types should still talk about it:
> 
>     Whitelist = 0
> 
>     Only allow contacts that are in a certain whitelist.
> 
>     Setting this policy only makes sense in the context of an
>     Access_Control structure, in which the variant must contain
>     a list of _Contact_Handle_ representing the whitelist, with
>     signature 'au'.
> 

Policy will always be set with an associated variant, because of the API change. So I changed the language.
53ef1ba

> Do we have a use-case for S_O_P_L (i.e. a form of communication in some
> protocol that can have this restriction), or should it be shelved for now? I
> believe the use-case was that it was a possible access control mode for Skype
> avatars, in which case we could add it if/when we add access control to the
> Avatars interface :-)

Yeah, we could add this in the future.

I think Conn.I.Avatars needs to be documented as a rich presence iface, I don't think it is clear enough.

> 
> The API you've chosen for SetPolicy etc. excludes the possibility of ever using
> an access control type that needs a variant (e.g. Whitelist or Group) in
> CommunicationPolicy.
> 
> I'm not sure that that's a good idea - even if we don't need the variant for
> any of the access control types we support *now*, it doesn't cost us much to
> have a variant with a dummy value (i.e. replace the Access_Control_Type in
> SetPolicy, PolicyChanged and ActivePolicies with an Access_Control (uv), but
> keep Access_Control_Type in SupportedPolicies).
> 

Changed. 
0420594

> > +  <tp:copyright>Copyright (C) 2010 Collabora Limited</tp:copyright>
> 
> Nitpicking: can we have a © instead of ASCII art? :-)
Comment 25 Eitan Isaacson 2010-09-28 11:52:28 UTC
Let's not let this rot, guys! Reviewers? Preferably Simon, since he gave me most feedback till now.
Comment 26 Simon McVittie 2010-09-29 03:48:45 UTC
This looks good as a draft (with Access_Control as insta-stable API).
Comment 27 Simon McVittie 2010-09-29 10:41:38 UTC
Draft 1 in 0.21.1.
Comment 28 Danielle Madeley 2010-12-01 20:25:04 UTC
http://git.collabora.co.uk/?p=user/danni/telepathy-spec.git;a=shortlog;h=refs/heads/commpol-trivia

This patch fixes two typos in the argument names of this spec.
Comment 29 Will Thompson 2010-12-02 03:26:32 UTC
(In reply to comment #28)
> http://git.collabora.co.uk/?p=user/danni/telepathy-spec.git;a=shortlog;h=refs/heads/commpol-trivia
> 
> This patch fixes two typos in the argument names of this spec.

ship it.
Comment 30 Danielle Madeley 2010-12-02 03:29:43 UTC
I ship it!
Comment 31 Danielle Madeley 2011-03-14 14:44:47 UTC
This interface fails to meet some features we're interested in.

 - Block PSTN calls vs Client-Client calls

Some protocols let us differentiate between how we want to block Client-to-Client calls (i.e. between to instances of BadgerCommunicator) vs calls that are coming into the BadgerCommunicator network from the PSTN.

 - Need PSTN specific blocking

e.g. Known_Numbers_Only (blocks those without caller-id)

 - Block non-channel Protocol features

e.g. Avatar, contact-info, showing number of buddies, external presence service

 - Rejecting video from some users

We need a mechanism to say whether what your default policy is for accepting the video stream that forms part of a Call, or just the audio. Users who do not automatically accept video from other users should be asked if they want to also accept the video stream. [It should be easy to then whitelist users/contacts/etc]. This is the anti-flasher setting.
Comment 32 Simon McVittie 2011-05-05 04:12:11 UTC
(In reply to comment #31)
> e.g. Avatar, contact-info, showing number of buddies, external presence service

I think you're conflating two different things:

* communication policy: who is allowed to initiate a conversation with me?
  (that's this interface)

* privacy/secrecy of published information ("rich presence"): when I
  publish my avatar, contact info, etc., who's allowed to see it?  

I personally think the latter is out-of-scope for this interface: access control for Avatars should be on the Avatars interface, and access control for ContactInfo should be on the ContactInfo interface. (Rather like the way Location works, except with an actual implementation...) Perhaps it should be a settable property that is also a CM parameter? Although, having it be asynchronous to get and set might make more sense; you'd need to look at how it behaves in real protocols.

By "external presence service", do you mean like the way you can choose whether your ICQ presence is visible on the ICQ website or not? That could be a settable boolean on SimplePresence (if it's common enough to justify it) or a subsidiary interface, maybe? Or on this interface if people don't mind blurring its purpose.

>  - Block PSTN calls vs Client-Client calls
>  - Need PSTN specific blocking
>  - Block non-channel Protocol features

Those are all in-scope for this interface, I think: they're communication policy analogous to what's already here.

>  - Rejecting video from some users

Is this a protocol-level feature at all? CMs should only have API for this if there's a network-protocol operation for it (manipulating a server-stored list, maybe).

I would have thought the behaviour at the CM level should be to not auto-accept video at all, and the UI can auto-accept if that's what the user wants?
Comment 33 GitLab Migration User 2019-12-03 20:20:14 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/47.

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.