From 33545bb9f0c7a9126ea9a45428d1a9cce7a07092 Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Wed, 15 Nov 2023 04:20:18 +0530 Subject: [PATCH] [feature] Added password expiration #359 Closes #359 Co-authored-by: Federico Capoano --- .gitignore | 5 +- README.rst | 109 +++++++++++- .../accounts/css/change-password.css | 53 ++++++ .../accounts/templates/account/base.html | 2 + .../templates/account/password_change.html | 55 ++++++ .../account/password_change_success.html | 20 +++ openwisp_users/accounts/urls.py | 13 ++ openwisp_users/accounts/views.py | 33 ++++ openwisp_users/admin.py | 4 +- openwisp_users/backends.py | 1 - openwisp_users/base/models.py | 23 +++ openwisp_users/middleware.py | 42 +++++ .../migrations/0019_user_password_updated.py | 20 +++ .../0020_populate_password_updated_field.py | 25 +++ openwisp_users/password_validation.py | 21 +++ openwisp_users/settings.py | 6 + .../openwisp-users/css/change-password.css | 26 +++ openwisp_users/tasks.py | 65 +++++++ openwisp_users/tests/test_accounts.py | 96 ++++++++++ openwisp_users/tests/test_admin.py | 168 +++++++++++++++++- openwisp_users/tests/test_middlewares.py | 49 +++++ openwisp_users/tests/test_models.py | 107 +++++++++++ requirements.txt | 2 +- tests/openwisp2/__init__.py | 3 + tests/openwisp2/celery.py | 9 + .../sample_users/migrations/0001_initial.py | 6 + tests/openwisp2/sample_users/tests.py | 14 ++ tests/openwisp2/settings.py | 22 +++ tests/testapp/tests/tests.py | 8 + 29 files changed, 1000 insertions(+), 7 deletions(-) create mode 100644 openwisp_users/accounts/static/openwisp-users/accounts/css/change-password.css create mode 100644 openwisp_users/accounts/templates/account/password_change.html create mode 100644 openwisp_users/accounts/templates/account/password_change_success.html create mode 100644 openwisp_users/accounts/views.py create mode 100644 openwisp_users/middleware.py create mode 100644 openwisp_users/migrations/0019_user_password_updated.py create mode 100644 openwisp_users/migrations/0020_populate_password_updated_field.py create mode 100644 openwisp_users/password_validation.py create mode 100644 openwisp_users/static/openwisp-users/css/change-password.css create mode 100644 openwisp_users/tasks.py create mode 100644 openwisp_users/tests/test_accounts.py create mode 100644 openwisp_users/tests/test_middlewares.py create mode 100644 tests/openwisp2/celery.py diff --git a/.gitignore b/.gitignore index 45a79e7b..e14129b4 100644 --- a/.gitignore +++ b/.gitignore @@ -39,7 +39,7 @@ pip-delete-this-directory.txt htmlcov/ .tox/ .coverage -# When running coverage testing +# When running coverage testing # it creates .coverage-username.123x* files .coverage.* .cache @@ -53,6 +53,9 @@ coverage.xml # Django stuff: *.log +# celery +tests/celerybeat* + # Sphinx documentation docs/_build/ diff --git a/README.rst b/README.rst index 5129edff..f236c2bb 100644 --- a/README.rst +++ b/README.rst @@ -102,7 +102,7 @@ Setup (integrate in an existing django project) 'drf_yasg', ] -also add ``AUTH_USER_MODEL``, ``SITE_ID`` and ``AUTHENTICATION_BACKENDS`` +also add ``AUTH_USER_MODEL``, ``SITE_ID``, ``AUTHENTICATION_BACKENDS`` and ``MIDDLEWARE`` to your ``settings.py``: .. code-block:: python @@ -112,6 +112,31 @@ to your ``settings.py``: AUTHENTICATION_BACKENDS = [ 'openwisp_users.backends.UsersAuthenticationBackend', ] + MIDDLEWARE = [ + # Other middlewares + 'openwisp_users.middleware.PasswordExpirationMiddleware', + ] + +Configure celery (you may use a different broker if you want): + +.. code-block:: python + + # here we show how to configure celery with redis but you can + # use other brokers if you want, consult the celery docs + CELERY_BROKER_URL = 'redis://localhost/1' + CELERY_BEAT_SCHEDULE = { + 'password_expiry_email': { + 'task': 'openwisp_users.tasks.password_expiration_email', + 'schedule': crontab(hour=1, minute=0), + }, + } + +If you decide to use Redis (as shown in these examples), +install the following python packages. + +.. code-block:: shell + + pip install redis django-redis ``urls.py``: @@ -178,6 +203,15 @@ Create database: ./manage.py migrate ./manage.py createsuperuser + +Run celery and celery-beat with the following commands (separate terminal windows are needed): + +.. code-block:: shell + + cd tests/ + celery -A openwisp2 worker -l info + celery -A openwisp2 beat -l info + Launch development server: .. code-block:: shell @@ -305,6 +339,34 @@ command. The ``select_related`` property can be used to optimize the database query. +``OPENWISP_USERS_USER_PASSWORD_EXPIRATION`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + ++--------------+-------------+ +| **type**: | ``integer`` | ++--------------+-------------+ +| **default**: | ``0`` | ++--------------+-------------+ + +Number of days after which a user's password will expire. +In other words, it determines when users will be prompted to +change their passwords. + +If set to ``0``, this feature is disabled, and users are not +required to change their passwords. + +``OPENWISP_USERS_STAFF_USER_PASSWORD_EXPIRATION`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + ++--------------+-------------+ +| **type**: | ``integer`` | ++--------------+-------------+ +| **default**: | ``0`` | ++--------------+-------------+ + +Similar to `OPENWISP_USERS_USER_PASSWORD_EXPIRATION <#openwisp-users-user-password-expiration>`_, +but for **staff users**. + REST API -------- @@ -764,6 +826,51 @@ The authentication backend can also be used as follows: backend = UsersAuthenticationBackend() backend.authenticate(request, identifier, password) +Password Validators +------------------- + +``openwisp_users.password_validation.PasswordReuseValidator`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +On password change views, the ``PasswordReuseValidator`` +ensures that users cannot use their current password as the new password. + +You need to add the validator to ``AUTH_PASSWORD_VALIDATORS`` Django +setting as shown below: + +.. code-block:: python + + # in your-project/settings.py + AUTH_PASSWORD_VALIDATORS = [ + # Other password validators + { + "NAME": "openwisp_users.password_validation.PasswordReuseValidator", + }, + ] + +Middlewares +----------- + +``openwisp_users.middleware.PasswordExpirationMiddleware`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +When password expiration feature is on +(see `OPENWISP_USERS_USER_PASSWORD_EXPIRATION <#openwisp-users-user-password-expiration>`_ +and `OPENWISP_USERS_STAFF_USER_PASSWORD_EXPIRATION <#openwisp-users-staff-user-password-expiration>`_), +this middleware confines the user to the *password change view* until they change their password. + +This middleware should come after ``AuthenticationMiddleware`` and ``MessageMiddleware``, as following: + +.. code-block:: python + + # in your_project/settings.py + MIDDLEWARE = [ + # Other middlewares + 'django.contrib.auth.middleware.AuthenticationMiddleware', + 'django.contrib.messages.middleware.MessageMiddleware', + 'openwisp_users.middleware.PasswordExpirationMiddleware', + ] + Django REST Framework Authentication Classes -------------------------------------------- diff --git a/openwisp_users/accounts/static/openwisp-users/accounts/css/change-password.css b/openwisp_users/accounts/static/openwisp-users/accounts/css/change-password.css new file mode 100644 index 00000000..bad72580 --- /dev/null +++ b/openwisp_users/accounts/static/openwisp-users/accounts/css/change-password.css @@ -0,0 +1,53 @@ +form.password_change p { + overflow: hidden; + padding: 15px; + font-size: 15px; + border-bottom: 1px solid var(--hairline-color); + box-sizing: border-box; +} +form.password_change label { + display: inline-block; + padding: 6px 10px 0 0; + width: 160px; + word-wrap: break-word; + line-height: 1; + font-weight: bold; +} +#main form.password_change .submit-row { + text-align: left; +} +#main form.password_change .default { + float: unset; +} +#main form.password_change .submit-row input { + margin: 0 15px 0 0; +} +#main .submit-row .button { + font-size: 14px; + padding: 0.625rem 1rem; +} +#main .errorlist { + margin: 5px 0; +} +#main .errorlist li { + font-size: 15px; + font-weight: bold; +} + +@media (max-width: 767px) { + #main form.password_change label { + padding: 0 0 10px; + width: 100%; + } + + #main form.password_change input { + width: 100%; + } + + #main form.password_change .submit-row .button { + width: 100%; + margin: 0 0 15px 0; + padding-right: 0; + padding-left: 0; + } +} diff --git a/openwisp_users/accounts/templates/account/base.html b/openwisp_users/accounts/templates/account/base.html index 37cdd511..a536d5a7 100644 --- a/openwisp_users/accounts/templates/account/base.html +++ b/openwisp_users/accounts/templates/account/base.html @@ -1,2 +1,4 @@ {% extends "admin/base_site.html" %} {% block title %}OpenWISP2{% endblock %} + +{% block breadcrumbs %}{% endblock %} diff --git a/openwisp_users/accounts/templates/account/password_change.html b/openwisp_users/accounts/templates/account/password_change.html new file mode 100644 index 00000000..3a42ba10 --- /dev/null +++ b/openwisp_users/accounts/templates/account/password_change.html @@ -0,0 +1,55 @@ +{% extends "account/base.html" %} + +{% load i18n %} +{% load static %} + +{% block title %}{% trans "Change Password" %}{% endblock %} +{% block extrahead %} + +{% endblock %} + +{% block extrastyle %} +{{ block.super }} +{{ form.media.css }} +{% endblock extrastyle %} + +{% block content %} +

{% trans "Change Password" %}

+ +
+ {% csrf_token %} +
+
+ {{ form.oldpassword.errors }} + {{ form.oldpassword.label_tag }} + {{ form.oldpassword }} + {% if form.oldpassword.help_text %} +
{{ form.oldpassword.help_text|safe }}
+ {% endif %} +
+
+ {{ form.password1.errors }} + {{ form.password1.label_tag }} + {{ form.password1 }} + {% if form.password1.help_text %} +
{{ form.password1.help_text|safe }}
+ {% endif %} +
+
+ {{ form.password2.errors }} + {{ form.password2.label_tag }} + {{ form.password2 }} + {% if form.password2.help_text %} +
{{ form.password2.help_text|safe }}
+ {% endif %} +
+
+
+ + {% trans "Forgot Password?" %} +
+
+{% endblock %} diff --git a/openwisp_users/accounts/templates/account/password_change_success.html b/openwisp_users/accounts/templates/account/password_change_success.html new file mode 100644 index 00000000..2f39d26c --- /dev/null +++ b/openwisp_users/accounts/templates/account/password_change_success.html @@ -0,0 +1,20 @@ +{% extends "account/base.html" %} +{% load i18n %} + +{% block extrastyle %} +{{ block.super }} + +{% endblock %} + +{% block content %} +

{% trans "Your password has been changed successfully." %}

+

{% trans "This web page can be closed." %}

+{% endblock %} diff --git a/openwisp_users/accounts/urls.py b/openwisp_users/accounts/urls.py index 6064aa62..233be2e8 100644 --- a/openwisp_users/accounts/urls.py +++ b/openwisp_users/accounts/urls.py @@ -14,6 +14,8 @@ from django.views.generic import RedirectView from django.views.generic.base import TemplateView +from .views import password_change, password_change_success + redirect_view = RedirectView.as_view(url=reverse_lazy('admin:index')) @@ -33,6 +35,17 @@ views.confirm_email, name='account_confirm_email', ), + # password change + path( + 'password/change/', + password_change, + name="account_change_password", + ), + path( + 'password/change/success/', + password_change_success, + name='account_change_password_success', + ), # password reset path('password/reset/', views.password_reset, name='account_reset_password'), path( diff --git a/openwisp_users/accounts/views.py b/openwisp_users/accounts/views.py new file mode 100644 index 00000000..eb44eaa0 --- /dev/null +++ b/openwisp_users/accounts/views.py @@ -0,0 +1,33 @@ +from allauth.account.forms import ChangePasswordForm as BaseChangePasswordForm +from allauth.account.views import PasswordChangeView as BasePasswordChangeView +from django import forms +from django.contrib.auth import REDIRECT_FIELD_NAME +from django.contrib.auth.decorators import login_required +from django.urls import reverse_lazy +from django.views.generic.base import TemplateView + + +class ChangePasswordForm(BaseChangePasswordForm): + next = forms.CharField(widget=forms.HiddenInput, required=False) + + +class PasswordChangeView(BasePasswordChangeView): + form_class = ChangePasswordForm + template_name = 'account/password_change.html' + success_url = reverse_lazy('account_change_password_success') + + def get_success_url(self): + if self.request.POST.get(REDIRECT_FIELD_NAME): + return self.request.POST.get(REDIRECT_FIELD_NAME) + return super().get_success_url() + + def get_initial(self): + data = super().get_initial() + data['next'] = self.request.GET.get(REDIRECT_FIELD_NAME) + return data + + +password_change = login_required(PasswordChangeView.as_view()) +password_change_success = login_required( + TemplateView.as_view(template_name='account/password_change_success.html') +) diff --git a/openwisp_users/admin.py b/openwisp_users/admin.py index 36db7d14..59d5ce35 100644 --- a/openwisp_users/admin.py +++ b/openwisp_users/admin.py @@ -193,7 +193,7 @@ class UserAdmin(MultitenantAdminMixin, BaseUserAdmin, BaseAdmin): add_form = UserCreationForm form = UserChangeForm ordering = ['-date_joined'] - readonly_fields = ['last_login', 'date_joined'] + readonly_fields = ['last_login', 'date_joined', 'password_updated'] list_display = [ 'username', 'email', @@ -476,6 +476,8 @@ def queryset(self, request, queryset): additional_fields = ['bio', 'url', 'company', 'location', 'phone_number', 'birth_date'] UserAdmin.fieldsets[1][1]['fields'] = base_fields + additional_fields UserAdmin.fieldsets.insert(3, ('Internal', {'fields': ('notes',)})) +primary_fields = list(UserAdmin.fieldsets[0][1]['fields']) +UserAdmin.fieldsets[0][1]['fields'] = primary_fields + ['password_updated'] UserAdmin.add_fieldsets[0][1]['fields'] = ( 'username', 'email', diff --git a/openwisp_users/backends.py b/openwisp_users/backends.py index 33ead15a..cd64c9ba 100644 --- a/openwisp_users/backends.py +++ b/openwisp_users/backends.py @@ -20,7 +20,6 @@ def authenticate(self, request, username=None, password=None, **kwargs): return None if user.check_password(password) and self.user_can_authenticate(user): return user - return None def get_users(self, identifier): conditions = Q(email=identifier) | Q(username=identifier) diff --git a/openwisp_users/base/models.py b/openwisp_users/base/models.py index 860f7469..2876014c 100644 --- a/openwisp_users/base/models.py +++ b/openwisp_users/base/models.py @@ -8,11 +8,14 @@ from django.core.cache import cache from django.core.exceptions import ValidationError from django.db import models +from django.utils import timezone from django.utils.functional import cached_property from django.utils.translation import gettext_lazy as _ from phonenumber_field.modelfields import PhoneNumberField from swapper import load_model +from .. import settings as app_settings + logger = logging.getLogger(__name__) @@ -64,6 +67,7 @@ class AbstractUser(BaseUser): choices=settings.LANGUAGES, default=settings.LANGUAGE_CODE, ) + password_updated = models.DateField(_('password updated'), blank=True, null=True) objects = UserManager() @@ -84,6 +88,25 @@ def _get_pk(obj): raise ValueError('expected UUID, str or Organization instance') return str(pk) + def set_password(self, *args, **kwargs): + self.password_updated = timezone.now().date() + return super().set_password(*args, **kwargs) + + def has_password_expired(self): + if not self.has_usable_password(): + return False + if self.is_staff and app_settings.STAFF_USER_PASSWORD_EXPIRATION: + expiry_date = self.password_updated + timezone.timedelta( + days=app_settings.STAFF_USER_PASSWORD_EXPIRATION + ) + elif app_settings.USER_PASSWORD_EXPIRATION: + expiry_date = self.password_updated + timezone.timedelta( + days=app_settings.USER_PASSWORD_EXPIRATION + ) + else: + return False + return expiry_date < timezone.now().date() + def is_member(self, organization): return self._get_pk(organization) in self.organizations_dict diff --git a/openwisp_users/middleware.py b/openwisp_users/middleware.py new file mode 100644 index 00000000..0c16addd --- /dev/null +++ b/openwisp_users/middleware.py @@ -0,0 +1,42 @@ +from django.contrib import messages +from django.contrib.auth import REDIRECT_FIELD_NAME +from django.shortcuts import redirect +from django.urls import reverse_lazy +from django.utils.translation import gettext_lazy as _ + + +class PasswordExpirationMiddleware: + exempted_paths = [ + reverse_lazy('account_change_password'), + reverse_lazy('admin:logout'), + reverse_lazy('account_logout'), + ] + admin_login_path = reverse_lazy('admin:login') + admin_index_path = reverse_lazy('admin:index') + account_change_password_path = reverse_lazy('account_change_password') + + def __init__(self, get_response): + self.get_response = get_response + + def __call__(self, request): + response = self.get_response(request) + # Check if the user is authenticated and their password has expired + if ( + request.user.is_authenticated + and request.user.has_password_expired() + and request.path not in self.exempted_paths + ): + messages.warning( + request, + _('Your password has expired, please update your password.'), + ) + redirect_path = self.account_change_password_path + if request.user.is_staff: + next_path = ( + request.path + if request.path != self.admin_login_path + else self.admin_index_path + ) + redirect_path = f'{redirect_path}?{REDIRECT_FIELD_NAME}={next_path}' + return redirect(redirect_path) + return response diff --git a/openwisp_users/migrations/0019_user_password_updated.py b/openwisp_users/migrations/0019_user_password_updated.py new file mode 100644 index 00000000..6f655ba9 --- /dev/null +++ b/openwisp_users/migrations/0019_user_password_updated.py @@ -0,0 +1,20 @@ +# Generated by Django 4.2.2 on 2023-10-18 10:13 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("openwisp_users", "0018_allow_operator_view_organisation"), + ] + + operations = [ + migrations.AddField( + model_name="user", + name="password_updated", + field=models.DateField( + blank=True, null=True, verbose_name="password updated" + ), + ), + ] diff --git a/openwisp_users/migrations/0020_populate_password_updated_field.py b/openwisp_users/migrations/0020_populate_password_updated_field.py new file mode 100644 index 00000000..c2dc2191 --- /dev/null +++ b/openwisp_users/migrations/0020_populate_password_updated_field.py @@ -0,0 +1,25 @@ +# Generated by Django 4.2.2 on 2023-10-18 10:13 + +from django.contrib.auth import get_user_model +from django.contrib.auth.hashers import UNUSABLE_PASSWORD_PREFIX +from django.db import migrations +from django.utils import timezone + + +def populate_password_updated_field(apps, schema_editor): + User = get_user_model() + User.objects.exclude( + # Exclude users having unusable password + password__startswith=UNUSABLE_PASSWORD_PREFIX, + ).update(password_updated=timezone.now().date()) + + +class Migration(migrations.Migration): + + dependencies = [ + ("openwisp_users", "0019_user_password_updated"), + ] + + operations = [ + migrations.RunPython(populate_password_updated_field), + ] diff --git a/openwisp_users/password_validation.py b/openwisp_users/password_validation.py new file mode 100644 index 00000000..07f0a386 --- /dev/null +++ b/openwisp_users/password_validation.py @@ -0,0 +1,21 @@ +from django.core.exceptions import ValidationError +from django.utils.translation import gettext_lazy as _ + + +class PasswordReuseValidator: + """ + Django password validator class that does not allow re-using + user's current password. + """ + + def validate(self, password, user=None): + if user is None: + return + if user.check_password(password): + # The new password is same as the current password + raise ValidationError( + _('You cannot re-use your current password. Enter a new password.') + ) + + def get_help_text(self): + return '' diff --git a/openwisp_users/settings.py b/openwisp_users/settings.py index a713f59c..ca81c721 100644 --- a/openwisp_users/settings.py +++ b/openwisp_users/settings.py @@ -32,6 +32,12 @@ ], 'select_related': [], } +USER_PASSWORD_EXPIRATION = getattr( + settings, 'OPENWISP_USERS_USER_PASSWORD_EXPIRATION', 0 +) +STAFF_USER_PASSWORD_EXPIRATION = getattr( + settings, 'OPENWISP_USERS_STAFF_USER_PASSWORD_EXPIRATION', 0 +) # Set the AutocompleteFilter view if it is not defined in the settings setattr( settings, diff --git a/openwisp_users/static/openwisp-users/css/change-password.css b/openwisp_users/static/openwisp-users/css/change-password.css new file mode 100644 index 00000000..67d877e8 --- /dev/null +++ b/openwisp_users/static/openwisp-users/css/change-password.css @@ -0,0 +1,26 @@ +form.password_change p { + overflow: hidden; + padding: 15px; + font-size: 15px; + border-bottom: 1px solid var(--hairline-color); + box-sizing: border-box; +} +form.password_change label { + display: inline-block; + padding: 6px 10px 0 0; + width: 160px; + word-wrap: break-word; + line-height: 1; + font-weight: bold; +} + +@media (max-width: 767px) { + form.password_change label { + padding: 0 0 10px; + width: 100%; + } + + form.password_change input { + width: 100%; + } +} diff --git a/openwisp_users/tasks.py b/openwisp_users/tasks.py new file mode 100644 index 00000000..f056bec6 --- /dev/null +++ b/openwisp_users/tasks.py @@ -0,0 +1,65 @@ +from celery import shared_task +from django.contrib.auth import get_user_model +from django.contrib.auth.hashers import UNUSABLE_PASSWORD_PREFIX +from django.contrib.sites.models import Site +from django.db.models import Q +from django.urls import reverse +from django.utils.timezone import now, timedelta +from django.utils.translation import gettext_lazy as _ +from openwisp_utils.admin_theme.email import send_email + +from . import settings as app_settings + +User = get_user_model() + + +@shared_task +def password_expiration_email(): + """ + Notify users whose password is expiring in exactly 7 days. + """ + if ( + not app_settings.USER_PASSWORD_EXPIRATION + and not app_settings.STAFF_USER_PASSWORD_EXPIRATION + ): + # The password expiration feature is not enabled + return + expiry_date = now().date() + timedelta(days=7) + query = Q() + if app_settings.USER_PASSWORD_EXPIRATION: + query |= Q( + is_staff=False, + password_updated=expiry_date + - timedelta(days=app_settings.USER_PASSWORD_EXPIRATION), + ) + if app_settings.STAFF_USER_PASSWORD_EXPIRATION: + query |= Q( + is_staff=True, + password_updated=expiry_date + - timedelta(days=app_settings.STAFF_USER_PASSWORD_EXPIRATION), + ) + current_site = Site.objects.get_current() + qs = ( + User.objects.exclude( + # Exclude users having unusable password + password__startswith=UNUSABLE_PASSWORD_PREFIX, + ) + .filter( + emailaddress__verified=True, + ) + .filter(query) + ) + for user in qs.iterator(): + send_email( + subject=_('Your password is about to expire'), + body_text=_('Your password is about to expire in 7 days'), + body_html=_('You password is about to expire in 7 days.'), + recipients=[user.email], + extra_context={ + 'call_to_action_url': 'https://{0}{1}'.format( + current_site.domain, + reverse('account_change_password'), + ), + 'call_to_action_text': _('Change password'), + }, + ) diff --git a/openwisp_users/tests/test_accounts.py b/openwisp_users/tests/test_accounts.py new file mode 100644 index 00000000..c85e6803 --- /dev/null +++ b/openwisp_users/tests/test_accounts.py @@ -0,0 +1,96 @@ +from unittest.mock import patch + +from django.contrib.auth import get_user_model +from django.test import TestCase +from django.urls import reverse +from django.utils.timezone import now, timedelta + +from .. import settings as app_settings +from .utils import TestOrganizationMixin + +User = get_user_model() + + +class TestAccountView(TestOrganizationMixin, TestCase): + def _login_user(self, username='tester', password='tester'): + response = self.client.post( + reverse('account_login'), + data={'login': username, 'password': password}, + follow=True, + ) + return response + + @patch.object(app_settings, 'USER_PASSWORD_EXPIRATION', 30) + def test_password_expired_user_logins(self): + self._create_org_user() + User.objects.update(password_updated=now() - timedelta(days=60)) + response = self._login_user() + self.assertContains( + response, + ( + '' + ), + html=True, + ) + self.assertEqual( + response.request.get('PATH_INFO'), reverse('account_change_password') + ) + # Password expired users can browse accounts views + self.assertContains( + response, '' + ) + self.assertContains(response, '') + + def _test_login_flow(self): + self._create_org_user() + User.objects.update(password_updated=now() - timedelta(days=60)) + response = self._login_user() + self.assertContains( + response, + ( + '' + ), + html=True, + ) + self.assertNotContains( + response, + ( + '
  • Your password has expired, please update ' + 'your password at http://testserver/accounts/password/change/
    • ' + ), + ) + + @patch.object(app_settings, 'USER_PASSWORD_EXPIRATION', 0) + def test_user_login_password_expiration_disabled(self): + self._test_login_flow() + + @patch.object(app_settings, 'USER_PASSWORD_EXPIRATION', 90) + def test_user_login_password_expiration_enabled(self): + self._test_login_flow() + + def test_redirection_to_success_page_after_password_update(self): + user = self._create_operator() + self.client.force_login(user) + response = self.client.post( + reverse('account_change_password'), + data={ + 'oldpassword': 'tester', + 'password1': 'newpassword', + 'password2': 'newpassword', + }, + follow=True, + ) + self.assertContains(response, 'Your password has been changed successfully.') + self.assertContains(response, 'This web page can be closed.') + + def test_inactive_user_login(self): + self._create_org_user() + User.objects.update(is_active=False) + response = self._login_user() + self.assertContains( + response, 'The username and/or password you specified are not correct.' + ) diff --git a/openwisp_users/tests/test_admin.py b/openwisp_users/tests/test_admin.py index ddc39e1d..74b99e49 100644 --- a/openwisp_users/tests/test_admin.py +++ b/openwisp_users/tests/test_admin.py @@ -5,16 +5,19 @@ from unittest.mock import patch from django.contrib import admin -from django.contrib.auth import get_user_model +from django.contrib.auth import REDIRECT_FIELD_NAME, get_user_model from django.contrib.auth.models import Permission from django.core import mail from django.core.exceptions import ValidationError from django.db import DEFAULT_DB_ALIAS -from django.test import TestCase +from django.template.defaultfilters import date +from django.test import TestCase, override_settings from django.urls import reverse +from django.utils.timezone import now, timedelta from openwisp_utils.tests import capture_any_output from swapper import load_model +from .. import settings as app_settings from ..admin import OrganizationOwnerAdmin from ..apps import logger as apps_logger from ..multitenancy import MultitenantAdminMixin @@ -128,6 +131,7 @@ def test_admin_change_user_auto_email(self): params.pop('password', None) params.pop('_password', None) params.pop('last_login') + params.pop('password_updated') params = self._additional_params_pop(params) # inline emails params.update(self.add_user_inline_params) @@ -200,6 +204,71 @@ def test_admin_change_user_page_get_invalid_UUID(self): content = f'User with ID “{id}” doesn’t exist. Perhaps it was deleted?' self.assertContains(response, content, status_code=200) + def test_admin_change_user_password_updated(self): + admin = self._create_admin() + # User.objects.create_user does not execute User.set_password + # which is required for setting User.password_updated field + admin.set_password('tester') + admin.save() + self.client.force_login(admin) + response = self.client.get( + reverse(f'admin:{self.app_label}_user_change', args=[admin.pk]), + ) + self.assertContains( + response, + ( + '\n\n' + f'
    {date(now())}
    ' + ), + html=True, + ) + + def test_admin_change_user_reuse_password(self): + admin = self._create_admin(password='tester') + self.client.force_login(admin) + path = reverse('admin:auth_user_password_change', args=[admin.pk]) + data = {'password1': 'tester', 'password2': 'tester'} + with override_settings( + AUTH_PASSWORD_VALIDATORS=[ + { + "NAME": "openwisp_users.password_validation.PasswordReuseValidator", + }, + ] + ): + response = self.client.post( + path, + data=data, + follow=True, + ) + self.assertNotContains( + response, '
  • Password changed successfully.
    • ' + ) + self.assertContains( + response, + ( + '' + ), + ) + with override_settings(AUTH_PASSWORD_VALIDATORS=[]): + response = self.client.post( + path, + data=data, + follow=True, + ) + self.assertNotContains( + response, + ( + '' + ), + ) + self.assertContains( + response, '
  • Password changed successfully.
    • ' + ) + def test_organization_view_on_site(self): admin = self._create_admin() self.client.force_login(admin) @@ -514,6 +583,7 @@ def test_update_user_no_validation_error(self): params = user.__dict__ params['username'] = 'user2' params.pop('last_login') + params.pop('password_updated') params.pop('phone_number') params.pop('password', None) params.pop('_password', None) @@ -554,6 +624,7 @@ def test_edit_user_email_exists(self): params.pop('password', None) params.pop('_password', None) params.pop('last_login') + params.pop('password_updated') params = self._additional_params_pop(params) params.update(self.add_user_inline_params) params.update( @@ -584,6 +655,7 @@ def test_change_staff_without_group(self): params.pop('password', None) params.pop('_password', None) params.pop('last_login') + params.pop('password_updated') params.pop('phone_number') params.update(self.add_user_inline_params) params.update(self._additional_params_add()) @@ -609,6 +681,7 @@ def test_change_staff_with_group(self): params.pop('password', None) params.pop('_password', None) params.pop('last_login') + params.pop('password_updated') params.update(self.add_user_inline_params) params.update(self._additional_params_add()) params.update(self._get_user_edit_form_inline_params(user, org)) @@ -674,6 +747,7 @@ def _test_change(self, options): params.pop('password', None) params.pop('_password', None) params.pop('last_login') + params.pop('password_updated') params.update(self.add_user_inline_params) params.update(self._additional_params_add()) params.update(self._get_user_edit_form_inline_params(user2, org)) @@ -1569,6 +1643,7 @@ def test_change_user(self): params.pop('password', None) params.pop('_password', None) params.pop('last_login') + params.pop('password_updated') params['birth_date'] = user.date_joined.date() params = self._additional_params_pop(params) params.update(self._get_user_edit_form_inline_params(user, org)) @@ -1598,6 +1673,7 @@ def _delete_inline_org_user(self, is_admin=False): params.pop('password', None) params.pop('_password', None) params.pop('last_login') + params.pop('password_updated') params = self._additional_params_pop(params) params.update(self._get_user_edit_form_inline_params(user, org)) params.update({f'{self.app_label}_organizationuser-0-DELETE': 'on'}) @@ -1820,3 +1896,91 @@ def test_organization_user_filter(self): assert option['id'] not in Organization.objects.exclude( pk__in=user.organizations_managed ) + + +class TestUserPasswordExpiration(TestOrganizationMixin, TestCase): + @patch.object(app_settings, 'STAFF_USER_PASSWORD_EXPIRATION', 30) + def test_expired_password_user_redirected(self): + self.client.logout() + user = self._create_admin() + user.password_updated = now() - timedelta(days=31) + user.save() + login_response = self.client.post( + reverse('admin:login'), + data={'username': user.username, 'password': 'tester'}, + ) + self.assertEqual(login_response.status_code, 302) + self.assertEqual(login_response.url, '/accounts/password/change/?next=/admin/') + + change_password_response = self.client.get(login_response.url) + self.assertEqual(change_password_response.status_code, 200) + self.assertContains( + change_password_response, + 'Your password has expired, please update your password.', + ) + + @patch.object(app_settings, 'STAFF_USER_PASSWORD_EXPIRATION', 30) + def test_non_expired_user_django_redirection(self): + self.client.logout() + redirect_url = reverse('admin:sites_site_changelist') + user = self._create_admin() + user.set_password('tester') + user.save() + login_response = self.client.post( + '{0}?next={1}'.format(reverse('admin:login'), redirect_url), + data={'username': user.username, 'password': 'tester'}, + ) + self.assertEqual(login_response.status_code, 302) + self.assertEqual( + login_response.url, + redirect_url, + ) + + @patch.object(app_settings, 'STAFF_USER_PASSWORD_EXPIRATION', 30) + def test_redirection_for_expired_user_after_password_update(self): + """ + This test ensures that user is confined to change password + page until they change thier password. + """ + self.client.logout() + user = self._create_admin() + user.password_updated = now() - timedelta(days=31) + user.save() + self.client.force_login(user) + site_changelist_path = reverse('admin:sites_site_changelist') + password_change_redirect_response = self.client.get(site_changelist_path) + self.assertEqual(password_change_redirect_response.status_code, 302) + self.assertEqual( + password_change_redirect_response.url, + '{0}?{1}={2}'.format( + reverse('account_change_password'), + REDIRECT_FIELD_NAME, + site_changelist_path, + ), + ) + response = self.client.get(password_change_redirect_response.url) + self.assertContains( + response, 'Your password has expired, please update your password.' + ) + change_password_response = self.client.post( + password_change_redirect_response.url, + data={ + 'oldpassword': 'tester', + 'password1': 'newpassword', + 'password2': 'newpassword', + 'next': site_changelist_path, + }, + follow=True, + ) + self.assertContains( + change_password_response, + ( + '' + ), + html=True, + ) + self.assertEqual( + change_password_response.request.get('PATH_INFO'), site_changelist_path + ) diff --git a/openwisp_users/tests/test_middlewares.py b/openwisp_users/tests/test_middlewares.py new file mode 100644 index 00000000..14ec4853 --- /dev/null +++ b/openwisp_users/tests/test_middlewares.py @@ -0,0 +1,49 @@ +from unittest.mock import patch + +from django.contrib.auth import get_user_model +from django.test import TestCase, modify_settings +from django.urls import reverse +from django.utils.timezone import now, timedelta + +from .. import settings as app_settings +from .utils import TestOrganizationMixin + +User = get_user_model() + + +class TestPasswordExpirationMiddleware(TestOrganizationMixin, TestCase): + @modify_settings( + MIDDLEWARE={ + 'remove': ['openwisp_users.middleware.PasswordExpirationMiddleware'] + } + ) + @patch.object(app_settings, 'STAFF_USER_PASSWORD_EXPIRATION', 10) + def test_queries_middleware_absent(self): + admin = self._create_admin() + with self.assertNumQueries(2): + response = self.client.post( + reverse('admin:login'), + data={'username': admin.username, 'password': 'tester'}, + ) + self.assertEqual(response.status_code, 302) + self.assertEqual(response.url, '/admin/') + with self.assertNumQueries(1): + self.client.force_login(admin) + + @modify_settings( + MIDDLEWARE={ + 'append': ['openwisp_users.middleware.PasswordExpirationMiddleware'] + } + ) + @patch.object(app_settings, 'STAFF_USER_PASSWORD_EXPIRATION', 10) + def test_queries_middleware_present(self): + admin = self._create_admin(password_updated=now().date() - timedelta(days=180)) + with self.assertNumQueries(2): + response = self.client.post( + reverse('admin:login'), + data={'username': admin.username, 'password': 'tester'}, + ) + self.assertEqual(response.status_code, 302) + self.assertEqual(response.url, '/accounts/password/change/?next=/admin/') + with self.assertNumQueries(1): + self.client.force_login(admin) diff --git a/openwisp_users/tests/test_models.py b/openwisp_users/tests/test_models.py index 10cdecf3..0b00a575 100644 --- a/openwisp_users/tests/test_models.py +++ b/openwisp_users/tests/test_models.py @@ -1,9 +1,16 @@ +from unittest.mock import patch + +from allauth.account.models import EmailAddress from django.contrib.auth import get_user_model +from django.core import mail from django.core.exceptions import ValidationError from django.test import TestCase, override_settings from django.urls import reverse +from django.utils.timezone import now, timedelta from swapper import load_model +from .. import settings as app_settings +from ..tasks import password_expiration_email from .utils import TestOrganizationMixin Organization = load_model('openwisp_users', 'Organization') @@ -340,3 +347,103 @@ def test_add_user(self): org_user = org.add_user(user) self.assertIsInstance(org_user, OrganizationUser) self.assertTrue(org_user.is_admin) + + def test_has_password_expired(self): + staff_user = self._create_operator() + end_user = self._create_user() + + # User.objects.create_user does not call User.set_password. + # Therefore, we set the password_updated field manually. + User.objects.update(password_updated=now().date()) + staff_user.refresh_from_db() + end_user.refresh_from_db() + + with self.subTest('Test password expiration disabled'): + with patch.object( + app_settings, 'USER_PASSWORD_EXPIRATION', 0 + ), patch.object(app_settings, 'STAFF_USER_PASSWORD_EXPIRATION', 0): + self.assertEqual(staff_user.has_password_expired(), False) + self.assertEqual(end_user.has_password_expired(), False) + + with self.subTest( + 'Test password expiration enabled, but user password not expired' + ): + with patch.object( + app_settings, 'USER_PASSWORD_EXPIRATION', 10 + ), patch.object(app_settings, 'STAFF_USER_PASSWORD_EXPIRATION', 10): + self.assertEqual(staff_user.has_password_expired(), False) + self.assertEqual(end_user.has_password_expired(), False) + + with self.subTest( + 'Test password expiration enabled, but user password is expired' + ): + User.objects.update(password_updated=now().date() - timedelta(days=180)) + staff_user.refresh_from_db() + end_user.refresh_from_db() + with patch.object( + app_settings, 'USER_PASSWORD_EXPIRATION', 10 + ), patch.object(app_settings, 'STAFF_USER_PASSWORD_EXPIRATION', 10): + self.assertEqual(staff_user.has_password_expired(), True) + self.assertEqual(end_user.has_password_expired(), True) + + @patch.object(app_settings, 'USER_PASSWORD_EXPIRATION', 30) + @patch.object(app_settings, 'STAFF_USER_PASSWORD_EXPIRATION', 90) + def test_password_expiration_mail(self): + user_expiry_date = now().today() - timedelta( + days=(app_settings.USER_PASSWORD_EXPIRATION - 7) + ) + staff_user_expiry_date = now().today() - timedelta( + days=(app_settings.STAFF_USER_PASSWORD_EXPIRATION - 7) + ) + staff_user = self._create_operator() + end_user = self._create_user() + + with self.subTest( + 'Test end-user and staff user has different expiration dates' + ): + User.objects.filter(is_staff=False).update( + password_updated=user_expiry_date + ) + User.objects.filter(is_staff=True).update( + password_updated=staff_user_expiry_date + ) + password_expiration_email.delay() + self.assertEqual(len(mail.outbox), 2) + self.assertEqual(mail.outbox.pop().to, [end_user.email]) + self.assertEqual(mail.outbox.pop().to, [staff_user.email]) + + staff_user.delete() + + with self.subTest('Test email is sent to users with verified email'): + unverified_email_user = self._create_user( + username='unverified_email', + email='unverified_email@example.com', + ) + # Re-using object to make tests run quickly + verified_email_user = end_user + EmailAddress.objects.filter(user_id=unverified_email_user.id).update( + verified=False + ) + User.objects.filter(is_staff=False).update( + password_updated=user_expiry_date + ) + password_expiration_email.delay() + self.assertEqual(len(mail.outbox), 1) + email = mail.outbox.pop() + self.assertEqual(email.to, [verified_email_user.email]) + self.assertEqual(email.subject, 'Your password is about to expire') + self.assertNotEqual(email.to, [unverified_email_user.email]) + + @patch.object(app_settings, 'USER_PASSWORD_EXPIRATION', 0) + @patch.object(app_settings, 'STAFF_USER_PASSWORD_EXPIRATION', 0) + def test_password_expiration_mail_settings_disabled(self): + """ + Tests that email are not sent when password expiration feature is disabled + """ + self.assertEqual(app_settings.USER_PASSWORD_EXPIRATION, 0) + self.assertEqual(app_settings.STAFF_USER_PASSWORD_EXPIRATION, 0) + self._create_user() + User.objects.update(password_updated=now().date() - timedelta(days=180)) + with patch('openwisp_utils.admin_theme.email.send_email') as mocked_send_email: + password_expiration_email.delay() + mocked_send_email.assert_not_called() diff --git a/requirements.txt b/requirements.txt index 77dec47e..f4a3e13d 100644 --- a/requirements.txt +++ b/requirements.txt @@ -3,6 +3,6 @@ django-extensions~=3.2.2 django-allauth~=0.54.0 django-phonenumber-field~=6.1.0 phonenumbers~=8.13.0 -openwisp-utils[rest] @ https://github.com/openwisp/openwisp-utils/tarball/master +openwisp-utils[rest,celery] @ https://github.com/openwisp/openwisp-utils/tarball/master packaging django-sesame~=2.4.0 diff --git a/tests/openwisp2/__init__.py b/tests/openwisp2/__init__.py index e69de29b..cd042640 100644 --- a/tests/openwisp2/__init__.py +++ b/tests/openwisp2/__init__.py @@ -0,0 +1,3 @@ +from .celery import app as celery_app + +__all__ = ['celery_app'] diff --git a/tests/openwisp2/celery.py b/tests/openwisp2/celery.py new file mode 100644 index 00000000..e3c1425d --- /dev/null +++ b/tests/openwisp2/celery.py @@ -0,0 +1,9 @@ +import os + +from celery import Celery + +os.environ.setdefault('DJANGO_SETTINGS_MODULE', 'openwisp2.settings') + +app = Celery('openwisp2') +app.config_from_object('django.conf:settings', namespace='CELERY') +app.autodiscover_tasks() diff --git a/tests/openwisp2/sample_users/migrations/0001_initial.py b/tests/openwisp2/sample_users/migrations/0001_initial.py index 71b12f6a..f9ff378a 100644 --- a/tests/openwisp2/sample_users/migrations/0001_initial.py +++ b/tests/openwisp2/sample_users/migrations/0001_initial.py @@ -189,6 +189,12 @@ class Migration(migrations.Migration): max_length=8, ), ), + ( + 'password_updated', + models.DateField( + blank=True, null=True, verbose_name="password updated" + ), + ), ], options={ 'verbose_name': 'user', diff --git a/tests/openwisp2/sample_users/tests.py b/tests/openwisp2/sample_users/tests.py index e1c50626..df45daad 100644 --- a/tests/openwisp2/sample_users/tests.py +++ b/tests/openwisp2/sample_users/tests.py @@ -1,9 +1,13 @@ +from openwisp_users.tests.test_accounts import TestAccountView as BaseTestAccountView from openwisp_users.tests.test_admin import ( TestBasicUsersIntegration as BaseTestBasicUsersIntegration, ) from openwisp_users.tests.test_admin import ( TestMultitenantAdmin as BaseTestMultitenantAdmin, ) +from openwisp_users.tests.test_admin import ( + TestUserPasswordExpiration as BaseTestUserPasswordExpiration, +) from openwisp_users.tests.test_admin import TestUsersAdmin as BaseTestUsersAdmin from openwisp_users.tests.test_api.test_api import TestUsersApi as BaseTestUsersApi from openwisp_users.tests.test_api.test_authentication import ( @@ -75,6 +79,10 @@ class TestUsersAdmin(GetEditFormInlineMixin, BaseTestUsersAdmin): _additional_user_fields = additional_fields +class TestUserPasswordExpiration(BaseTestUserPasswordExpiration): + pass + + class TestBasicUsersIntegration(GetEditFormInlineMixin, BaseTestBasicUsersIntegration): app_label = 'sample_users' _additional_user_fields = additional_fields @@ -104,11 +112,16 @@ class TestBackends(BaseTestBackends): pass +class TestAccountView(BaseTestAccountView): + pass + + class TestUsersApi(BaseTestUsersApi): pass del BaseTestUsersAdmin +del BaseTestUserPasswordExpiration del BaseTestBasicUsersIntegration del BaseTestMultitenantAdmin del BaseTestUsers @@ -117,3 +130,4 @@ class TestUsersApi(BaseTestUsersApi): del BaseTestRestFrameworkViews del BaseTestBackends del BaseTestUsersApi +del BaseTestAccountView diff --git a/tests/openwisp2/settings.py b/tests/openwisp2/settings.py index 1461e919..a3ca2ba3 100644 --- a/tests/openwisp2/settings.py +++ b/tests/openwisp2/settings.py @@ -1,6 +1,8 @@ import os import sys +from celery.schedules import crontab + BASE_DIR = os.path.dirname(os.path.abspath(__file__)) DEBUG = True TESTING = sys.argv[1] == 'test' @@ -67,6 +69,11 @@ 'django.contrib.auth.middleware.AuthenticationMiddleware', 'django.contrib.messages.middleware.MessageMiddleware', 'django.middleware.clickjacking.XFrameOptionsMiddleware', + 'openwisp_users.middleware.PasswordExpirationMiddleware', +] + +AUTH_PASSWORD_VALIDATORS = [ + {'NAME': 'openwisp_users.password_validation.PasswordReuseValidator'} ] ROOT_URLCONF = 'openwisp2.urls' @@ -134,6 +141,21 @@ }, } + +if not TESTING: + CELERY_BROKER_URL = 'redis://localhost/6' +else: + CELERY_TASK_ALWAYS_EAGER = True + CELERY_TASK_EAGER_PROPAGATES = True + CELERY_BROKER_URL = 'memory://' + +CELERY_BEAT_SCHEDULE = { + 'password_expiry_email': { + 'task': 'openwisp_users.tasks.password_expiration_email', + 'schedule': crontab(hour=1, minute=0), + }, +} + # during development only EMAIL_BACKEND = 'django.core.mail.backends.console.EmailBackend' LOGIN_REDIRECT_URL = 'admin:index' diff --git a/tests/testapp/tests/tests.py b/tests/testapp/tests/tests.py index b9f2b441..ae539aff 100644 --- a/tests/testapp/tests/tests.py +++ b/tests/testapp/tests/tests.py @@ -67,6 +67,8 @@ def test_resolve_account_URLs(self): self.assertEqual(resolver.view_name, 'account_logout') resolver = resolve('/accounts/password/reset/') self.assertEqual(resolver.view_name, 'account_reset_password') + resolver = resolve('/accounts/password/change/') + self.assertEqual(resolver.view_name, 'account_change_password') def test_account_email_verification_sent(self): r = self.client.get(reverse('account_email_verification_sent')) @@ -85,3 +87,9 @@ def test_account_reset_password(self): self.assertEqual(r.status_code, 200) self.assertNotContains(r, 'Change E-mail') self.assertNotContains(r, 'Sign Up') + + def test_account_change_password(self): + response = self.client.get(reverse('account_change_password')) + self.assertRedirects( + response, '/accounts/login/?next=/accounts/password/change/' + )