From 4e143f28aebcf5b6b0be2aa16f44d87bcc787ed0 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 23 Apr 2025 00:38:40 -0300 Subject: [PATCH 01/29] feat: enforce pw strength at login --- ietf/ietfauth/views.py | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/ietf/ietfauth/views.py b/ietf/ietfauth/views.py index 23f66ce824..8a8dcc41fa 100644 --- a/ietf/ietfauth/views.py +++ b/ietf/ietfauth/views.py @@ -36,6 +36,7 @@ import datetime import importlib +import string # needed if we revert to higher barrier for account creation #from datetime import datetime as DateTime, timedelta as TimeDelta, date as Date @@ -729,7 +730,32 @@ def change_username(request): return render(request, 'registration/change_username.html', {'form': form}) -class AnyEmailAuthenticationForm(AuthenticationForm): +class StrongPasswordAuthenticationForm(AuthenticationForm): + def _is_strong_password(self, password: str): + if settings.SERVER_MODE == "development": + return True + + # todo choose a real set of password strength criteria + has_lowercase = not set(string.ascii_lowercase).isdisjoint(password) + has_uppercase = not set(string.ascii_uppercase).isdisjoint(password) + has_digit = not set(string.digits).isdisjoint(password) + return has_lowercase and has_uppercase and has_digit and len(password) >= 10 + + def clean(self): + result = super().clean() # raises an exception on login failure + + # Check whether the otherwise successfully authenticated user has a strong password + if not self._is_strong_password(self.cleaned_data.get("password")): + self.add_error( + "password", + 'Your password does not meet complexity requirements and is easily guessable. ' + 'Please use the "Forgot your password?" button below to set a new password ' + 'for your account.', + ) + return result + + +class AnyEmailAuthenticationForm(StrongPasswordAuthenticationForm): """AuthenticationForm that allows any email address as the username Also performs a check for a cleared password field and provides a helpful error message From 7d6b1f4c50b9738e021dc7420cb309f9f425e18d Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 10 Jun 2025 14:33:45 -0300 Subject: [PATCH 02/29] chore(deps): add zxcvbn to requirements.txt --- requirements.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/requirements.txt b/requirements.txt index eb72600fe3..b1469d7216 100644 --- a/requirements.txt +++ b/requirements.txt @@ -84,3 +84,4 @@ urllib3>=1.26,<2 weasyprint>=64.1 xml2rfc>=3.23.0 xym>=0.6,<1.0 +zxcvbn>=4.5.0 From 001dfaf58556a35e8d219ed726c6db5ee9f29988 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 10 Jun 2025 17:02:17 -0300 Subject: [PATCH 03/29] feat: use zxcvbn for password strength check --- ietf/ietfauth/validators.py | 21 +++++++++++++++++++++ ietf/ietfauth/views.py | 33 ++++++++++++--------------------- requirements.txt | 1 + 3 files changed, 34 insertions(+), 21 deletions(-) diff --git a/ietf/ietfauth/validators.py b/ietf/ietfauth/validators.py index 84684f34d5..3ad3b378c0 100644 --- a/ietf/ietfauth/validators.py +++ b/ietf/ietfauth/validators.py @@ -4,6 +4,7 @@ from django import forms from django.conf import settings from django.core.exceptions import ValidationError +from zxcvbn import zxcvbn def prevent_at_symbol(name): @@ -32,3 +33,23 @@ def is_allowed_address(value): raise ValidationError( "This email address is not valid in a datatracker account" ) + + +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 __call__(self, password): + """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/views.py b/ietf/ietfauth/views.py index 8a8dcc41fa..cdc7c090ea 100644 --- a/ietf/ietfauth/views.py +++ b/ietf/ietfauth/views.py @@ -59,6 +59,7 @@ from django.http import Http404, HttpResponseRedirect, HttpResponseForbidden from django.shortcuts import render, redirect, get_object_or_404 from django.utils.encoding import force_bytes +from zxcvbn import zxcvbn import debug # pyflakes:ignore @@ -67,6 +68,7 @@ ChangePasswordForm, get_person_form, RoleEmailForm, NewEmailForm, ChangeUsernameForm, PersonPasswordForm) from ietf.ietfauth.utils import has_role, send_new_email_confirmation_request +from ietf.ietfauth.validators import StrongPasswordValidator from ietf.name.models import ExtResourceName from ietf.nomcom.models import NomCom from ietf.person.models import Person, Email, Alias, PersonalApiKey, PERSON_API_KEY_VALUES @@ -731,28 +733,17 @@ def change_username(request): class StrongPasswordAuthenticationForm(AuthenticationForm): - def _is_strong_password(self, password: str): - if settings.SERVER_MODE == "development": - return True - - # todo choose a real set of password strength criteria - has_lowercase = not set(string.ascii_lowercase).isdisjoint(password) - has_uppercase = not set(string.ascii_uppercase).isdisjoint(password) - has_digit = not set(string.digits).isdisjoint(password) - return has_lowercase and has_uppercase and has_digit and len(password) >= 10 - - def clean(self): - result = super().clean() # raises an exception on login failure - - # Check whether the otherwise successfully authenticated user has a strong password - if not self._is_strong_password(self.cleaned_data.get("password")): - self.add_error( - "password", - 'Your password does not meet complexity requirements and is easily guessable. ' - 'Please use the "Forgot your password?" button below to set a new password ' - 'for your account.', + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.fields["password"].validators.append( + StrongPasswordValidator( + message=( + "Your password does not meet complexity requirements and is " + 'easily guessable. Please use the "Forgot your password?" ' + "button below to set a new password for your account." + ) ) - return result + ) class AnyEmailAuthenticationForm(StrongPasswordAuthenticationForm): diff --git a/requirements.txt b/requirements.txt index b1469d7216..4eb573ce36 100644 --- a/requirements.txt +++ b/requirements.txt @@ -79,6 +79,7 @@ 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 From 425b38fb3f98f4f96538a4981d55915d20cec246 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 10 Jun 2025 17:19:05 -0300 Subject: [PATCH 04/29] feat: validate password strength for setting pw --- ietf/ietfauth/forms.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/ietf/ietfauth/forms.py b/ietf/ietfauth/forms.py index a70f7b6ca1..ed19d407ff 100644 --- a/ietf/ietfauth/forms.py +++ b/ietf/ietfauth/forms.py @@ -14,7 +14,8 @@ from ietf.mailinglists.models import Allowlisted from ietf.utils.text import isascii -from .validators import prevent_at_symbol, prevent_system_name, prevent_anonymous_name, is_allowed_address +from .validators import prevent_at_symbol, prevent_system_name, prevent_anonymous_name, is_allowed_address, \ + StrongPasswordValidator from .widgets import PasswordStrengthInput, PasswordConfirmationInput @@ -177,7 +178,10 @@ class Meta: class ChangePasswordForm(forms.Form): current_password = forms.CharField(widget=forms.PasswordInput) - new_password = forms.CharField(widget=PasswordStrengthInput(attrs={'class':'password_strength'})) + new_password = forms.CharField( + validators=[StrongPasswordValidator()], + widget=PasswordStrengthInput(attrs={'class':'password_strength'}), + ) new_password_confirmation = forms.CharField(widget=PasswordConfirmationInput( confirm_with='new_password', attrs={'class':'password_confirmation'})) From fdc82bbe31d3004aaa943e24f5f2f28516926d14 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 10 Jun 2025 18:52:34 -0300 Subject: [PATCH 05/29] feat: feedback on password change page --- ietf/static/js/password_strength.js | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/ietf/static/js/password_strength.js b/ietf/static/js/password_strength.js index 4df5c14439..779965d260 100644 --- a/ietf/static/js/password_strength.js +++ b/ietf/static/js/password_strength.js @@ -68,11 +68,17 @@ .addClass('text-bg-warning'); password_strength_info.find('.badge') .removeClass('d-none'); + // Mark input as invalid + this.setCustomValidity('This password does not meet complexity requirements'); + this.classList.add('is-invalid') } else { password_strength_bar.removeClass('text-bg-warning') .addClass('text-bg-success'); password_strength_info.find('.badge') .addClass('d-none'); + // Mark input as valid + this.setCustomValidity(''); + this.classList.remove('is-invalid') } password_strength_bar.width(((result.score + 1) / 5) * 100 + '%') @@ -152,23 +158,31 @@ .data('confirm-with'); if (confirm_with && confirm_with == password_field.attr('id')) { - if (confirm_value && password) { + if (password) { if (confirm_value === password) { $(confirm_field) .parent() .find('.password_strength_info') .addClass('d-none'); + confirm_field.setCustomValidity('') + confirm_field.classList.remove('is-invalid') } else { - $(confirm_field) - .parent() - .find('.password_strength_info') - .removeClass('d-none'); + if (confirm_value !== '') { + $(confirm_field) + .parent() + .find('.password_strength_info') + .removeClass('d-none'); + } + confirm_field.setCustomValidity('Does not match new password') + confirm_field.classList.add('is-invalid') } } else { $(confirm_field) .parent() .find('.password_strength_info') .addClass('d-none'); + confirm_field.setCustomValidity('') + confirm_field.classList.remove('is-invalid') } } }); From 10f88a1c808021c005711de40b97f3554366e06b Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 10 Jun 2025 18:59:30 -0300 Subject: [PATCH 06/29] refactor: avoid field validator munging --- ietf/ietfauth/views.py | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/ietf/ietfauth/views.py b/ietf/ietfauth/views.py index cdc7c090ea..09e4575cee 100644 --- a/ietf/ietfauth/views.py +++ b/ietf/ietfauth/views.py @@ -733,17 +733,18 @@ def change_username(request): class StrongPasswordAuthenticationForm(AuthenticationForm): - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - self.fields["password"].validators.append( - StrongPasswordValidator( - message=( - "Your password does not meet complexity requirements and is " - 'easily guessable. Please use the "Forgot your password?" ' - "button below to set a new password for your account." - ) - ) + validate_password_strength = StrongPasswordValidator( + message=( + "Your password does not meet complexity requirements and is " + 'easily guessable. Please use the "Forgot your password?" ' + "button below to set a new password for your account." ) + ) + + def confirm_login_allowed(self, user): + """Raises validation error if login is not allowed""" + super().confirm_login_allowed(user) + self.validate_password_strength(self.cleaned_data["password"]) class AnyEmailAuthenticationForm(StrongPasswordAuthenticationForm): From 4d6a543de4e884edb59073aa326790eac69b02c7 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 10 Jun 2025 20:45:36 -0300 Subject: [PATCH 07/29] feat: give more info about how to choose a pw --- ietf/ietfauth/widgets.py | 11 +++++++---- ietf/static/js/password_strength.js | 4 ++++ ietf/templates/registration/change_password.html | 11 +++++++---- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/ietf/ietfauth/widgets.py b/ietf/ietfauth/widgets.py index c9a0523402..5b6214306a 100644 --- a/ietf/ietfauth/widgets.py +++ b/ietf/ietfauth/widgets.py @@ -39,18 +39,21 @@ def render(self, name, value, attrs=None, renderer=None): strength_markup = """
-
+
- """ % ( + """.format( _("Warning"), _( 'This password would take to crack.' diff --git a/ietf/static/js/password_strength.js b/ietf/static/js/password_strength.js index 779965d260..5eda5064eb 100644 --- a/ietf/static/js/password_strength.js +++ b/ietf/static/js/password_strength.js @@ -57,6 +57,8 @@ .parent() .parent() .find('.password_strength_offline_info'); + let password_improvement_hint = $(password_strength_info) + .find('.password_improvement_hint'); if ($(this) .val()) { @@ -71,6 +73,7 @@ // Mark input as invalid this.setCustomValidity('This password does not meet complexity requirements'); this.classList.add('is-invalid') + password_improvement_hint.removeClass('d-none'); } else { password_strength_bar.removeClass('text-bg-warning') .addClass('text-bg-success'); @@ -79,6 +82,7 @@ // Mark input as valid this.setCustomValidity(''); this.classList.remove('is-invalid') + password_improvement_hint.addClass('d-none'); } password_strength_bar.width(((result.score + 1) / 5) * 100 + '%') diff --git a/ietf/templates/registration/change_password.html b/ietf/templates/registration/change_password.html index 21c102bd0a..e93c304545 100644 --- a/ietf/templates/registration/change_password.html +++ b/ietf/templates/registration/change_password.html @@ -34,11 +34,14 @@

Change password

- Online attack: This password form uses the + Password strength requirements: + You must choose a password that scores at least a 3 according to the zxcvbn - password strength estimator to give an indication of password strength. - The crack time estimate given above assume online attack without rate - limiting, at a rate of 10 attempts per second. + password strength estimator. A warning will appear if your password does not meet this standard. +
+ Online attack: + The crack time estimate given above assumes an online attack at a rate of 10 attempts per second. + It is only a very rough guideline.
Offline cracking: The datatracker currently uses the {{ hasher.algorithm }} From db20b54abf53b0cbd375b47b5366e17ed0fa0cc1 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 11 Jun 2025 10:17:45 -0300 Subject: [PATCH 08/29] refactor: use password_validation module Only for changing password so; may need to update StrongPasswordAuthenticationForm to match --- ietf/ietfauth/forms.py | 21 ++++++++++++--------- ietf/ietfauth/password_validation.py | 23 +++++++++++++++++++++++ ietf/ietfauth/validators.py | 21 --------------------- ietf/ietfauth/views.py | 6 +++--- ietf/settings.py | 6 ++++++ 5 files changed, 44 insertions(+), 33 deletions(-) create mode 100644 ietf/ietfauth/password_validation.py diff --git a/ietf/ietfauth/forms.py b/ietf/ietfauth/forms.py index ed19d407ff..98366e6be1 100644 --- a/ietf/ietfauth/forms.py +++ b/ietf/ietfauth/forms.py @@ -6,16 +6,16 @@ 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 .validators import prevent_at_symbol, prevent_system_name, prevent_anonymous_name, is_allowed_address, \ - StrongPasswordValidator +from .validators import prevent_at_symbol, prevent_system_name, prevent_anonymous_name, is_allowed_address from .widgets import PasswordStrengthInput, PasswordConfirmationInput @@ -179,7 +179,6 @@ class ChangePasswordForm(forms.Form): current_password = forms.CharField(widget=forms.PasswordInput) new_password = forms.CharField( - validators=[StrongPasswordValidator()], widget=PasswordStrengthInput(attrs={'class':'password_strength'}), ) new_password_confirmation = forms.CharField(widget=PasswordConfirmationInput( @@ -191,16 +190,20 @@ def __init__(self, user, data=None): super(ChangePasswordForm, self).__init__(data) def clean_current_password(self): + # 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') 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/validators.py b/ietf/ietfauth/validators.py index 3ad3b378c0..84684f34d5 100644 --- a/ietf/ietfauth/validators.py +++ b/ietf/ietfauth/validators.py @@ -4,7 +4,6 @@ from django import forms from django.conf import settings from django.core.exceptions import ValidationError -from zxcvbn import zxcvbn def prevent_at_symbol(name): @@ -33,23 +32,3 @@ def is_allowed_address(value): raise ValidationError( "This email address is not valid in a datatracker account" ) - - -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 __call__(self, password): - """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/views.py b/ietf/ietfauth/views.py index 09e4575cee..d6745b5da2 100644 --- a/ietf/ietfauth/views.py +++ b/ietf/ietfauth/views.py @@ -68,7 +68,7 @@ ChangePasswordForm, get_person_form, RoleEmailForm, NewEmailForm, ChangeUsernameForm, PersonPasswordForm) from ietf.ietfauth.utils import has_role, send_new_email_confirmation_request -from ietf.ietfauth.validators import StrongPasswordValidator +from ietf.ietfauth.password_validation import StrongPasswordValidator from ietf.name.models import ExtResourceName from ietf.nomcom.models import NomCom from ietf.person.models import Person, Email, Alias, PersonalApiKey, PERSON_API_KEY_VALUES @@ -733,7 +733,7 @@ def change_username(request): class StrongPasswordAuthenticationForm(AuthenticationForm): - validate_password_strength = StrongPasswordValidator( + password_strength_validator = StrongPasswordValidator( message=( "Your password does not meet complexity requirements and is " 'easily guessable. Please use the "Forgot your password?" ' @@ -744,7 +744,7 @@ class StrongPasswordAuthenticationForm(AuthenticationForm): def confirm_login_allowed(self, user): """Raises validation error if login is not allowed""" super().confirm_login_allowed(user) - self.validate_password_strength(self.cleaned_data["password"]) + self.password_strength_validator.validate(self.cleaned_data["password"], user) class AnyEmailAuthenticationForm(StrongPasswordAuthenticationForm): diff --git a/ietf/settings.py b/ietf/settings.py index c21120f77a..c9bd64c471 100644 --- a/ietf/settings.py +++ b/ietf/settings.py @@ -61,6 +61,12 @@ 'django.contrib.auth.hashers.CryptPasswordHasher', ] +AUTH_PASSWORD_VALIDATORS = [ + { + "NAME": "ietf.ietfauth.password_validation.StrongPasswordValidator", + }, +] + ALLOWED_HOSTS = [".ietf.org", ".ietf.org.", "209.208.19.216", "4.31.198.44", "127.0.0.1", "localhost", ] # Server name of the tools server From fc26691bb14d409b63e25e20ce3cd141386e777a Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 11 Jun 2025 10:18:15 -0300 Subject: [PATCH 09/29] feat: password min_length validdation --- ietf/settings.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ietf/settings.py b/ietf/settings.py index c9bd64c471..7fa535c888 100644 --- a/ietf/settings.py +++ b/ietf/settings.py @@ -62,6 +62,12 @@ ] AUTH_PASSWORD_VALIDATORS = [ + { + "NAME": "django.contrib.auth.password_validation.MinimumLengthValidator", + "OPTIONS": { + "min_length": 12, + } + }, { "NAME": "ietf.ietfauth.password_validation.StrongPasswordValidator", }, From 29414ef010e9597e20249667ed66d81926470ea3 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 11 Jun 2025 16:04:53 -0300 Subject: [PATCH 10/29] refactor: use password_validation for login form --- ietf/ietfauth/views.py | 46 +++++++++++++++++++++--------------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/ietf/ietfauth/views.py b/ietf/ietfauth/views.py index d6745b5da2..1effb10cd6 100644 --- a/ietf/ietfauth/views.py +++ b/ietf/ietfauth/views.py @@ -39,14 +39,14 @@ import string # 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 @@ -83,7 +83,6 @@ # These are needed if we revert to the higher bar for account creation - def index(request): return render(request, 'registration/index.html') @@ -100,7 +99,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))) @@ -585,7 +584,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)") @@ -699,7 +697,7 @@ def change_password(request): 'hasher': hasher, }) - + @login_required @person_required def change_username(request): @@ -732,22 +730,7 @@ def change_username(request): return render(request, 'registration/change_username.html', {'form': form}) -class StrongPasswordAuthenticationForm(AuthenticationForm): - password_strength_validator = StrongPasswordValidator( - message=( - "Your password does not meet complexity requirements and is " - 'easily guessable. Please use the "Forgot your password?" ' - "button below to set a new password for your account." - ) - ) - - def confirm_login_allowed(self, user): - """Raises validation error if login is not allowed""" - super().confirm_login_allowed(user) - self.password_strength_validator.validate(self.cleaned_data["password"], user) - - -class AnyEmailAuthenticationForm(StrongPasswordAuthenticationForm): +class AnyEmailAuthenticationForm(AuthenticationForm): """AuthenticationForm that allows any email address as the username Also performs a check for a cleared password field and provides a helpful error message @@ -782,6 +765,23 @@ def clean(self): ) return super().clean() + def confirm_login_allowed(self, user): + """Only allow login if password complies with current validators""" + super().confirm_login_allowed(user) + try: + password_validation.validate_password(self.cleaned_data["password"], user) + except ValidationError as error: + raise ValidationError( + # dict mapping field to error / error list + { + "password": error.error_list, + "__all__": ValidationError( + f'Please use the "Forgot your password?" button below to ' + f'set a new password for your account.' + ), + } + ) + class AnyEmailLoginView(LoginView): """LoginView that allows any email address as the username @@ -797,7 +797,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 From e3a806ef03071188ebacb1c500ad5bff467eff4a Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 11 Jun 2025 17:12:13 -0300 Subject: [PATCH 11/29] fix: UI feedback consistent with validation --- ietf/ietfauth/forms.py | 3 +++ ietf/ietfauth/widgets.py | 4 +--- ietf/settings.py | 5 ++++- ietf/static/js/password_strength.js | 29 ++++++++++++++++++----------- 4 files changed, 26 insertions(+), 15 deletions(-) diff --git a/ietf/ietfauth/forms.py b/ietf/ietfauth/forms.py index 98366e6be1..bed3cf3fd6 100644 --- a/ietf/ietfauth/forms.py +++ b/ietf/ietfauth/forms.py @@ -3,6 +3,8 @@ import re + +from django.conf import settings from unidecode import unidecode from django import forms @@ -180,6 +182,7 @@ class ChangePasswordForm(forms.Form): new_password = forms.CharField( widget=PasswordStrengthInput(attrs={'class':'password_strength'}), + min_length=settings.PASSWORD_POLICY_MIN_LENGTH, ) new_password_confirmation = forms.CharField(widget=PasswordConfirmationInput( confirm_with='new_password', diff --git a/ietf/ietfauth/widgets.py b/ietf/ietfauth/widgets.py index 5b6214306a..fd7fa16726 100644 --- a/ietf/ietfauth/widgets.py +++ b/ietf/ietfauth/widgets.py @@ -48,9 +48,7 @@ def render(self, name, value, attrs=None, renderer=None): {} - - You must choose a more complex password. - +

""".format( diff --git a/ietf/settings.py b/ietf/settings.py index 7fa535c888..f8d75da44c 100644 --- a/ietf/settings.py +++ b/ietf/settings.py @@ -61,11 +61,14 @@ 'django.contrib.auth.hashers.CryptPasswordHasher', ] + +PASSWORD_POLICY_MIN_LENGTH = 12 + AUTH_PASSWORD_VALIDATORS = [ { "NAME": "django.contrib.auth.password_validation.MinimumLengthValidator", "OPTIONS": { - "min_length": 12, + "min_length": PASSWORD_POLICY_MIN_LENGTH, } }, { diff --git a/ietf/static/js/password_strength.js b/ietf/static/js/password_strength.js index 5eda5064eb..4933574e48 100644 --- a/ietf/static/js/password_strength.js +++ b/ietf/static/js/password_strength.js @@ -64,25 +64,32 @@ .val()) { var result = zxcvbn($(this) .val()); - - if (result.score < 3) { - password_strength_bar.removeClass('text-bg-success') - .addClass('text-bg-warning'); - password_strength_info.find('.badge') - .removeClass('d-none'); + const strongEnough = result.score >= 3; + if (strongEnough) { + // Mark input as valid + this.setCustomValidity(''); + } else { // Mark input as invalid this.setCustomValidity('This password does not meet complexity requirements'); - this.classList.add('is-invalid') - password_improvement_hint.removeClass('d-none'); - } else { + } + + if (this.checkValidity()) { password_strength_bar.removeClass('text-bg-warning') .addClass('text-bg-success'); password_strength_info.find('.badge') .addClass('d-none'); - // Mark input as valid - this.setCustomValidity(''); this.classList.remove('is-invalid') password_improvement_hint.addClass('d-none'); + password_improvement_hint.text('') + } else { + this.classList.add('is-invalid') + password_improvement_hint.text(this.validationMessage) + password_improvement_hint.removeClass('d-none'); + + password_strength_bar.removeClass('text-bg-success') + .addClass('text-bg-warning'); + password_strength_info.find('.badge') + .removeClass('d-none'); } password_strength_bar.width(((result.score + 1) / 5) * 100 + '%') From 3fb44a1198403ae6a25a931d08df356d63c888f5 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 11 Jun 2025 17:15:51 -0300 Subject: [PATCH 12/29] chore: update chpw page to state length req --- ietf/templates/registration/change_password.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ietf/templates/registration/change_password.html b/ietf/templates/registration/change_password.html index e93c304545..58bc2d2587 100644 --- a/ietf/templates/registration/change_password.html +++ b/ietf/templates/registration/change_password.html @@ -35,7 +35,7 @@

Change password

Password strength requirements: - You must choose a password that scores at least a 3 according to the + You must choose a password at least 12 characters long that scores at least a 3 according to the zxcvbn password strength estimator. A warning will appear if your password does not meet this standard.
From cbec4bcf30f2e5ed76053dced946325021ffb16f Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 11 Jun 2025 21:08:09 -0300 Subject: [PATCH 13/29] chore(dev): disable password validators in dev --- dev/deploy-to-container/settings_local.py | 3 +++ docker/configs/settings_local.py | 3 +++ 2 files changed, 6 insertions(+) diff --git a/dev/deploy-to-container/settings_local.py b/dev/deploy-to-container/settings_local.py index e878206bd5..050d8a7386 100644 --- a/dev/deploy-to-container/settings_local.py +++ b/dev/deploy-to-container/settings_local.py @@ -100,3 +100,6 @@ bucket_name=f"test-{storagename}", ), } + +# Disable password validators for dev sites - comment this out to enable them. +AUTH_PASSWORD_VALIDATORS = [] diff --git a/docker/configs/settings_local.py b/docker/configs/settings_local.py index 46833451c1..a2d05a8188 100644 --- a/docker/configs/settings_local.py +++ b/docker/configs/settings_local.py @@ -80,3 +80,6 @@ STATIC_IETF_ORG = "/_static" STATIC_IETF_ORG_INTERNAL = "http://static" + +# Disable password validators in dev mode - comment this out to enable them +AUTH_PASSWORD_VALIDATORS = [] From d6cea5621e20097fcd9236fa32a94aaa375aea0d Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 11 Jun 2025 22:00:25 -0300 Subject: [PATCH 14/29] fix: drop JS validation when password val disabled --- ietf/ietfauth/forms.py | 20 ++++++++++++++------ ietf/static/js/password_strength.js | 3 ++- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/ietf/ietfauth/forms.py b/ietf/ietfauth/forms.py index bed3cf3fd6..211d35a520 100644 --- a/ietf/ietfauth/forms.py +++ b/ietf/ietfauth/forms.py @@ -16,6 +16,7 @@ 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 @@ -173,16 +174,16 @@ 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'}), - min_length=settings.PASSWORD_POLICY_MIN_LENGTH, + 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', @@ -190,7 +191,14 @@ class ChangePasswordForm(forms.Form): 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): # n.b., password = None is handled by check_password and results in a failed check diff --git a/ietf/static/js/password_strength.js b/ietf/static/js/password_strength.js index 4933574e48..74a245fb5c 100644 --- a/ietf/static/js/password_strength.js +++ b/ietf/static/js/password_strength.js @@ -64,7 +64,8 @@ .val()) { var result = zxcvbn($(this) .val()); - const strongEnough = result.score >= 3; + const enforceStrength = !('disableStrengthEnforcement' in this.dataset); + const strongEnough = !enforceStrength || (result.score >= 3); if (strongEnough) { // Mark input as valid this.setCustomValidity(''); From 8c721ba497e52a9001ce95b76f6adf0a9d96e294 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 11 Jun 2025 22:01:12 -0300 Subject: [PATCH 15/29] style: ruff on ChangePasswordForm --- ietf/ietfauth/forms.py | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/ietf/ietfauth/forms.py b/ietf/ietfauth/forms.py index 211d35a520..036c64b4f1 100644 --- a/ietf/ietfauth/forms.py +++ b/ietf/ietfauth/forms.py @@ -181,13 +181,16 @@ class ChangePasswordForm(forms.Form): new_password = forms.CharField( widget=PasswordStrengthInput( attrs={ - 'class':'password_strength', - 'data-disable-strength-enforcement': '', # usually removed in init - }), + "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"} + ) ) - new_password_confirmation = forms.CharField(widget=PasswordConfirmationInput( - confirm_with='new_password', - attrs={'class':'password_confirmation'})) def __init__(self, user, data=None): self.user = user @@ -198,13 +201,15 @@ def __init__(self, user, data=None): 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) + new_password_field.widget.attrs.pop( + "data-disable-strength-enforcement", None + ) def clean_current_password(self): # n.b., password = None is handled by check_password and results in a failed check - password = self.cleaned_data.get('current_password', None) + 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): From a3ab53e3ac7919d45f1c241c06ce7b5f27c013bb Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 18 Jun 2025 10:48:42 -0300 Subject: [PATCH 16/29] chore: lint --- ietf/ietfauth/forms.py | 1 - ietf/ietfauth/views.py | 7 ++----- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/ietf/ietfauth/forms.py b/ietf/ietfauth/forms.py index 036c64b4f1..53bf7b5888 100644 --- a/ietf/ietfauth/forms.py +++ b/ietf/ietfauth/forms.py @@ -4,7 +4,6 @@ import re -from django.conf import settings from unidecode import unidecode from django import forms diff --git a/ietf/ietfauth/views.py b/ietf/ietfauth/views.py index 1effb10cd6..d3672505d1 100644 --- a/ietf/ietfauth/views.py +++ b/ietf/ietfauth/views.py @@ -36,7 +36,6 @@ import datetime import importlib -import string # needed if we revert to higher barrier for account creation # from datetime import datetime as DateTime, timedelta as TimeDelta, date as Date @@ -59,7 +58,6 @@ from django.http import Http404, HttpResponseRedirect, HttpResponseForbidden from django.shortcuts import render, redirect, get_object_or_404 from django.utils.encoding import force_bytes -from zxcvbn import zxcvbn import debug # pyflakes:ignore @@ -68,7 +66,6 @@ ChangePasswordForm, get_person_form, RoleEmailForm, NewEmailForm, ChangeUsernameForm, PersonPasswordForm) from ietf.ietfauth.utils import has_role, send_new_email_confirmation_request -from ietf.ietfauth.password_validation import StrongPasswordValidator from ietf.name.models import ExtResourceName from ietf.nomcom.models import NomCom from ietf.person.models import Person, Email, Alias, PersonalApiKey, PERSON_API_KEY_VALUES @@ -776,8 +773,8 @@ def confirm_login_allowed(self, user): { "password": error.error_list, "__all__": ValidationError( - f'Please use the "Forgot your password?" button below to ' - f'set a new password for your account.' + 'Please use the "Forgot your password?" button below to ' + 'set a new password for your account.' ), } ) From 510900c16d42d6d76123e5e7c69c551bfc7cce5a Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 18 Jun 2025 11:17:57 -0300 Subject: [PATCH 17/29] chore(dev): preserve pw validator cfg for tests --- ietf/settings.py | 4 ++++ ietf/settings_test.py | 16 +++++++++++++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/ietf/settings.py b/ietf/settings.py index f8d75da44c..3dfe754029 100644 --- a/ietf/settings.py +++ b/ietf/settings.py @@ -75,6 +75,10 @@ "NAME": "ietf.ietfauth.password_validation.StrongPasswordValidator", }, ] +# In dev environments, settings_local overrides the password validators. Save +# a handle to the original value so settings_test can restore it so tests match +# production. +ORIG_AUTH_PASSWORD_VALIDATORS = AUTH_PASSWORD_VALIDATORS ALLOWED_HOSTS = [".ietf.org", ".ietf.org.", "209.208.19.216", "4.31.198.44", "127.0.0.1", "localhost", ] diff --git a/ietf/settings_test.py b/ietf/settings_test.py index fe77152d42..879c059494 100755 --- a/ietf/settings_test.py +++ b/ietf/settings_test.py @@ -14,7 +14,15 @@ import shutil import tempfile from ietf.settings import * # pyflakes:ignore -from ietf.settings import STORAGES, TEST_CODE_COVERAGE_CHECKER, MORE_STORAGE_NAMES, BLOBSTORAGE_CONNECT_TIMEOUT, BLOBSTORAGE_READ_TIMEOUT, BLOBSTORAGE_MAX_ATTEMPTS +from ietf.settings import ( + STORAGES, + TEST_CODE_COVERAGE_CHECKER, + MORE_STORAGE_NAMES, + BLOBSTORAGE_CONNECT_TIMEOUT, + BLOBSTORAGE_READ_TIMEOUT, + BLOBSTORAGE_MAX_ATTEMPTS, + ORIG_AUTH_PASSWORD_VALIDATORS, +) import botocore.config import debug # pyflakes:ignore debug.debug = True @@ -133,3 +141,9 @@ def tempdir_with_cleanup(**kwargs): ietf_log_blob_timing=_blob_store_enable_profiling, ), } + +# Restore AUTH_PASSWORD_VALIDATORS if they were reset in settings_local +try: + AUTH_PASSWORD_VALIDATORS = ORIG_AUTH_PASSWORD_VALIDATORS +except NameError: + pass From df4e58833eb97385c2bee822a73d6f3a47ac0c83 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 18 Jun 2025 11:19:23 -0300 Subject: [PATCH 18/29] test: fix test_change_password --- ietf/ietfauth/tests.py | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/ietf/ietfauth/tests.py b/ietf/ietfauth/tests.py index fed764e8bd..6dc2bfcf3e 100644 --- a/ietf/ietfauth/tests.py +++ b/ietf/ietfauth/tests.py @@ -587,6 +587,8 @@ 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") @@ -597,40 +599,40 @@ def test_change_password(self): self.assertRedirects(r, redir_url) user = User.objects.create(username="someone@example.com", email="someone@example.com") - user.set_password("password") + 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", + "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') # 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") # 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): From c63bcd81d29a76eb5aff28a99f5f2a5b2029b629 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 18 Jun 2025 11:20:49 -0300 Subject: [PATCH 19/29] test: fix test_change_username --- ietf/ietfauth/tests.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/ietf/ietfauth/tests.py b/ietf/ietfauth/tests.py index 6dc2bfcf3e..db26adbfdb 100644 --- a/ietf/ietfauth/tests.py +++ b/ietf/ietfauth/tests.py @@ -635,7 +635,7 @@ def test_change_password(self): 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") @@ -646,19 +646,19 @@ def test_change_username(self): self.assertRedirects(r, redir_url) user = User.objects.create(username="someone@example.com", email="someone@example.com") - user.set_password("password") + 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) # 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", + "password": VALID_PASSWORD, }) self.assertEqual(r.status_code, 200) self.assertFormError(r.context["form"], 'username', @@ -673,14 +673,14 @@ def test_change_username(self): # correct username change r = self.client.post(chun_url, {"username": "othername@example.org", - "password": "password", + "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 From 9e6c03d750825830889ebb5d67089a2e0ef504c2 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 18 Jun 2025 11:28:15 -0300 Subject: [PATCH 20/29] test: fix test_reset_password --- ietf/ietfauth/tests.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/ietf/ietfauth/tests.py b/ietf/ietfauth/tests.py index db26adbfdb..7bdcab5532 100644 --- a/ietf/ietfauth/tests.py +++ b/ietf/ietfauth/tests.py @@ -395,12 +395,13 @@ def test_nomcom_dressing_on_profile(self): 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' user = PersonFactory(user__email=email).user - user.set_password(password) + user.set_password(VALID_PASSWORD) user.save() # get @@ -435,13 +436,13 @@ def test_reset_password(self): '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) @@ -452,7 +453,7 @@ 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 }) @@ -460,7 +461,7 @@ def test_reset_password(self): 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) @@ -473,7 +474,7 @@ def test_reset_password(self): 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) From 3ac7ade958df3b704e1b82411ec25b19baad7006 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 18 Jun 2025 11:29:24 -0300 Subject: [PATCH 21/29] style: ruff refactored tests --- ietf/ietfauth/tests.py | 150 ++++++++++++++++++++++++++++------------- 1 file changed, 105 insertions(+), 45 deletions(-) diff --git a/ietf/ietfauth/tests.py b/ietf/ietfauth/tests.py index 7bdcab5532..3948e7c4d0 100644 --- a/ietf/ietfauth/tests.py +++ b/ietf/ietfauth/tests.py @@ -393,12 +393,11 @@ 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' + email = "someone@example.com" user = PersonFactory(user__email=email).user user.set_password(VALID_PASSWORD) @@ -409,21 +408,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) @@ -432,17 +433,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': ANOTHER_VALID_PASSWORD, 'password_confirmation': ANOTHER_VALID_PASSWORD[::-1] }) + 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': ANOTHER_VALID_PASSWORD, 'password_confirmation': ANOTHER_VALID_PASSWORD }) + 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) @@ -456,12 +472,15 @@ def test_reset_password(self): 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': VALID_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) @@ -469,7 +488,7 @@ 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]) @@ -593,43 +612,63 @@ def test_change_password(self): 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 = 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": VALID_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": ANOTHER_VALID_PASSWORD, - "new_password_confirmation": ANOTHER_VALID_PASSWORD, - }) + 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": VALID_PASSWORD, - "new_password": ANOTHER_VALID_PASSWORD, - "new_password_confirmation": ANOTHER_VALID_PASSWORD[::-1], - }) + 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") + self.assertFormError( + r.context["form"], + None, + "The password confirmation is different than the new password", + ) # correct password change - r = self.client.post(chpw_url, {"current_password": VALID_PASSWORD, - "new_password": ANOTHER_VALID_PASSWORD, - "new_password_confirmation": ANOTHER_VALID_PASSWORD, - }) + 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") @@ -640,42 +679,63 @@ def test_change_username(self): 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 = 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":VALID_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": VALID_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": VALID_PASSWORD, - }) + r = self.client.post( + chun_url, + { + "username": "othername@example.org", + "password": VALID_PASSWORD, + }, + ) self.assertRedirects(r, prof_url) # refresh user object prev = user From 377def642cf0005c3254433ac205adcf52f17cb8 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 18 Jun 2025 11:46:15 -0300 Subject: [PATCH 22/29] chore: type lint --- docker/configs/settings_local.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker/configs/settings_local.py b/docker/configs/settings_local.py index a2d05a8188..24da966b8b 100644 --- a/docker/configs/settings_local.py +++ b/docker/configs/settings_local.py @@ -82,4 +82,4 @@ STATIC_IETF_ORG_INTERNAL = "http://static" # Disable password validators in dev mode - comment this out to enable them -AUTH_PASSWORD_VALIDATORS = [] +AUTH_PASSWORD_VALIDATORS = [] # type:ignore From 1a98ef4c63500e4cbebbf6150d9bb5e15815fb4a Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 18 Jun 2025 12:17:31 -0300 Subject: [PATCH 23/29] feat: require pw reset for very stale accounts --- ietf/ietfauth/views.py | 29 +++++++++++++++++++++++------ ietf/settings.py | 1 + 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/ietf/ietfauth/views.py b/ietf/ietfauth/views.py index d3672505d1..a733ee4fa9 100644 --- a/ietf/ietfauth/views.py +++ b/ietf/ietfauth/views.py @@ -763,8 +763,28 @@ def clean(self): return super().clean() def confirm_login_allowed(self, user): - """Only allow login if password complies with current validators""" - super().confirm_login_allowed(user) + """Determine whether user is allowed to log in + + Only allow login if: + * account is active (and other Django default checks) + * password complies with current validators + * last_login is not too long ago + """ + please_reset_message = ( + 'Please use the "Forgot your password?" button below to ' + "set a new password for your account." + ) + super().confirm_login_allowed(user) # default checks + # Check time since last login + if hasattr(settings, "PASSWORD_POLICY_MAX_LOGIN_AGE"): + now = datetime.datetime.now(datetime.timezone.utc) + last_login = user.last_login or user.date_joined + login_age = now - last_login + if login_age > settings.PASSWORD_POLICY_MAX_LOGIN_AGE: + raise ValidationError( + "It has been too long since your last login. " + + please_reset_message + ) try: password_validation.validate_password(self.cleaned_data["password"], user) except ValidationError as error: @@ -772,10 +792,7 @@ def confirm_login_allowed(self, user): # dict mapping field to error / error list { "password": error.error_list, - "__all__": ValidationError( - 'Please use the "Forgot your password?" button below to ' - 'set a new password for your account.' - ), + "__all__": ValidationError(please_reset_message), } ) diff --git a/ietf/settings.py b/ietf/settings.py index 7506e64bb4..e8475c0388 100644 --- a/ietf/settings.py +++ b/ietf/settings.py @@ -63,6 +63,7 @@ PASSWORD_POLICY_MIN_LENGTH = 12 +PASSWORD_POLICY_MAX_LOGIN_AGE = datetime.timedelta(days=2 * 365) AUTH_PASSWORD_VALIDATORS = [ { From ae0d90bb29fb379e547dd33f1a6c9aa04c03477b Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 18 Jun 2025 12:39:41 -0300 Subject: [PATCH 24/29] test: test stale account login --- ietf/ietfauth/tests.py | 58 ++++++++++++++++++++++++++++++++++-------- 1 file changed, 48 insertions(+), 10 deletions(-) diff --git a/ietf/ietfauth/tests.py b/ietf/ietfauth/tests.py index 9a8dd09ebe..f8b60444a5 100644 --- a/ietf/ietfauth/tests.py +++ b/ietf/ietfauth/tests.py @@ -25,6 +25,7 @@ from django.contrib.auth.models import User from django.conf import settings from django.template.loader import render_to_string +from django.test.utils import override_settings from django.utils import timezone import debug # pyflakes:ignore @@ -42,6 +43,7 @@ from ietf.utils.mail import outbox, empty_outbox, get_payload_text from ietf.utils.test_utils import TestCase, login_testing_unauthorized from ietf.utils.timezone import date_today +from ..settings import PASSWORD_POLICY_MAX_LOGIN_AGE class IetfAuthTests(TestCase): @@ -115,6 +117,46 @@ def test_login_with_different_email(self): self.assertEqual(r.status_code, 302) self.assertEqual(urlsplit(r["Location"])[2], urlreverse("ietf.ietfauth.views.profile")) + def test_login_with_stale_account(self): + VALID_PASSWORD = "complex-and-long-valid-password" + LONG_TIME = datetime.timedelta(days=730) + now = datetime.datetime.now(datetime.timezone.utc) + login_url = urlreverse("ietf.ietfauth.views.login") + success_url = urlreverse("ietf.ietfauth.views.profile") # redirect target + user = PersonFactory(user__username="stale@example.com").user + user.set_password(VALID_PASSWORD) + user.save() + login_data = { + "username": user.username, + "password": VALID_PASSWORD, + } + + # newly-created user (no last_login, recent date_joined) + with override_settings(PASSWORD_POLICY_MAX_LOGIN_AGE=LONG_TIME): + r = self.client.post(login_url, login_data) + self.assertEqual(r.status_code, 302) + self.assertEqual(urlsplit(r["Location"])[2], success_url) + + # just barely recent enough + user.date_joined = now - 2 * LONG_TIME # joined long, long ago... + user.last_login = now - LONG_TIME + datetime.timedelta(days=1) + user.save() + with override_settings(PASSWORD_POLICY_MAX_LOGIN_AGE=LONG_TIME): + r = self.client.post(login_url, login_data) + self.assertEqual(r.status_code, 302) + self.assertEqual(urlsplit(r["Location"])[2], success_url) + + # too long ago + user.last_login = now - LONG_TIME - datetime.timedelta(days=1) + user.save() + with override_settings(PASSWORD_POLICY_MAX_LOGIN_AGE=LONG_TIME): + r = self.client.post(login_url, login_data) + self.assertContains( + r, + "too long since your last login", + status_code=200, + ) + def extract_confirm_url(self, confirm_email): # dig out confirm_email link msg = get_payload_text(confirm_email) @@ -127,8 +169,7 @@ def extract_confirm_url(self, confirm_email): return confirm_url - -# For the lowered barrier to account creation period, we are disabling this kind of failure + # For the lowered barrier to account creation period, we are disabling this kind of failure # def test_create_account_failure(self): # url = urlreverse("ietf.ietfauth.views.create_account") @@ -144,7 +185,7 @@ def extract_confirm_url(self, confirm_email): # self.assertEqual(r.status_code, 200) # self.assertContains(r, "Additional Assistance Required") -# Rather than delete the failure template just yet, here's a test to make sure it still renders should we need to revert to it. + # Rather than delete the failure template just yet, here's a test to make sure it still renders should we need to revert to it. def test_create_account_failure_template(self): r = render_to_string('registration/manual.html', { 'account_request_email': settings.ACCOUNT_REQUEST_EMAIL }) self.assertTrue("Additional Assistance Required" in r) @@ -179,7 +220,6 @@ def register_and_verify(self, email): self.assertEqual(Person.objects.filter(user__username=email).count(), 1) self.assertEqual(Email.objects.filter(person__user__username=email).count(), 1) - # This also tests new account creation. def test_create_existing_account(self): # create account once @@ -205,7 +245,6 @@ def test_ietfauth_profile(self): url = urlreverse("ietf.ietfauth.views.profile") login_testing_unauthorized(self, username, url) - # get r = self.client.get(url) self.assertEqual(r.status_code, 200) @@ -325,7 +364,6 @@ def test_ietfauth_profile(self): q = PyQuery(r.content) self.assertTrue(len(q("form div.invalid-feedback")) == 1) - # change role email role = Role.objects.create( person=Person.objects.get(user__username=username), @@ -340,7 +378,7 @@ def test_ietfauth_profile(self): self.assertEqual(r.status_code, 200) q = PyQuery(r.content) self.assertEqual(len(q('[name="%s"]' % role_email_input_name)), 1) - + with_changed_role_email = base_data.copy() with_changed_role_email["active_emails"] = new_email_address with_changed_role_email[role_email_input_name] = new_email_address @@ -767,7 +805,7 @@ def test_apikey_management(self): for endpoint, display in endpoints: r = self.client.post(url, {'endpoint': endpoint}) self.assertRedirects(r, urlreverse('ietf.ietfauth.views.apikey_index')) - + # Check api key list content url = urlreverse('ietf.ietfauth.views.apikey_index') r = self.client.get(url) @@ -798,7 +836,7 @@ def test_apikey_management(self): r = self.client.post(url, {'hash': otherkey.hash()}) self.assertEqual(r.status_code, 200) self.assertContains(r,"Key validation failed; key not disabled") - + # Delete a key r = self.client.post(url, {'hash': key.hash()}) self.assertRedirects(r, urlreverse('ietf.ietfauth.views.apikey_index')) @@ -885,7 +923,7 @@ def test_send_apikey_report(self): for endpoint, display in endpoints: r = self.client.post(url, {'endpoint': endpoint}) self.assertRedirects(r, urlreverse('ietf.ietfauth.views.apikey_index')) - + # Use the endpoints (the form content will not be acceptable, but the # apikey usage will be registered) count = 2 From 8097e307f19eb45d413128b8ed2c7bbaa373f455 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 18 Jun 2025 12:50:43 -0300 Subject: [PATCH 25/29] test: rejection of short/simple PWs --- ietf/ietfauth/tests.py | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/ietf/ietfauth/tests.py b/ietf/ietfauth/tests.py index f8b60444a5..ed77b0ed10 100644 --- a/ietf/ietfauth/tests.py +++ b/ietf/ietfauth/tests.py @@ -43,7 +43,6 @@ from ietf.utils.mail import outbox, empty_outbox, get_payload_text from ietf.utils.test_utils import TestCase, login_testing_unauthorized from ietf.utils.timezone import date_today -from ..settings import PASSWORD_POLICY_MAX_LOGIN_AGE class IetfAuthTests(TestCase): @@ -697,6 +696,40 @@ def test_change_password(self): "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, + "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, From 0eae8a1bfc89e7e0f03d7b92ba7910d3c060ad14 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 18 Jun 2025 14:23:00 -0300 Subject: [PATCH 26/29] Revert "test: test stale account login" This reverts commit ae0d90bb29fb379e547dd33f1a6c9aa04c03477b. --- ietf/ietfauth/tests.py | 57 ++++++++---------------------------------- 1 file changed, 10 insertions(+), 47 deletions(-) diff --git a/ietf/ietfauth/tests.py b/ietf/ietfauth/tests.py index ed77b0ed10..dd23277b63 100644 --- a/ietf/ietfauth/tests.py +++ b/ietf/ietfauth/tests.py @@ -25,7 +25,6 @@ from django.contrib.auth.models import User from django.conf import settings from django.template.loader import render_to_string -from django.test.utils import override_settings from django.utils import timezone import debug # pyflakes:ignore @@ -116,46 +115,6 @@ def test_login_with_different_email(self): self.assertEqual(r.status_code, 302) self.assertEqual(urlsplit(r["Location"])[2], urlreverse("ietf.ietfauth.views.profile")) - def test_login_with_stale_account(self): - VALID_PASSWORD = "complex-and-long-valid-password" - LONG_TIME = datetime.timedelta(days=730) - now = datetime.datetime.now(datetime.timezone.utc) - login_url = urlreverse("ietf.ietfauth.views.login") - success_url = urlreverse("ietf.ietfauth.views.profile") # redirect target - user = PersonFactory(user__username="stale@example.com").user - user.set_password(VALID_PASSWORD) - user.save() - login_data = { - "username": user.username, - "password": VALID_PASSWORD, - } - - # newly-created user (no last_login, recent date_joined) - with override_settings(PASSWORD_POLICY_MAX_LOGIN_AGE=LONG_TIME): - r = self.client.post(login_url, login_data) - self.assertEqual(r.status_code, 302) - self.assertEqual(urlsplit(r["Location"])[2], success_url) - - # just barely recent enough - user.date_joined = now - 2 * LONG_TIME # joined long, long ago... - user.last_login = now - LONG_TIME + datetime.timedelta(days=1) - user.save() - with override_settings(PASSWORD_POLICY_MAX_LOGIN_AGE=LONG_TIME): - r = self.client.post(login_url, login_data) - self.assertEqual(r.status_code, 302) - self.assertEqual(urlsplit(r["Location"])[2], success_url) - - # too long ago - user.last_login = now - LONG_TIME - datetime.timedelta(days=1) - user.save() - with override_settings(PASSWORD_POLICY_MAX_LOGIN_AGE=LONG_TIME): - r = self.client.post(login_url, login_data) - self.assertContains( - r, - "too long since your last login", - status_code=200, - ) - def extract_confirm_url(self, confirm_email): # dig out confirm_email link msg = get_payload_text(confirm_email) @@ -168,7 +127,8 @@ def extract_confirm_url(self, confirm_email): return confirm_url - # For the lowered barrier to account creation period, we are disabling this kind of failure + +# For the lowered barrier to account creation period, we are disabling this kind of failure # def test_create_account_failure(self): # url = urlreverse("ietf.ietfauth.views.create_account") @@ -184,7 +144,7 @@ def extract_confirm_url(self, confirm_email): # self.assertEqual(r.status_code, 200) # self.assertContains(r, "Additional Assistance Required") - # Rather than delete the failure template just yet, here's a test to make sure it still renders should we need to revert to it. +# Rather than delete the failure template just yet, here's a test to make sure it still renders should we need to revert to it. def test_create_account_failure_template(self): r = render_to_string('registration/manual.html', { 'account_request_email': settings.ACCOUNT_REQUEST_EMAIL }) self.assertTrue("Additional Assistance Required" in r) @@ -219,6 +179,7 @@ def register_and_verify(self, email): self.assertEqual(Person.objects.filter(user__username=email).count(), 1) self.assertEqual(Email.objects.filter(person__user__username=email).count(), 1) + # This also tests new account creation. def test_create_existing_account(self): # create account once @@ -244,6 +205,7 @@ def test_ietfauth_profile(self): url = urlreverse("ietf.ietfauth.views.profile") login_testing_unauthorized(self, username, url) + # get r = self.client.get(url) self.assertEqual(r.status_code, 200) @@ -363,6 +325,7 @@ def test_ietfauth_profile(self): q = PyQuery(r.content) self.assertTrue(len(q("form div.invalid-feedback")) == 1) + # change role email role = Role.objects.create( person=Person.objects.get(user__username=username), @@ -377,7 +340,7 @@ def test_ietfauth_profile(self): self.assertEqual(r.status_code, 200) q = PyQuery(r.content) self.assertEqual(len(q('[name="%s"]' % role_email_input_name)), 1) - + with_changed_role_email = base_data.copy() with_changed_role_email["active_emails"] = new_email_address with_changed_role_email[role_email_input_name] = new_email_address @@ -838,7 +801,7 @@ def test_apikey_management(self): for endpoint, display in endpoints: r = self.client.post(url, {'endpoint': endpoint}) self.assertRedirects(r, urlreverse('ietf.ietfauth.views.apikey_index')) - + # Check api key list content url = urlreverse('ietf.ietfauth.views.apikey_index') r = self.client.get(url) @@ -869,7 +832,7 @@ def test_apikey_management(self): r = self.client.post(url, {'hash': otherkey.hash()}) self.assertEqual(r.status_code, 200) self.assertContains(r,"Key validation failed; key not disabled") - + # Delete a key r = self.client.post(url, {'hash': key.hash()}) self.assertRedirects(r, urlreverse('ietf.ietfauth.views.apikey_index')) @@ -956,7 +919,7 @@ def test_send_apikey_report(self): for endpoint, display in endpoints: r = self.client.post(url, {'endpoint': endpoint}) self.assertRedirects(r, urlreverse('ietf.ietfauth.views.apikey_index')) - + # Use the endpoints (the form content will not be acceptable, but the # apikey usage will be registered) count = 2 From 7db71a966105525151c9da07192ef5ba4cf56c78 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 18 Jun 2025 14:23:01 -0300 Subject: [PATCH 27/29] Revert "feat: require pw reset for very stale accounts" This reverts commit 1a98ef4c63500e4cbebbf6150d9bb5e15815fb4a. --- ietf/ietfauth/views.py | 29 ++++++----------------------- ietf/settings.py | 1 - 2 files changed, 6 insertions(+), 24 deletions(-) diff --git a/ietf/ietfauth/views.py b/ietf/ietfauth/views.py index a733ee4fa9..d3672505d1 100644 --- a/ietf/ietfauth/views.py +++ b/ietf/ietfauth/views.py @@ -763,28 +763,8 @@ def clean(self): return super().clean() def confirm_login_allowed(self, user): - """Determine whether user is allowed to log in - - Only allow login if: - * account is active (and other Django default checks) - * password complies with current validators - * last_login is not too long ago - """ - please_reset_message = ( - 'Please use the "Forgot your password?" button below to ' - "set a new password for your account." - ) - super().confirm_login_allowed(user) # default checks - # Check time since last login - if hasattr(settings, "PASSWORD_POLICY_MAX_LOGIN_AGE"): - now = datetime.datetime.now(datetime.timezone.utc) - last_login = user.last_login or user.date_joined - login_age = now - last_login - if login_age > settings.PASSWORD_POLICY_MAX_LOGIN_AGE: - raise ValidationError( - "It has been too long since your last login. " - + please_reset_message - ) + """Only allow login if password complies with current validators""" + super().confirm_login_allowed(user) try: password_validation.validate_password(self.cleaned_data["password"], user) except ValidationError as error: @@ -792,7 +772,10 @@ def confirm_login_allowed(self, user): # dict mapping field to error / error list { "password": error.error_list, - "__all__": ValidationError(please_reset_message), + "__all__": ValidationError( + 'Please use the "Forgot your password?" button below to ' + 'set a new password for your account.' + ), } ) diff --git a/ietf/settings.py b/ietf/settings.py index e8475c0388..7506e64bb4 100644 --- a/ietf/settings.py +++ b/ietf/settings.py @@ -63,7 +63,6 @@ PASSWORD_POLICY_MIN_LENGTH = 12 -PASSWORD_POLICY_MAX_LOGIN_AGE = datetime.timedelta(days=2 * 365) AUTH_PASSWORD_VALIDATORS = [ { From 8d486776a54c24f74f0589af535bc7c22fd16bea Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 18 Jun 2025 16:02:58 -0300 Subject: [PATCH 28/29] test: disable pw validators for playwright legacy tests --- .github/workflows/tests.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index f10c1db9a3..e4c521507b 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -149,6 +149,9 @@ jobs: run: | chmod +x ./dev/tests/prepare.sh sh ./dev/tests/prepare.sh + echo >> ./ietf/settings_local.py + echo "# Disable password validation for playwright legacy tests" >> ./ietf/settings_local.py + echo "AUTH_PASSWORD_VALIDATORS = [] # type:ignore" >> ./ietf/settings_local.py - name: Ensure DB is ready run: | From 3898f4d775affffd6f30d3d265ae69f13a02b588 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Mon, 23 Jun 2025 21:31:28 -0300 Subject: [PATCH 29/29] feat: make pw enforcement at login optional --- .github/workflows/tests.yml | 3 --- dev/deploy-to-container/settings_local.py | 3 --- docker/configs/settings_local.py | 3 --- ietf/ietfauth/views.py | 33 +++++++++++++---------- ietf/settings.py | 1 + k8s/settings_local.py | 5 ++++ 6 files changed, 25 insertions(+), 23 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index e4c521507b..f10c1db9a3 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -149,9 +149,6 @@ jobs: run: | chmod +x ./dev/tests/prepare.sh sh ./dev/tests/prepare.sh - echo >> ./ietf/settings_local.py - echo "# Disable password validation for playwright legacy tests" >> ./ietf/settings_local.py - echo "AUTH_PASSWORD_VALIDATORS = [] # type:ignore" >> ./ietf/settings_local.py - name: Ensure DB is ready run: | diff --git a/dev/deploy-to-container/settings_local.py b/dev/deploy-to-container/settings_local.py index e45701ee6e..aacf000093 100644 --- a/dev/deploy-to-container/settings_local.py +++ b/dev/deploy-to-container/settings_local.py @@ -79,6 +79,3 @@ # OIDC configuration SITE_URL = 'https://__HOSTNAME__' - -# Disable password validators for dev sites - comment this out to enable them. -AUTH_PASSWORD_VALIDATORS = [] diff --git a/docker/configs/settings_local.py b/docker/configs/settings_local.py index b884da1183..ca51871463 100644 --- a/docker/configs/settings_local.py +++ b/docker/configs/settings_local.py @@ -98,6 +98,3 @@ bucket_name=f"{storagename}", ), } - -# Disable password validators in dev mode - comment this out to enable them -AUTH_PASSWORD_VALIDATORS = [] # type:ignore diff --git a/ietf/ietfauth/views.py b/ietf/ietfauth/views.py index d3672505d1..84d5490873 100644 --- a/ietf/ietfauth/views.py +++ b/ietf/ietfauth/views.py @@ -763,21 +763,26 @@ def clean(self): return super().clean() def confirm_login_allowed(self, user): - """Only allow login if password complies with current validators""" + """Check whether a successfully authenticated user is permitted to log in""" super().confirm_login_allowed(user) - try: - password_validation.validate_password(self.cleaned_data["password"], user) - except ValidationError as error: - raise ValidationError( - # dict mapping field to error / error list - { - "password": error.error_list, - "__all__": ValidationError( - 'Please use the "Forgot your password?" button below to ' - 'set a new password for your account.' - ), - } - ) + # 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): diff --git a/ietf/settings.py b/ietf/settings.py index 7506e64bb4..5e33673611 100644 --- a/ietf/settings.py +++ b/ietf/settings.py @@ -63,6 +63,7 @@ PASSWORD_POLICY_MIN_LENGTH = 12 +PASSWORD_POLICY_ENFORCE_AT_LOGIN = False # should turn this on for prod AUTH_PASSWORD_VALIDATORS = [ { 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" +)