launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #17038
[Merge] lp:~wgrant/launchpad/oauth-NOnce into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/oauth-NOnce into lp:launchpad.
Commit message:
Stop checking OAuth nonces and timestamps. They provide no value with PLAINTEXT "signatures" over TLS.
Quoth RFC 5849: “The PLAINTEXT method [...] does not utilize the signature base string or the oauth_timestamp and oauth_nonce parameters.”
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #493925 in Launchpad itself: "OAuthNonce very high traffic"
https://bugs.launchpad.net/launchpad/+bug/493925
Bug #645640 in Launchpad itself: "API gives 401 errors on occasion"
https://bugs.launchpad.net/launchpad/+bug/645640
Bug #645651 in Launchpad itself: "pageids on nonce failures are a bit bogus"
https://bugs.launchpad.net/launchpad/+bug/645651
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/oauth-NOnce/+merge/224818
Launchpad's OAuth implementation is only accessible over TLS, and only supports PLAINTEXT signatures. When authenticating a request, it verifies that the token secret matches the database, that the timestamp is reasonably close to correct, and that the (token, timestamp, nonce) combination has not been seen before.
But nonce and timestamp validation aren't useful here. The PLAINTEXT signature method relies totally on transport security, as anyone able to see the "signature" can just change the timestamp and nonce anyway. In our implementation, all the timestamp and nonce validation provides is protection against TLS replays, and we're in serious trouble for more important reasons if we find ourselves in a scenario where those are a significant risk.
I've always thought it weird that nonces were used, and that the OAuth 1.0a spec (http://oauth.net/core/1.0a/#anchor21) was somewhat remiss in this regard. But since I came to that conclusion, OAuth 1.0a was standardised as RFC 5849, with the omission corrected. From http://tools.ietf.org/html/rfc5849#section-3.4.4:
"""
The "PLAINTEXT" method does not employ a signature algorithm. It
MUST be used with a transport-layer mechanism such as TLS or SSL (or
sent over a secure channel with equivalent protections). It does not
utilize the signature base string or the "oauth_timestamp" and
"oauth_nonce" parameters.
"""
So we should stop checking timestamps and nonces. This has the benefit of enabling us to do read-only authenticated API requests, and eventually serve API requests from slave databases and during master outages. And we get to drop the high-write OAuthNonce table.
Apart from the DB bits, I've retained just one artifact: WebServicePublication still commits at the end of GET requests, as I'm not 100% sure yet that no API methods rely on that behaviour. I intend to do a separate branch which places that commit under a feature flag, so we can test it on production and quickly revert if it breaks anything.
--
https://code.launchpad.net/~wgrant/launchpad/oauth-NOnce/+merge/224818
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/oauth-NOnce into lp:launchpad.
=== modified file 'lib/lp/scripts/garbo.py'
--- lib/lp/scripts/garbo.py 2014-04-30 13:17:23 +0000
+++ lib/lp/scripts/garbo.py 2014-06-27 13:35:05 +0000
@@ -103,7 +103,6 @@
from lp.services.librarian.model import TimeLimitedToken
from lp.services.log.logger import PrefixFilter
from lp.services.looptuner import TunableLoop
-from lp.services.oauth.model import OAuthNonce
from lp.services.openid.model.openidconsumer import OpenIDConsumerNonce
from lp.services.propertycache import cachedproperty
from lp.services.salesforce.interfaces import (
@@ -359,23 +358,6 @@
"""
-class OAuthNoncePruner(BulkPruner):
- """An ITunableLoop to prune old OAuthNonce records.
-
- We remove all OAuthNonce records older than 1 day.
- """
- target_table_key = 'access_token, request_timestamp, nonce'
- target_table_key_type = (
- 'access_token integer, request_timestamp timestamp without time zone,'
- ' nonce text')
- target_table_class = OAuthNonce
- ids_to_prune_query = """
- SELECT access_token, request_timestamp, nonce FROM OAuthNonce
- WHERE request_timestamp
- < CURRENT_TIMESTAMP AT TIME ZONE 'UTC' - CAST('1 day' AS interval)
- """
-
-
class PreviewDiffPruner(BulkPruner):
"""A BulkPruner to remove old PreviewDiffs.
@@ -1613,7 +1595,6 @@
script_name = 'garbo-frequently'
tunable_loops = [
BugSummaryJournalRollup,
- OAuthNoncePruner,
OpenIDConsumerNoncePruner,
OpenIDConsumerAssociationPruner,
AntiqueSessionPruner,
=== modified file 'lib/lp/scripts/tests/test_garbo.py'
--- lib/lp/scripts/tests/test_garbo.py 2014-04-30 13:17:39 +0000
+++ lib/lp/scripts/tests/test_garbo.py 2014-06-27 13:35:05 +0000
@@ -95,10 +95,6 @@
from lp.services.librarian.model import TimeLimitedToken
from lp.services.log.logger import NullHandler
from lp.services.messages.model.message import Message
-from lp.services.oauth.model import (
- OAuthAccessToken,
- OAuthNonce,
- )
from lp.services.openid.model.openidconsumer import OpenIDConsumerNonce
from lp.services.salesforce.interfaces import ISalesforceVoucherProxy
from lp.services.salesforce.tests.proxy import TestSalesforceVoucherProxy
@@ -440,47 +436,6 @@
data = load_garbo_job_state('job')
self.assertEqual({'data': 2}, data)
- def test_OAuthNoncePruner(self):
- now = datetime.now(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
- ]
- switch_dbuser('testadmin')
- store = IMasterStore(OAuthNonce)
-
- # Make sure we start with 0 nonces.
- self.failUnlessEqual(store.find(OAuthNonce).count(), 0)
-
- for timestamp in timestamps:
- store.add(OAuthNonce(
- access_token=OAuthAccessToken.get(1),
- request_timestamp=timestamp,
- nonce=str(timestamp)))
- transaction.commit()
-
- # Make sure we have 4 nonces now.
- self.failUnlessEqual(store.find(OAuthNonce).count(), 4)
-
- self.runFrequently(
- 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/lp/services/oauth/configure.zcml'
--- lib/lp/services/oauth/configure.zcml 2011-12-08 05:48:32 +0000
+++ lib/lp/services/oauth/configure.zcml 2014-06-27 13:35:05 +0000
@@ -47,7 +47,4 @@
set_schema="lp.services.oauth.interfaces.IOAuthAccessToken"/>
</class>
- <class class="lp.services.oauth.model.OAuthNonce">
- <allow interface="lp.services.oauth.interfaces.IOAuthNonce"/>
- </class>
</configure>
=== removed file 'lib/lp/services/oauth/doc/oauth.txt'
--- lib/lp/services/oauth/doc/oauth.txt 2014-01-30 15:04:06 +0000
+++ lib/lp/services/oauth/doc/oauth.txt 1970-01-01 00:00:00 +0000
@@ -1,151 +0,0 @@
-= OAuth =
-
-Most of the OAuth doctests have been converted into unit tests and
-moved to test_oauth_tokens.py
-
-== Nonces and timestamps ==
-
-A nonce is a random string, generated by the client for each request.
-
-A timestamp is also generated by the client, and indicates when the request
-was generated, according to the client's clock, expressed in the number of
-seconds since January 1, 1970 00:00:00 GMT.
-
-A nonce must be unique for the combination of a client and a timestamp.
-Launchpad implements this constraint. If a nonce is not unique for a client
-and a timestamp, the code will raise a NonceAlreadyUsed exception.
-
-Here's a quick demonstration of the behaviour.
-
- >>> login('foo.bar@xxxxxxxxxxxxx')
- >>> access_token = factory.makeOAuthAccessToken()
- >>> logout()
-
-- We can use a nonce.
-
- >>> from lp.services.oauth.interfaces import IOAuthNonce
- >>> import time
- >>> now = time.time() - 1
- >>> nonce1 = access_token.checkNonceAndTimestamp('boo', now)
- >>> verifyObject(IOAuthNonce, nonce1)
- True
-
-- 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
-
-- 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
-
-- But we cannot use an existing nonce used for the same time.
-
- >>> access_token.checkNonceAndTimestamp('boo', now)
- Traceback (most recent call last):
- ...
- NonceAlreadyUsed: ...
-
-According to the oauth specification <http://oauth.net/core/1.0/#nonce>, for a
-given client, an application should not accept a timestamp older than the most
-recent timestamp received.
-
-For instance, if a client sends a timestamp of 2009-02-24T13:43:08Z, a
-subsequent request for any earlier time (even one second earlier,
-2009-02-24T13:43:07Z) should generate an error. A request with the same
-timestamp or later is acceptable.
-
-These limitations of the nonce and the timestamp are intended to reduce the
-opportunity for replay attacks, particularly within the context of an insecure
-channel such as HTTP.
-
-Launchpad relaxes the constraint on the timestamp. Rather than accepting any
-request within the same second, Launchpad accepts any request within the past
-60 seconds (or, as defined by the code, within the past number of seconds
-specified by lp.services.oauth.model.TIMESTAMP_ACCEPTANCE_WINDOW).
-
-For instance, in Launchpad, if a client sends a timestamp of
-2009-02-24T13:43:08Z, a subsequent request of any time up to 60 seconds prior
-will be accepted. That means that the next request can specify
-2009-02-24T13:42:08Z or later. Otherwise the code will raise a
-TimestampOrderingError.
-
-Here's a quick demonstration of the behaviour.
-
-- Within the TIMESTAMP_ACCEPTANCE_WINDOW, we can create a nonce with an
- earlier time.
-
- >>> from lp.services.oauth.model import (
- ... 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
-
-- Once outside of the window (defined by the *latest* timestamp, even if it
- is not the most recent), we get a TimestampOrderingError.
-
- >>> access_token.checkNonceAndTimestamp('boo', now-61)
- Traceback (most recent call last):
- ...
- TimestampOrderingError: ...
-
-This change is intended to support pipelining requests, and to acknowledge
-that network latency, multiple concurrent clients with the same credentials,
-and other situations can cause timestamps to get out of order (see
-http://groups.google.com/group/oauth/msg/387fdafcf0be322a and LP bug 319710
-for more background).
-
-The code also raises a ClockSkew error if the timestamp is more than
-TIMESTAMP_SKEW_WINDOW seconds away from the server's clock (ahead or
-behind). This number of seconds is currently equal to one hour (60*60). The
-primary reason for this behaviour is so that a clock skew too far in the future
-does not essentially make the authentication tokens useless once the client's
-clock is corrected. In addition, it gives us a constraint within which we can
-reason about clock-related errors reported by customers.
-
-Here's a quick demonstration of the behaviour.
-
-- We can access the system with a timestamp 55 minutes in the future.
-
- >>> from lp.services.oauth.model import (
- ... TIMESTAMP_SKEW_WINDOW)
- >>> TIMESTAMP_SKEW_WINDOW
- 3600
- >>> nonce6 = access_token.checkNonceAndTimestamp('boo', now + 55*60)
- >>> IOAuthNonce.providedBy(nonce6)
- True
-
-- We cannot access it 65 minutes in the future.
-
- >>> access_token.checkNonceAndTimestamp('boo', now + 65*60)
- Traceback (most recent call last):
- ...
- ClockSkew: ...
-
-- 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 + 60)
- Traceback (most recent call last):
- ...
- TimestampOrderingError: ...
- >>> access_token.checkNonceAndTimestamp('boo', now + 53*60)
- Traceback (most recent call last):
- ...
- TimestampOrderingError: ...
=== modified file 'lib/lp/services/oauth/interfaces.py'
--- lib/lp/services/oauth/interfaces.py 2013-01-07 02:40:55 +0000
+++ lib/lp/services/oauth/interfaces.py 2014-06-27 13:35:05 +0000
@@ -11,13 +11,9 @@
'IOAuthAccessToken',
'IOAuthConsumer',
'IOAuthConsumerSet',
- 'IOAuthNonce',
'IOAuthRequestToken',
'IOAuthRequestTokenSet',
'IOAuthSignedRequest',
- 'NonceAlreadyUsed',
- 'TimestampOrderingError',
- 'ClockSkew',
'TokenException',
]
@@ -194,27 +190,6 @@
description=_('From this date onwards this token can not be used '
'by the consumer to access protected resources.'))
- def checkNonceAndTimestamp(nonce, timestamp):
- """Verify the nonce and timestamp.
-
- - Ensure the nonce hasn't been used with the same timestamp.
- - Ensure this is a first access, or this timestamp is no older than
- last timestamp minus `TIMESTAMP_ACCEPTANCE_WINDOW`.
- - Ensure this timestamp is within +/- `TIMESTAMP_SKEW_WINDOW` of the
- server's concept of now.
-
- :raises NonceAlreadyUsed: If the nonce has been used before with the
- same timestamp.
- :raises TimestampOrderingError: If the timestamp is older than the
- last timestamp minus `TIMESTAMP_ACCEPTANCE_WINDOW`.
- :raises ClockSkew: If the timestamp is not within
- +/- `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
- and associated with this token.
- """
-
class IOAuthRequestToken(IOAuthToken):
"""A token used by a consumer to ask the user to authenticate on LP.
@@ -284,20 +259,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."""
@@ -308,19 +269,7 @@
@error_status(httplib.UNAUTHORIZED)
class _TokenException(Exception):
- """Base class for token, nonce, and timestamp exceptions."""
-
-
-class NonceAlreadyUsed(_TokenException):
- """Nonce has been used together with same token but another timestamp."""
-
-
-class TimestampOrderingError(_TokenException):
- """Timestamp is too old, compared to the last request."""
-
-
-class ClockSkew(_TokenException):
- """Timestamp is too far off from server's clock."""
+ """Base class for token exceptions."""
class TokenException(_TokenException):
=== modified file 'lib/lp/services/oauth/model.py'
--- lib/lp/services/oauth/model.py 2013-06-20 05:50:00 +0000
+++ lib/lp/services/oauth/model.py 2014-06-27 13:35:05 +0000
@@ -6,7 +6,6 @@
'OAuthAccessToken',
'OAuthConsumer',
'OAuthConsumerSet',
- 'OAuthNonce',
'OAuthRequestToken',
'OAuthRequestTokenSet',
'OAuthValidationError',
@@ -24,11 +23,6 @@
ForeignKey,
StringCol,
)
-from storm.expr import And
-from storm.locals import (
- Int,
- Reference,
- )
from zope.interface import implements
from lp.registry.interfaces.distribution import IDistribution
@@ -42,18 +36,13 @@
from lp.services.database.enumcol import EnumCol
from lp.services.database.interfaces import IMasterStore
from lp.services.database.sqlbase import SQLBase
-from lp.services.database.stormbase import StormBase
from lp.services.librarian.model import LibraryFileAlias
from lp.services.oauth.interfaces import (
- ClockSkew,
IOAuthAccessToken,
IOAuthConsumer,
IOAuthConsumerSet,
- IOAuthNonce,
IOAuthRequestToken,
IOAuthRequestTokenSet,
- NonceAlreadyUsed,
- TimestampOrderingError,
)
from lp.services.tokens import (
create_token,
@@ -66,22 +55,6 @@
# How many hours should a request token be valid for?
REQUEST_TOKEN_VALIDITY = 2
-# The OAuth Core 1.0 spec (http://oauth.net/core/1.0/#nonce) says that a
-# 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
-# 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.
-TIMESTAMP_ACCEPTANCE_WINDOW = 60 # seconds
-# If the timestamp is far in the future because of a client's clock skew,
-# it will effectively invalidate the authentication tokens when the clock is
-# corrected. To prevent that from becoming too serious a problem, we raise an
-# exception if the timestamp is off by more than this amount from the server's
-# concept of "now". We also reject timestamps that are too old by the same
-# amount.
-TIMESTAMP_SKEW_WINDOW = 60 * 60 # seconds, +/-
class OAuthValidationError(Exception):
@@ -249,40 +222,6 @@
now = datetime.now(pytz.timezone('UTC'))
return self.date_expires is not None and self.date_expires <= now
- def checkNonceAndTimestamp(self, nonce, timestamp):
- """See `IOAuthAccessToken`."""
- timestamp = float(timestamp)
- date = datetime.fromtimestamp(timestamp, pytz.UTC)
- # Determine if the timestamp is too far off from now.
- skew = timedelta(seconds=TIMESTAMP_SKEW_WINDOW)
- now = datetime.now(pytz.UTC)
- if date < (now - skew) or date > (now + skew):
- raise ClockSkew('Timestamp appears to come from bad system clock')
- # 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:
- 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:
- raise TimestampOrderingError(
- 'Timestamp too old compared to most recent request')
- # Looks OK. Give a Nonce object back.
- nonce = OAuthNonce(
- access_token=self, nonce=nonce, request_timestamp=date)
- store.add(nonce)
- return nonce
-
class OAuthRequestToken(OAuthBase, SQLBase):
"""See `IOAuthRequestToken`."""
@@ -406,25 +345,6 @@
return OAuthRequestToken.selectOneBy(key=key)
-class OAuthNonce(OAuthBase, StormBase):
- """See `IOAuthNonce`."""
- implements(IOAuthNonce)
-
- __storm_table__ = 'OAuthNonce'
- __storm_primary__ = 'access_token_id', 'request_timestamp', 'nonce'
-
- access_token_id = Int(name='access_token')
- access_token = Reference(access_token_id, OAuthAccessToken.id)
- request_timestamp = UtcDateTimeCol(default=UTC_NOW, notNull=True)
- nonce = StringCol(notNull=True)
-
- def __init__(self, access_token, request_timestamp, nonce):
- super(OAuthNonce, self).__init__()
- self.access_token = access_token
- self.request_timestamp = request_timestamp
- self.nonce = nonce
-
-
def create_token_key_and_secret(table):
"""Create a key and secret for an OAuth token.
=== modified file 'lib/lp/services/oauth/stories/access-token.txt'
--- lib/lp/services/oauth/stories/access-token.txt 2011-12-24 17:49:30 +0000
+++ lib/lp/services/oauth/stories/access-token.txt 2014-06-27 13:35:05 +0000
@@ -25,9 +25,7 @@
... oauth_version='1.0',
... oauth_token=token.key,
... oauth_signature_method='PLAINTEXT',
- ... oauth_signature='&'.join([consumer.secret, token.secret]),
- ... oauth_timestamp=time.time(),
- ... oauth_nonce='4572616e48616d6d65724c61686176')
+ ... oauth_signature='&'.join([consumer.secret, token.secret]))
>>> anon_browser.open(
... 'http://launchpad.dev/+access-token', data=urlencode(data))
=== modified file 'lib/lp/services/oauth/stories/request-token.txt'
--- lib/lp/services/oauth/stories/request-token.txt 2012-03-07 07:15:00 +0000
+++ lib/lp/services/oauth/stories/request-token.txt 2014-06-27 13:35:05 +0000
@@ -9,9 +9,7 @@
... oauth_consumer_key='foobar123451432',
... oauth_version='1.0',
... oauth_signature_method='PLAINTEXT',
- ... oauth_signature='&',
- ... oauth_timestamp=time.time(),
- ... oauth_nonce='4572616e48616d6d65724c61686176')
+ ... oauth_signature='&')
>>> anon_browser.open(
... 'http://launchpad.dev/+request-token', data=urlencode(data))
=== modified file 'lib/lp/services/oauth/tests/test_oauth.py'
--- lib/lp/services/oauth/tests/test_oauth.py 2012-09-28 06:25:44 +0000
+++ lib/lp/services/oauth/tests/test_oauth.py 2014-06-27 13:35:05 +0000
@@ -18,7 +18,6 @@
from lp.services.oauth.model import (
OAuthAccessToken,
OAuthConsumer,
- OAuthNonce,
OAuthRequestToken,
)
from lp.testing.layers import DatabaseFunctionalLayer
@@ -51,14 +50,9 @@
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),
))
=== modified file 'lib/lp/services/webapp/doc/webapp-publication.txt'
--- lib/lp/services/webapp/doc/webapp-publication.txt 2014-02-27 08:39:44 +0000
+++ lib/lp/services/webapp/doc/webapp-publication.txt 2014-06-27 13:35:05 +0000
@@ -900,62 +900,6 @@
Darwin
-GET requests on api.launchpad.net
-.................................
-
-In the case of WebServicePublication, though, we have to commit the
-transaction after GET requests in order to persist new entries added to
-the OAuthNonce table.
-
- >>> import time
- >>> from lp.services.oauth.interfaces import IOAuthConsumerSet
- >>> from lp.services.webapp.interfaces import OAuthPermission
- >>> from lp.registry.interfaces.person import IPersonSet
-
- >>> from zope.component import getUtility
- >>> from lp.services.database.policy import MasterDatabasePolicy
- >>> from lp.services.database.interfaces import IStoreSelector
- >>> getUtility(IStoreSelector).push(MasterDatabasePolicy())
-
- >>> salgado = getUtility(IPersonSet).getByName('salgado')
- >>> consumer = getUtility(IOAuthConsumerSet).getByKey('foobar123451432')
- >>> token = consumer.newRequestToken()
- >>> token.review(salgado, OAuthPermission.WRITE_PUBLIC)
- >>> access_token = token.createAccessToken()
- >>> policy = getUtility(IStoreSelector).pop()
- >>> transaction.commit()
- >>> form = dict(
- ... oauth_consumer_key='foobar123451432',
- ... oauth_token=access_token.key,
- ... oauth_version='1.0',
- ... oauth_signature_method='PLAINTEXT',
- ... oauth_signature="&".join([consumer.secret, access_token.secret]),
- ... oauth_timestamp=time.time(),
- ... oauth_nonce='1234572616e48616d6d65724c61686176')
- >>> test_request, publication = get_request_and_publication(
- ... 'api.launchpad.dev', 'GET',
- ... extra_environment=dict(QUERY_STRING=urlencode(form)))
- >>> test_request.processInputs()
- >>> print publication.getPrincipal(test_request).title
- Guilherme Salgado
-
- # Our afterCall() implementation expects to find a
- # _publicationticks_start in its request, which is set by
- # callObject(). Since we don't want to callObject() here, we'll
- # have to change the request manually.
- >>> test_request._publicationticks_start = 1345
- >>> publication.afterCall(test_request, None)
-
- >>> test_request, publication = get_request_and_publication(
- ... 'api.launchpad.dev', 'GET',
- ... extra_environment=dict(QUERY_STRING=urlencode(form)))
- >>> test_request.processInputs()
- >>> publication.getPrincipal(test_request)
- Traceback (most recent call last):
- ...
- NonceAlreadyUsed: This nonce has been used already.
-
-
Doomed transactions are aborted
-------------------------------
@@ -1103,7 +1047,12 @@
principal's access_level and scope will match what was specified in the
token.
+ >>> import time
>>> from lp.registry.interfaces.product import IProductSet
+ >>> from lp.services.database.policy import MasterDatabasePolicy
+ >>> from lp.services.database.interfaces import IStoreSelector
+ >>> from lp.services.oauth.interfaces import IOAuthConsumerSet
+ >>> from lp.services.webapp.interfaces import OAuthPermission
>>> getUtility(IStoreSelector).push(MasterDatabasePolicy())
>>> salgado = getUtility(IPersonSet).getByName('salgado')
>>> consumer = getUtility(IOAuthConsumerSet).getByKey('foobar123451432')
@@ -1180,52 +1129,6 @@
...
TokenException: Invalid signature.
-The nonce specified in the request must not have been used before in a request
-with this same token and timestamp.
-
- >>> form2 = form.copy()
- >>> form2['oauth_nonce'] = '6e48616d6d65724c61686176457261'
- >>> test_request = LaunchpadTestRequest(form=form2)
- >>> print publication.getPrincipal(test_request).title
- Guilherme Salgado
-
- >>> test_request = LaunchpadTestRequest(form=form2)
- >>> publication.getPrincipal(test_request)
- Traceback (most recent call last):
- ...
- NonceAlreadyUsed: This nonce has been used already.
-
-The timestamp must not be older than TIMESTAMP_ACCEPTANCE_WINDOW from the most
-recent request for this token.
-
- >>> from lp.services.oauth.model import (
- ... TIMESTAMP_ACCEPTANCE_WINDOW)
- >>> form2 = form.copy()
- >>> form2['oauth_timestamp'] -= (TIMESTAMP_ACCEPTANCE_WINDOW-5)
- >>> test_request = LaunchpadTestRequest(form=form2)
- >>> print publication.getPrincipal(test_request).title
- Guilherme Salgado
-
- >>> form2['oauth_timestamp'] -= 10
- >>> test_request = LaunchpadTestRequest(form=form2)
- >>> publication.getPrincipal(test_request)
- Traceback (most recent call last):
- ...
- TimestampOrderingError: Timestamp too old compared ...
-
-Last but not least, the timestamp must not be too far in the future, as
-defined by TIMESTAMP_SKEW_WINDOW.
-
- >>> from lp.services.oauth.model import (
- ... TIMESTAMP_SKEW_WINDOW)
- >>> form2 = form.copy()
- >>> form2['oauth_timestamp'] += (TIMESTAMP_SKEW_WINDOW+10)
- >>> test_request = LaunchpadTestRequest(form=form2)
- >>> publication.getPrincipal(test_request)
- Traceback (most recent call last):
- ...
- ClockSkew: Timestamp ... from bad system clock
-
Close the bogus request that was started by the call to
beforeTraversal, in order to ensure we leave our state sane.
Also, pop all the database policies we have been accumulating.
=== modified file 'lib/lp/services/webapp/servers.py'
--- lib/lp/services/webapp/servers.py 2014-05-08 06:57:35 +0000
+++ lib/lp/services/webapp/servers.py 2014-06-27 13:35:05 +0000
@@ -1203,7 +1203,12 @@
return super(WebServicePublication, self).getResource(request, ob)
def finishReadOnlyRequest(self, request, ob, txn):
- """Commit the transaction so that created OAuthNonces are stored."""
+ """Commit the transaction even though there should be no writes."""
+ # WebServicePublication used to commit on every request to store
+ # OAuthNonces to prevent replay attacks. But TLS prevents replay
+ # attacks too, so we don't bother with nonces any more. However,
+ # this commit will stay here until we can switch it off in a
+ # controlled test to ensure that nothing depends on it on prod.
notify(FinishReadOnlyRequestEvent(ob, request))
# Transaction commits usually need to be aware of the possibility of
# a doomed transaction. We do not expect that this code will
@@ -1217,10 +1222,8 @@
Web service requests are authenticated using OAuth, except for the
one made using (presumably) JavaScript on the /api override path.
- Raises a variety of token errors (ClockSkew, NonceAlreadyUsed,
- TimestampOrderingError, TokenException) which have a webservice error
- status of Unauthorized - 401. All of these exceptions represent
- errors on the part of the client.
+ Raises TokenException which has a webservice error status of
+ Unauthorized - 401.
Raises Unauthorized directly in the case where the consumer is None
for a non-anonymous request as it may represent a server error.
@@ -1287,9 +1290,6 @@
token = consumer.getAccessToken(token_key)
if token is None:
raise TokenException('Unknown access token (%s).' % token_key)
- nonce = form.get('oauth_nonce')
- timestamp = form.get('oauth_timestamp')
- token.checkNonceAndTimestamp(nonce, timestamp)
if token.permission == OAuthPermission.UNAUTHORIZED:
raise TokenException('Unauthorized token (%s).' % token.key)
elif token.is_expired:
Follow ups
