From 7ab92c578f9b0bee5ae2208c34e87812863c9a91 Mon Sep 17 00:00:00 2001
From: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date: Fri, 3 Apr 2020 12:20:07 +0200
Subject: [PATCH 3/3] misc: do no clear last_account_deletion_start on login
 (#41284)

---
 .../commands/clean-unused-accounts.py         | 46 +++++++++++--------
 src/authentic2/utils/__init__.py              |  2 -
 2 files changed, 28 insertions(+), 20 deletions(-)

diff --git a/src/authentic2/management/commands/clean-unused-accounts.py b/src/authentic2/management/commands/clean-unused-accounts.py
index 7ec6dfd7..36d9bdbf 100644
--- a/src/authentic2/management/commands/clean-unused-accounts.py
+++ b/src/authentic2/management/commands/clean-unused-accounts.py
@@ -21,6 +21,8 @@ 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.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
@@ -59,45 +61,52 @@ class Command(BaseCommand):
         if self.fake:
             logger.propagate = False
 
+        self.now = timezone.now()
         try:
             self.clean_unused_accounts()
         except Exception:
             logger.exception('clean-unused-accounts failed')
 
     def clean_unused_accounts(self):
-        now = timezone.now()
-
         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)
+
+            inactive_users = ou_users.filter(last_login__lte=self.now - alert_delay)
 
-            for user in users.filter(last_account_deletion_alert__isnull=True):
+            # 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),
         }
-        self.send_mail('authentic2/unused_account_alert', user, ctx)
-        if not self.fake:
-            user.last_account_deletion_alert = timezone.now()
-            user.save()
+        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)
 
     def send_mail(self, prefix, user, ctx):
         if not user.email:
@@ -109,6 +118,7 @@ class Command(BaseCommand):
 
     def delete_user(self, user):
         ctx = {'user': user}
-        self.send_mail('authentic2/unused_account_delete', user, ctx)
-        if not self.fake:
-            DeletedUser.objects.delete_user(user)
+        with atomic():
+            if not self.fake:
+                DeletedUser.objects.delete_user(user)
+            self.send_mail('authentic2/unused_account_delete', user, ctx)
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