Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fixup! pivot to treating retired emails as banned forever

Browse Source
This commit is contained in:
Troy Sankey
2018-05-10 15:15:14 -04:00
parent e9276ba246
commit 232b359258
7 changed files with 122 additions and 44 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
common/djangoapps/student/models.py
View File

@@ -215,6 +215,19 @@ def is_username_retired(username):
return User.objects.filter(username__in=list(locally_hashed_usernames)).exists()
def is_email_retired(email):
"""
Checks to see if the given email has been previously retired
"""
locally_hashed_emails = user_util.get_all_retired_emails(
email,
settings.RETIRED_USER_SALTS,
settings.RETIRED_EMAIL_FMT
)
return User.objects.filter(email__in=list(locally_hashed_emails)).exists()
def get_retired_username_by_username(username):
"""
If a UserRetirementStatus object with an original_username matching the given username exists,

12
common/djangoapps/student/signals/receivers.py
View File

@@ -11,7 +11,10 @@ from student.helpers import (
AccountValidationError,
USERNAME_EXISTS_MSG_FMT
)
from student.models import is_username_retired
from student.models import (
is_email_retired,
is_username_retired,
)
def update_last_login(sender, user, **kwargs): # pylint: disable=unused-argument
@@ -46,3 +49,10 @@ def on_user_updated(sender, instance, **kwargs): # pylint: disable=unused-argum
USERNAME_EXISTS_MSG_FMT.format(username=instance.username),
field="username"
)
# Check for a retired email.
if is_email_retired(instance.email):
raise AccountValidationError(
EMAIL_EXISTS_MSG_FMT.format(username=instance.email),
field="email"
)

17
common/djangoapps/student/tests/test_email.py
View File

@@ -18,7 +18,12 @@ from openedx.core.djangoapps.site_configuration import helpers as configuration_
from openedx.core.djangoapps.theming.tests.test_util import with_comprehensive_theme
from openedx.core.djangoapps.user_api.config.waffle import PREVENT_AUTH_USER_WRITES, SYSTEM_MAINTENANCE_MSG, waffle
from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, CacheIsolationMixin
from student.models import PendingEmailChange, Registration, UserProfile
from student.models import (
PendingEmailChange,
Registration,
UserProfile,
get_retired_email_by_email
)
from student.tests.factories import PendingEmailChangeFactory, RegistrationFactory, UserFactory
from student.views import (
SETTING_CHANGE_INITIATED,
@@ -325,6 +330,16 @@ class EmailChangeRequestTests(EventTestMixin, CacheIsolationTestCase):
UserFactory.create(email=self.new_email)
self.assertEqual(self.do_email_validation(self.new_email), 'An account with this e-mail already exists.')
def test_retired_email(self):
"""
Assert the expected error message from the email validation method for an email address
that corresponds with an already-retired account.
"""
user = UserFactory.create(email=self.new_email)
user.email = get_retired_email_by_email(self.new_email)
user.save()
self.assertEqual(self.do_email_validation(self.new_email), 'An account with this e-mail already exists.')
@patch('django.core.mail.send_mail')
@patch('student.views.management.render_to_string', Mock(side_effect=mock_render_to_string, autospec=True))
def test_email_failure(self, send_mail):

26
common/djangoapps/student/tests/test_retirement.py
View File

@@ -17,7 +17,8 @@ from student.models import (
get_potentially_retired_user_by_username_and_hash,
get_retired_email_by_email,
get_retired_username_by_username,
is_username_retired
is_username_retired,
is_email_retired
)
from student.tests.factories import UserFactory
@@ -132,6 +133,29 @@ def test_is_username_retired_not_retired():
assert not is_username_retired(user.username)
def test_is_email_retired_is_retired():
"""
Check functionality of is_email_retired when email is retired
"""
user = UserFactory()
original_email = user.email
retired_email = get_retired_email_by_email(user.email)
# Fake email retirement.
user.email = retired_email
user.save()
assert is_email_retired(original_email)
def test_is_email_retired_not_retired():
"""
Check functionality of is_email_retired when email is not retired
"""
user = UserFactory()
assert not is_email_retired(user.email)
def test_get_retired_email():
"""
Basic testing of retired emails.

44
lms/djangoapps/instructor/tests/test_api.py
View File

@@ -86,7 +86,9 @@ from student.models import (
CourseEnrollment,
CourseEnrollmentAllowed,
ManualEnrollmentAudit,
NonExistentCourseError
NonExistentCourseError,
get_retired_email_by_email,
get_retired_username_by_username
)
from student.roles import CourseBetaTesterRole, CourseFinanceAdminRole, CourseInstructorRole, CourseSalesAdminRole
from student.tests.factories import AdminFactory, UserFactory
@@ -842,6 +844,28 @@ class TestInstructorAPIBulkAccountCreationAndEnrollment(SharedModuleStoreTestCas
self.assertEqual(manual_enrollments.count(), 1)
self.assertTrue(manual_enrollments[0].state_transition, UNENROLLED_TO_ENROLLED)
def test_user_with_retired_email_in_csv(self):
# prep a retired user
user = UserFactory.create(username='old_test_student', email='test_student@example.com')
user.email = get_retired_email_by_email(user.email)
user.username = get_retired_username_by_username(user.username)
user.is_active = False
user.save()
csv_content = "test_student@example.com,test_student,tester,USA" \
uploaded_file = SimpleUploadedFile("temp.csv", csv_content)
response = self.client.post(self.url, {'students_list': uploaded_file})
self.assertEqual(response.status_code, 200)
data = json.loads(response.content)
self.assertNotEquals(len(data['row_errors']), 0)
self.assertEquals(
data['row_errors'][0]['response'],
'Invalid email {email}.'.format(email='test_student@example.com')
)
self.assertFalse(User.objects.filter(email='test_student@example.com').exists())
def test_user_with_already_existing_username_in_csv(self):
"""
If the username already exists (but not the email),
@@ -907,8 +931,16 @@ class TestInstructorAPIBulkAccountCreationAndEnrollment(SharedModuleStoreTestCas
def test_users_created_and_enrolled_successfully_if_others_fail(self):
# prep a retired user
user = UserFactory.create(username='old_test_student_4', email='test_student4@example.com')
user.email = get_retired_email_by_email(user.email)
user.username = get_retired_username_by_username(user.username)
user.is_active = False
user.save()
csv_content = "test_student1@example.com,test_student_1,tester1,USA\n" \
"test_student3@example.com,test_student_1,tester3,CA\n" \
"test_student4@example.com,test_student_4,tester4,USA\n" \
"test_student2@example.com,test_student_2,tester2,USA"
uploaded_file = SimpleUploadedFile("temp.csv", csv_content)
@@ -916,10 +948,18 @@ class TestInstructorAPIBulkAccountCreationAndEnrollment(SharedModuleStoreTestCas
self.assertEqual(response.status_code, 200)
data = json.loads(response.content)
self.assertNotEquals(len(data['row_errors']), 0)
self.assertEquals(data['row_errors'][0]['response'], 'Username {user} already exists.'.format(user='test_student_1'))
self.assertEquals(
data['row_errors'][0]['response'],
'Username {user} already exists.'.format(user='test_student_1')
)
self.assertEquals(
data['row_errors'][1]['response'],
'Invalid email {email}.'.format(email='test_student4@example.com')
)
self.assertTrue(User.objects.filter(username='test_student_1', email='test_student1@example.com').exists())
self.assertTrue(User.objects.filter(username='test_student_2', email='test_student2@example.com').exists())
self.assertFalse(User.objects.filter(email='test_student3@example.com').exists())
self.assertFalse(User.objects.filter(email='test_student4@example.com').exists())
manual_enrollments = ManualEnrollmentAudit.objects.all()
self.assertEqual(manual_enrollments.count(), 2)

29
lms/djangoapps/instructor/views/api.py
View File

@@ -16,7 +16,6 @@ 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
@@ -106,7 +105,8 @@ from student.models import (
UserProfile,
anonymous_id_for_user,
get_user_by_username_or_email,
unique_id_for_user
unique_id_for_user,
is_email_retired
)
from student.roles import CourseFinanceAdminRole, CourseSalesAdminRole
from submissions import api as sub_api # installed from the edx-submissions repository
@@ -377,7 +377,6 @@ 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.
@@ -414,20 +413,16 @@ 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)
elif is_email_retired(email):
# We are either attempting to enroll a retired user or create a new user with an email which is
# already associated with a retired account. Simply block these attempts.
row_errors.append({
'username': username,
'email': email,
'response': _('Invalid email {email_address}.').format(email_address=email),
})
log.warning(u'Email address %s is associated with a retired user, so course enrollment was ' +
u'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

25
openedx/core/djangoapps/user_api/accounts/utils.py
View File

@@ -6,7 +6,6 @@ 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 _
@@ -16,6 +15,7 @@ from completion import waffle as completion_waffle
from completion.models import BlockCompletion
from openedx.core.djangoapps.site_configuration.models import SiteConfiguration
from openedx.core.djangoapps.theming.helpers import get_config_value_from_site_or_settings, get_current_site
from student.models import is_email_retired
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.exceptions import ItemNotFoundError
@@ -197,25 +197,6 @@ def generate_password(length=12, chars=string.letters + string.digits):
def email_exists(email):
"""
Check an email against the User and UserRetirementStatus models for
existence.
Check an email against the User model 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
return User.objects.filter(email=email).exists() or is_email_retired(email)