Make sure only active AccountRecovery records are used

common/djangoapps/student/forms.py

@@ -160,7 +160,7 @@ class AccountRecoveryForm(PasswordResetFormNoActive):
         email = self.cleaned_data["email"]
         # The line below contains the only change, getting users via AccountRecovery
         self.users_cache = User.objects.filter(
-            id__in=AccountRecovery.objects.filter(secondary_email__iexact=email).values_list('user')
+            id__in=AccountRecovery.objects.filter(secondary_email__iexact=email, is_active=True).values_list('user')
         )
 
         if not len(self.users_cache):

common/djangoapps/student/models.py

@@ -2700,6 +2700,31 @@ class LogoutViewConfiguration(ConfigurationModel):
         return u'Logout view configuration: {enabled}'.format(enabled=self.enabled)
 
 
+class AccountRecoveryManager(models.Manager):
+    """
+    Custom Manager for AccountRecovery model
+    """
+
+    def get_active(self, **filters):
+        """
+        Return only active AccountRecovery record after applying the given filters.
+
+        Arguments:
+            filters (**kwargs): Filter parameters for AccountRecovery records.
+
+        Returns:
+            AccountRecovery: AccountRecovery object with is_active=true
+        """
+        filters['is_active'] = True
+        return super(AccountRecoveryManager, self).get_queryset().get(**filters)
+
+    def activate(self):
+        """
+        Set is_active flag to True.
+        """
+        super(AccountRecoveryManager, self).get_queryset().update(is_active=True)
+
+
 class AccountRecovery(models.Model):
     """
     Model for storing information for user's account recovery in case of access loss.
@@ -2716,3 +2741,5 @@ class AccountRecovery(models.Model):
 
     class Meta(object):
         db_table = "auth_accountrecovery"
+
+    objects = AccountRecoveryManager()

common/djangoapps/student/tests/factories.py

@@ -210,3 +210,4 @@ class AccountRecoveryFactory(DjangoModelFactory):
 
     user = None
     secondary_email = factory.Sequence(u'robot+test+recovery+{0}@edx.org'.format)
+    is_active = True

common/djangoapps/student/views/management.py

@@ -759,7 +759,7 @@ def account_recovery_request_handler(request):
 
             # Check if a user exists with the given secondary email, if so then invalidate the existing oauth tokens.
             user = user if user.is_authenticated else User.objects.get(
-                id=AccountRecovery.objects.get(secondary_email__iexact=email).user.id
+                id=AccountRecovery.objects.get_active(secondary_email__iexact=email).user.id
             )
             destroy_oauth_tokens(user)
         except UserNotFound:

openedx/core/djangoapps/user_authn/views/tests/test_views.py

@@ -276,7 +276,7 @@ class UserAccountUpdateTest(CacheIsolationTestCase, UrlResetMixin):
 
     @override_settings(FEATURES=FEATURES_WITH_FAILED_PASSWORD_RESET_EMAIL)
     def test_account_recovery_failure_email(self):
-        """Test that a password reset failure email notification is sent, when enabled."""
+        """Test that log message is added when email does not match any in the system."""
         # Log the user out
         self.client.logout()
 
@@ -293,6 +293,26 @@ class UserAccountUpdateTest(CacheIsolationTestCase, UrlResetMixin):
                 )
             )
 
+    @override_settings(FEATURES=FEATURES_WITH_FAILED_PASSWORD_RESET_EMAIL)
+    def test_account_recovery_failure_not_active(self):
+        """Test that log message is added when email does not match any active account recovery records."""
+        # Log the user out
+        self.client.logout()
+        self.account_recovery.is_active = False
+        self.account_recovery.save()
+
+        with LogCapture(LOGGER_NAME, level=logging.INFO) as logger:
+            response = self._recover_account(email=self.account_recovery.secondary_email)
+            self.assertEqual(response.status_code, 200)
+            logger.check(
+                (
+                    LOGGER_NAME,
+                    "WARNING", "Account recovery attempt via invalid secondary email '{email}'.".format(
+                        email=self.account_recovery.secondary_email
+                    )
+                )
+            )
+
     def test_password_change_rate_limited_during_account_recovery(self):
         # Log out the user created during test setup, to prevent the view from
         # selecting the logged-in user's email address over the email provided