#2507 closed enhancement (wontfix)
[patch] LDAPBackend in django/contrib/auth/backends.py
Reported by: | Owned by: | nobody | |
---|---|---|---|
Component: | Contrib apps | Version: | dev |
Severity: | normal | Keywords: | ldap, usernames |
Cc: | wangchun@…, cmawebsite@…, silassewell@…, spkane00@…, semente@…, listuser@…, ben@…, hcarvalhoalves@…, django@…, hr.bjarni+django@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | yes | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
This patch provides a backend that will allow users to authenticate against LDAP. It creates User objects based on fields in LDAP.
Various LDAP settings are provided by the projects settings.py file.
Attachments (18)
Change History (57)
by , 18 years ago
Attachment: | backends.py.diff added |
---|
by , 18 years ago
Attachment: | backends.py.2.diff added |
---|
update to allow ldap options, and custom bind string functions. Also fixes a typo.
by , 18 years ago
Attachment: | backends.py.3.diff added |
---|
Fixing bug with BIND_STRING_FUNC and adding a method to be used with BIND_STRING_FUNC.
by , 18 years ago
Attachment: | backends.py.4.diff added |
---|
Slight update to make the users experience better with the pre_auth_bind.
by , 18 years ago
Attachment: | backends.py.5.diff added |
---|
Better handling of searches, and setting good defaults to mk_pre_auth_bind()
by , 18 years ago
Attachment: | backends.py.6.diff added |
---|
fixing use of LDAP_OPTIONS hash so code follows documentation
by , 18 years ago
Random password, and adding defaults to global_settings.py to avoid loading errors.
by , 18 years ago
Attachment: | ldap.2.diff added |
---|
Update to allow inheritance of LDAPBackend, so you can override the User object and generation of the bind string. This is the full patch for LDAP auth support.
by , 18 years ago
Attachment: | ldap.3.diff added |
---|
Fixes a few run-time bugs, adding some more error checking.
by , 18 years ago
Attachment: | ldap.4.diff added |
---|
Shouldn't have removed staticmethod from mk_pre_auth_bind.
comment:1 by , 18 years ago
Excuse my ignorance here, but how tightly bound is this to any particular LDAP backend? Should it work against more or less any implementation that is accessible via ldap://...?
I'm not sure yet if this should go into Django itself, but I'm leaning towards thinking it would be useful. The "pollution" (for want of a better word) of global_settings.py is a little unfortunate: we need a better way of having contrib/ applications providing their own setting defaults. Obviously not going to commit it right this minute, but it is being noticed.
comment:2 by , 18 years ago
It should work with any implementation that can be gotten to via ldap://. I've found references online that python-ldap works with Active Directory and eDirectory. It uses OpenLDAP's client libraries, which I've seen work with Apple's Open Directory as well.
I'm not happy either with the settings in global_settings.py, and would like to find a better solution.
FYI, I've been using this backend in production for about 3 months without issue.
comment:3 by , 18 years ago
for the record: I'm rewritting part of the patch so it has it's default values inside it without touching anything else so it can be a single .py file in contrib or downloaded by the user.
Also changing the _pre_bind() so it searches for the user dn: or makes it from a template (like it does now) so the user does not need to override anything, just setup some settings.
I'll let you know when it's done.
comment:4 by , 18 years ago
Marc:
I actually had been thinking about ways to get the settings in the file, just haven't had time to do anything about it.
Tweaking _pre_bind() sounds good.
Looking forward to see the changes.
comment:5 by , 18 years ago
Needs documentation: | set |
---|---|
Patch needs improvement: | set |
Triage Stage: | Unreviewed → Accepted |
Marc: do you still have those modifications available? This would be a very nice feature to get into contrib.
comment:6 by , 18 years ago
Hi Simon,
I'll try to attach my current patch this Monday (I'm out for the weekend), it currently handles authentication (tested against Active Directory) and getting user information. I'm working two more issues now, and I have one question for everybody interested.
- Map arbitrary LDAP attributes to user.get_profile() fields.
- Get group membership information (creating groups if needed)
The problem with getting group membership is that django does not support inheritance (a group being member of a group), while LDAP does. How would you handle this? Only get the first level memberships or get all the memberships and flatten them making the user be member of all the inherited groups directly?
Again, I'll try to upload the authentication part which "should word" at least with A.D. so you can put more comments this Monday and I'll work on those two issues during the week.
comment:7 by , 18 years ago
I'll provide OpenLDAP testing once the patch gets posted. If you're still fairly close to the original patch it should work just fine with OpenLDAP.
With user.get_profile(): I'm not familiar with the backend code for this. I'm guessing you're think of basically providing a dictionary straight from LDAP? If so I'd at least make it an optional feature.
Improving the group handling would be great. Considering the current patch just checks for staff or admin status it'd be nice to provide better control. Personally I'd go for flattening the groups.
We might want to bring this over to the dev list once the patch gets posted since other people aren't watching the ticket.
comment:8 by , 18 years ago
Here's a new patch, two thing to do before applying:
- Create a new directory: django/contrib/auth/contrib (that's, the last contrib is new)
- touch/create a blank init.py file on this new directory
I think this is the best place to place "contributed authentication modules", documentation is still inside the code, there's one issue for documenting this (more below), the patch will create ldapauth.py and you should be able to use "django.contrib.auth.contrib.ldapauth.LDAPBackend" as an authentication backend and It should almost work as before.
The documentation issue is that djangoproject.com has no way to provide subdirectories (in this case /authentication/contributed/) which would place docs for this in an ugly url (/contributed_authenticators/ and /ldap_autenticator/), my idea was to create a new doc (/authentication/contributed/ or /contributed_authenticators/) to explain small contribued authenticators and link bigger ones like (/authentication/contributed/ldap/ or /ldap_authenticator/) just to be ready in case more authenticators get contributed. As you see URLs are far more clean with some directory depth support so I've left the docs where they are until we get this discussed ;)
The second thing left is the attributes mapping, right now the mapping is done "by hand" in _update_user my idea was to provide a setting with the ldap<>User mappings and loop over them:
LDAP_ATTR_MAP = {'first_name': 'givenName', 'last_name': 'sn'}
That would make things far more customizable and a cleaner code, I talked on the get_profile() a few comments above, the idea would be to have a similar mapping directory for it being fully optional, but usefull if you have more attributes than the ones in User.
The third and last issue is group membership, until django support inheritance the cleanest approach is to flatten the memberships, On Active Directory I'll only have to take all "memberOf" attributes from the user and query all those cn's for more "memberOf" until I'm done, how is this done on OpenLDAP?
If this behaviour is much different we could ship a base LDAPBackend and an ADBackend and OLBackend which implement the different behaviours. I'll work with the mapping and the memberships this week as I need them.
Cheers,
Marc.
follow-up: 10 comment:9 by , 18 years ago
Works out of the box with my OpenLDAP setup. I didn't need to change any settings.
I've attached a slight modification of the patch which changes:
- The except statement when we bind as the user to catch the case where someone doesn't enter their password.
- Minor adjustments to the internal documentation to clear up the wording and remove some now obsolete statements.
Also, why are the calls to the ldap module always done as self.ldap? It works either way, and removing the self would be cleaner code in my opinion.
Do we want to create the contrib directory? auth is already a contrib and it seems a bit excessive to have a contrib in contrib, but that just might be me.
It does seem that the documentation should be in a subdirectory. Maybe we can just write it up in the current authentication page for now.
I'm all for a LDAP_ATTR_MAP, it seems like a much cleaner method, and should be easy to maintain current functionality.
Group membership isn't the same between AD and OpenLDAP. OpenLDAP has group objects with have lists of users. We should go for a base LDAPBackend (which doesn't bother with groups), and subclass it for AD and OpenLDAP.
Looks good, thanks for the work, Scott
by , 18 years ago
Attachment: | ldapauth.1.diff added |
---|
Patch with one bug fix and some documentation improvements.
follow-up: 11 comment:10 by , 18 years ago
Replying to Scott Paul Robertson <spr@mahonri5.net>:
Works out of the box with my OpenLDAP setup. I didn't need to change any settings.
Nice to know!!
Also, why are the calls to the ldap module always done as self.ldap? It works either way, and removing the self would be cleaner code in my opinion.
import ldap was inside the class when I took the patch, so I left it there. I think there would be no problem to move the import to the top of the file and then s/self.ldap/ldap/ ;)
Do we want to create the contrib directory? auth is already a contrib and it seems a bit excessive to have a contrib in contrib, but that just might be me.
Auth is in contrib, but "contributed authentication backends" seem as "contribs to auth", or "contribs to the auth contrib" hence the subdirectory.
It does seem that the documentation should be in a subdirectory. Maybe we can just write it up in the current authentication page for now.
It would mess the page a bit, we should try to do things fine from the start, sure somebody (willison?) will work on an OpenID backend and having the structure for those contribs (where to place the .py and the .txt) would be nice. And LDAP is a really speciffic matter, it should not go with the "generic" authentication documentation (I think).
I'm all for a LDAP_ATTR_MAP, it seems like a much cleaner method, and should be easy to maintain current functionality.
I'm a bit busy now but I'll go for it ASAP :)
Group membership isn't the same between AD and OpenLDAP. OpenLDAP has group objects with have lists of users. We should go for a base LDAPBackend (which doesn't bother with groups), and subclass it for AD and OpenLDAP.
AD has the "members" inside the group objects and users have "memberOf", in OpenLDAP how do you know which groups a user is member of? with a search?
Looks good, thanks for the work, Scott
you're welcome.
Maybe we can bring discussion to django-developers about where to place the docs and the .py itself :)
comment:11 by , 18 years ago
Replying to Marc Fargas <telenieko@telenieko.com>:
Replying to Scott Paul Robertson <spr@mahonri5.net>:
Also, why are the calls to the ldap module always done as self.ldap? It works either way, and removing the self would be cleaner code in my opinion.
import ldap was inside the class when I took the patch, so I left it there. I think there would be no problem to move the import to the top of the file and then s/self.ldap/ldap/ ;)
Sounds good to me.
Group membership isn't the same between AD and OpenLDAP. OpenLDAP has group objects with have lists of users. We should go for a base LDAPBackend (which doesn't bother with groups), and subclass it for AD and OpenLDAP.
AD has the "members" inside the group objects and users have "memberOf", in OpenLDAP how do you know which groups a user is member of? with a search?
Yeah, you do a search on a group sub-tree to find what groups a user is a member of. We have a lot of LDAP traffic in our labs. :)
Maybe we can bring discussion to django-developers about where to place the docs and the .py itself :)
I'll make a post tonight.
by , 18 years ago
Attachment: | ldapauth.2.diff added |
---|
Fix a typo that is causing a bug in the prebind code.
comment:13 by , 17 years ago
Cc: | added |
---|
comment:14 by , 17 years ago
Version: | → SVN |
---|
I added the patch with documentation. I am sure it needs improvement, but I'd like to see the ldap backend get merged into trunk, so I wrote up some documentation. Some of it comes from comments in the source code. This patch is against svn 6980 (current as of now)
comment:15 by , 17 years ago
Needs documentation: | unset |
---|
I wrote the documentation for it. I forgot to mark it as being such when I uploaded the patch, so I'm marking it now.
comment:16 by , 17 years ago
Cc: | added |
---|
comment:17 by , 17 years ago
Cc: | added |
---|
by , 17 years ago
Attachment: | ldapauth.3.diff added |
---|
changed random password generation, added is_active fields.
comment:18 by , 17 years ago
Thanks, works great for me :) however it breaks the "change password" feature in the admin utility...
and something else I noted: the code does try to use StartTLS over ldap:// urls. after the .initialize just do something like this:
if LDAP_SERVER_URI.startswith('ldap:'): try: l.start_tls_s() except FindOutWhatExceptionItwouldRaiseIfStartTLSWasNotSupported ;): pass
sorry that I don't write a proper patch but I'm just too tired right now ;)
comment:19 by , 17 years ago
Comment for ldapauth.4.diff:
- LDAP_DEBUG was added. If True, the logging module is used for debug messages.
- authenticate(): if _pre_bind returns nothing, stop and return None.
- If LDAP_ACTIVE_FIELD is not set, all users are active.
comment:20 by , 17 years ago
Another feature would be nice: While sometimes you can group your users by simply using an "ou" entry in the dn, often you have to use a more complex group structure, like this:
cn:groupname,ou=Groups,dc=examplefoo objectClass:posixGroup memberUid:foo memberUid:bar memberUid:baz
or like this:
cn:groupname2,ou=Groups,dc=examplefoo objectClass:groupOfPeople member: uid=foo,ou=Users,dc=examplefoo member: uid=bar,ou=Users,dc=examplefoo member: uid=baz,ou=Users,dc=examplefoo
I would like to use the first one to determine which users are active/staff/admin but I can't find a way to do this right now without patching the authentification module...
comment:21 by , 17 years ago
Comment for file: ldapauth.py
- Fixes issue losing ldap connect when using LDAP_PREBINDDN and LDAP_PREBINDPW for systems that don't allow anonymous ldap lookups
- Fixes logic for checking GID's for superusers and staff.
- Now can pass an identifier to see if the user belongs to and if the user is in the matching group then they pass i.e.:
'LDAP_GID': 'memberOf', 'LDAP_SU_GIDS': ['CN=systemadmins,OU=Headoffice,DC=example,DC=com'], 'LDAP_STAFF_GIDS': ['CN=allstaff,,OU=Headoffice,DC=example,DC=com'], # User x has the following values for memberOf: ['CN=systemadmins,OU=Headoffice,DC=example,DC=com', 'CN=allstaff,OU=Headoffice,DC=example,DC=com'] # Will have the following user attributes: user.is_super = True user.is_staff = True # User y who has the following values for memberOf: ['CN=admin,OU=Headoffice,DC=example,DC=com', 'CN=allstaff,OU=Headoffice,DC=example,DC=com'] # Will have the following user attributes: user.is_super = False user.is_staff = True # User z who has the following values for memberOf: ['CN=tmpstaff,OU=Headoffice,DC=example,DC=com'] # Will have the following user attributes: user.is_super = False user.is_staff = False
- Now can pass an identifier to see if the user belongs to and if the user is in the matching group then they pass i.e.:
- Fixes logic for isactive if not set
comment:22 by , 17 years ago
self.settings.LDAP_ACTIVE* should be self.settingsLDAP_ACTIVE*, or am I wrong? current ldapauth.py throws exception because of this. greets, c
comment:24 by , 16 years ago
Cc: | added |
---|
comment:25 by , 16 years ago
In order for this not to throw up an error:
Exception Value: 'dict' object has no attribute 'LDAP_OPTIONS' Exception Location: /usr/lib/python2.5/site-packages/django/contrib/auth/ldapauth.py in authenticate, line 113
line 113 of ldapauth.py:
self.ldap.set_option(k, self.settings.LDAP_OPTIONS[k])
needs to be:
self.ldap.set_option(k, self.settings["LDAP_OPTIONS"][k])
I presume that other instances of that syntax need to be changed.
Also, the syntax for settings.py that worked for me were:
import ldap LDAP_OPTIONS = 'ldap.OPT_X_TLS_DEMAND,1'
by , 16 years ago
Attachment: | ldapauth.patch added |
---|
Example patch to handle converting periods in ldap usernames to underscrore for the django username
follow-up: 28 comment:26 by , 16 years ago
Cc: | added |
---|---|
Keywords: | ldap usernames added |
ldapauth.patch (4.5 kB) - added by spkane on 10/24/08 10:33:11.
This patch is reasonably specific to my needs, but I would bet hard money, that this is an issue faced by other people, so a more robust version of this patch should likely be considered. The issue is that the best thing for me to use as a username within our LDAP implementation is the prefix of the email address, since it is the only thing guaranteed to be unique (we have many employees with the same name). However, our email addresses are, firtname.lastname@…. Django does not allow a period in the username, so I added some logic to handle the username (for django) and the ldap_username separately and use the proper one in the proper place. Basically, the users login as "sean_kane" and the ldap backend converts that to "sean.kane" when talking to the ldap server and then uses "sean_kane" when talking to the Django server.
comment:27 by , 16 years ago
I intend to write tests for this patch using the iw.mock.ldap module
. This is a very low priority project for me. I'll likely work on it at/during a sprint-- it'd be nice to finally have included. The patches submitted are kind of a mess-- some are patches, some are standalone python files.
This ticket needs to have the docs, new code, and tests in a single patch file in order for it to be ready for check-in. I'll do my best to sort through the various versions of the patch and see if I can get something in the right format. Right now it's a big mess. :)
follow-up: 29 comment:28 by , 16 years ago
Needs tests: | set |
---|
Replying to spkane:
ldapauth.patch (4.5 kB) - added by spkane on 10/24/08 10:33:11.
This patch is reasonably specific to my needs, but I would bet hard money, that this is an issue faced by other people, so a more robust version of this patch should likely be considered. The issue is that the best thing for me to use as a username within our LDAP implementation is the prefix of the email address, since it is the only thing guaranteed to be unique (we have many employees with the same name). However, our email addresses are, firtname.lastname@…. Django does not allow a period in the username, so I added some logic to handle the username (for django) and the ldap_username separately and use the proper one in the proper place. Basically, the users login as "sean_kane" and the ldap backend converts that to "sean.kane" when talking to the ldap server and then uses "sean_kane" when talking to the Django server.
I'm not sure that's where that kind of logic belongs. I'd be inclined to use the cn as the "username" field, and then just tell users to login with their e-mail address. I've never implemented that, but I've heard of people using the Django auth system in such a way that people can log in with their e-mail address instead of their username. You'd basically just have to roll your own login forms, and make sure your views don't spit out the actual "username"-- they'll just spit out the e-mail address.
I've also seen some discussion come up about allowing dots in a username. I don't know where that discussion went though. Ultimately the decision to include this logic in the ldapbackend will need to come from a core dev, but I'm a big fat -1 on the issue.
comment:29 by , 16 years ago
Replying to programmerq:
Replying to spkane:
ldapauth.patch (4.5 kB) - added by spkane on 10/24/08 10:33:11.
This patch is reasonably specific to my needs, but I would bet hard money, that this is an issue faced by other people, so a more robust version of this patch should likely be considered. The issue is that the best thing for me to use as a username within our LDAP implementation is the prefix of the email address, since it is the only thing guaranteed to be unique (we have many employees with the same name). However, our email addresses are, firtname.lastname@…. Django does not allow a period in the username, so I added some logic to handle the username (for django) and the ldap_username separately and use the proper one in the proper place. Basically, the users login as "sean_kane" and the ldap backend converts that to "sean.kane" when talking to the ldap server and then uses "sean_kane" when talking to the Django server.
I'm not sure that's where that kind of logic belongs. I'd be inclined to use the cn as the "username" field, and then just tell users to login with their e-mail address. I've never implemented that, but I've heard of people using the Django auth system in such a way that people can log in with their e-mail address instead of their username. You'd basically just have to roll your own login forms, and make sure your views don't spit out the actual "username"-- they'll just spit out the e-mail address.
I've also seen some discussion come up about allowing dots in a username. I don't know where that discussion went though. Ultimately the decision to include this logic in the ldapbackend will need to come from a core dev, but I'm a big fat -1 on the issue.
I don't really care about it being implemented here, and I'm not the one to make such a decision, but I had to do the work and wanted something that would not prevent me from upgrading Django so I put it here for myself. Since I did the work, I wanted to make it available to others, until something is properly designed. If the decision is to remove it from here, I would strongly suggest opening up a ticket to handle creating a mechanism in the auth framework to handle providing proper mapping of usernames used in auth backends with Django usernames.
comment:30 by , 16 years ago
Cc: | added |
---|
comment:31 by , 16 years ago
Cc: | added |
---|
comment:32 by , 16 years ago
Cc: | added |
---|
comment:33 by , 16 years ago
Cc: | added |
---|
follow-up: 35 comment:34 by , 15 years ago
I've submitted a more complete LDAP authentication backend in [11526], complete with full documentation and unit tests. I think we can get it into the distribution, but it could use some test coverage.
Thanks
comment:35 by , 15 years ago
comment:36 by , 15 years ago
Cc: | added |
---|
comment:37 by , 15 years ago
Cc: | added |
---|
comment:38 by , 14 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
There's no reason this needs to exist in Django core. We added an extension API specifically so people can build external interfaces to auth.
django-auth-ldap is one example of an external LDAP implementation.
Marking wontfix in favor of having a vibrant external project cover this need.
comment:39 by , 13 years ago
Cc: | removed |
---|---|
Easy pickings: | unset |
UI/UX: | unset |
patch