diff --git a/common/djangoapps/student/forms.py b/common/djangoapps/student/forms.py index 676c4f89de..00c9af4193 100644 --- a/common/djangoapps/student/forms.py +++ b/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): diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 0142b7cb41..31a0542bc2 100644 --- a/common/djangoapps/student/models.py +++ b/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() diff --git a/common/djangoapps/student/tests/factories.py b/common/djangoapps/student/tests/factories.py index c5c4d82840..38ff93f0bd 100644 --- a/common/djangoapps/student/tests/factories.py +++ b/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 diff --git a/common/djangoapps/student/views/management.py b/common/djangoapps/student/views/management.py index 5593f9de6a..d33ffa84bd 100644 --- a/common/djangoapps/student/views/management.py +++ b/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: diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_views.py b/openedx/core/djangoapps/user_authn/views/tests/test_views.py index ed072d5c72..5ae81ffe2b 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_views.py +++ b/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