Skip to content
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

Support basic authentication with custom user models that change username field #2952

Merged
merged 6 commits into from
May 20, 2015

Conversation

Ernest0x
Copy link
Contributor

Support basic authentication with custom user models with a username field that is not named 'username'.

Ernest0x added 3 commits May 19, 2015 17:42
…name field

Support basic authentication with custom user models with a username field that is not named 'username'.
Compatibility code for getting user model
Import get_user_model from compat module to be compatible with older django versions (e.g. 1.4).
@tomchristie
Copy link
Member

Using the username argument appears to work fine with Django's ModelBackend auth backend, regardless of the name of the username field. Looks like it only uses UserModel.USERNAME_FIELD as a back up. https://docs.djangoproject.com/en/1.8/_modules/django/contrib/auth/backends/

Looks okay, but I might need a little more persuading of the necessity for the change.

Support User model in Django 1.4 that has not a USERNAME_FIELD attribute.
@Ernest0x
Copy link
Contributor Author

Using the username argument appears to work fine with Django's ModelBackend auth backend, regardless of the name of the username field. Looks like it only uses UserModel.USERNAME_FIELD as a back up. https://docs.djangoproject.com/en/1.8/_modules/django/contrib/auth/backends/

Looks okay, but I might need a little more persuading of the necessity for the change.

It works fine with Django's ModelBackend but it may not work with other backends, so better be safer than sorry. Particularly, it does not work with django-python3-ldap.

According to Django's documentation, an authentication backend is required to implement authenticate(**credentials), which does not dictate the parameter names. That said, using USERNAME_FIELD instead of a hardcoded 'username' is a safer approach imho.

if hasattr(user_model, 'USERNAME_FIELD'):
username_field = user_model.USERNAME_FIELD
else:
username_field = 'username'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could slim this down with username_field = getattr(user_model, 'USERNAME_FIELD', 'username') I think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just addressed that

Ernest0x added 2 commits May 19, 2015 19:48
Fix flake8 error
On Tom's suggestion, improve coding style by using a single-line call to getattr() with a default value instead of a multi-line if/else clause.
@tomchristie tomchristie added this to the 3.1.3 Release milestone May 20, 2015
tomchristie added a commit that referenced this pull request May 20, 2015
Support basic authentication with custom user models that change username field
@tomchristie tomchristie merged commit 010f2ee into encode:master May 20, 2015
@tomchristie
Copy link
Member

Ace! ✨

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

Successfully merging this pull request may close these issues.

2 participants