From e9276ba246025d92a558d10d7590c90c2f352acc Mon Sep 17 00:00:00 2001 From: Troy Sankey Date: Fri, 4 May 2018 13:36:52 -0400 Subject: [PATCH] Disallow registration when the proposed email is half-retired Our learner retirement implementation shall allow re-use of email addresses, but we currently do not disallow re-use of emails for learners whose retirement is still in-progress (i.e. their retirement state is between PENDING and LMS_COMPLETE inclusive). The time between a user initiating retirement, and the jenkins job actually picking up the user and driving their account retirement might be as long as 1 hour, so this is a serious concern. Addresses EDUCATOR-2824. --- common/djangoapps/student/forms.py | 3 ++- common/djangoapps/student/views/management.py | 3 ++- lms/djangoapps/instructor/views/api.py | 16 +++++++++++ .../core/djangoapps/user_api/accounts/api.py | 3 ++- .../djangoapps/user_api/accounts/utils.py | 27 +++++++++++++++++++ 5 files changed, 49 insertions(+), 3 deletions(-) diff --git a/common/djangoapps/student/forms.py b/common/djangoapps/student/forms.py index ae12931518..68a6618667 100644 --- a/common/djangoapps/student/forms.py +++ b/common/djangoapps/student/forms.py @@ -19,6 +19,7 @@ from django.core.validators import RegexValidator, slug_re from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.djangoapps.user_api import accounts as accounts_settings +from openedx.core.djangoapps.user_api.accounts.utils import email_exists from student.models import CourseEnrollmentAllowed from util.password_policy_validators import password_max_length, password_min_length, validate_password @@ -294,7 +295,7 @@ class AccountCreationForm(forms.Form): # reject the registration. if not CourseEnrollmentAllowed.objects.filter(email=email).exists(): raise ValidationError(_("Unauthorized email address.")) - if User.objects.filter(email__iexact=email).exists(): + if email_exists(email): raise ValidationError( _( "It looks like {email} belongs to an existing account. Try again with a different email address." diff --git a/common/djangoapps/student/views/management.py b/common/djangoapps/student/views/management.py index 41e4a44b3e..1d899007ee 100644 --- a/common/djangoapps/student/views/management.py +++ b/common/djangoapps/student/views/management.py @@ -95,6 +95,7 @@ from student.models import ( UserSignupSource, UserStanding, create_comments_service_user, + username_or_email_exists_or_retired, ) from student.signals import REFUND_ORDER from student.tasks import send_activation_email @@ -1251,7 +1252,7 @@ def validate_new_email(user, new_email): if new_email == user.email: raise ValueError(_('Old email is the same as the new email.')) - if User.objects.filter(email=new_email).count() != 0: + if username_or_email_exists_or_retired(new_email): raise ValueError(_('An account with this e-mail already exists.')) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 9e9337e7a6..329195a0f1 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -16,6 +16,7 @@ import StringIO import time import unicodecsv +from django.apps import apps from django.conf import settings from django.contrib.auth.models import User from django.core.exceptions import ObjectDoesNotExist, PermissionDenied, ValidationError @@ -376,6 +377,7 @@ def register_and_enroll_students(request, course_id): # pylint: disable=too-man row_errors.append({ 'username': username, 'email': email, 'response': _('Invalid email {email_address}.').format(email_address=email)}) else: + UserRetirementStatus = apps.get_model('user_api', 'UserRetirementStatus') if User.objects.filter(email=email).exists(): # Email address already exists. assume it is the correct user # and just register the user in the course and send an enrollment email. @@ -412,6 +414,20 @@ def register_and_enroll_students(request, course_id): # pylint: disable=too-man state_transition=UNENROLLED_TO_ENROLLED, ) enroll_email(course_id=course_id, student_email=email, auto_enroll=True, email_students=True, email_params=email_params) + elif (UserRetirementStatus.objects + .select_related('current_state') + .filter(original_email=email) + .exclude(current_state__state_name='COMPLETE') + .exists()): + # Somebody is attempting to enroll a user who has initiated retirement but still exists in + # general. Simply block these attempts. + general_errors.append({ + 'username': '', + 'email': '', + 'response': _('Invalid email {email_address}.').format(email_address=email), + }) + log.warning(u'Email address %s is associated with a user which has initiated account retirement, ' + + u'so course enrollment was blocked.', email) else: # This email does not yet exist, so we need to create a new account # If username already exists in the database, then create_and_enroll_user diff --git a/openedx/core/djangoapps/user_api/accounts/api.py b/openedx/core/djangoapps/user_api/accounts/api.py index 3520eeacff..833d34916f 100644 --- a/openedx/core/djangoapps/user_api/accounts/api.py +++ b/openedx/core/djangoapps/user_api/accounts/api.py @@ -21,6 +21,7 @@ from util.password_policy_validators import validate_password from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.djangoapps.user_api import errors, accounts, forms, helpers +from openedx.core.djangoapps.user_api.accounts.utils import email_exists 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, @@ -692,7 +693,7 @@ def _validate_email_doesnt_exist(email): :return: None :raises: errors.AccountEmailAlreadyExists """ - if email is not None and User.objects.filter(email=email).exists(): + if email is not None and email_exists(email): raise errors.AccountEmailAlreadyExists(_(accounts.EMAIL_CONFLICT_MSG).format(email_address=email)) diff --git a/openedx/core/djangoapps/user_api/accounts/utils.py b/openedx/core/djangoapps/user_api/accounts/utils.py index 2967863861..2c0e08d219 100644 --- a/openedx/core/djangoapps/user_api/accounts/utils.py +++ b/openedx/core/djangoapps/user_api/accounts/utils.py @@ -6,6 +6,7 @@ import re import string from urlparse import urlparse +from django.apps import apps from django.conf import settings from django.contrib.auth.models import User from django.utils.translation import ugettext as _ @@ -192,3 +193,29 @@ 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 email_exists(email): + """ + Check an email against the User and UserRetirementStatus models for + existence. + """ + exists = False + # Normal case, check users in the auth_user table. + if User.objects.filter(email=email).exists(): + exists = True + else: + # Handle case where another user with the same email address has + # initiated retirement (account deletion), but they are still in + # the retirement queue in any state which is not COMPLETE. + UserRetirementStatus = apps.get_model('user_api', 'UserRetirementStatus') + try: + if (UserRetirementStatus.objects + .select_related('current_state') + .filter(original_email=email) + .exclude(current_state__state_name='COMPLETE') + .exists()): + exists = True + except UserRetirementStatus.DoesNotExist: + pass + return exists