From 235e406aec6f2435a22e9eef437e7d7d8536fabc Mon Sep 17 00:00:00 2001 From: Benjamin Dauvergne Date: Sun, 12 Jul 2020 10:19:31 +0200 Subject: [PATCH] commands: send account deletion notifications to real email (#45054) Add an helper call_command() to test utils to provoke run of on_commit hooks. --- .../commands/clean-unused-accounts.py | 14 ++++--- tests/test_commands.py | 41 ++++++++++--------- tests/utils.py | 19 +++++++++ 3 files changed, 49 insertions(+), 25 deletions(-) diff --git a/src/authentic2/management/commands/clean-unused-accounts.py b/src/authentic2/management/commands/clean-unused-accounts.py index 953afa87..bc0a93f9 100644 --- a/src/authentic2/management/commands/clean-unused-accounts.py +++ b/src/authentic2/management/commands/clean-unused-accounts.py @@ -21,7 +21,7 @@ import logging from datetime import timedelta from django.contrib.auth import get_user_model from django.core.management.base import BaseCommand -from django.db.transaction import atomic +from django.db import transaction from django.db.models import F from django.utils import timezone, translation from django.utils.six.moves.urllib import parse as urlparse @@ -103,7 +103,7 @@ class Command(BaseCommand): 'days_to_deletion': days_to_deletion, 'login_url': urlparse.urljoin(settings.SITE_BASE_URL, settings.LOGIN_URL), } - with atomic(): + with transaction.atomic(): if not self.fake: User.objects.filter(pk=user.pk).update(last_account_deletion_alert=self.now) self.send_mail('authentic2/unused_account_alert', user, ctx) @@ -114,11 +114,15 @@ class Command(BaseCommand): else: logger.debug('sending mail to %s', user.email) if not self.fake: - send_templated_mail(user.email, prefix, ctx) + email = user.email + + def send_mail(): + send_templated_mail(email, prefix, ctx) + transaction.on_commit(send_mail) def delete_user(self, user): ctx = {'user': user} - with atomic(): + with transaction.atomic(): + self.send_mail('authentic2/unused_account_delete', user, ctx) if not self.fake: user.mark_as_deleted(timestamp=self.now) - self.send_mail('authentic2/unused_account_delete', user, ctx) diff --git a/tests/test_commands.py b/tests/test_commands.py index ef80164d..0d606b52 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -20,7 +20,6 @@ import json import pytest -from django.core import management from django.utils import six from django.utils.timezone import now import py @@ -28,7 +27,7 @@ import py from authentic2_auth_oidc.models import OIDCProvider from django_rbac.utils import get_ou_model -from .utils import login +from .utils import login, call_command if six.PY2: FileType = file # noqa: F821 @@ -44,13 +43,14 @@ def test_changepassword(db, simple_user, monkeypatch): return 'pass' monkeypatch.setattr(getpass, 'getpass', _getpass) - management.call_command('changepassword', 'user') + call_command('changepassword', 'user') old_pass = simple_user.password simple_user.refresh_from_db() assert old_pass != simple_user.password def test_clean_unused_account(db, simple_user, mailoutbox, freezer): + email = simple_user.email freezer.move_to('2018-01-01') simple_user.ou.clean_unused_accounts_alert = 2 simple_user.ou.clean_unused_accounts_deletion = 3 @@ -59,23 +59,24 @@ def test_clean_unused_account(db, simple_user, mailoutbox, freezer): simple_user.last_login = now() - datetime.timedelta(days=2) simple_user.save() - management.call_command('clean-unused-accounts') + call_command('clean-unused-accounts') simple_user.refresh_from_db() assert not simple_user.deleted assert len(mailoutbox) == 1 freezer.move_to('2018-01-01 12:00:00') # no new mail, no deletion - management.call_command('clean-unused-accounts') + call_command('clean-unused-accounts') simple_user.refresh_from_db() assert not simple_user.deleted assert len(mailoutbox) == 1 freezer.move_to('2018-01-02') - management.call_command('clean-unused-accounts') + call_command('clean-unused-accounts') simple_user.refresh_from_db() assert simple_user.deleted assert len(mailoutbox) == 2 + assert mailoutbox[-1].to == [email] def test_clean_unused_account_user_logs_in(app, db, simple_user, mailoutbox, freezer): @@ -87,7 +88,7 @@ def test_clean_unused_account_user_logs_in(app, db, simple_user, mailoutbox, fre simple_user.last_login = now() - datetime.timedelta(days=2) simple_user.save() - management.call_command('clean-unused-accounts') + call_command('clean-unused-accounts') assert len(mailoutbox) == 1 login(app, simple_user) @@ -100,7 +101,7 @@ def test_clean_unused_account_user_logs_in(app, db, simple_user, mailoutbox, fre # when new alert delay is reached, user gets alerted again freezer.move_to('2018-01-04') - management.call_command('clean-unused-accounts') + call_command('clean-unused-accounts') simple_user.refresh_from_db() assert not simple_user.deleted assert len(mailoutbox) == 2 @@ -110,7 +111,7 @@ def test_clean_unused_account_disabled_by_default(db, simple_user, mailoutbox): simple_user.last_login = now() - datetime.timedelta(days=2) simple_user.save() - management.call_command('clean-unused-accounts') + call_command('clean-unused-accounts') simple_user.refresh_from_db() assert not simple_user.deleted assert len(mailoutbox) == 0 @@ -125,13 +126,13 @@ def test_clean_unused_account_always_alert(db, simple_user, mailoutbox, freezer) simple_user.save() # even if account last login in past deletion delay, an alert is always sent first - management.call_command('clean-unused-accounts') + call_command('clean-unused-accounts') simple_user.refresh_from_db() assert not simple_user.deleted assert len(mailoutbox) == 1 # and calling again as no effect, since one day must pass before account is deleted - management.call_command('clean-unused-accounts') + call_command('clean-unused-accounts') simple_user.refresh_from_db() assert not simple_user.deleted assert len(mailoutbox) == 1 @@ -147,7 +148,7 @@ def test_clean_unused_account_human_duration_format(simple_user, mailoutbox, del simple_user.save() # alert email - management.call_command('clean-unused-accounts') + call_command('clean-unused-accounts') mail = mailoutbox[0] assert formatted in mail.body assert formatted in mail.subject and not '\n' in mail.subject @@ -155,7 +156,7 @@ def test_clean_unused_account_human_duration_format(simple_user, mailoutbox, del # deletion email simple_user.last_account_deletion_alert = now() - datetime.timedelta(days=2) simple_user.save() - management.call_command('clean-unused-accounts') + call_command('clean-unused-accounts') mail = mailoutbox[1] assert formatted in mail.body @@ -166,13 +167,13 @@ def test_clean_unused_account_login_url(simple_user, mailoutbox): simple_user.ou.save() simple_user.last_login = now() - datetime.timedelta(days=1) simple_user.save() - management.call_command('clean-unused-accounts') + call_command('clean-unused-accounts') mail = mailoutbox[0] assert 'href="http://testserver/login/"' in mail.message().as_string() def test_cleanupauthentic(db): - management.call_command('cleanupauthentic') + call_command('cleanupauthentic') def test_load_ldif(db, monkeypatch, tmpdir): @@ -193,7 +194,7 @@ def test_load_ldif(db, monkeypatch, tmpdir): oidc_cmd = importlib.import_module( 'authentic2.management.commands.load-ldif') monkeypatch.setattr(oidc_cmd, 'DjangoUserLDIFParser', MockPArser) - management.call_command( + call_command( 'load-ldif', ldif.strpath, result='result', extra_attribute={'ldap_attr': 'first_name'}) # test ExtraAttributeAction @@ -210,7 +211,7 @@ def test_load_ldif(db, monkeypatch, tmpdir): pass monkeypatch.setattr(oidc_cmd, 'DjangoUserLDIFParser', MockPArser) - management.call_command( + call_command( 'load-ldif', '--extra-attribute', 'ldap_attr', 'first_name', '--result', 'result', ldif.strpath) @@ -237,7 +238,7 @@ def test_oidc_register_issuer(db, tmpdir, monkeypatch): monkeypatch.setattr(oidc_cmd, 'register_issuer', register_issuer) oidc_conf = py.path.local(__file__).dirpath('openid_configuration.json').strpath - management.call_command( + call_command( 'oidc-register-issuer', '--openid-configuration', oidc_conf, '--issuer', 'issuer', 'somename') @@ -246,7 +247,7 @@ def test_oidc_register_issuer(db, tmpdir, monkeypatch): def test_resetpassword(simple_user): - management.call_command('resetpassword', 'user') + call_command('resetpassword', 'user') old_pass = simple_user.password simple_user.refresh_from_db() assert old_pass != simple_user.password @@ -254,4 +255,4 @@ def test_resetpassword(simple_user): def test_sync_metadata(db): test_file = py.path.local(__file__).dirpath('metadata.xml').strpath - management.call_command('sync-metadata', test_file) + call_command('sync-metadata', test_file) diff --git a/tests/utils.py b/tests/utils.py index 2c0ccc12..f39755ad 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -22,6 +22,7 @@ from contextlib import contextmanager, closing from lxml import etree +from django.core.management import call_command as django_call_command from django.test import TestCase from django.urls import reverse from django.utils.encoding import iri_to_uri, force_text @@ -215,3 +216,21 @@ def request_select2(app, response, term=''): select2_field_id = response.pyquery('select')[0].attrib['data-field_id'] select2_response = app.get(select2_url, params={'field_id': select2_field_id, 'term': term}) return select2_response.json + + +@contextmanager +def run_on_commit_hooks(): + yield + + from django.db import connection + + current_run_on_commit = connection.run_on_commit + connection.run_on_commit = [] + while current_run_on_commit: + sids, func = current_run_on_commit.pop(0) + func() + + +def call_command(*args, **kwargs): + with run_on_commit_hooks(): + return django_call_command(*args, **kwargs) -- 2.26.2