diff --git a/common/djangoapps/student/forms.py b/common/djangoapps/student/forms.py index 00c9af4193..a44b35fdab 100644 --- a/common/djangoapps/student/forms.py +++ b/common/djangoapps/student/forms.py @@ -21,10 +21,10 @@ from edx_ace import ace from edx_ace.recipient import Recipient from openedx.core.djangoapps.ace_common.template_context import get_base_template_context from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY -from openedx.core.djangolib.markup import HTML, Text from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.djangoapps.theming.helpers import get_current_site from openedx.core.djangoapps.user_api import accounts as accounts_settings +from openedx.core.djangoapps.user_api.accounts.utils import is_secondary_email_feature_enabled from openedx.core.djangoapps.user_api.preferences.api import get_user_preference from student.message_types import AccountRecovery as AccountRecoveryMessage, PasswordReset from student.models import AccountRecovery, CourseEnrollmentAllowed, email_exists_or_retired @@ -78,10 +78,10 @@ def send_account_recovery_email_for_user(user, request, email=None): message_context.update({ 'request': request, # Used by google_analytics_tracking_pixel 'platform_name': configuration_helpers.get_value('PLATFORM_NAME', settings.PLATFORM_NAME), - 'reset_link': '{protocol}://{site}{link}'.format( + 'reset_link': '{protocol}://{site}{link}?is_account_recovery=true'.format( protocol='https' if request.is_secure() else 'http', site=configuration_helpers.get_value('SITE_NAME', settings.SITE_NAME), - link=reverse('account_recovery_confirm', kwargs={ + link=reverse('password_reset_confirm', kwargs={ 'uidb36': int_to_base36(user.id), 'token': default_token_generator.make_token(user), }), @@ -104,6 +104,8 @@ class PasswordResetFormNoActive(PasswordResetForm): "address cannot reset the password."), } + is_account_recovery = True + def clean_email(self): """ This is a literal copy from Django 1.4.5's django.contrib.auth.forms.PasswordResetForm @@ -113,6 +115,14 @@ class PasswordResetFormNoActive(PasswordResetForm): email = self.cleaned_data["email"] #The line below contains the only change, removing is_active=True self.users_cache = User.objects.filter(email__iexact=email) + + if len(self.users_cache) == 0 and is_secondary_email_feature_enabled(): + # Check if user has entered the secondary email. + self.users_cache = User.objects.filter( + id__in=AccountRecovery.objects.filter(secondary_email__iexact=email, is_active=True).values_list('user') + ) + self.is_account_recovery = not bool(self.users_cache) + if not len(self.users_cache): raise forms.ValidationError(self.error_messages['unknown']) if any((user.password.startswith(UNUSABLE_PASSWORD_PREFIX)) @@ -130,57 +140,10 @@ class PasswordResetFormNoActive(PasswordResetForm): user. """ for user in self.users_cache: - send_password_reset_email_for_user(user, request) - - -class AccountRecoveryForm(PasswordResetFormNoActive): - error_messages = { - 'unknown': _( - HTML( - 'That secondary e-mail address doesn\'t have an associated user account. Are you sure you had added ' - 'a verified secondary email address for account recovery in your account settings? Please ' - 'contact support for further assistance.' - ) - ).format( - support_url=configuration_helpers.get_value('SUPPORT_SITE_LINK', settings.SUPPORT_SITE_LINK), - ), - 'unusable': _( - Text( - 'The user account associated with this secondary e-mail address cannot reset the password.' - ) - ), - } - - def clean_email(self): - """ - This is a literal copy from Django's django.contrib.auth.forms.PasswordResetForm - Except removing the requirement of active users - Validates that a user exists with the given secondary email. - """ - 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, is_active=True).values_list('user') - ) - - if not len(self.users_cache): - raise forms.ValidationError(self.error_messages['unknown']) - if any((user.password.startswith(UNUSABLE_PASSWORD_PREFIX)) - for user in self.users_cache): - raise forms.ValidationError(self.error_messages['unusable']) - return email - - def save(self, # pylint: disable=arguments-differ - use_https=False, - token_generator=default_token_generator, - request=None, - **_kwargs): - """ - Generates a one-use only link for setting the password and sends to the - user. - """ - for user in self.users_cache: - send_account_recovery_email_for_user(user, request, user.account_recovery.secondary_email) + if self.is_account_recovery: + send_password_reset_email_for_user(user, request) + else: + send_account_recovery_email_for_user(user, request, user.account_recovery.secondary_email) class TrueCheckbox(widgets.CheckboxInput): diff --git a/common/djangoapps/student/urls.py b/common/djangoapps/student/urls.py index bb5f237e73..ac4b6bfb2f 100644 --- a/common/djangoapps/student/urls.py +++ b/common/djangoapps/student/urls.py @@ -22,18 +22,12 @@ urlpatterns = [ # password reset in views (see below for password reset django views) url(r'^account/password$', views.password_change_request_handler, name='password_change_request'), - url(r'^account/account_recovery', views.account_recovery_request_handler, name='account_recovery'), url(r'^password_reset/$', views.password_reset, name='password_reset'), url( r'^password_reset_confirm/(?P[0-9A-Za-z]+)-(?P.+)/$', views.password_reset_confirm_wrapper, name='password_reset_confirm', ), - url( - r'^account_recovery_confirm/(?P[0-9A-Za-z]+)-(?P.+)/$', - views.account_recovery_confirm_wrapper, - name='account_recovery_confirm', - ), url(r'^course_run/{}/refund_status$'.format(settings.COURSE_ID_PATTERN), views.course_run_refund_status, diff --git a/common/djangoapps/student/views/management.py b/common/djangoapps/student/views/management.py index d33ffa84bd..5a30d3bf84 100644 --- a/common/djangoapps/student/views/management.py +++ b/common/djangoapps/student/views/management.py @@ -15,6 +15,7 @@ from django.contrib.auth.decorators import login_required from django.contrib.auth.models import AnonymousUser, User from django.contrib.auth.views import password_reset_confirm from django.contrib.sites.models import Site +from django.core.exceptions import ObjectDoesNotExist from django.core import mail from django.urls import reverse from django.core.validators import ValidationError, validate_email @@ -681,7 +682,7 @@ def password_change_request_handler(request): try: from openedx.core.djangoapps.user_api.accounts.api import request_password_change request_password_change(email, request.is_secure()) - user = user if user.is_authenticated else User.objects.get(email=email) + user = user if user.is_authenticated else get_user_from_email(email=email) destroy_oauth_tokens(user) except UserNotFound: AUDIT_LOG.info("Invalid password reset attempt") @@ -719,58 +720,26 @@ def password_change_request_handler(request): return HttpResponseBadRequest(_("No email address provided.")) -@require_http_methods(['POST']) -def account_recovery_request_handler(request): +def get_user_from_email(email): """ - Handle account recovery requests. + Find a user using given email and return it. Arguments: - request (HttpRequest) + email (str): primary or secondary email address of the user. + + Raises: + (User.ObjectNotFound): If no user is found with the given email. + (User.MultipleObjectsReturned): If more than one user is found with the given email. Returns: - HttpResponse: 200 if the email was sent successfully - HttpResponse: 400 if there is no 'email' POST parameter - HttpResponse: 403 if the client has been rate limited - HttpResponse: 405 if using an unsupported HTTP method - HttpResponse: 404 if account recovery feature is not enabled - - Example: - - POST /account/account_recovery - + User: Django user object. """ - if not is_secondary_email_feature_enabled(): - raise Http404 - - limiter = BadRequestRateLimiter() - if limiter.is_rate_limit_exceeded(request): - AUDIT_LOG.warning("Account recovery rate limit exceeded") - return HttpResponseForbidden() - - user = request.user - # Prefer logged-in user's email - email = request.POST.get('email') - - if email: - try: - # Send an email with a link to direct user towards account recovery. - from openedx.core.djangoapps.user_api.accounts.api import request_account_recovery - request_account_recovery(email, request.is_secure()) - - # 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_active(secondary_email__iexact=email).user.id - ) - destroy_oauth_tokens(user) - except UserNotFound: - AUDIT_LOG.warning( - "Account recovery attempt via invalid secondary email '{email}'.".format(email=email) - ) - limiter.tick_bad_request_counter(request) - - return HttpResponse(status=200) - else: - return HttpResponseBadRequest(_("No email address provided.")) + try: + return User.objects.get(email=email) + except ObjectDoesNotExist: + return User.objects.filter( + id__in=AccountRecovery.objects.filter(secondary_email__iexact=email, is_active=True).values_list('user') + ).get() @csrf_exempt @@ -841,6 +810,11 @@ def password_reset_confirm_wrapper(request, uidb36=None, token=None): platform_name = { "platform_name": configuration_helpers.get_value('platform_name', settings.PLATFORM_NAME) } + + # User can not get this link unless account recovery feature is enabled. + if 'is_account_recovery' in request.GET and not is_secondary_email_feature_enabled(): + raise Http404 + try: uid_int = base36_to_int(uidb36) user = User.objects.get(id=uid_int) @@ -909,9 +883,19 @@ def password_reset_confirm_wrapper(request, uidb36=None, token=None): # remember what the old password hash is before we call down old_password_hash = user.password - response = password_reset_confirm( - request, uidb64=uidb64, token=token, extra_context=platform_name - ) + if 'is_account_recovery' in request.GET: + response = password_reset_confirm( + request, + uidb64=uidb64, + token=token, + extra_context=platform_name, + template_name='registration/password_reset_confirm.html', + post_reset_redirect='signin_user', + ) + else: + response = password_reset_confirm( + request, uidb64=uidb64, token=token, extra_context=platform_name + ) # If password reset was unsuccessful a template response is returned (status_code 200). # Check if form is invalid then show an error to the user. @@ -930,113 +914,20 @@ def password_reset_confirm_wrapper(request, uidb36=None, token=None): # get the updated user updated_user = User.objects.get(id=uid_int) - else: - response = password_reset_confirm( - request, uidb64=uidb64, token=token, extra_context=platform_name - ) - - response_was_successful = response.context_data.get('validlink') - if response_was_successful and not user.is_active: - user.is_active = True - user.save() - - return response - - -def account_recovery_confirm_wrapper(request, uidb36=None, token=None): - """ - A wrapper around django.contrib.auth.views.password_reset_confirm. - Needed because we want to set the user as active at this step. - We also optionally do some additional password policy checks. - """ - # convert old-style base36-encoded user id to base64 - uidb64 = uidb36_to_uidb64(uidb36) - platform_name = { - "platform_name": configuration_helpers.get_value('platform_name', settings.PLATFORM_NAME) - } - - # User can not get this link unless secondary email feature is enabled. - if not is_secondary_email_feature_enabled(): - raise Http404 - - try: - uid_int = base36_to_int(uidb36) - user = User.objects.get(id=uid_int) - except (ValueError, User.DoesNotExist): - # if there's any error getting a user, just let django's - # password_reset_confirm function handle it. - - return password_reset_confirm( - request, - uidb64=uidb64, - token=token, - extra_context=platform_name, - template_name='account_recovery/password_create_confirm.html' - ) - - if request.method == 'POST': - # We have to make a copy of request.POST because it is a QueryDict object which is immutable until copied. - # We have to use request.POST because the password_reset_confirm method takes in the request and a user's - # password is set to the request.POST['new_password1'] field. We have to also normalize the new_password2 - # field so it passes the equivalence check that new_password1 == new_password2 - # In order to switch out of having to do this copy, we would want to move the normalize_password code into - # a custom User model's set_password method to ensure it is always happening upon calling set_password. - request.POST = request.POST.copy() - request.POST['new_password1'] = normalize_password(request.POST['new_password1']) - request.POST['new_password2'] = normalize_password(request.POST['new_password2']) - - password = request.POST['new_password1'] - - try: - validate_password(password, user=user) - except ValidationError as err: - # We have a password reset attempt which violates some security - # policy, or any other validation. Use the existing Django template to communicate that - # back to the user. - context = { - 'validlink': True, - 'form': None, - 'title': _('Password creation unsuccessful'), - 'err_msg': ' '.join(err.messages), - } - context.update(platform_name) - - return TemplateResponse( - request, 'account_recovery/password_create_confirm.html', context - ) - - # remember what the old password hash is before we call down - old_password_hash = user.password - - response = password_reset_confirm( - request, - uidb64=uidb64, - token=token, - extra_context=platform_name, - template_name='account_recovery/password_create_confirm.html', - post_reset_redirect='signin_user', - ) - - # If password reset was unsuccessful a template response is returned (status_code 200). - # Check if form is invalid then show an error to the user. - # Note if password reset was successful we get response redirect (status_code 302). - if response.status_code == 200: - form_valid = response.context_data['form'].is_valid() if response.context_data['form'] else False - if not form_valid: - log.warning( - u'Unable to create password for user [%s] because form is not valid. ' - u'A possible cause is that the user had an invalid create token', - user.username, + if 'is_account_recovery' in request.GET: + try: + updated_user.email = updated_user.account_recovery.secondary_email + updated_user.account_recovery.delete() + except ObjectDoesNotExist: + log.error( + 'Account recovery process initiated without AccountRecovery instance for user {username}'.format( + username=updated_user.username + ) ) - response.context_data['err_msg'] = _('Error in creating your password. Please try again.') - return response - # get the updated user - updated_user = User.objects.get(id=uid_int) - updated_user.email = updated_user.account_recovery.secondary_email updated_user.save() - if response.status_code == 302: + if response.status_code == 302 and 'is_account_recovery' in request.GET: messages.success( request, HTML(_( @@ -1054,11 +945,7 @@ def account_recovery_confirm_wrapper(request, uidb36=None, token=None): ) else: response = password_reset_confirm( - request, - uidb64=uidb64, - token=token, - extra_context=platform_name, - template_name='account_recovery/password_create_confirm.html', + request, uidb64=uidb64, token=token, extra_context=platform_name ) response_was_successful = response.context_data.get('validlink') diff --git a/lms/static/js/spec/student_account/account_recovery_spec.js b/lms/static/js/spec/student_account/account_recovery_spec.js deleted file mode 100644 index 32c82a9cfa..0000000000 --- a/lms/static/js/spec/student_account/account_recovery_spec.js +++ /dev/null @@ -1,152 +0,0 @@ -(function(define) { - 'use strict'; - define([ - 'jquery', - 'underscore', - 'common/js/spec_helpers/template_helpers', - 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers', - 'js/student_account/models/AccountRecoveryModel', - 'js/student_account/views/AccountRecoveryView' - ], - function($, _, TemplateHelpers, AjaxHelpers, AccountRecoveryModel, AccountRecoveryView) { - describe('edx.student.account.AccountRecoveryView', function() { - var model = null, - view = null, - requests = null, - EMAIL = 'xsy@edx.org', - FORM_DESCRIPTION = { - method: 'post', - submit_url: '/account/password', - fields: [{ - name: 'email', - label: 'Secondary email', - defaultValue: '', - type: 'text', - required: true, - placeholder: 'place@holder.org', - instructions: 'Enter your secondary email.', - restrictions: {} - }] - }; - - var createAccountRecoveryView = function(that) { - // Initialize the account recovery model - model = new AccountRecoveryModel({}, { - url: FORM_DESCRIPTION.submit_url, - method: FORM_DESCRIPTION.method - }); - - // Initialize the account recovery view - view = new AccountRecoveryView({ - fields: FORM_DESCRIPTION.fields, - model: model - }); - - // Spy on AJAX requests - requests = AjaxHelpers.requests(that); - }; - - var submitEmail = function(validationSuccess) { - // Create a fake click event - var clickEvent = $.Event('click'); - - // Simulate manual entry of an email address - $('#password-reset-email').val(EMAIL); - - // If validationSuccess isn't passed, we avoid - // spying on `view.validate` twice - if (!_.isUndefined(validationSuccess)) { - // Force validation to return as expected - spyOn(view, 'validate').and.returnValue({ - isValid: validationSuccess, - message: 'Submission was validated.' - }); - } - - // Submit the email address - view.submitForm(clickEvent); - }; - - beforeEach(function() { - setFixtures(''); - TemplateHelpers.installTemplate('templates/student_account/account_recovery'); - TemplateHelpers.installTemplate('templates/student_account/form_field'); - }); - - it('allows the user to request account recovery', function() { - var syncSpy, passwordEmailSentSpy; - - createAccountRecoveryView(this); - - // We expect these events to be triggered upon a successful account recovery - syncSpy = jasmine.createSpy('syncEvent'); - passwordEmailSentSpy = jasmine.createSpy('passwordEmailSentEvent'); - view.listenTo(view.model, 'sync', syncSpy); - view.listenTo(view, 'account-recovery-email-sent', passwordEmailSentSpy); - - // Submit the form, with successful validation - submitEmail(true); - - // Verify that the client contacts the server with the expected data - AjaxHelpers.expectRequest( - requests, 'POST', - FORM_DESCRIPTION.submit_url, - $.param({email: EMAIL}) - ); - - // Respond with status code 200 - AjaxHelpers.respondWithJson(requests, {}); - - // Verify that the events were triggered - expect(syncSpy).toHaveBeenCalled(); - expect(passwordEmailSentSpy).toHaveBeenCalled(); - - // Verify that account recovery view has been removed - expect($(view.el).html().length).toEqual(0); - }); - - it('validates the email field', function() { - createAccountRecoveryView(this); - - // Submit the form, with successful validation - submitEmail(true); - - // Verify that validation of the email field occurred - expect(view.validate).toHaveBeenCalledWith($('#password-reset-email')[0]); - - // Verify that no submission errors are visible - expect(view.$formFeedback.find('.' + view.formErrorsJsHook).length).toEqual(0); - }); - - it('displays account recovery validation errors', function() { - createAccountRecoveryView(this); - - // Submit the form, with failed validation - submitEmail(false); - - // Verify that submission errors are visible - expect(view.$formFeedback.find('.' + view.formErrorsJsHook).length).toEqual(1); - }); - - it('displays error if the server returns an error while sending account recovery email', function() { - createAccountRecoveryView(this); - submitEmail(true); - - // Simulate an error from the LMS servers - AjaxHelpers.respondWithError(requests); - - // Expect that an error is displayed - expect(view.$formFeedback.find('.' + view.formErrorsJsHook).length).toEqual(1); - - // If we try again and succeed, the error should go away - submitEmail(); - - // This time, respond with status code 200 - AjaxHelpers.respondWithJson(requests, {}); - - // Expect that the error is hidden - expect(view.$formFeedback.find('.' + view.formErrorsJsHook).length).toEqual(0); - }); - }); - }); -}).call(this, define || RequireJS.define); diff --git a/lms/static/js/student_account/views/AccessView.js b/lms/static/js/student_account/views/AccessView.js index 3b8d8398e0..0aa0a3df16 100644 --- a/lms/static/js/student_account/views/AccessView.js +++ b/lms/static/js/student_account/views/AccessView.js @@ -15,13 +15,11 @@ 'js/student_account/views/RegisterView', 'js/student_account/views/InstitutionLoginView', 'js/student_account/views/HintedLoginView', - 'js/student_account/views/AccountRecoveryView', 'edx-ui-toolkit/js/utils/html-utils', 'js/vendor/history' ], function($, utility, _, _s, Backbone, LoginModel, PasswordResetModel, RegisterModel, AccountRecoveryModel, - LoginView, PasswordResetView, RegisterView, InstitutionLoginView, HintedLoginView, AccountRecoveryView, - HtmlUtils) { + LoginView, PasswordResetView, RegisterView, InstitutionLoginView, HintedLoginView, HtmlUtils) { return Backbone.View.extend({ tpl: '#access-tpl', events: { @@ -69,7 +67,6 @@ login: options.login_form_desc, register: options.registration_form_desc, reset: options.password_reset_form_desc, - account_recovery: options.account_recovery_form_desc, institution_login: null, hinted_login: null }; @@ -125,9 +122,6 @@ if (Backbone.history.getHash() === 'forgot-password-modal') { this.resetPassword(); } - else if (Backbone.history.getHash() === 'account-recovery-modal') { - this.accountRecovery(); - } this.loadForm(this.activeForm); }, @@ -163,9 +157,6 @@ // Listen for 'password-help' event to toggle sub-views this.listenTo(this.subview.login, 'password-help', this.resetPassword); - // Listen for 'account-recovery-help' event to toggle sub-views - this.listenTo(this.subview.login, 'account-recovery-help', this.accountRecovery); - // Listen for 'auth-complete' event so we can enroll/redirect the user appropriately. this.listenTo(this.subview.login, 'auth-complete', this.authComplete); }, @@ -186,24 +177,6 @@ $('.password-reset-form').focus(); }, - account_recovery: function(data) { - this.accountRecoveryModel.ajaxType = data.method; - this.accountRecoveryModel.urlRoot = data.submit_url; - - this.subview.accountRecoveryHelp = new AccountRecoveryView({ - fields: data.fields, - model: this.accountRecoveryModel - }); - - // Listen for 'account-recovery-email-sent' event to toggle sub-views - this.listenTo( - this.subview.accountRecoveryHelp, 'account-recovery-email-sent', this.passwordEmailSent - ); - - // Focus on the form - $('.password-reset-form').focus(); - }, - register: function(data) { var model = new RegisterModel({}, { method: data.method, @@ -260,19 +233,6 @@ this.element.scrollTop($('#password-reset-anchor')); }, - accountRecovery: function() { - if (this.isAccountRecoveryFeatureEnabled) { - window.analytics.track('edx.bi.account_recovery.viewed', { - category: 'user-engagement' - }); - - this.element.hide($(this.el).find('#login-anchor')); - this.loadForm('account_recovery'); - this.element.scrollTop($('#password-reset-anchor')); - } - - }, - toggleForm: function(e) { var type = $(e.currentTarget).data('type'), $form = $('#' + type + '-form'), diff --git a/lms/static/js/student_account/views/AccountRecoveryView.js b/lms/static/js/student_account/views/AccountRecoveryView.js deleted file mode 100644 index 5fa974dbad..0000000000 --- a/lms/static/js/student_account/views/AccountRecoveryView.js +++ /dev/null @@ -1,39 +0,0 @@ -(function(define) { - 'use strict'; - define([ - 'jquery', - 'js/student_account/views/FormView' - ], - function($, FormView) { - return FormView.extend({ - el: '#password-reset-form', - - tpl: '#account_recovery-tpl', - - events: { - 'click .js-reset': 'submitForm' - }, - - formType: 'password-reset', - - requiredStr: '', - optionalStr: '', - - submitButton: '.js-reset', - - preRender: function() { - this.element.show($(this.el)); - this.element.show($(this.el).parent()); - this.listenTo(this.model, 'sync', this.saveSuccess); - }, - - saveSuccess: function() { - this.trigger('account-recovery-email-sent'); - - // Destroy the view (but not el) and unbind events - this.$el.empty().off(); - this.stopListening(); - } - }); - }); -}).call(this, define || RequireJS.define); diff --git a/lms/static/js/student_account/views/FormView.js b/lms/static/js/student_account/views/FormView.js index e82e8d4c2a..dd136da322 100644 --- a/lms/static/js/student_account/views/FormView.js +++ b/lms/static/js/student_account/views/FormView.js @@ -139,12 +139,6 @@ this.trigger('password-help'); }, - accountRecovery: function(event) { - event.preventDefault(); - - this.trigger('account-recovery-help'); - }, - getFormData: function() { var obj = {}, $form = this.$form, diff --git a/lms/static/js/student_account/views/LoginView.js b/lms/static/js/student_account/views/LoginView.js index e93b86271d..f385685992 100644 --- a/lms/static/js/student_account/views/LoginView.js +++ b/lms/static/js/student_account/views/LoginView.js @@ -23,7 +23,6 @@ events: { 'click .js-login': 'submitForm', 'click .forgot-password': 'forgotPassword', - 'click .account-recovery': 'accountRecovery', 'click .login-provider': 'thirdPartyAuth' }, formType: 'login', @@ -137,13 +136,6 @@ this.clearPasswordResetSuccess(); }, - accountRecovery: function(event) { - event.preventDefault(); - - this.trigger('account-recovery-help'); - this.clearPasswordResetSuccess(); - }, - postFormSubmission: function() { this.clearPasswordResetSuccess(); }, @@ -152,7 +144,7 @@ var email = $('#password-reset-email').val(), successTitle = gettext('Check Your Email'), successMessageHtml = HtmlUtils.interpolateHtml( - gettext('{paragraphStart}You entered {boldStart}{email}{boldEnd}. If this email address is associated with your {platform_name} account, we will send a message with password reset instructions to this email address.{paragraphEnd}' + // eslint-disable-line max-len + gettext('{paragraphStart}You entered {boldStart}{email}{boldEnd}. If this email address is associated with your {platform_name} account, we will send a message with password recovery instructions to this email address.{paragraphEnd}' + // eslint-disable-line max-len '{paragraphStart}If you do not receive a password reset message, verify that you entered the correct email address, or check your spam folder.{paragraphEnd}' + // eslint-disable-line max-len '{paragraphStart}If you need further assistance, {anchorStart}contact technical support{anchorEnd}.{paragraphEnd}'), { // eslint-disable-line max-len boldStart: HtmlUtils.HTML(''), diff --git a/lms/static/lms/js/spec/main.js b/lms/static/lms/js/spec/main.js index bf9a337837..787591b083 100644 --- a/lms/static/lms/js/spec/main.js +++ b/lms/static/lms/js/spec/main.js @@ -787,7 +787,6 @@ 'js/spec/student_account/login_spec.js', 'js/spec/student_account/logistration_factory_spec.js', 'js/spec/student_account/password_reset_spec.js', - 'js/spec/student_account/account_recovery_spec.js', 'js/spec/student_account/register_spec.js', 'js/spec/student_account/shoppingcart_spec.js', 'js/spec/verify_student/image_input_spec.js', diff --git a/lms/templates/account_recovery/password_create_confirm.html b/lms/templates/account_recovery/password_create_confirm.html deleted file mode 100644 index 1f031fff18..0000000000 --- a/lms/templates/account_recovery/password_create_confirm.html +++ /dev/null @@ -1,56 +0,0 @@ -## mako - -<%page expression_filter="h"/> - -<%! -from django.utils.translation import ugettext as _ -from openedx.core.djangolib.js_utils import js_escaped_string -from openedx.core.djangolib.markup import HTML, Text -%> - -<%inherit file="../main.html"/> -<%namespace name='static' file='../static_content.html'/> - -<%block name="title"> - ${_("Create Your {platform_name} Password").format(platform_name=platform_name)} - - -<%block name="head_extra"> - - - -<%block name="bodyclass">view-passwordreset - -<%block name="body"> - - diff --git a/lms/templates/student_account/account_recovery.underscore b/lms/templates/student_account/account_recovery.underscore deleted file mode 100644 index d4ad0739af..0000000000 --- a/lms/templates/student_account/account_recovery.underscore +++ /dev/null @@ -1,13 +0,0 @@ - - -

<%- gettext("Account Recovery") %>

- - diff --git a/lms/templates/student_account/form_field.underscore b/lms/templates/student_account/form_field.underscore index 4fd26e7eb7..e313a7be96 100644 --- a/lms/templates/student_account/form_field.underscore +++ b/lms/templates/student_account/form_field.underscore @@ -133,6 +133,6 @@ <% } %> <% if( form === 'login' && name === 'password' ) { %> - + <% } %> diff --git a/lms/templates/student_account/login_and_register.html b/lms/templates/student_account/login_and_register.html index 25a6353b5c..41a3ab6e12 100644 --- a/lms/templates/student_account/login_and_register.html +++ b/lms/templates/student_account/login_and_register.html @@ -30,7 +30,7 @@ <%block name="header_extras"> - % for template_name in ["account", "access", "form_field", "login", "register", "institution_login", "institution_register", "password_reset", "account_recovery", "hinted_login"]: + % for template_name in ["account", "access", "form_field", "login", "register", "institution_login", "institution_register", "password_reset", "hinted_login"]: diff --git a/lms/templates/student_account/password_reset.underscore b/lms/templates/student_account/password_reset.underscore index 85e418df9b..bdfb2b4153 100644 --- a/lms/templates/student_account/password_reset.underscore +++ b/lms/templates/student_account/password_reset.underscore @@ -5,9 +5,9 @@
-

<%- gettext("Please enter your email address below and we will send you instructions for setting a new password.") %>

+

<%- gettext("Please enter your registration or recovery email address below and we will send you an email with instructions.") %>

- <%= fields %> + <%= HtmlUtils.HTML(fields) %> - +
diff --git a/openedx/core/djangoapps/user_api/accounts/api.py b/openedx/core/djangoapps/user_api/accounts/api.py index 296e94d033..d1c09f312c 100644 --- a/openedx/core/djangoapps/user_api/accounts/api.py +++ b/openedx/core/djangoapps/user_api/accounts/api.py @@ -486,36 +486,6 @@ def request_password_change(email, is_secure): raise errors.UserNotFound -@helpers.intercept_errors(errors.UserAPIInternalError, ignore_errors=[errors.UserAPIRequestError]) -def request_account_recovery(email, is_secure): - """ - Email a single-use link for performing a password reset so users can login with new email and password. - - Arguments: - email (str): An email address - is_secure (bool): Whether the request was made with HTTPS - - Raises: - errors.UserNotFound: Raised if secondary email address does not exist. - """ - # Binding data to a form requires that the data be passed as a dictionary - # to the Form class constructor. - form = student_forms.AccountRecoveryForm({'email': email}) - - # Validate that a user exists with the given email address. - if form.is_valid(): - # Generate a single-use link for performing a password reset - # and email it to the user. - form.save( - from_email=configuration_helpers.get_value('email_from_address', settings.DEFAULT_FROM_EMAIL), - use_https=is_secure, - request=get_current_request(), - ) - else: - # No user with the provided email address exists. - raise errors.UserNotFound - - def get_name_validation_error(name): """Get the built-in validation error message for when the user's real name is invalid in some way (we wonder how). diff --git a/openedx/core/djangoapps/user_api/api.py b/openedx/core/djangoapps/user_api/api.py index 541826aac9..eb3be3cf50 100644 --- a/openedx/core/djangoapps/user_api/api.py +++ b/openedx/core/djangoapps/user_api/api.py @@ -66,54 +66,6 @@ def get_password_reset_form(): return form_desc -def get_account_recovery_form(): - """ - Return a description of the password reset, using secondary email, form. - - This decouples clients from the API definition: - if the API decides to modify the form, clients won't need - to be updated. - - See `user_api.helpers.FormDescription` for examples - of the JSON-encoded form description. - - Returns: - HttpResponse - - """ - form_desc = FormDescription("post", reverse("account_recovery")) - - # Translators: This label appears above a field on the password reset - # form meant to hold the user's email address. - email_label = _(u"Secondary email") - - # Translators: This example email address is used as a placeholder in - # a field on the password reset form meant to hold the user's email address. - email_placeholder = _(u"username@domain.com") - - # Translators: These instructions appear on the password reset form, - # immediately below a field meant to hold the user's email address. - email_instructions = _( - u"Secondary email address you registered with {platform_name} using account settings page" - ).format( - platform_name=configuration_helpers.get_value('PLATFORM_NAME', settings.PLATFORM_NAME) - ) - - form_desc.add_field( - "email", - field_type="email", - label=email_label, - placeholder=email_placeholder, - instructions=email_instructions, - restrictions={ - "min_length": accounts.EMAIL_MIN_LENGTH, - "max_length": accounts.EMAIL_MAX_LENGTH, - } - ) - - return form_desc - - def get_login_session_form(request): """Return a description of the login form. diff --git a/openedx/core/djangoapps/user_authn/views/login_form.py b/openedx/core/djangoapps/user_authn/views/login_form.py index 26c1b5d6e0..4f25340e55 100644 --- a/openedx/core/djangoapps/user_authn/views/login_form.py +++ b/openedx/core/djangoapps/user_authn/views/login_form.py @@ -22,7 +22,6 @@ from openedx.core.djangoapps.theming.helpers import is_request_in_themed_site from openedx.core.djangoapps.user_api.accounts.utils import is_secondary_email_feature_enabled from openedx.core.djangoapps.user_api.api import ( RegistrationFormFactory, - get_account_recovery_form, get_login_session_form, get_password_reset_form, ) @@ -138,7 +137,6 @@ def login_and_registration_form(request, initial_mode="login"): 'login_form_desc': json.loads(form_descriptions['login']), 'registration_form_desc': json.loads(form_descriptions['registration']), 'password_reset_form_desc': json.loads(form_descriptions['password_reset']), - 'account_recovery_form_desc': json.loads(form_descriptions['account_recovery']), 'account_creation_allowed': configuration_helpers.get_value( 'ALLOW_PUBLIC_ACCOUNT_CREATION', settings.FEATURES.get('ALLOW_PUBLIC_ACCOUNT_CREATION', True)), 'is_account_recovery_feature_enabled': is_secondary_email_feature_enabled() @@ -177,7 +175,6 @@ def _get_form_descriptions(request): return { 'password_reset': get_password_reset_form().to_json(), - 'account_recovery': get_account_recovery_form().to_json(), 'login': get_login_session_form(request).to_json(), 'registration': RegistrationFormFactory().get_registration_form(request).to_json() } 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 5ae81ffe2b..4e84c664a8 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_views.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_views.py @@ -17,7 +17,6 @@ from django.contrib.messages.middleware import MessageMiddleware from django.contrib.sessions.middleware import SessionMiddleware from django.core import mail from django.core.files.uploadedfile import SimpleUploadedFile -from django.http import Http404 from django.urls import reverse from django.test import TestCase from django.test.client import RequestFactory @@ -30,7 +29,6 @@ from provider.oauth2.models import AccessToken as dop_access_token from provider.oauth2.models import RefreshToken as dop_refresh_token from testfixtures import LogCapture from waffle.models import Switch -from waffle.testutils import override_switch from course_modes.models import CourseMode from openedx.core.djangoapps.user_authn.views.login_form import login_and_registration_form @@ -261,83 +259,6 @@ class UserAccountUpdateTest(CacheIsolationTestCase, UrlResetMixin): response = getattr(self.client, method)(url) self.assertEqual(response.status_code, 405) - @override_switch(ENABLE_SECONDARY_EMAIL_FEATURE_SWITCH, active=False) - def test_404_if_account_recovery_not_enabled(self): - with mock.patch('openedx.core.djangoapps.user_api.accounts.api.request_account_recovery', - side_effect=UserAPIInternalError): - self._recover_account() - self.assertRaises(Http404) - - def test_account_recovery_failure(self): - with mock.patch('openedx.core.djangoapps.user_api.accounts.api.request_account_recovery', - side_effect=UserAPIInternalError): - self._recover_account() - self.assertRaises(UserAPIInternalError) - - @override_settings(FEATURES=FEATURES_WITH_FAILED_PASSWORD_RESET_EMAIL) - def test_account_recovery_failure_email(self): - """Test that log message is added when email does not match any in the system.""" - # Log the user out - self.client.logout() - - with LogCapture(LOGGER_NAME, level=logging.INFO) as logger: - bad_email = 'doesnotexist@example.com' - response = self._recover_account(email=bad_email) - self.assertEqual(response.status_code, 200) - logger.check( - ( - LOGGER_NAME, - "WARNING", "Account recovery attempt via invalid secondary email '{email}'.".format( - email=bad_email - ) - ) - ) - - @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 - # in the POST data - self.client.logout() - - # Make many consecutive bad requests in an attempt to trigger the rate limiter - for __ in xrange(self.INVALID_ATTEMPTS): - self._recover_account(email=self.NEW_EMAIL) - - response = self._recover_account(email=self.NEW_EMAIL) - self.assertEqual(response.status_code, 403) - - @ddt.data( - ('post', 'account_recovery', []), - ) - @ddt.unpack - def test_require_http_method_during_account_recovery(self, correct_method, url_name, args): - wrong_methods = {'get', 'put', 'post', 'head', 'options', 'delete'} - {correct_method} - url = reverse(url_name, args=args) - - for method in wrong_methods: - response = getattr(self.client, method)(url) - self.assertEqual(response.status_code, 405) - def _change_password(self, email=None): """Request to change the user's password. """ data = {} @@ -347,15 +268,6 @@ class UserAccountUpdateTest(CacheIsolationTestCase, UrlResetMixin): return self.client.post(path=reverse('password_change_request'), data=data) - def _recover_account(self, email=None): - """Request to create the user's password. """ - data = {} - - if email: - data['email'] = email - - return self.client.post(path=reverse('account_recovery'), data=data) - def _create_dop_tokens(self, user=None): """Create dop access token for given user if user provided else for default user.""" if not user: