diff --git a/cms/envs/aws.py b/cms/envs/aws.py index e98993369f..16d0d262c9 100644 --- a/cms/envs/aws.py +++ b/cms/envs/aws.py @@ -575,8 +575,9 @@ ENTERPRISE_REPORTING_SECRET = AUTH_TOKENS.get( ) ############### Settings for Retirement ##################### -RETIRED_USERNAME_FMT = ENV_TOKENS.get('RETIRED_USERNAME_FMT', RETIRED_USERNAME_FMT) -RETIRED_EMAIL_FMT = ENV_TOKENS.get('RETIRED_EMAIL_FMT', RETIRED_EMAIL_FMT) +RETIRED_USERNAME_PREFIX = ENV_TOKENS.get('RETIRED_USERNAME_PREFIX', RETIRED_USERNAME_PREFIX) +RETIRED_EMAIL_PREFIX = ENV_TOKENS.get('RETIRED_EMAIL_PREFIX', RETIRED_EMAIL_PREFIX) +RETIRED_EMAIL_DOMAIN = ENV_TOKENS.get('RETIRED_EMAIL_DOMAIN', RETIRED_EMAIL_DOMAIN) RETIREMENT_SERVICE_WORKER_USERNAME = ENV_TOKENS.get( 'RETIREMENT_SERVICE_WORKER_USERNAME', RETIREMENT_SERVICE_WORKER_USERNAME diff --git a/cms/envs/common.py b/cms/envs/common.py index 470d197dc5..7c5fd78683 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -127,7 +127,10 @@ from lms.envs.common import ( VIDEO_IMAGE_SETTINGS, VIDEO_TRANSCRIPTS_SETTINGS, + RETIRED_USERNAME_PREFIX, RETIRED_USERNAME_FMT, + RETIRED_EMAIL_PREFIX, + RETIRED_EMAIL_DOMAIN, RETIRED_EMAIL_FMT, RETIRED_USER_SALTS, RETIREMENT_SERVICE_WORKER_USERNAME, diff --git a/common/djangoapps/student/apps.py b/common/djangoapps/student/apps.py index 32520dceb4..9498dd2ef1 100644 --- a/common/djangoapps/student/apps.py +++ b/common/djangoapps/student/apps.py @@ -5,6 +5,7 @@ from __future__ import absolute_import from django.apps import AppConfig from django.contrib.auth.signals import user_logged_in +from django.db.models.signals import pre_save class StudentConfig(AppConfig): @@ -18,3 +19,7 @@ class StudentConfig(AppConfig): user_logged_in.disconnect(django_update_last_login) from .signals.receivers import update_last_login user_logged_in.connect(update_last_login) + + from django.contrib.auth.models import User + from .signals.receivers import on_user_updated + pre_save.connect(on_user_updated, sender=User) diff --git a/common/djangoapps/student/helpers.py b/common/djangoapps/student/helpers.py index fcf2cc17b7..fdf8de6a4c 100644 --- a/common/djangoapps/student/helpers.py +++ b/common/djangoapps/student/helpers.py @@ -65,6 +65,7 @@ DISABLE_UNENROLL_CERT_STATES = [ 'generating', 'downloadable', ] +USERNAME_EXISTS_MSG_FMT = _("An account with the Public Username '{username}' already exists.") log = logging.getLogger(__name__) @@ -621,8 +622,9 @@ def do_create_account(form, custom_form=None): if errors: raise ValidationError(errors) + proposed_username = form.cleaned_data["username"] user = User( - username=form.cleaned_data["username"], + username=proposed_username, email=form.cleaned_data["email"], is_active=False ) @@ -647,7 +649,7 @@ def do_create_account(form, custom_form=None): # different username.") if len(User.objects.filter(username=user.username)) > 0: raise AccountValidationError( - _("An account with the Public Username '{username}' already exists.").format(username=user.username), + USERNAME_EXISTS_MSG_FMT.format(username=proposed_username), field="username" ) elif len(User.objects.filter(email=user.email)) > 0: diff --git a/common/djangoapps/student/signals/receivers.py b/common/djangoapps/student/signals/receivers.py index c3aac7879f..4c0b9b6bb4 100644 --- a/common/djangoapps/student/signals/receivers.py +++ b/common/djangoapps/student/signals/receivers.py @@ -3,9 +3,15 @@ Signal receivers for the "student" application. """ from __future__ import absolute_import +from django.conf import settings from django.utils import timezone from openedx.core.djangoapps.user_api.config.waffle import PREVENT_AUTH_USER_WRITES, waffle +from student.helpers import ( + AccountValidationError, + USERNAME_EXISTS_MSG_FMT +) +from student.models import is_username_retired def update_last_login(sender, user, **kwargs): # pylint: disable=unused-argument @@ -17,3 +23,26 @@ def update_last_login(sender, user, **kwargs): # pylint: disable=unused-argumen if not waffle().is_enabled(PREVENT_AUTH_USER_WRITES): user.last_login = timezone.now() user.save(update_fields=['last_login']) + + +def on_user_updated(sender, instance, **kwargs): # pylint: disable=unused-argument + """ + Check for retired usernames. + """ + # Check only at User creation time and when not raw. + if not instance.id and not kwargs['raw']: + prefix_to_check = getattr(settings, 'RETIRED_USERNAME_PREFIX', None) + if prefix_to_check: + # Check for username that's too close to retired username format. + if instance.username.startswith(prefix_to_check): + raise AccountValidationError( + USERNAME_EXISTS_MSG_FMT.format(username=instance.username), + field="username" + ) + + # Check for a retired username. + if is_username_retired(instance.username): + raise AccountValidationError( + USERNAME_EXISTS_MSG_FMT.format(username=instance.username), + field="username" + ) diff --git a/common/djangoapps/student/tests/test_retirement.py b/common/djangoapps/student/tests/test_retirement.py index 695503163d..4147aee8b8 100644 --- a/common/djangoapps/student/tests/test_retirement.py +++ b/common/djangoapps/student/tests/test_retirement.py @@ -1,9 +1,14 @@ """ Test user retirement methods -TODO: When the hasher is working actually test it here with multiple salts """ +import json + +import ddt from django.conf import settings from django.contrib.auth.models import User +from django.core.urlresolvers import reverse +from django.test import TestCase +from django.test.utils import override_settings import pytest from student.models import ( @@ -21,41 +26,28 @@ from student.tests.factories import UserFactory pytestmark = pytest.mark.django_db # Make sure our settings are sane +assert settings.RETIRED_USERNAME_PREFIX +assert settings.RETIRED_EMAIL_PREFIX +assert settings.RETIRED_EMAIL_DOMAIN assert "{}" in settings.RETIRED_USERNAME_FMT -assert "{}" in settings.RETIRED_EMAIL_FMT - -RETIRED_USERNAME_START, RETIRED_USERNAME_END = settings.RETIRED_USERNAME_FMT.split("{}") -RETIRED_EMAIL_START, RETIRED_EMAIL_END = settings.RETIRED_EMAIL_FMT.split("{}") +assert "{}@" in settings.RETIRED_EMAIL_FMT def check_username_against_fmt(hashed_username): """ - Checks that the given username is formatted correctly using - our settings. The format string may put the hashed string - anywhere, and the hasher is opaque, so this just looks for - our surrounding strings, if they exist. + Checks that the given username is formatted correctly using our settings. """ assert len(hashed_username) > len(settings.RETIRED_USERNAME_FMT) - - if RETIRED_USERNAME_START: - assert hashed_username.startswith(RETIRED_USERNAME_START) - if RETIRED_USERNAME_END: - assert hashed_username.endswith(RETIRED_USERNAME_END) + assert hashed_username.startswith(settings.RETIRED_USERNAME_PREFIX) def check_email_against_fmt(hashed_email): """ - Checks that the given email is formatted correctly using - our settings. The format string may put the hashed string - anywhere, and the hasher is opaque, so this just looks for - our surrounding strings, if they exist. + Checks that the given email is formatted correctly using our settings. """ assert len(hashed_email) > len(settings.RETIRED_EMAIL_FMT) - - if RETIRED_EMAIL_START: - assert hashed_email.startswith(RETIRED_EMAIL_START) - if RETIRED_EMAIL_END: - assert hashed_email.endswith(RETIRED_EMAIL_END) + assert hashed_email.startswith(settings.RETIRED_EMAIL_PREFIX) + assert hashed_email.endswith(settings.RETIRED_EMAIL_DOMAIN) def test_get_retired_username(): @@ -182,3 +174,59 @@ def test_get_potentially_retired_user_bad_hash(): with pytest.raises(Exception): get_potentially_retired_user_by_username_and_hash(fake_username, "bad hash") + + +@ddt.ddt +class TestRegisterRetiredUsername(TestCase): + """ + Tests to ensure that retired usernames can no longer be used in registering new accounts. + """ + def setUp(self): + super(TestRegisterRetiredUsername, self).setUp() + self.url = reverse('create_account') + self.url_params = { + 'username': 'username', + 'email': 'foo_bar' + '@bar.com', + 'name': 'foo bar', + 'password': '123', + 'terms_of_service': 'true', + 'honor_code': 'true', + } + + def _validate_exiting_username_response(self, orig_username, response): + """ + Validates a response stating that a username already exists. + """ + assert response.status_code == 400 + obj = json.loads(response.content) + assert obj['value'].startswith('An account with the Public Username') + assert obj['value'].endswith('already exists.') + assert orig_username in obj['value'] + assert obj['field'] == 'username' + assert not obj['success'] + + def test_retired_username(self): + """ + Ensure that a retired username cannot be registered again. + """ + user = UserFactory() + orig_username = user.username + + # Fake retirement of the username. + user.username = get_retired_username_by_username(orig_username) + user.save() + + # Attempt to create another account with the same username that's been retired. + self.url_params['username'] = orig_username + response = self.client.post(self.url, self.url_params) + self._validate_exiting_username_response(orig_username, response) + + def test_username_close_to_retired_format_active(self): + """ + Ensure that a username similar to the format of a retired username cannot be created. + """ + # Attempt to create an account with a username similar to the format of a retired username + # which matches the RETIRED_USERNAME_PREFIX setting. + self.url_params['username'] = settings.RETIRED_USERNAME_PREFIX + response = self.client.post(self.url, self.url_params) + self._validate_exiting_username_response(settings.RETIRED_USERNAME_PREFIX, response) diff --git a/lms/envs/aws.py b/lms/envs/aws.py index 2082c997a6..8f904bfb29 100644 --- a/lms/envs/aws.py +++ b/lms/envs/aws.py @@ -1082,8 +1082,9 @@ FERNET_KEYS = AUTH_TOKENS.get('FERNET_KEYS', FERNET_KEYS) MAINTENANCE_BANNER_TEXT = ENV_TOKENS.get('MAINTENANCE_BANNER_TEXT', None) ############### Settings for Retirement ##################### -RETIRED_USERNAME_FMT = ENV_TOKENS.get('RETIRED_USERNAME_FMT', RETIRED_USERNAME_FMT) -RETIRED_EMAIL_FMT = ENV_TOKENS.get('RETIRED_EMAIL_FMT', RETIRED_EMAIL_FMT) +RETIRED_USERNAME_PREFIX = ENV_TOKENS.get('RETIRED_USERNAME_PREFIX', RETIRED_USERNAME_PREFIX) +RETIRED_EMAIL_PREFIX = ENV_TOKENS.get('RETIRED_EMAIL_PREFIX', RETIRED_EMAIL_PREFIX) +RETIRED_EMAIL_DOMAIN = ENV_TOKENS.get('RETIRED_EMAIL_DOMAIN', RETIRED_EMAIL_DOMAIN) RETIREMENT_SERVICE_WORKER_USERNAME = ENV_TOKENS.get( 'RETIREMENT_SERVICE_WORKER_USERNAME', RETIREMENT_SERVICE_WORKER_USERNAME diff --git a/lms/envs/common.py b/lms/envs/common.py index 4cf56e2341..8867066a94 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -3409,8 +3409,12 @@ RATELIMIT_ENABLE = True RATELIMIT_RATE = '120/m' ############### Settings for Retirement ##################### -RETIRED_USERNAME_FMT = 'retired__user_{}' -RETIRED_EMAIL_FMT = 'retired__user_{}@retired.invalid' +RETIRED_USERNAME_PREFIX = 'retired__user_' +RETIRED_EMAIL_PREFIX = 'retired__user_' +RETIRED_EMAIL_DOMAIN = 'retired.invalid' +RETIRED_USERNAME_FMT = lambda settings: settings.RETIRED_USERNAME_PREFIX + '{}' +RETIRED_EMAIL_FMT = lambda settings: settings.RETIRED_EMAIL_PREFIX + '{}@' + settings.RETIRED_EMAIL_DOMAIN +derived('RETIRED_USERNAME_FMT', 'RETIRED_EMAIL_FMT') RETIRED_USER_SALTS = ['abc', '123'] RETIREMENT_SERVICE_WORKER_USERNAME = 'RETIREMENT_SERVICE_USER'