From d4d09397b7eaea46a36296fb3d23bbbbf9de997a Mon Sep 17 00:00:00 2001 From: Graham Ullrich Date: Sat, 10 Sep 2016 13:49:27 -0600 Subject: [PATCH 1/3] Expand tests, fix logic, add docs Add mgmt command for setting user-specific password expiry. Add documentation for password expiry. --- README.rst | 7 +- .../commands/user_password_expiry.py | 31 ++++++++ account/migrations/0004_auto_20160909_1235.py | 20 +++++ account/models.py | 2 +- account/tests/test_password.py | 78 +++++++++++++------ account/utils.py | 12 ++- account/views.py | 10 +-- docs/changelog.rst | 7 ++ docs/usage.rst | 22 ++++++ 9 files changed, 153 insertions(+), 36 deletions(-) create mode 100644 account/management/commands/user_password_expiry.py create mode 100644 account/migrations/0004_auto_20160909_1235.py diff --git a/README.rst b/README.rst index 70825cb8..ad2f4333 100644 --- a/README.rst +++ b/README.rst @@ -47,6 +47,7 @@ Features - Email confirmation - Signup tokens for private betas - Password reset + - Password expiration - Account management (update account settings and change password) - Account deletion @@ -57,7 +58,7 @@ Features Requirements -------------- -* Django 1.8 or 1.9 +* Django 1.8, 1.9, or 1.10 * Python 2.7, 3.3, 3.4 or 3.5 * django-appconf (included in ``install_requires``) * pytz (included in ``install_requires``) @@ -79,13 +80,13 @@ See this blog post http://blog.pinaxproject.com/2016/02/26/recap-february-pinax- In case of any questions we recommend you join our Pinax Slack team (http://slack.pinaxproject.com) and ping us there instead of creating an issue on GitHub. Creating issues on GitHub is of course also valid but we are usually able to help you faster if you ping us in Slack. -We also highly recommend reading our Open Source and Self-Care blog post (http://blog.pinaxproject.com/2016/01/19/open-source-and-self-care/). +We also highly recommend reading our Open Source and Self-Care blog post (http://blog.pinaxproject.com/2016/01/19/open-source-and-self-care/). Code of Conduct ----------------- -In order to foster a kind, inclusive, and harassment-free community, the Pinax Project has a code of conduct, which can be found here http://pinaxproject.com/pinax/code_of_conduct/. +In order to foster a kind, inclusive, and harassment-free community, the Pinax Project has a code of conduct, which can be found here http://pinaxproject.com/pinax/code_of_conduct/. We ask you to treat everyone as a smart human programmer that shares an interest in Python, Django, and Pinax with you. diff --git a/account/management/commands/user_password_expiry.py b/account/management/commands/user_password_expiry.py new file mode 100644 index 00000000..6766284f --- /dev/null +++ b/account/management/commands/user_password_expiry.py @@ -0,0 +1,31 @@ +from django.core.management.base import LabelCommand + +from account.conf import settings +from account.models import PasswordExpiry + + +class Command(LabelCommand): + + help = "Create user-specific password expiration." + label = "username" + + def add_arguments(self, parser): + super(Command, self).add_arguments(parser) + parser.add_argument("-e", "--expire", default=settings.ACCOUNT_PASSWORD_EXPIRY) + + def handle_label(self, username, **options): + try: + user = settings.AUTH_USER_MODEL.objects.get(username=username) + except settings.AUTH_USER_MODEL.DoesNotExist: + return "User \"{}\" not found".format(username) + + expire = options["expire"] + + # Modify existing PasswordExpiry or create new if needed. + if not hasattr(user, "password_expiry"): + PasswordExpiry.objects.create(user=user, expiry=expire) + else: + user.password_expiry.expiry = expire + user.password_expiry.save() + + return "User \"{}\" password expiration now {} seconds".format(username, expire) diff --git a/account/migrations/0004_auto_20160909_1235.py b/account/migrations/0004_auto_20160909_1235.py new file mode 100644 index 00000000..8f32d77e --- /dev/null +++ b/account/migrations/0004_auto_20160909_1235.py @@ -0,0 +1,20 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.10.1 on 2016-09-09 12:35 +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('account', '0003_passwordexpiry_passwordhistory'), + ] + + operations = [ + migrations.AlterField( + model_name='passwordhistory', + name='timestamp', + field=models.DateTimeField(auto_now_add=True), + ), + ] diff --git a/account/models.py b/account/models.py index 047609b7..e0beff3e 100644 --- a/account/models.py +++ b/account/models.py @@ -398,7 +398,7 @@ class PasswordHistory(models.Model): """ user = models.ForeignKey(settings.AUTH_USER_MODEL, related_name="password_history") password = models.CharField(max_length=255) # encrypted password - timestamp = models.DateTimeField(auto_now=True) + timestamp = models.DateTimeField(auto_now_add=True) class PasswordExpiry(models.Model): diff --git a/account/tests/test_password.py b/account/tests/test_password.py index 2cdbca57..24b3fd1d 100644 --- a/account/tests/test_password.py +++ b/account/tests/test_password.py @@ -1,4 +1,5 @@ import datetime +import pytz from django.contrib.auth.hashers import make_password from django.contrib.auth.models import User @@ -12,12 +13,10 @@ PasswordExpiry, PasswordHistory, ) +from ..utils import check_password_expired @override_settings( - AUTHENTICATION_BACKENDS=[ - "account.auth_backends.EmailAuthenticationBackend" - ], ACCOUNT_PASSWORD_USE_HISTORY=True ) class PasswordExpirationTestCase(TestCase): @@ -27,7 +26,7 @@ def setUp(self): self.email = "user1@example.com" self.password = "changeme" self.user = User.objects.create_user( - self.email, + self.username, email=self.email, password=self.password, ) @@ -44,42 +43,73 @@ def setUp(self): def test_signup(self): """ - Ensure new user has one PasswordExpiry and one PasswordHistory. + Ensure new user has one PasswordHistory and no PasswordExpiry. """ - # PasswordExpiry.expiry should be same as ACCOUNT_PASSWORD_EXPIRY - pass + email = "foobar@example.com" + post_data = { + "username": "foo", + "password": "bar", + "password_confirm": "bar", + "email": email, + } + response = self.client.post(reverse("account_signup"), post_data) + self.assertEqual(response.status_code, 302) + user = User.objects.get(email=email) + self.assertFalse(hasattr(user, "password_expiry")) + self.assertTrue(hasattr(user, "password_history")) + + # verify password is not expired + self.assertFalse(check_password_expired(user)) + + def test_login_not_expired(self): + """ + Ensure user can log in successfully without redirect. + """ + # get login + self.client.login(username=self.username, password=self.password) + + response = self.client.get(reverse("account_login")) + self.assertRedirects(response, "/", fetch_redirect_response=False) def test_login_expired(self): """ Ensure user is redirected to change password if pw is expired. """ # set PasswordHistory timestamp in past so pw is expired. - self.history.timestamp = datetime.datetime.now() - datetime.timedelta(minutes=2) + self.history.timestamp = datetime.datetime.now(tz=pytz.UTC) - datetime.timedelta(days=1, seconds=self.expiry.expiry) self.history.save() # get login - self.client.login(username=self.email, password=self.password) + self.client.login(username=self.username, password=self.password) response = self.client.get(reverse("account_login")) self.assertRedirects(response, reverse("account_password")) - # post login + def test_pw_expiration_reset(self): + """ + Ensure changing password results in new PasswordHistory. + """ + qs = PasswordHistory.objects.all() + self.assertEquals(qs.count(), 1) + + # get login + self.client.login(username=self.username, password=self.password) + + # post new password to reset PasswordHistory + new_password = "lynyrdskynyrd" post_data = { - "username": self.username, - "email": self.email, - "password": self.password, + "password_current": self.password, + "password_new": new_password, + "password_new_confirm": new_password, } - response = self.client.post( - reverse("account_login"), + self.client.post( + reverse("account_password"), post_data ) - self.assertEquals(response.status_code, 200) - def test_login_not_expired(self): - """ - Ensure user logs in successfully without redirect. - """ - # set PasswordHistory timestamp so pw is not expired. - # attempt login - # assert success and user logged in - pass + qs = PasswordHistory.objects.all() + self.assertEquals(qs.count(), 2) + + latest = PasswordHistory.objects.latest("timestamp") + self.assertTrue(latest != self.history) + self.assertTrue(latest.timestamp > self.history.timestamp) diff --git a/account/utils.py b/account/utils.py index 941ff87e..783f6bcb 100644 --- a/account/utils.py +++ b/account/utils.py @@ -125,13 +125,19 @@ def check_password_expired(user): # use global value expiry = settings.ACCOUNT_PASSWORD_EXPIRY + if expiry == 0: # zero indicates no expiration + return False + try: # get latest password info latest = user.password_history.latest("timestamp") except PasswordHistory.DoesNotExist: return False - if datetime.datetime.now(tz=pytz.UTC) > (latest.timestamp + datetime.timedelta(seconds=expiry)): - return False + now = datetime.datetime.now(tz=pytz.UTC) + expiration = latest.timestamp + datetime.timedelta(seconds=expiry) - return True + if expiration < now: + return True + else: + return False diff --git a/account/views.py b/account/views.py index f3f1bafb..9d3e7022 100644 --- a/account/views.py +++ b/account/views.py @@ -97,11 +97,11 @@ def send_password_email(self, user): } hookset.send_password_change_email([user.email], ctx) - def create_password_history(self, form): + def create_password_history(self, form, user): if settings.ACCOUNT_PASSWORD_USE_HISTORY: password = form.cleaned_data[self.form_password_field] PasswordHistory.objects.create( - user=self.request.user, + user=user, password=make_password(password) ) @@ -209,7 +209,7 @@ def form_valid(self, form): self.created_user.is_active = False self.created_user.save() self.create_account(form) - self.create_password_history(form) + self.create_password_history(form, self.created_user) self.after_signup(form) if settings.ACCOUNT_EMAIL_CONFIRMATION_EMAIL and not email_address.verified: self.send_email_confirmation(email_address) @@ -535,7 +535,7 @@ def post(self, *args, **kwargs): def form_valid(self, form): self.change_password(form) - self.create_password_history(form) + self.create_password_history(form, self.request.user) self.after_change_password() return redirect(self.get_success_url()) @@ -634,7 +634,7 @@ def get_context_data(self, **kwargs): def form_valid(self, form): self.change_password(form) - self.create_password_history(form) + self.create_password_history(form, self.request.user) self.after_change_password() return redirect(self.get_success_url()) diff --git a/docs/changelog.rst b/docs/changelog.rst index ea0cad9a..1ed54927 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -3,6 +3,13 @@ CHANGELOG ========= +2.0 +--- + +* add password expiration +* add Django v1.10 to test compatibility matrix +* remove Python 3.2 from text compatibility matrix + 1.0 --- diff --git a/docs/usage.rst b/docs/usage.rst index e1b19a1a..1e52d17d 100644 --- a/docs/usage.rst +++ b/docs/usage.rst @@ -254,3 +254,25 @@ file called lib/tests.py:: And in your settings:: TEST_RUNNER = "lib.tests.MyTestDiscoverRunner" + + +Enabling password expiration +============================ + +Password expiration is disabled by default. In order to enable password expiration +you must add two entries to your settings file:: + + PASSWORD_EXPIRY = 60*60*24*5 # seconds until pw expires, this example shows five days + PASSWORD_USE_HISTORY = True + +PASSWORD_EXPIRY indicates the duration a password will stay valid. After that period +the password must be reset in order to log in. If PASSWORD_EXPIRY is zero (0) +then passwords never expire. + +If PASSWORD_USE_HISTORY is False, no history will be generated and password +expiration WILL NOT be checked. + +If PASSWORD_USE_HISTORY is True, a password history entry is created each time +the user changes their password. This entry links the user with their most recent +(encrypted) password and a timestamp. Unless deleted manually, PasswordHistory items +are saved forever, allowing password history checking for new passwords. From 23edd12e8d690084d780a13858485eea1c7ad2a5 Mon Sep 17 00:00:00 2001 From: Graham Ullrich Date: Tue, 13 Sep 2016 09:02:35 -0600 Subject: [PATCH 2/3] Add mgmt commands and tests Revise model migration. Add user_password_history command. Add management command tests. Improve password tests. Add Django admin functionality for new PasswordExpiry and PasswordHistory models. --- account/admin.py | 21 +- .../commands/user_password_expiry.py | 18 +- .../commands/user_password_history.py | 45 ++++ .../0003_passwordexpiry_passwordhistory.py | 5 +- account/migrations/0004_auto_20160909_1235.py | 20 -- account/models.py | 4 +- account/tests/test_commands.py | 227 ++++++++++++++++++ account/tests/test_password.py | 108 ++++++++- 8 files changed, 407 insertions(+), 41 deletions(-) create mode 100644 account/management/commands/user_password_history.py delete mode 100644 account/migrations/0004_auto_20160909_1235.py create mode 100644 account/tests/test_commands.py diff --git a/account/admin.py b/account/admin.py index 7e0e3a28..3f01f5ce 100644 --- a/account/admin.py +++ b/account/admin.py @@ -2,7 +2,14 @@ from django.contrib import admin -from account.models import Account, SignupCode, AccountDeletion, EmailAddress +from account.models import ( + Account, + AccountDeletion, + EmailAddress, + PasswordExpiry, + PasswordHistory, + SignupCode, +) class SignupCodeAdmin(admin.ModelAdmin): @@ -29,7 +36,19 @@ class EmailAddressAdmin(AccountAdmin): search_fields = ["email", "user__username"] +class PasswordExpiryAdmin(admin.ModelAdmin): + + raw_id_fields = ["user"] + + +class PasswordHistoryAdmin(admin.ModelAdmin): + + raw_id_fields = ["user"] + + admin.site.register(Account, AccountAdmin) admin.site.register(SignupCode, SignupCodeAdmin) admin.site.register(AccountDeletion, AccountDeletionAdmin) admin.site.register(EmailAddress, EmailAddressAdmin) +admin.site.register(PasswordExpiry, PasswordExpiryAdmin) +admin.site.register(PasswordHistory, PasswordHistoryAdmin) diff --git a/account/management/commands/user_password_expiry.py b/account/management/commands/user_password_expiry.py index 6766284f..b13424c8 100644 --- a/account/management/commands/user_password_expiry.py +++ b/account/management/commands/user_password_expiry.py @@ -1,3 +1,4 @@ +from django.contrib.auth import get_user_model from django.core.management.base import LabelCommand from account.conf import settings @@ -6,17 +7,24 @@ class Command(LabelCommand): - help = "Create user-specific password expiration." + help = "Create user-specific password expiration period." label = "username" def add_arguments(self, parser): super(Command, self).add_arguments(parser) - parser.add_argument("-e", "--expire", default=settings.ACCOUNT_PASSWORD_EXPIRY) + parser.add_argument( + "-e", "--expire", + type=int, + nargs="?", + default=settings.ACCOUNT_PASSWORD_EXPIRY, + help="number of seconds until password expires" + ) def handle_label(self, username, **options): + User = get_user_model() try: - user = settings.AUTH_USER_MODEL.objects.get(username=username) - except settings.AUTH_USER_MODEL.DoesNotExist: + user = User.objects.get(username=username) + except User.DoesNotExist: return "User \"{}\" not found".format(username) expire = options["expire"] @@ -28,4 +36,4 @@ def handle_label(self, username, **options): user.password_expiry.expiry = expire user.password_expiry.save() - return "User \"{}\" password expiration now {} seconds".format(username, expire) + return "User \"{}\" password expiration set to {} seconds".format(username, expire) diff --git a/account/management/commands/user_password_history.py b/account/management/commands/user_password_history.py new file mode 100644 index 00000000..4349416f --- /dev/null +++ b/account/management/commands/user_password_history.py @@ -0,0 +1,45 @@ +import datetime +import pytz + +from django.contrib.auth import get_user_model +from django.core.management.base import BaseCommand + +from account.models import PasswordHistory + + +class Command(BaseCommand): + + help = "Create password history for all users without existing history." + + def add_arguments(self, parser): + parser.add_argument( + "-d", "--days", + type=int, + nargs="?", + default=10, + help="age of current password (in days)" + ) + parser.add_argument( + "-f", "--force", + action="store_true", + help="create new password history for all users, regardless of existing history" + ) + + def handle(self, *args, **options): + User = get_user_model() + users = User.objects.all() + if not options["force"]: + users = users.filter(password_history=None) + + if not users: + return "No users found without password history" + + days = options["days"] + timestamp = datetime.datetime.now(tz=pytz.UTC) - datetime.timedelta(days=days) + + # Create new PasswordHistory on `timestamp` + PasswordHistory.objects.bulk_create( + [PasswordHistory(user=user, timestamp=timestamp) for user in users] + ) + + return "Password history set to {} for {} users".format(timestamp, len(users)) diff --git a/account/migrations/0003_passwordexpiry_passwordhistory.py b/account/migrations/0003_passwordexpiry_passwordhistory.py index 359017e9..2c71d0c3 100644 --- a/account/migrations/0003_passwordexpiry_passwordhistory.py +++ b/account/migrations/0003_passwordexpiry_passwordhistory.py @@ -1,10 +1,11 @@ # -*- coding: utf-8 -*- -# Generated by Django 1.9.7 on 2016-09-01 17:50 +# Generated by Django 1.10.1 on 2016-09-13 08:55 from __future__ import unicode_literals from django.conf import settings from django.db import migrations, models import django.db.models.deletion +import django.utils.timezone class Migration(migrations.Migration): @@ -28,7 +29,7 @@ class Migration(migrations.Migration): fields=[ ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), ('password', models.CharField(max_length=255)), - ('timestamp', models.DateTimeField(auto_now=True)), + ('timestamp', models.DateTimeField(default=django.utils.timezone.now)), ('user', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='password_history', to=settings.AUTH_USER_MODEL)), ], ), diff --git a/account/migrations/0004_auto_20160909_1235.py b/account/migrations/0004_auto_20160909_1235.py deleted file mode 100644 index 8f32d77e..00000000 --- a/account/migrations/0004_auto_20160909_1235.py +++ /dev/null @@ -1,20 +0,0 @@ -# -*- coding: utf-8 -*- -# Generated by Django 1.10.1 on 2016-09-09 12:35 -from __future__ import unicode_literals - -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ('account', '0003_passwordexpiry_passwordhistory'), - ] - - operations = [ - migrations.AlterField( - model_name='passwordhistory', - name='timestamp', - field=models.DateTimeField(auto_now_add=True), - ), - ] diff --git a/account/models.py b/account/models.py index e0beff3e..345f0d47 100644 --- a/account/models.py +++ b/account/models.py @@ -104,7 +104,7 @@ def user_post_save(sender, **kwargs): """ # Disable post_save during manage.py loaddata - if kwargs.get('raw', False): + if kwargs.get("raw", False): return False user, created = kwargs["instance"], kwargs["created"] @@ -398,7 +398,7 @@ class PasswordHistory(models.Model): """ user = models.ForeignKey(settings.AUTH_USER_MODEL, related_name="password_history") password = models.CharField(max_length=255) # encrypted password - timestamp = models.DateTimeField(auto_now_add=True) + timestamp = models.DateTimeField(default=timezone.now) # password creation time class PasswordExpiry(models.Model): diff --git a/account/tests/test_commands.py b/account/tests/test_commands.py new file mode 100644 index 00000000..594c9e71 --- /dev/null +++ b/account/tests/test_commands.py @@ -0,0 +1,227 @@ +from django.contrib.auth import get_user_model +from django.core.management import call_command +from django.test import ( + override_settings, + TestCase, +) +from django.utils.six import StringIO + +from ..conf import settings +from ..models import ( + PasswordExpiry, + PasswordHistory, +) + + +@override_settings( + ACCOUNT_PASSWORD_EXPIRY=500 +) +class UserPasswordExpiryTests(TestCase): + + def setUp(self): + self.UserModel = get_user_model() + self.user = self.UserModel.objects.create_user(username="patrick") + + def test_set_explicit_password_expiry(self): + """ + Ensure specific password expiry is set. + """ + self.assertFalse(hasattr(self.user, "password_expiry")) + expiration_period = 60 + out = StringIO() + call_command( + "user_password_expiry", + "patrick", + "--expire={}".format(expiration_period), + stdout=out + ) + + user = self.UserModel.objects.get(username="patrick") + user_expiry = user.password_expiry + self.assertEqual(user_expiry.expiry, expiration_period) + self.assertIn("User \"{}\" password expiration set to {} seconds".format(self.user.username, expiration_period), out.getvalue()) + + def test_set_default_password_expiry(self): + """ + Ensure default password expiry (from settings) is set. + """ + self.assertFalse(hasattr(self.user, "password_expiry")) + out = StringIO() + call_command( + "user_password_expiry", + "patrick", + stdout=out + ) + + user = self.UserModel.objects.get(username="patrick") + user_expiry = user.password_expiry + default_expiration = settings.ACCOUNT_PASSWORD_EXPIRY + self.assertEqual(user_expiry.expiry, default_expiration) + self.assertIn("User \"{}\" password expiration set to {} seconds".format(self.user.username, default_expiration), out.getvalue()) + + def test_reset_existing_password_expiry(self): + """ + Ensure existing password expiry is reset. + """ + previous_expiry = 123 + existing_expiry = PasswordExpiry.objects.create(user=self.user, expiry=previous_expiry) + out = StringIO() + call_command( + "user_password_expiry", + "patrick", + stdout=out + ) + + user = self.UserModel.objects.get(username="patrick") + user_expiry = user.password_expiry + self.assertEqual(user_expiry, existing_expiry) + default_expiration = settings.ACCOUNT_PASSWORD_EXPIRY + self.assertEqual(user_expiry.expiry, default_expiration) + self.assertNotEqual(user_expiry.expiry, previous_expiry) + + def test_bad_username(self): + """ + Ensure proper operation when username is not found. + """ + bad_username = "asldkfj" + out = StringIO() + call_command( + "user_password_expiry", + bad_username, + stdout=out + ) + self.assertIn("User \"{}\" not found".format(bad_username), out.getvalue()) + + +class UserPasswordHistoryTests(TestCase): + + def setUp(self): + self.UserModel = get_user_model() + self.user = self.UserModel.objects.create_user(username="patrick") + + def test_set_history(self): + """ + Ensure password history is created. + """ + self.assertFalse(self.user.password_history.all()) + password_age = 5 # days + out = StringIO() + call_command( + "user_password_history", + "--days={}".format(password_age), + stdout=out + ) + + user = self.UserModel.objects.get(username="patrick") + password_history = user.password_history.all() + self.assertEqual(password_history.count(), 1) + self.assertIn("Password history set to ", out.getvalue()) + self.assertIn("for {} users".format(1), out.getvalue()) + + def test_set_history_exists(self): + """ + Ensure password history is NOT created. + """ + PasswordHistory.objects.create(user=self.user) + password_age = 5 # days + out = StringIO() + call_command( + "user_password_history", + "--days={}".format(password_age), + stdout=out + ) + + user = self.UserModel.objects.get(username="patrick") + password_history = user.password_history.all() + self.assertEqual(password_history.count(), 1) + self.assertIn("No users found without password history", out.getvalue()) + + def test_set_history_one_exists(self): + """ + Ensure password history is created for users without existing history. + """ + another_user = self.UserModel.objects.create_user(username="james") + PasswordHistory.objects.create(user=another_user) + + password_age = 5 # days + out = StringIO() + call_command( + "user_password_history", + "--days={}".format(password_age), + stdout=out + ) + + user = self.UserModel.objects.get(username="patrick") + password_history = user.password_history.all() + self.assertEqual(password_history.count(), 1) + + # verify user with existing history did not get another entry + user = self.UserModel.objects.get(username="james") + password_history = user.password_history.all() + self.assertEqual(password_history.count(), 1) + + self.assertIn("Password history set to ", out.getvalue()) + self.assertIn("for {} users".format(1), out.getvalue()) + + def test_set_history_force(self): + """ + Ensure specific password history is created for all users. + """ + another_user = self.UserModel.objects.create_user(username="james") + PasswordHistory.objects.create(user=another_user) + + password_age = 5 # days + out = StringIO() + call_command( + "user_password_history", + "--days={}".format(password_age), + "--force", + stdout=out + ) + + user = self.UserModel.objects.get(username="patrick") + password_history = user.password_history.all() + self.assertEqual(password_history.count(), 1) + + # verify user with existing history DID get another entry + user = self.UserModel.objects.get(username="james") + password_history = user.password_history.all() + self.assertEqual(password_history.count(), 2) + + self.assertIn("Password history set to ", out.getvalue()) + self.assertIn("for {} users".format(2), out.getvalue()) + + def test_set_history_multiple(self): + """ + Ensure password history is created for all users without existing history. + """ + self.UserModel.objects.create_user(username="second") + self.UserModel.objects.create_user(username="third") + + password_age = 5 # days + out = StringIO() + call_command( + "user_password_history", + "--days={}".format(password_age), + stdout=out + ) + + user = self.UserModel.objects.get(username="patrick") + password_history = user.password_history.all() + self.assertEqual(password_history.count(), 1) + first_timestamp = password_history[0].timestamp + + user = self.UserModel.objects.get(username="second") + password_history = user.password_history.all() + self.assertEqual(password_history.count(), 1) + second_timestamp = password_history[0].timestamp + self.assertEqual(first_timestamp, second_timestamp) + + user = self.UserModel.objects.get(username="third") + password_history = user.password_history.all() + self.assertEqual(password_history.count(), 1) + third_timestamp = password_history[0].timestamp + self.assertEqual(first_timestamp, third_timestamp) + + self.assertIn("Password history set to ", out.getvalue()) + self.assertIn("for {} users".format(3), out.getvalue()) diff --git a/account/tests/test_password.py b/account/tests/test_password.py index 24b3fd1d..5cef7ba8 100644 --- a/account/tests/test_password.py +++ b/account/tests/test_password.py @@ -1,7 +1,10 @@ import datetime import pytz -from django.contrib.auth.hashers import make_password +from django.contrib.auth.hashers import ( + check_password, + make_password, +) from django.contrib.auth.models import User from django.core.urlresolvers import reverse from django.test import ( @@ -46,28 +49,32 @@ def test_signup(self): Ensure new user has one PasswordHistory and no PasswordExpiry. """ email = "foobar@example.com" + password = "bar" post_data = { "username": "foo", - "password": "bar", - "password_confirm": "bar", + "password": password, + "password_confirm": password, "email": email, } response = self.client.post(reverse("account_signup"), post_data) self.assertEqual(response.status_code, 302) user = User.objects.get(email=email) self.assertFalse(hasattr(user, "password_expiry")) - self.assertTrue(hasattr(user, "password_history")) + latest_history = user.password_history.latest("timestamp") + self.assertTrue(latest_history) # verify password is not expired self.assertFalse(check_password_expired(user)) + # verify raw password matches encrypted password in history + self.assertTrue(check_password(password, latest_history.password)) def test_login_not_expired(self): """ Ensure user can log in successfully without redirect. """ - # get login self.client.login(username=self.username, password=self.password) + # get login response = self.client.get(reverse("account_login")) self.assertRedirects(response, "/", fetch_redirect_response=False) @@ -79,9 +86,9 @@ def test_login_expired(self): self.history.timestamp = datetime.datetime.now(tz=pytz.UTC) - datetime.timedelta(days=1, seconds=self.expiry.expiry) self.history.save() - # get login self.client.login(username=self.username, password=self.password) + # get login response = self.client.get(reverse("account_login")) self.assertRedirects(response, reverse("account_password")) @@ -89,8 +96,7 @@ def test_pw_expiration_reset(self): """ Ensure changing password results in new PasswordHistory. """ - qs = PasswordHistory.objects.all() - self.assertEquals(qs.count(), 1) + history_count = self.user.password_history.count() # get login self.client.login(username=self.username, password=self.password) @@ -106,10 +112,90 @@ def test_pw_expiration_reset(self): reverse("account_password"), post_data ) - - qs = PasswordHistory.objects.all() - self.assertEquals(qs.count(), 2) + # Should see one more history entry for this user + self.assertEquals(self.user.password_history.count(), history_count + 1) latest = PasswordHistory.objects.latest("timestamp") self.assertTrue(latest != self.history) self.assertTrue(latest.timestamp > self.history.timestamp) + + +class ExistingUserNoHistoryTestCase(TestCase): + """ + Tests where user has no PasswordHistory. + """ + + def setUp(self): + self.username = "user1" + self.email = "user1@example.com" + self.password = "changeme" + self.user = User.objects.create_user( + self.username, + email=self.email, + password=self.password, + ) + + @override_settings( + ACCOUNT_PASSWORD_USE_HISTORY=True + ) + def test_login_not_expired(self): + """ + Ensure user without history can log in successfully without redirect. + """ + self.client.login(username=self.username, password=self.password) + + # get login + response = self.client.get(reverse("account_login")) + self.assertRedirects(response, "/", fetch_redirect_response=False) + + @override_settings( + ACCOUNT_PASSWORD_USE_HISTORY=True + ) + def test_pw_expiration_reset(self): + """ + Ensure changing password results in new PasswordHistory, + even when no PasswordHistory exists. + """ + history_count = self.user.password_history.count() + + # get login + self.client.login(username=self.username, password=self.password) + + # post new password to reset PasswordHistory + new_password = "lynyrdskynyrd" + post_data = { + "password_current": self.password, + "password_new": new_password, + "password_new_confirm": new_password, + } + self.client.post( + reverse("account_password"), + post_data + ) + # Should see one more history entry for this user + self.assertEquals(self.user.password_history.count(), history_count + 1) + + @override_settings( + ACCOUNT_PASSWORD_USE_HISTORY=False + ) + def test_password_reset(self): + """ + Ensure changing password results in NO new PasswordHistory + when ACCOUNT_PASSWORD_USE_HISTORY == False. + """ + # get login + self.client.login(username=self.username, password=self.password) + + # post new password to reset PasswordHistory + new_password = "lynyrdskynyrd" + post_data = { + "password_current": self.password, + "password_new": new_password, + "password_new_confirm": new_password, + } + self.client.post( + reverse("account_password"), + post_data + ) + # history count should be zero + self.assertEquals(self.user.password_history.count(), 0) From c9c73d881ed0a1cb478ad68e789875e530d8debc Mon Sep 17 00:00:00 2001 From: Graham Ullrich Date: Tue, 13 Sep 2016 12:33:13 -0600 Subject: [PATCH 3/3] Add ExpiredPasswordMiddleware Use middleware to detect password expiration and redirect to password change page if expired. Added to CHANGELOG.md. Removed outdated docs/changelog.rst. Added documentation for password middleware. --- CHANGELOG.md | 5 +- account/middleware.py | 16 ++++ account/tests/templates/account/settings.html | 1 + account/tests/test_password.py | 90 +++++++++++-------- account/views.py | 7 +- docs/changelog.rst | 17 ---- docs/index.rst | 1 - docs/usage.rst | 16 +++- 8 files changed, 87 insertions(+), 66 deletions(-) create mode 100644 account/tests/templates/account/settings.html delete mode 100644 docs/changelog.rst diff --git a/CHANGELOG.md b/CHANGELOG.md index 5879c937..b4170750 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,9 +7,12 @@ version with these. Your code will need to be updated to continue working. * BI: account deletion callbacks moved to hooksets * BI: dropped Django 1.7 support - * BI: removed deprecated `ACCOUNT_USE_AUTH_AUTHENTICATE` setting with behavior matching its `True` value + * BI: removed deprecated `ACCOUNT_USE_AUTH_AUTHENTICATE` setting with behavior matching its `True` value + * BI: remove Python 3.2 from test compatibility matrix + * add Django v1.10 to test compatibility matrix * added Turkish translations * fixed migration with language codes to dynamically set + * add password expiration ## 1.3.0 diff --git a/account/middleware.py b/account/middleware.py index 916eaf19..5118af63 100644 --- a/account/middleware.py +++ b/account/middleware.py @@ -1,10 +1,13 @@ from __future__ import unicode_literals +from django.core.urlresolvers import resolve +from django.shortcuts import redirect from django.utils import translation, timezone from django.utils.cache import patch_vary_headers from account.conf import settings from account.models import Account +from account.utils import check_password_expired class LocaleMiddleware(object): @@ -51,3 +54,16 @@ def process_request(self, request): if account: tz = settings.TIME_ZONE if not account.timezone else account.timezone timezone.activate(tz) + + +class ExpiredPasswordMiddleware(object): + + def process_request(self, request): + if request.user.is_authenticated() and not request.user.is_staff: + url_name = resolve(request.path).url_name + # All users must be allowed to access "change password" url. + if url_name not in settings.ACCOUNT_PASSWORD_CHANGE_REDIRECT_URL: + if check_password_expired(request.user): + return redirect( + settings.ACCOUNT_PASSWORD_CHANGE_REDIRECT_URL + ) diff --git a/account/tests/templates/account/settings.html b/account/tests/templates/account/settings.html new file mode 100644 index 00000000..9ed72bb4 --- /dev/null +++ b/account/tests/templates/account/settings.html @@ -0,0 +1 @@ +# empty for now diff --git a/account/tests/test_password.py b/account/tests/test_password.py index 5cef7ba8..42b19f03 100644 --- a/account/tests/test_password.py +++ b/account/tests/test_password.py @@ -9,6 +9,7 @@ from django.core.urlresolvers import reverse from django.test import ( TestCase, + modify_settings, override_settings, ) @@ -22,6 +23,11 @@ @override_settings( ACCOUNT_PASSWORD_USE_HISTORY=True ) +@modify_settings( + MIDDLEWARE_CLASSES={ + 'append': 'account.middleware.ExpiredPasswordMiddleware' + } +) class PasswordExpirationTestCase(TestCase): def setUp(self): @@ -68,31 +74,33 @@ def test_signup(self): # verify raw password matches encrypted password in history self.assertTrue(check_password(password, latest_history.password)) - def test_login_not_expired(self): + def test_get_not_expired(self): """ - Ensure user can log in successfully without redirect. + Ensure authenticated user can retrieve account settings page + without "password change" redirect. """ self.client.login(username=self.username, password=self.password) - # get login - response = self.client.get(reverse("account_login")) - self.assertRedirects(response, "/", fetch_redirect_response=False) + # get account settings page + response = self.client.get(reverse("account_settings")) + self.assertEquals(response.status_code, 200) - def test_login_expired(self): + def test_get_expired(self): """ - Ensure user is redirected to change password if pw is expired. + Ensure authenticated user is redirected to change password + when retrieving account settings page if password is expired. """ - # set PasswordHistory timestamp in past so pw is expired. + # set PasswordHistory timestamp in past so password is expired. self.history.timestamp = datetime.datetime.now(tz=pytz.UTC) - datetime.timedelta(days=1, seconds=self.expiry.expiry) self.history.save() self.client.login(username=self.username, password=self.password) - # get login - response = self.client.get(reverse("account_login")) + # get account settings page + response = self.client.get(reverse("account_settings")) self.assertRedirects(response, reverse("account_password")) - def test_pw_expiration_reset(self): + def test_password_expiration_reset(self): """ Ensure changing password results in new PasswordHistory. """ @@ -120,6 +128,11 @@ def test_pw_expiration_reset(self): self.assertTrue(latest.timestamp > self.history.timestamp) +@modify_settings( + MIDDLEWARE_CLASSES={ + 'append': 'account.middleware.ExpiredPasswordMiddleware' + } +) class ExistingUserNoHistoryTestCase(TestCase): """ Tests where user has no PasswordHistory. @@ -135,23 +148,21 @@ def setUp(self): password=self.password, ) - @override_settings( - ACCOUNT_PASSWORD_USE_HISTORY=True - ) - def test_login_not_expired(self): + def test_get_no_history(self): """ - Ensure user without history can log in successfully without redirect. + Ensure authenticated user without password history can retrieve + account settings page without "password change" redirect. """ self.client.login(username=self.username, password=self.password) - # get login - response = self.client.get(reverse("account_login")) - self.assertRedirects(response, "/", fetch_redirect_response=False) + with override_settings( + ACCOUNT_PASSWORD_USE_HISTORY=True + ): + # get account settings page + response = self.client.get(reverse("account_settings")) + self.assertEquals(response.status_code, 200) - @override_settings( - ACCOUNT_PASSWORD_USE_HISTORY=True - ) - def test_pw_expiration_reset(self): + def test_password_expiration_reset(self): """ Ensure changing password results in new PasswordHistory, even when no PasswordHistory exists. @@ -168,16 +179,16 @@ def test_pw_expiration_reset(self): "password_new": new_password, "password_new_confirm": new_password, } - self.client.post( - reverse("account_password"), - post_data - ) - # Should see one more history entry for this user - self.assertEquals(self.user.password_history.count(), history_count + 1) + with override_settings( + ACCOUNT_PASSWORD_USE_HISTORY=True + ): + self.client.post( + reverse("account_password"), + post_data + ) + # Should see one more history entry for this user + self.assertEquals(self.user.password_history.count(), history_count + 1) - @override_settings( - ACCOUNT_PASSWORD_USE_HISTORY=False - ) def test_password_reset(self): """ Ensure changing password results in NO new PasswordHistory @@ -193,9 +204,12 @@ def test_password_reset(self): "password_new": new_password, "password_new_confirm": new_password, } - self.client.post( - reverse("account_password"), - post_data - ) - # history count should be zero - self.assertEquals(self.user.password_history.count(), 0) + with override_settings( + ACCOUNT_PASSWORD_USE_HISTORY=False + ): + self.client.post( + reverse("account_password"), + post_data + ) + # history count should be zero + self.assertEquals(self.user.password_history.count(), 0) diff --git a/account/views.py b/account/views.py index 9d3e7022..e1faab21 100644 --- a/account/views.py +++ b/account/views.py @@ -22,7 +22,7 @@ from account.hooks import hookset from account.mixins import LoginRequiredMixin from account.models import SignupCode, EmailAddress, EmailConfirmation, Account, AccountDeletion, PasswordHistory -from account.utils import check_password_expired, default_redirect, get_form_data +from account.utils import default_redirect, get_form_data class PasswordMixin(object): @@ -344,11 +344,6 @@ class LoginView(FormView): def get(self, *args, **kwargs): if self.request.user.is_authenticated(): - - # Check for password expiration, redirect if needed. - if check_password_expired(self.request.user): - return redirect("account_password") - return redirect(self.get_success_url()) return super(LoginView, self).get(*args, **kwargs) diff --git a/docs/changelog.rst b/docs/changelog.rst deleted file mode 100644 index 1ed54927..00000000 --- a/docs/changelog.rst +++ /dev/null @@ -1,17 +0,0 @@ -.. _changelog: - -CHANGELOG -========= - -2.0 ---- - -* add password expiration -* add Django v1.10 to test compatibility matrix -* remove Python 3.2 from text compatibility matrix - -1.0 ---- - -* initial release -* if migrating from Pinax; see :ref:`migration` diff --git a/docs/index.rst b/docs/index.rst index 1cf5cf2f..0a97be34 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -21,6 +21,5 @@ Contents settings templates signals - changelog migration faq diff --git a/docs/usage.rst b/docs/usage.rst index 1e52d17d..9786a165 100644 --- a/docs/usage.rst +++ b/docs/usage.rst @@ -260,10 +260,17 @@ Enabling password expiration ============================ Password expiration is disabled by default. In order to enable password expiration -you must add two entries to your settings file:: +you must add entries to your settings file:: - PASSWORD_EXPIRY = 60*60*24*5 # seconds until pw expires, this example shows five days - PASSWORD_USE_HISTORY = True + ACCOUNT_PASSWORD_EXPIRY = 60*60*24*5 # seconds until pw expires, this example shows five days + ACCOUNT_PASSWORD_USE_HISTORY = True + +and include `ExpiredPasswordMiddleware` with your middleware settings:: + + MIDDLEWARE_CLASSES = { + ... + "account.middleware.ExpiredPasswordMiddleware", + } PASSWORD_EXPIRY indicates the duration a password will stay valid. After that period the password must be reset in order to log in. If PASSWORD_EXPIRY is zero (0) @@ -276,3 +283,6 @@ If PASSWORD_USE_HISTORY is True, a password history entry is created each time the user changes their password. This entry links the user with their most recent (encrypted) password and a timestamp. Unless deleted manually, PasswordHistory items are saved forever, allowing password history checking for new passwords. + +For an authenticated user, `ExpiredPasswordMiddleware` prevents retrieving or posting +to any page (except the password change page!) when the user password is expired.