Merge lp:~xnox/launchpad/devel into lp:launchpad

Proposed by Dimitri John Ledkov
Status: Work in progress
Proposed branch: lp:~xnox/launchpad/devel
Merge into: lp:launchpad
Diff against target: 96 lines (+68/-2)
3 files modified
lib/lp/services/oauth/browser/__init__.py (+1/-1)
lib/lp/services/oauth/doc/oauth.txt (+1/-1)
lib/lp/services/oauth/stories/access-token-header.txt (+66/-0)
To merge this branch: bzr merge lp:~xnox/launchpad/devel
Reviewer Review Type Date Requested Status
William Grant code Needs Fixing
Barry Warsaw Pending
Review via email: mp+217079@code.launchpad.net
To post a comment you must log in.
Revision history for this message
William Grant (wgrant) wrote :

This seems to remove the test that verifies the behaviour that every existing Launchpad API client relies on. Why do you want to change the behaviour in the first place?

review: Needs Fixing (code)
Revision history for this message
Dimitri John Ledkov (xnox) wrote :

> This seems to remove the test that verifies the behaviour that every existing
> Launchpad API client relies on. Why do you want to change the behaviour in the
> first place?

Right, i'll readd it back again. At the moment it exposes a bug though, since header-signature type doesn't appear to invalidate request token, thus one can exchange for access token unlimited amount of times. Only body-signature type seems to correctly raise 401 upon subsequent requests.

So at the moment the test change exposes buggy behaviour with header-signature. I'll make sure to re-introduce body-signature tests and test header-signatures separately.

----------------------------------------------------------------------
File "lib/lp/services/oauth/stories/access-token.txt", line 42, in access-token.txt
Failed example:
    auth_browser.open(
        'http://launchpad.dev/+access-token')
Differences (ndiff with -expected +actual):
    - Traceback (most recent call last):
    - ...
    - HTTPError: HTTP Error 401: Unauthorized

Revision history for this message
Dimitri John Ledkov (xnox) wrote :

Settings status to "work in progress" appropriately.

lp:~xnox/launchpad/devel updated
16989. By Dimitri John Ledkov

Support oauth_callback from header authentication

16990. By Dimitri John Ledkov

OAuth Revert access-token.txt to current, add header-signature story.

16991. By Dimitri John Ledkov

Revert callback change for now

Revision history for this message
Dimitri John Ledkov (xnox) wrote :

Barry, help! =) what's going on wrong here? See the added story, run with:

  ./bin/test -vvct access-token-header

It should fail at second token exchange, but it doesn't fail and instead allows exchanging request token over and over again.

Unmerged revisions

16991. By Dimitri John Ledkov

Revert callback change for now

16990. By Dimitri John Ledkov

OAuth Revert access-token.txt to current, add header-signature story.

16989. By Dimitri John Ledkov

Support oauth_callback from header authentication

16988. By Dimitri John Ledkov

Change access-token story to use header authentication.

16987. By Dimitri John Ledkov

Fix tests file references in the doctests comment.

16986. By Dimitri John Ledkov

Accept oauth signature from either headers or body in +access-token requests.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/services/oauth/browser/__init__.py'
--- lib/lp/services/oauth/browser/__init__.py 2013-04-11 02:12:09 +0000
+++ lib/lp/services/oauth/browser/__init__.py 2014-04-25 11:04:15 +0000
@@ -416,7 +416,7 @@
416 (or is not associated with the consumer), the signature does not match416 (or is not associated with the consumer), the signature does not match
417 or no permission has been granted by the user, respond with a 401.417 or no permission has been granted by the user, respond with a 401.
418 """418 """
419 form = self.request.form419 form = get_oauth_authorization(self.request)
420 consumer = getUtility(IOAuthConsumerSet).getByKey(420 consumer = getUtility(IOAuthConsumerSet).getByKey(
421 form.get('oauth_consumer_key'))421 form.get('oauth_consumer_key'))
422422
423423
=== modified 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 2014-04-25 11:04:15 +0000
@@ -1,7 +1,7 @@
1= OAuth =1= OAuth =
22
3Most of the OAuth doctests have been converted into unit tests and3Most of the OAuth doctests have been converted into unit tests and
4moved to test_oauth_tokens.py4moved to test_tokens.py
55
6== Nonces and timestamps ==6== Nonces and timestamps ==
77
88
=== added file 'lib/lp/services/oauth/stories/access-token-header.txt'
--- lib/lp/services/oauth/stories/access-token-header.txt 1970-01-01 00:00:00 +0000
+++ lib/lp/services/oauth/stories/access-token-header.txt 2014-04-25 11:04:15 +0000
@@ -0,0 +1,66 @@
1= Exchanging a request token for an access token, using headers =
2
3Once the user has logged into Launchpad and granted access to the
4consumer, the request token is eligible for being exchanged for an
5access token.
6
7This is boiler-plate from access-token.txt to begin with
8
9 # First we create a new request token and review it.
10 >>> from zope.component import getUtility
11 >>> from lp.services.oauth.interfaces import IOAuthConsumerSet
12 >>> from lp.services.webapp.interfaces import OAuthPermission
13 >>> from lp.registry.interfaces.person import IPersonSet
14 >>> from lp.registry.interfaces.product import IProductSet
15 >>> from lp.testing import login, logout
16 >>> login('salgado@ubuntu.com')
17 >>> consumer = getUtility(IOAuthConsumerSet).getByKey('foobar123451432')
18 >>> token = consumer.newRequestToken()
19 >>> salgado = getUtility(IPersonSet).getByName('salgado')
20 >>> token.review(salgado, OAuthPermission.WRITE_PUBLIC)
21 >>> logout()
22
23 >>> import time
24 >>> from urllib import urlencode
25 >>> data = dict(
26 ... oauth_consumer_key='foobar123451432',
27 ... oauth_version='1.0',
28 ... oauth_token=token.key,
29 ... oauth_signature_method='PLAINTEXT',
30 ... oauth_signature='&'.join([consumer.secret, token.secret]),
31 ... oauth_timestamp=time.time(),
32 ... oauth_nonce='4572616e48616d6d65724c61686176')
33
34Now, let's do the exchange using the OAuth 1.0 preferred way, by
35setting Authentication Header, instead of urlencoding oauth_
36parameters in the body of the request.
37
38 >>> auth = 'OAuth '+', '.join(['%s="%s"' % item for item in data.items()])
39 >>> auth_browser = setupBrowser(auth=auth)
40 >>> auth_browser.open(
41 ... 'http://launchpad.dev/+access-token')
42
43 >>> print auth_browser.contents
44 oauth_token=...&oauth_token_secret=...
45
46Yeah, token exchange was successful! Let's try again!
47
48 >>> auth_browser.open(
49 ... 'http://launchpad.dev/+access-token')
50
51 >>> print auth_browser.contents
52 oauth_token=...&oauth_token_secret=...
53
54Wait a minute, this sounds like a broken record ( or a PCD song "wait
55a minute"). I expected a 401 here! Let's assert that:
56
57 >>> auth_browser.open(
58 ... 'http://launchpad.dev/+access-token')
59 Traceback (most recent call last):
60 ...
61 HTTPError: HTTP Error 401: Unauthorized
62
63Bugger! +access-token essentially calls 'access_token =
64token.createAccessToken()' which does call 'self.destroySelf()' which
65totally works with body OAuth signature type in access-token.txt
66story, but not with my one-liner patch to enable header signatures.