launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/assertion-review-token into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #702911 in Launchpad itself: "RootObject:+authorize-token AssertionError when trying to use expired oauth token"
  https://bugs.launchpad.net/launchpad/+bug/702911

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/assertion-review-token/+merge/127639

Catch the AssertionErrors that are raised by IOAuthRequestToken.createAccessToken() and IOAuthRequestToken.review() in the browser code. Two out of the three cases for createAccessToken were handled, so I left one and generalized the other two. None of the cases for review() were handled, so now they both are.

I claim the LoC credit from https://code.launchpad.net/~stevenk/launchpad/destroy-simplified-branch-ff/+merge/127630 for this MP.
-- 
https://code.launchpad.net/~stevenk/launchpad/assertion-review-token/+merge/127639
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/assertion-review-token into lp:launchpad.

=== modified file 'lib/lp/services/oauth/browser/__init__.py'
--- lib/lp/services/oauth/browser/__init__.py	2012-01-01 02:58:52 +0000
+++ lib/lp/services/oauth/browser/__init__.py	2012-10-03 06:03:20 +0000
@@ -369,8 +369,12 @@
                 datetime.now(pytz.timezone('UTC')) + duration_delta)
         else:
             expiration_date = None
-        self.token.review(self.user, permission, self.token_context,
-                          date_expires=expiration_date)
+        try:
+            self.token.review(
+                self.user, permission, self.token_context,
+                      date_expires=expiration_date)
+        except AssertionError as e:
+            self.request.response.addNotification(str(e))
         callback = self.request.form.get('oauth_callback')
         if callback:
             self.next_url = callback
@@ -427,16 +431,12 @@
             return (
                 u"Request token has not yet been reviewed. Try again later.")
 
-        if token.permission == OAuthPermission.UNAUTHORIZED:
-            # The end-user explicitly refused to authorize this
-            # token. We send 403 ("Forbidden") instead of 401
-            # ("Unauthorized") to distinguish this case and to
-            # indicate that, as RFC2616 says, "authorization will not
-            # help."
+        try:
+            access_token = token.createAccessToken()
+        except AssertionError as e:
             self.request.response.setStatus(403)
-            return u'End-user refused to authorize request token.'
+            return unicode(e)
 
-        access_token = token.createAccessToken()
         context_name = None
         if access_token.context is not None:
             context_name = access_token.context.name

=== modified file 'lib/lp/services/oauth/stories/authorize-token.txt'
--- lib/lp/services/oauth/stories/authorize-token.txt	2012-02-25 03:01:34 +0000
+++ lib/lp/services/oauth/stories/authorize-token.txt	2012-10-03 06:03:20 +0000
@@ -275,6 +275,24 @@
     ...
     See all applications authorized to access Launchpad on your behalf.
 
+If the token has expired, we notify the user.
+
+    >>> token = request_token_for(consumer)
+    >>> from datetime import datetime, timedelta
+    >>> from pytz import UTC
+    >>> from zope.security.proxy import removeSecurityProxy
+    >>> date_created = datetime.now(UTC) - timedelta(hours=3)
+    >>> removeSecurityProxy(token).date_created = date_created
+    >>> params = dict(oauth_token=token.key)
+    >>> browser.open(
+    ...     "http://launchpad.dev/+authorize-token?%s"; % urlencode(params))
+    >>> browser.getControl('Read Anything').click()
+    >>> browser.url
+    'http://launchpad.dev/+authorize-token'
+    >>> [tag] = find_tags_by_class(browser.contents, 'informational message')
+    >>> print extract_text(tag)
+    This request token has expired and can no longer be reviewed.
+
 Desktop integration
 ===================