unused-accounts: fix no-mail federated-account deletion feature (#75181)

src/authentic2/management/commands/clean-unused-accounts.py

@@ -31,6 +31,7 @@ from authentic2.a2_rbac.models import OrganizationalUnit
 from authentic2.backends import get_user_queryset
 from authentic2.backends.ldap_backend import LDAPBackend
 from authentic2.journal_event_types import UserDeletionForInactivity, UserNotificationInactivity
+from authentic2.models import UserExternalId
 from authentic2.utils.misc import send_templated_mail
 
 logger = logging.getLogger(__name__)
@@ -126,13 +127,13 @@ class Command(BaseCommand):
                     user,
                     ou.clean_unused_accounts_deletion,
                 )
+
+                known_sources = set(UserExternalId.objects.filter(user=user).values_list('source', flat=True))
                 self.delete_user(
                     user,
                     days_of_inactivity=deletion_delay.days,
                     send_mail=user.last_login
-                    or not (
-                        getattr(user, 'oidc_account', None) or getattr(user, 'userexternalid', None) in realms
-                    ),
+                    or not (getattr(user, 'oidc_account', None) or known_sources & set(realms)),
                 )
 
     def send_alert(self, user, days_to_deletion, days_of_inactivity):

tests/test_commands.py

@@ -182,14 +182,16 @@ def test_clean_unused_account_never_logged_in(app, db, simple_user, mailoutbox,
     assert deleted_user.old_user_id == simple_user.id
 
 
-def test_clean_unused_federated_account_never_logged_in(app, db, simple_user, mailoutbox, freezer):
+def test_clean_unused_federated_account_never_logged_in(app, db, simple_user, mailoutbox, freezer, settings):
     freezer.move_to('2018-01-01')
+    settings.LDAP_AUTH_SETTINGS = [{'realm': 'ldap', 'url': 'ldap://ldap.com/', 'basedn': 'dc=ldap,dc=com'}]
+    ldap_user = User.objects.create(username='ldap-user', email='ldap-user@example.com', ou=simple_user.ou)
+    UserExternalId.objects.create(user=ldap_user, source='ldap', external_id='whatever')
     simple_user.ou.clean_unused_accounts_alert = 2
     simple_user.ou.clean_unused_accounts_deletion = 3
     simple_user.ou.save()
 
-    simple_user.last_login = None
-    simple_user.keepalive = None
+    simple_user.last_login = simple_user.keepalive = None
     simple_user.date_joined = now() - datetime.timedelta(days=4)
     simple_user.save()
 
@@ -200,6 +202,10 @@ def test_clean_unused_federated_account_never_logged_in(app, db, simple_user, ma
         sub='abc',
     )
 
+    ldap_user.last_login = ldap_user.keepalive = None
+    ldap_user.date_joined = now() - datetime.timedelta(days=4)
+    ldap_user.save()
+
     call_command('clean-unused-accounts')
     assert len(mailoutbox) == 0
 
@@ -212,8 +218,14 @@ def test_clean_unused_federated_account_never_logged_in(app, db, simple_user, ma
         ).count()
         == 1
     )
-    deleted_user = DeletedUser.objects.get()
-    assert deleted_user.old_user_id == simple_user.id
+    assert (
+        Event.objects.filter(
+            type__name='user.deletion.inactivity', user=ldap_user, data__email=ldap_user.email
+        ).count()
+        == 1
+    )
+    assert DeletedUser.objects.count() == 2
+    assert {deleted.old_user_id for deleted in DeletedUser.objects.all()} == {simple_user.id, ldap_user.id}
 
 
 def test_clean_unused_federated_account_logged_in_untouched(app, db, simple_user, mailoutbox, freezer):