Merge lp:~james-w/lava-dashboard/oauth into lp:lava-dashboard
Status: | Rejected |
---|---|
Rejected by: | Zygmunt Krynicki |
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 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Linaro Infrastructure | 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
Unmerged revisions
- 73. By James Westby
-
Allow request.user to be missing.
- 72. By James Westby
-
Add a middleware on top of django-oauth that puts the user on the request as normal.
Oh, and django-oauth's tests are not isolated, so running all the tests
in the project currently fails.
Thanks,
James