From fb47ef53bf1a47def1a40ea0e1c5d26efc7cbfad Mon Sep 17 00:00:00 2001 From: Florian Aucomte Date: Thu, 1 Feb 2024 10:04:27 +0000 Subject: [PATCH] feat: Create base password validator per user type (#64) * feat: Create base password validator per user type * First part of feedback * Feedbacks * Black and some pylint * Rename validators in settings * Merge branch 'main' into reset_password * Stop using setup class * Revert "Stop using setup class" This reverts commit 1134b8816d03b2bc2d73b3e127d39de0fb7aac42. * Feedback and pylint * Black and pylint * Feedback --- codeforlife/settings/django.py | 16 +++--- .../user/auth/password_validators/__init__.py | 8 +++ .../user/auth/password_validators/base.py | 17 ++++++ .../password_validators/dependent_student.py | 3 - .../auth/password_validators/independent.py | 45 +++++++++++++++ .../independent_student.py | 3 - .../user/auth/password_validators/student.py | 25 +++++++++ .../user/auth/password_validators/teacher.py | 56 ++++++++++++++++++- .../auth/password_validators/__init__.py | 4 ++ .../tests/auth/password_validators/base.py | 41 ++++++++++++++ .../password_validators/test_independent.py | 48 ++++++++++++++++ .../auth/password_validators/test_student.py | 25 +++++++++ .../auth/password_validators/test_teacher.py | 55 ++++++++++++++++++ 13 files changed, 329 insertions(+), 17 deletions(-) create mode 100644 codeforlife/user/auth/password_validators/base.py delete mode 100644 codeforlife/user/auth/password_validators/dependent_student.py create mode 100644 codeforlife/user/auth/password_validators/independent.py delete mode 100644 codeforlife/user/auth/password_validators/independent_student.py create mode 100644 codeforlife/user/auth/password_validators/student.py create mode 100644 codeforlife/user/tests/auth/password_validators/__init__.py create mode 100644 codeforlife/user/tests/auth/password_validators/base.py create mode 100644 codeforlife/user/tests/auth/password_validators/test_independent.py create mode 100644 codeforlife/user/tests/auth/password_validators/test_student.py create mode 100644 codeforlife/user/tests/auth/password_validators/test_teacher.py diff --git a/codeforlife/settings/django.py b/codeforlife/settings/django.py index 5d33718b..fd8359e7 100644 --- a/codeforlife/settings/django.py +++ b/codeforlife/settings/django.py @@ -103,23 +103,23 @@ # Password validation # https://docs.djangoproject.com/en/3.2/ref/settings/#auth-password-validators -# TODO: replace with custom validators: -# 1. codeforlife.user.auth.password_validators.TeacherPasswordValidator -# 2. codeforlife.user.auth.password_validators.DependentStudentPasswordValidator -# 3. codeforlife.user.auth.password_validators.IndependentStudentPasswordValidator -# 4. codeforlife.user.auth.password_validators.CommonPasswordValidator +# TODO: compare Django's default common password validator with our own and decide which to keep +# codeforlife.user.auth.password_validators.CommonPasswordValidator AUTH_PASSWORD_VALIDATORS = [ { "NAME": "django.contrib.auth.password_validation.UserAttributeSimilarityValidator", }, { - "NAME": "django.contrib.auth.password_validation.MinimumLengthValidator", + "NAME": "django.contrib.auth.password_validation.CommonPasswordValidator", }, { - "NAME": "django.contrib.auth.password_validation.CommonPasswordValidator", + "NAME": "codeforlife.user.auth.password_validators.TeacherPasswordValidator", + }, + { + "NAME": "codeforlife.user.auth.password_validators.StudentPasswordValidator", }, { - "NAME": "django.contrib.auth.password_validation.NumericPasswordValidator", + "NAME": "codeforlife.user.auth.password_validators.IndependentPasswordValidator", }, ] diff --git a/codeforlife/user/auth/password_validators/__init__.py b/codeforlife/user/auth/password_validators/__init__.py index e69de29b..510580ae 100644 --- a/codeforlife/user/auth/password_validators/__init__.py +++ b/codeforlife/user/auth/password_validators/__init__.py @@ -0,0 +1,8 @@ +""" +© Ocado Group +Created on 30/01/2024 at 12:28:00(+00:00). +""" + +from .student import StudentPasswordValidator +from .independent import IndependentPasswordValidator +from .teacher import TeacherPasswordValidator diff --git a/codeforlife/user/auth/password_validators/base.py b/codeforlife/user/auth/password_validators/base.py new file mode 100644 index 00000000..ee3bdb5c --- /dev/null +++ b/codeforlife/user/auth/password_validators/base.py @@ -0,0 +1,17 @@ +""" +© Ocado Group +Created on 30/01/2024 at 17:41:00(+00:00). +""" + +import typing as t + +from ...models.user import User + + +# pylint: disable-next=too-few-public-methods +class PasswordValidator: + """Base class for all password validators""" + + # pylint: disable-next=missing-function-docstring + def validate(self, password: str, user: t.Optional[User] = None): + raise NotImplementedError() diff --git a/codeforlife/user/auth/password_validators/dependent_student.py b/codeforlife/user/auth/password_validators/dependent_student.py deleted file mode 100644 index 0d499b4f..00000000 --- a/codeforlife/user/auth/password_validators/dependent_student.py +++ /dev/null @@ -1,3 +0,0 @@ -# TODO: https://docs.djangoproject.com/en/3.2/topics/auth/passwords/#writing-your-own-validator -class DependentStudentPasswordValidator: - pass diff --git a/codeforlife/user/auth/password_validators/independent.py b/codeforlife/user/auth/password_validators/independent.py new file mode 100644 index 00000000..bdf9db0b --- /dev/null +++ b/codeforlife/user/auth/password_validators/independent.py @@ -0,0 +1,45 @@ +""" +© Ocado Group +Created on 30/01/2024 at 12:30:00(+00:00). +""" + +import re + +from django.core.exceptions import ValidationError +from django.utils.translation import gettext as _ + +from .base import PasswordValidator + + +# pylint: disable-next=missing-class-docstring +class IndependentPasswordValidator(PasswordValidator): + def validate(self, password, user=None): + if user.teacher is None and user.student is None: + min_length = 8 + + if len(password) < min_length: + raise ValidationError( + _( + f"Your password must be at least {min_length} " + f"characters long." + ), + code="password_too_short", + ) + + if not re.search(r"[A-Z]", password): + raise ValidationError( + _("Your password must have at least 1 uppercase letter."), + code="password_no_uppercase", + ) + + if not re.search(r"[a-z]", password): + raise ValidationError( + _("Your password must have at least 1 lowercase letter."), + code="password_no_lowercase", + ) + + if not re.search(r"[0-9]", password): + raise ValidationError( + _("Your password must have at least 1 digit."), + code="password_no_digit", + ) diff --git a/codeforlife/user/auth/password_validators/independent_student.py b/codeforlife/user/auth/password_validators/independent_student.py deleted file mode 100644 index 9ef3f13d..00000000 --- a/codeforlife/user/auth/password_validators/independent_student.py +++ /dev/null @@ -1,3 +0,0 @@ -# TODO: https://docs.djangoproject.com/en/3.2/topics/auth/passwords/#writing-your-own-validator -class IndependentStudentPasswordValidator: - pass diff --git a/codeforlife/user/auth/password_validators/student.py b/codeforlife/user/auth/password_validators/student.py new file mode 100644 index 00000000..812e1114 --- /dev/null +++ b/codeforlife/user/auth/password_validators/student.py @@ -0,0 +1,25 @@ +""" +© Ocado Group +Created on 30/01/2024 at 12:28:00(+00:00). +""" + +from django.core.exceptions import ValidationError +from django.utils.translation import gettext as _ + +from .base import PasswordValidator + + +# pylint: disable-next=missing-class-docstring +class StudentPasswordValidator(PasswordValidator): + def validate(self, password, user=None): + if user.student is not None: + min_length = 6 + + if len(password) < min_length: + raise ValidationError( + _( + f"Your password must be at least {min_length} " + f"characters long." + ), + code="password_too_short", + ) diff --git a/codeforlife/user/auth/password_validators/teacher.py b/codeforlife/user/auth/password_validators/teacher.py index 6625415d..8b3ea2d2 100644 --- a/codeforlife/user/auth/password_validators/teacher.py +++ b/codeforlife/user/auth/password_validators/teacher.py @@ -1,3 +1,53 @@ -# TODO: https://docs.djangoproject.com/en/3.2/topics/auth/passwords/#writing-your-own-validator -class TeacherPasswordValidator: - pass +""" +© Ocado Group +Created on 30/01/2024 at 12:32:00(+00:00). +""" + +import re + +from django.core.exceptions import ValidationError +from django.utils.translation import gettext as _ + +from .base import PasswordValidator + + +# pylint: disable-next=missing-class-docstring +class TeacherPasswordValidator(PasswordValidator): + def validate(self, password, user=None): + if user.teacher is not None: + min_length = 10 + + if len(password) < min_length: + raise ValidationError( + _( + f"Your password needs to be at least {min_length} " + f"characters long." + ), + code="password_too_short", + ) + + if not re.search(r"[A-Z]", password): + raise ValidationError( + _("Your password must have at least 1 uppercase letter."), + code="password_no_uppercase", + ) + + if not re.search(r"[a-z]", password): + raise ValidationError( + _("Your password must have at least 1 lowercase letter."), + code="password_no_lowercase", + ) + + if not re.search(r"[0-9]", password): + raise ValidationError( + _("Your password must have at least 1 digit."), + code="password_no_digit", + ) + + if not re.search( + r"[!@#$%^&*()_+\-=\[\]{};':\"\\|,.<>\/?]", password + ): + raise ValidationError( + _("Your password must have at least 1 special character."), + code="password_no_special_character", + ) diff --git a/codeforlife/user/tests/auth/password_validators/__init__.py b/codeforlife/user/tests/auth/password_validators/__init__.py new file mode 100644 index 00000000..329ee012 --- /dev/null +++ b/codeforlife/user/tests/auth/password_validators/__init__.py @@ -0,0 +1,4 @@ +""" +© Ocado Group +Created on 30/01/2024 at 12:36:00(+00:00). +""" diff --git a/codeforlife/user/tests/auth/password_validators/base.py b/codeforlife/user/tests/auth/password_validators/base.py new file mode 100644 index 00000000..27bbb466 --- /dev/null +++ b/codeforlife/user/tests/auth/password_validators/base.py @@ -0,0 +1,41 @@ +""" +© Ocado Group +Created on 30/01/2024 at 19:01:00(+00:00). + +Base test case for all password validators. +""" + +from django.core.exceptions import ValidationError +from django.test import TestCase + + +class PasswordValidatorTestCase(TestCase): + """Base for all password validator test cases.""" + + def assert_raises_validation_error(self, code: str, *args, **kwargs): + """Assert code block raises a validation error. + + Args: + code: The validation code to assert. + + Returns: + The assert-raises context which will auto-assert the code. + """ + + context = self.assertRaises(ValidationError, *args, **kwargs) + + class ContextWrapper: + """Wrap context to assert code on exit.""" + + def __init__(self, context): + self.context = context + + def __enter__(self, *args, **kwargs): + return self.context.__enter__(*args, **kwargs) + + def __exit__(self, *args, **kwargs): + value = self.context.__exit__(*args, **kwargs) + assert self.context.exception.code == code + return value + + return ContextWrapper(context) diff --git a/codeforlife/user/tests/auth/password_validators/test_independent.py b/codeforlife/user/tests/auth/password_validators/test_independent.py new file mode 100644 index 00000000..440eb68b --- /dev/null +++ b/codeforlife/user/tests/auth/password_validators/test_independent.py @@ -0,0 +1,48 @@ +""" +© Ocado Group +Created on 30/01/2024 at 12:36:00(+00:00). +""" + +from .base import PasswordValidatorTestCase +from ....auth.password_validators import IndependentPasswordValidator +from ....models.user import User + + +class TestIndependentPasswordValidator(PasswordValidatorTestCase): + @classmethod + def setUpClass(cls): + cls.user = User.objects.filter( + new_teacher__isnull=True, new_student__isnull=True + ).first() + assert cls.user is not None + + cls.validator = IndependentPasswordValidator() + super(TestIndependentPasswordValidator, cls).setUpClass() + + def test_validate__password_too_short(self): + """Password cannot be too short""" + password = "fxwSn4}" + + with self.assert_raises_validation_error("password_too_short"): + self.validator.validate(password, self.user) + + def test_validate__password_no_uppercase(self): + """Password must contain an uppercase char""" + password = ">28v*@a)-{" + + with self.assert_raises_validation_error("password_no_uppercase"): + self.validator.validate(password, self.user) + + def test_validate__password_no_lowercase(self): + """Password must contain a lowercase char""" + password = "F:6]LH!_5>" + + with self.assert_raises_validation_error("password_no_lowercase"): + self.validator.validate(password, self.user) + + def test_validate__password_no_digit(self): + """Password must contain a digit""" + password = "{$#FJdxGvs" + + with self.assert_raises_validation_error("password_no_digit"): + self.validator.validate(password, self.user) diff --git a/codeforlife/user/tests/auth/password_validators/test_student.py b/codeforlife/user/tests/auth/password_validators/test_student.py new file mode 100644 index 00000000..29695388 --- /dev/null +++ b/codeforlife/user/tests/auth/password_validators/test_student.py @@ -0,0 +1,25 @@ +""" +© Ocado Group +Created on 30/01/2024 at 12:36:00(+00:00). +""" + +from .base import PasswordValidatorTestCase +from ....auth.password_validators import StudentPasswordValidator +from ....models.user import User + + +class TestStudentPasswordValidator(PasswordValidatorTestCase): + @classmethod + def setUpClass(cls): + cls.user = User.objects.filter(new_student__isnull=False).first() + assert cls.user is not None + + cls.validator = StudentPasswordValidator() + super(TestStudentPasswordValidator, cls).setUpClass() + + def test_validate__password_too_short(self): + """Password cannot be too short""" + password = "fxwSn" + + with self.assert_raises_validation_error("password_too_short"): + self.validator.validate(password, self.user) diff --git a/codeforlife/user/tests/auth/password_validators/test_teacher.py b/codeforlife/user/tests/auth/password_validators/test_teacher.py new file mode 100644 index 00000000..849a20b4 --- /dev/null +++ b/codeforlife/user/tests/auth/password_validators/test_teacher.py @@ -0,0 +1,55 @@ +""" +© Ocado Group +Created on 30/01/2024 at 12:36:00(+00:00). +""" + +from .base import PasswordValidatorTestCase +from ....auth.password_validators import TeacherPasswordValidator +from ....models.user import User + + +class TestTeacherPasswordValidator(PasswordValidatorTestCase): + @classmethod + def setUpClass(cls): + cls.user = User.objects.filter(new_teacher__isnull=False).first() + assert cls.user is not None + + cls.validator = TeacherPasswordValidator() + super(TestTeacherPasswordValidator, cls).setUpClass() + + def test_validate__password_too_short(self): + """Password cannot be too short""" + password = "fxwSn4}PW" + + with self.assert_raises_validation_error("password_too_short"): + self.validator.validate(password, self.user) + + def test_validate__password_no_uppercase(self): + """Password must contain an uppercase char""" + password = ">28v*@a)-{" + + with self.assert_raises_validation_error("password_no_uppercase"): + self.validator.validate(password, self.user) + + def test_validate__password_no_lowercase(self): + """Password must contain a lowercase char""" + password = "F:6]LH!_5>" + + with self.assert_raises_validation_error("password_no_lowercase"): + self.validator.validate(password, self.user) + + def test_validate__password_no_digit(self): + """Password must contain a digit""" + password = "{$#FJdxGvs" + + with self.assert_raises_validation_error("password_no_digit"): + self.validator.validate(password, self.user) + + def test_validate__password_no_special_character(self): + """Password must contain a special char""" + password = "kR48SsAwrE" + + with self.assert_raises_validation_error( + "password_no_special_character" + ): + self.validator.validate(password, self.user)