From ad1deeeeba24d36cfb60381530441d808aca049d Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Thu, 21 Mar 2024 19:57:50 +0530 Subject: [PATCH] [fix] Issues with unverify and delete inactive users #517 - Use date_joined if last_login is None for deleting inactive users - Don't unverify inactive users with unspecified, manual and email methods Fixes #517 --- openwisp_radius/base/models.py | 19 +++++-- openwisp_radius/tests/test_tasks.py | 83 ++++++++++++++++++++++++----- 2 files changed, 84 insertions(+), 18 deletions(-) diff --git a/openwisp_radius/base/models.py b/openwisp_radius/base/models.py index 9d575c36..c0534df6 100644 --- a/openwisp_radius/base/models.py +++ b/openwisp_radius/base/models.py @@ -1565,7 +1565,10 @@ class Meta: def unverify_inactive_users(cls): if not app_settings.UNVERIFY_INACTIVE_USERS: return - cls.objects.filter( + # Exclude users who have unspecified, manual, or email + # registration method because such users don't have an option + # to re-verify. See https://github.com/openwisp/openwisp-radius/issues/517 + cls.objects.exclude(method__in=['', 'manual', 'email']).filter( user__is_staff=False, user__last_login__lt=timezone.now() - timedelta(days=app_settings.UNVERIFY_INACTIVE_USERS), @@ -1575,8 +1578,16 @@ def unverify_inactive_users(cls): def delete_inactive_users(cls): if not app_settings.DELETE_INACTIVE_USERS: return + cutoff_date = timezone.now() - timedelta( + days=app_settings.DELETE_INACTIVE_USERS + ) User.objects.filter( - is_staff=False, - last_login__lt=timezone.now() - - timedelta(days=app_settings.DELETE_INACTIVE_USERS), + Q(is_staff=False) + & ( + Q(last_login__lt=cutoff_date) + | + # There could be manually created users which never logged in. + # We use the "date_joined" field for such users. + Q(last_login__isnull=True, date_joined__lt=cutoff_date) + ) ).delete() diff --git a/openwisp_radius/tests/test_tasks.py b/openwisp_radius/tests/test_tasks.py index 6a00f1fe..97d9459e 100644 --- a/openwisp_radius/tests/test_tasks.py +++ b/openwisp_radius/tests/test_tasks.py @@ -247,37 +247,92 @@ def test_send_login_email(self, translation_activate, utils_logger, task_logger) @mock.patch.object(app_settings, 'UNVERIFY_INACTIVE_USERS', 30) def test_unverify_inactive_users(self, *args): + """ + Checks that inactive users are unverified after the days + configured in OPENWISP_RADIUS_UNVERIFY_INACTIVE_USERS setting, + here 30 days. + + Only non-staff users that do not have unspecified(''), manual + and email registration methods are considered. + """ today = now() admin = self._create_admin(last_login=today - timedelta(days=90)) - user1 = self._create_org_user().user - user2 = self._create_org_user( - user=self._create_user(username='user2', email='user2@example.com') + active_user = self._create_org_user().user + unspecified_user = self._create_org_user( + user=self._create_user( + username='unspecified_user', email='unspecified_user@example.com' + ) ).user - User.objects.filter(id=user1.id).update(last_login=today) - User.objects.filter(id=user2.id).update(last_login=today - timedelta(days=60)) + manually_registered_user = self._create_org_user( + user=self._create_user( + username='manually_registered_user', + email='manually_registered_user@example.com', + ) + ).user + email_registered_user = self._create_org_user( + user=self._create_user( + username='email_registered_user', + email='email_registered_user@example.com', + ) + ).user + mobile_registered_user = self._create_org_user( + user=self._create_user( + username='mobile_registered_user', + email='mobile_registered_user@example.com', + ) + ).user + + User.objects.filter(id=active_user.id).update(last_login=today) + User.objects.exclude(id=active_user.id).update( + last_login=today - timedelta(days=60) + ) RegisteredUser.objects.create(user=admin, is_verified=True) - RegisteredUser.objects.create(user=user1, is_verified=True) - RegisteredUser.objects.create(user=user2, is_verified=True) + RegisteredUser.objects.create(user=active_user, is_verified=True) + RegisteredUser.objects.create( + user=unspecified_user, method='', is_verified=True + ) + RegisteredUser.objects.create( + user=manually_registered_user, method='manual', is_verified=True + ) + RegisteredUser.objects.create( + user=email_registered_user, method='email', is_verified=True + ) + RegisteredUser.objects.create( + user=mobile_registered_user, method='mobile_phone', is_verified=True + ) tasks.unverify_inactive_users.delay() admin.refresh_from_db() - user1.refresh_from_db() - user2.refresh_from_db() + active_user.refresh_from_db() + unspecified_user.refresh_from_db() + manually_registered_user.refresh_from_db() + email_registered_user.refresh_from_db() + mobile_registered_user.refresh_from_db() self.assertEqual(admin.registered_user.is_verified, True) - self.assertEqual(user1.registered_user.is_verified, True) - self.assertEqual(user2.registered_user.is_verified, False) + self.assertEqual(active_user.registered_user.is_verified, True) + self.assertEqual(unspecified_user.registered_user.is_verified, True) + self.assertEqual(manually_registered_user.registered_user.is_verified, True) + self.assertEqual(email_registered_user.registered_user.is_verified, True) + self.assertEqual(mobile_registered_user.registered_user.is_verified, False) @mock.patch.object(app_settings, 'DELETE_INACTIVE_USERS', 30) def test_delete_inactive_users(self, *args): today = now() - admin = self._create_admin(last_login=today - timedelta(days=90)) + inactive_date = today - timedelta(days=60) + admin = self._create_admin(last_login=inactive_date) user1 = self._create_org_user().user user2 = self._create_org_user( user=self._create_user(username='user2', email='user2@example.com') ).user + user3 = self._create_org_user( + user=self._create_user(username='user3', email='user3@example.com') + ).user User.objects.filter(id=user1.id).update(last_login=today) - User.objects.filter(id=user2.id).update(last_login=today - timedelta(days=60)) + User.objects.filter(id=user2.id).update(last_login=inactive_date) + User.objects.filter(id=user3.id).update( + last_login=None, date_joined=inactive_date + ) tasks.delete_inactive_users.delay() self.assertEqual(User.objects.filter(id__in=[admin.id, user1.id]).count(), 2) - self.assertEqual(User.objects.filter(id__in=[user2.id]).count(), 0) + self.assertEqual(User.objects.filter(id__in=[user2.id, user3.id]).count(), 0)