← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~leonardr/launchpad/accept-oauth-signatures into lp:launchpad

 

Leonard Richardson has proposed merging lp:~leonardr/launchpad/accept-oauth-signatures into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)


Backstory: the only way to get (non-anonymous or read-write) access to the Launchpad web service is to associate an OAuth access token with your account, and have your web service client sign all outgoing requests with that token. The level of access you get to the web service depends on the permission level associated with the token: READ_PRIVATE, WRITE_PUBLIC, etc.

Currently, a third-party desktop application obtains one of these OAuth tokens by opening a web-browser to the /+authorize-token page on Launchpad. The end-user logs into Launchpad and authorizes the token. Subsequently the desktop application can sign requests with the new token.

The problem is the "open a browser/log into Launchpad" step, which confuses users. We're working on a desktop application that wraps a web browser to eliminate confusion and reduce the number of times the end-user has to log into Launchpad. This branch changes Launchpad to make a portion of the _web site_ a little more like the _web service_. If you have the right OAuth access token, you can sign outdoing HTTP requests to the _web site_ and not have to go through the login procedure.

Recall that the whole point of an OAuth token is to authenticate against Launchpad without knowing a username/password combination. The desktop application will obtain an OAuth token with the special GRANT_PERMISSIONS access level. Then, when it needs to obtain some lesser access token for another application, it'll use that GRANT_PERMISSIONS token to skip the login step and take the end-user straight to /+authorize-token.

If you take a look at the "Access through OAuth" section of authorize-token.txt, you'll see this in action. Earlier in that test we run through the token authorization process using a Mechanize browser programmed with HTTP Basic Auth credentials. In "Access through OAuth", we go through the same process using a Mechanize browser that signs outgoing requests with a GRANT_PERMISSIONS token. Launchpad--specifically, the +authorize-token and +token-authorized views--will accept either.

To implement this, I removed the access controls on +authorize-token and +token-authorized, and implemented my own (ensureRequestIsAuthorizedOrSigned). The new controls delegate to the normal Launchpad publication controls, but will also allow access if the incoming request is signed with the right OAuth token. This let me keep all the special code near the two views that use it, and saved me from having to create a brand new publication (or alter the existing publication) just for those two views.

The other major code in this branch is a refactoring of WebServicePublication.get_principal. I moved the code for extracting OAuth information from a request, and validating a request's OAuth signature, into helper functions in webapp/authentication.py. It's called from get_principal, and also from ensureRequestIsAuthorizedOrSigned.

There's one more things I'd like to address in a follow-up branch (this branch is already huge, partly due to delinting), or possibly in this branch once it's been reviewed. There's no way to get access to any other page on Launchpad by signing a request with an OAuth token, but I'd like to add a test to prove it.
-- 
https://code.launchpad.net/~leonardr/launchpad/accept-oauth-signatures/+merge/35697
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~leonardr/launchpad/accept-oauth-signatures into lp:launchpad.
=== modified file 'lib/canonical/launchpad/browser/oauth.py'
--- lib/canonical/launchpad/browser/oauth.py	2010-08-24 16:44:42 +0000
+++ lib/canonical/launchpad/browser/oauth.py	2010-09-16 17:26:09 +0000
@@ -11,11 +11,13 @@
 
 from lazr.restful import HTTPResource
 import simplejson
+from zope.authentication.interfaces import IUnauthenticatedPrincipal
 from zope.component import getUtility
 from zope.formlib.form import (
     Action,
     Actions,
     )
+from zope.security.interfaces import Unauthorized
 
 from canonical.launchpad.interfaces.oauth import (
     IOAuthConsumerSet,
@@ -29,9 +31,15 @@
     )
 from canonical.launchpad.webapp.authentication import (
     check_oauth_signature,
+    extract_oauth_access_token,
     get_oauth_authorization,
-    )
-from canonical.launchpad.webapp.interfaces import OAuthPermission
+    get_oauth_principal
+    )
+from canonical.launchpad.webapp.interfaces import (
+    AccessLevel,
+    ILaunchBag,
+    OAuthPermission,
+    )
 from lp.app.errors import UnexpectedFormData
 from lp.registry.interfaces.distribution import IDistributionSet
 from lp.registry.interfaces.pillar import IPillarNameSet
@@ -98,6 +106,7 @@
         return u'oauth_token=%s&oauth_token_secret=%s' % (
             token.key, token.secret)
 
+
 def token_exists_and_is_not_reviewed(form, action):
     return form.token is not None and not form.token.is_reviewed
 
@@ -106,8 +115,10 @@
     """Return a list of `Action`s for each possible `OAuthPermission`."""
     actions = Actions()
     actions_excluding_grant_permissions = Actions()
+
     def success(form, action, data):
         form.reviewToken(action.permission)
+
     for permission in OAuthPermission.items:
         action = Action(
             permission.title, name=permission.name, success=success,
@@ -118,7 +129,86 @@
             actions_excluding_grant_permissions.append(action)
     return actions, actions_excluding_grant_permissions
 
-class OAuthAuthorizeTokenView(LaunchpadFormView, JSONTokenMixin):
+
+class CredentialManagerAwareMixin:
+    """A view for which a browser may authenticate with an OAuth token.
+
+    The OAuth token must be signed with a token that has the
+    GRANT_PERMISSIONS access level, and the browser must present
+    itself as the Launchpad Credentials Manager.
+    """
+    # A prefix identifying the Launchpad Credential Manager's
+    # User-Agent string.
+    GRANT_PERMISSIONS_USER_AGENT_PREFIX = "Launchpad Credentials Manager"
+
+    def ensureRequestIsAuthorizedOrSigned(self):
+        """Find the user who initiated the request.
+
+        This property is used by a view that wants to reject access
+        unless the end-user is authenticated with cookie auth, HTTP
+        Basic Auth, *or* a properly authorized OAuth token.
+
+        If the user is logged in with cookie auth or HTTP Basic, then
+        other parts of Launchpad have taken care of the login and we
+        don't have to do anything. But if the user's browser has
+        signed the request with an OAuth token, other parts of
+        Launchpad won't recognize that as an attempt to authorize the
+        request.
+
+        This method does the OAuth part of the work. It checks that
+        the OAuth token is valid, that it's got the correct access
+        level, and that the User-Agent is one that's allowed to sign
+        requests with OAuth tokens.
+
+        :return: The user who Launchpad identifies as the principal.
+         Or, if Launchpad identifies no one as the principal, the user
+         whose valid GRANT_PERMISSIONS OAuth token was used to sign
+         the request.
+
+        :raise Unauthorized: If the request is unauthorized and
+         unsigned, improperly signed, anonymously signed, or signed
+         with a token that does not have the right access level.
+        """
+        user = getUtility(ILaunchBag).user
+        if user is not None:
+            return user
+        # The normal Launchpad code was not able to identify any
+        # user, but we're going to try a little harder before
+        # concluding that no one's logged in. If the incoming
+        # request is signed by an OAuth access token with the
+        # GRANT_PERMISSIONS access level, we will force a
+        # temporary login with the user whose access token this
+        # is.
+        token = extract_oauth_access_token(self.request)
+        if token is None:
+            # The request is not OAuth-signed. The normal Launchpad
+            # code had it right: no one is authenticated.
+            raise Unauthorized("Anonymous access is not allowed.")
+        principal = get_oauth_principal(self.request)
+        if IUnauthenticatedPrincipal.providedBy(principal):
+            # The request is OAuth-signed, but as the anonymous
+            # user.
+            raise Unauthorized("Anonymous access is not allowed.")
+        if token.permission != AccessLevel.GRANT_PERMISSIONS:
+            # The request is OAuth-signed, but the token has
+            # the wrong access level.
+            raise Unauthorized("OAuth token has insufficient access level.")
+
+        # Both the consumer key and the User-Agent must identify the
+        # Launchpad Credentials Manager.
+        must_start_with_prefix = [
+            token.consumer.key, self.request.getHeader("User-Agent")]
+        for string in must_start_with_prefix:
+            if not string.startswith(
+                self.GRANT_PERMISSIONS_USER_AGENT_PREFIX):
+                raise Unauthorized(
+                    "Only the Launchpad Credentials Manager can access this "
+                    "page by signing requests with an OAuth token.")
+        return principal.person
+
+
+class OAuthAuthorizeTokenView(
+    LaunchpadFormView, JSONTokenMixin, CredentialManagerAwareMixin):
     """Where users authorize consumers to access Launchpad on their behalf."""
 
     actions, actions_excluding_grant_permissions = (
@@ -167,6 +257,12 @@
             and len(allowed_permissions) > 1):
             allowed_permissions.remove(OAuthPermission.GRANT_PERMISSIONS.name)
 
+        # GRANT_PERMISSIONS may only be requested by a specific User-Agent.
+        if (OAuthPermission.GRANT_PERMISSIONS.name in allowed_permissions
+            and not self.request.getHeader("User-Agent").startswith(
+                self.GRANT_PERMISSIONS_USER_AGENT_PREFIX)):
+            allowed_permissions.remove(OAuthPermission.GRANT_PERMISSIONS.name)
+
         for action in self.actions:
             if (action.permission.name in allowed_permissions
                 or action.permission is OAuthPermission.UNAUTHORIZED):
@@ -184,9 +280,10 @@
         return actions
 
     def initialize(self):
+        self.oauth_authorized_user = self.ensureRequestIsAuthorizedOrSigned()
         self.storeTokenContext()
-        form = get_oauth_authorization(self.request)
-        key = form.get('oauth_token')
+
+        key = self.request.form.get('oauth_token')
         if key:
             self.token = getUtility(IOAuthRequestTokenSet).getByKey(key)
         super(OAuthAuthorizeTokenView, self).initialize()
@@ -217,7 +314,8 @@
         self.token_context = context
 
     def reviewToken(self, permission):
-        self.token.review(self.user, permission, self.token_context)
+        self.token.review(self.user or self.oauth_authorized_user,
+                          permission, self.token_context)
         callback = self.request.form.get('oauth_callback')
         if callback:
             self.next_url = callback
@@ -245,7 +343,7 @@
     return context
 
 
-class OAuthTokenAuthorizedView(LaunchpadView):
+class OAuthTokenAuthorizedView(LaunchpadView, CredentialManagerAwareMixin):
     """Where users who reviewed tokens may get redirected to.
 
     If the consumer didn't include an oauth_callback when sending the user to
@@ -254,6 +352,7 @@
     """
 
     def initialize(self):
+        authorized_user = self.ensureRequestIsAuthorizedOrSigned()
         key = self.request.form.get('oauth_token')
         self.token = getUtility(IOAuthRequestTokenSet).getByKey(key)
         assert self.token.is_reviewed, (

=== modified file 'lib/canonical/launchpad/database/oauth.py'
--- lib/canonical/launchpad/database/oauth.py	2010-08-20 20:31:18 +0000
+++ lib/canonical/launchpad/database/oauth.py	2010-09-16 17:26:09 +0000
@@ -60,14 +60,14 @@
 
 # How many hours should a request token be valid for?
 REQUEST_TOKEN_VALIDITY = 12
-# 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.
+# 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
@@ -77,6 +77,7 @@
 # amount.
 TIMESTAMP_SKEW_WINDOW = 60*60 # seconds, +/-
 
+
 class OAuthBase(SQLBase):
     """Base class for all OAuth database classes."""
 
@@ -93,6 +94,7 @@
 
     getStore = _get_store
 
+
 class OAuthConsumer(OAuthBase):
     """See `IOAuthConsumer`."""
     implements(IOAuthConsumer)
@@ -323,6 +325,7 @@
     The key will have a length of 20 and we'll make sure it's not yet in the
     given table.  The secret will have a length of 80.
     """
+
     key_length = 20
     key = create_unique_token_for_table(key_length, getattr(table, "key"))
     secret_length = 80

=== modified file 'lib/canonical/launchpad/pagetests/oauth/authorize-token.txt'
--- lib/canonical/launchpad/pagetests/oauth/authorize-token.txt	2010-08-24 16:44:42 +0000
+++ lib/canonical/launchpad/pagetests/oauth/authorize-token.txt	2010-09-16 17:26:09 +0000
@@ -1,4 +1,6 @@
-= Authorizing a request token =
+***************************
+Authorizing a request token
+***************************
 
 Once the consumer gets a request token, it must send the user to
 Launchpad's +authorize-token page in order for the user to authenticate
@@ -19,9 +21,10 @@
 The oauth_token parameter, on the other hand, is required in the
 Launchpad implementation.
 
-The +authorize-token page is restricted to logged in users, so users will
-first be asked to log in. (We won't show the actual login process because
-it involves OpenID, which would complicate this test quite a bit.)
+Access to the page
+==================
+
+The +authorize-token page is restricted to authenticated users.
 
     >>> from urllib import urlencode
     >>> params = dict(
@@ -30,7 +33,18 @@
     >>> browser.open(url)
     Traceback (most recent call last):
     ...
-    Unauthorized:...
+    Unauthorized: Anonymous access is not allowed.
+
+However, the details of the authentication are different than from any
+other part of Launchpad. Unlike with other pages, a user can authorize
+an OAuth token by signing their outgoing requests with an _existing_
+OAuth token. This makes it possible for a desktop client to retrieve
+this page without knowing the end-user's username and password, or
+making them navigate the arbitrarily complex OpenID login procedure.
+
+But, let's deal with that a little later. First let's show how the
+process works through HTTP Basic Auth (the testing equivalent of a
+regular username-and-password login).
 
     >>> browser = setupBrowser(auth='Basic no-priv@xxxxxxxxxxxxx:test')
     >>> browser.open(url)
@@ -44,6 +58,10 @@
     ...
     See all applications authorized to access Launchpad on your behalf.
 
+
+Using the page
+==============
+
 This page contains one submit button for each item of OAuthPermission,
 except for 'Grant Permissions', which must be specifically requested.
 
@@ -74,7 +92,34 @@
 that isn't enough for the application. The user always has the option
 to deny permission altogether.
 
-    >>> def print_access_levels(allow_permission):
+    >>> def filter_user_agent(key, value, new_value):
+    ...     """A filter to replace the User-Agent header in a list of headers.
+    ...
+    ...     [XXX bug=638058] This is a hack to work around a bug in
+    ...     zope.testbrowser.
+    ...     """
+    ...
+    ...     if key.lower() == "user-agent":
+    ...         return (key, new_value)
+    ...     return (key, value)
+
+    >>> def print_access_levels(allow_permission, user_agent=None):
+    ...     if user_agent is not None:
+    ...         # [XXX bug=638058] This is a hack to work around a bug in
+    ...         # zope.testbrowser which prevents browser.addHeader
+    ...         # from working with User-Agent.
+    ...         mech_browser = browser.mech_browser
+    ...         # Store the original User-Agent for later.
+    ...         old_user_agent = [
+    ...             value for key, value in mech_browser.addheaders
+    ...             if key.lower() == "user-agent"][0]
+    ...         # Replace the User-Agent with the value passed into this
+    ...         # function.
+    ...         mech_browser.addheaders = [
+    ...             filter_user_agent(key, value, user_agent)
+    ...             for key, value in mech_browser.addheaders]
+    ...
+    ...     # Okay, now we can make the request.
     ...     browser.open(
     ...         "http://launchpad.dev/+authorize-token?%s&%s";
     ...         % (urlencode(params), allow_permission))
@@ -82,6 +127,13 @@
     ...     actions = main_content.findAll('input', attrs={'type': 'submit'})
     ...     for action in actions:
     ...         print action['value']
+    ...
+    ...     if user_agent is not None:
+    ...         # Finally, restore the old User-Agent.
+    ...         mech_browser.addheaders = [
+    ...             filter_user_agent(key, value, old_user_agent)
+    ...             for key, value in mech_browser.addheaders]
+
 
     >>> print_access_levels(
     ...     'allow_permission=WRITE_PUBLIC&allow_permission=WRITE_PRIVATE')
@@ -90,23 +142,38 @@
     Change Anything
 
 The only time the 'Grant Permissions' permission shows up in this list
-is if the client specifically requests it, and no other
-permission. (Also requesting UNAUTHORIZED is okay--it will show up
-anyway.)
+is if a client identifying itself as the Launchpad Credentials Manager
+specifically requests it, and no other permission. (Also requesting
+UNAUTHORIZED is okay--it will show up anyway.)
+
+    >>> USER_AGENT = "Launchpad Credentials Manager v1.0"
+    >>> print_access_levels(
+    ...     'allow_permission=GRANT_PERMISSIONS', USER_AGENT)
+    No Access
+    Grant Permissions
+
+    >>> print_access_levels(
+    ...     ('allow_permission=GRANT_PERMISSIONS&'
+    ...      'allow_permission=UNAUTHORIZED'),
+    ...     USER_AGENT)
+    No Access
+    Grant Permissions
+
+    >>> print_access_levels(
+    ...     ('allow_permission=WRITE_PUBLIC&'
+    ...      'allow_permission=GRANT_PERMISSIONS'))
+    No Access
+    Change Non-Private Data
+
+If a client asks for GRANT_PERMISSIONS but doesn't claim to be the
+Launchpad Credentials Manager, Launchpad will not show GRANT_PERMISSIONS.
 
     >>> print_access_levels('allow_permission=GRANT_PERMISSIONS')
     No Access
-    Grant Permissions
-
-    >>> print_access_levels(
-    ...     'allow_permission=GRANT_PERMISSIONS&allow_permission=UNAUTHORIZED')
-    No Access
-    Grant Permissions
-
-    >>> print_access_levels(
-    ...     'allow_permission=WRITE_PUBLIC&allow_permission=GRANT_PERMISSIONS')
-    No Access
+    Read Non-Private Data
     Change Non-Private Data
+    Read Anything
+    Change Anything
 
 If an application doesn't specify any valid access levels, or only
 specifies the UNAUTHORIZED access level, Launchpad will show all the
@@ -263,3 +330,120 @@
     This request for accessing Launchpad on your behalf has been
     reviewed ... ago.
     See all applications authorized to access Launchpad on your behalf.
+
+Access through OAuth
+====================
+
+Now it's time to show how to go through the same process without
+knowing the end-user's username and password. All you need is an OAuth
+token issued with the GRANT_PERMISSIONS access level, in the name of
+the Launchpad Credentials Manager.
+
+Let's go through the approval process again, without ever sending the
+no-priv user's username or password over HTTP. First we'll create a
+new request token to be approved.
+
+    >>> login('no-priv@xxxxxxxxxxxxx')
+    >>> consumer = getUtility(IOAuthConsumerSet).getByKey('foobar123451432')
+    >>> request_token = consumer.newRequestToken()
+    >>> logout()
+
+    >>> params = dict(oauth_token=request_token.key)
+    >>> url = "http://launchpad.dev/+authorize-token?%s"; % urlencode(params)
+
+Next, we'll create an access token to sign the requests that will
+approve the request token.
+
+    >>> from oauth.oauth import OAuthConsumer
+    >>> from canonical.launchpad.interfaces import IPersonSet
+    >>> from lp.testing import oauth_access_token_for
+
+    >>> manager_consumer = OAuthConsumer("Launchpad Credentials Manager", "")
+    >>> login(ANONYMOUS)
+    >>> no_priv = getUtility(IPersonSet).getByEmail('no-priv@xxxxxxxxxxxxx')
+    >>> grant_permissions_token = oauth_access_token_for(
+    ...     manager_consumer.key, no_priv, "GRANT_PERMISSIONS")
+    >>> logout()
+
+Next, we create a browser object that knows how to sign requests with
+the newly created access token.
+
+    >>> from lp.testing import OAuthSigningBrowser
+    >>> browser = OAuthSigningBrowser(
+    ...     manager_consumer, grant_permissions_token, USER_AGENT)
+    >>> browser.open(url)
+    >>> print browser.title
+    Authorize application to access Launchpad on your behalf
+
+Now we can approve the request and see the appropriate messages.
+
+    >>> browser.getControl('Read Anything').click()
+
+    >>> browser.url
+    'http://launchpad.dev/+token-authorized?...'
+    >>> 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.
+
+OAuth error conditions
+----------------------
+
+The OAuth token used to sign the requests must have the
+GRANT_PERMISSIONS access level; no other access level will work.
+
+    >>> login(ANONYMOUS)
+    >>> insufficient_token = oauth_access_token_for(
+    ...     manager_consumer.key, no_priv, "WRITE_PRIVATE")
+    >>> logout()
+
+    >>> browser = OAuthSigningBrowser(
+    ...     manager_consumer, insufficient_token, USER_AGENT)
+    >>> browser.open(url)
+    Traceback (most recent call last):
+    ...
+    Unauthorized: OAuth token has insufficient access level.
+
+The OAuth token must be for the Launchpad Credentials Manager, or it
+cannot be used. (Launchpad shouldn't even _issue_ a GRANT_PERMISSIONS
+token for any other consumer, but even if it somehow does, that token
+can't be used for this.)
+
+    >>> login(ANONYMOUS)
+    >>> wrong_consumer = OAuthConsumer(
+    ...     "Not the Launchpad Credentials Manager", "")
+    >>> wrong_consumer_token = oauth_access_token_for(
+    ...     wrong_consumer.key, no_priv, "GRANT_PERMISSIONS")
+    >>> logout()
+
+    >>> browser = OAuthSigningBrowser(wrong_consumer, wrong_consumer_token)
+    >>> browser.open(url)
+    Traceback (most recent call last):
+    ...
+    Unauthorized: Only the Launchpad Credentials Manager can access
+    this page by signing requests with an OAuth token.
+
+Signing with an anonymous token will also not work.
+
+    >>> from oauth.oauth import OAuthToken
+    >>> anonymous_token = OAuthToken(key="", secret="")
+    >>> browser = OAuthSigningBrowser(manager_consumer, anonymous_token)
+    >>> browser.open(url)
+    Traceback (most recent call last):
+    ...
+    Unauthorized: Anonymous access is not allowed.
+
+Even if it presents the right token, the user agent sending the signed
+request must *also* identify *itself* as the Launchpad Credentials
+Manager.
+
+    >>> browser = OAuthSigningBrowser(
+    ...     manager_consumer, grant_permissions_token,
+    ...     "Not the Launchpad Credentials Manager")
+    >>> browser.open(url)
+    Traceback (most recent call last):
+    ...
+    Unauthorized: Only the Launchpad Credentials Manager can access
+    this page by signing requests with an OAuth token.

=== modified file 'lib/canonical/launchpad/webapp/authentication.py'
--- lib/canonical/launchpad/webapp/authentication.py	2010-08-20 20:31:18 +0000
+++ lib/canonical/launchpad/webapp/authentication.py	2010-09-16 17:26:09 +0000
@@ -5,16 +5,21 @@
 
 __all__ = [
     'check_oauth_signature',
+    'extract_oauth_access_token',
+    'get_oauth_principal',
     'get_oauth_authorization',
     'LaunchpadLoginSource',
     'LaunchpadPrincipal',
+    'OAuthSignedRequest',
     'PlacelessAuthUtility',
     'SSHADigestEncryptor',
     ]
 
 
 import binascii
+from datetime import datetime
 import hashlib
+import pytz
 import random
 from UserDict import UserDict
 
@@ -23,13 +28,18 @@
 from zope.app.security.interfaces import ILoginPassword
 from zope.app.security.principalregistry import UnauthenticatedPrincipal
 from zope.authentication.interfaces import IUnauthenticatedPrincipal
+
 from zope.component import (
     adapts,
     getUtility,
     )
 from zope.event import notify
-from zope.interface import implements
+from zope.interface import (
+    alsoProvides,
+    implements,
+    )
 from zope.preference.interfaces import IPreferenceGroup
+from zope.security.interfaces import Unauthorized
 from zope.security.proxy import removeSecurityProxy
 from zope.session.interfaces import ISession
 
@@ -44,6 +54,14 @@
     ILaunchpadPrincipal,
     IPlacelessAuthUtility,
     IPlacelessLoginSource,
+    OAuthPermission,
+    )
+from canonical.launchpad.interfaces.oauth import (
+    ClockSkew,
+    IOAuthConsumerSet,
+    IOAuthSignedRequest,
+    NonceAlreadyUsed,
+    TimestampOrderingError,
     )
 from lp.registry.interfaces.person import (
     IPerson,
@@ -51,6 +69,113 @@
     )
 
 
+def extract_oauth_access_token(request):
+    """Find the OAuth access token that signed the given request.
+
+    :param request: An incoming request.
+
+    :return: an IOAuthAccessToken, or None if the request is not
+        signed at all.
+
+    :raise Unauthorized: If the token is invalid or the request is an
+        anonymously-signed request that doesn't meet our requirements.
+    """
+    # Fetch OAuth authorization information from the request.
+    form = get_oauth_authorization(request)
+
+    consumer_key = form.get('oauth_consumer_key')
+    consumers = getUtility(IOAuthConsumerSet)
+    consumer = consumers.getByKey(consumer_key)
+    token_key = form.get('oauth_token')
+    anonymous_request = (token_key == '')
+
+    if consumer_key is None:
+        # Either the client's OAuth implementation is broken, or
+        # the user is trying to make an unauthenticated request
+        # using wget or another OAuth-ignorant application.
+        # Try to retrieve a consumer based on the User-Agent
+        # header.
+        anonymous_request = True
+        consumer_key = request.getHeader('User-Agent', '')
+        if consumer_key == '':
+            raise Unauthorized(
+                'Anonymous requests must provide a User-Agent.')
+        consumer = consumers.getByKey(consumer_key)
+
+    if consumer is None:
+        if anonymous_request:
+            # This is the first time anyone has tried to make an
+            # anonymous request using this consumer name (or user
+            # agent). Dynamically create the consumer.
+            #
+            # In the normal website this wouldn't be possible
+            # because GET requests have their transactions rolled
+            # back. But webservice requests always have their
+            # transactions committed so that we can keep track of
+            # the OAuth nonces and prevent replay attacks.
+            if consumer_key == '' or consumer_key is None:
+                raise Unauthorized("No consumer key specified.")
+            consumer = consumers.new(consumer_key, '')
+        else:
+            # An unknown consumer can never make a non-anonymous
+            # request, because access tokens are registered with a
+            # specific, known consumer.
+            raise Unauthorized('Unknown consumer (%s).' % consumer_key)
+    if anonymous_request:
+        # Skip the OAuth verification step and let the user access the
+        # web service as an unauthenticated user.
+        #
+        # XXX leonardr 2009-12-15 bug=496964: Ideally we'd be
+        # auto-creating a token for the anonymous user the first
+        # time, passing it through the OAuth verification step,
+        # and using it on all subsequent anonymous requests.
+        return None
+
+    token = consumer.getAccessToken(token_key)
+    if token is None:
+        raise Unauthorized('Unknown access token (%s).' % token_key)
+    return token
+
+
+def get_oauth_principal(request):
+    """Find the principal to use for this OAuth-signed request.
+
+    :param request: An incoming request.
+    :return: An ILaunchpadPrincipal with the appropriate access level.
+    """
+    token = extract_oauth_access_token(request)
+
+    if token is None:
+        # The consumer is making an anonymous request. If there was a
+        # problem with the access token, extract_oauth_access_token
+        # would have raised Unauthorized.
+        alsoProvides(request, IOAuthSignedRequest)
+        auth_utility = getUtility(IPlacelessAuthUtility)
+        return auth_utility.unauthenticatedPrincipal()
+
+    form = get_oauth_authorization(request)
+    nonce = form.get('oauth_nonce')
+    timestamp = form.get('oauth_timestamp')
+    try:
+        token.checkNonceAndTimestamp(nonce, timestamp)
+    except (NonceAlreadyUsed, TimestampOrderingError, ClockSkew), e:
+        raise Unauthorized('Invalid nonce/timestamp: %s' % e)
+    now = datetime.now(pytz.timezone('UTC'))
+    if token.permission == OAuthPermission.UNAUTHORIZED:
+        raise Unauthorized('Unauthorized token (%s).' % token.key)
+    elif token.date_expires is not None and token.date_expires <= now:
+        raise Unauthorized('Expired token (%s).' % token.key)
+    elif not check_oauth_signature(request, token.consumer, token):
+        raise Unauthorized('Invalid signature.')
+    else:
+        # Everything is fine, let's return the principal.
+        pass
+    alsoProvides(request, IOAuthSignedRequest)
+    return getUtility(IPlacelessLoginSource).getPrincipal(
+        token.person.account.id, access_level=token.permission,
+        scope=token.context)
+
+
 class PlacelessAuthUtility:
     """An authentication service which holds no state aside from its
     ZCML configuration, implemented as a utility.
@@ -75,9 +200,8 @@
                     # as the login form is never visited for BasicAuth.
                     # This we treat each request as a separate
                     # login/logout.
-                    notify(BasicAuthLoggedInEvent(
-                        request, login, principal
-                        ))
+                    notify(
+                        BasicAuthLoggedInEvent(request, login, principal))
                     return principal
 
     def _authenticateUsingCookieAuth(self, request):
@@ -190,7 +314,8 @@
         plaintext = str(plaintext)
         if salt is None:
             salt = self.generate_salt()
-        v = binascii.b2a_base64(hashlib.sha1(plaintext + salt).digest() + salt)
+        v = binascii.b2a_base64(
+            hashlib.sha1(plaintext + salt).digest() + salt)
         return v[:-1]
 
     def validate(self, plaintext, encrypted):
@@ -334,6 +459,7 @@
 
 # zope.app.apidoc expects our principals to be adaptable into IAnnotations, so
 # we use these dummy adapters here just to make that code not OOPS.
+
 class TemporaryPrincipalAnnotations(UserDict):
     implements(IAnnotations)
     adapts(ILaunchpadPrincipal, IPreferenceGroup)

=== modified file 'lib/canonical/launchpad/webapp/servers.py'
--- lib/canonical/launchpad/webapp/servers.py	2010-09-10 06:38:15 +0000
+++ lib/canonical/launchpad/webapp/servers.py	2010-09-16 17:26:09 +0000
@@ -8,7 +8,6 @@
 __metaclass__ = type
 
 import cgi
-from datetime import datetime
 import threading
 import xmlrpclib
 
@@ -22,7 +21,6 @@
     WebServiceRequestTraversal,
     )
 from lazr.uri import URI
-import pytz
 import transaction
 from transaction.interfaces import ISynchronizer
 from zc.zservertracelog.tracelog import Server as ZServerTracelogServer
@@ -50,10 +48,7 @@
     XMLRPCRequest,
     XMLRPCResponse,
     )
-from zope.security.interfaces import (
-    IParticipation,
-    Unauthorized,
-    )
+from zope.security.interfaces import IParticipation
 from zope.security.proxy import (
     isinstance as zope_isinstance,
     removeSecurityProxy,
@@ -68,17 +63,9 @@
     IPrivateApplication,
     IWebServiceApplication,
     )
-from canonical.launchpad.interfaces.oauth import (
-    ClockSkew,
-    IOAuthConsumerSet,
-    IOAuthSignedRequest,
-    NonceAlreadyUsed,
-    TimestampOrderingError,
-    )
 import canonical.launchpad.layers
 from canonical.launchpad.webapp.authentication import (
-    check_oauth_signature,
-    get_oauth_authorization,
+    get_oauth_principal,
     )
 from canonical.launchpad.webapp.authorization import (
     LAUNCHPAD_SECURITY_POLICY_CACHE_KEY,
@@ -93,8 +80,6 @@
     INotificationRequest,
     INotificationResponse,
     IPlacelessAuthUtility,
-    IPlacelessLoginSource,
-    OAuthPermission,
     )
 from canonical.launchpad.webapp.notifications import (
     NotificationList,
@@ -1216,83 +1201,7 @@
         if request_path.startswith("/%s" % web_service_config.path_override):
             return super(WebServicePublication, self).getPrincipal(request)
 
-        # Fetch OAuth authorization information from the request.
-        form = get_oauth_authorization(request)
-
-        consumer_key = form.get('oauth_consumer_key')
-        consumers = getUtility(IOAuthConsumerSet)
-        consumer = consumers.getByKey(consumer_key)
-        token_key = form.get('oauth_token')
-        anonymous_request = (token_key == '')
-
-        if consumer_key is None:
-            # Either the client's OAuth implementation is broken, or
-            # the user is trying to make an unauthenticated request
-            # using wget or another OAuth-ignorant application.
-            # Try to retrieve a consumer based on the User-Agent
-            # header.
-            anonymous_request = True
-            consumer_key = request.getHeader('User-Agent', '')
-            if consumer_key == '':
-                raise Unauthorized(
-                    'Anonymous requests must provide a User-Agent.')
-            consumer = consumers.getByKey(consumer_key)
-
-        if consumer is None:
-            if anonymous_request:
-                # This is the first time anyone has tried to make an
-                # anonymous request using this consumer name (or user
-                # agent). Dynamically create the consumer.
-                #
-                # In the normal website this wouldn't be possible
-                # because GET requests have their transactions rolled
-                # back. But webservice requests always have their
-                # transactions committed so that we can keep track of
-                # the OAuth nonces and prevent replay attacks.
-                if consumer_key == '' or consumer_key is None:
-                    raise Unauthorized("No consumer key specified.")
-                consumer = consumers.new(consumer_key, '')
-            else:
-                # An unknown consumer can never make a non-anonymous
-                # request, because access tokens are registered with a
-                # specific, known consumer.
-                raise Unauthorized('Unknown consumer (%s).' % consumer_key)
-        if anonymous_request:
-            # Skip the OAuth verification step and let the user access the
-            # web service as an unauthenticated user.
-            #
-            # XXX leonardr 2009-12-15 bug=496964: Ideally we'd be
-            # auto-creating a token for the anonymous user the first
-            # time, passing it through the OAuth verification step,
-            # and using it on all subsequent anonymous requests.
-            alsoProvides(request, IOAuthSignedRequest)
-            auth_utility = getUtility(IPlacelessAuthUtility)
-            return auth_utility.unauthenticatedPrincipal()
-        token = consumer.getAccessToken(token_key)
-        if token is None:
-            raise Unauthorized('Unknown access token (%s).' % token_key)
-        nonce = form.get('oauth_nonce')
-        timestamp = form.get('oauth_timestamp')
-        try:
-            token.checkNonceAndTimestamp(nonce, timestamp)
-        except (NonceAlreadyUsed, TimestampOrderingError, ClockSkew), e:
-            raise Unauthorized('Invalid nonce/timestamp: %s' % e)
-        now = datetime.now(pytz.timezone('UTC'))
-        if token.permission == OAuthPermission.UNAUTHORIZED:
-            raise Unauthorized('Unauthorized token (%s).' % token.key)
-        elif token.date_expires is not None and token.date_expires <= now:
-            raise Unauthorized('Expired token (%s).' % token.key)
-        elif not check_oauth_signature(request, consumer, token):
-            raise Unauthorized('Invalid signature.')
-        else:
-            # Everything is fine, let's return the principal.
-            pass
-        alsoProvides(request, IOAuthSignedRequest)
-        principal = getUtility(IPlacelessLoginSource).getPrincipal(
-            token.person.account.id, access_level=token.permission,
-            scope=token.context)
-
-        return principal
+        return get_oauth_principal(request)
 
 
 class LaunchpadWebServiceRequestTraversal(WebServiceRequestTraversal):

=== modified file 'lib/canonical/launchpad/zcml/launchpad.zcml'
--- lib/canonical/launchpad/zcml/launchpad.zcml	2010-08-02 02:23:26 +0000
+++ lib/canonical/launchpad/zcml/launchpad.zcml	2010-09-16 17:26:09 +0000
@@ -266,14 +266,14 @@
       name="+authorize-token"
       class="canonical.launchpad.browser.OAuthAuthorizeTokenView"
       template="../templates/oauth-authorize.pt"
-      permission="launchpad.AnyPerson" />
+      permission="zope.Public" />
 
   <browser:page
       for="canonical.launchpad.webapp.interfaces.ILaunchpadApplication"
       name="+token-authorized"
       class="canonical.launchpad.browser.OAuthTokenAuthorizedView"
       template="../templates/token-authorized.pt"
-      permission="launchpad.AnyPerson" />
+      permission="zope.Public" />
 
   <browser:page
       for="canonical.launchpad.webapp.interfaces.ILaunchpadApplication"

=== modified file 'lib/lp/testing/__init__.py'
--- lib/lp/testing/__init__.py	2010-09-09 17:02:33 +0000
+++ lib/lp/testing/__init__.py	2010-09-16 17:26:09 +0000
@@ -28,6 +28,7 @@
     'map_branch_contents',
     'normalize_whitespace',
     'oauth_access_token_for',
+    'OAuthSigningBrowser',
     'person_logged_in',
     'record_statements',
     'run_with_login',
@@ -144,6 +145,7 @@
     launchpadlib_credentials_for,
     launchpadlib_for,
     oauth_access_token_for,
+    OAuthSigningBrowser,
     )
 from lp.testing.fixture import ZopeEventHandlerFixture
 from lp.testing.matchers import Provides
@@ -221,7 +223,7 @@
 
 class StormStatementRecorder:
     """A storm tracer to count queries.
-    
+
     This exposes the count and queries as lp.testing._webservice.QueryCollector
     does permitting its use with the HasQueryCount matcher.
 
@@ -680,6 +682,7 @@
     def assertTextMatchesExpressionIgnoreWhitespace(self,
                                                     regular_expression_txt,
                                                     text):
+
         def normalise_whitespace(text):
             return ' '.join(text.split())
         pattern = re.compile(
@@ -856,6 +859,7 @@
         callable, and events are the events emitted by the callable.
     """
     events = []
+
     def on_notify(event):
         events.append(event)
     old_subscribers = zope.event.subscribers[:]

=== modified file 'lib/lp/testing/_webservice.py'
--- lib/lp/testing/_webservice.py	2010-08-20 20:31:18 +0000
+++ lib/lp/testing/_webservice.py	2010-09-16 17:26:09 +0000
@@ -9,34 +9,104 @@
     'launchpadlib_credentials_for',
     'launchpadlib_for',
     'oauth_access_token_for',
+    'OAuthSigningBrowser',
     ]
 
 
 import shutil
 import tempfile
-
-from launchpadlib.credentials import (
-    AccessToken,
-    Credentials,
-    )
-from launchpadlib.launchpad import Launchpad
 import transaction
+from urllib2 import BaseHandler
+
+from oauth.oauth import OAuthRequest, OAuthSignatureMethod_PLAINTEXT
+
 from zope.app.publication.interfaces import IEndRequestEvent
 from zope.app.testing import ztapi
+from zope.testbrowser.testing import Browser
 from zope.component import getUtility
 import zope.testing.cleanup
 
+from launchpadlib.credentials import (
+    AccessToken,
+    Credentials,
+    )
+from launchpadlib.launchpad import Launchpad
+
+from lp.testing._login import (
+    login,
+    logout,
+    )
+
 from canonical.launchpad.interfaces import (
     IOAuthConsumerSet,
     IPersonSet,
+    OAUTH_REALM,
     )
 from canonical.launchpad.webapp.adapter import get_request_statements
 from canonical.launchpad.webapp.interaction import ANONYMOUS
 from canonical.launchpad.webapp.interfaces import OAuthPermission
-from lp.testing._login import (
-    login,
-    logout,
-    )
+
+
+class OAuthSigningHandler(BaseHandler):
+    """A urllib2 handler that signs requests with an OAuth token."""
+
+    def __init__(self, consumer, token):
+        """Constructor
+
+        :param consumer: An OAuth consumer.
+        :param token: An OAuth token.
+        """
+        self.consumer = consumer
+        self.token = token
+
+    def default_open(self, req):
+        """Set the Authorization header for the outgoing request."""
+        signer = OAuthRequest.from_consumer_and_token(
+            self.consumer, self.token)
+        signer.sign_request(
+            OAuthSignatureMethod_PLAINTEXT(), self.consumer, self.token)
+        auth_header = signer.to_header(OAUTH_REALM)['Authorization']
+        req.headers['Authorization'] = auth_header
+
+
+class UserAgentFilteringHandler(BaseHandler):
+    """A urllib2 handler that replaces the User-Agent header.
+
+    [XXX bug=638058] This is a hack to work around a bug in
+    zope.testbrowser.
+    """
+    def __init__(self, user_agent):
+        """Constructor."""
+        self.user_agent = user_agent
+
+    def default_open(self, req):
+        """Set the User-Agent header for the outgoing request."""
+        req.headers['User-Agent'] = self.user_agent
+
+
+class OAuthSigningBrowser(Browser):
+    """A browser that signs each outgoing request with an OAuth token.
+
+    This lets us simulate the behavior of the Launchpad Credentials
+    Manager.
+    """
+    def __init__(self, consumer, token, user_agent=None):
+        """Constructor.
+
+        :param consumer: An OAuth consumer.
+        :param token: An OAuth token.
+        :param user_agent: The User-Agent string to send.
+        """
+        super(OAuthSigningBrowser, self).__init__()
+        self.mech_browser.add_handler(
+            OAuthSigningHandler(consumer, token))
+        if user_agent is not None:
+            self.mech_browser.add_handler(
+                UserAgentFilteringHandler(user_agent))
+
+        # This will give us tracebacks instead of unhelpful error
+        # messages.
+        self.handleErrors = False
 
 
 def oauth_access_token_for(consumer_name, person, permission, context=None):