#689 closed enhancement (fixed)
Honor Web server provided authentication
Reported by: | Owned by: | Gary Wilson | |
---|---|---|---|
Component: | Core (Other) | Version: | dev |
Severity: | normal | Keywords: | |
Cc: | telenieko@…, hozer@…, koen.biermans@…, hugh@…, ericvw@…, grf@…, hinnerk@…, me@…, zilingzhao@…, dvd@…, erik.engbrecht@…, David Larlet | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
I would like to see a 2nd mod_python handler. one which takes the REMOTE_USER parameter passed to it and for it to use that as the user-id so that I can hook my app into the default intranet security system we use over here.
here is the patch to the original to make that the case.
NOTE: it creates a user-record if none exist for the user.
Helios:/src/django_newadmin/django/core/handlers ianh$ diff modpython_apacheauth.py modpython.py 102,103c102 < #user_id = self.session[users.SESSION_KEY] < user_id = self._req.user --- > user_id = self.session[users.SESSION_KEY] 106,117c105,106 < try: < self._user = users.get_object(username__exact=user_id) < except (users.UserDoesNotExist): < from django.models.auth import User < import md5 < import datetime < password_md5 = md5.new('fake').hexdigest() < now = datetime.datetime.now() < self._user = User(None, user_id,'','', user_id+'@cnet.com',password_md5,False,True,False,now,now) < self._user.save() < #except (AttributeError, KeyError, ValueError, users.UserDoesNotExist): < except (AttributeError, KeyError, ValueError): --- > self._user = users.get_object(pk=user_id) > except (AttributeError, KeyError, ValueError, users.UserDoesNotExist): 120d108 <
Attachments (19)
Change History (73)
by , 19 years ago
Attachment: | modpython.patch added |
---|
comment:1 by , 19 years ago
Why don't you just write a middleware? That is a lot more appropriate than another handler.
(Oh, and in the future, please create unified diffs (the -u
flag), they are much more readable.)
comment:2 by , 19 years ago
Yep, a middleware that just searches for the user object based on the REMOTE_USER and stores that in request.user should be simpler, I think.
comment:3 by , 19 years ago
I've attached a middleware version of the same process.
I'd be happy if this went into the 'contrib' (or general middleware) section of django
comment:4 by , 19 years ago
Summary: | allow apache to provide authentication instead of django → [patch] middleware to allow apache to provide authentication instead of django |
---|
comment:5 by , 19 years ago
If you're using mod_auth_kerb to authenticate your users against Active Directory, REMOTE_USER will be in the format userid@REALM. My attached variant on Ian's middleware code strips out the @, copes with REMOTE_USER being missing, and only adjusts request.user if necessary.
by , 19 years ago
Attachment: | httpdauth.2.py added |
---|
Revision of Ian's module; new version moves settings to the settings module
comment:6 by , 19 years ago
Resolution: | → duplicate |
---|---|
Status: | new → closed |
Superceded by MultipleAuthBackends page.
comment:7 by , 18 years ago
If I may add - IMHO this particular enhacement cannot be obtained using the new authentication model. The middleware approach is IMO the only way to get this right.
The basic assumption of the http auth is that you log in into the system only once - using the web browser capabilities. Afterwards you should be automaticaly logged in into the system (or at least verified if you can log in).
The new model (http://www.djangoproject.com/documentation/authentication/) is (as far as I know) only able to verify the credentials after they are provided to a login method - at least I couldn't obtain nothing more. So, in my situation, even though I've been logged in "through http" I also had to provide my username and password in the login form.
Moreover - even if I always returned User.objects.get(id=1) inside authenticate() I had to press submit button or the framework left me unauthorized.
As for the "lack of logout" - I think I've managed to deal with that by
class HttpAuthMiddleware(object): def process_request(self, request): if request.path == '/accounts/logout/': httpResponse = HttpResponse() httpResponse.status_code = 401 httpResponse.headers['WWW-Authenticate'] = 'Basic realm="<same realm as in the http server>"' return httpResponse else: # check for http headers - maybe the user is already logged in using http auth ...
Yes, I know - there's a lot of hardcoding, but it works. So, in a sense, the arguments presented in http://www.djangoproject.com/weblog/2005/oct/24/http/ are no longer valid - you *can* logout a user, even if she or he is logged using http auth (inspired by comment made by "Aaron").
...or maybe the docs need some more examples :)
comment:8 by , 18 years ago
Resolution: | duplicate |
---|---|
Status: | closed → reopened |
Summary: | [patch] middleware to allow apache to provide authentication instead of django → Honour web-server provided authentication |
Would be nice if Django honoured REMOTE_USER, specially for the kerberos case so one can have multiple django sites with single sign-on (that is, when you signed in your workstation).
Could that be possible that Django honoured REMOTE_USER leaving authentication to the web server? (that is, apache sends the 401, apache required a user name, apache checks it, and passed it to django). That would be very very nice :)
I saw the ticket is closed as duplicate, but the patch is about to honour apache's authentication efforts (so, respecting REMOTE_USER), the reason for the duplicate is that MultipleAuthBackends superceeds that. I don't think it's true, Multiple Auth Backends allow you to provide other authentication mechanisms to Django, but do not allow you to make django honour REMOTE_USER (as the backend is called from a login page).
So... maybe I get it wrong, but the ticket should be open or wontfix.
Cheers,
Marc.
comment:9 by , 18 years ago
Cc: | added |
---|
comment:10 by , 18 years ago
Triage Stage: | Unreviewed → Design decision needed |
---|
comment:11 by , 18 years ago
Cc: | added |
---|
I'm quite interesting in Django understanding REMOTE_USER provided by apache, as this seems to be my best chance for having a reasonable openid server implementation (using djangoid) that can backend to a kerberos-single sign-on system.
comment:12 by , 17 years ago
Patch needs improvement: | set |
---|---|
Triage Stage: | Design decision needed → Accepted |
This middleware would be appropriate as an optional part of django.contrib.auth, but has some problems (as listed in the patch).
by , 17 years ago
Attachment: | remote_user.diff added |
---|
patch using both middleware and authentication backend (doc included)
comment:13 by , 17 years ago
Cc: | added |
---|---|
Has patch: | set |
The patch uses both middleware and auth backend. This solves the session problems I think. By default it auto-creates new users without any extra info, but you can easily override this by subclassing the backend (see the doc for examples).
An example configuration for apache using mod_auth_sspi (preferred module on Windows) is in the doc.
I have only tested this like this, I have no info about mod_ntlm (best used on Linux).
Don't know how to write tests for it though.
comment:14 by , 17 years ago
Needs tests: | set |
---|
comment:15 by , 17 years ago
Needs tests: | unset |
---|---|
Patch needs improvement: | unset |
New patch attached:
- Removed Apache specific documentation from the previous patch (maybe people want to use this with IIS!) and left only Django specific parts of it.
- The provied backend cannot be used "as-is" to make sure people know what they're doing (or atleast that they read the documentation).
- Added tests to make sure this works and does not break anything else.
For some reason TRAC won't show my nice "git made" diffs (you'll have to download it to see it!) I think TRAC is too clever and does not want SmileyChris to view my patches without testing them!! see that :P
follow-up: 17 comment:16 by , 17 years ago
I had some unsubmitted work on this ticket with some similar documentation generalization/fixes and with fixes to some Python errors, but your English is far better than mine and you've added tests :-). Also, some time ago I asked for some refactoring guidance on -dev in lieu of [6375]: http://groups.google.com/group/django-developers/browse_thread/thread/44104954ebaa219a/4fd066030cf23bfe?hl=en&lnk=gst#4fd066030cf23bfe
because I intended to attach an enhanced patch to this ticket.
I asked again today on #django-sprint and Florian Apolloner ([6357] initial author) suggested the following directions:
<apollo13> cramm: Final statement, all together :) I would change the patch to a) not inherit from ModelBackend b) don't implement the auth methods c) implement authenticate and get_user d) return False if password is not None
He explained d) is security-related. It makes sure no user can login through the usual Django way as admin without entering a password en the event the web server configuration is (accidentally) changed to not send the REMOTE_USER
envvar. I've tried to implemented what I've interpreted from his ideas:
http://ikki.seeonee.rmorales.net/cgi-bin/hgwebdir.cgi/t689/rev/3bf31ada8526
follow-up: 18 comment:17 by , 17 years ago
Patch needs improvement: | set |
---|
Replying to ramiro:
<apollo13> cramm: Final statement, all together :) I would change the patch to a) not inherit from ModelBackend b) don't implement the auth methods c) implement authenticate and get_user d) return False if password is not None
Ok, I didn't see that it was inheritting ModelBackend! ;)
He explained d) is security-related. It makes sure no user can login through the usual Django way as admin without entering a password en the event the web server configuration is (accidentally) changed to not send the
REMOTE_USER
envvar.
I feel it fine.
The only think I do not get clearly is what does "b" mean. I'll take a round on #django-sprint to ask apollo13 myself.
I visited you hgweb, will you post the updated patch youself? If not I'll do it later, I use git anyway :=)
comment:18 by , 17 years ago
Replying to telenieko:
The only think I do not get clearly is what does "b" mean. I'll take a round on #django-sprint to ask apollo13 myself.
B should say: don't implement the get_perm methods (as this backend doesn't deal with them). Don't ask me why I wrote auth...
by , 17 years ago
Attachment: | 689_full-3.diff added |
---|
New version of the patch, incoporates ideas from several people for RemoteUserAuthBackend implementation
comment:19 by , 17 years ago
689_full-3.diff contains:
- Documentation and tests implemented by telenieko
- Modifications to RemoteUserAuthBackend class to
- Not inherit from ModelBackend but expose a minimal, similar enough interface
- Security: Don't allow login as any user w/o password through normal channel when webserver/middleware aren't configured to process auth and to set REMOTE_USER/request.user
Implemented after dicussion with apollo13 and ekarulf on #django-sprint. History available at http://ikki.seeonee.rmorales.net/cgi-bin/hgwebdir.cgi/t689-final/
comment:20 by , 17 years ago
Owner: | changed from | to
---|---|
Status: | reopened → new |
comment:21 by , 17 years ago
Patch needs improvement: | unset |
---|---|
Triage Stage: | Accepted → Ready for checkin |
I think it's ready for checkin now. It passes all the tests and works :)))
follow-up: 23 comment:22 by , 17 years ago
Patch needs improvement: | set |
---|---|
Triage Stage: | Ready for checkin → Accepted |
teleniko: please don't mark tickets ready for checkin without proper review by a ticket triager or a committer. I'll try to provide a complete review, but for now I've got one question: why the forced subclassing of RemoteUserAuthBackend? That strikes me as pointless busywork; this isn't Java!
comment:23 by , 17 years ago
Replying to jacob:
teleniko: please don't mark tickets ready for checkin without proper review by a ticket triager or a committer.
Sorry
I'll try to provide a complete review, but for now I've got one question: why the forced subclassing of RemoteUserAuthBackend? That strikes me as pointless busywork; this isn't Java!
I was afraid about security, the class given assumes that any user given in REMOTE_USER is valid, so making it "not just works" forcing people to take a look at the documentation would make them be aware that this class doesn't do any kind of validation on REMOTE_USER.
Anyway, the functionality of the patch is for some specific needs so maybe the people that are likely to use this are already aware of this problem so removing the RemoteUserAuthBackend.init() method would be fine (and updating the docs accordingly!), I'll post a patch later with this change so you can decide wich one you prefer! ;)
comment:24 by , 17 years ago
Owner: | changed from | to
---|---|
Patch needs improvement: | unset |
Status: | new → assigned |
Here's the new patch as promised! RemoteUserAuthBackend can be used "as-is" now, tests and docs updated accordingly.
Looking at it now maybe it makes more sense this way :)
Hope it's ok!
comment:25 by , 17 years ago
Version: | → SVN |
---|
I'm attaching a new revision of the patch that applies cleanly to r7350 and fixing the django.contrib.auth
tests so both the new unittest-based tests and the previously existing doctests are run (in previous patch revision only the first ones were getting exercised).
by , 17 years ago
Attachment: | t689-r7350.diff added |
---|
comment:26 by , 17 years ago
Cc: | added |
---|
by , 17 years ago
follow-up: 28 comment:27 by , 17 years ago
I uploaded a new patch. Changes:
- Removed old documentation. Subclassing is not necessary any more.
- Typo: fur*th*er
- removed documentation in request_response. REMOTE_USER is not available, except you added this middleware.
Somehow Trac does not display this patch, although it was created with 'svn diff'.
comment:28 by , 17 years ago
guettli,
Replying to guettli:
I uploaded a new patch. Changes:
from a quick reviewof your patch it seems the only change you implemented
in relation to t689-r7350.diff
is:
- removed documentation in request_response. REMOTE_USER is not available, except you added this middleware.
Could you confirm this? (interdiff isn't being of great help)
comment:29 by , 17 years ago
I compared old diff and mine (689.diff) with ediff-buffers in emacs.
Changes to t689-r7350.diff (old)
django/contrib/auth/backends.py: Old patch contained not needed whitespace change.
docs/auth_remote_user.txt: furder --> further
docs/auth_remote_user.txt: removed
After this, you'll have to create you authentication backend that will take care of checking that ``REMOTE_USER`` is valid. But don't be scared,...
docs/request_response.txt: removed REMOTE_USER since it is only available if
you install this middleware. I guess 99% won't do this. And if someone does it,
he knows that already.
comment:30 by , 17 years ago
Hi ramiro,
Please add a sentence to configure_user (in auth_remote_user.txt and in the docstring),
that this method will only be called if the user does not already exist in the
django database.
Thank you
by , 17 years ago
Attachment: | t689-r7609.diff added |
---|
Patch updated to apply cleanly to r7609 incorporating fixes and suggestions by Thomas Guettler plus additional fixes and docs content
comment:31 by , 17 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
Everything seems fine now without new concerns. Hope it's ready... (if any triager disagrees, please comment #6320)
comment:32 by , 16 years ago
Triage Stage: | Ready for checkin → Accepted |
---|
t689-r7823.diff contains:
- Rebased to apply cleanly to trunk as of r7823
- Fixes a typo and enhacements to some paragraphs in the docs
- test added to django/contrib/auth/tests.py:
- Convert it to use Django TestCase to be in line with recent mods to this file, previously we used unittest's one
- Drop a bunch of unneeded imports and an unneeded attribute
Rolling the ticket back to Accepted until it gets a review from a triager.
by , 16 years ago
Attachment: | t689-r7823.diff added |
---|
comment:33 by , 16 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
As per IRC:
<jacobkm> Nah, go ahead and mark it ready - it is.
comment:34 by , 16 years ago
Summary: | Honour web-server provided authentication → Honor Web server provided authentication |
---|
comment:35 by , 16 years ago
Cc: | added |
---|
comment:37 by , 16 years ago
The docs-refactor broke the patch, always up-to-date patch here in colours and raw
comment:38 by , 16 years ago
Cc: | added |
---|
comment:39 by , 16 years ago
Cc: | added |
---|
comment:40 by , 16 years ago
Cc: | added |
---|
by , 16 years ago
Attachment: | t689-r9099.diff added |
---|
Patch updated to apply to trunk as of r9099: Edited and expanded documentation, fixed tests
comment:42 by , 16 years ago
Description: | modified (diff) |
---|
comment:43 by , 16 years ago
Cc: | added |
---|
comment:44 by , 16 years ago
Cc: | added |
---|
comment:45 by , 16 years ago
Just attached an updated patch, it should be RFCheckin.
I took the changes from Ramiro's latest patch except calling REMOTE_USER a variable, I'd prefer to
call it a Header, at least on the web server part. And used the shiny "versionadded::" directive.
comment:46 by , 16 years ago
Cc: | added |
---|
comment:47 by , 16 years ago
Owner: | changed from | to
---|---|
Status: | assigned → new |
comment:48 by , 16 years ago
Status: | new → assigned |
---|
comment:49 by , 16 years ago
Cc: | added |
---|
comment:50 by , 16 years ago
milestone: | → 1.1 beta |
---|---|
Triage Stage: | Ready for checkin → Accepted |
by , 16 years ago
Attachment: | 689.3.diff added |
---|
comment:51 by , 16 years ago
Cc: | added |
---|
comment:52 by , 16 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
comment:53 by , 15 years ago
Cc: | removed |
---|
Here is the documentation:
http://docs.djangoproject.com/en/dev/howto/auth-remote-user/#howto-auth-remote-user
Patch to existing mod_python handler