From a4a06206e366bf0684b1ad27d8648d7fc498dc73 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 28 May 2007 17:25:52 +0100 Subject: [PATCH 01/12] services/presence/: identify Buddies by "key ID" (pubkey hash), not whole key. This allows us to create Buddy objects as soon as we see a contact on the server. For contacts not on trusted servers, or seen in anonymous MUCs, we create a Buddy identified by JID instead (so we have some way to talk about the anonymous contact within the Sugar API). The concept of "trusted server" means a server which we trust to validate that users with a keyID as the username part of their JID do in fact have that key. Currently we just pretend that olpc.collabora.co.uk does this - in future, the school servers will do this validation by using key rather than password authentication. Also create Buddy object paths based on the keyID or JID (for easier debugging). --- services/presence/buddy.py | 47 ++++++------ services/presence/presenceservice.py | 48 +++++++----- services/presence/pstest.py | 10 ++- services/presence/server_plugin.py | 108 +++++++++++++++++++++++++-- 4 files changed, 165 insertions(+), 48 deletions(-) diff --git a/services/presence/buddy.py b/services/presence/buddy.py index 1b45fd5e..b858b41a 100644 --- a/services/presence/buddy.py +++ b/services/presence/buddy.py @@ -37,6 +37,7 @@ _PROP_CURACT = "current-activity" _PROP_COLOR = "color" _PROP_OWNER = "owner" _PROP_VALID = "valid" +_PROP_OBJID = 'objid' # Will go away soon _PROP_IP4_ADDRESS = "ip4-address" @@ -90,15 +91,14 @@ class Buddy(ExportedGObject): } __gproperties__ = { - _PROP_KEY : (str, None, None, None, - gobject.PARAM_READWRITE | - gobject.PARAM_CONSTRUCT_ONLY), + _PROP_KEY : (str, None, None, None, gobject.PARAM_READWRITE), _PROP_ICON : (object, None, None, gobject.PARAM_READWRITE), _PROP_NICK : (str, None, None, None, gobject.PARAM_READWRITE), _PROP_COLOR : (str, None, None, None, gobject.PARAM_READWRITE), _PROP_CURACT : (str, None, None, None, gobject.PARAM_READWRITE), _PROP_VALID : (bool, None, None, False, gobject.PARAM_READABLE), _PROP_OWNER : (bool, None, None, False, gobject.PARAM_READABLE), + _PROP_OBJID : (str, None, None, None, gobject.PARAM_READABLE), _PROP_IP4_ADDRESS : (str, None, None, None, gobject.PARAM_READWRITE) } @@ -106,16 +106,16 @@ class Buddy(ExportedGObject): """Initialize the Buddy object bus -- connection to the D-Bus session bus - object_id -- the activity's unique identifier + object_id -- the buddy's unique identifier, either based on their + key-ID or JID kwargs -- used to initialize the object's properties constructs a DBUS "object path" from the _BUDDY_PATH and object_id """ - if not object_id or not isinstance(object_id, int): - raise ValueError("object id must be a valid number") - self._object_path = _BUDDY_PATH + str(object_id) + self._object_id = object_id + self._object_path = dbus.ObjectPath(_BUDDY_PATH + object_id) self._activities = {} # Activity ID -> Activity self._activity_sigids = {} @@ -130,9 +130,6 @@ class Buddy(ExportedGObject): self._color = None self._ip4_address = None - if not kwargs.get(_PROP_KEY): - raise ValueError("key required") - _ALLOWED_INIT_PROPS = [_PROP_NICK, _PROP_KEY, _PROP_ICON, _PROP_CURACT, _PROP_COLOR, _PROP_IP4_ADDRESS] for (key, value) in kwargs.items(): @@ -158,7 +155,9 @@ class Buddy(ExportedGObject): pspec -- property specifier with a "name" attribute """ - if pspec.name == _PROP_KEY: + if pspec.name == _PROP_OBJID: + return self._object_id + elif pspec.name == _PROP_KEY: return self._key elif pspec.name == _PROP_ICON: return self._icon @@ -422,32 +421,40 @@ class Buddy(ExportedGObject): """ changed = False changed_props = {} - if _PROP_NICK in properties.keys(): + if _PROP_NICK in properties: nick = properties[_PROP_NICK] if nick != self._nick: self._nick = nick changed_props[_PROP_NICK] = nick changed = True - if _PROP_COLOR in properties.keys(): + if _PROP_COLOR in properties: color = properties[_PROP_COLOR] if color != self._color: self._color = color changed_props[_PROP_COLOR] = color changed = True - if _PROP_CURACT in properties.keys(): + if _PROP_CURACT in properties: curact = properties[_PROP_CURACT] if curact != self._current_activity: self._current_activity = curact changed_props[_PROP_CURACT] = curact changed = True - if _PROP_IP4_ADDRESS in properties.keys(): + if _PROP_IP4_ADDRESS in properties: ip4addr = properties[_PROP_IP4_ADDRESS] if ip4addr != self._ip4_address: self._ip4_address = ip4addr changed_props[_PROP_IP4_ADDRESS] = ip4addr changed = True + if _PROP_KEY in properties: + # don't allow key to be set more than once + if self._key is None: + key = properties[_PROP_KEY] + if key is not None: + self._key = key + changed_props[_PROP_KEY] = key + changed = True - if not changed or not len(changed_props.keys()): + if not changed or not changed_props: return # Try emitting PropertyChanged before updating validity @@ -558,13 +565,11 @@ class ShellOwner(GenericOwner): _SHELL_OWNER_INTERFACE = "org.laptop.Shell.Owner" _SHELL_PATH = "/org/laptop/Shell" - def __init__(self, ps, bus, object_id, test=False): + def __init__(self, ps, bus): """Initialize the ShellOwner instance ps -- presenceservice.PresenceService object bus -- a connection to the D-Bus session bus - object_id -- the activity's unique identifier - test -- ignored Retrieves initial property values from the profile module. Loads the buddy icon from file as well. @@ -584,8 +589,8 @@ class ShellOwner(GenericOwner): icon = f.read() f.close() - GenericOwner.__init__(self, ps, bus, object_id, key=key, - nick=nick, color=color, icon=icon, server=server, + GenericOwner.__init__(self, ps, bus, psutils.pubkey_to_keyid(key), + key=key, nick=nick, color=color, icon=icon, server=server, key_hash=key_hash, registered=registered) # Ask to get notifications on Owner object property changes in the diff --git a/services/presence/presenceservice.py b/services/presence/presenceservice.py index bf058d3c..4f84a3b2 100644 --- a/services/presence/presenceservice.py +++ b/services/presence/presenceservice.py @@ -1,4 +1,5 @@ # Copyright (C) 2007, Red Hat, Inc. +# Copyright (C) 2007 Collabora Ltd. # # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -33,6 +34,7 @@ from sugar import util from buddy import Buddy, ShellOwner from activity import Activity +from psutils import pubkey_to_keyid _PRESENCE_SERVICE = "org.laptop.Sugar.Presence" _PRESENCE_INTERFACE = "org.laptop.Sugar.Presence" @@ -57,15 +59,17 @@ class PresenceService(ExportedGObject): def _create_owner(self): # Overridden by TestPresenceService - return ShellOwner(self, self._session_bus, self._get_next_object_id()) + return ShellOwner(self, self._session_bus) def __init__(self): self._next_object_id = 0 self._connected = False - self._buddies = {} # key -> Buddy + self._buddies = {} # identifier -> Buddy + self._buddies_by_pubkey = {} # base64 public key -> Buddy self._handles_buddies = {} # tp client -> (handle -> Buddy) - self._activities = {} # activity id -> Activity + + self._activities = {} # activity id -> Activity self._session_bus = dbus.SessionBus() self._session_bus.add_signal_receiver(self._connection_disconnected_cb, @@ -74,7 +78,10 @@ class PresenceService(ExportedGObject): # Create the Owner object self._owner = self._create_owner() - self._buddies[self._owner.props.key] = self._owner + key = self._owner.props.key + keyid = pubkey_to_keyid(key) + self._buddies['keyid/' + keyid] = self._owner + self._buddies_by_pubkey[key] = self._owner self._registry = ManagerRegistry() self._registry.LoadManagers() @@ -133,31 +140,35 @@ class PresenceService(ExportedGObject): if self._connected != old_status: self.emit('connection-status', self._connected) - def _contact_online(self, tp, handle, props): - new_buddy = False - key = props["key"] - buddy = self._buddies.get(key) - if not buddy: + def get_buddy(self, objid): + buddy = self._buddies.get(objid) + if buddy is None: + _logger.debug('Creating new buddy at .../%s', objid) # we don't know yet this buddy - objid = self._get_next_object_id() - buddy = Buddy(self._session_bus, objid, key=key) + buddy = Buddy(self._session_bus, objid) buddy.connect("validity-changed", self._buddy_validity_changed_cb) buddy.connect("disappeared", self._buddy_disappeared_cb) - self._buddies[key] = buddy + self._buddies[objid] = buddy + return buddy + + def _contact_online(self, tp, objid, handle, props): + _logger.debug('Handle %u, .../%s is now online', handle, objid) + buddy = self.get_buddy(objid) self._handles_buddies[tp][handle] = buddy # store the handle of the buddy for this CM buddy.add_telepathy_handle(tp, handle) - buddy.set_properties(props) def _buddy_validity_changed_cb(self, buddy, valid): if valid: self.BuddyAppeared(buddy.object_path()) + self._buddies_by_pubkey[buddy.props.key] = buddy _logger.debug("New Buddy: %s (%s)", buddy.props.nick, buddy.props.color) else: self.BuddyDisappeared(buddy.object_path()) + self._buddies_by_pubkey.pop(buddy.props.key, None) _logger.debug("Buddy left: %s (%s)", buddy.props.nick, buddy.props.color) @@ -166,16 +177,17 @@ class PresenceService(ExportedGObject): self.BuddyDisappeared(buddy.object_path()) _logger.debug('Buddy left: %s (%s)', buddy.props.nick, buddy.props.color) - self._buddies.pop(buddy.props.key) + self._buddies_by_pubkey.pop(buddy.props.key, None) + self._buddies.pop(buddy.props.objid, None) def _contact_offline(self, tp, handle): if not self._handles_buddies[tp].has_key(handle): return buddy = self._handles_buddies[tp].pop(handle) - key = buddy.props.key - # the handle of the buddy for this CM is not valid anymore + # (this might trigger _buddy_disappeared_cb if they are not visible + # via any CM) buddy.remove_telepathy_handle(tp, handle) def _get_next_object_id(self): @@ -326,8 +338,8 @@ class PresenceService(ExportedGObject): in_signature="ay", out_signature="o", byte_arrays=True) def GetBuddyByPublicKey(self, key): - if self._buddies.has_key(key): - buddy = self._buddies[key] + buddy = self._buddies_by_pubkey.get(key) + if buddy is not None: if buddy.props.valid: return buddy.object_path() raise NotFoundError("The buddy was not found.") diff --git a/services/presence/pstest.py b/services/presence/pstest.py index 19009932..3054e488 100644 --- a/services/presence/pstest.py +++ b/services/presence/pstest.py @@ -26,6 +26,7 @@ from sugar import env, util from buddy import GenericOwner, _PROP_NICK, _PROP_CURACT, _PROP_COLOR from presenceservice import PresenceService +from psutils import pubkey_to_keyid _logger = logging.getLogger('s-p-s.pstest') @@ -37,7 +38,7 @@ class TestOwner(GenericOwner): __gtype_name__ = "TestOwner" - def __init__(self, ps, bus, object_id, test_num, randomize): + def __init__(self, ps, bus, test_num, randomize): self._cp = ConfigParser() self._section = "Info" self._test_activities = [] @@ -62,8 +63,9 @@ class TestOwner(GenericOwner): icon = _get_random_image() _logger.debug("pubkey is %s" % pubkey) - GenericOwner.__init__(self, ps, bus, object_id, key=pubkey, nick=nick, - color=color, icon=icon, registered=registered, key_hash=privkey_hash) + GenericOwner.__init__(self, ps, bus, pubkey_to_keyid(pubkey), + key=pubkey, nick=nick, color=color, icon=icon, + registered=registered, key_hash=privkey_hash) # Only do the random stuff if randomize is true if randomize: @@ -169,7 +171,7 @@ class TestPresenceService(PresenceService): PresenceService.__init__(self) def _create_owner(self): - return TestOwner(self, self._session_bus, self._get_next_object_id(), + return TestOwner(self, self._session_bus, self.__test_num, self.__randomize) def internal_get_activity(self, actid): diff --git a/services/presence/server_plugin.py b/services/presence/server_plugin.py index 26adba94..b0202861 100644 --- a/services/presence/server_plugin.py +++ b/services/presence/server_plugin.py @@ -20,6 +20,7 @@ import logging import os import sys +from string import hexdigits try: # Python >= 2.5 from hashlib import md5 @@ -42,6 +43,7 @@ from telepathy.constants import (HANDLE_TYPE_CONTACT, CONNECTION_STATUS_CONNECTING, CONNECTION_STATUS_REASON_AUTHENTICATION_FAILED, CONNECTION_STATUS_REASON_NONE_SPECIFIED, + CHANNEL_GROUP_FLAG_CHANNEL_SPECIFIC_HANDLES, PROPERTY_FLAG_WRITE) from sugar import util @@ -105,8 +107,11 @@ class ServerPlugin(gobject.GObject): 'contact-online': # Contact has come online and we've discovered all their buddy # properties. - # args: contact handle: int; dict {name: str => property: object} - (gobject.SIGNAL_RUN_FIRST, None, [object, object]), + # args: + # contact identification (based on key ID or JID): str + # contact handle: int or long + # dict {name: str => property: object} + (gobject.SIGNAL_RUN_FIRST, None, [str, object, object]), 'contact-offline': # Contact has gone offline. # args: contact handle @@ -263,7 +268,7 @@ class ServerPlugin(gobject.GObject): account_info['server'] = self._owner.get_server() - khash = util.printable_hash(util._sha_data(self._owner.props.key)) + khash = psutils.pubkey_to_keyid(self._owner.props.key) account_info['account'] = "%s@%s" % (khash, account_info['server']) account_info['password'] = self._owner.get_key_hash() @@ -770,10 +775,13 @@ class ServerPlugin(gobject.GObject): return props['nick'] = aliases[0] + jid = self._conn[CONN_INTERFACE].InspectHandles(HANDLE_TYPE_CONTACT, [handle])[0] self._online_contacts[handle] = jid - self.emit("contact-online", handle, props) + objid = self.identify_contacts(None, [handle])[handle] + + self.emit("contact-online", objid, handle, props) self._conn[CONN_INTERFACE_BUDDY_INFO].GetActivities(handle, reply_handler=lambda *args: self._contact_online_activities_cb( @@ -841,7 +849,7 @@ class ServerPlugin(gobject.GObject): handle not in self._subscribe_local_pending and handle not in self._subscribe_remote_pending): # it's probably a channel-specific handle - can't create a Buddy - # object + # object for those yet return self._online_contacts[handle] = None @@ -1063,3 +1071,93 @@ class ServerPlugin(gobject.GObject): if room == act_handle: self.emit("activity-properties-changed", act_id, properties) return + + def _server_is_trusted(self, hostname): + """Return True if the server with the given hostname is trusted to + verify public-key ownership correctly, and only allows users to + register JIDs whose username part is either a public key fingerprint, + or of the wrong form to be a public key fingerprint (to allow for + ejabberd's admin@example.com address). + + If we trust the server, we can skip verifying the key ourselves, + which leads to simplifications. In the current implementation we + never verify that people actually own the key they claim to, so + we will always give contacts on untrusted servers a JID- rather than + key-based identity. + + For the moment we assume that the test server, olpc.collabora.co.uk, + does this verification. + """ + return (hostname == 'olpc.collabora.co.uk') + + def identify_contacts(self, tp_chan, handles): + """Work out the "best" unique identifier we can for the given handles, + in the context of the given channel (which may be None), using only + 'fast' connection manager API (that does not involve network + round-trips). + + For the XMPP server case, we proceed as follows: + + * Find the owners of the given handles, if the channel has + channel-specific handles + * If the owner (globally-valid JID) is on a trusted server, return + 'keyid/' plus the 'key fingerprint' (the user part of their JID, + currently implemented as the SHA-1 of the Base64 blob in + owner.key.pub) + * If the owner (globally-valid JID) cannot be found or is on an + untrusted server, return 'xmpp/' plus an escaped form of the JID + + The idea is that we identify buddies by key-ID (i.e. by key, assuming + no collisions) if we can find it without making network round-trips, + but if that's not possible we just use their JIDs. + + :Parameters: + `tp_chan` : telepathy.client.Channel or None + The channel in which the handles were found, or None if they + are known to be channel-specific handles + `handles` : iterable over (int or long) + The contacts' handles in that channel + :Returns: + A dict mapping the provided handles to the best available + unique identifier, which is a string that could be used as a + suffix to an object path + """ + # we need to be able to index into handles, so force them to + # be a sequence + if not isinstance(handles, (tuple, list)): + handles = tuple(handles) + + owners = handles + + if tp_chan is not None and CHANNEL_INTERFACE_GROUP in tp_chan: + + group = tp_chan[CHANNEL_INTERFACE_GROUP] + if group.GetFlags() & CHANNEL_GROUP_FLAG_CHANNEL_SPECIFIC_HANDLES: + + owners = group.GetHandleOwners(handles) + for i, owner in enumerate(owners): + if owner == 0: + owners[i] = handles[i] + + jids = self._conn[CONN_INTERFACE].InspectHandles(HANDLE_TYPE_CONTACT, + owners) + + ret = {} + for handle, jid in zip(handles, jids): + if '/' in jid: + # the contact is unidentifiable (in an anonymous MUC) - create + # a temporary identity for them, based on their room-JID + ret[handle] = 'xmpp/' + psutils.escape_identifier(jid) + else: + user, host = jid.split('@', 1) + if (self._server_is_trusted(host) and len(user) == 40 and + user.strip(hexdigits) == ''): + # they're on a trusted server and their username looks + # like a key-ID + ret[handle] = 'keyid/' + user.lower() + else: + # untrusted server, or not the right format to be a + # key-ID - identify the contact by their JID + ret[handle] = 'xmpp/' + psutils.escape_identifier(jid) + + return ret From ea892796ae37da2f841f1490e9abcf2d7f648976 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Tue, 29 May 2007 14:05:12 +0100 Subject: [PATCH 02/12] services/presence/Makefile.am: Include test_psutils.py in "make check" --- services/presence/Makefile.am | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/services/presence/Makefile.am b/services/presence/Makefile.am index 569fec71..44b2be76 100644 --- a/services/presence/Makefile.am +++ b/services/presence/Makefile.am @@ -17,8 +17,15 @@ sugar_PYTHON = \ psutils.py \ server_plugin.py -bin_SCRIPTS = sugar-presence-service +dist_bin_SCRIPTS = sugar-presence-service DISTCLEANFILES = $(service_DATA) -EXTRA_DIST = $(service_in_files) $(bin_SCRIPTS) +EXTRA_DIST = $(service_in_files) + +dist_check_SCRIPTS = test_psutils.py + +TESTS_ENVIRONMENT = \ + PYTHONPATH=$(top_srcdir):$(top_srcdir)/services/presence \ + $(PYTHON) +TESTS = $(dist_check_SCRIPTS) From 2f8ef7bd3bdf8c92d9f1e2b742c8314dc9adb115 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Tue, 29 May 2007 16:10:19 +0100 Subject: [PATCH 03/12] services/presence/test_psutils: trivial check for pubkey_to_keyid() --- services/presence/test_psutils.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/services/presence/test_psutils.py b/services/presence/test_psutils.py index deaf1039..7436d98a 100644 --- a/services/presence/test_psutils.py +++ b/services/presence/test_psutils.py @@ -1,4 +1,8 @@ -from psutils import escape_identifier +print "Running test_psutils..." + +from psutils import escape_identifier, pubkey_to_keyid + +assert pubkey_to_keyid('abc') == 'a9993e364706816aba3e25717850c26c9cd0d89d' assert escape_identifier('') == '_' assert escape_identifier('_') == '_5f' From 9d812430bf63b44cd0c32488c72874fe54d8055f Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Tue, 29 May 2007 16:11:07 +0100 Subject: [PATCH 04/12] services/presence/psutils.py: don't bother using sugar.util, it's easier to use hexdigest or sha directly --- services/presence/psutils.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/services/presence/psutils.py b/services/presence/psutils.py index 04e7eecd..25b24b98 100644 --- a/services/presence/psutils.py +++ b/services/presence/psutils.py @@ -17,12 +17,15 @@ import logging from string import ascii_letters, digits +try: + from hashlib import sha1 +except ImportError: + # Python < 2.5 + from sha import new as sha1 import dbus import gobject -from sugar import util - _logger = logging.getLogger('s-p-s.psutils') @@ -39,7 +42,7 @@ def pubkey_to_keyid(key): :Returns: The key ID as a string of hex digits """ - return util.printable_hash(util._sha_data(key)) + return sha1(key).hexdigest() def escape_identifier(identifier): From 6957446167d756f9edd86d54e90dbfbb8a6f7809 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Tue, 29 May 2007 16:12:05 +0100 Subject: [PATCH 05/12] services/presence/: Make Activities responsible for tracking their members. Add a signal to notify the PS when all members have gone away. --- services/presence/activity.py | 18 ++++++++++++++++-- services/presence/presenceservice.py | 10 +++------- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/services/presence/activity.py b/services/presence/activity.py index be60f1e2..0743b2b8 100644 --- a/services/presence/activity.py +++ b/services/presence/activity.py @@ -48,8 +48,17 @@ class Activity(ExportedGObject): __gtype_name__ = "Activity" __gsignals__ = { - 'validity-changed': (gobject.SIGNAL_RUN_FIRST, gobject.TYPE_NONE, - ([gobject.TYPE_BOOLEAN])) + 'validity-changed': + # The activity's validity has changed. + # An activity is valid if its name, color, type and ID have been + # set. + # Arguments: + # validity: bool + (gobject.SIGNAL_RUN_FIRST, None, [bool]), + 'disappeared': + # Nobody is in this activity any more. + # No arguments. + (gobject.SIGNAL_RUN_FIRST, None, []), } __gproperties__ = { @@ -381,6 +390,7 @@ class Activity(ExportedGObject): """ if buddy not in self._buddies: self._buddies.append(buddy) + buddy.add_activity(self) if self.props.valid: self.BuddyJoined(buddy.object_path()) @@ -396,9 +406,13 @@ class Activity(ExportedGObject): """ if buddy in self._buddies: self._buddies.remove(buddy) + buddy.remove_activity(self) if self.props.valid: self.BuddyLeft(buddy.object_path()) + if not self._buddies: + self.emit('disappeared') + def _handle_share_join(self, tp, text_channel): """Called when a join to a network activity was successful. diff --git a/services/presence/presenceservice.py b/services/presence/presenceservice.py index 4f84a3b2..84814f08 100644 --- a/services/presence/presenceservice.py +++ b/services/presence/presenceservice.py @@ -219,11 +219,12 @@ class PresenceService(ExportedGObject): activity.connect("validity-changed", self._activity_validity_changed_cb) + activity.connect("disappeared", self._activity_disappeared_cb) self._activities[activity_id] = activity return activity - def _remove_activity(self, activity): - _logger.debug("remove activity %s" % activity.props.id) + def _activity_disappeared_cb(self, activity): + _logger.debug("activity %s disappeared" % activity.props.id) self.ActivityDisappeared(activity.object_path()) del self._activities[activity.props.id] @@ -259,7 +260,6 @@ class PresenceService(ExportedGObject): if activity is not None: activity.buddy_joined(buddy) - buddy.add_activity(activity) activities_left = old_activities - new_activities for act in activities_left: @@ -269,10 +269,6 @@ class PresenceService(ExportedGObject): continue activity.buddy_left(buddy) - buddy.remove_activity(activity) - - if not activity.get_joined_buddies(): - self._remove_activity(activity) def _activity_invitation(self, tp, act_id): activity = self._activities.get(act_id) From 87446bfb7fcabe52089c4118f6fe9e62a9f8dfb4 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Wed, 30 May 2007 17:04:16 +0100 Subject: [PATCH 06/12] services/presence/buddyiconcache: remove unused import of sugar.util --- services/presence/buddyiconcache.py | 1 - 1 file changed, 1 deletion(-) diff --git a/services/presence/buddyiconcache.py b/services/presence/buddyiconcache.py index c647fa81..9d355bb5 100644 --- a/services/presence/buddyiconcache.py +++ b/services/presence/buddyiconcache.py @@ -16,7 +16,6 @@ # Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA from sugar import env -from sugar import util import os.path import cPickle From ee6c1b428389268892889bc634650429c61b38b8 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Wed, 30 May 2007 17:36:42 +0100 Subject: [PATCH 07/12] services/presence/: Have joined Activities track membership via group interface. This allows us to ignore the (trivially spoofable) PEP info for activities that we're actually in, in favour of looking at the actual members. --- services/presence/activity.py | 145 +++++++++++++++++++++++---- services/presence/presenceservice.py | 55 ++++++++-- 2 files changed, 176 insertions(+), 24 deletions(-) diff --git a/services/presence/activity.py b/services/presence/activity.py index 0743b2b8..2eb21f6c 100644 --- a/services/presence/activity.py +++ b/services/presence/activity.py @@ -22,7 +22,8 @@ from dbus.gobject_service import ExportedGObject from sugar import util import logging -from telepathy.interfaces import (CHANNEL_INTERFACE) +from telepathy.constants import CHANNEL_GROUP_FLAG_CHANNEL_SPECIFIC_HANDLES +from telepathy.interfaces import (CHANNEL_INTERFACE, CHANNEL_INTERFACE_GROUP) _ACTIVITY_PATH = "/org/laptop/Sugar/Presence/Activities/" _ACTIVITY_INTERFACE = "org.laptop.Sugar.Presence.Activity" @@ -80,7 +81,7 @@ class Activity(ExportedGObject): _RESERVED_PROPNAMES = __gproperties__.keys() - def __init__(self, bus, object_id, tp, **kwargs): + def __init__(self, bus, object_id, ps, tp, **kwargs): """Initializes the activity and sets its properties to default values. :Parameters: @@ -88,6 +89,8 @@ class Activity(ExportedGObject): A connection to the D-Bus session bus `object_id` : int PS ID for this activity, used to construct the object-path + `ps` : presenceservice.PresenceService + The presence service `tp` : server plugin The server plugin object (stands for "telepathy plugin") :Keywords: @@ -112,16 +115,20 @@ class Activity(ExportedGObject): if not tp: raise ValueError("telepathy CM must be valid") + self._ps = ps self._object_id = object_id self._object_path = dbus.ObjectPath(_ACTIVITY_PATH + str(self._object_id)) - self._buddies = [] + self._buddies = set() + self._member_handles = set() self._joined = False # the telepathy client self._tp = tp + self._self_handle = None self._text_channel = None + self._text_channel_group_flags = 0 self._valid = False self._id = None @@ -376,8 +383,10 @@ class Activity(ExportedGObject): ret.append(buddy) return ret - def buddy_joined(self, buddy): - """Adds a buddy to this activity and sends a BuddyJoined signal + def buddy_apparently_joined(self, buddy): + """Adds a buddy to this activity and sends a BuddyJoined signal, + unless we can already see who's in the activity by being in it + ourselves. buddy -- Buddy object representing the buddy being added @@ -388,30 +397,54 @@ class Activity(ExportedGObject): This method is called by the PresenceService on the local machine. """ - if buddy not in self._buddies: - self._buddies.append(buddy) + if not self._joined: + self._add_buddies((buddy,)) + + def _add_buddies(self, buddies): + buddies = set(buddies) + + # disregard any who are already there + buddies -= self._buddies + + self._buddies |= buddies + + for buddy in buddies: buddy.add_activity(self) if self.props.valid: self.BuddyJoined(buddy.object_path()) - def buddy_left(self, buddy): - """Removes a buddy from this activity and sends a BuddyLeft signal. + def _remove_buddies(self, buddies): + buddies = set(buddies) + + # disregard any who are not already there + buddies &= self._buddies + + self._buddies -= buddies + + for buddy in buddies: + buddy.remove_activity(self) + if self.props.valid: + self.BuddyJoined(buddy.object_path()) + + if not self._buddies: + self.emit('disappeared') + + def buddy_apparently_left(self, buddy): + """Removes a buddy from this activity and sends a BuddyLeft signal, + unless we can already see who's in the activity by being in it + ourselves. buddy -- Buddy object representing the buddy being removed Removes a buddy from this activity if the buddy is in the buddy list. If this activity is "valid", a BuddyLeft signal is also sent. This method is called by the PresenceService on the local machine. - """ - if buddy in self._buddies: - self._buddies.remove(buddy) - buddy.remove_activity(self) - if self.props.valid: - self.BuddyLeft(buddy.object_path()) + if not self._joined: + self._remove_buddies((buddy,)) - if not self._buddies: - self.emit('disappeared') + def _text_channel_group_flags_changed_cb(self, flags): + self._text_channel_group_flags = flags def _handle_share_join(self, tp, text_channel): """Called when a join to a network activity was successful. @@ -426,7 +459,36 @@ class Activity(ExportedGObject): self._text_channel = text_channel self._text_channel[CHANNEL_INTERFACE].connect_to_signal('Closed', self._text_channel_closed_cb) - self._joined = True + if CHANNEL_INTERFACE_GROUP in self._text_channel: + group = self._text_channel[CHANNEL_INTERFACE_GROUP] + + # FIXME: make these method calls async? + + group.connect_to_signal('GroupFlagsChanged', + self._text_channel_group_flags_changed_cb) + self._text_channel_group_flags = group.GetGroupFlags() + + self._self_handle = group.GetSelfHandle() + + # by the time we hook this, we need to know the group flags + group.connect_to_signal('MembersChanged', + self._text_channel_members_changed_cb) + # bootstrap by getting the current state. This is where we find + # out whether anyone was lying to us in their PEP info + members = set(group.GetMembers()) + added = members - self._member_handles + removed = self._member_handles - members + if added or removed: + self._text_channel_members_changed_cb('', added, removed, + (), (), 0, 0) + + # if we can see any member handles, we're probably able to see + # all members, so can stop caring about PEP announcements for this + # activity + self._joined = (self._self_handle in self._member_handles) + else: + self._joined = True + return True def _shared_cb(self, tp, activity_id, text_channel, exc, userdata): @@ -519,12 +581,59 @@ class Activity(ExportedGObject): if self._joined: self._text_channel[CHANNEL_INTERFACE].Close() + def _text_channel_members_changed_cb(self, message, added, removed, + local_pending, remote_pending, + actor, reason): + # Note: D-Bus calls this with list arguments, but after GetMembers() + # we call it with set and tuple arguments; we cope with any iterable. + + if (self._text_channel_group_flags & + CHANNEL_GROUP_FLAG_CHANNEL_SPECIFIC_HANDLES): + map_chan = self._text_channel + else: + # we have global handles here + map_chan = None + + # disregard any who are already there + added = set(added) + added -= self._member_handles + self._member_handles |= added + + # for added people, we need a Buddy object + added_buddies = self._ps.map_handles_to_buddies(self._tp, + map_chan, + added) + self._add_buddies(added_buddies.itervalues()) + + # we treat all pending members as if they weren't there + removed = set(removed) + removed |= set(local_pending) + removed |= set(remote_pending) + # disregard any who aren't already there + removed &= self._member_handles + self._member_handles -= removed + + # for removed people, don't bother creating a Buddy just so we can + # say it left. If we don't already have a Buddy object for someone, + # then obviously they're not in self._buddies! + removed_buddies = self._ps.map_handles_to_buddies(self._tp, + map_chan, + removed, + create=False) + self._remove_buddies(removed_buddies.itervalues()) + + # if we were among those removed, we'll have to start believing + # the spoofable PEP-based activity tracking again. + if self._self_handle not in self._member_handles: + self._joined = False + def _text_channel_closed_cb(self): """Callback method called when the text channel is closed. This callback is set up in the _handle_share_join method. """ self._joined = False + self._self_handle = None self._text_channel = None def send_properties(self): diff --git a/services/presence/presenceservice.py b/services/presence/presenceservice.py index 84814f08..6f28bf5f 100644 --- a/services/presence/presenceservice.py +++ b/services/presence/presenceservice.py @@ -211,7 +211,8 @@ class PresenceService(ExportedGObject): def _new_activity(self, activity_id, tp): try: objid = self._get_next_object_id() - activity = Activity(self._session_bus, objid, tp, id=activity_id) + activity = Activity(self._session_bus, objid, self, tp, + id=activity_id) except Exception: # FIXME: catching bare Exception considered harmful _logger.debug("Invalid activity:", exc_info=1) @@ -259,7 +260,7 @@ class PresenceService(ExportedGObject): activity = self._new_activity(act, tp) if activity is not None: - activity.buddy_joined(buddy) + activity.buddy_apparently_joined(buddy) activities_left = old_activities - new_activities for act in activities_left: @@ -268,7 +269,7 @@ class PresenceService(ExportedGObject): if not activity: continue - activity.buddy_left(buddy) + activity.buddy_apparently_left(buddy) def _activity_invitation(self, tp, act_id): activity = self._activities.get(act_id) @@ -376,6 +377,48 @@ class PresenceService(ExportedGObject): "connection to %s:%s" % (handle, tp_conn_name, tp_conn_path)) + def map_handles_to_buddies(self, tp, tp_chan, handles, create=True): + """ + + :Parameters: + `tp` : Telepathy plugin + The server or link-local plugin + `tp_chan` : telepathy.client.Channel or None + If not None, the channel in which these handles are + channel-specific + `handles` : iterable over int or long + The handles to be mapped to Buddy objects + `create` : bool + If true (default), if a corresponding `Buddy` object is not + found, create one. + :Returns: + A dict mapping handles from `handles` to `Buddy` objects. + If `create` is true, the dict's keys will be exactly the + items of `handles` in some order. If `create` is false, + the dict will contain no entry for handles for which no + `Buddy` is already available. + :Raises LookupError: if `tp` is not a plugin attached to this PS. + """ + handle_to_buddy = self._handles_buddies[tp] + + ret = {} + missing = [] + for handle in handles: + buddy = handle_to_buddy.get(handle) + if buddy is None: + missing.append(handle) + else: + ret[handle] = buddy + + if missing and create: + handle_to_objid = tp.identify_contacts(tp_chan, missing) + for handle, objid in handle_to_objid.iteritems(): + buddy = self.get_buddy(objid) + ret[handle] = buddy + if tp_chan is None: + handle_to_buddy[handle] = buddy + return ret + @dbus.service.method(_PRESENCE_INTERFACE, in_signature='', out_signature="o") def GetOwner(self): @@ -405,9 +448,9 @@ class PresenceService(ExportedGObject): objid = self._get_next_object_id() # FIXME check which tp client we should use to share the activity color = self._owner.props.color - activity = Activity(self._session_bus, objid, self._server_plugin, - id=actid, type=atype, name=name, color=color, - local=True) + activity = Activity(self._session_bus, objid, self, + self._server_plugin, id=actid, type=atype, + name=name, color=color, local=True) activity.connect("validity-changed", self._activity_validity_changed_cb) self._activities[actid] = activity From fd4e514e21f6cc5009c45d166b1714ff9c9c4aac Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Wed, 30 May 2007 17:38:58 +0100 Subject: [PATCH 08/12] services/presence/buddy: Fix thinko - register Owner with correct objid Previously the keyid/ prefix was missing. --- services/presence/buddy.py | 3 ++- services/presence/pstest.py | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/services/presence/buddy.py b/services/presence/buddy.py index b858b41a..82a9b44f 100644 --- a/services/presence/buddy.py +++ b/services/presence/buddy.py @@ -589,7 +589,8 @@ class ShellOwner(GenericOwner): icon = f.read() f.close() - GenericOwner.__init__(self, ps, bus, psutils.pubkey_to_keyid(key), + GenericOwner.__init__(self, ps, bus, + 'keyid/' + psutils.pubkey_to_keyid(key), key=key, nick=nick, color=color, icon=icon, server=server, key_hash=key_hash, registered=registered) diff --git a/services/presence/pstest.py b/services/presence/pstest.py index 3054e488..7715fd35 100644 --- a/services/presence/pstest.py +++ b/services/presence/pstest.py @@ -63,7 +63,8 @@ class TestOwner(GenericOwner): icon = _get_random_image() _logger.debug("pubkey is %s" % pubkey) - GenericOwner.__init__(self, ps, bus, pubkey_to_keyid(pubkey), + GenericOwner.__init__(self, ps, bus, + 'keyid/' + pubkey_to_keyid(pubkey), key=pubkey, nick=nick, color=color, icon=icon, registered=registered, key_hash=privkey_hash) From f90de752f66e7f3484e2b93d7530f00c0a9517fd Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Wed, 30 May 2007 17:40:31 +0100 Subject: [PATCH 09/12] services/presence/presenceservice: weakly reference Buddy objects in _buddies This makes sure we re-use an existing Buddy object if it's still referenced somewhere, rather than trying to make another and fighting over the object path. --- services/presence/presenceservice.py | 50 ++++++++++++++++++---------- 1 file changed, 32 insertions(+), 18 deletions(-) diff --git a/services/presence/presenceservice.py b/services/presence/presenceservice.py index 6f28bf5f..6c17082e 100644 --- a/services/presence/presenceservice.py +++ b/services/presence/presenceservice.py @@ -15,23 +15,24 @@ # along with this program; if not, write to the Free Software # Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA -import gobject +import logging +from weakref import WeakValueDictionary + import dbus import dbus.service +import gobject from dbus.gobject_service import ExportedGObject from dbus.mainloop.glib import DBusGMainLoop -import logging - from telepathy.client import ManagerRegistry, Connection from telepathy.interfaces import (CONN_MGR_INTERFACE, CONN_INTERFACE) from telepathy.constants import (CONNECTION_STATUS_CONNECTING, CONNECTION_STATUS_CONNECTED, CONNECTION_STATUS_DISCONNECTED) -from server_plugin import ServerPlugin -from linklocal_plugin import LinkLocalPlugin from sugar import util +from server_plugin import ServerPlugin +from linklocal_plugin import LinkLocalPlugin from buddy import Buddy, ShellOwner from activity import Activity from psutils import pubkey_to_keyid @@ -65,11 +66,20 @@ class PresenceService(ExportedGObject): self._next_object_id = 0 self._connected = False - self._buddies = {} # identifier -> Buddy - self._buddies_by_pubkey = {} # base64 public key -> Buddy - self._handles_buddies = {} # tp client -> (handle -> Buddy) + # all Buddy objects + # identifier -> Buddy, GC'd when no more refs exist + self._buddies = WeakValueDictionary() - self._activities = {} # activity id -> Activity + # the online buddies for whom we know the full public key + # base64 public key -> Buddy + self._buddies_by_pubkey = {} + + # The online buddies (those who're available via some CM) + # TP plugin -> (handle -> Buddy) + self._handles_buddies = {} + + # activity id -> Activity + self._activities = {} self._session_bus = dbus.SessionBus() self._session_bus.add_signal_receiver(self._connection_disconnected_cb, @@ -174,11 +184,7 @@ class PresenceService(ExportedGObject): def _buddy_disappeared_cb(self, buddy): if buddy.props.valid: - self.BuddyDisappeared(buddy.object_path()) - _logger.debug('Buddy left: %s (%s)', buddy.props.nick, - buddy.props.color) - self._buddies_by_pubkey.pop(buddy.props.key, None) - self._buddies.pop(buddy.props.objid, None) + self._buddy_validity_changed_cb(buddy, False) def _contact_offline(self, tp, handle): if not self._handles_buddies[tp].has_key(handle): @@ -325,10 +331,18 @@ class PresenceService(ExportedGObject): @dbus.service.method(_PRESENCE_INTERFACE, in_signature='', out_signature="ao") def GetBuddies(self): - ret = [] - for buddy in self._buddies.values(): - if buddy.props.valid: - ret.append(buddy.object_path()) + # in the presence of an out_signature, dbus-python will convert + # this set into an Array automatically (because it's iterable), + # so it's easy to use for uniquification (we want to avoid returning + # buddies who're visible on both Salut and Gabble twice) + + # always include myself even if I have no handles + ret = set((self._owner,)) + + for handles_buddies in self._handles_buddies.itervalues(): + for buddy in handles_buddies.itervalues(): + if buddy.props.valid: + ret.add(buddy.object_path()) return ret @dbus.service.method(_PRESENCE_INTERFACE, From a54aaa54beacf1579c5fe14c7ebf8c45de3a8c6a Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Wed, 30 May 2007 17:41:08 +0100 Subject: [PATCH 10/12] services/presence/presenceservice: look up buddies by key-ID if we don't know the full key yet. --- services/presence/presenceservice.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/services/presence/presenceservice.py b/services/presence/presenceservice.py index 6c17082e..5bcfd45d 100644 --- a/services/presence/presenceservice.py +++ b/services/presence/presenceservice.py @@ -350,6 +350,11 @@ class PresenceService(ExportedGObject): byte_arrays=True) def GetBuddyByPublicKey(self, key): buddy = self._buddies_by_pubkey.get(key) + if buddy is not None: + if buddy.props.valid: + return buddy.object_path() + keyid = pubkey_to_keyid(key) + buddy = self._buddies.get('keyid/' + keyid) if buddy is not None: if buddy.props.valid: return buddy.object_path() From b362ed625c6740385322bdfdf685a47e6fe625fb Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Wed, 30 May 2007 17:42:19 +0100 Subject: [PATCH 11/12] services/presence/server_plugin: fix thinko - take the right arguments to _subscribe_members_changed_cb --- services/presence/server_plugin.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/services/presence/server_plugin.py b/services/presence/server_plugin.py index b0202861..544e0220 100644 --- a/services/presence/server_plugin.py +++ b/services/presence/server_plugin.py @@ -863,8 +863,9 @@ class ServerPlugin(gobject.GObject): self._contact_online_request_properties(handle, 1) - def _subscribe_members_changed_cb(self, added, removed, local_pending, - remote_pending, actor, reason): + def _subscribe_members_changed_cb(self, message, added, removed, + local_pending, remote_pending, + actor, reason): added = set(added) removed = set(removed) From 22b1338ac5559e46136289711f5f7e91ec293839 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Wed, 30 May 2007 17:43:16 +0100 Subject: [PATCH 12/12] services/presence/server_plugin: special-case the Owner when IDing buddies. We always know who we are, so don't need to inspect our own handle or anything. --- services/presence/server_plugin.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/services/presence/server_plugin.py b/services/presence/server_plugin.py index 544e0220..548b41fd 100644 --- a/services/presence/server_plugin.py +++ b/services/presence/server_plugin.py @@ -1131,20 +1131,27 @@ class ServerPlugin(gobject.GObject): owners = handles if tp_chan is not None and CHANNEL_INTERFACE_GROUP in tp_chan: - group = tp_chan[CHANNEL_INTERFACE_GROUP] - if group.GetFlags() & CHANNEL_GROUP_FLAG_CHANNEL_SPECIFIC_HANDLES: - + if (group.GetGroupFlags() & + CHANNEL_GROUP_FLAG_CHANNEL_SPECIFIC_HANDLES): owners = group.GetHandleOwners(handles) for i, owner in enumerate(owners): if owner == 0: owners[i] = handles[i] + else: + group = None jids = self._conn[CONN_INTERFACE].InspectHandles(HANDLE_TYPE_CONTACT, owners) ret = {} for handle, jid in zip(handles, jids): + # special-case the Owner - we always know who we are + if (handle == self.self_handle or + (group is not None and handle == group.GetSelfHandle())): + ret[handle] = self._owner.props.objid + continue + if '/' in jid: # the contact is unidentifiable (in an anonymous MUC) - create # a temporary identity for them, based on their room-JID