Fix for duplicate email validation on account settings page

LEARNER-6216
This commit is contained in:
tasawernawaz
2018-08-29 11:58:13 +05:00
parent ba2f1ca272
commit cc51494910
4 changed files with 29 additions and 21 deletions

View File

@@ -349,24 +349,6 @@ class EmailChangeRequestTests(EventTestMixin, CacheIsolationTestCase):
"""
self.assertEqual(self.do_email_validation(self.user.email), 'Old email is the same as the new email.')
def test_duplicate_email(self):
"""
Assert the expected error message from the email validation method for an email address
that is already in use by another account.
"""
UserFactory.create(email=self.new_email)
self.assertEqual(self.do_email_validation(self.new_email), 'An account with this e-mail already exists.')
def test_retired_email(self):
"""
Assert the expected error message from the email validation method for an email address
that corresponds with an already-retired account.
"""
user = UserFactory.create(email=self.new_email)
user.email = get_retired_email_by_email(self.new_email)
user.save()
self.assertEqual(self.do_email_validation(self.new_email), 'An account with this e-mail already exists.')
@patch('django.core.mail.send_mail')
@patch('student.views.management.render_to_string', Mock(side_effect=mock_render_to_string, autospec=True))
def test_email_failure(self, send_mail):

View File

@@ -1256,9 +1256,6 @@ def validate_new_email(user, new_email):
if new_email == user.email:
raise ValueError(_('Old email is the same as the new email.'))
if email_exists_or_retired(new_email):
raise ValueError(_('An account with this e-mail already exists.'))
def do_email_change_request(user, new_email, activation_key=None):
"""

View File

@@ -179,6 +179,11 @@ def update_account_settings(requesting_user, update, username=None):
"user_message": text_type(err)
}
# Don't process with sending email to given new email, if it is already associated with
# an account. User must see same success message with no error.
# This is so that this endpoint cannot be used to determine if an email is valid or not.
changing_email = not _verify_email_does_not_already_exists(new_email)
# If the user asked to change full name, validate it
if changing_full_name:
try:
@@ -268,6 +273,13 @@ def update_account_settings(requesting_user, update, username=None):
)
def _verify_email_does_not_already_exists(new_email):
"""
Return `True` if an account with given email already exists.
"""
return User.objects.filter(email=new_email).count() != 0
@helpers.intercept_errors(errors.UserAPIInternalError, ignore_errors=[errors.UserAPIRequestError])
@transaction.atomic
def create_account(username, password, email):

View File

@@ -677,6 +677,23 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase):
)
self.assertEqual("Valid e-mail address required.", field_errors["email"]["user_message"])
@mock.patch('student.views.management.do_email_change_request')
def test_patch_duplicate_email(self, do_email_change_request):
"""
Test that same success response will be sent to user even if the given email already used.
"""
existing_email = "same@example.com"
UserFactory.create(email=existing_email)
client = self.login_client("client", "user")
# Try changing to an existing email to make sure no error messages returned.
response = self.send_patch(client, {"email": existing_email})
self.assertEqual(200, response.status_code)
# Verify that no actual request made for email change
self.assertFalse(do_email_change_request.called)
def test_patch_language_proficiencies(self):
"""
Verify that patching the language_proficiencies field of the user