Launchpad itself

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

  1. true-anonymous-access
  2. Merge into db-devel
Proposed by Leonard Richardson
Status: Merged
Merged at revision: 9552
Proposed branch: lp:~leonardr/launchpad/true-anonymous-access
Merge into: lp:launchpad/db-devel
Diff against target: 141 lines (+88/-9)
2 files modified
lib/canonical/launchpad/pagetests/webservice/xx-service.txt (+70/-2)
lib/canonical/launchpad/webapp/servers.py (+18/-7)
To merge this branch: bzr merge lp:~leonardr/launchpad/true-anonymous-access
Reviewer Review Type Date Requested Status
Guilherme Salgado (community) Approve
Gary Poster (community) Approve
Martin Pool (community) Approve
Review via email: mp+30088@code.launchpad.net

Description of the change

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

To post a comment you must log in.
Revision history for this message
Martin Pool (mbp) wrote :

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
Revision history for this message
Leonard Richardson (leonardr) wrote :

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.

Revision history for this message
Gary Poster (gary) wrote :

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
Revision history for this message
Guilherme Salgado (salgado) wrote :

<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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
156 (<Person at...>, 'displayname', 'launchpad.Edit')156 (<Person at...>, 'displayname', 'launchpad.Edit')
157 ...157 ...
158158
159A completely unsigned web service request is treated as an anonymous
160request, with the OAuth consumer name being equal to the User-Agent.
161
162 >>> agent = "unsigned-user-agent"
163 >>> login(ANONYMOUS)
164 >>> print consumer_set.getByKey(agent)
165 None
166 >>> logout()
167
168 >>> from zope.app.testing.functional import HTTPCaller
169 >>> def request_with_user_agent(agent, url="/devel"):
170 ... if agent is None:
171 ... agent_string = ''
172 ... else:
173 ... agent_string = '\nUser-Agent: %s' % agent
174 ... http = HTTPCaller()
175 ... request = ("GET %s HTTP/1.1\n"
176 ... "Host: api.launchpad.dev"
177 ... "%s\n\n") % (url, agent_string)
178 ... return http(request)
179
180 >>> response = request_with_user_agent(agent)
181 >>> print response.getOutput()
182 HTTP/1.1 200 Ok
183 ...
184 {...}
185
186Here, too, the OAuth consumer name is automatically registered if it
187doesn't exist.
188
189 >>> login(ANONYMOUS)
190 >>> print consumer_set.getByKey(agent).key
191 unsigned-user-agent
192 >>> logout()
193
194Here's another request now that the User-Agent has been registered as
195a consumer name.
196
197 >>> response = request_with_user_agent(agent)
198 >>> print response.getOutput()
199 HTTP/1.1 200 Ok
200 ...
201 {...}
202
203An unsigned request, like a request signed with the empty string,
204isn't logged in as any particular user:
205
206 >>> response = request_with_user_agent(agent, "/devel/people/+me")
207 >>> print response.getOutput()
208 HTTP/1.1 401 Unauthorized
209 ...
210 Unauthorized: You need to be logged in to view this URL.
211
212An empty or missing user agent results in an error.
213
214 >>> response = request_with_user_agent(' ')
215 >>> print response.getOutput()
216 HTTP/1.1 401 Unauthorized
217 ...
218 Unauthorized: Anonymous requests must provide a User-Agent.
219
220 >>> response = request_with_user_agent(None)
221 >>> print response.getOutput()
222 HTTP/1.1 401 Unauthorized
223 ...
224 Unauthorized: Anonymous requests must provide a User-Agent.
225
159API Requests to other hosts226API Requests to other hosts
160===========================227===========================
161228
@@ -200,7 +267,8 @@
200 ...267 ...
201268
202But the regular authentication still doesn't work on the normal API269But the regular authentication still doesn't work on the normal API
203virtual host:270virtual host: an attempt to do HTTP Basic Auth will be treated as an
271anonymous request.
204272
205 >>> noauth_webservice.domain = 'api.launchpad.dev'273 >>> noauth_webservice.domain = 'api.launchpad.dev'
206 >>> print noauth_webservice.get(274 >>> print noauth_webservice.get(
@@ -208,7 +276,7 @@
208 ... headers={'Authorization': sample_auth})276 ... headers={'Authorization': sample_auth})
209 HTTP/1.1 401 Unauthorized277 HTTP/1.1 401 Unauthorized
210 ...278 ...
211 Request is missing an OAuth consumer key.279 Unauthorized: Anonymous requests must provide a User-Agent.
212280
213281
214The 'Vary' Header282The 'Vary' Header
215283
=== 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 @@
1199 consumer = consumers.getByKey(consumer_key)1199 consumer = consumers.getByKey(consumer_key)
1200 token_key = form.get('oauth_token')1200 token_key = form.get('oauth_token')
1201 anonymous_request = (token_key == '')1201 anonymous_request = (token_key == '')
1202
1203 if consumer_key is None:
1204 # Either the client's OAuth implementation is broken, or
1205 # the user is trying to make an unauthenticated request
1206 # using wget or another OAuth-ignorant application.
1207 # Try to retrieve a consumer based on the User-Agent
1208 # header.
1209 anonymous_request = True
1210 consumer_key = request.getHeader('User-Agent', '')
1211 if consumer_key == '':
1212 raise Unauthorized(
1213 'Anonymous requests must provide a User-Agent.')
1214 consumer = consumers.getByKey(consumer_key)
1215
1202 if consumer is None:1216 if consumer is None:
1203 if consumer_key is None:
1204 # Most likely the user is trying to make a totally
1205 # unauthenticated request.
1206 raise Unauthorized(
1207 'Request is missing an OAuth consumer key.')
1208 if anonymous_request:1217 if anonymous_request:
1209 # This is the first time anyone has tried to make an1218 # This is the first time anyone has tried to make an
1210 # anonymous request using this consumer1219 # anonymous request using this consumer name (or user
1211 # name. Dynamically create the consumer.1220 # agent). Dynamically create the consumer.
1212 #1221 #
1213 # In the normal website this wouldn't be possible1222 # In the normal website this wouldn't be possible
1214 # because GET requests have their transactions rolled1223 # because GET requests have their transactions rolled
1215 # back. But webservice requests always have their1224 # back. But webservice requests always have their
1216 # transactions committed so that we can keep track of1225 # transactions committed so that we can keep track of
1217 # the OAuth nonces and prevent replay attacks.1226 # the OAuth nonces and prevent replay attacks.
1227 if consumer_key == '' or consumer_key is None:
1228 raise Unauthorized("No consumer key specified.")
1218 consumer = consumers.new(consumer_key, '')1229 consumer = consumers.new(consumer_key, '')
1219 else:1230 else:
1220 # An unknown consumer can never make a non-anonymous1231 # An unknown consumer can never make a non-anonymous

Subscribers

People subscribed via source and target branches

to status/vote changes: