Opened 19 years ago

Closed 16 years ago

Last modified 13 years ago

#689 closed enhancement (fixed)

Honor Web server provided authentication

Reported by: Ian@… 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 Gary Wilson)

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)

modpython.patch (1.0 KB ) - added by ian@… 19 years ago.
Patch to existing mod_python handler
httpauth.py (1.2 KB ) - added by ian@… 19 years ago.
middleware version of same idea
httpdauth.py (1.5 KB ) - added by garthk 19 years ago.
Slight revision of Ian's code
httpdauth.2.py (2.8 KB ) - added by garthk 19 years ago.
Revision of Ian's module; new version moves settings to the settings module
remote_user.diff (6.2 KB ) - added by Koen Biermans <koen.biermans@…> 17 years ago.
patch using both middleware and authentication backend (doc included)
remote_user_2.diff (6.7 KB ) - added by Koen Biermans <koen.biermans@…> 17 years ago.
Modified version without settings
689_full.diff (9.9 KB ) - added by Marc Fargas 17 years ago.
the code, the docs, the tests; And it works!
689_full-2.diff (9.9 KB ) - added by Erik Karulf 17 years ago.
Changed 689_full to use unusable password
689_full-3.diff (13.7 KB ) - added by Ramiro Morales 17 years ago.
New version of the patch, incoporates ideas from several people for RemoteUserAuthBackend implementation
689_nojavaversion.diff (12.7 KB ) - added by Marc Fargas 17 years ago.
New version (aka. The Non Java One)
t689-r7350.diff (9.9 KB ) - added by Ramiro Morales 17 years ago.
689.diff (8.0 KB ) - added by Thomas Güttler 17 years ago.
t689-r7609.diff (8.4 KB ) - added by Ramiro Morales 17 years ago.
Patch updated to apply cleanly to r7609 incorporating fixes and suggestions by Thomas Guettler plus additional fixes and docs content
t689-r7823.diff (8.1 KB ) - added by Ramiro Morales 16 years ago.
t689-r9099.diff (9.3 KB ) - added by Ramiro Morales 16 years ago.
Patch updated to apply to trunk as of r9099: Edited and expanded documentation, fixed tests
689-r9733.diff (10.1 KB ) - added by Marc Fargas 16 years ago.
Updated patch
689.2.diff (10.0 KB ) - added by Marc Fargas 16 years ago.
New patch with Gary's comments
689.3.diff (20.3 KB ) - added by Gary Wilson 16 years ago.
689.4.diff (20.6 KB ) - added by Gary Wilson 16 years ago.
add to docs paragraph about custom header value

Download all attachments as: .zip

Change History (73)

by ian@…, 19 years ago

Attachment: modpython.patch added

Patch to existing mod_python handler

comment:1 by sune.kirkeby@…, 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 hugo, 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.

by ian@…, 19 years ago

Attachment: httpauth.py added

middleware version of same idea

comment:3 by ian@…, 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 anonymous, 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 garthk, 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 garthk, 19 years ago

Attachment: httpdauth.py added

Slight revision of Ian's code

by garthk, 19 years ago

Attachment: httpdauth.2.py added

Revision of Ian's module; new version moves settings to the settings module

comment:6 by Adrian Holovaty, 19 years ago

Resolution: duplicate
Status: newclosed

Superceded by MultipleAuthBackends page.

comment:7 by Pawel J. Sawicki (http://pjs.name), 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 Marc Fargas <telenieko@…>, 18 years ago

Resolution: duplicate
Status: closedreopened
Summary: [patch] middleware to allow apache to provide authentication instead of djangoHonour 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 Marc Fargas <telenieko@…>, 18 years ago

Cc: telenieko@… added

comment:10 by Chris Beaven, 18 years ago

Triage Stage: UnreviewedDesign decision needed

comment:11 by anonymous, 18 years ago

Cc: hozer@… 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 Jacob, 17 years ago

Patch needs improvement: set
Triage Stage: Design decision neededAccepted

This middleware would be appropriate as an optional part of django.contrib.auth, but has some problems (as listed in the patch).

by Koen Biermans <koen.biermans@…>, 17 years ago

Attachment: remote_user.diff added

patch using both middleware and authentication backend (doc included)

by Koen Biermans <koen.biermans@…>, 17 years ago

Attachment: remote_user_2.diff added

Modified version without settings

comment:13 by Koen Biermans <koen.biermans@…>, 17 years ago

Cc: koen.biermans@… 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 Marc Fargas, 17 years ago

Needs tests: set

by Marc Fargas, 17 years ago

Attachment: 689_full.diff added

the code, the docs, the tests; And it works!

comment:15 by Marc Fargas, 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

comment:16 by Ramiro Morales, 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

in reply to:  16 ; comment:17 by Marc Fargas, 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 :=)

in reply to:  17 comment:18 by Florian Apolloner, 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 Erik Karulf, 17 years ago

Attachment: 689_full-2.diff added

Changed 689_full to use unusable password

by Ramiro Morales, 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 Ramiro Morales, 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 Ramiro Morales, 17 years ago

Owner: changed from nobody to Ramiro Morales
Status: reopenednew

comment:21 by Marc Fargas, 17 years ago

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

I think it's ready for checkin now. It passes all the tests and works :)))

comment:22 by Jacob, 17 years ago

Patch needs improvement: set
Triage Stage: Ready for checkinAccepted

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!

in reply to:  22 comment:23 by Marc Fargas, 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! ;)

by Marc Fargas, 17 years ago

Attachment: 689_nojavaversion.diff added

New version (aka. The Non Java One)

comment:24 by Marc Fargas, 17 years ago

Owner: changed from Ramiro Morales to Marc Fargas
Patch needs improvement: unset
Status: newassigned

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 Ramiro Morales, 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 Ramiro Morales, 17 years ago

Attachment: t689-r7350.diff added

comment:26 by Thomas Güttler, 17 years ago

Cc: hv@… added

by Thomas Güttler, 17 years ago

Attachment: 689.diff added

comment:27 by Thomas Güttler, 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'.

in reply to:  27 comment:28 by Ramiro Morales, 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 Thomas Güttler, 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 Thomas Güttler, 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 Ramiro Morales, 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 Marc Fargas, 17 years ago

Triage Stage: AcceptedReady for checkin

Everything seems fine now without new concerns. Hope it's ready... (if any triager disagrees, please comment #6320)

comment:32 by Ramiro Morales, 16 years ago

Triage Stage: Ready for checkinAccepted

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 Ramiro Morales, 16 years ago

Attachment: t689-r7823.diff added

comment:33 by Marc Fargas, 16 years ago

Triage Stage: AcceptedReady for checkin

As per IRC:

<jacobkm> Nah, go ahead and mark it ready - it is.

comment:34 by Adrian Holovaty, 16 years ago

Summary: Honour web-server provided authenticationHonor Web server provided authentication

comment:35 by Hugh Saunders <hugh@…>, 16 years ago

Cc: hugh@… added

comment:36 by yoz, 16 years ago

Is this going to make it into 1.0? It would be so useful.

comment:37 by Marc Fargas, 16 years ago

The docs-refactor broke the patch, always up-to-date patch here in colours and raw

comment:38 by Eric Vander Weele, 16 years ago

Cc: ericvw@… added

comment:39 by anonymous, 16 years ago

Cc: grf@… added

comment:40 by hinnerk, 16 years ago

Cc: hinnerk@… added

comment:41 by Thomas Güttler, 16 years ago

This ticket is on the Version1.1Features list.

by Ramiro Morales, 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 Gary Wilson, 16 years ago

Description: modified (diff)

comment:43 by anonymous, 16 years ago

Cc: me@… added

comment:44 by zhaoz, 16 years ago

Cc: zilingzhao@… added

by Marc Fargas, 16 years ago

Attachment: 689-r9733.diff added

Updated patch

comment:45 by Marc Fargas, 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 dvd, 16 years ago

Cc: dvd@… added

by Marc Fargas, 16 years ago

Attachment: 689.2.diff added

New patch with Gary's comments

comment:47 by Gary Wilson, 16 years ago

Owner: changed from Marc Fargas to Gary Wilson
Status: assignednew

comment:48 by Gary Wilson, 16 years ago

Status: newassigned

comment:49 by eengbrec, 16 years ago

Cc: erik.engbrecht@… added

comment:50 by Jacob, 16 years ago

milestone: 1.1 beta
Triage Stage: Ready for checkinAccepted

by Gary Wilson, 16 years ago

Attachment: 689.3.diff added

by Gary Wilson, 16 years ago

Attachment: 689.4.diff added

add to docs paragraph about custom header value

comment:51 by David Larlet, 16 years ago

Cc: David Larlet added

comment:52 by Gary Wilson, 16 years ago

Resolution: fixed
Status: assignedclosed

(In [10063]) Fixed #689 -- Added a middleware and authentication backend to contrib.auth for supporting external authentication solutions. Thanks to all who contributed to this patch, including Ian Holsman, garthk, Koen Biermans, Marc Fargas, ekarulf, and Ramiro Morales.

comment:54 by Jacob, 13 years ago

milestone: 1.1 beta

Milestone 1.1 beta deleted

Note: See TracTickets for help on using tickets.
Back to Top