Opened 6 years ago

Closed 5 years ago

#14827 closed New feature (fixed)

Authentication Backends should be responsible for checking and setting passwords

Reported by: andornaut Owned by: nobody
Component: contrib.auth Version: 1.2
Severity: Normal Keywords:
Cc: David Reynolds Triage Stage: Someday/Maybe
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Background & Reasoning

This feature request arose when trying to change the default hash algorithm from SHA1 to MD5. The django.contrib.auth.models.User.set_password() method assumes sha1. This method is called in a number of places, such as SetPasswordForm.save() and UserCreationForm.save(). User can be subclassed and set_password is overridden, but this change would be intrusive, because the new User Type would have to be substituted in several built-in views and in other places.

Suggested Solution

Instead, the scope of the authentication backend should be expanded to handle checking and setting passwords. Since multiple backends are supported, the first one in the list will take on these responsibilities.

Here is an example of some of the potential changes, for illustrative purposes:

# django.contrib.auth.backends.py
def get_authoritative_backend():   
   # The first backend is responsible for checking and setting passwords
   return settings.AUTHENTICATION_BACKENDS[0]

# django.contrib.auth.models.User
def set_password(self, raw_password):
   backend = django.contrib.auth.backends.get_authoritative_backend()
   backend.set_password(self, raw_password)

def check_password(self, raw_password):
   backend = django.contrib.auth.backends.get_authoritative_backend()
   return backend.set_password(self, raw_password)

The advantage of this approach is that it allows for arbitrary hashing algorithms to be used, and it enables the rest of the system to be unaware and unaffected by these particulars.

Change History (9)

comment:1 Changed 6 years ago by David Reynolds

Triage Stage: UnreviewedDesign decision needed

I like the idea of this, not only because it allows you to change hash method easily, but it makes it easier to use the standard auth forms/views to manage users from different authentication providers

comment:2 Changed 6 years ago by David Reynolds

Cc: David Reynolds added

comment:3 Changed 6 years ago by Jannis Leidel

milestone: 1.3
Triage Stage: Design decision neededSomeday/Maybe

So this is entirely possible right now by overriding the password_change view's password_change_form parameter with a subclass of SetPasswordForm/PasswordChangeForm that overrides its save() method to save the password somewhere else.

comment:4 Changed 6 years ago by andornaut

Replying to jezdez:

So this is entirely possible right now by overriding the password_change view's password_change_form parameter with a subclass of SetPasswordForm/PasswordChangeForm that overrides its save() method to save the password somewhere else.

The User model's save() method will still default to md5. Subclassing User in various places in order to override its save() seems like a less ideal solution.

Additionally, this change does not need to be disruptive. It could default to the current behavior if the AuthenticationBackend doesn't provide save and check password functionality.

comment:5 in reply to:  4 Changed 6 years ago by Jannis Leidel

Replying to andornaut:

Replying to jezdez:

So this is entirely possible right now by overriding the password_change view's password_change_form parameter with a subclass of SetPasswordForm/PasswordChangeForm that overrides its save() method to save the password somewhere else.

The User model's save() method will still default to md5. Subclassing User in various places in order to override its save() seems like a less ideal solution.

Additionally, this change does not need to be disruptive. It could default to the current behavior if the AuthenticationBackend doesn't provide save and check password functionality.

While the waterfall approach for checking credentials works pretty good, I don't think this should be applied to saving passwords, too. It adds a bit of uncertainty to something rather important. Feel free to start a discussion on django-developers if you want to discuss this further.

comment:6 Changed 6 years ago by anonymous

Severity: Normal
Type: New feature

comment:7 Changed 5 years ago by Aymeric Augustin

UI/UX: unset

Change UI/UX from NULL to False.

comment:8 Changed 5 years ago by Aymeric Augustin

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:9 Changed 5 years ago by Claude Paroz

Resolution: fixed
Status: newclosed

I think and hope that the recently added password hashers mechanism should fulfill this use case.

https://docs.djangoproject.com/en/dev/topics/auth/#how-django-stores-passwords

Note: See TracTickets for help on using tickets.
Back to Top