Opened 14 years ago
Closed 12 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 by , 14 years ago
Triage Stage: | Unreviewed → Design decision needed |
---|
comment:2 by , 14 years ago
Cc: | added |
---|
comment:3 by , 14 years ago
milestone: | 1.3 |
---|---|
Triage Stage: | Design decision needed → 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.
follow-up: 5 comment:4 by , 14 years ago
Replying to jezdez:
So this is entirely possible right now by overriding the password_change view's
password_change_form
parameter with a subclass ofSetPasswordForm/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 by , 14 years ago
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 ofSetPasswordForm/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 itssave()
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 by , 14 years ago
Severity: | → Normal |
---|---|
Type: | → New feature |
comment:9 by , 12 years ago
Resolution: | → fixed |
---|---|
Status: | new → 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
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