launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02813
[Merge] lp:~leonardr/launchpad/bug-271010 into lp:launchpad
Leonard Richardson has proposed merging lp:~leonardr/launchpad/bug-271010 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~leonardr/launchpad/bug-271010/+merge/51760
When you authorize a request token on +authorize-token, your browser is redirected to another page. If there's an OAuth callback URL set, you get redirected to that URL. If there's no callback URL, you get redirected to +token-authorized, which just prints a message "yay, you authorized the request token".
Here's the problem: newer versions of launchpadlib are polling Launchpad once a second, trying to exchange that request token for an access token. Once the exchange happens, the request token is destroyed and +token-authorized stops working. If the exchange happens before your browser makes that redirect request, by the time you arrive on +token-authorized your request token will no longer exist. You'll get an OOPS (see bug 271010). Worse, it looks like your integration failed, even though it actually succeeded.
This branch gets rid of +token-authorized altogether. The "yay, you authorized the request token" message is now part of the +authorize-token view, and is sent in response to the request that authorizes the request token. There's no redirect (except to an OAuth callback URL, which is a completely different case) and thus no possibility that the request token is destroyed before the message can be printed.
--
https://code.launchpad.net/~leonardr/launchpad/bug-271010/+merge/51760
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~leonardr/launchpad/bug-271010 into lp:launchpad.
=== modified file 'lib/canonical/launchpad/browser/oauth.py'
--- lib/canonical/launchpad/browser/oauth.py 2010-11-23 23:22:27 +0000
+++ lib/canonical/launchpad/browser/oauth.py 2011-03-01 15:22:04 +0000
@@ -6,7 +6,6 @@
'OAuthAccessTokenView',
'OAuthAuthorizeTokenView',
'OAuthRequestTokenView',
- 'OAuthTokenAuthorizedView',
'lookup_oauth_context']
from datetime import (
@@ -372,9 +371,6 @@
callback = self.request.form.get('oauth_callback')
if callback:
self.next_url = callback
- else:
- self.next_url = (
- '+token-authorized?oauth_token=%s' % self.token.key)
def lookup_oauth_context(context):
@@ -397,22 +393,6 @@
return context
-class OAuthTokenAuthorizedView(LaunchpadView):
- """Where users who reviewed tokens may get redirected to.
-
- If the consumer didn't include an oauth_callback when sending the user to
- Launchpad, this is the page the user is redirected to after he logs in and
- reviews the token.
- """
-
- def initialize(self):
- key = self.request.form.get('oauth_token')
- self.token = getUtility(IOAuthRequestTokenSet).getByKey(key)
- assert self.token.is_reviewed, (
- 'Users should be directed to this page only if they already '
- 'authorized the token.')
-
-
class OAuthAccessTokenView(LaunchpadView):
"""Where consumers may exchange a request token for an access token."""
=== modified file 'lib/canonical/launchpad/pagetests/oauth/authorize-token.txt'
--- lib/canonical/launchpad/pagetests/oauth/authorize-token.txt 2010-11-02 17:06:44 +0000
+++ lib/canonical/launchpad/pagetests/oauth/authorize-token.txt 2011-03-01 15:22:04 +0000
@@ -202,8 +202,7 @@
>>> token.is_reviewed
True
-If no oauth_callback is specified, we redirect the user to
-+authorized-token.
+If no oauth_callback is specified, we don't redirect the user.
# Create a new (unreviewed) token.
>>> token = request_token_for(consumer)
@@ -215,15 +214,17 @@
>>> browser.getControl('Read Anything').click()
>>> browser.url
- 'http://launchpad.dev/+token-authorized?...'
+ 'http://launchpad.dev/+authorize-token'
>>> print extract_text(find_tag_by_id(browser.contents, 'maincontent'))
Almost finished ...
To finish authorizing the application identified as foobar123451432 to
access Launchpad on your behalf you should go back to the application
window in which you started the process and inform it that you have done
your part of the process.
+ See all applications authorized to access Launchpad on your behalf.
-If we can't find the token, we will explain that to the user.
+If we can't find the request token (possibly because it was already
+exchanged for an access token), we will explain that to the user.
>>> params = dict(oauth_callback='http://example.com/oauth')
>>> browser.open(
@@ -233,6 +234,7 @@
The information provided by the remote application was incorrect or
incomplete. Because of that we were unable to identify the application
which would access Launchpad on your behalf.
+ You may have already authorized this application.
See all applications authorized to access Launchpad on your behalf.
>>> params = dict(
@@ -244,11 +246,12 @@
The information provided by the remote application was incorrect or
incomplete. Because of that we were unable to identify the application
which would access Launchpad on your behalf.
+ You may have already authorized this application.
See all applications authorized to access Launchpad on your behalf.
-The same happens if the token is already reviewed. Although that's
-probably an indication that the user has already reviewed that token in
-another window/tab so there's no need to worry.
+If the token is already reviewed (perhaps by the same user in another
+window or tab), but has not yet been exchanged for an access token,
+the success message is printed.
# Need to get the token again as it's been changed in another
# transaction.
@@ -260,9 +263,9 @@
>>> browser.open(
... "http://launchpad.dev/+authorize-token?%s"; % urlencode(params))
>>> print extract_text(find_tag_by_id(browser.contents, 'maincontent'))
- Request already reviewed
- This request for accessing Launchpad on your behalf has been
- reviewed ... ago.
+ Almost finished ...
+ To finish authorizing the application identified as foobar123451432
+ ...
See all applications authorized to access Launchpad on your behalf.
Desktop integration
@@ -397,6 +400,7 @@
The Ubuntu computer called mycomputer now has access to your
Launchpad account. Within a few seconds, you should be able to
start using its Launchpad integration features.
+ See all applications authorized to access Launchpad on your behalf.
>>> print token.is_reviewed
True
@@ -422,6 +426,7 @@
The integration you just authorized will expire in 59 minutes. At
that time, you'll have to re-authorize mycomputer, if you want to
keep using its Launchpad integration features.
+ See all applications authorized to access Launchpad on your behalf.
>>> print token.is_reviewed
True
@@ -481,6 +486,7 @@
You decided against desktop integration
You decided not to give mycomputer access to your Launchpad
account. You can always change your mind later.
+ See all applications authorized to access Launchpad on your behalf.
>>> print token.is_reviewed
True
=== modified file 'lib/canonical/launchpad/templates/oauth-authorize.pt'
--- lib/canonical/launchpad/templates/oauth-authorize.pt 2010-11-02 17:06:44 +0000
+++ lib/canonical/launchpad/templates/oauth-authorize.pt 2011-03-01 15:22:04 +0000
@@ -15,6 +15,11 @@
incomplete. Because of that we were unable to identify the
application which would access Launchpad on your behalf.
</p>
+
+ <p>
+ You may have already authorized this application.
+ </p>
+
</tal:no-token>
<tal:has-token condition="token">
@@ -123,12 +128,80 @@
</tal:token-not-reviewed>
<tal:token-reviewed condition="token/is_reviewed">
- <h1>Request already reviewed</h1>
- <p>
- This request for accessing Launchpad on your behalf has been
- reviewed <tal:date-reviewed
- content="token/date_reviewed/fmt:approximatedate" />.
- </p>
+
+ <tal:desktop-integration
+ condition="view/token/consumer/is_integrated_desktop">
+
+ <tal:unauthorized
+ condition="view/token/permission/enumvalue:UNAUTHORIZED">
+ <h1>You decided against desktop integration</h1>
+
+ <p>
+ You decided not to give
+ <strong
+ tal:content="view/token/consumer/integrated_desktop_name">hostname</strong>
+ access to your Launchpad account. You can always change your
+ mind later.
+ </p>
+ </tal:unauthorized>
+
+ <tal:authorized
+ condition="not:view/token/permission/enumvalue:UNAUTHORIZED">
+ <h1>Almost finished ...</h1>
+
+ <p>
+ The
+ <tal:desktop
+ replace="structure view/token/consumer/integrated_desktop_type" />
+ computer called
+ <strong
+ tal:content="view/token/consumer/integrated_desktop_name">hostname</strong>
+ now has access to your Launchpad account. Within a few
+ seconds, you should be able to start using its Launchpad
+ integration features.
+ </p>
+
+ <p tal:condition="view/token/date_expires">
+ The integration you just authorized will expire
+ <tal:date
+ replace="structure view/token/date_expires/fmt:approximatedate" />.
+ At that time, you'll have to re-authorize
+ <strong
+ tal:content="view/token/consumer/integrated_desktop_name">hostname</strong>,
+ if you want to keep using its Launchpad integration features.
+ </p>
+
+ </tal:authorized>
+ </tal:desktop-integration>
+
+ <tal:application-integration
+ condition="not:view/token/consumer/is_integrated_desktop">
+
+ <tal:unauthorized
+ condition="view/token/permission/enumvalue:UNAUTHORIZED">
+ <h1>Access not granted to application</h1>
+ <p>
+ The application identified as
+ <strong tal:content="view/token/consumer/key">key</strong> has not
+ been given access to your protected resources on Launchpad.
+ </p>
+ </tal:unauthorized>
+
+ <tal:authorized
+ condition="not:view/token/permission/enumvalue:UNAUTHORIZED">
+ <h1>Almost finished ...</h1>
+ <p>
+ To finish authorizing the application identified as
+ <strong tal:content="view/token/consumer/key">key</strong>
+ to access Launchpad on your behalf you should go back to
+ the application window in which you started the process
+ and inform it that you have done your part of the
+ process.
+ </p>
+ </tal:authorized>
+
+ </tal:application-integration>
+
</tal:token-reviewed>
</tal:has-token>
=== removed file 'lib/canonical/launchpad/templates/token-authorized.pt'
--- lib/canonical/launchpad/templates/token-authorized.pt 2010-11-02 17:06:44 +0000
+++ lib/canonical/launchpad/templates/token-authorized.pt 1970-01-01 00:00:00 +0000
@@ -1,76 +0,0 @@
-<html
- xmlns="http://www.w3.org/1999/xhtml";
- xmlns:tal="http://xml.zope.org/namespaces/tal";
- xmlns:metal="http://xml.zope.org/namespaces/metal";
- xmlns:i18n="http://xml.zope.org/namespaces/i18n";
- metal:use-macro="view/macro:page/locationless"
- i18n:domain="launchpad"
->
-<body>
- <div class="top-portlet" metal:fill-slot="main">
-
- <tal:desktop condition="view/token/consumer/is_integrated_desktop">
-
- <tal:unauthorized condition="view/token/permission/enumvalue:UNAUTHORIZED">
- <h1>You decided against desktop integration</h1>
-
- <p>
- You decided not to give
- <strong tal:content="view/token/consumer/integrated_desktop_name">hostname</strong>
- access to your Launchpad account. You can always change your
- mind later.
- </p>
- </tal:unauthorized>
-
- <tal:authorized condition="not:view/token/permission/enumvalue:UNAUTHORIZED">
- <h1>Almost finished ...</h1>
-
- <p>The
- <tal:desktop replace="structure view/token/consumer/integrated_desktop_type" />
- computer called
- <strong tal:content="view/token/consumer/integrated_desktop_name">hostname</strong>
- now has access to your Launchpad account. Within a few
- seconds, you should be able to start using its Launchpad
- integration features.</p>
-
- <p tal:condition="view/token/date_expires">
- The integration you just authorized will expire
- <tal:date
- replace="structure view/token/date_expires/fmt:approximatedate" />.
- At that time, you'll have to re-authorize
- <strong tal:content="view/token/consumer/integrated_desktop_name">hostname</strong>,
- if you want to keep using its Launchpad integration features.
-
- </p>
-
- </tal:authorized>
- </tal:desktop>
-
- <tal:application condition="not:view/token/consumer/is_integrated_desktop">
-
- <tal:unauthorized condition="view/token/permission/enumvalue:UNAUTHORIZED">
- <h1>Access not granted to application</h1>
- <p>
- The application identified as
- <strong tal:content="view/token/consumer/key">key</strong> has not
- been given access to your protected resources on Launchpad.
- </p>
- </tal:unauthorized>
-
- <tal:authorized condition="not:view/token/permission/enumvalue:UNAUTHORIZED">
- <h1>Almost finished ...</h1>
- <p>
- To finish authorizing the application identified as
- <strong tal:content="view/token/consumer/key">key</strong> to access
- Launchpad on your behalf you should go back to the application window
- in which you started the process and inform it that you have done your
- part of the process.
- </p>
- </tal:authorized>
-
- </tal:application>
- </div>
-
-</body>
-</html>
-
=== modified file 'lib/canonical/launchpad/zcml/launchpad.zcml'
--- lib/canonical/launchpad/zcml/launchpad.zcml 2010-11-08 14:16:17 +0000
+++ lib/canonical/launchpad/zcml/launchpad.zcml 2011-03-01 15:22:04 +0000
@@ -261,13 +261,6 @@
<browser:page
for="canonical.launchpad.webapp.interfaces.ILaunchpadApplication"
- name="+token-authorized"
- class="canonical.launchpad.browser.OAuthTokenAuthorizedView"
- template="../templates/token-authorized.pt"
- permission="launchpad.AnyPerson" />
-
- <browser:page
- for="canonical.launchpad.webapp.interfaces.ILaunchpadApplication"
name="+access-token"
class="canonical.launchpad.browser.OAuthAccessTokenView"
permission="zope.Public" />
