New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allauth incompatible with auth-ldap #199

Closed
jbazik opened this Issue Feb 26, 2013 · 4 comments

Comments

Projects
None yet
2 participants
@jbazik
Copy link
Contributor

jbazik commented Feb 26, 2013

I'd like to run allauth side-by-side with auth-ldap. I have local ldap users, but I'd like also
to support non-local and social network logins.

The problem is that the User model requires a unique username, and allauth provides one
(if not supplied by the user) by generating it based on varying seed values (which depend
on whether it is a social network or email login). Because ldap usernames are externally
defined, there is a potential for conflicts.

My problem can be solved simply if allauth could take a configuration option to
provide that seed value. Then I can carefully choose it to not ever conflict with my
ldap username values.

So, if I offer a patch that introduces configuration options such as
ACCOUNT_USERNAME_PREFIX and SOCIALACCOUNT_USERNAME_PREFIX,
which would be used by generate_unique_username to create a unique username,
would you consider a pull request?

@pennersr

This comment has been minimized.

Copy link
Owner

pennersr commented Feb 26, 2013

In this case I am not really in favor of adding yet another setting. Your use case is rather specific, and a prefix-like setting is rather arbitrary -- I don't expect it to be useful in many other use cases. Still, I would like to accomodate for your scenario.

How about moving the generate_unique_username to allauth.account.adapter.DefaultAccountAdapter ? Then, in your project you could override the default adapter, adding prefixes where needed. I would gladly accept a pull request if you opt for this route ...

@jbazik

This comment has been minimized.

Copy link
Contributor Author

jbazik commented Feb 27, 2013

Sure, any way you'd like to do it. That's why I asked.

jbazik added a commit to jbazik/django-allauth that referenced this issue Feb 28, 2013

@jbazik

This comment has been minimized.

Copy link
Contributor Author

jbazik commented Mar 12, 2013

Pull request is #201. I'd like to know if you're okay with that before I code against it in production...

@pennersr

This comment has been minimized.

Copy link
Owner

pennersr commented Mar 13, 2013

Fixed in 85cd7ca

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment