Add recovery email to account settings page

This commit is contained in:
Saleem Latif
2018-11-27 17:40:55 +05:00
parent a55ed88378
commit bd411de12d
14 changed files with 248 additions and 16 deletions

View File

@@ -14,7 +14,14 @@ from django.http import HttpResponseForbidden
from openedx.core.djangoapps.theming.helpers import get_current_request
from six import text_type
from student.models import User, UserProfile, Registration, email_exists_or_retired, username_exists_or_retired
from student.models import (
AccountRecovery,
User,
UserProfile,
Registration,
email_exists_or_retired,
username_exists_or_retired
)
from student import forms as student_forms
from student import views as student_views
from util.model_utils import emit_setting_changed_event
@@ -131,6 +138,7 @@ def update_account_settings(requesting_user, update, username=None):
username = requesting_user.username
existing_user, existing_user_profile = _get_user_and_profile(username)
account_recovery = _get_account_recovery(existing_user)
if requesting_user.username != username:
raise errors.UserNotAuthorized()
@@ -151,6 +159,10 @@ def update_account_settings(requesting_user, update, username=None):
changing_full_name = True
old_name = existing_user_profile.name
changing_secondary_email = False
if "secondary_email" in update:
changing_secondary_email = True
# Check for fields that are not editable. Marking them read-only causes them to be ignored, but we wish to 400.
read_only_fields = set(update.keys()).intersection(
AccountUserSerializer.get_read_only_fields() + AccountLegacyProfileSerializer.get_read_only_fields()
@@ -188,6 +200,18 @@ def update_account_settings(requesting_user, update, username=None):
# This is so that this endpoint cannot be used to determine if an email is valid or not.
changing_email = new_email and not email_exists_or_retired(new_email)
if changing_secondary_email:
try:
student_views.validate_secondary_email(account_recovery, update["secondary_email"])
except ValueError as err:
field_errors["secondary_email"] = {
"developer_message": u"Error thrown from validate_secondary_email: '{}'".format(text_type(err)),
"user_message": text_type(err)
}
else:
account_recovery.secondary_email = update["secondary_email"]
account_recovery.save()
# If the user asked to change full name, validate it
if changing_full_name:
try:
@@ -485,6 +509,19 @@ def get_email_validation_error(email):
return _validate(_validate_email, errors.AccountEmailInvalid, email)
def get_secondary_email_validation_error(email):
"""
Get the built-in validation error message for when the email is invalid in some way.
Arguments:
email (str): The proposed email (unicode).
Returns:
(str): Validation error message.
"""
return _validate(_validate_secondary_email_doesnt_exist, errors.AccountEmailAlreadyExists, email)
def get_confirm_email_validation_error(confirm_email, email):
"""Get the built-in validation error message for when
the confirmation email is invalid in some way.
@@ -560,6 +597,18 @@ def _get_user_and_profile(username):
return existing_user, existing_user_profile
def _get_account_recovery(user):
"""
helper method to return the account recovery object based on user.
"""
try:
account_recovery = user.account_recovery
except ObjectDoesNotExist:
account_recovery = AccountRecovery(user=user)
return account_recovery
def _validate(validation_func, err, *args):
"""Generic validation function that returns default on
no errors, but the message associated with the err class
@@ -709,6 +758,25 @@ def _validate_email_doesnt_exist(email):
raise errors.AccountEmailAlreadyExists(_(accounts.EMAIL_CONFLICT_MSG).format(email_address=email))
def _validate_secondary_email_doesnt_exist(email):
"""
Validate that the email is not associated as a secondary email of an existing user.
Arguments:
email (unicode): The proposed email.
Returns:
None
Raises:
errors.AccountEmailAlreadyExists: Raised if given email address is already associated as another
user's secondary email.
"""
if email is not None and AccountRecovery.objects.filter(secondary_email=email).exists():
# pylint: disable=no-member
raise errors.AccountEmailAlreadyExists(accounts.EMAIL_CONFLICT_MSG.format(email_address=email))
def _validate_password_works_with_username(password, username=None):
"""Run validation checks on whether the password and username
go well together.

View File

@@ -14,6 +14,7 @@ from six import text_type
from lms.djangoapps.badges.utils import badges_enabled
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
from openedx.core.djangoapps.user_api import errors
from openedx.core.djangoapps.user_api.accounts.utils import is_secondary_email_feature_enabled_for_user
from openedx.core.djangoapps.user_api.models import (
RetirementState,
UserPreference,
@@ -81,7 +82,7 @@ class UserReadOnlySerializer(serializers.Serializer):
def to_representation(self, user):
"""
Overwrite to_native to handle custom logic since we are serializing two models as one here
Overwrite to_native to handle custom logic since we are serializing three models as one here
:param user: User object
:return: Dict serialized account
"""
@@ -91,6 +92,11 @@ class UserReadOnlySerializer(serializers.Serializer):
user_profile = None
LOGGER.warning("user profile for the user [%s] does not exist", user.username)
try:
account_recovery = user.account_recovery
except ObjectDoesNotExist:
account_recovery = None
accomplishments_shared = badges_enabled()
data = {
@@ -150,6 +156,14 @@ class UserReadOnlySerializer(serializers.Serializer):
}
)
if account_recovery:
if is_secondary_email_feature_enabled_for_user(user):
data.update(
{
"secondary_email": account_recovery.secondary_email,
}
)
if self.custom_fields:
fields = self.custom_fields
elif user_profile:

View File

@@ -357,6 +357,7 @@ class AccountSettingsOnCreationTest(TestCase):
'account_privacy': PRIVATE_VISIBILITY,
'accomplishments_shared': False,
'extended_profile': [],
'secondary_email': None
})
def test_normalize_password(self):

View File

@@ -243,7 +243,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase):
Verify that all account fields are returned (even those that are not shareable).
"""
data = response.data
self.assertEqual(19, len(data))
self.assertEqual(20, len(data))
self.assertEqual(self.user.username, data["username"])
self.assertEqual(self.user.first_name + " " + self.user.last_name, data["name"])
self.assertEqual("US", data["country"])
@@ -301,7 +301,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase):
"""
self.different_client.login(username=self.different_user.username, password=TEST_PASSWORD)
self.create_mock_profile(self.user)
with self.assertNumQueries(21):
with self.assertNumQueries(22):
response = self.send_get(self.different_client)
self._verify_full_shareable_account_response(response, account_privacy=ALL_USERS_VISIBILITY)
@@ -316,7 +316,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase):
"""
self.different_client.login(username=self.different_user.username, password=TEST_PASSWORD)
self.create_mock_profile(self.user)
with self.assertNumQueries(21):
with self.assertNumQueries(22):
response = self.send_get(self.different_client)
self._verify_private_account_response(response, account_privacy=PRIVATE_VISIBILITY)
@@ -372,7 +372,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase):
with self.assertNumQueries(queries):
response = self.send_get(self.client)
data = response.data
self.assertEqual(19, len(data))
self.assertEqual(20, len(data))
self.assertEqual(self.user.username, data["username"])
self.assertEqual(self.user.first_name + " " + self.user.last_name, data["name"])
for empty_field in ("year_of_birth", "level_of_education", "mailing_address", "bio"):
@@ -391,12 +391,12 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase):
self.assertEqual(False, data["accomplishments_shared"])
self.client.login(username=self.user.username, password=TEST_PASSWORD)
verify_get_own_information(19)
verify_get_own_information(20)
# Now make sure that the user can get the same information, even if not active
self.user.is_active = False
self.user.save()
verify_get_own_information(13)
verify_get_own_information(14)
def test_get_account_empty_string(self):
"""
@@ -410,7 +410,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase):
legacy_profile.save()
self.client.login(username=self.user.username, password=TEST_PASSWORD)
with self.assertNumQueries(19):
with self.assertNumQueries(20):
response = self.send_get(self.client)
for empty_field in ("level_of_education", "gender", "country", "bio"):
self.assertIsNone(response.data[empty_field])
@@ -782,7 +782,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase):
response = self.send_get(client)
if has_full_access:
data = response.data
self.assertEqual(19, len(data))
self.assertEqual(20, len(data))
self.assertEqual(self.user.username, data["username"])
self.assertEqual(self.user.first_name + " " + self.user.last_name, data["name"])
self.assertEqual(self.user.email, data["email"])

View File

@@ -8,6 +8,7 @@ import re
import string
from urlparse import urlparse
import waffle
from django.conf import settings
from django.utils.translation import ugettext as _
from six import text_type
@@ -19,6 +20,8 @@ from openedx.core.djangoapps.theming.helpers import get_config_value_from_site_o
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.exceptions import ItemNotFoundError
ENABLE_SECONDARY_EMAIL_FEATURE_SWITCH = 'enable_secondary_email_feature'
def validate_social_link(platform_name, new_social_link):
"""
@@ -189,3 +192,25 @@ def generate_password(length=12, chars=string.letters + string.digits):
password += choice(string.letters)
password += ''.join([choice(chars) for _i in xrange(length - 2)])
return password
def is_secondary_email_feature_enabled():
"""
Checks to see if the django-waffle switch for enabling the secondary email feature is active
Returns:
Boolean value representing switch status
"""
return waffle.switch_is_active(ENABLE_SECONDARY_EMAIL_FEATURE_SWITCH)
def is_secondary_email_feature_enabled_for_user(user):
"""
Checks to see if secondary email feature is enabled for the given user.
Returns:
Boolean value representing the status of secondary email feature.
"""
# import is placed here to avoid cyclic import.
from openedx.features.enterprise_support.utils import is_enterprise_learner
return is_secondary_email_feature_enabled() and is_enterprise_learner(user)