diff --git a/common/djangoapps/student/forms.py b/common/djangoapps/student/forms.py index 48c956669b..7afbb383d6 100644 --- a/common/djangoapps/student/forms.py +++ b/common/djangoapps/student/forms.py @@ -2,16 +2,23 @@ Utility functions for validating forms """ from django import forms +from django.core.exceptions import ValidationError from django.contrib.auth.models import User from django.contrib.auth.forms import PasswordResetForm from django.contrib.auth.hashers import UNUSABLE_PASSWORD from django.contrib.auth.tokens import default_token_generator from django.utils.http import int_to_base36 +from django.utils.translation import ugettext_lazy as _ from django.template import loader from django.conf import settings from microsite_configuration import microsite +from util.password_policy_validators import ( + validate_password_length, + validate_password_complexity, + validate_password_dictionary, +) class PasswordResetFormNoActive(PasswordResetForm): @@ -70,3 +77,160 @@ class PasswordResetFormNoActive(PasswordResetForm): subject = subject.replace('\n', '') email = loader.render_to_string(email_template_name, context) send_mail(subject, email, from_email, [user.email]) + + +class TrueField(forms.BooleanField): + """ + A boolean field that only accepts "true" (case-insensitive) as true + """ + def to_python(self, value): + # CheckboxInput converts string to bool by case-insensitive match to "true" or "false" + if value is True: + return value + else: + return None + + +_USERNAME_TOO_SHORT_MSG = _("Username must be minimum of two characters long") +_EMAIL_INVALID_MSG = _("A properly formatted e-mail is required") +_PASSWORD_INVALID_MSG = _("A valid password is required") +_NAME_TOO_SHORT_MSG = _("Your legal name must be a minimum of two characters long") + + +class AccountCreationForm(forms.Form): + """ + A form to for account creation data. It is currently only used for + validation, not rendering. + """ + # TODO: Resolve repetition + username = forms.SlugField( + min_length=2, + max_length=30, + error_messages={ + "required": _USERNAME_TOO_SHORT_MSG, + "invalid": _("Username should only consist of A-Z and 0-9, with no spaces."), + "min_length": _USERNAME_TOO_SHORT_MSG, + "max_length": _("Username cannot be more than %(limit_value)s characters long"), + } + ) + email = forms.EmailField( + max_length=75, # Limit per RFCs is 254, but User's email field in django 1.4 only takes 75 + error_messages={ + "required": _EMAIL_INVALID_MSG, + "invalid": _EMAIL_INVALID_MSG, + "max_length": _("Email cannot be more than %(limit_value)s characters long"), + } + ) + password = forms.CharField( + min_length=2, + error_messages={ + "required": _PASSWORD_INVALID_MSG, + "min_length": _PASSWORD_INVALID_MSG, + } + ) + name = forms.CharField( + min_length=2, + error_messages={ + "required": _NAME_TOO_SHORT_MSG, + "min_length": _NAME_TOO_SHORT_MSG, + } + ) + + def __init__( + self, + data=None, + extra_fields=None, + extended_profile_fields=None, + enforce_username_neq_password=False, + enforce_password_policy=False, + tos_required=True + ): + super(AccountCreationForm, self).__init__(data) + + extra_fields = extra_fields or {} + self.extended_profile_fields = extended_profile_fields or {} + self.enforce_username_neq_password = enforce_username_neq_password + self.enforce_password_policy = enforce_password_policy + if tos_required: + self.fields["terms_of_service"] = TrueField( + error_messages={"required": _("You must accept the terms of service.")} + ) + + # TODO: These messages don't say anything about minimum length + error_message_dict = { + "level_of_education": _("A level of education is required"), + "gender": _("Your gender is required"), + "year_of_birth": _("Your year of birth is required"), + "mailing_address": _("Your mailing address is required"), + "goals": _("A description of your goals is required"), + "city": _("A city is required"), + "country": _("A country is required") + } + for field_name, field_value in extra_fields.items(): + if field_name not in self.fields: + if field_name == "honor_code": + if field_value == "required": + self.fields[field_name] = TrueField( + error_messages={ + "required": _("To enroll, you must follow the honor code.") + } + ) + else: + required = field_value == "required" + min_length = 1 if field_name in ("gender", "level_of_education") else 2 + error_message = error_message_dict.get( + field_name, + _("You are missing one or more required fields") + ) + self.fields[field_name] = forms.CharField( + required=required, + min_length=min_length, + error_messages={ + "required": error_message, + "min_length": error_message, + } + ) + + for field in self.extended_profile_fields: + if field not in self.fields: + self.fields[field] = forms.CharField(required=False) + + def clean_password(self): + """Enforce password policies (if applicable)""" + password = self.cleaned_data["password"] + if ( + self.enforce_username_neq_password and + "username" in self.cleaned_data and + self.cleaned_data["username"] == password + ): + raise ValidationError(_("Username and password fields cannot match")) + if self.enforce_password_policy: + try: + validate_password_length(password) + validate_password_complexity(password) + validate_password_dictionary(password) + except ValidationError, err: + raise ValidationError(_("Password: ") + "; ".join(err.messages)) + return password + + def clean_year_of_birth(self): + """ + Parse year_of_birth to an integer, but just use None instead of raising + an error if it is malformed + """ + try: + year_str = self.cleaned_data["year_of_birth"] + return int(year_str) if year_str is not None else None + except ValueError: + return None + + @property + def cleaned_extended_profile(self): + """ + Return a dictionary containing the extended_profile_fields and values + """ + return { + key: value + for key, value in self.cleaned_data.items() + if key in self.extended_profile_fields and value is not None + } diff --git a/common/djangoapps/student/management/commands/create_random_users.py b/common/djangoapps/student/management/commands/create_random_users.py index 088d4484ab..ba44c58020 100644 --- a/common/djangoapps/student/management/commands/create_random_users.py +++ b/common/djangoapps/student/management/commands/create_random_users.py @@ -8,26 +8,30 @@ from student.models import CourseEnrollment from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locations import SlashSeparatedCourseKey +from student.forms import AccountCreationForm from student.views import _do_create_account -def get_random_post_override(): +def make_random_form(): """ Generate unique user data for dummy users. """ identification = uuid.uuid4().hex[:8] - return { - 'username': 'user_{id}'.format(id=identification), - 'email': 'email_{id}@example.com'.format(id=identification), - 'password': '12345', - 'name': 'User {id}'.format(id=identification), - } + return AccountCreationForm( + data={ + 'username': 'user_{id}'.format(id=identification), + 'email': 'email_{id}@example.com'.format(id=identification), + 'password': '12345', + 'name': 'User {id}'.format(id=identification), + }, + tos_required=False + ) def create(num, course_key): """Create num users, enrolling them in course_key if it's not None""" for idx in range(num): - (user, user_profile, __) = _do_create_account(get_random_post_override()) + (user, _, _) = _do_create_account(make_random_form()) if course_key is not None: CourseEnrollment.enroll(user, course_key) diff --git a/common/djangoapps/student/management/commands/create_user.py b/common/djangoapps/student/management/commands/create_user.py index 2da0a6e4fa..4da9519baf 100644 --- a/common/djangoapps/student/management/commands/create_user.py +++ b/common/djangoapps/student/management/commands/create_user.py @@ -8,6 +8,7 @@ from django.utils import translation from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locations import SlashSeparatedCourseKey +from student.forms import AccountCreationForm from student.models import CourseEnrollment, Registration, create_comments_service_user from student.views import _do_create_account, AccountValidationError from track.management.tracked_command import TrackedCommand @@ -80,21 +81,22 @@ class Command(TrackedCommand): except InvalidKeyError: course = SlashSeparatedCourseKey.from_deprecated_string(options['course']) - post_data = { - 'username': username, - 'email': options['email'], - 'password': options['password'], - 'name': name, - 'honor_code': u'true', - 'terms_of_service': u'true', - } + form = AccountCreationForm( + data={ + 'username': username, + 'email': options['email'], + 'password': options['password'], + 'name': name, + }, + tos_required=False + ) # django.utils.translation.get_language() will be used to set the new # user's preferred language. This line ensures that the result will # match this installation's default locale. Otherwise, inside a # management command, it will always return "en-us". translation.activate(settings.LANGUAGE_CODE) try: - user, profile, reg = _do_create_account(post_data) + user, _, reg = _do_create_account(form) if options['staff']: user.is_staff = True user.save() diff --git a/common/djangoapps/student/tests/test_create_account.py b/common/djangoapps/student/tests/test_create_account.py index f921504ea3..ee1bf632d8 100644 --- a/common/djangoapps/student/tests/test_create_account.py +++ b/common/djangoapps/student/tests/test_create_account.py @@ -93,13 +93,19 @@ class TestCreateAccount(TestCase): def test_profile_saved_no_optional_fields(self): profile = self.create_account_and_fetch_profile() self.assertEqual(profile.name, self.params["name"]) - self.assertIsNone(profile.level_of_education) - self.assertIsNone(profile.gender) - self.assertIsNone(profile.mailing_address) - self.assertIsNone(profile.city) + self.assertEqual(profile.level_of_education, "") + self.assertEqual(profile.gender, "") + self.assertEqual(profile.mailing_address, "") + self.assertEqual(profile.city, "") self.assertEqual(profile.country, "") - self.assertIsNone(profile.goals) - self.assertEqual(profile.meta, "") + self.assertEqual(profile.goals, "") + self.assertEqual( + profile.get_meta(), + { + "extra1": "", + "extra2": "", + } + ) self.assertIsNone(profile.year_of_birth) @unittest.skipUnless( @@ -267,7 +273,7 @@ class TestCreateAccountValidation(TestCase): # Missing del params["username"] - assert_username_error("Error (401 username). E-mail us.") + assert_username_error("Username must be minimum of two characters long") # Empty, too short for username in ["", "a"]: @@ -282,10 +288,6 @@ class TestCreateAccountValidation(TestCase): params["username"] = "invalid username" assert_username_error("Username should only consist of A-Z and 0-9, with no spaces.") - # Matching password - params["username"] = params["password"] = "test_username_and_password" - assert_username_error("Username and password fields cannot match") - def test_email(self): params = dict(self.minimal_params) @@ -298,7 +300,7 @@ class TestCreateAccountValidation(TestCase): # Missing del params["email"] - assert_email_error("Error (401 email). E-mail us.") + assert_email_error("A properly formatted e-mail is required") # Empty, too short for email in ["", "a"]: @@ -311,7 +313,7 @@ class TestCreateAccountValidation(TestCase): # Invalid params["email"] = "not_an_email_address" - assert_email_error("Valid e-mail is required.") + assert_email_error("A properly formatted e-mail is required") def test_password(self): params = dict(self.minimal_params) @@ -325,7 +327,7 @@ class TestCreateAccountValidation(TestCase): # Missing del params["password"] - assert_password_error("Error (401 password). E-mail us.") + assert_password_error("A valid password is required") # Empty, too short for password in ["", "a"]: @@ -334,6 +336,10 @@ class TestCreateAccountValidation(TestCase): # Password policy is tested elsewhere + # Matching username + params["username"] = params["password"] = "test_username_and_password" + assert_password_error("Username and password fields cannot match") + def test_name(self): params = dict(self.minimal_params) @@ -346,7 +352,7 @@ class TestCreateAccountValidation(TestCase): # Missing del params["name"] - assert_name_error("Error (401 name). E-mail us.") + assert_name_error("Your legal name must be a minimum of two characters long") # Empty, too short for name in ["", "a"]: @@ -369,13 +375,20 @@ class TestCreateAccountValidation(TestCase): assert_honor_code_error("To enroll, you must follow the honor code.") # Empty, invalid - for honor_code in ["", "false", "True"]: + for honor_code in ["", "false", "not_boolean"]: params["honor_code"] = honor_code assert_honor_code_error("To enroll, you must follow the honor code.") + # True + params["honor_code"] = "tRUe" + self.assert_success(params) + with override_settings(REGISTRATION_EXTRA_FIELDS={"honor_code": "optional"}): # Missing del params["honor_code"] + # Need to change username/email because user was created above + params["username"] = "another_test_username" + params["email"] = "another_test_email@example.com" self.assert_success(params) def test_terms_of_service(self): @@ -393,10 +406,14 @@ class TestCreateAccountValidation(TestCase): assert_terms_of_service_error("You must accept the terms of service.") # Empty, invalid - for terms_of_service in ["", "false", "True"]: + for terms_of_service in ["", "false", "not_boolean"]: params["terms_of_service"] = terms_of_service assert_terms_of_service_error("You must accept the terms of service.") + # True + params["terms_of_service"] = "tRUe" + self.assert_success(params) + @ddt.data( ("level_of_education", 1, "A level of education is required"), ("gender", 1, "Your gender is required"), diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 02a1abf86c..1accfac5ca 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -56,7 +56,7 @@ from student.models import ( CourseEnrollmentAllowed, UserStanding, LoginFailures, create_comments_service_user, PasswordHistory, UserSignupSource, DashboardConfiguration, LinkedInAddToProfileConfiguration) -from student.forms import PasswordResetFormNoActive +from student.forms import AccountCreationForm, PasswordResetFormNoActive from verify_student.models import SoftwareSecurePhotoVerification, MidcourseReverificationWindow from certificates.models import CertificateStatuses, certificate_status_for_student @@ -1336,7 +1336,7 @@ def user_signup_handler(sender, **kwargs): # pylint: disable=unused-argument log.info(u'user {} originated from a white labeled "Microsite"'.format(kwargs['instance'].id)) -def _do_create_account(post_vars, extended_profile=None): +def _do_create_account(form): """ Given cleaned post variables, create the User and UserProfile objects, as well as the registration for this user. @@ -1345,10 +1345,15 @@ def _do_create_account(post_vars, extended_profile=None): Note: this function is also used for creating test users. """ - user = User(username=post_vars['username'], - email=post_vars['email'], - is_active=False) - user.set_password(post_vars['password']) + if not form.is_valid(): + raise ValidationError(form.errors) + + user = User( + username=form.cleaned_data["username"], + email=form.cleaned_data["email"], + is_active=False + ) + user.set_password(form.cleaned_data["password"]) registration = Registration() # TODO: Rearrange so that if part of the process fails, the whole process fails. @@ -1357,14 +1362,14 @@ def _do_create_account(post_vars, extended_profile=None): user.save() except IntegrityError: # Figure out the cause of the integrity error - if len(User.objects.filter(username=post_vars['username'])) > 0: + if len(User.objects.filter(username=user.username)) > 0: raise AccountValidationError( - _("An account with the Public Username '{username}' already exists.").format(username=post_vars['username']), + _("An account with the Public Username '{username}' already exists.").format(username=user.username), field="username" ) - elif len(User.objects.filter(email=post_vars['email'])) > 0: + elif len(User.objects.filter(email=user.email)) > 0: raise AccountValidationError( - _("An account with the Email '{email}' already exists.").format(email=post_vars['email']), + _("An account with the Email '{email}' already exists.").format(email=user.email), field="email" ) else: @@ -1377,25 +1382,17 @@ def _do_create_account(post_vars, extended_profile=None): registration.register(user) - profile = UserProfile(user=user) - profile.name = post_vars['name'] - profile.level_of_education = post_vars.get('level_of_education') - profile.gender = post_vars.get('gender') - profile.mailing_address = post_vars.get('mailing_address') - profile.city = post_vars.get('city') - profile.country = post_vars.get('country') - profile.goals = post_vars.get('goals') - - # add any extended profile information in the denormalized 'meta' field in the profile + profile_fields = [ + "name", "level_of_education", "gender", "mailing_address", "city", "country", "goals", + "year_of_birth" + ] + profile = UserProfile( + user=user, + **{key: form.cleaned_data.get(key) for key in profile_fields} + ) + extended_profile = form.cleaned_extended_profile if extended_profile: profile.meta = json.dumps(extended_profile) - - try: - profile.year_of_birth = int(post_vars['year_of_birth']) - except (ValueError, KeyError): - # If they give us garbage, just ignore it instead - # of asking them to put an integer. - profile.year_of_birth = None try: profile.save() except Exception: # pylint: disable=broad-except @@ -1447,19 +1444,15 @@ def create_account(request, post_override=None): # pylint: disable-msg=too-many post_vars.update(dict(email=email, name=name, password=password)) log.debug(u'In create_account with external_auth: user = %s, email=%s', name, email) - # Confirm we have a properly formed request - for req_field in ['username', 'email', 'password', 'name']: - if req_field not in post_vars: - js['value'] = _("Error (401 {field}). E-mail us.").format(field=req_field) - js['field'] = req_field - return JsonResponse(js, status=400) - - if extra_fields.get('honor_code', 'required') == 'required' and \ - post_vars.get('honor_code', 'false') != u'true': - js['value'] = _("To enroll, you must follow the honor code.") - js['field'] = 'honor_code' - return JsonResponse(js, status=400) - + extra_fields = microsite.get_value( + 'REGISTRATION_EXTRA_FIELDS', + getattr(settings, 'REGISTRATION_EXTRA_FIELDS', {}) + ) + extended_profile_fields = microsite.get_value('extended_profile_fields', []) + enforce_password_policy = ( + settings.FEATURES.get("ENFORCE_PASSWORD_POLICY", False) and + not do_external_auth + ) # Can't have terms of service for certain SHIB users, like at Stanford tos_required = ( not settings.FEATURES.get("AUTH_USE_SHIB") or @@ -1470,120 +1463,29 @@ def create_account(request, post_override=None): # pylint: disable-msg=too-many ) ) - if tos_required: - if post_vars.get('terms_of_service', 'false') != u'true': - js['value'] = _("You must accept the terms of service.") - js['field'] = 'terms_of_service' - return JsonResponse(js, status=400) + form = AccountCreationForm( + data=post_vars, + extra_fields=extra_fields, + extended_profile_fields=extended_profile_fields, + enforce_username_neq_password=True, + enforce_password_policy=enforce_password_policy, + tos_required=tos_required + ) - # Confirm appropriate fields are there. - # TODO: Check e-mail format is correct. - # TODO: Confirm e-mail is not from a generic domain (mailinator, etc.)? Not sure if - # this is a good idea - # TODO: Check password is sane + if not form.is_valid(): + field, error_list = next(form.errors.iteritems()) + return JsonResponse( + { + "success": False, + "field": field, + "value": error_list[0], + }, + status=400 + ) - required_post_vars = ['username', 'email', 'name', 'password'] - required_post_vars += [fieldname for fieldname, val in extra_fields.items() - if val == 'required'] - if tos_required: - required_post_vars.append('terms_of_service') - - for field_name in required_post_vars: - if field_name in ('gender', 'level_of_education'): - min_length = 1 - else: - min_length = 2 - - if field_name not in post_vars or len(post_vars[field_name]) < min_length: - error_str = { - 'username': _('Username must be minimum of two characters long'), - 'email': _('A properly formatted e-mail is required'), - 'name': _('Your legal name must be a minimum of two characters long'), - 'password': _('A valid password is required'), - 'terms_of_service': _('Accepting Terms of Service is required'), - 'honor_code': _('Agreeing to the Honor Code is required'), - 'level_of_education': _('A level of education is required'), - 'gender': _('Your gender is required'), - 'year_of_birth': _('Your year of birth is required'), - 'mailing_address': _('Your mailing address is required'), - 'goals': _('A description of your goals is required'), - 'city': _('A city is required'), - 'country': _('A country is required') - } - - if field_name in error_str: - js['value'] = error_str[field_name] - else: - js['value'] = _('You are missing one or more required fields') - - js['field'] = field_name - return JsonResponse(js, status=400) - - max_length = 75 - if field_name == 'username': - max_length = 30 - - if field_name in ('email', 'username') and len(post_vars[field_name]) > max_length: - error_str = { - 'username': _('Username cannot be more than {num} characters long').format(num=max_length), - 'email': _('Email cannot be more than {num} characters long').format(num=max_length) - } - js['value'] = error_str[field_name] - js['field'] = field_name - return JsonResponse(js, status=400) - - try: - validate_email(post_vars['email']) - except ValidationError: - js['value'] = _("Valid e-mail is required.") - js['field'] = 'email' - return JsonResponse(js, status=400) - - try: - validate_slug(post_vars['username']) - except ValidationError: - js['value'] = _("Username should only consist of A-Z and 0-9, with no spaces.") - js['field'] = 'username' - return JsonResponse(js, status=400) - - # enforce password complexity as an optional feature - # but not if we're doing ext auth b/c those pws never get used and are auto-generated so might not pass validation - if settings.FEATURES.get('ENFORCE_PASSWORD_POLICY', False) and not do_external_auth: - try: - password = post_vars['password'] - - validate_password_length(password) - validate_password_complexity(password) - validate_password_dictionary(password) - except ValidationError, err: - js['value'] = _('Password: ') + '; '.join(err.messages) - js['field'] = 'password' - return JsonResponse(js, status=400) - - # allow microsites to define 'extended profile fields' which are - # captured on user signup (for example via an overriden registration.html) - # and then stored in the UserProfile - extended_profile_fields = microsite.get_value('extended_profile_fields', []) - extended_profile = None - - for field in extended_profile_fields: - if field in post_vars: - if not extended_profile: - extended_profile = {} - extended_profile[field] = post_vars[field] - - # Make sure that password and username fields do not match - username = post_vars['username'] - password = post_vars['password'] - if username == password: - js['value'] = _("Username and password fields cannot match") - js['field'] = 'username' - return JsonResponse(js, status=400) - - # Ok, looks like everything is legit. Create the account. try: with transaction.commit_on_success(): - ret = _do_create_account(post_vars, extended_profile) + ret = _do_create_account(form) except AccountValidationError as exc: return JsonResponse({'success': False, 'value': exc.message, 'field': exc.field}, status=400) @@ -1757,21 +1659,21 @@ def auto_auth(request): role_names = [v.strip() for v in request.GET.get('roles', '').split(',') if v.strip()] login_when_done = 'no_login' not in request.GET - # Get or create the user object - post_data = { - 'username': username, - 'email': email, - 'password': password, - 'name': full_name, - 'honor_code': u'true', - 'terms_of_service': u'true', - } + form = AccountCreationForm( + data={ + 'username': username, + 'email': email, + 'password': password, + 'name': full_name, + }, + tos_required=False + ) # Attempt to create the account. # If successful, this will return a tuple containing # the new user object. try: - user, _profile, reg = _do_create_account(post_data) + user, _profile, reg = _do_create_account(form) except AccountValidationError: # Attempt to retrieve the existing user. user = User.objects.get(username=username)