Merge lp:~james-w/lava-dashboard/oauth into lp:lava-dashboard

Proposed by James Westby on 2010-10-08
Status: Rejected
Rejected by: Zygmunt Krynicki on 2011-11-09
Proposed branch: lp:~james-w/lava-dashboard/oauth
Merge into: lp:lava-dashboard
Diff against target: 431 lines (+400/-0)
3 files modified
dashboard_server/default_settings.py (+3/-0)
oauth_middleware/__init__.py (+90/-0)
oauth_middleware/tests.py (+307/-0)
To merge this branch: bzr merge lp:~james-w/lava-dashboard/oauth
Reviewer Review Type Date Requested Status
Linaro Infrastructure 2010-10-08 Pending
Review via email: mp+38013@code.launchpad.net

Description of the change

Hi,

Another inital code push for discussion, this time adding oauth support.

I looked at the existing django apps for doing this, and didn't really
like the approach of any of them. django-oauth requires separate decorators
on views that can and should accept oauth signed requests. django-piston
is closer I think, but implies lots of other things that don't really
fit in to the current architecture. If we want to move towards something
like django-piston provides then we should drop this and go that way I think.

The approach I took is to provide a middleware that sets request.user
if the request is oauth signed. This means that django's existing auth
framework continues to work, and you can just use it's existing decorators
etc. to do access control, and use request.user to know who is making
a request.

There are some issues though:

1. This relies on an external project that is unpackaged at this time.
2. That external project ships a patched embedded copy of python-oauth, though
I don't know what the patches are for.
3. That project requires consumers to be pre-registered, and I'm not sure
we want that. It would be possible to work around it, but would require
some work.
4. I'm not sure I have the Resource stuff correct in this branch.
5. I'm not convinced that I have thought through all the corners and so there
may be security holes.
6. There is nothing so far for the view to know if the request is oauth,
which consumer it is etc., and no support for differing token access levels.
We won't need any of that right away, but if we want that then django-piston
may be the way to go rather than adding all of that.

Thanks,

James

To post a comment you must log in.
James Westby (james-w) wrote :

Oh, and django-oauth's tests are not isolated, so running all the tests
in the project currently fails.

Thanks,

James

Zygmunt Krynicki (zyga) wrote :

This branch could be re-targeted at linaro-django-xmlrpc. If we are going to do oauth (eventually we might) then that's the logical place it should go to.

Unmerged revisions

73. By James Westby on 2010-10-08

Allow request.user to be missing.

72. By James Westby on 2010-10-08

Add a middleware on top of django-oauth that puts the user on the request as normal.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'dashboard_server/default_settings.py'
2--- dashboard_server/default_settings.py 2010-10-02 21:25:06 +0000
3+++ dashboard_server/default_settings.py 2010-10-08 19:37:59 +0000
4@@ -103,6 +103,7 @@
5 'django.middleware.common.CommonMiddleware',
6 'django.contrib.sessions.middleware.SessionMiddleware',
7 'django.contrib.auth.middleware.AuthenticationMiddleware',
8+ 'oauth_middleware.OAuthMiddleware',
9 'django.middleware.transaction.TransactionMiddleware',
10 )
11
12@@ -129,6 +130,8 @@
13 'django.contrib.sites',
14 'django.contrib.databrowse',
15 'django.contrib.humanize',
16+ 'oauth_provider',
17+ 'oauth_middleware',
18 'dashboard_app',
19 )
20
21
22=== added directory 'oauth_middleware'
23=== added file 'oauth_middleware/__init__.py'
24--- oauth_middleware/__init__.py 1970-01-01 00:00:00 +0000
25+++ oauth_middleware/__init__.py 2010-10-08 19:37:59 +0000
26@@ -0,0 +1,90 @@
27+from django.conf import settings
28+from django.http import HttpResponseForbidden
29+
30+from oauth.oauth import (
31+ OAuthError,
32+ OAuthRequest,
33+ OAuthServer,
34+ OAuthSignatureMethod_PLAINTEXT,
35+ OAuthSignatureMethod_HMAC_SHA1,
36+ )
37+
38+from oauth_provider.decorators import CheckOAuth
39+from oauth_provider.stores import DataStore as _DataStore
40+
41+
42+def get_oauth_auth_params(request):
43+ # Django converts Authorization header in HTTP_AUTHORIZATION
44+ auth_header = {}
45+ if 'Authorization' in request.META:
46+ auth_header = {'Authorization': request.META['Authorization']}
47+ elif 'HTTP_AUTHORIZATION' in request.META:
48+ auth_header = {'Authorization': request.META['HTTP_AUTHORIZATION']}
49+ # Don't include extra parameters when request.method is POST and
50+ # request.MIME['CONTENT_TYPE'] is "application/x-www-form-urlencoded"
51+ # (See http://oauth.net/core/1.0a/#consumer_req_param).
52+ # But there is an issue with Django's test Client and custom content types
53+ # so an ugly test is made here, if you find a better solution...
54+ parameters = {}
55+ if request.method == "POST" and \
56+ (request.META.get('CONTENT_TYPE') == "application/x-www-form-urlencoded" \
57+ or request.META.get('SERVER_NAME') == 'testserver'):
58+ parameters = dict(request.REQUEST.items())
59+ return {"method": request.method,
60+ "uri": request.build_absolute_uri(),
61+ "auth_header": auth_header,
62+ "parameters": parameters,
63+ "query_string": request.META.get('QUERY_STRING', ''),
64+ }
65+
66+
67+class DataStore(_DataStore):
68+
69+ def lookup_consumer(self, key):
70+ # Do something here to allow arbitrary consumers based on a
71+ # settings key. Also need to override other methods in that case
72+ # to avoid key checks
73+ return super(DataStore, self).lookup_consumer(key)
74+
75+
76+def authenticate(method=None, uri=None, auth_header=None,
77+ parameters=None, query_string=None):
78+ oauth_request = OAuthRequest.from_request(method,
79+ uri,
80+ headers=auth_header,
81+ parameters=parameters,
82+ query_string=query_string)
83+ if oauth_request is None:
84+ # Do we want to 403 here, it's half an oauth signed request?
85+ return None
86+ OAUTH_SIGNATURE_METHODS = getattr(settings, 'OAUTH_SIGNATURE_METHODS', ['plaintext', 'hmac-sha1'])
87+ oauth_server = OAuthServer(DataStore(oauth_request))
88+ if 'plaintext' in OAUTH_SIGNATURE_METHODS:
89+ oauth_server.add_signature_method(OAuthSignatureMethod_PLAINTEXT())
90+ if 'hmac-sha1' in OAUTH_SIGNATURE_METHODS:
91+ oauth_server.add_signature_method(OAuthSignatureMethod_HMAC_SHA1())
92+ consumer, token, parameters = oauth_server.verify_request(oauth_request)
93+ # Handle resource names?
94+ return token.user
95+
96+
97+class OAuthMiddleware(object):
98+
99+ def process_request(self, request):
100+ oauth_auth_params = get_oauth_auth_params(request)
101+ try:
102+ user = authenticate(**oauth_auth_params)
103+ except OAuthError, e:
104+ return HttpResponseForbidden(content=str(e))
105+ if user:
106+ # If the user is already authenticated and that user doesn't
107+ # match the one that the headers are for then something
108+ # is wrong, so give an Unauthenticated response
109+ if getattr(request, "user", None) and request.user.is_authenticated():
110+ if request.user != user:
111+ return HttpResponseForbidden()
112+ # User is valid. Overwrite request.user with the user that
113+ # signed the request, which will prevent any
114+ # django.contrib.auth checks, and so sessions will be
115+ # invalid.
116+ request.user = user
117
118=== added file 'oauth_middleware/models.py'
119=== added file 'oauth_middleware/tests.py'
120--- oauth_middleware/tests.py 1970-01-01 00:00:00 +0000
121+++ oauth_middleware/tests.py 2010-10-08 19:37:59 +0000
122@@ -0,0 +1,307 @@
123+import time
124+
125+from django.conf import settings
126+from django.contrib.auth.models import AnonymousUser, User
127+from django.http import HttpRequest
128+from django.test import TestCase
129+
130+from oauth.oauth import (
131+ OAuthError,
132+ OAuthRequest,
133+ OAuthSignatureMethod_PLAINTEXT,
134+ )
135+from oauth_provider.models import (
136+ Consumer,
137+ Resource,
138+ Token,
139+ )
140+
141+from oauth_middleware import (
142+ authenticate,
143+ get_oauth_auth_params,
144+ OAuthMiddleware,
145+ )
146+
147+
148+class TestRequest(HttpRequest):
149+
150+ def __init__(self, params=None):
151+ super(TestRequest, self).__init__()
152+ self.META["HTTP_HOST"] = "foo"
153+ self._params = params or {}
154+
155+ def _get_request(self):
156+ return self._params
157+
158+ REQUEST = property(_get_request)
159+
160+
161+class GetOAuthAuthParamsTests(TestCase):
162+
163+ def get_request(self, params=None):
164+ request = TestRequest(params=params)
165+ return request
166+
167+ def test_empty_auth_header_by_default(self):
168+ request = self.get_request()
169+ params = get_oauth_auth_params(request)
170+ self.assertEqual({}, params["auth_header"])
171+
172+ def test_auth_header_from_Authorization(self):
173+ request = self.get_request()
174+ request.META["Authorization"] = "foo"
175+ params = get_oauth_auth_params(request)
176+ self.assertEqual({"Authorization": "foo"}, params["auth_header"])
177+
178+ def test_auth_header_from_Http_Authorization(self):
179+ request = self.get_request()
180+ request.META["HTTP_AUTHORIZATION"] = "bar"
181+ params = get_oauth_auth_params(request)
182+ self.assertEqual({"Authorization": "bar"}, params["auth_header"])
183+
184+ def test_auth_header_from_Authorization_not_Http_Auth(self):
185+ request = self.get_request()
186+ request.META["Authorization"] = "foo"
187+ request.META["HTTP_AUTHORIZATION"] = "bar"
188+ params = get_oauth_auth_params(request)
189+ self.assertEqual({"Authorization": "foo"}, params["auth_header"])
190+
191+ def test_parameters_empty_by_default(self):
192+ request = self.get_request()
193+ params = get_oauth_auth_params(request)
194+ self.assertEqual({}, params["parameters"])
195+
196+ def test_parameters_with_form_post(self):
197+ params = {"foo": "bar"}
198+ request = self.get_request(params=params)
199+ request.method = "POST"
200+ request.META["CONTENT_TYPE"] = "application/x-www-form-urlencoded"
201+ oauth_params = get_oauth_auth_params(request)
202+ self.assertEqual(params, oauth_params["parameters"])
203+
204+ def test_parameters_with_post_to_test_server(self):
205+ params = {"foo": "bar"}
206+ request = self.get_request(params=params)
207+ request.method = "POST"
208+ request.META["SERVER_NAME"] = "testserver"
209+ oauth_params = get_oauth_auth_params(request)
210+ self.assertEqual(params, oauth_params["parameters"])
211+
212+ def test_parameters_includes_method(self):
213+ request = self.get_request()
214+ request.method = "PATCH"
215+ oauth_params = get_oauth_auth_params(request)
216+ self.assertEqual("PATCH", oauth_params["method"])
217+
218+ def test_parameters_includes_absolute_uri(self):
219+ request = self.get_request()
220+ oauth_params = get_oauth_auth_params(request)
221+ self.assertEqual(request.build_absolute_uri(), oauth_params["uri"])
222+
223+ def test_parameters_includes_empty_query_string_by_default(self):
224+ request = self.get_request()
225+ oauth_params = get_oauth_auth_params(request)
226+ self.assertEqual('', oauth_params["query_string"])
227+
228+ def test_parameters_includes_query_string(self):
229+ query_string = "?foo=bar"
230+ request = self.get_request()
231+ request.META["QUERY_STRING"] = query_string
232+ oauth_params = get_oauth_auth_params(request)
233+ self.assertEqual(query_string, oauth_params["query_string"])
234+
235+
236+class OAuthTestCase(TestCase):
237+
238+ def setUp(self):
239+ self._original_signature_methods = getattr(
240+ settings, 'OAUTH_SIGNATURE_METHODS', None)
241+ super(OAuthTestCase, self).setUp()
242+
243+ def tearDown(self):
244+ super(OAuthTestCase, self).tearDown()
245+ if self._original_signature_methods is not None:
246+ setattr(
247+ settings, 'OAUTH_SIGNATURE_METHODS',
248+ self._original_signature_methods)
249+ elif getattr(settings, 'OAUTH_SIGNATURE_METHODS', None) is not None:
250+ delattr(settings, 'OAUTH_SIGNATURE_METHODS')
251+
252+ def get_user(self, username=None):
253+ if username is None:
254+ username = "test-user"
255+ user = User.objects.create(username=username)
256+ user.save()
257+ return user
258+
259+ def get_consumer(self):
260+ return Consumer.objects.create_consumer("consumer")
261+
262+ def get_resource(self):
263+ return Resource.objects.create(name="foo", url="bar")
264+
265+ def get_token(self, user, consumer, resource):
266+ return Token.objects.create_token(
267+ consumer, Token.ACCESS, time.time(), resource,
268+ user=user)
269+
270+ def get_request(self, consumer, token, method, uri):
271+ request = OAuthRequest.from_consumer_and_token(
272+ consumer, token=token, http_method=method, http_url=uri)
273+ request.sign_request(
274+ OAuthSignatureMethod_PLAINTEXT(), consumer, token)
275+ return request
276+
277+ def get_signed_request(self, user, method, uri):
278+ consumer = self.get_consumer()
279+ resource = self.get_resource()
280+ token = self.get_token(user, consumer, resource)
281+ request = OAuthRequest.from_consumer_and_token(
282+ consumer, token=token, http_method=method, http_url=uri)
283+ request.sign_request(
284+ OAuthSignatureMethod_PLAINTEXT(), consumer, token)
285+ return request
286+
287+
288+class AuthenticateTests(OAuthTestCase):
289+
290+ def test_invalid_oauth_request_returns_None(self):
291+ user = authenticate(uri="http://example.com")
292+ self.assertEqual(None, user)
293+
294+ def test_invalid_oauth_params_raise_error(self):
295+ self.assertRaises(
296+ OAuthError, authenticate, uri="http://example.com",
297+ auth_header={"Authorization": "OAuth foo"})
298+
299+ def test_authenticate_returns_user(self):
300+ user = self.get_user()
301+ method = "POST"
302+ uri = "http://example.com"
303+ request = self.get_signed_request(user, method, uri)
304+ self.assertEqual(
305+ user,
306+ authenticate(
307+ method=method, uri=uri,
308+ auth_header=request.to_header()))
309+
310+ def test_authenticate_refuses_unwanted_methods(self):
311+ settings.OAUTH_SIGNATURE_METHODS = ["hmac-sha1"]
312+ user = self.get_user()
313+ method = "POST"
314+ uri = "http://example.com"
315+ request = self.get_signed_request(user, method, uri)
316+ self.assertRaises(
317+ OAuthError, authenticate, method=method, uri=uri,
318+ auth_header=request.to_header())
319+
320+ def test_authenticate_refuses_bad_signature(self):
321+ user = self.get_user()
322+ method = "POST"
323+ uri = "http://example.com"
324+ request = self.get_signed_request(user, method, uri)
325+ request.set_parameter('oauth_signature', '000')
326+ self.assertRaises(
327+ OAuthError, authenticate, method=method, uri=uri,
328+ auth_header=request.to_header())
329+
330+ def test_authenticate_refuses_unknown_token(self):
331+ user = self.get_user()
332+ consumer = self.get_consumer()
333+ resource = self.get_resource()
334+ token = self.get_token(user, consumer, resource)
335+ method = "POST"
336+ uri = "http://example.com"
337+ request = self.get_request(consumer, token, method, uri)
338+ request.set_parameter('oauth_signature', '000')
339+ token.delete()
340+ self.assertRaises(
341+ OAuthError, authenticate, method=method, uri=uri,
342+ auth_header=request.to_header())
343+
344+ def test_authenticate_refuses_unknown_user(self):
345+ user = self.get_user()
346+ consumer = self.get_consumer()
347+ resource = self.get_resource()
348+ token = self.get_token(user, consumer, resource)
349+ method = "POST"
350+ uri = "http://example.com"
351+ request = self.get_request(consumer, token, method, uri)
352+ request.set_parameter('oauth_signature', '000')
353+ user.delete()
354+ self.assertRaises(
355+ OAuthError, authenticate, method=method, uri=uri,
356+ auth_header=request.to_header())
357+
358+
359+class OAuthMiddlewareTests(OAuthTestCase):
360+
361+ def test_nothing_happens_on_non_oauth_requests(self):
362+ request = TestRequest()
363+ request.user = "user"
364+ middleware = OAuthMiddleware()
365+ response = middleware.process_request(request)
366+ self.assertEqual("user", request.user)
367+ self.assertEqual(None, response)
368+
369+ def test_forbidden_on_bad_oauth_request(self):
370+ request = TestRequest()
371+ request.user = "user"
372+ request.META["HTTP_AUTHORIZATION"] = (
373+ "OAuth oauth_consumer_key=foo oauth_token=bar "
374+ "oauth_signature=baz oauth_signature_method=zap "
375+ "oauth_timestamp=zabg oauth_nonce=zot")
376+ middleware = OAuthMiddleware()
377+ response = middleware.process_request(request)
378+ self.assertEqual(403, response.status_code)
379+
380+ def test_authenticated_user_set_on_request(self):
381+ user = self.get_user()
382+ request = TestRequest()
383+ request.user = AnonymousUser()
384+ method = "POST"
385+ uri = "http://example.com"
386+ oauth_request = self.get_signed_request(user, method, uri)
387+ request.META["HTTP_AUTHORIZATION"] = oauth_request.to_header().values()[0]
388+ middleware = OAuthMiddleware()
389+ response = middleware.process_request(request)
390+ self.assertEqual(None, response)
391+ self.assertEqual(user, request.user)
392+
393+ def test_forbidden_if_request_already_authenticated(self):
394+ user = self.get_user(username="test-user")
395+ other_user = self.get_user(username="other-user")
396+ request = TestRequest()
397+ request.user = other_user
398+ method = "POST"
399+ uri = "http://example.com"
400+ oauth_request = self.get_signed_request(user, method, uri)
401+ request.META["HTTP_AUTHORIZATION"] = oauth_request.to_header().values()[0]
402+ middleware = OAuthMiddleware()
403+ response = middleware.process_request(request)
404+ self.assertEqual(403, response.status_code)
405+
406+ def test_not_forbidden_if_same_user_authenticated(self):
407+ user = self.get_user()
408+ request = TestRequest()
409+ request.user = user
410+ method = "POST"
411+ uri = "http://example.com"
412+ oauth_request = self.get_signed_request(user, method, uri)
413+ request.META["HTTP_AUTHORIZATION"] = oauth_request.to_header().values()[0]
414+ middleware = OAuthMiddleware()
415+ response = middleware.process_request(request)
416+ self.assertEqual(None, response)
417+ self.assertEqual(user, request.user)
418+
419+ def test_allows_no_user_on_request(self):
420+ user = self.get_user()
421+ request = TestRequest()
422+ method = "POST"
423+ uri = "http://example.com"
424+ oauth_request = self.get_signed_request(user, method, uri)
425+ request.META["HTTP_AUTHORIZATION"] = oauth_request.to_header().values()[0]
426+ middleware = OAuthMiddleware()
427+ response = middleware.process_request(request)
428+ self.assertEqual(None, response)
429+ self.assertEqual(user, request.user)
430
431=== added file 'oauth_middleware/views.py'

Subscribers

People subscribed via source and target branches