Opened 18 years ago
Closed 14 years ago
#3583 closed (wontfix)
cookie-based modpython / apache authentication
Reported by: | Owned by: | ||
---|---|---|---|
Component: | Contrib apps | Version: | dev |
Severity: | Keywords: | authentication modpython apache | |
Cc: | faheem@…, andy@…, jzylks@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
The modpython authentication handler described in
http://www.djangoproject.com/documentation/apache_auth/
is great but it uses HTTP authentication. This is not exactly transparent: to see protected static content from within a protected django site the user will have to log in TWICE, once for the django bits, and once for the static bits. In general there are problems with mixing authentication types: security, logging out, etc.
I'm pretty sure it should not be too hard to write an authentication handler (hooking into PythonAuthenHandler) which looks for the session / authentication cookie instead of using HTTP authentication. I'd do it myself but it has been a long time since I have written apache / modpython modules, so if anyone out there is an expert some help would be much appreciated.
Attachments (10)
Change History (41)
comment:1 by , 18 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 18 years ago
Summary: | REQ: cookie-based modpython / apache authentication → cookie-based modpython / apache authentication |
---|
comment:3 by , 18 years ago
by , 18 years ago
Attachment: | modpython.py added |
---|
apache authentication via session and authentication framework (using cookies)
follow-up: 5 comment:4 by , 18 years ago
Hey thanks, that works great. I had to fiddle with the SESSION_COOKIE_DOMAIN but otherwise it seems fantastic. +1 for putting this into the main source, or at least making this more public -- HTTP basic authentication sucks!
comment:5 by , 18 years ago
Replying to midfield@gmail.com:
Hey thanks, that works great. I had to fiddle with the SESSION_COOKIE_DOMAIN but otherwise it seems fantastic. +1 for putting this into the main source, or at least making this more public -- HTTP basic authentication sucks!
Could you elaborate on the SESSION_COOKIE_DOMAIN things you had to do?
I could write a patch for django.contrib.auth.handlers.modpython, but i want to get any issues sorted out before I do so.
comment:6 by , 18 years ago
The SESSION_COOKIE_DOMAIN thing was my fault, not yours. (I was protecting http://static.foo.com, whereas my main site is http://foo.com.) Maybe just something to put in the docs. The only other thing I'd say is that you might consider changing the logic so that if there is no specific permission set, and there's no "staff only / super only", then any authenticated user is allowed. That's a judgment call, but seems like the most obvious thing to me.
In whatever docs you put up, an example using nested <Location> tags and permissions might be nice -- that's the usual way people are going to use this, I imagine.
A question: right now I'm using VirtualHosts, but in the interests of saving memory, I am forcing my main site and the static protected site to both use the "main_interpreter." Is this the right idea?
Thanks a million.
comment:7 by , 17 years ago
Has patch: | set |
---|---|
Needs documentation: | set |
Patch needs improvement: | set |
The patch should really just import and use the session
and auth
middleware the comment above says.
by , 17 years ago
Attachment: | apache_auth.patch added |
---|
comment:8 by , 17 years ago
Component: | Uncategorized → Contrib apps |
---|---|
Needs documentation: | unset |
Owner: | changed from | to
Patch needs improvement: | unset |
Triage Stage: | Accepted → Ready for checkin |
Right, I actually reneged on my earlier comment that it should 'Just use the session
and auth
middleware', since we should be covering other back ends, not just contrib.auth.models.User
.
What the new patch does:
- Refactored the
contrib.auth.handlers.modpython
to cut out the common elements so they can be used by both the existingauthenhandler
and the newaccesshandler
functions.
- Changed the existing
authenhandler
function to make it work with any back end which takes ausername
andpassword
as the arguments.
- Added the
accesshandler
function which sets up a Django request, applies all active middleware to it, then looks forrequest.user
(which is done bydjango.contrib.auth.middleware.AuthenticationMiddleware
) and validates against it.
- Documented the new method.
comment:9 by , 17 years ago
Triage Stage: | Ready for checkin → Design decision needed |
---|
From discussion about #4354 on IRC, the point was raised that the mod_python
docs suggest using HTTP_FORBIDDEN
instead of HTTP_UNAUTHORIZED
if the user authenticates but does not validate.
Patch 2 contains this further change to the existing authenhandler
function...
And now I guess this should go back to a design decision... :(
by , 17 years ago
Attachment: | apache_auth.2.patch added |
---|
HTTP_FORBIDDEN if user authenticates but does not have correct permissions
comment:10 by , 17 years ago
In Line 42 of apache_auth.2.patch "self" does not exist. It has to be "user.has_perm(options.permission_name)".
if options.permission_name and (not hasattr(user, 'has_perm') or not user.has_perm(options.permission_name)):
by , 17 years ago
Attachment: | modpython.2.py added |
---|
This is an update to apache_auth.2.patch. First: error of wrong "self" is fixed, second: The check was splitted to check if user is authenticated (if not raise UNAUTHORIZED) and the validate user for permissions (if not raise FORBIDDEN)
comment:11 by , 17 years ago
Patch needs improvement: | set |
---|
Good catch of the self
bug. I don't like how your authenticate_user
method requires an is_authenticated
property - you're not taking other backends into account.
Your changes need to be merged into the patch and the patch's docs updated.
comment:12 by , 17 years ago
I am not sure if I am familiar enough with this authentication stuff. If you add this handler to check access for "authenticated users" on an apache location, what case is it, that the "is_authenticated" property is missing. Shouldn't you always be able to check "user.is_authenticated" and if not, give no access to your location ?
comment:13 by , 17 years ago
While the usual case is that authentication returns a contrib.auth
User
model, it could be from any custom created user model (plugged in as an authentication backend) which may not have an is_authenticated
method.
comment:14 by , 17 years ago
From looking at the patch, it's actually doing the right thing with respect to is_authenticated
; the user
argument being passed to authenticate_user
must, at that point, be either None
or an instance of django.contrib.auth.models.User
, because it was obtained by calling django.contrib.auth.authenticate
, which can't return anything else (the requirement of returning a User
or None
is -- per the documentation -- passed on to the authenticate
methods of all auth backends).
I do have a hard time following what's going on here, though, because that check seems to be pointless: if user
is None
, the if not user
check will catch it (and should actually be if user is None
), and if not it must be an instance of User
, in which case is_authenticated
is guaranteed to return True
. Given that, the authenticate_user
function is somewhat redundant, since the if user is not None
check can move into validate_user
, and that can be called directly with the result of django.contrib.auth.authenticate
.
comment:15 by , 17 years ago
Just to get it rigth what you have written above:
28 def authenticate_user(user, options): 29 if not user: 30 return False 31 # Require an is_authenticated property and check it 32 if not hasattr(user, 'is_authenticated') or not user.is_authenticated(): 33 return False 34 return True ... 109 # Check authentification of the user 110 if not authenticate_user(user, options): 111 return apache.HTTP_UNAUTHORIZED
is same as only
109 # Check authentification of the user by existing of user object 110 if not user: 111 return apache.HTTP_UNAUTHORIZED
is this sure, then this will be the best I guess.
by , 17 years ago
Attachment: | apache_auth.3.patch added |
---|
by , 17 years ago
Attachment: | apache_auth.4.patch added |
---|
comment:16 by , 17 years ago
Patch needs improvement: | unset |
---|
New patch up:
- provides a
DjangoRaiseForbidden
option (off
by default to keep the current behaviour) - see docs - cookie authentication works very well! It redirects to
settings.LOGIN_URL
if authentication is required - added a section to the docs about preventing caching in Apache
comment:17 by , 17 years ago
Triage Stage: | Design decision needed → Accepted |
---|
I'm promoting back to Accepted, as we don't have the issue of changing existing behaviour now that I added the DjangoRaiseForbidden
option. Tempted to just promote to checkin, but I'll hold off that for now and bring this up in the dev group
comment:18 by , 17 years ago
I cannot find to which file applies the apache_auth.4.patch (i've tried the django contrib trunk, modpython.py and modpython2.py here). Can you upload a modpython.py file updated? thanks in advance.
comment:19 by , 17 years ago
Owner: | changed from | to
---|
comment:20 by , 17 years ago
Replying to alessandro.ronchi@gmail.com:
I cannot find to which file applies the apache_auth.4.patch (i've tried the django contrib trunk, modpython.py and modpython2.py here). Can you upload a modpython.py file updated? thanks in advance.
It's not that hard actually, you have to patch against revision 6000 :)
- svn co -r 6000 http://code.djangoproject.com/svn/django/trunk/django/
- patch -p0 < apache_auth.4.patch
comment:21 by , 17 years ago
Please feel free to upload a newer patch if it doesn't patch cleanly to the current trunk. Contact me directly if you need a hand.
comment:22 by , 17 years ago
Here's a new patch for revision 7403
And I've included a complete modpython.py for people that have trouble with merging files.
by , 17 years ago
Attachment: | apache_auth.5.patch added |
---|
Patch for django/contrib/auth/handlers/modpython.py@7403
by , 17 years ago
Attachment: | modpython.3.py added |
---|
django/contrib/auth/handlers/modpython.py with patches to enable both PythonAuthenHandler and PythonAccessHandler
by , 16 years ago
Attachment: | apache_auth.6.patch added |
---|
Modified to apply cleanly against rev. 8584
comment:23 by , 16 years ago
apache_auth.6.patch does not apply cleanly for me against rev 8584. Can someone help?
I earlier used apache_auth.4.patch. Have there been non-trivial changes to the patch
since then, and if not, would it be reasonable to try to rebase that patch?
Thanks, Faheem.
comment:24 by , 16 years ago
I just co'd a fresh copy of rev 8584 and applied this latest patch with no issue. What's the problem you're encountering? You are downloading the raw version of the patch, aren't you? http://code.djangoproject.com/attachment/ticket/3583/apache_auth.6.patch?format=raw
by , 16 years ago
Attachment: | apach_auth.7.patch added |
---|
Updated to work with refactored django.dispatch. Applies cleanly against rev 8630.
comment:25 by , 16 years ago
Cc: | added |
---|
comment:26 by , 16 years ago
apach_auth.7.patch applies cleanly for me against current trunk.
i still need to test it.
Thanks, Faheem.
comment:27 by , 16 years ago
Cc: | added |
---|
comment:28 by , 16 years ago
Cc: | added |
---|
comment:29 by , 15 years ago
Owner: | removed |
---|
unassigning myself from the ticket since I don't have any personal interest in continuing with it for now
comment:30 by , 14 years ago
mod_python
will most likely be deprecated in Django 1.3 and removed in future versions, so this ticket collides with #13820
comment:31 by , 14 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
No activity on this, no champion, and mod_python is deprecated. Seems like there's no point in keeping this ticket open.
I wrote a little PythonAccessHandler tightly based on the authhandler mentioned. I don't think you can use the PythonAuthHandler, because that needs all the authentication stuff from apache and I haven't found a way to surpress the prompt.
There is a lot of overhead in the current implementation, as all request middleware components are applied, but only session and authentication middleware would be needed.
I use this apache configuration for the access control: