From 61a108d3771e32bc26692ba1e2d036dde08a8ce6 Mon Sep 17 00:00:00 2001 From: cahrens Date: Fri, 27 Feb 2015 13:20:57 -0500 Subject: [PATCH] Delete unused code for changing email. --- common/djangoapps/student/models.py | 26 --- .../core/djangoapps/user_api/api/account.py | 121 -------------- .../user_api/tests/test_account_api.py | 153 ------------------ 3 files changed, 300 deletions(-) diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index e18b078696..7f26eac7b1 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -309,32 +309,6 @@ class UserProfile(models.Model): self.name = new_name self.save() - @transaction.commit_on_success - def update_email(self, new_email): - """Update the user's email and save the change in the history. - - Implicitly saves the model. - If the new email is the same as the old email, do not update the history. - - Arguments: - new_email (unicode): The new email for the user. - - Returns: - None - """ - if self.user.email == new_email: - return - - meta = self.get_meta() - if 'old_emails' not in meta: - meta['old_emails'] = [] - meta['old_emails'].append([self.user.email, datetime.now(UTC).isoformat()]) - self.set_meta(meta) - self.save() - - self.user.email = new_email - self.user.save() - class UserSignupSource(models.Model): """ diff --git a/openedx/core/djangoapps/user_api/api/account.py b/openedx/core/djangoapps/user_api/api/account.py index 92a7e9a4fc..f2cf4a2a62 100644 --- a/openedx/core/djangoapps/user_api/api/account.py +++ b/openedx/core/djangoapps/user_api/api/account.py @@ -45,11 +45,6 @@ class AccountUsernameAlreadyExists(AccountUserAlreadyExists): pass -class AccountEmailAlreadyExists(AccountUserAlreadyExists): - """An account already exists with the requested email. """ - pass - - class AccountUsernameInvalid(AccountRequestError): """The requested username is not in a valid format. """ pass @@ -216,122 +211,6 @@ def activate_account(activation_key): # This implicitly saves the registration registration.activate() - -@intercept_errors(AccountInternalError, ignore_errors=[AccountRequestError]) -def request_email_change(username, new_email, password): - """Request an email change. - - Users must confirm the change before we update their information. - - Args: - username (unicode): The username associated with the account. - new_email (unicode): The user's new email address. - password (unicode): The password the user entered to authorize the change. - - Returns: - unicode: an activation key for the account. - - Raises: - AccountUserNotFound - AccountEmailAlreadyExists - AccountEmailInvalid - AccountNotAuthorized - - """ - try: - user = User.objects.get(username=username) - except User.DoesNotExist: - raise AccountUserNotFound - - # Check the user's credentials - if not user.check_password(password): - raise AccountNotAuthorized - - # Validate the email, raising an exception if it is not in the correct format - _validate_email(new_email) - - # Verify that no active account has taken the email in between - # the request and the activation. - # We'll check again before confirming and persisting the change, - # but if the email is already taken by an active account, we should - # let the user know as soon as possible. - if User.objects.filter(email=new_email, is_active=True).exists(): - raise AccountEmailAlreadyExists - - try: - pending_change = PendingEmailChange.objects.get(user=user) - except PendingEmailChange.DoesNotExist: - pending_change = PendingEmailChange(user=user) - - # Update the change (re-using the same record if it already exists) - # This will generate a new activation key and save the record. - return pending_change.request_change(new_email) - - -@intercept_errors(AccountInternalError, ignore_errors=[AccountRequestError]) -@transaction.commit_on_success -def confirm_email_change(activation_key): - """Confirm an email change. - - Users can confirm the change by providing an activation key - they received via email. - - Args: - activation_key (unicode): The activation key the user received - when he/she requested the email change. - - Returns: - Tuple: (old_email, new_email) - - Raises: - AccountNotAuthorized: The activation code is invalid. - AccountEmailAlreadyExists: Someone else has already taken the email address. - AccountInternalError - - """ - - try: - # Activation key has a uniqueness constraint, so we're guaranteed to get - # at most one pending change. - pending_change = PendingEmailChange.objects.select_related('user').get( - activation_key=activation_key - ) - except PendingEmailChange.DoesNotExist: - # If there are no changes, then the activation key is invalid - raise AccountNotAuthorized - else: - old_email = pending_change.user.email - new_email = pending_change.new_email - - # Verify that no one else has taken the email in between - # the request and the activation. - # In our production database, email has a uniqueness constraint, - # so there is no danger of a race condition here. - if User.objects.filter(email=new_email).exists(): - raise AccountEmailAlreadyExists - - # Update the email history (in the user profile) - try: - profile = UserProfile.objects.get(user=pending_change.user) - except UserProfile.DoesNotExist: - raise AccountInternalError( - "No profile exists for the user '{username}'".format( - username=pending_change.user.username - ) - ) - else: - profile.update_email(new_email) - - # Delete the pending change, so that the activation code - # will be single-use - pending_change.delete() - - # Return the old and new email - # This allows the caller of the function to notify users at both - # the new and old email, which is necessary for security reasons. - return (old_email, new_email) - - @intercept_errors(AccountInternalError, ignore_errors=[AccountRequestError]) def request_password_change(email, orig_host, is_secure): """Email a single-use link for performing a password reset. diff --git a/openedx/core/djangoapps/user_api/tests/test_account_api.py b/openedx/core/djangoapps/user_api/tests/test_account_api.py index 506487e56d..3040708dfc 100644 --- a/openedx/core/djangoapps/user_api/tests/test_account_api.py +++ b/openedx/core/djangoapps/user_api/tests/test_account_api.py @@ -76,40 +76,6 @@ class AccountApiTest(TestCase): account = account_api.account_info(self.USERNAME) self.assertTrue(account['is_active']) - def test_change_email(self): - # Request an email change - account_api.create_account(self.USERNAME, self.PASSWORD, self.EMAIL) - activation_key = account_api.request_email_change( - self.USERNAME, u'new+email@example.com', self.PASSWORD - ) - - # Verify that the email has not yet changed - account = account_api.account_info(self.USERNAME) - self.assertEqual(account['email'], self.EMAIL) - - # Confirm the change, using the activation code - old_email, new_email = account_api.confirm_email_change(activation_key) - self.assertEqual(old_email, self.EMAIL) - self.assertEqual(new_email, u'new+email@example.com') - - # Verify that the email is changed - account = account_api.account_info(self.USERNAME) - self.assertEqual(account['email'], u'new+email@example.com') - - def test_confirm_email_change_repeat(self): - account_api.create_account(self.USERNAME, self.PASSWORD, self.EMAIL) - activation_key = account_api.request_email_change( - self.USERNAME, u'new+email@example.com', self.PASSWORD - ) - - # Confirm the change once - account_api.confirm_email_change(activation_key) - - # Confirm the change again. The activation code should be - # single-use, so this should raise an error. - with self.assertRaises(account_api.AccountNotAuthorized): - account_api.confirm_email_change(activation_key) - def test_create_account_duplicate_username(self): account_api.create_account(self.USERNAME, self.PASSWORD, self.EMAIL) with self.assertRaises(account_api.AccountUserAlreadyExists): @@ -155,125 +121,6 @@ class AccountApiTest(TestCase): def test_activate_account_invalid_key(self): account_api.activate_account(u'invalid') - @raises(account_api.AccountUserNotFound) - def test_request_email_change_no_user(self): - account_api.request_email_change(u'no_such_user', self.EMAIL, self.PASSWORD) - - @ddt.data(*INVALID_EMAILS) - def test_request_email_change_invalid_email(self, invalid_email): - # Create an account with a valid email address - account_api.create_account(self.USERNAME, self.PASSWORD, self.EMAIL) - - # Attempt to change the account to an invalid email - with self.assertRaises(account_api.AccountEmailInvalid): - account_api.request_email_change(self.USERNAME, invalid_email, self.PASSWORD) - - def test_request_email_change_already_exists(self): - # Create two accounts, both activated - activation_key = account_api.create_account(self.USERNAME, self.PASSWORD, self.EMAIL) - account_api.activate_account(activation_key) - activation_key = account_api.create_account(u'another_user', u'password', u'another+user@example.com') - account_api.activate_account(activation_key) - - # Try to change the first user's email to the same as the second user's - with self.assertRaises(account_api.AccountEmailAlreadyExists): - account_api.request_email_change(self.USERNAME, u'another+user@example.com', self.PASSWORD) - - def test_request_email_change_duplicates_unactivated_account(self): - # Create two accounts, but the second account is inactive - activation_key = account_api.create_account(self.USERNAME, self.PASSWORD, self.EMAIL) - account_api.activate_account(activation_key) - account_api.create_account(u'another_user', u'password', u'another+user@example.com') - - # Try to change the first user's email to the same as the second user's - # Since the second user has not yet activated, this should succeed. - account_api.request_email_change(self.USERNAME, u'another+user@example.com', self.PASSWORD) - - def test_request_email_change_same_address(self): - # Create and activate the account - activation_key = account_api.create_account(self.USERNAME, self.PASSWORD, self.EMAIL) - account_api.activate_account(activation_key) - - # Try to change the email address to the current address - with self.assertRaises(account_api.AccountEmailAlreadyExists): - account_api.request_email_change(self.USERNAME, self.EMAIL, self.PASSWORD) - - def test_request_email_change_wrong_password(self): - account_api.create_account(self.USERNAME, self.PASSWORD, self.EMAIL) - - # Use the wrong password - with self.assertRaises(account_api.AccountNotAuthorized): - account_api.request_email_change(self.USERNAME, u'new+email@example.com', u'wrong password') - - def test_confirm_email_change_invalid_activation_key(self): - account_api.create_account(self.USERNAME, self.PASSWORD, self.EMAIL) - account_api.request_email_change(self.USERNAME, u'new+email@example.com', self.PASSWORD) - - with self.assertRaises(account_api.AccountNotAuthorized): - account_api.confirm_email_change(u'invalid') - - def test_confirm_email_change_no_request_pending(self): - account_api.create_account(self.USERNAME, self.PASSWORD, self.EMAIL) - - def test_confirm_email_already_exists(self): - account_api.create_account(self.USERNAME, self.PASSWORD, self.EMAIL) - - # Request a change - activation_key = account_api.request_email_change( - self.USERNAME, u'new+email@example.com', self.PASSWORD - ) - - # Another use takes the email before we confirm the change - account_api.create_account(u'other_user', u'password', u'new+email@example.com') - - # When we try to confirm our change, we get an error because the email is taken - with self.assertRaises(account_api.AccountEmailAlreadyExists): - account_api.confirm_email_change(activation_key) - - # Verify that the email was NOT changed - self.assertEqual(account_api.account_info(self.USERNAME)['email'], self.EMAIL) - - def test_confirm_email_no_user_profile(self): - account_api.create_account(self.USERNAME, self.PASSWORD, self.EMAIL) - activation_key = account_api.request_email_change( - self.USERNAME, u'new+email@example.com', self.PASSWORD - ) - - # This should never happen, but just in case... - UserProfile.objects.get(user__username=self.USERNAME).delete() - - with self.assertRaises(account_api.AccountInternalError): - account_api.confirm_email_change(activation_key) - - def test_record_email_change_history(self): - account_api.create_account(self.USERNAME, self.PASSWORD, self.EMAIL) - - # Change the email once - activation_key = account_api.request_email_change( - self.USERNAME, u'new+email@example.com', self.PASSWORD - ) - account_api.confirm_email_change(activation_key) - - # Verify that the old email appears in the history - meta = UserProfile.objects.get(user__username=self.USERNAME).get_meta() - self.assertEqual(len(meta['old_emails']), 1) - email, timestamp = meta['old_emails'][0] - self.assertEqual(email, self.EMAIL) - self._assert_is_datetime(timestamp) - - # Change the email again - activation_key = account_api.request_email_change( - self.USERNAME, u'another_new+email@example.com', self.PASSWORD - ) - account_api.confirm_email_change(activation_key) - - # Verify that both emails appear in the history - meta = UserProfile.objects.get(user__username=self.USERNAME).get_meta() - self.assertEqual(len(meta['old_emails']), 2) - email, timestamp = meta['old_emails'][1] - self.assertEqual(email, 'new+email@example.com') - self._assert_is_datetime(timestamp) - @skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in LMS') def test_request_password_change(self): # Create and activate an account