diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 0491471de1f6b9ec82edfe81ca5d188ff188dd13..7d088c355b760ee2a5e96650c0f164ce5e1ba907 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -18,31 +18,30 @@ import logging import uuid from collections import OrderedDict, defaultdict, namedtuple from datetime import datetime, timedelta -from django.core.validators import FileExtensionValidator from functools import total_ordering from importlib import import_module import six from config_models.models import ConfigurationModel +from course_modes.models import CourseMode, get_cosmetic_verified_display_price from django.apps import apps from django.conf import settings -from django.contrib.auth.hashers import make_password from django.contrib.auth.models import User from django.contrib.auth.signals import user_logged_in, user_logged_out from django.contrib.sites.models import Site from django.core.cache import cache from django.core.exceptions import MultipleObjectsReturned, ObjectDoesNotExist +from django.core.validators import FileExtensionValidator from django.db import IntegrityError, models from django.db.models import Count, Q, Index from django.db.models.signals import post_save, pre_save from django.db.utils import ProgrammingError from django.dispatch import receiver -from django.utils import timezone +from django.utils.encoding import python_2_unicode_compatible from django.utils.functional import cached_property from django.utils.translation import ugettext_lazy as _ from django.utils.translation import ugettext_noop from django_countries.fields import CountryField -from django.utils.encoding import python_2_unicode_compatible from edx_django_utils.cache import RequestCache from edx_rest_api_client.exceptions import SlumberBaseException from eventtracking import tracker @@ -55,18 +54,22 @@ from six import text_type from six.moves import range from six.moves.urllib.parse import urlencode from slumber.exceptions import HttpClientError, HttpServerError +from student.signals import ENROLL_STATUS_CHANGE, ENROLLMENT_TRACK_UPDATED, UNENROLL_DONE +from track import contexts, segment from user_util import user_util +from util.milestones_helpers import is_entrance_exams_enabled +from util.model_utils import emit_field_changed_events, get_changed_fields_dict +from util.query import use_read_replica_if_available -from course_modes.models import CourseMode, get_cosmetic_verified_display_price +import openedx.core.djangoapps.django_comment_common.comment_client as cc +from lms.djangoapps.certificates.models import GeneratedCertificate from lms.djangoapps.courseware.models import ( CourseDynamicUpgradeDeadlineConfiguration, DynamicUpgradeDeadlineConfiguration, OrgDynamicUpgradeDeadlineConfiguration ) -from lms.djangoapps.certificates.models import GeneratedCertificate from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification from openedx.core.djangoapps.content.course_overviews.models import CourseOverview -import openedx.core.djangoapps.django_comment_common.comment_client as cc from openedx.core.djangoapps.enrollments.api import ( _default_course_mode, get_enrollment_attributes, @@ -75,11 +78,6 @@ from openedx.core.djangoapps.enrollments.api import ( from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.djangoapps.xmodule_django.models import NoneToEmptyManager from openedx.core.djangolib.model_mixins import DeletableByUserValue -from student.signals import ENROLL_STATUS_CHANGE, ENROLLMENT_TRACK_UPDATED, UNENROLL_DONE -from track import contexts, segment -from util.milestones_helpers import is_entrance_exams_enabled -from util.model_utils import emit_field_changed_events, get_changed_fields_dict -from util.query import use_read_replica_if_available log = logging.getLogger(__name__) AUDIT_LOG = logging.getLogger("audit") @@ -2983,7 +2981,7 @@ class AccountRecovery(models.Model): email (str): New email address to be set as the secondary email address. """ self.secondary_email = email - self.is_active = False + self.is_active = True self.save() @classmethod diff --git a/common/djangoapps/student/views/dashboard.py b/common/djangoapps/student/views/dashboard.py index 8b9ae0cf17c4abce75b69ac9b14a11d7c8d2d330..38d4ef1fdad06d22eba37e26b0291056efc1dcda 100644 --- a/common/djangoapps/student/views/dashboard.py +++ b/common/djangoapps/student/views/dashboard.py @@ -7,6 +7,10 @@ import datetime import logging from collections import defaultdict +import track.views +from bulk_email.api import is_bulk_email_feature_enabled +from bulk_email.models import Optout # pylint: disable=import-error +from course_modes.models import CourseMode from django.conf import settings from django.contrib import messages from django.contrib.auth.decorators import login_required @@ -15,18 +19,26 @@ from django.urls import reverse from django.utils.translation import ugettext as _ from django.views.decorators.csrf import ensure_csrf_cookie from edx_django_utils import monitoring as monitoring_utils +from edxmako.shortcuts import render_to_response, render_to_string +from entitlements.models import CourseEntitlement from opaque_keys.edx.keys import CourseKey from pytz import UTC +from shoppingcart.models import CourseRegistrationCode, DonationConfiguration from six import iteritems, text_type +from student.helpers import cert_info, check_verify_status_by_course, get_resume_urls_for_enrollments +from student.models import ( + AccountRecovery, + CourseEnrollment, + CourseEnrollmentAttribute, + DashboardConfiguration, + PendingSecondaryEmailChange, + UserProfile +) +from util.milestones_helpers import get_pre_requisite_courses_not_completed +from xmodule.modulestore.django import modulestore -import track.views -from bulk_email.api import is_bulk_email_feature_enabled -from bulk_email.models import Optout # pylint: disable=import-error -from course_modes.models import CourseMode -from lms.djangoapps.courseware.access import has_access -from edxmako.shortcuts import render_to_response, render_to_string -from entitlements.models import CourseEntitlement from lms.djangoapps.commerce.utils import EcommerceService # pylint: disable=import-error +from lms.djangoapps.courseware.access import has_access from lms.djangoapps.experiments.utils import get_dashboard_course_info from lms.djangoapps.verify_student.services import IDVerificationService from openedx.core.djangoapps.catalog.utils import ( @@ -39,22 +51,10 @@ from openedx.core.djangoapps.programs.models import ProgramsApiConfig from openedx.core.djangoapps.programs.utils import ProgramDataExtender, ProgramProgressMeter from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.djangoapps.user_api.accounts.utils import is_secondary_email_feature_enabled_for_user -from openedx.core.djangoapps.user_authn.cookies import set_logged_in_cookies from openedx.core.djangoapps.util.maintenance_banner import add_maintenance_banner from openedx.core.djangoapps.waffle_utils import WaffleFlag, WaffleFlagNamespace from openedx.core.djangolib.markup import HTML, Text from openedx.features.enterprise_support.api import get_dashboard_consent_notification -from shoppingcart.models import CourseRegistrationCode, DonationConfiguration -from student.helpers import cert_info, check_verify_status_by_course, get_resume_urls_for_enrollments -from student.models import ( - AccountRecovery, - CourseEnrollment, - CourseEnrollmentAttribute, - DashboardConfiguration, - UserProfile -) -from util.milestones_helpers import get_pre_requisite_courses_not_completed -from xmodule.modulestore.django import modulestore log = logging.getLogger("edx.student") @@ -654,28 +654,31 @@ def student_dashboard(request): recovery_email_message = recovery_email_activation_message = None if is_secondary_email_feature_enabled_for_user(user=user): try: - account_recovery_obj = AccountRecovery.objects.get(user=user) - except AccountRecovery.DoesNotExist: - recovery_email_message = Text( - _( - "Add a recovery email to retain access when single-sign on is not available. " - "Go to {link_start}your Account Settings{link_end}.") - ).format( - link_start=HTML("<a href='{account_setting_page}'>").format( - account_setting_page=reverse('account_settings'), - ), - link_end=HTML("</a>") - ) - else: - if not account_recovery_obj.is_active: - recovery_email_activation_message = Text( + pending_email = PendingSecondaryEmailChange.objects.get(user=user) + except PendingSecondaryEmailChange.DoesNotExist: + try: + account_recovery_obj = AccountRecovery.objects.get(user=user) + except AccountRecovery.DoesNotExist: + recovery_email_message = Text( _( - "Recovery email is not activated yet. " - "Kindly visit your email and follow the instructions to activate it." - ) + "Add a recovery email to retain access when single-sign on is not available. " + "Go to {link_start}your Account Settings{link_end}.") + ).format( + link_start=HTML("<a href='{account_setting_page}'>").format( + account_setting_page=reverse('account_settings'), + ), + link_end=HTML("</a>") ) + else: + recovery_email_activation_message = Text( + _( + "Recovery email is not activated yet. " + "Kindly visit your email and follow the instructions to activate it." + ) + ) + - # Disable lookup of Enterprise consent_required_course due to ENT-727 +# Disable lookup of Enterprise consent_required_course due to ENT-727 # Will re-enable after fixing WL-1315 consent_required_courses = set() enterprise_customer_name = None diff --git a/common/djangoapps/student/views/management.py b/common/djangoapps/student/views/management.py index 10ff1dfe70fe655bd94d7c4bd2b2176a7bd7e2b7..3096af94d5852f0a511cf9b1f9a5631fbd0ae387 100644 --- a/common/djangoapps/student/views/management.py +++ b/common/djangoapps/student/views/management.py @@ -601,7 +601,7 @@ def validate_new_email(user, new_email): raise ValueError(_('Old email is the same as the new email.')) -def validate_secondary_email(account_recovery, new_email): +def validate_secondary_email(user, new_email): """ Enforce valid email addresses. """ @@ -612,11 +612,13 @@ def validate_secondary_email(account_recovery, new_email): if get_email_validation_error(new_email): raise ValueError(_('Valid e-mail address required.')) - if new_email == account_recovery.secondary_email: - raise ValueError(_('Old email is the same as the new email.')) + # Make sure that if there is an active recovery email address, that is not the same as the new one. + if hasattr(user, "account_recovery"): + if user.account_recovery.is_active and new_email == user.account_recovery.secondary_email: + raise ValueError(_('Old email is the same as the new email.')) # Make sure that secondary email address is not same as user's primary email. - if new_email == account_recovery.user.email: + if new_email == user.email: raise ValueError(_('Cannot be same as your sign in email address.')) # Make sure that secondary email address is not same as any of the primary emails currently in use or retired @@ -723,14 +725,19 @@ def activate_secondary_email(request, key): # pylint: disable=unused-argument return render_to_response("invalid_email_key.html", {}) try: - account_recovery_obj = AccountRecovery.objects.get(user_id=pending_secondary_email_change.user) + account_recovery = pending_secondary_email_change.user.account_recovery except AccountRecovery.DoesNotExist: + account_recovery = AccountRecovery(user=pending_secondary_email_change.user) + + try: + account_recovery.update_recovery_email(pending_secondary_email_change.new_secondary_email) + except ValidationError: return render_to_response("secondary_email_change_failed.html", { 'secondary_email': pending_secondary_email_change.new_secondary_email }) - account_recovery_obj.is_active = True - account_recovery_obj.save() + pending_secondary_email_change.delete() + return render_to_response("secondary_email_change_successful.html") diff --git a/openedx/core/djangoapps/user_api/accounts/api.py b/openedx/core/djangoapps/user_api/accounts/api.py index 71145a8f1017deff66dcaa50401c6022960887e0..79f6c13b7bd5f570ab944b6be78a3c10dd13dd79 100644 --- a/openedx/core/djangoapps/user_api/accounts/api.py +++ b/openedx/core/djangoapps/user_api/accounts/api.py @@ -15,30 +15,27 @@ from django.utils.translation import override as override_language from django.utils.translation import ugettext as _ from pytz import UTC from six import text_type # pylint: disable=ungrouped-imports - -from openedx.core.djangoapps.user_api import accounts, errors, helpers -from openedx.core.djangoapps.user_api.config.waffle import PREVENT_AUTH_USER_WRITES, SYSTEM_MAINTENANCE_MSG, waffle -from openedx.core.djangoapps.user_api.errors import ( - AccountUpdateError, - AccountValidationError, - PreferenceValidationError -) -from openedx.core.djangoapps.user_api.preferences.api import update_user_preferences -from openedx.core.lib.api.view_utils import add_serializer_errors -from openedx.core.djangoapps.user_authn.views.registration_form import validate_name, validate_username -from openedx.features.enterprise_support.utils import get_enterprise_readonly_account_fields from student import views as student_views from student.models import ( AccountRecovery, - Registration, User, UserProfile, email_exists_or_retired, username_exists_or_retired ) from util.model_utils import emit_setting_changed_event -from util.password_policy_validators import normalize_password, validate_password +from util.password_policy_validators import validate_password +from openedx.core.djangoapps.user_api import accounts, errors, helpers +from openedx.core.djangoapps.user_api.errors import ( + AccountUpdateError, + AccountValidationError, + PreferenceValidationError +) +from openedx.core.djangoapps.user_api.preferences.api import update_user_preferences +from openedx.core.djangoapps.user_authn.views.registration_form import validate_name, validate_username +from openedx.core.lib.api.view_utils import add_serializer_errors +from openedx.features.enterprise_support.utils import get_enterprise_readonly_account_fields from .serializers import AccountLegacyProfileSerializer, AccountUserSerializer, UserReadOnlySerializer, _visible_fields # Public access point for this function. @@ -221,16 +218,13 @@ def _validate_secondary_email(user, data, field_errors): if "secondary_email" not in data: return - account_recovery = _get_account_recovery(user) try: - student_views.validate_secondary_email(account_recovery, data["secondary_email"]) + student_views.validate_secondary_email(user, data["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.update_recovery_email(data["secondary_email"]) def _validate_name_change(user_profile, data, field_errors): @@ -453,18 +447,6 @@ 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 diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_api.py b/openedx/core/djangoapps/user_api/accounts/tests/test_api.py index 89a583978910e284b2bf9fe4517aa93775f1b4ef..cea200d7973170ed80e89cf68110585ca0ac6355 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_api.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_api.py @@ -6,26 +6,31 @@ Most of the functionality is covered in test_views.py. import itertools -import re import unicodedata import ddt -import pytest -from dateutil.parser import parse as parse_datetime from django.conf import settings from django.contrib.auth.hashers import make_password from django.contrib.auth.models import User -from django.core import mail +from django.http import HttpResponse from django.test import TestCase from django.test.client import RequestFactory from django.urls import reverse from mock import Mock, patch from six import iteritems from social_django.models import UserSocialAuth +from student.models import ( + AccountRecovery, + PendingEmailChange, + PendingSecondaryEmailChange, + UserProfile +) +from student.tests.factories import UserFactory +from student.tests.tests import UserSettingsEventTestMixin +from student.views.management import activate_secondary_email from openedx.core.djangoapps.ace_common.tests.mixins import EmailTemplateTagMixin -from openedx.core.djangoapps.site_configuration.tests.factories import SiteFactory -from openedx.core.djangoapps.user_api.accounts import PRIVATE_VISIBILITY, USERNAME_MAX_LENGTH +from openedx.core.djangoapps.user_api.accounts import PRIVATE_VISIBILITY from openedx.core.djangoapps.user_api.accounts.api import ( get_account_settings, update_account_settings @@ -35,30 +40,14 @@ from openedx.core.djangoapps.user_api.accounts.tests.retirement_helpers import ( fake_requested_retirement, setup_retirement_states ) -from openedx.core.djangoapps.user_api.accounts.tests.testutils import ( - INVALID_EMAILS, - INVALID_PASSWORDS, - INVALID_USERNAMES, - VALID_USERNAMES_UNICODE -) -from openedx.core.djangoapps.user_api.config.waffle import PREVENT_AUTH_USER_WRITES, SYSTEM_MAINTENANCE_MSG, waffle from openedx.core.djangoapps.user_api.errors import ( - AccountEmailInvalid, - AccountPasswordInvalid, - AccountRequestError, AccountUpdateError, - AccountUserAlreadyExists, - AccountUsernameInvalid, AccountValidationError, - UserAPIInternalError, UserNotAuthorized, UserNotFound ) from openedx.core.djangolib.testing.utils import skip_unless_lms from openedx.features.enterprise_support.tests.factories import EnterpriseCustomerUserFactory -from student.models import PendingEmailChange, Registration -from student.tests.factories import UserFactory -from student.tests.tests import UserSettingsEventTestMixin def mock_render_to_string(template_name, context): @@ -66,6 +55,15 @@ def mock_render_to_string(template_name, context): return str((template_name, sorted(iteritems(context)))) +def mock_render_to_response(template_name): + """ + Return an HttpResponse with content that encodes template_name and context + """ + # This simulates any db access in the templates. + UserProfile.objects.exists() + return HttpResponse(template_name) + + class CreateAccountMixin(object): def create_account(self, username, password, email): # pylint: disable=missing-docstring @@ -82,6 +80,7 @@ class CreateAccountMixin(object): @skip_unless_lms @ddt.ddt +@patch('student.views.management.render_to_response', Mock(side_effect=mock_render_to_response, autospec=True)) class TestAccountApi(UserSettingsEventTestMixin, EmailTemplateTagMixin, CreateAccountMixin, RetirementTestCase): """ These tests specifically cover the parts of the API methods that are not covered by test_views.py. @@ -457,6 +456,31 @@ class TestAccountApi(UserSettingsEventTestMixin, EmailTemplateTagMixin, CreateAc verify_event_emitted([{"code": "en"}, {"code": "fr"}], [{"code": "en"}, {"code": "fr"}]) verify_event_emitted([], [{"code": "en"}, {"code": "fr"}]) + def test_add_account_recovery(self): + test_email = "test@example.com" + pending_secondary_email_changes = PendingSecondaryEmailChange.objects.filter(user=self.user) + self.assertEqual(0, len(pending_secondary_email_changes)) + + account_recovery_objects = AccountRecovery.objects.filter(user=self.user) + self.assertEqual(0, len(account_recovery_objects)) + + with patch('crum.get_current_request', return_value=self.fake_request): + update = {"secondary_email": test_email} + update_account_settings(self.user, update) + + pending_secondary_email_change = PendingSecondaryEmailChange.objects.get(user=self.user) + self.assertIsNot(pending_secondary_email_change, None) + self.assertEqual(pending_secondary_email_change.new_secondary_email, test_email) + + activate_secondary_email(self.fake_request, pending_secondary_email_change.activation_key) + + pending_secondary_email_changes = PendingSecondaryEmailChange.objects.filter(user=self.user) + self.assertEqual(0, len(pending_secondary_email_changes)) + + account_recovery = AccountRecovery.objects.get(user=self.user) + self.assertIsNot(account_recovery, None) + self.assertEqual(account_recovery.secondary_email, test_email) + @patch('openedx.core.djangoapps.user_api.accounts.image_helpers._PROFILE_IMAGE_SIZES', [50, 10]) @patch.dict(