From f2e1cfca63d9f0697e739c76d70208b334be371b Mon Sep 17 00:00:00 2001 From: Benjamin Dauvergne Date: Thu, 2 Apr 2020 19:43:21 +0200 Subject: [PATCH 2/2] misc: do not update last_account_deletion_alert on login (#41284) --- .../commands/clean-unused-accounts.py | 84 ++++++++++--------- src/authentic2/utils/__init__.py | 2 - 2 files changed, 46 insertions(+), 40 deletions(-) diff --git a/src/authentic2/management/commands/clean-unused-accounts.py b/src/authentic2/management/commands/clean-unused-accounts.py index 6a0ce2f0..3420ec7e 100644 --- a/src/authentic2/management/commands/clean-unused-accounts.py +++ b/src/authentic2/management/commands/clean-unused-accounts.py @@ -22,6 +22,8 @@ import smtplib 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.models import F from django.utils import timezone from django.utils.six.moves.urllib import parse as urlparse from django_rbac.utils import get_ou_model @@ -36,72 +38,77 @@ logger = logging.getLogger(__name__) User = get_user_model() -def print_table(table): - col_width = [max(len(x) for x in col) for col in zip(*table)] - for line in table: - line = u"| " + u" | ".join(u"{0:>{1}}".format(x, col_width[i]) for i, x in enumerate(line)) + u" |" - print(line) - - class Command(BaseCommand): help = '''Clean unused accounts''' + verbosity_to_log_level = { + 0: logging.CRITICAL, + 1: logging.WARNING, + 2: logging.INFO, + 3: logging.DEBUG, + } + def add_arguments(self, parser): parser.add_argument("--fake", action='store_true', help='do nothing', default=False) def handle(self, *args, **options): - if options['verbosity'] == '0': - logger.setLevel(level=logging.CRITICAL) - if options['verbosity'] == '1': - logger.setLevel(level=logging.WARNING) - elif options['verbosity'] == '2': - logger.setLevel(level=logging.INFO) - elif options['verbosity'] == '3': - logger.setLevel(level=logging.DEBUG) self.fake = options['fake'] - self.clean_unused_accounts() + # add StreamHandler for console output + handler = logging.StreamHandler() + handler.setLevel(level=self.verbosity_to_log_level[options['verbosity']]) + logger.addHandler(handler) - def clean_unused_accounts(self): - now = timezone.now() + # prevent logging to external logs when fake if self.fake: - logger.info('fake call to clean-unused-accounts') + logger.propagate = False + self.now = timezone.now() + self.clean_unused_accounts() + + def clean_unused_accounts(self): 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) - users = User.objects.filter(ou=ou, last_login__lte=now - alert_delay) + ou_users = User.objects.filter(ou=ou) + + # reset last_account_deletion_alert for users which connected since last alert + active_users = ou_users.filter(last_login__gte=F('last_account_deletion_alert')) + active_users.update(last_account_deletion_alert=None) - for user in users.filter(last_account_deletion_alert__isnull=True): + inactive_users = ou_users.filter(last_login__lte=self.now - alert_delay) + + # 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: logger.info('%s last login %d days ago, sending alert', user, ou.clean_unused_accounts_alert) - self.send_alert(user) + self.send_alert(user, days_to_deletion) - to_delete = users.filter( - last_login__lte=now - deletion_delay, + inactive_users_to_delete = inactive_users.filter( + last_login__lte=self.now - deletion_delay, # ensure respect of alert delay before deletion - last_account_deletion_alert__lte=now - (deletion_delay - alert_delay) + last_account_deletion_alert__lte=self.now - (deletion_delay - alert_delay) ) - for user in to_delete: + for user in inactive_users_to_delete: logger.info( '%s last login more than %d days ago, deleting user', user, ou.clean_unused_accounts_deletion) self.delete_user(user) - def send_alert(self, user): - days_to_deletion = user.ou.clean_unused_accounts_deletion - user.ou.clean_unused_accounts_alert + def send_alert(self, user, days_to_deletion): ctx = { 'user': user, 'days_to_deletion': days_to_deletion, 'login_url': urlparse.urljoin(settings.SITE_BASE_URL, settings.LOGIN_URL), } try: - self.send_mail('authentic2/unused_account_alert', user, ctx) + with 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) except smtplib.SMTPException as e: - logger.exception('email sending failure: %s', e) - else: - if not self.fake: - user.last_account_deletion_alert = timezone.now() - user.save() + logger.warning('email sending failure: %s', e) def send_mail(self, prefix, user, ctx): if not user.email: @@ -113,8 +120,9 @@ class Command(BaseCommand): def delete_user(self, user): ctx = {'user': user} try: - self.send_mail('authentic2/unused_account_delete', user, ctx) + with atomic(): + if not self.fake: + DeletedUser.objects.delete_user(user) + self.send_mail('authentic2/unused_account_delete', user, ctx) except smtplib.SMTPException as e: - logger.exception('email sending failure: %s', e) - if not self.fake: - DeletedUser.objects.delete_user(user) + logger.warning('email sending failure: %s', e) diff --git a/src/authentic2/utils/__init__.py b/src/authentic2/utils/__init__.py index c2ee4cc7..e0ce6630 100644 --- a/src/authentic2/utils/__init__.py +++ b/src/authentic2/utils/__init__.py @@ -437,8 +437,6 @@ def login(request, user, how, service_slug=None, nonce=None, **kwargs): if constants.LAST_LOGIN_SESSION_KEY not in request.session: request.session[constants.LAST_LOGIN_SESSION_KEY] = \ localize(to_current_timezone(last_login), True) - user.last_account_deletion_alert = None - user.save() record_authentication_event(request, how, nonce=nonce) hooks.call_hooks('event', name='login', user=user, how=how, service=service_slug) return continue_to_next_url(request, **kwargs) -- 2.24.0