Merge lp:~stub/launchpad/oauth-db into lp:launchpad

[Stuart Bishop]

The OAuthNonce table is our most heavily updated table by an order of magnitude and responsible for the majority of our replication load. It generally sits at 40 updates per second and spikes at over two hundred for short periods.

We maintain the nonce seen history per the OAuth spec to help prevent replay attacks. There is no need for this information to be stored in a relational database at all - it is overkill. Memcache seems a better storage mechanism.

If memory pressure causes memcached to prematurely evict nonce history, users will not notice - what we lose is the replay protection. Monitoring is being put in place so we will see when evictions start happening and we can add RAM to our memcached instances or investigate alternative storages.

[lp:~stub/launchpad/oauth-db]

5590. By Stuart Bishop on 2010-06-08

Better expiry to avoid memcached pressure

5591. By Stuart Bishop on 2010-06-08

Betterer expiry

5592. By Stuart Bishop on 2010-06-08

Shorten keys

Unmerged revisions

5592. By Stuart Bishop on 2010-06-08

Shorten keys

5591. By Stuart Bishop on 2010-06-08

Betterer expiry

5590. By Stuart Bishop on 2010-06-08

Better expiry to avoid memcached pressure

5589. By Stuart Bishop on 2010-06-07

Drop OAuthNonce DB class, replacing its use with memcached

5588. By Stuart Bishop on 2010-06-07

Log OAuthNonces in memcached rather than PostgreSQL

5587. By Stuart Bishop on 2010-06-07

Merge from lp:~launchpad-pqm/launchpad/devel

=== modified file 'lib/canonical/launchpad/database/oauth.py'
--- lib/canonical/launchpad/database/oauth.py	2010-04-19 09:39:29 +0000
+++ lib/canonical/launchpad/database/oauth.py	2010-06-08 12:11:25 +0000
@@ -6,16 +6,16 @@
     'OAuthAccessToken',
     'OAuthConsumer',
     'OAuthConsumerSet',
-    'OAuthNonce',
     'OAuthRequestToken',
     'OAuthRequestTokenSet']
 
-import pytz
+import time
 from datetime import datetime, timedelta
 
 from zope.component import getUtility
 from zope.interface import implements
 
+from pytz import UTC
 from sqlobject import BoolCol, ForeignKey, StringCol
 from storm.expr import And
 
@@ -32,8 +32,9 @@
 from lp.registry.interfaces.projectgroup import IProjectGroup
 from lp.registry.interfaces.distributionsourcepackage import (
     IDistributionSourcePackage)
+from lp.services.memcache.interfaces import IMemcacheClient
 from canonical.launchpad.interfaces import (
-    IOAuthAccessToken, IOAuthConsumer, IOAuthConsumerSet, IOAuthNonce,
+    IOAuthAccessToken, IOAuthConsumer, IOAuthConsumerSet,
     IOAuthRequestToken, IOAuthRequestTokenSet, NonceAlreadyUsed,
     TimestampOrderingError, ClockSkew)
 from canonical.launchpad.webapp.interfaces import (
@@ -46,7 +47,7 @@
 # timestamp "MUST be equal or greater than the timestamp used in previous
 # requests," but this is likely to cause problems if the client does request
 # pipelining, so we use a time window (relative to the timestamp of the
-# existing OAuthNonce) to check if the timestamp can is acceptable. As
+# existing nonce) to check if the timestamp can is acceptable. As
 # suggested by Robert, we use a window which is at least twice the size of our
 # hard time out. This is a safe bet since no requests should take more than
 # one hard time out.
@@ -87,7 +88,7 @@
     def newRequestToken(self):
         """See `IOAuthConsumer`."""
         key, secret = create_token_key_and_secret(table=OAuthRequestToken)
-        date_expires = (datetime.now(pytz.timezone('UTC'))
+        date_expires = (datetime.now(UTC)
                         + timedelta(hours=REQUEST_TOKEN_VALIDITY))
         return OAuthRequestToken(
             consumer=self, key=key, secret=secret, date_expires=date_expires)
@@ -161,35 +162,52 @@
 
     def checkNonceAndTimestamp(self, nonce, timestamp):
         """See `IOAuthAccessToken`."""
-        timestamp = float(timestamp)
-        date = datetime.fromtimestamp(timestamp, pytz.UTC)
+        date = float(timestamp)
         # Determine if the timestamp is too far off from now.
-        skew = timedelta(seconds=TIMESTAMP_SKEW_WINDOW)
-        now = datetime.now(pytz.UTC)
+        skew = TIMESTAMP_SKEW_WINDOW
+        now = time.time()
         if date < (now-skew) or date > (now+skew):
             raise ClockSkew('Timestamp appears to come from bad system clock')
+
+        # Have we seen this nonce before with this token?
+        # Pull the nonce details, if they exists, from memcached.
+        cache = getUtility(IMemcacheClient)
+        nonce_seen_key = 'oauth:t_%s:n_%s' % (
+            self.key.encode('US-ASCII'), nonce.encode('US-ASCII'))
+        nonce_seen = cache.get(nonce_seen_key)
+
         # Determine if the nonce was already used for this timestamp.
-        store = OAuthNonce.getStore()
-        oauth_nonce = store.find(OAuthNonce,
-                                 And(OAuthNonce.access_token==self,
-                                     OAuthNonce.nonce==nonce,
-                                     OAuthNonce.request_timestamp==date)
-                                 ).one()
-        if oauth_nonce is not None:
+        if nonce_seen == date:
             raise NonceAlreadyUsed('This nonce has been used already.')
+
         # Determine if the timestamp is too old compared to most recent
         # request.
-        limit = date + timedelta(seconds=TIMESTAMP_ACCEPTANCE_WINDOW)
-        match = store.find(OAuthNonce,
-                           And(OAuthNonce.access_token==self,
-                               OAuthNonce.request_timestamp>limit)
-                           ).any()
-        if match is not None:
+        token_seen_key = 'oauth:t_%s:seen' % self.key.encode('US-ASCII')
+        token_seen = cache.get(token_seen_key)
+
+        limit = date + TIMESTAMP_ACCEPTANCE_WINDOW
+        if token_seen > limit:
             raise TimestampOrderingError(
                 'Timestamp too old compared to most recent request')
-        # Looks OK.  Give a Nonce object back.
-        return OAuthNonce(
-            access_token=self, nonce=nonce, request_timestamp=date)
+
+        # Looks OK.
+
+        # No need to remember things beyond
+        # TIMESTAMP_ACCEPTANCE_WINDOW * 2 seconds
+        # Where n==TIMESTAMP_ACCEPTANCE_WINDOW, n * 2 is the worst case.
+        # We will accept a timestamp claiming to be from n seconds in
+        # the future. If, in n*2 seconds time we receive the same
+        # timestamp, it would still be withing the acceptable skew
+        # and we need to generate an 'already seen' exception.
+        expires = TIMESTAMP_SKEW_WINDOW * 2 + TIMESTAMP_ACCEPTANCE_WINDOW
+
+        # Remember when we saw this nonce to avoid replay attacks.
+        cache.set(nonce_seen_key, date, expires)
+
+        # Remember when we last checked a nonce for this access
+        # token so we don't accept nonces in the past but still within
+        # acceptable skew.
+        cache.set(token_seen_key, max(date, token_seen), expires)
 
 
 class OAuthRequestToken(OAuthBase):
@@ -240,7 +258,7 @@
         """See `IOAuthRequestToken`."""
         assert not self.is_reviewed, (
             "Request tokens can be reviewed only once.")
-        self.date_reviewed = datetime.now(pytz.timezone('UTC'))
+        self.date_reviewed = datetime.now(UTC)
         self.person = user
         self.permission = permission
         if IProduct.providedBy(context):
@@ -286,16 +304,6 @@
         return OAuthRequestToken.selectOneBy(key=key)
 
 
-class OAuthNonce(OAuthBase):
-    """See `IOAuthNonce`."""
-    implements(IOAuthNonce)
-
-    access_token = ForeignKey(
-        dbName='access_token', foreignKey='OAuthAccessToken', notNull=True)
-    request_timestamp = UtcDateTimeCol(default=UTC_NOW, notNull=True)
-    nonce = StringCol(notNull=True)
-
-
 def create_token_key_and_secret(table):
     """Create a key and secret for an OAuth token.
 

=== modified file 'lib/canonical/launchpad/database/tests/test_oauth.py'
--- lib/canonical/launchpad/database/tests/test_oauth.py	2010-03-24 17:37:26 +0000
+++ lib/canonical/launchpad/database/tests/test_oauth.py	2010-06-08 12:11:25 +0000
@@ -13,7 +13,7 @@
 from zope.component import getUtility
 
 from canonical.launchpad.database.oauth import (
-    OAuthAccessToken, OAuthConsumer, OAuthNonce, OAuthRequestToken)
+    OAuthAccessToken, OAuthConsumer, OAuthRequestToken)
 from canonical.testing.layers import DatabaseFunctionalLayer
 from canonical.launchpad.webapp.interfaces import MAIN_STORE, MASTER_FLAVOR
 
@@ -45,14 +45,8 @@
     class_ = OAuthConsumer
 
 
-class OAuthNonceTestCase(BaseOAuthTestCase):
-    class_ = OAuthNonce
-
-
 def test_suite():
     return unittest.TestSuite((
         unittest.makeSuite(OAuthAccessTokenTestCase),
         unittest.makeSuite(OAuthRequestTokenTestCase),
-        unittest.makeSuite(OAuthNonceTestCase),
-        unittest.makeSuite(OAuthConsumerTestCase),
-            ))
+        unittest.makeSuite(OAuthConsumerTestCase)))

=== modified file 'lib/canonical/launchpad/doc/oauth.txt'
--- lib/canonical/launchpad/doc/oauth.txt	2010-04-16 15:06:55 +0000
+++ lib/canonical/launchpad/doc/oauth.txt	2010-06-08 12:11:25 +0000
@@ -15,7 +15,7 @@
     ...     AccessLevel, OAuthPermission)
     >>> from canonical.launchpad.interfaces import (
     ...     IOAuthAccessToken, IOAuthConsumer, IOAuthConsumerSet,
-    ...     IOAuthNonce, IOAuthRequestToken, IPersonSet)
+    ...     IOAuthRequestToken, IPersonSet)
     >>> consumer_set = getUtility(IOAuthConsumerSet)
     >>> verifyObject(IOAuthConsumerSet, consumer_set)
     True
@@ -307,26 +307,16 @@
 
     >>> import time
     >>> now = time.time() - 1
-    >>> nonce1 = access_token.checkNonceAndTimestamp('boo', now)
-    >>> verifyObject(IOAuthNonce, nonce1)
-    True
+    >>> access_token.checkNonceAndTimestamp('boo', now)
 
 - We can use an existing nonce with a new time.
 
     >>> now += 1
-    >>> nonce2 = access_token.checkNonceAndTimestamp('boo', now)
-    >>> IOAuthNonce.providedBy(nonce2)
-    True
-    >>> nonce1 is nonce2
-    False
+    >>> access_token.checkNonceAndTimestamp('boo', now)
 
 - We can use a new nonce with the same time.
 
-    >>> nonce3 = access_token.checkNonceAndTimestamp('surprise!', now)
-    >>> IOAuthNonce.providedBy(nonce3)
-    True
-    >>> nonce1 is nonce3 or nonce2 is nonce3
-    False
+    >>> access_token.checkNonceAndTimestamp('surprise!', now)
 
 - But we cannot use an existing nonce used for the same time.
 
@@ -368,12 +358,8 @@
     ...     TIMESTAMP_ACCEPTANCE_WINDOW)
     >>> TIMESTAMP_ACCEPTANCE_WINDOW
     60
-    >>> nonce4 = access_token.checkNonceAndTimestamp('boo', now-30)
-    >>> IOAuthNonce.providedBy(nonce4)
-    True
-    >>> nonce5 = access_token.checkNonceAndTimestamp('boo', now-60)
-    >>> IOAuthNonce.providedBy(nonce5)
-    True
+    >>> access_token.checkNonceAndTimestamp('boo', now-30)
+    >>> access_token.checkNonceAndTimestamp('boo', now-60)
 
 - Once outside of the window (defined by the *latest* timestamp, even if it
   is not the most recent), we get a TimestampOrderingError.
@@ -405,9 +391,7 @@
     ...     TIMESTAMP_SKEW_WINDOW)
     >>> TIMESTAMP_SKEW_WINDOW
     3600
-    >>> nonce6 = access_token.checkNonceAndTimestamp('boo', now + 55*60)
-    >>> IOAuthNonce.providedBy(nonce6)
-    True
+    >>> access_token.checkNonceAndTimestamp('boo', now + 55*60)
 
 - We cannot access it 65 minutes in the future.
 
@@ -419,9 +403,7 @@
 - It's also worth noting that now the TIMESTAMP_ACCEPTANCE_WINDOW is based
   off of the time 55 minutes in the future.
 
-    >>> nonce7 = access_token.checkNonceAndTimestamp('boo', now + 54*60 + 30)
-    >>> IOAuthNonce.providedBy(nonce7)
-    True
+    >>> access_token.checkNonceAndTimestamp('boo', now + 54*60 + 30)
     >>> access_token.checkNonceAndTimestamp('boo', now + 60)
     Traceback (most recent call last):
     ...

=== modified file 'lib/canonical/launchpad/interfaces/oauth.py'
--- lib/canonical/launchpad/interfaces/oauth.py	2010-04-19 14:47:49 +0000
+++ lib/canonical/launchpad/interfaces/oauth.py	2010-06-08 12:11:25 +0000
@@ -13,7 +13,6 @@
     'IOAuthAccessToken',
     'IOAuthConsumer',
     'IOAuthConsumerSet',
-    'IOAuthNonce',
     'IOAuthRequestToken',
     'IOAuthRequestTokenSet',
     'IOAuthSignedRequest',
@@ -174,7 +173,7 @@
             +/- `TIMESTAMP_SKEW_WINDOW` of now.
 
         If the nonce has never been used together with this token and
-        timestamp before, we store it in the database with the given timestamp
+        timestamp before, we store it with the given timestamp
         and associated with this token.
         """
 
@@ -236,20 +235,6 @@
         """
 
 
-class IOAuthNonce(Interface):
-    """The unique (nonce,timestamp) for requests using a given access token.
-
-    The nonce value (which is unique for all requests with that timestamp)
-    is generated by the consumer and included, together with the timestamp,
-    in each request made.  It's used to prevent replay attacks.
-    """
-
-    request_timestamp = Datetime(
-        title=_('Date issued'), required=True, readonly=True)
-    access_token = Object(schema=IOAuthAccessToken, title=_('The token'))
-    nonce = TextLine(title=_('Nonce'), required=True, readonly=True)
-
-
 class IOAuthSignedRequest(Interface):
     """Marker interface for a request signed with OAuth credentials."""
 

=== modified file 'lib/canonical/launchpad/scripts/garbo.py'
--- lib/canonical/launchpad/scripts/garbo.py	2010-05-27 16:49:09 +0000
+++ lib/canonical/launchpad/scripts/garbo.py	2010-06-08 12:11:25 +0000
@@ -23,7 +23,6 @@
 from canonical.launchpad.database.emailaddress import EmailAddress
 from lp.hardwaredb.model.hwdb import HWSubmission
 from canonical.launchpad.database.librarian import LibraryFileAlias
-from canonical.launchpad.database.oauth import OAuthNonce
 from canonical.launchpad.database.openidconsumer import OpenIDConsumerNonce
 from canonical.launchpad.interfaces import IMasterStore
 from canonical.launchpad.interfaces.emailaddress import EmailAddressStatus
@@ -51,42 +50,6 @@
 ONE_DAY_IN_SECONDS = 24*60*60
 
 
-class OAuthNoncePruner(TunableLoop):
-    """An ITunableLoop to prune old OAuthNonce records.
-
-    We remove all OAuthNonce records older than 1 day.
-    """
-    maximum_chunk_size = 6*60*60 # 6 hours in seconds.
-
-    def __init__(self, log, abort_time=None):
-        super(OAuthNoncePruner, self).__init__(log, abort_time)
-        self.store = IMasterStore(OAuthNonce)
-        self.oldest_age = self.store.execute("""
-            SELECT COALESCE(EXTRACT(EPOCH FROM
-                CURRENT_TIMESTAMP AT TIME ZONE 'UTC'
-                - MIN(request_timestamp)), 0)
-            FROM OAuthNonce
-            """).get_one()[0]
-
-    def isDone(self):
-        return self.oldest_age <= ONE_DAY_IN_SECONDS
-
-    def __call__(self, chunk_size):
-        self.oldest_age = max(
-            ONE_DAY_IN_SECONDS, self.oldest_age - chunk_size)
-
-        self.log.debug(
-            "Removed OAuthNonce rows older than %d seconds"
-            % self.oldest_age)
-
-        self.store.find(
-            OAuthNonce,
-            OAuthNonce.request_timestamp < SQL(
-                "CURRENT_TIMESTAMP AT TIME ZONE 'UTC' - interval '%d seconds'"
-                % self.oldest_age)).remove()
-        transaction.commit()
-
-
 class OpenIDConsumerNoncePruner(TunableLoop):
     """An ITunableLoop to prune old OpenIDConsumerNonce records.
 
@@ -739,7 +702,6 @@
 class HourlyDatabaseGarbageCollector(BaseDatabaseGarbageCollector):
     script_name = 'garbo-hourly'
     tunable_loops = [
-        OAuthNoncePruner,
         OpenIDConsumerNoncePruner,
         OpenIDConsumerAssociationPruner,
         RevisionCachePruner,

=== modified file 'lib/canonical/launchpad/scripts/tests/test_garbo.py'
--- lib/canonical/launchpad/scripts/tests/test_garbo.py	2010-05-26 01:48:12 +0000
+++ lib/canonical/launchpad/scripts/tests/test_garbo.py	2010-06-08 12:11:25 +0000
@@ -20,7 +20,6 @@
 from canonical.config import config
 from canonical.database.constants import THIRTY_DAYS_AGO, UTC_NOW
 from canonical.launchpad.database.message import Message
-from canonical.launchpad.database.oauth import OAuthNonce
 from canonical.launchpad.database.openidconsumer import OpenIDConsumerNonce
 from canonical.launchpad.interfaces import IMasterStore
 from canonical.launchpad.interfaces.emailaddress import EmailAddressStatus
@@ -90,46 +89,6 @@
         collector.main()
         return collector
 
-    def test_OAuthNoncePruner(self):
-        now = datetime.utcnow().replace(tzinfo=UTC)
-        timestamps = [
-            now - timedelta(days=2), # Garbage
-            now - timedelta(days=1) - timedelta(seconds=60), # Garbage
-            now - timedelta(days=1) + timedelta(seconds=60), # Not garbage
-            now, # Not garbage
-            ]
-        LaunchpadZopelessLayer.switchDbUser('testadmin')
-        store = IMasterStore(OAuthNonce)
-
-        # Make sure we start with 0 nonces.
-        self.failUnlessEqual(store.find(OAuthNonce).count(), 0)
-
-        for timestamp in timestamps:
-            OAuthNonce(
-                access_tokenID=1,
-                request_timestamp = timestamp,
-                nonce = str(timestamp))
-        transaction.commit()
-
-        # Make sure we have 4 nonces now.
-        self.failUnlessEqual(store.find(OAuthNonce).count(), 4)
-
-        self.runHourly(maximum_chunk_size=60) # 1 minute maximum chunk size
-
-        store = IMasterStore(OAuthNonce)
-
-        # Now back to two, having removed the two garbage entries.
-        self.failUnlessEqual(store.find(OAuthNonce).count(), 2)
-
-        # And none of them are older than a day.
-        # Hmm... why is it I'm putting tz aware datetimes in and getting
-        # naive datetimes back? Bug in the SQLObject compatibility layer?
-        # Test is still fine as we know the timezone.
-        self.failUnless(
-            store.find(
-                Min(OAuthNonce.request_timestamp)).one().replace(tzinfo=UTC)
-            >= now - timedelta(days=1))
-
     def test_OpenIDConsumerNoncePruner(self):
         now = int(time.mktime(time.gmtime()))
         MINUTES = 60

=== modified file 'lib/canonical/launchpad/zcml/oauth.zcml'
--- lib/canonical/launchpad/zcml/oauth.zcml	2009-07-13 18:15:02 +0000
+++ lib/canonical/launchpad/zcml/oauth.zcml	2010-06-08 12:11:25 +0000
@@ -44,8 +44,4 @@
           permission="launchpad.Edit"
           set_schema="canonical.launchpad.interfaces.IOAuthAccessToken"/>
   </class>
-
-  <class class="canonical.launchpad.database.OAuthNonce">
-      <allow interface="canonical.launchpad.interfaces.IOAuthNonce"/>
-  </class>
 </configure>