Merge lp:~leonardr/launchpad/distinguish-between-unauthorized-and-forbidden into lp:launchpad/db-devel

Proposed by Leonard Richardson
Status: Merged
Merge reported by: Leonard Richardson
Merged at revision: not available
Proposed branch: lp:~leonardr/launchpad/distinguish-between-unauthorized-and-forbidden
Merge into: lp:launchpad/db-devel
Diff against target: 80 lines (+34/-8)
2 files modified
lib/canonical/launchpad/browser/oauth.py (+13/-5)
lib/canonical/launchpad/pagetests/oauth/access-token.txt (+21/-3)
To merge this branch: bzr merge lp:~leonardr/launchpad/distinguish-between-unauthorized-and-forbidden
Reviewer Review Type Date Requested Status
Guilherme Salgado (community) Approve
Review via email: mp+29803@code.launchpad.net

Description of the change

This branch returns the response code 403 ("Forbidden") when the client attempts to exchange a request token for an access token, but the end-user has explicitly declined to authorize the request token. Previously the response code was 401 ("Unauthorized"), which made it impossible to distinguish between this case and the case where the end-user has not gotten around to make a decision about the request token.

I also return human-readable strings for various error conditions instead of the empty string.

To post a comment you must log in.
Revision history for this message
Guilherme Salgado (salgado) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/canonical/launchpad/browser/oauth.py'
2--- lib/canonical/launchpad/browser/oauth.py 2010-03-30 15:36:27 +0000
3+++ lib/canonical/launchpad/browser/oauth.py 2010-07-13 15:48:51 +0000
4@@ -245,15 +245,23 @@
5 token = consumer.getRequestToken(form.get('oauth_token'))
6 if token is None:
7 self.request.unauthorized(OAUTH_CHALLENGE)
8- return u''
9+ return u'No request token specified.'
10
11 if not check_oauth_signature(self.request, consumer, token):
12- return u''
13+ return u'Invalid OAuth signature.'
14
15- if (not token.is_reviewed
16- or token.permission == OAuthPermission.UNAUTHORIZED):
17+ if not token.is_reviewed:
18 self.request.unauthorized(OAUTH_CHALLENGE)
19- return u''
20+ return u'Request token has not yet been reviewed. Try again later.'
21+
22+ if token.permission == OAuthPermission.UNAUTHORIZED:
23+ # The end-user explicitly refused to authorize this
24+ # token. We send 403 ("Forbidden") instead of 401
25+ # ("Unauthorized") to distinguish this case and to
26+ # indicate that, as RFC2616 says, "authorization will not
27+ # help."
28+ self.request.response.setStatus(403)
29+ return u'End-user refused to authorize request token.'
30
31 access_token = token.createAccessToken()
32 context_name = None
33
34=== modified file 'lib/canonical/launchpad/pagetests/oauth/access-token.txt'
35--- lib/canonical/launchpad/pagetests/oauth/access-token.txt 2009-10-22 13:02:12 +0000
36+++ lib/canonical/launchpad/pagetests/oauth/access-token.txt 2010-07-13 15:48:51 +0000
37@@ -74,9 +74,10 @@
38 ... """ % urlencode(data2))
39 HTTP/1.1 401 Unauthorized
40 ...
41+ Request token has not yet been reviewed. Try again later.
42
43-The same will happen if the signature is wrong or if the token's
44-permission is UNAUTHORIZED.
45+If the token is missing or the signature is wrong the response will
46+also be 401.
47
48 >>> data2['oauth_signature'] = '&'.join(['foobar', token.secret])
49 >>> print http(r"""
50@@ -85,6 +86,22 @@
51 ... """ % urlencode(data2))
52 HTTP/1.1 401 Unauthorized
53 ...
54+ Invalid OAuth signature.
55+
56+ >>> data3 = data.copy()
57+ >>> del(data3['oauth_token'])
58+ >>> print http(r"""
59+ ... GET /+access-token?%s HTTP/1.1
60+ ... Host: launchpad.dev
61+ ... """ % urlencode(data3))
62+ HTTP/1.1 401 Unauthorized
63+ ...
64+ No request token specified.
65+
66+It the token's permission is set to UNAUTHORIZED, the response code is
67+403 ("Forbidden"). This conveys that (to quote the HTTP RFC)
68+"authorization will not help" and that the token will _never_ be
69+exchanged for an access token.
70
71 >>> token.review(salgado, OAuthPermission.UNAUTHORIZED)
72 >>> data2['oauth_signature'] = '&'.join([consumer.secret, token.secret])
73@@ -92,6 +109,7 @@
74 ... GET /+access-token?%s HTTP/1.1
75 ... Host: launchpad.dev
76 ... """ % urlencode(data2))
77- HTTP/1.1 401 Unauthorized
78+ HTTP/1.1 403 Forbidden
79 ...
80+ End-user refused to authorize request token.
81

Subscribers

People subscribed via source and target branches

to status/vote changes: