Fix/Improvement to django-auth-ldap

[Felipe 'chronos' Prenholato]

Hello people.

I'm now using django-auth-ldap to authenticate and create user from a Active Directory server, I made some changes and a opnion or critic about it should be a good idea, before a pull request.

Changes are at https://bitbucket.org/chronossc/django-auth-ldap/changeset/257a741d6785

from lines 415 to 441:

Changed where to save user. I need to do it because signals attached to django.contrib.auth.models.User can't see ldap_user in useful time to use it in post_save. Also I have fail when we get user profile (https://bitbucket.org/chronossc/django-auth-ldap/src/257a741d6785/django_auth_ldap/backend.py#cl-458) because profile isn't yet created (it is created in a user post_save signal).

from lines 459 to 478:

Added some code to fill ForeignKey fields with correct option instead text from AD. Can be extended in similar way for m2m relationships.

I guess that change 1 can be done in better way, but change 2 is a improvement.

Thx!

[Felipe 'chronos' Prenholato]

Guys wanna check this changeset: https://bitbucket.org/chronossc/django-auth-ldap/changeset/292eab01da3a where LDAPBackend.get_or_create_user search for user based on AUTH_LDAP_USER_ATTR_MAP and also return user with ldap_user attribute.

[Peter Sagerson]

For the first issue, I think your original idea is fine. We can coordinate better with the save signals by moving the ldap_user assignment above user.save() and moving the profile population below. That should be safe for existing clients and can become documented behavior. Moving the ldap_user assignment to get_or_create_user isn't an option because it changes the semantics of this subclass hook. I can send you a build to test.

The second one is a little more complicated. One problem is that it relies on _meta, which is a private and undocumented property. I have occasionally used it in my own code, but distributing a package that relies on it is a different matter. A better approach might be to have the configuration indicate which fields are foreign keys and how to locate the associated records. That said, this is sounding like an awful lot of configuration that ought to be code. Once the save signals are sorted out, is there any reason you can't populate your profile in post_save? Then you can make it as complex as you like without worrying about generalizing.

Thanks

[Felipe Prenholato]

I read the code on tar.gz that you sended to me.

For the first issue, your solution is perfect for me. I guess we can document it and release on next version. Personally I will make my version like you did yours.

About the second issue, personaly I don't see big problems using stuff of _meta. I know that can change over Django versions and isn't documented but I use _meta.get_field_by_name(field_name) have about two years without any problem. Also the method in question is used in various points of framework ( a grep counted 28 entries, most on db/models/sql/query.py and contrib/admin/util.py ). With this facts I can't see any problem in use it to get foreign keys :). Off course this feature can be extended to OneToOne fields and ManyToMany fields with a little of hacking and once that is really stable don't need to touch on any configuration if you use a complex UserProfile class.

I guess that we can ask on Django Developers or Django Users about _meta.get_field_by_name and stability of the method and also make some draft of extended mapping configuration. Personally I'll maintain the version using meta because this  is enought for me at work and I can't losse much more time with that :), but I help with pleasure if your final idea is avoid _meta.

Backing to first issue, what about LDAPBackend and LDAPUsers fire some signals instead make us connect to Users signals? I gues that is a good idea, so developers can use User.post_save to do User related stuff and LDAPUser.post_save to LDAP User related stuff. Is a idea that need to discuss :).

Cheers,

Felipe 'chronos' Prenholato.
Linux User nº 405489
Home page: http://chronosbox.org/blog
Twitter: http://twitter.com/chronossc

2011/3/24 Peter Sagerson <psa...@ignorare.net>

[Felipe Prenholato]

Peter, and about get username attribute from user attribute maps and than value from this attribute on ldap_user instead force it to username value sent in arguments? Here I'm authenticating via e-mail and get wrong entries because username is felipe.rafael instead felipe...@pdg.com.br (with I use to authenticate).

Here is the change: https://bitbucket.org/chronossc/django-auth-ldap/changeset/292eab01da3a#chg_django_auth_ldap/backend.py_newline166

Felipe 'chronos' Prenholato.
Linux User nº 405489
Home page: http://chronosbox.org/blog
Twitter: http://twitter.com/chronossc

2011/3/24 Felipe Prenholato <phili...@gmail.com>

[Peter Sagerson]

Just as a point of philosophy, it's not a goal to implement every plausible scenario directly. The goal of django_auth_ldap (beyond the original goal of doing what I need myself) is to provide the general framework, implement some common scenarios as a convenience, and provide hooks to facilitate more complex customization.

To get the username from the attributes, the expectation is that you will subclass LDAPBackend and override get_or_create_user. That's more or less what you're doing now, so you should be all set. A feature to pull the username from an LDAP attribute might be a reasonable addition, although overloading AUTH_LDAP_USER_ATTR_MAP may not be the best way. It bears some thought, but since it's purely a convenience feature, there's no need to rush into it.

The issues you had with populating users are valid and I think your suggestion of a custom signal makes a lot of sense. I would propose two signals: populate_ldap_user and populate_ldap_user_profile. These would be sent after the built-in user/profile population is done but before the user/profile has been saved (respectively), thus minimizing DB access. Combined with the new order of operations, I think this should give you everything you need for your cases.

Thanks

[Felipe Prenholato]

Amazing, it's fine for me :).

I'll adapt the code subclassing LDAPBackend as you told as soon I have time :) in my code.

Tonight I make changes on my fork at bitbucket and send you as patch.

Thank you very much Peter.

[Jeff Schroeder]

s/Paul/Peter/. Just finished a convo with my friend Paul.

On Thu, Mar 24, 2011 at 1:54 PM, Jeff Schroeder
<jeffsc...@computer.org> wrote:
> Paul,
>
> If the code is cleaned up, and suitable to your standards, would you
> merge a subclass which worked for Active Directory things out of the
> box, or will you make that work in the existing LDAPBackend?

> --
> Jeff Schroeder
>
> Don't drink and derive, alcohol and analysis don't mix.
> http://www.digitalprognosis.com
>

--
Jeff Schroeder

Don't drink and derive, alcohol and analysis don't mix.
http://www.digitalprognosis.com

[Jeff Schroeder]

Paul,

If the code is cleaned up, and suitable to your standards, would you
merge a subclass which worked for Active Directory things out of the
box, or will you make that work in the existing LDAPBackend?

On Thu, Mar 24, 2011 at 1:35 PM, Felipe Prenholato <phili...@gmail.com> wrote:

--

[Peter Sagerson]

A version of LDAPBackend that works well with Active Directory out of the box sounds like a fine idea. I don't have any experience with or access to AD, so it's not really something that I can maintain. I can think of a few general approaches to such a thing.

1. Fork django_auth_ldap and customize it for AD. This is obviously the sledgehammer approach and not ideal.

2. Create a separate project (django_auth_ldap_ad?) that extends django_auth_ldap by implementing all of the hooks and signals necessary to give it AD-specific functionality. If some enterprising soul wanted to take that on, I would be happy to support the effort by exposing additional hooks and customization points as necessary.

3. The same as #2, but ship the derived classes and support code somewhere in the django_auth_ldap distribution. This could be interesting logistically, as someone else would still have to maintain it.

4. Somehow integrate an AD "mode" into the existing django_auth_ldap. At that point, we're talking about creating a new level of abstraction whereby django_auth_ldap has pre-defined configurations to support--in principle--a variety of common servers and configurations, starting with AD. One could imagine building up a set of configurations that correspond to the default installations of various platforms and distributions. An intriguing idea, but one that would definitely require some careful research and design work.

My instinct would be to start with #2. If there's interest in a strategy for providing push-button configurations for common server designs, doing an AD implementation and letting it bake for a while would probably provide some very useful insights into the right way to go about that. It may be that django_auth_ldap would just evolve a more powerful plugin API of some kind and server-specific configurations would be expected to live as separate modules.

Thanks

[Felipe Prenholato]

I using active directory and what I see for now is that I need LDAP.OPT_REFERRALS as 0 to work with ldap and fields in AD are different than in OpenLdap.

Someone with more experience in different types of LDAP here should talk, but I guess a dict mapping attributes to openldap, AD, another kind of ldap if exists, etc, ... to fixed names can solve most of differentes.

I just did it.

[Peter Sagerson]

Adding a "Tips for Active Directory" section to the documentation would also be a sensible thing to do for the simple stuff.

[Felipe Prenholato]

Hello Peter.

I used 1.0.9 right now and basically, what I did is create a custom backend.py in my module with changes needed: https://gist.github.com/895432

Now what I can talk is that, djang-auth-ldap should try create profile by it self instead depend of a signal to create a profile, because in major of cases, user_profile doesn't have complex fields with foreign keys and mapping **should** works.

The idea is: just get the Profile class from settings and do a get_or_create(user=self._user) (backends.py @ 486), if not works raises a warning with error that we have today instead of just log it when debugging.

What about?

Felipe 'chronos' Prenholato.
Linux User nº 405489
Home page: http://chronosbox.org/blog
Twitter: http://twitter.com/chronossc

2011/3/25 Peter Sagerson <psa...@ignorare.net>

[Peter Sagerson]

Even if we get the class, we don't actually know the name of the foreign key field, so we'd have to go spelunking for that. This would also contravene the Django docs, which state pretty clearly that creating profiles is the app developer's responsibility.[1] Automatically creating profiles in the LDAP backend might be a minor convenience to some, but it would also make it more difficult to seamlessly switch from one auth backend to another since they won't all do it. In the end, any robust application would end up with the same amount of code and less deterministic operation.

Hopefully there's no barrier to using the post_save handler as Django recommends?

[1] http://docs.djangoproject.com/en/1.3/topics/auth/#storing-additional-information-about-users

[Di majo]

MT103/202 DIRECT WIRE TRANSFER
PAYPAL TRANSFER
CASHAPP TRANSFER
ZELLE TRANSFER
LOAN DEAL
TRANSFER WISE
WESTERN UNION TRANSFER
BITCOIN FLASHING
BANK ACCOUNT LOADING/FLASHING
IBAN TO IBAN TRANSFER
MONEYGRAM TRANSFER
IPIP/DTC
SLBC PROVIDER
CREDIT CARD TOP UP
DUMPS/ PINS
SEPA TRANSFER
WIRE TRANSFER
BITCOIN TOP UP
GLOBALPAY INC US
SKRILL USA
UNIONPAY RECEIVER

Thanks.


NOTE; ONLY SERIOUS / RELIABLE RECEIVERS CAN CONTACT.

DM ME ON WHATSAPP
+44 7529 555638