Skip to content

Commit

Permalink
Fixed #34429 -- Allowed setting unusable passwords for users in the a…
Browse files Browse the repository at this point in the history
…uth forms.

Co-authored-by: Natalia <[email protected]>
  • Loading branch information
fsbraun and nessita committed Feb 20, 2024
1 parent 8a75724 commit e626716
Show file tree
Hide file tree
Showing 12 changed files with 581 additions and 42 deletions.
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,7 @@ answer newbie questions, and generally made Django that much better:
Eugene Lazutkin <http://lazutkin.com/blog/>
Evan Grim <https://github.com/egrim>
Fabian Büchler <[email protected]>
Fabian Braun <[email protected]>
Fabrice Aneche <[email protected]>
Faishal Manzar <https://github.com/faishal882>
Farhaan Bukhsh <[email protected]>
Expand Down
19 changes: 19 additions & 0 deletions django/contrib/admin/static/admin/css/unusable_password_field.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/* Hide warnings fields if usable password is selected */
form:has(#id_usable_password input[value="true"]:checked) .messagelist {
display: none;
}

/* Hide password fields if unusable password is selected */
form:has(#id_usable_password input[value="false"]:checked) .field-password1,
form:has(#id_usable_password input[value="false"]:checked) .field-password2 {
display: none;
}

/* Select appropriate submit button */
form:has(#id_usable_password input[value="true"]:checked) input[type="submit"].unset-password {
display: none;
}

form:has(#id_usable_password input[value="false"]:checked) input[type="submit"].set-password {
display: none;
}
29 changes: 29 additions & 0 deletions django/contrib/admin/static/admin/js/unusable_password_field.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
"use strict";
// Fallback JS for browsers which do not support :has selector used in
// admin/css/unusable_password_fields.css
// Remove file once all supported browsers support :has selector
try {
// If browser does not support :has selector this will raise an error
document.querySelector("form:has(input)");
} catch (error) {
console.log("Defaulting to javascript for usable password form management: " + error);
// JS replacement for unsupported :has selector
document.querySelectorAll('input[name="usable_password"]').forEach(option => {
option.addEventListener('change', function() {
const usablePassword = (this.value === "true" ? this.checked : !this.checked);
const submit1 = document.querySelector('input[type="submit"].set-password');
const submit2 = document.querySelector('input[type="submit"].unset-password');
const messages = document.querySelector('#id_unusable_warning');
document.getElementById('id_password1').closest('.form-row').hidden = !usablePassword;
document.getElementById('id_password2').closest('.form-row').hidden = !usablePassword;
if (messages) {
messages.hidden = usablePassword;
}
if (submit1 && submit2) {
submit1.hidden = !usablePassword;
submit2.hidden = usablePassword;
}
});
option.dispatchEvent(new Event('change'));
});
}
10 changes: 9 additions & 1 deletion django/contrib/admin/templates/admin/auth/user/add_form.html
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{% extends "admin/change_form.html" %}
{% load i18n %}
{% load i18n static %}

{% block form_top %}
{% if not is_popup %}
Expand All @@ -8,3 +8,11 @@
<p>{% translate "Enter a username and password." %}</p>
{% endif %}
{% endblock %}
{% block extrahead %}
{{ block.super }}
<link rel="stylesheet" href="{% static 'admin/css/unusable_password_field.css' %}">
{% endblock %}
{% block admin_change_form_document_ready %}
{{ block.super }}
<script src="{% static 'admin/js/unusable_password_field.js' %}" defer></script>
{% endblock %}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@
{% load i18n static %}
{% load admin_urls %}

{% block extrastyle %}{{ block.super }}<link rel="stylesheet" href="{% static "admin/css/forms.css" %}">{% endblock %}
{% block extrastyle %}
{{ block.super }}
<link rel="stylesheet" href="{% static "admin/css/forms.css" %}">
<link rel="stylesheet" href="{% static 'admin/css/unusable_password_field.css' %}">
{% endblock %}
{% block bodyclass %}{{ block.super }} {{ opts.app_label }}-{{ opts.model_name }} change-form{% endblock %}
{% if not is_popup %}
{% block breadcrumbs %}
Expand All @@ -11,7 +15,7 @@
&rsaquo; <a href="{% url 'admin:app_list' app_label=opts.app_label %}">{{ opts.app_config.verbose_name }}</a>
&rsaquo; <a href="{% url opts|admin_urlname:'changelist' %}">{{ opts.verbose_name_plural|capfirst }}</a>
&rsaquo; <a href="{% url opts|admin_urlname:'change' original.pk|admin_urlquote %}">{{ original|truncatewords:"18" }}</a>
&rsaquo; {% translate 'Change password' %}
&rsaquo; {% if form.user.has_usable_password %}{% translate 'Change password' %}{% else %}{% translate 'Set password' %}{% endif %}
</div>
{% endblock %}
{% endif %}
Expand All @@ -27,18 +31,31 @@
{% endif %}

<p>{% blocktranslate with username=original %}Enter a new password for the user <strong>{{ username }}</strong>.{% endblocktranslate %}</p>
{% if not form.user.has_usable_password %}
<p>{% blocktranslate %}This action will <strong>enable</strong> password-based authentication for this user.{% endblocktranslate %}</p>
{% endif %}

<fieldset class="module aligned">

<div class="form-row">
{{ form.usable_password.errors }}
<div class="flex-container">{{ form.usable_password.label_tag }} {{ form.usable_password }}</div>
{% if form.usable_password.help_text %}
<div class="help"{% if form.usable_password.id_for_label %} id="{{ form.usable_password.id_for_label }}_helptext"{% endif %}>
<p>{{ form.usable_password.help_text|safe }}</p>
</div>
{% endif %}
</div>

<div class="form-row field-password1">
{{ form.password1.errors }}
<div class="flex-container">{{ form.password1.label_tag }} {{ form.password1 }}</div>
{% if form.password1.help_text %}
<div class="help"{% if form.password1.id_for_label %} id="{{ form.password1.id_for_label }}_helptext"{% endif %}>{{ form.password1.help_text|safe }}</div>
{% endif %}
</div>

<div class="form-row">
<div class="form-row field-password2">
{{ form.password2.errors }}
<div class="flex-container">{{ form.password2.label_tag }} {{ form.password2 }}</div>
{% if form.password2.help_text %}
Expand All @@ -49,9 +66,15 @@
</fieldset>

<div class="submit-row">
<input type="submit" value="{% translate 'Change password' %}" class="default">
{% if form.user.has_usable_password %}
<input type="submit" name="set-password" value="{% translate 'Change password' %}" class="default set-password">
<input type="submit" name="unset-password" value="{% translate 'Disable password-based authentication' %}" class="unset-password">
{% else %}
<input type="submit" name="set-password" value="{% translate 'Enable password-based authentication' %}" class="default set-password">
{% endif %}
</div>

</div>
</form></div>
<script src="{% static 'admin/js/unusable_password_field.js' %}" defer></script>
{% endblock %}
29 changes: 25 additions & 4 deletions django/contrib/auth/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ class UserAdmin(admin.ModelAdmin):
None,
{
"classes": ("wide",),
"fields": ("username", "password1", "password2"),
"fields": ("username", "usable_password", "password1", "password2"),
},
),
)
Expand Down Expand Up @@ -164,10 +164,27 @@ def user_change_password(self, request, id, form_url=""):
if request.method == "POST":
form = self.change_password_form(user, request.POST)
if form.is_valid():
form.save()
# If disabling password-based authentication was requested
# (via the form field `usable_password`), the submit action
# must be "unset-password". This check is most relevant when
# the admin user has two submit buttons available (for example
# when Javascript is disabled).
valid_submission = (
form.cleaned_data["set_usable_password"]
or "unset-password" in request.POST
)
if not valid_submission:
msg = gettext("Conflicting form data submitted. Please try again.")
messages.error(request, msg)
return HttpResponseRedirect(request.get_full_path())

user = form.save()
change_message = self.construct_change_message(request, form, None)
self.log_change(request, user, change_message)
msg = gettext("Password changed successfully.")
if user.has_usable_password():
msg = gettext("Password changed successfully.")
else:
msg = gettext("Password-based authentication was disabled.")
messages.success(request, msg)
update_session_auth_hash(request, form.user)
return HttpResponseRedirect(
Expand All @@ -187,8 +204,12 @@ def user_change_password(self, request, id, form_url=""):
fieldsets = [(None, {"fields": list(form.base_fields)})]
admin_form = admin.helpers.AdminForm(form, fieldsets, {})

if user.has_usable_password():
title = _("Change password: %s")
else:
title = _("Set password: %s")
context = {
"title": _("Change password: %s") % escape(user.get_username()),
"title": title % escape(user.get_username()),
"adminForm": admin_form,
"form_url": form_url,
"form": form,
Expand Down
79 changes: 71 additions & 8 deletions django/contrib/auth/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,33 +92,78 @@ def widget_attrs(self, widget):
class SetPasswordMixin:
"""
Form mixin that validates and sets a password for a user.
This mixin also support setting an unusable password for a user.
"""

error_messages = {
"password_mismatch": _("The two password fields didn’t match."),
}
usable_password_help_text = _(
"Whether the user will be able to authenticate using a password or not. "
"If disabled, they may still be able to authenticate using other backends, "
"such as Single Sign-On or LDAP."
)

@staticmethod
def create_password_fields(label1=_("Password"), label2=_("Password confirmation")):
password1 = forms.CharField(
label=label1,
required=False,
strip=False,
widget=forms.PasswordInput(attrs={"autocomplete": "new-password"}),
help_text=password_validation.password_validators_help_text_html(),
)
password2 = forms.CharField(
label=label2,
required=False,
widget=forms.PasswordInput(attrs={"autocomplete": "new-password"}),
strip=False,
help_text=_("Enter the same password as before, for verification."),
)
return password1, password2

@staticmethod
def create_usable_password_field(help_text=usable_password_help_text):
return forms.ChoiceField(
label=_("Password-based authentication"),
required=False,
initial="true",
choices={"true": _("Enabled"), "false": _("Disabled")},
widget=forms.RadioSelect(attrs={"class": "radiolist inline"}),
help_text=help_text,
)

def validate_passwords(
self, password1_field_name="password1", password2_field_name="password2"
self,
password1_field_name="password1",
password2_field_name="password2",
usable_password_field_name="usable_password",
):
usable_password = (
self.cleaned_data.pop(usable_password_field_name, None) != "false"
)
self.cleaned_data["set_usable_password"] = usable_password
password1 = self.cleaned_data.get(password1_field_name)
password2 = self.cleaned_data.get(password2_field_name)

if not usable_password:
return self.cleaned_data

if not password1:
error = ValidationError(
self.fields[password1_field_name].error_messages["required"],
code="required",
)
self.add_error(password1_field_name, error)

if not password2:
error = ValidationError(
self.fields[password2_field_name].error_messages["required"],
code="required",
)
self.add_error(password2_field_name, error)

if password1 and password2 and password1 != password2:
error = ValidationError(
self.error_messages["password_mismatch"],
Expand All @@ -128,14 +173,17 @@ def validate_passwords(

def validate_password_for_user(self, user, password_field_name="password2"):
password = self.cleaned_data.get(password_field_name)
if password:
if password and self.cleaned_data["set_usable_password"]:
try:
password_validation.validate_password(password, user)
except ValidationError as error:
self.add_error(password_field_name, error)

def set_password_and_save(self, user, password_field_name="password1", commit=True):
user.set_password(self.cleaned_data[password_field_name])
if self.cleaned_data["set_usable_password"]:
user.set_password(self.cleaned_data[password_field_name])
else:
user.set_unusable_password()
if commit:
user.save()
return user
Expand All @@ -148,6 +196,7 @@ class BaseUserCreationForm(SetPasswordMixin, forms.ModelForm):
"""

password1, password2 = SetPasswordMixin.create_password_fields()
usable_password = SetPasswordMixin.create_usable_password_field()

class Meta:
model = User
Expand Down Expand Up @@ -205,7 +254,7 @@ class UserChangeForm(forms.ModelForm):
label=_("Password"),
help_text=_(
"Raw passwords are not stored, so there is no way to see this "
"user’s password, but you can change the password using "
"user’s password, but you can change or unset the password using "
'<a href="{}">this form</a>.'
),
)
Expand All @@ -219,6 +268,11 @@ def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
password = self.fields.get("password")
if password:
if self.instance and not self.instance.has_usable_password():
password.help_text = _(
"Enable password-based authentication for this user by setting a "
'password using <a href="{}">this form</a>.'
)
password.help_text = password.help_text.format(
f"../../{self.instance.pk}/password/"
)
Expand Down Expand Up @@ -472,12 +526,22 @@ class AdminPasswordChangeForm(SetPasswordMixin, forms.Form):
"""

required_css_class = "required"
usable_password_help_text = SetPasswordMixin.usable_password_help_text + (
'<ul id="id_unusable_warning" class="messagelist"><li class="warning">'
"If disabled, the current password for this user will be lost.</li></ul>"
)
password1, password2 = SetPasswordMixin.create_password_fields()

def __init__(self, user, *args, **kwargs):
self.user = user
super().__init__(*args, **kwargs)
self.fields["password1"].widget.attrs["autofocus"] = True
if self.user.has_usable_password():
self.fields["usable_password"] = (
SetPasswordMixin.create_usable_password_field(
self.usable_password_help_text
)
)

def clean(self):
self.validate_passwords()
Expand All @@ -491,7 +555,6 @@ def save(self, commit=True):
@property
def changed_data(self):
data = super().changed_data
for name in self.fields:
if name not in data:
return []
return ["password"]
if "set_usable_password" in data or "password1" in data and "password2" in data:
return ["password"]
return []
6 changes: 6 additions & 0 deletions docs/releases/5.1.txt
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,12 @@ Minor features
* The default iteration count for the PBKDF2 password hasher is increased from
720,000 to 870,000.

* :class:`~django.contrib.auth.forms.BaseUserCreationForm` and
:class:`~django.contrib.auth.forms.AdminPasswordChangeForm` now support
disabling password-based authentication by setting an unusable password on
form save. This is now available in the admin when visiting the user creation
and password change pages.

:mod:`django.contrib.contenttypes`
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Expand Down
Loading

0 comments on commit e626716

Please sign in to comment.