Reject acct creation when using a retired username.

Change from specifying a retired username/email format to specifying
  a retired username prefix and a retired email prefix/domain,
  preventing possible config errors due to unexpected/bad formats.
This commit is contained in:
John Eskew
2018-04-03 18:13:01 -04:00
parent a272132eb2
commit 338d4551ab
8 changed files with 124 additions and 31 deletions

View File

@@ -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

View File

@@ -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,

View File

@@ -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)

View File

@@ -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:

View File

@@ -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"
)

View File

@@ -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)

View File

@@ -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

View File

@@ -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'