Honor Web server provided authentication

[Ian@…]

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
< 

[ian@…]

Patch to existing mod_python handler

[sune.kirkeby@…]

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

[hugo]

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.

[ian@…]

middleware version of same idea

[ian@…]

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

[garthk]

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.

[garthk]

Slight revision of Ian's code

[garthk]

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

[Adrian Holovaty]

Superceded by MultipleAuthBackends page.

[Pawel J. Sawicki (http://pjs.name)]

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

[Marc Fargas <telenieko@…>]

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.

[anonymous]

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.

[Jacob]

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

[Koen Biermans <koen.biermans@…>]

patch using both middleware and authentication backend (doc included)

[Koen Biermans <koen.biermans@…>]

Modified version without settings

[Koen Biermans <koen.biermans@…>]

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.

[Marc Fargas]

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

[Marc Fargas]

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

[Ramiro Morales]

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

[Marc Fargas]

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 :=)

[Florian Apolloner]

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

[Erik Karulf]

Changed 689_full to use unusable password

[Ramiro Morales]

New version of the patch, incoporates ideas from several people for RemoteUserAuthBackend implementation

[Ramiro Morales]

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/

[Marc Fargas]

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

[Jacob]

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!

[Marc Fargas]

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! ;)

[Marc Fargas]

New version (aka. The Non Java One)

[Marc Fargas]

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!

[Ramiro Morales]

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

[Thomas Güttler]

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

[Ramiro Morales]

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)

[Thomas Güttler]

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.

[Thomas Güttler]

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

[Ramiro Morales]

Patch updated to apply cleanly to r7609 incorporating fixes and suggestions by Thomas Guettler plus additional fixes and docs content

[Marc Fargas]

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

[Ramiro Morales]

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.

[Marc Fargas]

As per IRC:

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

[yoz]

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

[Marc Fargas]

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

[Thomas Güttler]

This ticket is on the Version1.1Features list.

[Ramiro Morales]

Patch updated to apply to trunk as of r9099: Edited and expanded documentation, fixed tests