Merge lp:~leonardr/launchpad/true-anonymous-access into lp:launchpad/db-devel

[Leonard Richardson]

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.)

[Martin Pool (mbp)]

That looks good to me, though I'm not very expert on Launchpad's security internals.

> 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.)

I think it would be reasonable to handle that as a separate bug if and when anyone actually files it. If the OAuth header is corrupt as opposed to just absent I think we should still give an error.

Did you interactively test that against your dev instance?

Thanks very much, this is a nice step forward for API usability.

review: Approve

[Leonard Richardson (leonardr)]

I did test this branch interactively against my dev instance, using a web browser and wget.

I'd like salgado to take a quick look from a security standpoint and then I'll land it.

[Gary Poster (gary)]

Looks good to me.

I don't see how this changes the security profile, so I'm +1 on landing it. I'm certainly happy to get another review, though.

review: Approve

[Guilherme Salgado (salgado)]

<salgado> leonardr, it looks fine to me, but an unrelated thing is that it is not clear to me that consumers.getByKey() may create a new token as the comment states

review: Approve

=== 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-19 14:51:33 +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-19 14:51:33 +0000
@@ -1199,22 +1199,33 @@
         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 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
-                # name. Dynamically create the consumer.
+                # 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