Code

Opened 3 years ago

Closed 23 months 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: DavidReynolds 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.

Attachments (0)

Change History (9)

comment:1 Changed 3 years ago by DavidReynolds

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Design 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 3 years ago by DavidReynolds

  • Cc DavidReynolds added

comment:3 Changed 3 years ago by jezdez

  • milestone 1.3 deleted
  • Triage Stage changed from Design decision needed to Someday/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 follow-up: Changed 3 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 3 years ago by jezdez

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 3 years ago by anonymous

  • Severity set to Normal
  • Type set to New feature

comment:7 Changed 2 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:8 Changed 2 years ago by aaugustin

  • Easy pickings unset

Change Easy pickings from NULL to False.

comment:9 Changed 23 months ago by claudep

  • Resolution set to fixed
  • Status changed from new to closed

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

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.