Merge lp:~alecu/ubuntu-sso-client/keyring-api-change into lp:ubuntu-sso-client

[Alejandro J. Cura]

Use the keyring unlocking gnomekeyring APIs (LP: #623622)
Search all keyrings for the credentials (LP: #624033)

Use the keyring unlocking gnomekeyring apis and search all keyrings for the credentials

[John Lenton (chipaca)]

tests are delicious!

review: Approve

[Rodrigo Moya (rodrigo-moya)]

review: Approve

=== modified file 'ubuntu_sso/keyring.py'
--- ubuntu_sso/keyring.py	2010-08-19 13:39:50 +0000
+++ ubuntu_sso/keyring.py	2010-08-26 21:18:42 +0000
@@ -19,77 +19,128 @@
 
 """Handle keys in the gnome kerying."""
 
+import socket
+import urllib
+import urlparse
+
 import gnomekeyring
 
-from urllib import urlencode
-from urlparse import parse_qsl
+from ubuntu_sso.logger import setupLogging
+logger = setupLogging("ubuntu_sso.main")
+
+U1_APP_NAME = "Ubuntu One"
+U1_KEY_NAME = "UbuntuOne token for https://ubuntuone.com"
+U1_KEY_ATTR = {
+    "oauth-consumer-key": "ubuntuone",
+    "ubuntuone-realm": "https://ubuntuone.com",
+}
+
+
+def get_token_name(app_name):
+    """Build the token name."""
+    quoted_app_name = urllib.quote(app_name)
+    computer_name = socket.gethostname()
+    quoted_computer_name = urllib.quote(computer_name)
+    return "%s - %s" % (quoted_app_name, quoted_computer_name)
 
 
 class Keyring(object):
-
+    """A Keyring for a given application name."""
     KEYRING_NAME = "login"
 
     def __init__(self, app_name):
+        """Initialize this instance given the app_name."""
         if not gnomekeyring.is_available():
             raise gnomekeyring.NoKeyringDaemonError
         self.app_name = app_name
+        self.token_name = get_token_name(self.app_name)
 
     def _create_keyring(self, name):
-        """Creates a keyring, if it already exists, do nothing."""
+        """Creates a keyring, or if it already exists, it does nothing."""
         keyring_names = gnomekeyring.list_keyring_names_sync()
         if not name in keyring_names:
             gnomekeyring.create_sync(name)
 
-    def _get_item_id_from_name(self, sync, name):
-        """Return the ID for a named item."""
-        for item_id in gnomekeyring.list_item_ids_sync(sync):
-            item_info = gnomekeyring.item_get_info_sync(sync, item_id)
-            display_name = item_info.get_display_name()
-            if display_name == name:
-                return item_id
-
-    def _get_item_info_from_name(self, sync, name):
-        """Return the ID for a named item."""
-        for item_id in gnomekeyring.list_item_ids_sync(sync):
-            item_info = gnomekeyring.item_get_info_sync(sync, item_id)
-            display_name = item_info.get_display_name()
-            if display_name == name:
-                return item_info
+    def _find_keyring_item(self):
+        """Return the keyring item or None if not found."""
+        try:
+            items = gnomekeyring.find_items_sync(
+                                        gnomekeyring.ITEM_GENERIC_SECRET,
+                                        self._get_keyring_attr())
+        except gnomekeyring.NoMatchError:
+            # if no items found, return None
+            return None
+
+        # we priorize the item in the "login" keyring
+        for item in items:
+            if item.keyring == "login":
+                return item
+
+        # if not on the "login" keyring, we return the first item
+        return items[0]
+
+    def _get_keyring_attr(self):
+        """Build the keyring attributes for this credentials."""
+        attr = {"key-type": "Ubuntu SSO credentials",
+                "token-name": self.token_name}
+        return attr
 
     def set_ubuntusso_attr(self, cred):
         """Set the credentials of the Ubuntu SSO item."""
         # Creates the secret from the credentials
-        secret = urlencode(cred)
+        secret = urllib.urlencode(cred)
 
-        # Create the keyring
+        # Create the keyring if it does not exists
         self._create_keyring(self.KEYRING_NAME)
 
-        # No need to delete the item if it already exists
-
-        # A set of attributes for this credentials
-        attr = {"key-type": "Ubuntu SSO credentials",
-                "token-name": cred["name"].encode("utf8")}
-
         # Add our SSO credentials to the keyring
         gnomekeyring.item_create_sync(self.KEYRING_NAME,
-                gnomekeyring.ITEM_GENERIC_SECRET, self.app_name, attr,
-                secret, True)
+                gnomekeyring.ITEM_GENERIC_SECRET, self.app_name,
+                self._get_keyring_attr(), secret, True)
 
     def get_ubuntusso_attr(self):
         """Return the secret of the SSO item in a dictionary."""
         # If we have no attributes, return None
-        exist = self._get_item_info_from_name(self.KEYRING_NAME, self.app_name)
-        if exist is not None:
-            secret = self._get_item_info_from_name(self.KEYRING_NAME,
-                                                   self.app_name).get_secret()
-            return dict(parse_qsl(secret))
+        item = self._find_keyring_item()
+        if item is not None:
+            return dict(urlparse.parse_qsl(item.secret))
+        else:
+            # if no item found, try getting the old credentials
+            if self.app_name == U1_APP_NAME:
+                return try_old_credentials(self.app_name)
+        # nothing was found
+        return None
 
     def delete_ubuntusso_attr(self):
         """Delete a set of credentials from the keyring."""
-        item_id = self._get_item_id_from_name(self.KEYRING_NAME,
-                                              self.app_name)
-        if item_id is not None:
-            gnomekeyring.item_delete_sync(self.KEYRING_NAME, item_id)
+        item = self._find_keyring_item()
+        if item is not None:
+            gnomekeyring.item_delete_sync(item.keyring, item.item_id)
+
+
+class UbuntuOneOAuthKeyring(Keyring):
+
+    def _get_keyring_attr(self):
+        """Build the keyring attributes for this credentials."""
+        return U1_KEY_ATTR
+
+
+def try_old_credentials(app_name):
+    """Try to get old U1 credentials and format them as new."""
+    logger.debug('trying to get old credentials.')
+    old_creds = UbuntuOneOAuthKeyring(U1_KEY_NAME).get_ubuntusso_attr()
+    if old_creds is not None:
+        # Old creds found, build a new credentials dict with them
+        creds = {
+            'consumer_key': "ubuntuone",
+            'consumer_secret': "hammertime",
+            'name': U1_KEY_NAME,
+            'token': old_creds["oauth_token"],
+            'token_secret': old_creds["oauth_token_secret"],
+        }
+        logger.debug('found old credentials')
+        return creds
+    logger.debug('try_old_credentials: No old credentials for this app.')
 
 
 if __name__ == "__main__":

=== modified file 'ubuntu_sso/main.py'
--- ubuntu_sso/main.py	2010-08-26 13:43:31 +0000
+++ ubuntu_sso/main.py	2010-08-26 21:18:42 +0000
@@ -28,11 +28,9 @@
 """
 
 import re
-import socket
 import time
 import threading
 import traceback
-import urllib
 import urllib2
 import urlparse
 
@@ -49,7 +47,7 @@
 from ubuntu_sso import (DBUS_IFACE_AUTH_NAME, DBUS_IFACE_USER_NAME,
     DBUS_IFACE_CRED_NAME, DBUS_CRED_PATH, DBUS_BUS_NAME, gui)
 from ubuntu_sso.config import get_config
-from ubuntu_sso.keyring import Keyring
+from ubuntu_sso.keyring import Keyring, get_token_name, U1_APP_NAME
 from ubuntu_sso.logger import setupLogging
 logger = setupLogging("ubuntu_sso.main")
 
@@ -57,8 +55,6 @@
 # Disable the invalid name warning, as we have a lot of DBus style names
 # pylint: disable-msg=C0103
 
-OLD_KEY_NAME = "UbuntuOne token for https://ubuntuone.com"
-U1_APP_NAME = "Ubuntu One"
 PING_URL = "http://edge.one.ubuntu.com/oauth/sso-finished-so-get-tokens/"
 
 
@@ -113,32 +109,9 @@
     creds = Keyring(app_name).get_ubuntusso_attr()
     logger.debug('keyring_get_credentials: Keyring returned credentials? %r',
                  creds is not None)
-    if creds is None and app_name == U1_APP_NAME:
-        logger.debug('keyring_get_credentials: trying for old service.')
-        # No new creds, try to get old credentials
-        old_creds = Keyring(OLD_KEY_NAME).get_ubuntusso_attr()
-        if old_creds is not None:
-            # Old creds found, build a new credentials dict with them
-            creds = {
-                'consumer_key': "ubuntuone",
-                'consumer_secret': "hammertime",
-                'token_name': OLD_KEY_NAME,
-                'token': old_creds["oauth_token"],
-                'token_secret': old_creds["oauth_token_secret"],
-            }
-    else:
-        logger.debug('keyring_get_credentials: Keyring returned credentials.')
     return creds
 
 
-def get_token_name(app_name):
-    """Build the token name."""
-    quoted_app_name = urllib.quote(app_name)
-    computer_name = socket.gethostname()
-    quoted_computer_name = urllib.quote(computer_name)
-    return "%s - %s" % (quoted_app_name, quoted_computer_name)
-
-
 class SSOLoginProcessor(object):
     """Login and register users using the Ubuntu Single Sign On service."""
 

=== modified file 'ubuntu_sso/tests/test_keyring.py'
--- ubuntu_sso/tests/test_keyring.py	2010-08-19 05:50:41 +0000
+++ ubuntu_sso/tests/test_keyring.py	2010-08-26 21:18:42 +0000
@@ -17,24 +17,39 @@
 # with this program.  If not, see <http://www.gnu.org/licenses/>.
 """Tests for the keyring.py module."""
 
+import itertools
+import socket
+import urllib
+
 import gnomekeyring
 from twisted.trial.unittest import TestCase
 
 from ubuntu_sso import keyring
 
 
-class MockInfoSync(object):
-    """A mock object that represents a fake key."""
-
-    def __init__(self, (key, key_name, secret)):
+def build_fake_gethostname(fake_hostname):
+    """Return a fake hostname getter."""
+    return lambda *a: fake_hostname
+
+
+class MockKeyringItem(object):
+    """A mock object that fakes an item found in a keyring search."""
+
+    def __init__(self, item_id, keyring, attributes, secret):
         """Initialize this instance."""
-        self.key = key
-        self.key_name = key_name
+        self.item_id = item_id
+        self.keyring = keyring
+        self.attributes = attributes
         self.secret = secret
 
-    def get_display_name(self):
-        """Return info on the mocked object."""
-        return self.key_name
+    def matches(self, search_attr):
+        """See if this item matches a given search."""
+        for k, v in search_attr.items():
+            if k not in self.attributes:
+                return False
+            if self.attributes[k] != v:
+                return False
+        return True
 
 
 class MockGnomeKeyring(object):
@@ -43,28 +58,33 @@
     def __init__(self):
         """Initialize this instance."""
         self.keyrings = set()
-        self.store = []
+        self.id_counter = itertools.count()
+        self.store = {}
         self.deleted = []
 
+    def _get_next_id(self):
+        """Return the next keyring id."""
+        return self.id_counter.next()
+
     def item_create_sync(self, keyring_name, item_type, key_name, attr,
                          secret, update_if_exists):
-        """Sets a value in the keyring."""
-        # we'll use the attr as the dict key, so make it a tuple
-        key = tuple(attr.items())
-        self.store.append((key, key_name, secret))
+        """Add a key to a keyring."""
+        new_id = self._get_next_id()
+        i = MockKeyringItem(new_id, keyring_name, attr, secret)
+        self.store[new_id] = i
 
     def item_delete_sync(self, keyring_name, item_id):
-        """Makes a list of deleted items."""
-        key, key_name, secret = self.store.pop(item_id)
-        self.deleted.append(key_name)
-
-    def list_item_ids_sync(self, keyring_name):
-        """Return a list of ids for all the items."""
-        return [n for n, v in enumerate(self.store)]
-
-    def item_get_info_sync(self, keyring_name, item_id):
-        """Return an info sync object for the item."""
-        return MockInfoSync(self.store[item_id])
+        """Delete a key from a keyring, and keep a list of deleted keys."""
+        item = self.store.pop(item_id)
+        assert keyring_name == item.keyring
+        self.deleted.append(item)
+
+    def find_items_sync(self, item_type, attr):
+        """Find all keys that match the given attributes."""
+        items = [i for i in self.store.values() if i.matches(attr)]
+        if len(items) == 0:
+            raise gnomekeyring.NoMatchError()
+        return items
 
     def list_keyring_names_sync(self):
         """The keyring you are looking for may be here."""
@@ -79,6 +99,32 @@
         return True
 
 
+class TestTokenNameBuilder(TestCase):
+    """Test the method that builds the token name."""
+
+    def test_get_simple_token_name(self):
+        """A simple token name is built right."""
+        sample_app_name = "UbuntuTwo"
+        sample_hostname = "Darkstar"
+        expected_result = "UbuntuTwo - Darkstar"
+
+        fake_gethostname = build_fake_gethostname(sample_hostname)
+        self.patch(socket, "gethostname", fake_gethostname)
+        result = keyring.get_token_name(sample_app_name)
+        self.assertEqual(result, expected_result)
+
+    def test_get_complex_token_name(self):
+        """A complex token name is built right too."""
+        sample_app_name = "Ubuntu Eleven"
+        sample_hostname = "Mate+Cocido"
+        expected_result = "Ubuntu%20Eleven - Mate%2BCocido"
+
+        fake_gethostname = build_fake_gethostname(sample_hostname)
+        self.patch(socket, "gethostname", fake_gethostname)
+        result = keyring.get_token_name(sample_app_name)
+        self.assertEqual(result, expected_result)
+
+
 class TestKeyring(TestCase):
     """Test the gnome keyring related functions."""
     def setUp(self):
@@ -89,11 +135,10 @@
         self.patch(gnomekeyring, "create_sync", self.mgk.create_sync)
         self.patch(gnomekeyring, "list_keyring_names_sync",
                                               self.mgk.list_keyring_names_sync)
-        self.patch(gnomekeyring, "list_item_ids_sync",
-                                                   self.mgk.list_item_ids_sync)
-        self.patch(gnomekeyring, "item_get_info_sync",
-                                                   self.mgk.item_get_info_sync)
+        self.patch(gnomekeyring, "find_items_sync", self.mgk.find_items_sync)
         self.patch(gnomekeyring, "item_delete_sync", self.mgk.item_delete_sync)
+        fake_gethostname = build_fake_gethostname("darkstar")
+        self.patch(socket, "gethostname", fake_gethostname)
 
     def test_set_ubuntusso(self):
         """Test that the set method does not erase previous keys."""
@@ -113,3 +158,54 @@
 
         self.assertEqual(len(self.mgk.store), 0)
         self.assertEqual(len(self.mgk.deleted), 1)
+
+    def test_get_credentials(self):
+        """Test that credentials are properly retrieved."""
+        sample_creds = {"name": "sample creds name"}
+        keyring.Keyring("appname").set_ubuntusso_attr(sample_creds)
+
+        result = keyring.Keyring("appname").get_ubuntusso_attr()
+        self.assertEqual(result, sample_creds)
+
+    def test_get_old_cred_found(self):
+        """The method returns a new set of creds if old creds are found."""
+        sample_oauth_token = "sample oauth token"
+        sample_oauth_secret = "sample oauth secret"
+        old_creds = {
+            "oauth_token": sample_oauth_token,
+            "oauth_token_secret": sample_oauth_secret,
+        }
+        secret = urllib.urlencode(old_creds)
+        self.mgk.item_create_sync(keyring.U1_APP_NAME, None,
+                                  keyring.Keyring.KEYRING_NAME,
+                                  keyring.U1_KEY_ATTR,
+                                  secret, True)
+
+        result = keyring.Keyring(keyring.U1_APP_NAME).get_ubuntusso_attr()
+
+        self.assertIn("token", result)
+        self.assertEqual(result["token"], sample_oauth_token)
+        self.assertIn("token_secret", result)
+        self.assertEqual(result["token_secret"], sample_oauth_secret)
+
+    def test_get_old_cred_found_but_not_asked_for(self):
+        """Returns None if old creds are present but the appname is not U1"""
+        sample_oauth_token = "sample oauth token"
+        sample_oauth_secret = "sample oauth secret"
+        old_creds = {
+            "oauth_token": sample_oauth_token,
+            "oauth_token_secret": sample_oauth_secret,
+        }
+        secret = urllib.urlencode(old_creds)
+        self.mgk.item_create_sync(keyring.U1_APP_NAME, None,
+                                  keyring.Keyring.KEYRING_NAME,
+                                  keyring.U1_KEY_ATTR,
+                                  secret, True)
+
+        result = keyring.Keyring("Software Center").get_ubuntusso_attr()
+        self.assertEqual(result, None)
+
+    def test_get_old_cred_not_found(self):
+        """The method returns None if no old nor new credentials found."""
+        result = keyring.Keyring(keyring.U1_APP_NAME).get_ubuntusso_attr()
+        self.assertEqual(result, None)

=== modified file 'ubuntu_sso/tests/test_main.py'
--- ubuntu_sso/tests/test_main.py	2010-08-26 13:43:31 +0000
+++ ubuntu_sso/tests/test_main.py	2010-08-26 21:18:42 +0000
@@ -35,15 +35,14 @@
 
 from contrib.testing.testcase import MementoHandler
 from ubuntu_sso import config, gui
+from ubuntu_sso.keyring import get_token_name, U1_APP_NAME
 from ubuntu_sso.main import (
     AuthenticationError, BadRealmError, blocking, EmailTokenError,
-    except_to_errdict, get_token_name,
-    InvalidEmailError, InvalidPasswordError,
+    except_to_errdict, InvalidEmailError, InvalidPasswordError,
     keyring_get_credentials, keyring_store_credentials, logger,
-    LoginProcessor, NewPasswordError, OLD_KEY_NAME, PING_URL,
+    LoginProcessor, NewPasswordError, PING_URL,
     RegistrationError, ResetPasswordTokenError,
-    SSOCredentials, SSOLogin, SSOLoginProcessor,
-    U1_APP_NAME)
+    SSOCredentials, SSOLogin, SSOLoginProcessor)
 
 
 APP_NAME = 'The Coolest App Ever'
@@ -842,58 +841,6 @@
         token = keyring_get_credentials(APP_NAME)
         self.assertEqual(token, None)
 
-    def test_keyring_get_old_cred_found(self):
-        """The method returns a new set of creds if old creds are found."""
-        sample_oauth_token = "sample oauth token"
-        sample_oauth_secret = "sample oauth secret"
-        old_creds = {
-            "oauth_token": sample_oauth_token,
-            "oauth_token_secret": sample_oauth_secret,
-        }
-
-        mockKeyringClass = self.mocker.replace("ubuntu_sso.keyring.Keyring")
-        mockKeyringClass(U1_APP_NAME)
-        mockKeyring = self.mocker.mock()
-        self.mocker.result(mockKeyring)
-        mockKeyring.get_ubuntusso_attr()
-        self.mocker.result(None)
-
-        mockKeyringClass(OLD_KEY_NAME)
-        mockKeyring2 = self.mocker.mock()
-        self.mocker.result(mockKeyring2)
-        mockKeyring2.get_ubuntusso_attr()
-        self.mocker.result(old_creds)
-
-        self.mocker.replay()
-
-        new_creds = keyring_get_credentials(U1_APP_NAME)
-        self.assertIn("token", new_creds)
-        self.assertEqual(new_creds["token"], sample_oauth_token)
-        self.assertIn("token_secret", new_creds)
-        self.assertEqual(new_creds["token_secret"], sample_oauth_secret)
-        self.assertIn("token_name", new_creds)
-        self.assertEqual(new_creds["token_name"], OLD_KEY_NAME)
-
-    def test_keyring_get_old_cred_not_found(self):
-        """The method returns None if no old nor new credentials found."""
-        mockKeyringClass = self.mocker.replace("ubuntu_sso.keyring.Keyring")
-        mockKeyringClass(U1_APP_NAME)
-        mockKeyring = self.mocker.mock()
-        self.mocker.result(mockKeyring)
-        mockKeyring.get_ubuntusso_attr()
-        self.mocker.result(None)
-
-        mockKeyringClass(OLD_KEY_NAME)
-        mockKeyring2 = self.mocker.mock()
-        self.mocker.result(mockKeyring2)
-        mockKeyring2.get_ubuntusso_attr()
-        self.mocker.result(None)
-
-        self.mocker.replay()
-
-        token = keyring_get_credentials(U1_APP_NAME)
-        self.assertEqual(token, None)
-
 
 class RegisterSampleException(Exception):
     """A mock exception thrown just when testing."""