launchpad-reviewers team mailing list archive

[Leonard Richardson <leonard.richardson@xxxxxxxxxxxxx>]

Leonard Richardson has proposed merging lp:~leonardr/launchpad/true-anonymous-access into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)


This branch makes it possible to get read-only access to the Launchpad web service (the actual one, at api.launchpad.net, not the one for Ajax apps) using an OAuth-ignorant client like wget or a web browser. Internally, the User-Agent string is treated as the "consumer key", and anonymous read-only access is allowed on the same terms as we currently allow OAuth requests signed with a token that is the empty string.

Although I'm confident there are none, I would like the reviewer to take a close look for possible security problems such as privilege escalation attacks within getPrincipal().

My real problem with this branch is that there is no longer a real failure mode for web service implementations. If you screw up your OAuth implementation, or you try to authenticate with Basic Auth and your Launchpad username/password, you will no longer get an exception. You'll get anonymous read-only access. But, I don't think it's worth making a big deal of this, trying to get failure modes back by checking for incomplete OAuth implementations, etc. (At most, I might check for an Authorization header that's not a valid OAuth Authorization header.)
-- 
https://code.launchpad.net/~leonardr/launchpad/true-anonymous-access/+merge/30088
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~leonardr/launchpad/true-anonymous-access into lp:launchpad.

=== modified file 'lib/canonical/launchpad/pagetests/webservice/xx-service.txt'
--- lib/canonical/launchpad/pagetests/webservice/xx-service.txt	2010-04-27 15:56:20 +0000
+++ lib/canonical/launchpad/pagetests/webservice/xx-service.txt	2010-07-16 11:02:44 +0000
@@ -156,6 +156,73 @@
     (<Person at...>, 'displayname', 'launchpad.Edit')
     ...
 
+A completely unsigned web service request is treated as an anonymous
+request, with the OAuth consumer name being equal to the User-Agent.
+
+    >>> agent = "unsigned-user-agent"
+    >>> login(ANONYMOUS)
+    >>> print consumer_set.getByKey(agent)
+    None
+    >>> logout()
+
+    >>> from zope.app.testing.functional import HTTPCaller
+    >>> def request_with_user_agent(agent, url="/devel"):
+    ...     if agent is None:
+    ...         agent_string = ''
+    ...     else:
+    ...         agent_string = '\nUser-Agent: %s' % agent
+    ...     http = HTTPCaller()
+    ...     request = ("GET %s HTTP/1.1\n"
+    ...                "Host: api.launchpad.dev"
+    ...                "%s\n\n") % (url, agent_string)
+    ...     return http(request)
+
+    >>> response = request_with_user_agent(agent)
+    >>> print response.getOutput()
+    HTTP/1.1 200 Ok
+    ...
+    {...}
+
+Here, too, the OAuth consumer name is automatically registered if it
+doesn't exist.
+
+    >>> login(ANONYMOUS)
+    >>> print consumer_set.getByKey(agent).key
+    unsigned-user-agent
+    >>> logout()
+
+Here's another request now that the User-Agent has been registered as
+a consumer name.
+
+    >>> response = request_with_user_agent(agent)
+    >>> print response.getOutput()
+    HTTP/1.1 200 Ok
+    ...
+    {...}
+
+An unsigned request, like a request signed with the empty string,
+isn't logged in as any particular user:
+
+    >>> response = request_with_user_agent(agent, "/devel/people/+me")
+    >>> print response.getOutput()
+    HTTP/1.1 401 Unauthorized
+    ...
+    Unauthorized: You need to be logged in to view this URL.
+
+An empty or missing user agent results in an error.
+
+    >>> response = request_with_user_agent(' ')
+    >>> print response.getOutput()
+    HTTP/1.1 401 Unauthorized
+    ...
+    Unauthorized: Anonymous requests must provide a User-Agent.
+
+    >>> response = request_with_user_agent(None)
+    >>> print response.getOutput()
+    HTTP/1.1 401 Unauthorized
+    ...
+    Unauthorized: Anonymous requests must provide a User-Agent.
+
 API Requests to other hosts
 ===========================
 
@@ -200,7 +267,8 @@
     ...
 
 But the regular authentication still doesn't work on the normal API
-virtual host:
+virtual host: an attempt to do HTTP Basic Auth will be treated as an
+anonymous request.
 
     >>> noauth_webservice.domain = 'api.launchpad.dev'
     >>> print noauth_webservice.get(
@@ -208,7 +276,7 @@
     ...     headers={'Authorization': sample_auth})
     HTTP/1.1 401 Unauthorized
     ...
-    Request is missing an OAuth consumer key.
+    Unauthorized: Anonymous requests must provide a User-Agent.
 
 
 The 'Vary' Header

=== modified file 'lib/canonical/launchpad/webapp/servers.py'
--- lib/canonical/launchpad/webapp/servers.py	2010-04-27 15:56:20 +0000
+++ lib/canonical/launchpad/webapp/servers.py	2010-07-16 11:02:44 +0000
@@ -1199,12 +1199,21 @@
         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.
+            # Create or 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 consumer_key is None:
-                # Most likely the user is trying to make a totally
-                # unauthenticated request.
-                raise Unauthorized(
-                    'Request is missing an OAuth consumer key.')
             if anonymous_request:
                 # This is the first time anyone has tried to make an
                 # anonymous request using this consumer
@@ -1215,6 +1224,8 @@
                 # 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