← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~cjwatson/launchpad/stormify-oauth into lp:launchpad

 

Colin Watson has proposed merging lp:~cjwatson/launchpad/stormify-oauth into lp:launchpad.

Commit message:
Convert the OAuth service to Storm.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/stormify-oauth/+merge/283985

Convert the OAuth service to Storm.

I wanted to do this before doing more extensive work on it for improved token contexts.  The complicated bit, of course, was that consumer keys etc. now need to be consistently Unicode.  I fixed some of the tests directly, but a few places needed to tolerate receiving bytes, especially things that interact directly with requests.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/stormify-oauth into lp:launchpad.
=== modified file 'lib/lp/services/oauth/browser/__init__.py'
--- lib/lp/services/oauth/browser/__init__.py	2014-06-27 08:27:31 +0000
+++ lib/lp/services/oauth/browser/__init__.py	2016-01-26 15:52:17 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -79,7 +79,10 @@
         with a 401 status.  If the key is not empty but there's no consumer
         with it, we register a new consumer.
         """
-        form = get_oauth_authorization(self.request)
+        try:
+            form = get_oauth_authorization(self.request)
+        except UnicodeDecodeError:
+            raise UnexpectedFormData("Invalid UTF-8.")
         consumer_key = form.get('oauth_consumer_key')
         if not consumer_key:
             self.request.unauthorized(OAUTH_CHALLENGE)
@@ -306,7 +309,10 @@
 
     def initialize(self):
         self.storeTokenContext()
-        form = get_oauth_authorization(self.request)
+        try:
+            form = get_oauth_authorization(self.request)
+        except UnicodeDecodeError:
+            raise UnexpectedFormData("Invalid UTF-8.")
         key = form.get('oauth_token')
         if key:
             self.token = getUtility(IOAuthRequestTokenSet).getByKey(key)
@@ -349,7 +355,10 @@
         # We have no guarantees that lp.context will be together with the
         # OAuth parameters, so we need to check in the Authorization header
         # and on the request's form if it's not in the former.
-        oauth_data = get_oauth_authorization(self.request)
+        try:
+            oauth_data = get_oauth_authorization(self.request)
+        except UnicodeDecodeError:
+            raise UnexpectedFormData("Invalid UTF-8.")
         context = oauth_data.get('lp.context')
         if not context:
             context = self.request.form.get('lp.context')

=== modified file 'lib/lp/services/oauth/doc/oauth-pages.txt'
--- lib/lp/services/oauth/doc/oauth-pages.txt	2014-06-13 14:22:09 +0000
+++ lib/lp/services/oauth/doc/oauth-pages.txt	2016-01-26 15:52:17 +0000
@@ -14,7 +14,7 @@
     >>> from lp.services.webapp.interfaces import ILaunchpadRoot
     >>> from lp.services.webapp.servers import LaunchpadTestRequest
 
-    >>> consumer = getUtility(IOAuthConsumerSet).getByKey('launchpad-library')
+    >>> consumer = getUtility(IOAuthConsumerSet).getByKey(u'launchpad-library')
     >>> root = getUtility(ILaunchpadRoot)
     >>> def get_view_with_fresh_token(form):
     ...     token, _ = consumer.newRequestToken()
@@ -44,7 +44,6 @@
 When the client specifies a duration, the resulting request
 token will have an appropriate expiration date set.
 
-    >>> from datetime import datetime
     >>> import pytz
     >>> from lp.services.oauth.browser import (
     ...     TemporaryIntegrations)

=== modified file 'lib/lp/services/oauth/model.py'
--- lib/lp/services/oauth/model.py	2015-07-08 16:05:11 +0000
+++ lib/lp/services/oauth/model.py	2016-01-26 15:52:17 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2014 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -17,13 +17,15 @@
     )
 import hashlib
 import re
+from storm.locals import (
+    Bool,
+    DateTime,
+    Int,
+    Reference,
+    Unicode,
+    )
 
 import pytz
-from sqlobject import (
-    BoolCol,
-    ForeignKey,
-    StringCol,
-    )
 from zope.interface import implementer
 
 from lp.registry.interfaces.distribution import IDistribution
@@ -33,11 +35,9 @@
 from lp.registry.interfaces.product import IProduct
 from lp.registry.interfaces.projectgroup import IProjectGroup
 from lp.services.database.constants import UTC_NOW
-from lp.services.database.datetimecol import UtcDateTimeCol
-from lp.services.database.enumcol import EnumCol
+from lp.services.database.enumcol import DBEnum
 from lp.services.database.interfaces import IMasterStore
-from lp.services.database.sqlbase import SQLBase
-from lp.services.librarian.model import LibraryFileAlias
+from lp.services.database.stormbase import StormBase
 from lp.services.oauth.interfaces import (
     IOAuthAccessToken,
     IOAuthConsumer,
@@ -62,32 +62,34 @@
 class OAuthBase:
     """Base class for all OAuth database classes."""
 
-    @staticmethod
-    def _get_store():
-        """See `SQLBase`.
+    @classmethod
+    def _getStore(cls):
+        """Return the correct store for this class.
 
         We want all OAuth classes to be retrieved from the master flavour.  If
         they are retrieved from the slave, there will be problems in the
         authorization exchange, since it will be done across applications that
         won't share the session cookies.
         """
-        return IMasterStore(LibraryFileAlias)
-
-    getStore = _get_store
+        return IMasterStore(cls)
 
 
 @implementer(IOAuthConsumer)
-class OAuthConsumer(OAuthBase, SQLBase):
+class OAuthConsumer(OAuthBase, StormBase):
     """See `IOAuthConsumer`."""
 
-    date_created = UtcDateTimeCol(default=UTC_NOW, notNull=True)
-    disabled = BoolCol(notNull=True, default=False)
-    key = StringCol(notNull=True)
-    _secret = StringCol(dbName="secret", notNull=False, default='')
+    __storm_table__ = 'OAuthConsumer'
+
+    id = Int(primary=True)
+    date_created = DateTime(tzinfo=pytz.UTC, allow_none=False, default=UTC_NOW)
+    disabled = Bool(allow_none=False, default=False)
+    key = Unicode(allow_none=False)
+    _secret = Unicode(name='secret', allow_none=True, default=u'')
 
     def __init__(self, key, secret):
-        secret = hashlib.sha256(secret).hexdigest()
-        super(OAuthConsumer, self).__init__(key=key, _secret=secret)
+        super(OAuthConsumer, self).__init__()
+        self.key = key
+        self._secret = unicode(hashlib.sha256(secret).hexdigest())
 
     # This regular expression singles out a consumer key that
     # represents any and all apps running on a specific computer. The
@@ -148,74 +150,90 @@
 
     def isSecretValid(self, secret):
         """See `IOAuthConsumer`."""
-        return hashlib.sha256(secret).hexdigest() == self._secret
+        return unicode(hashlib.sha256(secret).hexdigest()) == self._secret
 
     def newRequestToken(self):
         """See `IOAuthConsumer`."""
         key, secret = create_token_key_and_secret(table=OAuthRequestToken)
-        return (
-            OAuthRequestToken(consumer=self, key=key, secret=secret), secret)
+        token = OAuthRequestToken(consumer=self, key=key, secret=secret)
+        OAuthRequestToken._getStore().add(token)
+        return token, secret
 
     def getAccessToken(self, key):
         """See `IOAuthConsumer`."""
-        return OAuthAccessToken.selectOneBy(key=key, consumer=self)
+        return OAuthAccessToken._getStore().find(
+            OAuthAccessToken,
+            OAuthAccessToken.key == key,
+            OAuthAccessToken.consumer == self).one()
 
     def getRequestToken(self, key):
         """See `IOAuthConsumer`."""
-        return OAuthRequestToken.selectOneBy(key=key, consumer=self)
+        return OAuthRequestToken._getStore().find(
+            OAuthRequestToken,
+            OAuthRequestToken.key == key,
+            OAuthRequestToken.consumer == self).one()
 
 
 @implementer(IOAuthConsumerSet)
 class OAuthConsumerSet:
     """See `IOAuthConsumerSet`."""
 
-    def new(self, key, secret=''):
+    def new(self, key, secret=u''):
         """See `IOAuthConsumerSet`."""
         assert self.getByKey(key) is None, (
             "The key '%s' is already in use by another consumer." % key)
-        return OAuthConsumer(key=key, secret=secret)
+        consumer = OAuthConsumer(key=key, secret=secret)
+        OAuthConsumer._getStore().add(consumer)
+        return consumer
 
     def getByKey(self, key):
         """See `IOAuthConsumerSet`."""
-        return OAuthConsumer.selectOneBy(key=key)
+        return OAuthConsumer._getStore().find(
+            OAuthConsumer, OAuthConsumer.key == key).one()
 
 
 @implementer(IOAuthAccessToken)
-class OAuthAccessToken(OAuthBase, SQLBase):
+class OAuthAccessToken(OAuthBase, StormBase):
     """See `IOAuthAccessToken`."""
 
-    consumer = ForeignKey(
-        dbName='consumer', foreignKey='OAuthConsumer', notNull=True)
-    person = ForeignKey(
-        dbName='person', foreignKey='Person', notNull=False, default=None)
-    date_created = UtcDateTimeCol(default=UTC_NOW, notNull=True)
-    date_expires = UtcDateTimeCol(notNull=False, default=None)
-    key = StringCol(notNull=True)
-    _secret = StringCol(dbName="secret", notNull=False, default='')
-
-    permission = EnumCol(enum=AccessLevel, notNull=True)
-
-    product = ForeignKey(
-        dbName='product', foreignKey='Product', notNull=False, default=None)
-    projectgroup = ForeignKey(
-        dbName='project', foreignKey='ProjectGroup', notNull=False,
-        default=None)
-    sourcepackagename = ForeignKey(
-        dbName='sourcepackagename', foreignKey='SourcePackageName',
-        notNull=False, default=None)
-    distribution = ForeignKey(
-        dbName='distribution', foreignKey='Distribution',
-        notNull=False, default=None)
-
-    def __init__(self, consumer, permission, key, secret='', person=None,
+    __storm_table__ = 'OAuthAccessToken'
+
+    id = Int(primary=True)
+    consumer_id = Int(name='consumer', allow_none=False)
+    consumer = Reference(consumer_id, 'OAuthConsumer.id')
+    person_id = Int(name='person', allow_none=False)
+    person = Reference(person_id, 'Person.id')
+    date_created = DateTime(tzinfo=pytz.UTC, allow_none=False, default=UTC_NOW)
+    date_expires = DateTime(tzinfo=pytz.UTC, allow_none=True, default=None)
+    key = Unicode(allow_none=False)
+    _secret = Unicode(name='secret', allow_none=True, default=u'')
+
+    permission = DBEnum(enum=AccessLevel, allow_none=False)
+
+    product_id = Int(name='product', allow_none=True, default=None)
+    product = Reference(product_id, 'Product.id')
+    projectgroup_id = Int(name='project', allow_none=True, default=None)
+    projectgroup = Reference(projectgroup_id, 'ProjectGroup.id')
+    sourcepackagename_id = Int(
+        name='sourcepackagename', allow_none=True, default=None)
+    sourcepackagename = Reference(sourcepackagename_id, 'SourcePackageName.id')
+    distribution_id = Int(name='distribution', allow_none=True, default=None)
+    distribution = Reference(distribution_id, 'Distribution.id')
+
+    def __init__(self, consumer, permission, key, secret=u'', person=None,
                  date_expires=None, product=None, projectgroup=None,
                  distribution=None, sourcepackagename=None):
-        secret = hashlib.sha256(secret).hexdigest()
-        super(OAuthAccessToken, self).__init__(
-            consumer=consumer, permission=permission, key=key,
-            _secret=secret, person=person, date_expires=date_expires,
-            product=product, projectgroup=projectgroup,
-            distribution=distribution, sourcepackagename=sourcepackagename)
+        super(OAuthAccessToken, self).__init__()
+        self.consumer = consumer
+        self.permission = permission
+        self.key = key
+        self._secret = unicode(hashlib.sha256(secret).hexdigest())
+        self.person = person
+        self.date_expires = date_expires
+        self.product = product
+        self.projectgroup = projectgroup
+        self.distribution = distribution
+        self.sourcepackagename = sourcepackagename
 
     @property
     def context(self):
@@ -235,51 +253,57 @@
 
     @property
     def is_expired(self):
-        now = datetime.now(pytz.timezone('UTC'))
+        now = datetime.now(pytz.UTC)
         return self.date_expires is not None and self.date_expires <= now
 
     def isSecretValid(self, secret):
-        """See `IOAuthConsumer`."""
-        return hashlib.sha256(secret).hexdigest() == self._secret
+        """See `IOAuthToken`."""
+        return unicode(hashlib.sha256(secret).hexdigest()) == self._secret
 
 
 @implementer(IOAuthRequestToken)
-class OAuthRequestToken(OAuthBase, SQLBase):
+class OAuthRequestToken(OAuthBase, StormBase):
     """See `IOAuthRequestToken`."""
 
-    consumer = ForeignKey(
-        dbName='consumer', foreignKey='OAuthConsumer', notNull=True)
-    person = ForeignKey(
-        dbName='person', foreignKey='Person', notNull=False, default=None)
-    date_created = UtcDateTimeCol(default=UTC_NOW, notNull=True)
-    date_expires = UtcDateTimeCol(notNull=False, default=None)
-    key = StringCol(notNull=True)
-    _secret = StringCol(dbName="secret", notNull=False, default='')
-
-    permission = EnumCol(enum=OAuthPermission, notNull=False, default=None)
-    date_reviewed = UtcDateTimeCol(default=None, notNull=False)
-
-    product = ForeignKey(
-        dbName='product', foreignKey='Product', notNull=False, default=None)
-    projectgroup = ForeignKey(
-        dbName='project', foreignKey='ProjectGroup', notNull=False,
-        default=None)
-    sourcepackagename = ForeignKey(
-        dbName='sourcepackagename', foreignKey='SourcePackageName',
-        notNull=False, default=None)
-    distribution = ForeignKey(
-        dbName='distribution', foreignKey='Distribution',
-        notNull=False, default=None)
-
-    def __init__(self, consumer, key, secret='', permission=None, person=None,
+    __storm_table__ = 'OAuthRequestToken'
+
+    id = Int(primary=True)
+    consumer_id = Int(name='consumer', allow_none=False)
+    consumer = Reference(consumer_id, 'OAuthConsumer.id')
+    person_id = Int(name='person', allow_none=True, default=None)
+    person = Reference(person_id, 'Person.id')
+    date_created = DateTime(tzinfo=pytz.UTC, allow_none=False, default=UTC_NOW)
+    date_expires = DateTime(tzinfo=pytz.UTC, allow_none=True, default=None)
+    key = Unicode(allow_none=False)
+    _secret = Unicode(name='secret', allow_none=True, default=u'')
+
+    permission = DBEnum(enum=OAuthPermission, allow_none=True, default=None)
+    date_reviewed = DateTime(tzinfo=pytz.UTC, allow_none=True, default=None)
+
+    product_id = Int(name='product', allow_none=True, default=None)
+    product = Reference(product_id, 'Product.id')
+    projectgroup_id = Int(name='project', allow_none=True, default=None)
+    projectgroup = Reference(projectgroup_id, 'ProjectGroup.id')
+    sourcepackagename_id = Int(
+        name='sourcepackagename', allow_none=True, default=None)
+    sourcepackagename = Reference(sourcepackagename_id, 'SourcePackageName.id')
+    distribution_id = Int(name='distribution', allow_none=True, default=None)
+    distribution = Reference(distribution_id, 'Distribution.id')
+
+    def __init__(self, consumer, key, secret=u'', permission=None, person=None,
                  date_expires=None, product=None, projectgroup=None,
                  distribution=None, sourcepackagename=None):
-        secret = hashlib.sha256(secret).hexdigest()
-        super(OAuthRequestToken, self).__init__(
-            consumer=consumer, permission=permission, key=key,
-            _secret=secret, person=person, date_expires=date_expires,
-            product=product, projectgroup=projectgroup,
-            distribution=distribution, sourcepackagename=sourcepackagename)
+        super(OAuthRequestToken, self).__init__()
+        self.consumer = consumer
+        self.permission = permission
+        self.key = key
+        self._secret = unicode(hashlib.sha256(secret).hexdigest())
+        self.person = person
+        self.date_expires = date_expires
+        self.product = product
+        self.projectgroup = projectgroup
+        self.distribution = distribution
+        self.sourcepackagename = sourcepackagename
 
     @property
     def context(self):
@@ -299,13 +323,13 @@
 
     @property
     def is_expired(self):
-        now = datetime.now(pytz.timezone('UTC'))
+        now = datetime.now(pytz.UTC)
         expires = self.date_created + timedelta(hours=REQUEST_TOKEN_VALIDITY)
         return expires <= now
 
     def isSecretValid(self, secret):
-        """See `IOAuthConsumer`."""
-        return hashlib.sha256(secret).hexdigest() == self._secret
+        """See `IOAuthToken`."""
+        return unicode(hashlib.sha256(secret).hexdigest()) == self._secret
 
     def review(self, user, permission, context=None, date_expires=None):
         """See `IOAuthRequestToken`."""
@@ -316,7 +340,7 @@
             raise OAuthValidationError(
                 'This request token has expired and can no longer be '
                 'reviewed.')
-        self.date_reviewed = datetime.now(pytz.timezone('UTC'))
+        self.date_reviewed = datetime.now(pytz.UTC)
         self.date_expires = date_expires
         self.person = user
         self.permission = permission
@@ -354,6 +378,7 @@
             date_expires=self.date_expires, product=self.product,
             projectgroup=self.projectgroup, distribution=self.distribution,
             sourcepackagename=self.sourcepackagename)
+        OAuthAccessToken._getStore().add(access_token)
 
         # We want to notify the user that this oauth token has been generated
         # for them for security reasons.
@@ -361,7 +386,7 @@
             "OAuth token generated in Launchpad",
             "A new OAuth token consumer was enabled in Launchpad.")
 
-        self.destroySelf()
+        self._getStore().remove(self)
         return access_token, secret
 
     @property
@@ -376,7 +401,8 @@
 
     def getByKey(self, key):
         """See `IOAuthRequestTokenSet`."""
-        return OAuthRequestToken.selectOneBy(key=key)
+        return OAuthRequestToken._getStore().find(
+            OAuthRequestToken, OAuthRequestToken.key == key).one()
 
 
 def create_token_key_and_secret(table):

=== modified file 'lib/lp/services/oauth/stories/access-token.txt'
--- lib/lp/services/oauth/stories/access-token.txt	2014-09-01 03:41:31 +0000
+++ lib/lp/services/oauth/stories/access-token.txt	2016-01-26 15:52:17 +0000
@@ -12,13 +12,12 @@
     >>> from lp.registry.interfaces.product import IProductSet
     >>> from lp.testing import login, logout
     >>> login('salgado@xxxxxxxxxx')
-    >>> consumer = getUtility(IOAuthConsumerSet).getByKey('foobar123451432')
+    >>> consumer = getUtility(IOAuthConsumerSet).getByKey(u'foobar123451432')
     >>> token, secret = consumer.newRequestToken()
     >>> salgado = getUtility(IPersonSet).getByName('salgado')
     >>> token.review(salgado, OAuthPermission.WRITE_PUBLIC)
     >>> logout()
 
-    >>> import time
     >>> from urllib import urlencode
     >>> data = dict(
     ...     oauth_consumer_key='foobar123451432',

=== modified file 'lib/lp/services/oauth/stories/authorize-token.txt'
--- lib/lp/services/oauth/stories/authorize-token.txt	2014-06-13 14:22:09 +0000
+++ lib/lp/services/oauth/stories/authorize-token.txt	2016-01-26 15:52:17 +0000
@@ -17,7 +17,7 @@
     >>> from lp.services.oauth.interfaces import IOAuthConsumerSet
     >>> from lp.testing import ANONYMOUS, login, logout
     >>> login('salgado@xxxxxxxxxx')
-    >>> consumer = getUtility(IOAuthConsumerSet).getByKey('foobar123451432')
+    >>> consumer = getUtility(IOAuthConsumerSet).getByKey(u'foobar123451432')
     >>> logout()
     >>> token = request_token_for(consumer)
 
@@ -308,7 +308,7 @@
 
     >>> login('foo.bar@xxxxxxxxxxxxx')
     >>> consumer = factory.makeOAuthConsumer(
-    ...     "System-wide: Ubuntu (mycomputer)")
+    ...     u"System-wide: Ubuntu (mycomputer)")
     >>> logout()
 
     >>> token = request_token_for(consumer)

=== modified file 'lib/lp/services/oauth/stories/managing-tokens.txt'
--- lib/lp/services/oauth/stories/managing-tokens.txt	2014-06-13 15:07:12 +0000
+++ lib/lp/services/oauth/stories/managing-tokens.txt	2016-01-26 15:52:17 +0000
@@ -10,7 +10,7 @@
     # Create a desktop integration token.
     >>> login('salgado@xxxxxxxxxx')
     >>> consumer = factory.makeOAuthConsumer(
-    ...     "System-wide: Ubuntu (mycomputer)")
+    ...     u"System-wide: Ubuntu (mycomputer)")
     >>> salgado = getUtility(IPersonSet).getByName('salgado')
     >>> desktop_token, _ = factory.makeOAuthAccessToken(
     ...     consumer, salgado, OAuthPermission.DESKTOP_INTEGRATION)
@@ -18,7 +18,7 @@
     # Create a request token, authorize it for READ_PRIVATE access,
     # but don't exchange it for an access token.
     >>> consumer = factory.makeOAuthConsumer(
-    ...     "Example consumer for READ_PRIVATE")
+    ...     u"Example consumer for READ_PRIVATE")
     >>> request_token = factory.makeOAuthRequestToken()
     >>> request_token.review(salgado, OAuthPermission.READ_PRIVATE)
     >>> logout()
@@ -112,7 +112,7 @@
     >>> from lp.services.oauth.interfaces import IOAuthConsumerSet
     >>> login('salgado@xxxxxxxxxx')
     >>> token, _ = getUtility(IOAuthConsumerSet).getByKey(
-    ...     'launchpad-library').newRequestToken()
+    ...     u'launchpad-library').newRequestToken()
     >>> token.review(salgado, OAuthPermission.WRITE_PUBLIC,
     ...              context=getUtility(IProductSet)['firefox'])
     >>> access_token, _ = token.createAccessToken()

=== modified file 'lib/lp/services/oauth/stories/request-token.txt'
--- lib/lp/services/oauth/stories/request-token.txt	2014-06-27 12:51:13 +0000
+++ lib/lp/services/oauth/stories/request-token.txt	2016-01-26 15:52:17 +0000
@@ -3,7 +3,6 @@
 Our sample consumer (whose key is 'foobar123451432') asks Launchpad for
 a request token which may later be exchanged for an access token.
 
-    >>> import time
     >>> from urllib import urlencode
     >>> data = dict(
     ...     oauth_consumer_key='foobar123451432',
@@ -47,7 +46,7 @@
     >>> from lp.testing import login, logout
     >>> from lp.services.oauth.interfaces import IOAuthConsumerSet
     >>> login('salgado@xxxxxxxxxx')
-    >>> print getUtility(IOAuthConsumerSet).getByKey('joe-feed-reader')
+    >>> print getUtility(IOAuthConsumerSet).getByKey(u'joe-feed-reader')
     None
 
     >>> logout()
@@ -60,7 +59,7 @@
     oauth_token=...&oauth_token_secret=...
 
     >>> login('salgado@xxxxxxxxxx')
-    >>> getUtility(IOAuthConsumerSet).getByKey('joe-feed-reader')
+    >>> getUtility(IOAuthConsumerSet).getByKey(u'joe-feed-reader')
     <...OAuthConsumer...
     >>> logout()
 

=== modified file 'lib/lp/services/oauth/tests/test_oauth.py'
--- lib/lp/services/oauth/tests/test_oauth.py	2014-06-27 12:51:13 +0000
+++ lib/lp/services/oauth/tests/test_oauth.py	2016-01-26 15:52:17 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for the OAuth database classes."""
@@ -27,7 +27,7 @@
     """Base tests for the OAuth database classes."""
     layer = DatabaseFunctionalLayer
 
-    def test__get_store_should_return_the_main_master_store(self):
+    def test__getStore_should_return_the_main_master_store(self):
         """We want all OAuth classes to use the master store.
         Otherwise, the OAuth exchanges will fail because the authorize
         screen won't probably find the new request token on the slave store.
@@ -35,7 +35,7 @@
         zstorm = getUtility(IZStorm)
         self.assertEquals(
             '%s-%s' % (MAIN_STORE, MASTER_FLAVOR),
-            zstorm.get_name(self.class_._get_store()))
+            zstorm.get_name(self.class_._getStore()))
 
 
 class OAuthAccessTokenTestCase(BaseOAuthTestCase):

=== modified file 'lib/lp/services/oauth/tests/test_tokens.py'
--- lib/lp/services/oauth/tests/test_tokens.py	2015-10-14 15:22:01 +0000
+++ lib/lp/services/oauth/tests/test_tokens.py	2016-01-26 15:52:17 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """OAuth is a mechanism for allowing a user's desktop or a third-party
@@ -67,7 +67,7 @@
 
     def test_new(self):
         consumer = self.consumers.new(
-            self.factory.getUniqueString("oauthconsumerkey"))
+            self.factory.getUniqueUnicode(u"oauthconsumerkey"))
         verifyObject(IOAuthConsumer, consumer)
 
     def test_new_wont_create_duplicate_consumer(self):
@@ -80,8 +80,8 @@
 
     def test_getByKey_returns_none_for_nonexistent_consumer(self):
         # There is no consumer called "oauthconsumerkey-nonexistent".
-        nonexistent_key = self.factory.getUniqueString(
-            "oauthconsumerkey-nonexistent")
+        nonexistent_key = self.factory.getUniqueUnicode(
+            u"oauthconsumerkey-nonexistent")
         self.assertEqual(self.consumers.getByKey(nonexistent_key), None)
 
 
@@ -98,7 +98,7 @@
         self.assertEquals(token, self.tokens.getByKey(token.key))
 
     def test_getByKey_returns_none_for_unused_key(self):
-        self.assertEquals(None, self.tokens.getByKey("no-such-token"))
+        self.assertEquals(None, self.tokens.getByKey(u"no-such-token"))
 
 
 class TestRequestTokens(TestOAuth):
@@ -147,7 +147,7 @@
 
     def test_getRequestToken_for_nonexistent_key_returns_none(self):
         self.assertEquals(
-            None, self.consumer.getRequestToken("no-such-token"))
+            None, self.consumer.getRequestToken(u"no-such-token"))
 
     def test_isSecretValid(self):
         token, secret = self.consumer.newRequestToken()
@@ -362,7 +362,7 @@
         request_token.review(self.person, OAuthPermission.WRITE_PRIVATE)
         token, secret = request_token.createAccessToken()
         self.assertTrue(token.isSecretValid(secret))
-        self.assertFalse(token.isSecretValid(secret + 'a'))
+        self.assertFalse(token.isSecretValid(secret + u'a'))
 
     def test_get_access_tokens_for_person(self):
         """It's possible to get a person's access tokens."""

=== modified file 'lib/lp/services/webapp/authentication.py'
--- lib/lp/services/webapp/authentication.py	2015-07-08 16:05:11 +0000
+++ lib/lp/services/webapp/authentication.py	2016-01-26 15:52:17 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -274,9 +274,16 @@
     or it might be in the query string or entity-body.
 
     :return: a dictionary of authorization information.
+    :raises UnicodeDecodeError: If the Authorization header is not valid
+        UTF-8.
     """
     header = request._auth
     if header is not None and header.startswith("OAuth "):
+        # http://oauth.net/core/1.0/#encoding_parameters says "Text names
+        # and values MUST be encoded as UTF-8 octets before percent-encoding
+        # them", so we can reasonably fail if this hasn't been done.
+        if isinstance(header, bytes):
+            header = header.decode("UTF-8")
         return OAuthRequest._split_header(header)
     else:
         return request.form
@@ -288,7 +295,11 @@
     If the signature is incorrect or its method is not supported, set the
     appropriate status in the request's response and return False.
     """
-    authorization = get_oauth_authorization(request)
+    try:
+        authorization = get_oauth_authorization(request)
+    except UnicodeDecodeError:
+        request.response.setStatus(400)
+        return False
 
     if authorization.get('oauth_signature_method') != 'PLAINTEXT':
         # XXX: 2008-03-04, salgado: Only the PLAINTEXT method is supported

=== modified file 'lib/lp/services/webapp/doc/webapp-publication.txt'
--- lib/lp/services/webapp/doc/webapp-publication.txt	2015-11-16 03:18:08 +0000
+++ lib/lp/services/webapp/doc/webapp-publication.txt	2016-01-26 15:52:17 +0000
@@ -1067,13 +1067,13 @@
     >>> from lp.services.webapp.interfaces import OAuthPermission
     >>> getUtility(IStoreSelector).push(MasterDatabasePolicy())
     >>> salgado = getUtility(IPersonSet).getByName('salgado')
-    >>> consumer = getUtility(IOAuthConsumerSet).getByKey('foobar123451432')
+    >>> consumer = getUtility(IOAuthConsumerSet).getByKey(u'foobar123451432')
     >>> token, _ = consumer.newRequestToken()
     >>> firefox = getUtility(IProductSet)['firefox']
     >>> token.review(salgado, OAuthPermission.WRITE_PUBLIC, context=firefox)
     >>> access_token, access_secret = token.createAccessToken()
     >>> form = dict(
-    ...     oauth_consumer_key='foobar123451432',
+    ...     oauth_consumer_key=u'foobar123451432',
     ...     oauth_token=access_token.key,
     ...     oauth_version='1.0',
     ...     oauth_signature_method='PLAINTEXT',

=== modified file 'lib/lp/services/webapp/servers.py'
--- lib/lp/services/webapp/servers.py	2015-11-16 03:18:08 +0000
+++ lib/lp/services/webapp/servers.py	2016-01-26 15:52:17 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Definition of the internet servers that Launchpad uses."""
@@ -1255,7 +1255,10 @@
             return super(WebServicePublication, self).getPrincipal(request)
 
         # Fetch OAuth authorization information from the request.
-        form = get_oauth_authorization(request)
+        try:
+            form = get_oauth_authorization(request)
+        except UnicodeDecodeError:
+            raise TokenException('Invalid UTF-8.')
 
         consumer_key = form.get('oauth_consumer_key')
         consumers = getUtility(IOAuthConsumerSet)
@@ -1271,8 +1274,10 @@
             # header.
             anonymous_request = True
             consumer_key = request.getHeader('User-Agent', '')
-            if consumer_key == '':
-                consumer_key = 'anonymous client'
+            if isinstance(consumer_key, bytes):
+                consumer_key = consumer_key.decode('UTF-8')
+            if consumer_key == u'':
+                consumer_key = u'anonymous client'
             consumer = consumers.getByKey(consumer_key)
 
         if consumer is None:

=== modified file 'lib/lp/services/webapp/tests/test_publication.py'
--- lib/lp/services/webapp/tests/test_publication.py	2014-09-01 04:54:38 +0000
+++ lib/lp/services/webapp/tests/test_publication.py	2016-01-26 15:52:17 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests publication.py"""
@@ -101,7 +101,7 @@
         self.failIfEqual(person.id, person.account.id)
 
         # Create an access token for our new person.
-        consumer = getUtility(IOAuthConsumerSet).new('test-consumer')
+        consumer = getUtility(IOAuthConsumerSet).new(u'test-consumer')
         request_token, _ = consumer.newRequestToken()
         request_token.review(
             person, permission=OAuthPermission.READ_PUBLIC, context=None)
@@ -110,7 +110,7 @@
         # Use oauth.OAuthRequest just to generate a dictionary containing all
         # the parameters we need to use in a valid OAuth request, using the
         # access token we just created for our new person.
-        oauth_consumer = OAuthConsumer(consumer.key, '')
+        oauth_consumer = OAuthConsumer(consumer.key, u'')
         oauth_token = OAuthToken(access_token.key, access_secret)
         oauth_request = OAuthRequest.from_consumer_and_token(
             oauth_consumer, oauth_token)

=== modified file 'lib/lp/services/webservice/stories/launchpadlib.txt'
--- lib/lp/services/webservice/stories/launchpadlib.txt	2012-04-17 08:27:38 +0000
+++ lib/lp/services/webservice/stories/launchpadlib.txt	2016-01-26 15:52:17 +0000
@@ -10,7 +10,7 @@
 Launchpad object for a given user with a single call.
 
     >>> launchpad = launchpadlib_for(
-    ...     'launchpadlib test', 'salgado', 'WRITE_PUBLIC')
+    ...     u'launchpadlib test', 'salgado', 'WRITE_PUBLIC')
     >>> print launchpad.me.name
     salgado
 
@@ -24,8 +24,8 @@
     # yields a 401 error.
     #
     #>>> launchpad = launchpadlib_for(
-    #...     'launchpadlib test', 'no-priv', 'READ_PRIVATE', 'firefox',
-    #...      version="devel")
+    #...     u'launchpadlib test', 'no-priv', 'READ_PRIVATE', 'firefox',
+    #...     version="devel")
     #>>> print launchpad.projects['firefox'].name
     #firefox
 
@@ -34,7 +34,7 @@
 
     >>> from lp.testing import launchpadlib_credentials_for
     >>> credentials = launchpadlib_credentials_for(
-    ...     'launchpadlib test', 'no-priv', 'READ_PUBLIC')
+    ...     u'launchpadlib test', 'no-priv', 'READ_PUBLIC')
     >>> credentials
     <launchpadlib.credentials.Credentials object ...>
 
@@ -124,7 +124,7 @@
 temp directory.
 
     >>> launchpad = launchpadlib_for(
-    ...     'launchpadlib test', 'salgado', 'WRITE_PUBLIC')
+    ...     u'launchpadlib test', 'salgado', 'WRITE_PUBLIC')
 
     >>> launchpad._browser._connection.cache._cache_dir
     '/.../launchpadlib-cache-...'
@@ -132,7 +132,7 @@
 If we create another Launchpad object, it'll get its own cache directory.
 
     >>> launchpad_2 = launchpadlib_for(
-    ...     'launchpadlib test', 'salgado', 'WRITE_PUBLIC')
+    ...     u'launchpadlib test', 'salgado', 'WRITE_PUBLIC')
 
     >>> cache_dir_1 = launchpad._browser._connection.cache._cache_dir
     >>> cache_dir_2 = launchpad_2._browser._connection.cache._cache_dir

=== modified file 'lib/lp/services/webservice/stories/xx-service.txt'
--- lib/lp/services/webservice/stories/xx-service.txt	2015-10-01 10:25:19 +0000
+++ lib/lp/services/webservice/stories/xx-service.txt	2016-01-26 15:52:17 +0000
@@ -52,7 +52,7 @@
     ...     LaunchpadWebServiceCaller)
     >>> from lp.services.oauth.interfaces import IOAuthConsumerSet
 
-    >>> caller = LaunchpadWebServiceCaller('new-consumer', 'access-key')
+    >>> caller = LaunchpadWebServiceCaller(u'new-consumer', u'access-key')
     >>> response = caller.get(root)
     >>> print response.getheader('status')
     401 Unauthorized
@@ -67,11 +67,11 @@
     >>> login(ANONYMOUS)
     >>> from zope.component import getUtility
     >>> consumer_set = getUtility(IOAuthConsumerSet)
-    >>> print consumer_set.getByKey('another-new-consumer')
+    >>> print consumer_set.getByKey(u'another-new-consumer')
     None
     >>> logout()
 
-    >>> caller = LaunchpadWebServiceCaller('another-new-consumer', '')
+    >>> caller = LaunchpadWebServiceCaller(u'another-new-consumer', u'')
     >>> response = caller.get(root)
     >>> print response.getheader('status')
     200 Ok
@@ -79,7 +79,7 @@
 Launchpad automatically adds new consumer keys it sees to its database.
 
     >>> login(ANONYMOUS)
-    >>> print consumer_set.getByKey('another-new-consumer').key
+    >>> print consumer_set.getByKey(u'another-new-consumer').key
     another-new-consumer
     >>> logout()
 
@@ -105,7 +105,7 @@
 A completely unsigned web service request is treated as an anonymous
 request, with the OAuth consumer name being equal to the User-Agent.
 
-    >>> agent = "unsigned-user-agent"
+    >>> agent = u"unsigned-user-agent"
     >>> login(ANONYMOUS)
     >>> print consumer_set.getByKey(agent)
     None

=== modified file 'lib/lp/testing/_webservice.py'
--- lib/lp/testing/_webservice.py	2015-09-29 01:38:34 +0000
+++ lib/lp/testing/_webservice.py	2016-01-26 15:52:17 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -131,6 +131,10 @@
 
     :return: A launchpadlib Launchpad object.
     """
+    # XXX cjwatson 2016-01-22: Callers should be updated to pass Unicode
+    # directly, but that's a big change.
+    if isinstance(consumer_name, bytes):
+        consumer_name = unicode(consumer_name)
     if person is None:
         token = AnonymousAccessToken()
         credentials = Credentials(consumer_name, access_token=token)

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2016-01-12 01:24:06 +0000
+++ lib/lp/testing/factory.py	2016-01-26 15:52:17 +0000
@@ -456,8 +456,8 @@
         string = "%s-%s" % (prefix, self.getUniqueInteger())
         return string
 
-    def getUniqueUnicode(self):
-        return self.getUniqueString().decode('latin-1')
+    def getUniqueUnicode(self, prefix=None):
+        return self.getUniqueString(prefix=prefix).decode('latin-1')
 
     def getUniqueURL(self, scheme=None, host=None):
         """Return a URL unique to this run of the test case."""
@@ -4263,7 +4263,7 @@
             person = self.makePerson()
         from lp.testing.layers import BaseLayer
         launchpad = launchpadlib_for(
-            "test", person, service_root=BaseLayer.appserver_root_url("api"),
+            u"test", person, service_root=BaseLayer.appserver_root_url("api"),
             version=version)
         login_person(person)
         return launchpad
@@ -4295,9 +4295,9 @@
     # Factory methods for OAuth tokens.
     def makeOAuthConsumer(self, key=None, secret=None):
         if key is None:
-            key = self.getUniqueString("oauthconsumerkey")
+            key = self.getUniqueUnicode("oauthconsumerkey")
         if secret is None:
-            secret = ''
+            secret = u''
         return getUtility(IOAuthConsumerSet).new(key, secret)
 
     def makeOAuthRequestToken(self, consumer=None, date_created=None,

=== modified file 'lib/lp/testing/pages.py'
--- lib/lp/testing/pages.py	2015-10-26 14:54:43 +0000
+++ lib/lp/testing/pages.py	2016-01-26 15:52:17 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Testing infrastructure for page tests."""
@@ -75,9 +75,9 @@
     )
 
 SAMPLEDATA_ACCESS_SECRETS = {
-    'salgado-read-nonprivate': 'secret',
-    'salgado-change-anything': 'test',
-    'nopriv-read-nonprivate': 'mystery',
+    u'salgado-read-nonprivate': u'secret',
+    u'salgado-change-anything': u'test',
+    u'nopriv-read-nonprivate': u'mystery',
     }
 
 
@@ -136,10 +136,14 @@
         Other parameters are passed to the HTTPCaller used to make the calls.
         """
         if oauth_consumer_key is not None and oauth_access_key is not None:
-            self.consumer = OAuthConsumer(oauth_consumer_key, '')
+            # XXX cjwatson 2016-01-25: Callers should be updated to pass
+            # Unicode directly, but that's a big change.
+            if isinstance(oauth_consumer_key, bytes):
+                oauth_consumer_key = unicode(oauth_consumer_key)
+            self.consumer = OAuthConsumer(oauth_consumer_key, u'')
             if oauth_access_secret is None:
                 oauth_access_secret = SAMPLEDATA_ACCESS_SECRETS.get(
-                    oauth_access_key, '')
+                    oauth_access_key, u'')
             self.access_token = OAuthToken(
                 oauth_access_key, oauth_access_secret)
             # This shouldn't be here, but many old tests expect it.
@@ -691,7 +695,7 @@
     return str(canonical_url(*args, **kwargs))
 
 
-def webservice_for_person(person, consumer_key='launchpad-library',
+def webservice_for_person(person, consumer_key=u'launchpad-library',
                           permission=OAuthPermission.READ_PUBLIC,
                           context=None):
     """Return a valid LaunchpadWebServiceCaller for the person.


Follow ups