From 40eeaa95817753e70d79fdbe4c3f41dfed63c75f Mon Sep 17 00:00:00 2001 From: Benjamin Dauvergne Date: Thu, 1 Apr 2021 22:03:06 +0200 Subject: [PATCH] clean-unused-accounts: run every hour, but limit the number of notifications sent (#52644) --- debian/authentic2-multitenant.cron.d | 2 +- src/authentic2/app_settings.py | 4 +++ .../commands/clean-unused-accounts.py | 6 +++-- tests/test_commands.py | 26 ++++++++++++++++++- 4 files changed, 34 insertions(+), 4 deletions(-) diff --git a/debian/authentic2-multitenant.cron.d b/debian/authentic2-multitenant.cron.d index c1364ca6c..21374b3d4 100644 --- a/debian/authentic2-multitenant.cron.d +++ b/debian/authentic2-multitenant.cron.d @@ -4,5 +4,5 @@ MAILTO=root 0 * * * * authentic-multitenant authentic2-multitenant-manage tenant_command clearsessions --all-tenants 5 * * * * authentic-multitenant authentic2-multitenant-manage tenant_command cleanupauthentic --all-tenants 10 * * * * authentic-multitenant authentic2-multitenant-manage tenant_command sync-ldap-users --all-tenants -0 5 * * * authentic-multitenant authentic2-multitenant-manage tenant_command clean-unused-accounts --all-tenants +15 * * * * authentic-multitenant authentic2-multitenant-manage tenant_command clean-unused-accounts --all-tenants 30 5 * * * authentic-multitenant authentic2-multitenant-manage tenant_command deactivate-orphaned-ldap-users --all-tenants diff --git a/src/authentic2/app_settings.py b/src/authentic2/app_settings.py index 801d3430a..0d5040332 100644 --- a/src/authentic2/app_settings.py +++ b/src/authentic2/app_settings.py @@ -322,6 +322,10 @@ default_settings = dict( default='multipart/alternative', definition='Send email as "multiplart/alternative" or limit to "text/plain" or "text/html".', ), + A2_CLEAN_UNUSED_ACCOUNTS_MAX_MAIL_PER_PERIOD=Setting( + default=250, + definition='Maximum number of mails to send per period', + ), ) app_settings = AppSettings(default_settings) diff --git a/src/authentic2/management/commands/clean-unused-accounts.py b/src/authentic2/management/commands/clean-unused-accounts.py index 324f1d2ea..bed77723e 100644 --- a/src/authentic2/management/commands/clean-unused-accounts.py +++ b/src/authentic2/management/commands/clean-unused-accounts.py @@ -27,6 +27,7 @@ from django.db import transaction from django.db.models import F from django.utils import timezone, translation +from authentic2 import app_settings from authentic2.backends.ldap_backend import LDAPBackend from authentic2.utils import send_templated_mail from django_rbac.utils import get_ou_model @@ -75,6 +76,7 @@ class Command(BaseCommand): logger.exception('clean-unused-accounts failed') def clean_unused_accounts(self): + count = app_settings.A2_CLEAN_UNUSED_ACCOUNTS_MAX_MAIL_PER_PERIOD for ou in get_ou_model().objects.filter(clean_unused_accounts_alert__isnull=False): alert_delay = timedelta(days=ou.clean_unused_accounts_alert) deletion_delay = timedelta(days=ou.clean_unused_accounts_deletion) @@ -89,7 +91,7 @@ class Command(BaseCommand): # send first alert inactive_users_first_alert = inactive_users.filter(last_account_deletion_alert__isnull=True) days_to_deletion = ou.clean_unused_accounts_deletion - ou.clean_unused_accounts_alert - for user in inactive_users_first_alert: + for user in inactive_users_first_alert[:count]: logger.info('%s last login %d days ago, sending alert', user, ou.clean_unused_accounts_alert) self.send_alert(user, days_to_deletion) @@ -98,7 +100,7 @@ class Command(BaseCommand): # ensure respect of alert delay before deletion last_account_deletion_alert__lte=self.now - (deletion_delay - alert_delay), ) - for user in inactive_users_to_delete: + for user in inactive_users_to_delete[:count]: logger.info( '%s last login more than %d days ago, deleting user', user, diff --git a/tests/test_commands.py b/tests/test_commands.py index f1f113f0b..2f03e62f2 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -33,7 +33,7 @@ from authentic2_auth_oidc.models import OIDCAccount, OIDCProvider from django_rbac.models import ADMIN_OP, Operation from django_rbac.utils import get_operation, get_ou_model, get_permission_model, get_role_model -from .utils import call_command, login +from .utils import call_command, login, run_on_commit_hooks User = get_user_model() @@ -365,3 +365,27 @@ def test_check_identifiers_uniqueness(db, capsys, settings): captured = capsys.readouterr() assert 'found 2 user accounts with same username' in captured.out assert 'found 2 user accounts with same email' in captured.out + + +def test_clean_unused_account_max_mails_per_period(settings, db, mailoutbox, freezer): + ou = get_default_ou() + ou.clean_unused_accounts_alert = 1 + ou.clean_unused_accounts_deletion = 2 + ou.save() + settings.A2_CLEAN_UNUSED_ACCOUNTS_MAX_MAIL_PER_PERIOD = 4 + + for i in range(100): + User.objects.create(ou=ou, email='user-%s@example.com' % i, last_login=now()) + + call_command('clean-unused-accounts') + assert len(mailoutbox) == 0 + + freezer.move_to(datetime.timedelta(days=1)) + call_command('clean-unused-accounts') + # 4 alerts + assert len(mailoutbox) == 4 + + freezer.move_to(datetime.timedelta(days=1)) + call_command('clean-unused-accounts') + # 4 new alerts and 4 deletions notifications + assert len(mailoutbox) == 4 + 8