diff --git a/ietf/ietfauth/forms.py b/ietf/ietfauth/forms.py index a70f7b6ca1..53bf7b5888 100644 --- a/ietf/ietfauth/forms.py +++ b/ietf/ietfauth/forms.py @@ -3,16 +3,19 @@ import re + from unidecode import unidecode from django import forms +from django.contrib.auth.models import User +from django.contrib.auth import password_validation from django.core.exceptions import ValidationError from django.db import models -from django.contrib.auth.models import User from ietf.person.models import Person, Email from ietf.mailinglists.models import Allowlisted from ietf.utils.text import isascii +from .password_validation import StrongPasswordValidator from .validators import prevent_at_symbol, prevent_system_name, prevent_anonymous_name, is_allowed_address from .widgets import PasswordStrengthInput, PasswordConfirmationInput @@ -170,33 +173,52 @@ class Meta: model = Allowlisted exclude = ['by', 'time' ] - -from django import forms - class ChangePasswordForm(forms.Form): current_password = forms.CharField(widget=forms.PasswordInput) - new_password = forms.CharField(widget=PasswordStrengthInput(attrs={'class':'password_strength'})) - new_password_confirmation = forms.CharField(widget=PasswordConfirmationInput( - confirm_with='new_password', - attrs={'class':'password_confirmation'})) + new_password = forms.CharField( + widget=PasswordStrengthInput( + attrs={ + "class": "password_strength", + "data-disable-strength-enforcement": "", # usually removed in init + } + ), + ) + new_password_confirmation = forms.CharField( + widget=PasswordConfirmationInput( + confirm_with="new_password", attrs={"class": "password_confirmation"} + ) + ) def __init__(self, user, data=None): self.user = user - super(ChangePasswordForm, self).__init__(data) + super().__init__(data) + # Check whether we have validators to enforce + new_password_field = self.fields["new_password"] + for pwval in password_validation.get_default_password_validators(): + if isinstance(pwval, password_validation.MinimumLengthValidator): + new_password_field.widget.attrs["minlength"] = pwval.min_length + elif isinstance(pwval, StrongPasswordValidator): + new_password_field.widget.attrs.pop( + "data-disable-strength-enforcement", None + ) def clean_current_password(self): - password = self.cleaned_data.get('current_password', None) + # n.b., password = None is handled by check_password and results in a failed check + password = self.cleaned_data.get("current_password", None) if not self.user.check_password(password): - raise ValidationError('Invalid password') + raise ValidationError("Invalid password") return password - + def clean(self): - new_password = self.cleaned_data.get('new_password', None) - conf_password = self.cleaned_data.get('new_password_confirmation', None) - if not new_password == conf_password: - raise ValidationError("The password confirmation is different than the new password") + new_password = self.cleaned_data.get("new_password", "") + conf_password = self.cleaned_data.get("new_password_confirmation", "") + if new_password != conf_password: + raise ValidationError( + "The password confirmation is different than the new password" + ) + password_validation.validate_password(conf_password, self.user) class ChangeUsernameForm(forms.Form): diff --git a/ietf/ietfauth/password_validation.py b/ietf/ietfauth/password_validation.py new file mode 100644 index 0000000000..bfed4a784e --- /dev/null +++ b/ietf/ietfauth/password_validation.py @@ -0,0 +1,23 @@ +# Copyright The IETF Trust 2025, All Rights Reserved +from django.core.exceptions import ValidationError +from zxcvbn import zxcvbn + + +class StrongPasswordValidator: + message = "This password does not meet complexity requirements and is easily guessable." + code = "weak" + min_zxcvbn_score = 3 + + def __init__(self, message=None, code=None, min_zxcvbn_score=None): + if message is not None: + self.message = message + if code is not None: + self.code = code + if min_zxcvbn_score is not None: + self.min_zxcvbn_score = min_zxcvbn_score + + def validate(self, password, user=None): + """Validate that a password is strong enough""" + strength_report = zxcvbn(password[:72], max_length=72) + if strength_report["score"] < self.min_zxcvbn_score: + raise ValidationError(message=self.message, code=self.code) diff --git a/ietf/ietfauth/tests.py b/ietf/ietfauth/tests.py index f6d7671bc9..dd23277b63 100644 --- a/ietf/ietfauth/tests.py +++ b/ietf/ietfauth/tests.py @@ -392,14 +392,14 @@ def test_nomcom_dressing_on_profile(self): self.assertFalse(q('#volunteer-button')) self.assertTrue(q('#volunteered')) - def test_reset_password(self): + VALID_PASSWORD = "complex-and-long-valid-password" + ANOTHER_VALID_PASSWORD = "very-complicated-and-lengthy-password" url = urlreverse("ietf.ietfauth.views.password_reset") - email = 'someone@example.com' - password = 'foobar' + email = "someone@example.com" user = PersonFactory(user__email=email).user - user.set_password(password) + user.set_password(VALID_PASSWORD) user.save() # get @@ -407,21 +407,23 @@ def test_reset_password(self): self.assertEqual(r.status_code, 200) # ask for reset, wrong username (form should not fail) - r = self.client.post(url, { 'username': "nobody@example.com" }) + r = self.client.post(url, {"username": "nobody@example.com"}) self.assertEqual(r.status_code, 200) q = PyQuery(r.content) self.assertTrue(len(q("form .is-invalid")) == 0) # ask for reset empty_outbox() - r = self.client.post(url, { 'username': user.username }) + r = self.client.post(url, {"username": user.username}) self.assertEqual(r.status_code, 200) self.assertEqual(len(outbox), 1) # goto change password page, logged in as someone else confirm_url = self.extract_confirm_url(outbox[-1]) other_user = UserFactory() - self.client.login(username=other_user.username, password=other_user.username + '+password') + self.client.login( + username=other_user.username, password=other_user.username + "+password" + ) r = self.client.get(confirm_url) self.assertEqual(r.status_code, 403) @@ -430,17 +432,32 @@ def test_reset_password(self): r = self.client.get(confirm_url) self.assertEqual(r.status_code, 200) q = PyQuery(r.content) - self.assertNotIn(user.username, q('.nav').text(), - 'user should not appear signed in while resetting password') + self.assertNotIn( + user.username, + q(".nav").text(), + "user should not appear signed in while resetting password", + ) # password mismatch - r = self.client.post(confirm_url, { 'password': 'secret', 'password_confirmation': 'nosecret' }) + r = self.client.post( + confirm_url, + { + "password": ANOTHER_VALID_PASSWORD, + "password_confirmation": ANOTHER_VALID_PASSWORD[::-1], + }, + ) self.assertEqual(r.status_code, 200) q = PyQuery(r.content) self.assertTrue(len(q("form .is-invalid")) > 0) # confirm - r = self.client.post(confirm_url, { 'password': 'secret', 'password_confirmation': 'secret' }) + r = self.client.post( + confirm_url, + { + "password": ANOTHER_VALID_PASSWORD, + "password_confirmation": ANOTHER_VALID_PASSWORD, + }, + ) self.assertEqual(r.status_code, 200) q = PyQuery(r.content) self.assertEqual(len(q("form .is-invalid")), 0) @@ -451,15 +468,18 @@ def test_reset_password(self): # login after reset request empty_outbox() - user.set_password(password) + user.set_password(VALID_PASSWORD) user.save() - r = self.client.post(url, { 'username': user.username }) + r = self.client.post(url, {"username": user.username}) self.assertEqual(r.status_code, 200) self.assertEqual(len(outbox), 1) confirm_url = self.extract_confirm_url(outbox[-1]) - r = self.client.post(urlreverse("ietf.ietfauth.views.login"), {'username': email, 'password': password}) + r = self.client.post( + urlreverse("ietf.ietfauth.views.login"), + {"username": email, "password": VALID_PASSWORD}, + ) r = self.client.get(confirm_url) self.assertEqual(r.status_code, 404) @@ -467,12 +487,12 @@ def test_reset_password(self): # change password after reset request empty_outbox() - r = self.client.post(url, { 'username': user.username }) + r = self.client.post(url, {"username": user.username}) self.assertEqual(r.status_code, 200) self.assertEqual(len(outbox), 1) confirm_url = self.extract_confirm_url(outbox[-1]) - user.set_password('newpassword') + user.set_password(ANOTHER_VALID_PASSWORD) user.save() r = self.client.get(confirm_url) @@ -586,98 +606,175 @@ def test_review_overview(self): self.assertEqual(ReviewWish.objects.filter(doc=doc, team=review_req.team).count(), 0) def test_change_password(self): + VALID_PASSWORD = "complex-and-long-valid-password" + ANOTHER_VALID_PASSWORD = "very-complicated-and-lengthy-password" chpw_url = urlreverse("ietf.ietfauth.views.change_password") prof_url = urlreverse("ietf.ietfauth.views.profile") login_url = urlreverse("ietf.ietfauth.views.login") - redir_url = '%s?next=%s' % (login_url, chpw_url) + redir_url = "%s?next=%s" % (login_url, chpw_url) # get without logging in r = self.client.get(chpw_url) self.assertRedirects(r, redir_url) - user = User.objects.create(username="someone@example.com", email="someone@example.com") - user.set_password("password") + user = User.objects.create( + username="someone@example.com", email="someone@example.com" + ) + user.set_password(VALID_PASSWORD) user.save() p = Person.objects.create(name="Some One", ascii="Some One", user=user) Email.objects.create(address=user.username, person=p, origin=user.username) # log in - r = self.client.post(redir_url, {"username":user.username, "password":"password"}) + r = self.client.post( + redir_url, {"username": user.username, "password": VALID_PASSWORD} + ) self.assertRedirects(r, chpw_url) # wrong current password - r = self.client.post(chpw_url, {"current_password": "fiddlesticks", - "new_password": "foobar", - "new_password_confirmation": "foobar", - }) + r = self.client.post( + chpw_url, + { + "current_password": "fiddlesticks", + "new_password": ANOTHER_VALID_PASSWORD, + "new_password_confirmation": ANOTHER_VALID_PASSWORD, + }, + ) self.assertEqual(r.status_code, 200) - self.assertFormError(r.context["form"], 'current_password', 'Invalid password') + self.assertFormError(r.context["form"], "current_password", "Invalid password") # mismatching new passwords - r = self.client.post(chpw_url, {"current_password": "password", - "new_password": "foobar", - "new_password_confirmation": "barfoo", - }) + r = self.client.post( + chpw_url, + { + "current_password": VALID_PASSWORD, + "new_password": ANOTHER_VALID_PASSWORD, + "new_password_confirmation": ANOTHER_VALID_PASSWORD[::-1], + }, + ) + self.assertEqual(r.status_code, 200) + self.assertFormError( + r.context["form"], + None, + "The password confirmation is different than the new password", + ) + + # password too short + r = self.client.post( + chpw_url, + { + "current_password": VALID_PASSWORD, + "new_password": "sh0rtpw0rd", + "new_password_confirmation": "sh0rtpw0rd", + } + ) self.assertEqual(r.status_code, 200) - self.assertFormError(r.context["form"], None, "The password confirmation is different than the new password") + self.assertFormError( + r.context["form"], + None, + "This password is too short. It must contain at least " + f"{settings.PASSWORD_POLICY_MIN_LENGTH} characters." + ) + + # password too simple + r = self.client.post( + chpw_url, + { + "current_password": VALID_PASSWORD, + "new_password": "passwordpassword", + "new_password_confirmation": "passwordpassword", + } + ) + self.assertEqual(r.status_code, 200) + self.assertFormError( + r.context["form"], + None, + "This password does not meet complexity requirements " + "and is easily guessable." + ) # correct password change - r = self.client.post(chpw_url, {"current_password": "password", - "new_password": "foobar", - "new_password_confirmation": "foobar", - }) + r = self.client.post( + chpw_url, + { + "current_password": VALID_PASSWORD, + "new_password": ANOTHER_VALID_PASSWORD, + "new_password_confirmation": ANOTHER_VALID_PASSWORD, + }, + ) self.assertRedirects(r, prof_url) # refresh user object user = User.objects.get(username="someone@example.com") - self.assertTrue(user.check_password('foobar')) + self.assertTrue(user.check_password(ANOTHER_VALID_PASSWORD)) def test_change_username(self): - + VALID_PASSWORD = "complex-and-long-valid-password" chun_url = urlreverse("ietf.ietfauth.views.change_username") prof_url = urlreverse("ietf.ietfauth.views.profile") login_url = urlreverse("ietf.ietfauth.views.login") - redir_url = '%s?next=%s' % (login_url, chun_url) + redir_url = "%s?next=%s" % (login_url, chun_url) # get without logging in r = self.client.get(chun_url) self.assertRedirects(r, redir_url) - user = User.objects.create(username="someone@example.com", email="someone@example.com") - user.set_password("password") + user = User.objects.create( + username="someone@example.com", email="someone@example.com" + ) + user.set_password(VALID_PASSWORD) user.save() p = Person.objects.create(name="Some One", ascii="Some One", user=user) Email.objects.create(address=user.username, person=p, origin=user.username) - Email.objects.create(address="othername@example.org", person=p, origin=user.username) + Email.objects.create( + address="othername@example.org", person=p, origin=user.username + ) # log in - r = self.client.post(redir_url, {"username":user.username, "password":"password"}) + r = self.client.post( + redir_url, {"username": user.username, "password": VALID_PASSWORD} + ) self.assertRedirects(r, chun_url) # wrong username - r = self.client.post(chun_url, {"username": "fiddlesticks", - "password": "password", - }) + r = self.client.post( + chun_url, + { + "username": "fiddlesticks", + "password": VALID_PASSWORD, + }, + ) self.assertEqual(r.status_code, 200) - self.assertFormError(r.context["form"], 'username', - "Select a valid choice. fiddlesticks is not one of the available choices.") + self.assertFormError( + r.context["form"], + "username", + "Select a valid choice. fiddlesticks is not one of the available choices.", + ) # wrong password - r = self.client.post(chun_url, {"username": "othername@example.org", - "password": "foobar", - }) + r = self.client.post( + chun_url, + { + "username": "othername@example.org", + "password": "foobar", + }, + ) self.assertEqual(r.status_code, 200) - self.assertFormError(r.context["form"], 'password', 'Invalid password') + self.assertFormError(r.context["form"], "password", "Invalid password") # correct username change - r = self.client.post(chun_url, {"username": "othername@example.org", - "password": "password", - }) + r = self.client.post( + chun_url, + { + "username": "othername@example.org", + "password": VALID_PASSWORD, + }, + ) self.assertRedirects(r, prof_url) # refresh user object prev = user user = User.objects.get(username="othername@example.org") self.assertEqual(prev, user) - self.assertTrue(user.check_password('password')) + self.assertTrue(user.check_password(VALID_PASSWORD)) def test_apikey_management(self): # Create a person with a role that will give at least one valid apikey diff --git a/ietf/ietfauth/views.py b/ietf/ietfauth/views.py index 23f66ce824..84d5490873 100644 --- a/ietf/ietfauth/views.py +++ b/ietf/ietfauth/views.py @@ -38,14 +38,14 @@ import importlib # needed if we revert to higher barrier for account creation -#from datetime import datetime as DateTime, timedelta as TimeDelta, date as Date +# from datetime import datetime as DateTime, timedelta as TimeDelta, date as Date from collections import defaultdict import django.core.signing from django import forms from django.contrib import messages from django.conf import settings -from django.contrib.auth import logout, update_session_auth_hash +from django.contrib.auth import logout, update_session_auth_hash, password_validation from django.contrib.auth.decorators import login_required from django.contrib.auth.forms import AuthenticationForm from django.contrib.auth.hashers import identify_hasher @@ -80,7 +80,6 @@ # These are needed if we revert to the higher bar for account creation - def index(request): return render(request, 'registration/index.html') @@ -97,7 +96,7 @@ def index(request): # def ietf_login(request): # if not request.user.is_authenticated: # return HttpResponse("Not authenticated?", status=500) -# +# # redirect_to = request.REQUEST.get(REDIRECT_FIELD_NAME, '') # request.session.set_test_cookie() # return HttpResponseRedirect('/accounts/loggedin/?%s=%s' % (REDIRECT_FIELD_NAME, urlquote(redirect_to))) @@ -582,7 +581,6 @@ def test_email(request): return r - class AddReviewWishForm(forms.Form): doc = SearchableDocumentField(label="Document", doc_type="draft") team = forms.ModelChoiceField(queryset=Group.objects.all(), empty_label="(Choose review team)") @@ -696,7 +694,7 @@ def change_password(request): 'hasher': hasher, }) - + @login_required @person_required def change_username(request): @@ -764,6 +762,28 @@ def clean(self): ) return super().clean() + def confirm_login_allowed(self, user): + """Check whether a successfully authenticated user is permitted to log in""" + super().confirm_login_allowed(user) + # Optionally enforce password validation + if getattr(settings, "PASSWORD_POLICY_ENFORCE_AT_LOGIN", False): + try: + password_validation.validate_password( + self.cleaned_data["password"], user + ) + except ValidationError: + raise ValidationError( + # dict mapping field to error / error list + { + "__all__": ValidationError( + 'You entered your password correctly, but it does not ' + 'meet our current length and complexity requirements. ' + 'Please use the "Forgot your password?" button below to ' + 'set a new password for your account.' + ), + } + ) + class AnyEmailLoginView(LoginView): """LoginView that allows any email address as the username @@ -779,7 +799,7 @@ def form_valid(self, form): logout(self.request) # should not be logged in yet, but just in case... return render(self.request, "registration/missing_person.html") return super().form_valid(form) - + @login_required @person_required diff --git a/ietf/ietfauth/widgets.py b/ietf/ietfauth/widgets.py index c9a0523402..fd7fa16726 100644 --- a/ietf/ietfauth/widgets.py +++ b/ietf/ietfauth/widgets.py @@ -39,18 +39,19 @@ def render(self, name, value, attrs=None, renderer=None): strength_markup = """
{{ hasher.algorithm }}
diff --git a/k8s/settings_local.py b/k8s/settings_local.py
index 074888728f..7a3c369750 100644
--- a/k8s/settings_local.py
+++ b/k8s/settings_local.py
@@ -391,3 +391,8 @@ def _multiline_to_list(s):
"EXCLUDE_BUCKETS": ["staging"],
"VERBOSE_LOGGING": _blobdb_replication_verbose_logging,
}
+
+# Optionally disable password strength enforcement at login (on by default)
+PASSWORD_POLICY_ENFORCE_AT_LOGIN = (
+ os.environ.get("DATATRACKER_ENFORCE_PW_POLICY", "true").lower() != "false"
+)
diff --git a/requirements.txt b/requirements.txt
index eb72600fe3..4eb573ce36 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -79,8 +79,10 @@ selenium>=4.0
tblib>=1.7.0 # So that the django test runner provides tracebacks
tlds>=2022042700 # Used to teach bleach about which TLDs currently exist
tqdm>=4.64.0
+types-zxcvbn~=4.5.0.20250223 # match zxcvbn version
Unidecode>=1.3.4
urllib3>=1.26,<2
weasyprint>=64.1
xml2rfc>=3.23.0
xym>=0.6,<1.0
+zxcvbn>=4.5.0