From 1b46c3e646555cab98b1b8232f09a17bfe186356 Mon Sep 17 00:00:00 2001 From: Omar Al-Ithawi Date: Mon, 20 Mar 2017 19:13:28 +0200 Subject: [PATCH] Add optional support for Unicode usernames Refactoring: Use format with named variables --- cms/envs/common.py | 5 +- common/djangoapps/student/admin.py | 9 ++ common/djangoapps/student/forms.py | 99 +++++++++++++++---- .../student/tests/test_admin_views.py | 26 +++++ .../student/tests/test_create_account.py | 68 ++++++++++++- .../djangoapps/third_party_auth/api/urls.py | 16 ++- .../test/acceptance/pages/common/auto_auth.py | 9 +- common/test/acceptance/pages/lms/admin.py | 79 +++++++++++++++ .../tests/lms/test_unicode_username_admin.py | 53 ++++++++++ common/test/db_fixtures/unicode_user.json | 25 +++++ lms/djangoapps/teams/api_urls.py | 15 ++- lms/envs/common.py | 8 +- lms/urls.py | 4 +- .../djangoapps/user_api/accounts/__init__.py | 2 +- .../core/djangoapps/user_api/accounts/api.py | 16 +-- .../user_api/accounts/tests/test_api.py | 31 ++++++ 16 files changed, 423 insertions(+), 42 deletions(-) create mode 100644 common/test/acceptance/pages/lms/admin.py create mode 100644 common/test/acceptance/tests/lms/test_unicode_username_admin.py create mode 100644 common/test/db_fixtures/unicode_user.json diff --git a/cms/envs/common.py b/cms/envs/common.py index ac8773df98..30a341e09f 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -81,6 +81,9 @@ from lms.envs.common import ( JWT_AUTH, + USERNAME_REGEX_PARTIAL, + USERNAME_PATTERN, + # django-debug-toolbar DEBUG_TOOLBAR_PATCH_SETTINGS, BLOCK_STRUCTURES_SETTINGS, @@ -1286,8 +1289,6 @@ OAUTH_OIDC_ISSUER = 'https://www.example.com/oauth2' # 5 minute expiration time for JWT id tokens issued for external API requests. OAUTH_ID_TOKEN_EXPIRATION = 5 * 60 -USERNAME_PATTERN = r'(?P[\w.@+-]+)' - # Partner support link for CMS footer PARTNER_SUPPORT_EMAIL = '' diff --git a/common/djangoapps/student/admin.py b/common/djangoapps/student/admin.py index 98b1767d7a..2c1a4dae7d 100644 --- a/common/djangoapps/student/admin.py +++ b/common/djangoapps/student/admin.py @@ -175,6 +175,15 @@ class UserAdmin(BaseUserAdmin): """ Admin interface for the User model. """ inlines = (UserProfileInline,) + def get_readonly_fields(self, *args, **kwargs): + """ + Allows editing the users while skipping the username check, so we can have Unicode username with no problems. + The username is marked read-only regardless of `ENABLE_UNICODE_USERNAME`, to simplify the bokchoy tests. + """ + + django_readonly = super(UserAdmin, self).get_readonly_fields(*args, **kwargs) + return django_readonly + ('username',) + @admin.register(UserAttribute) class UserAttributeAdmin(admin.ModelAdmin): diff --git a/common/djangoapps/student/forms.py b/common/djangoapps/student/forms.py index 81038f5076..273af6663d 100644 --- a/common/djangoapps/student/forms.py +++ b/common/djangoapps/student/forms.py @@ -15,12 +15,26 @@ from django.forms import widgets from django.template import loader from django.utils.http import int_to_base36 from django.utils.translation import ugettext_lazy as _ +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 student.models import CourseEnrollmentAllowed from util.password_policy_validators import validate_password_strength +USERNAME_TOO_SHORT_MSG = _("Username must be minimum of two characters long") +USERNAME_TOO_LONG_MSG = _("Username cannot be more than %(limit_value)s characters long") + +# Translators: This message is shown when the Unicode usernames are NOT allowed +USERNAME_INVALID_CHARS_ASCII = _("Usernames can only contain Roman letters, western numerals (0-9), " + "underscores (_), and hyphens (-).") + +# Translators: This message is shown only when the Unicode usernames are allowed +USERNAME_INVALID_CHARS_UNICODE = _("Usernames can only contain letters, numerals, underscore (_), numbers " + "and @/./+/-/_ characters.") + + class PasswordResetFormNoActive(PasswordResetForm): error_messages = { 'unknown': _("That e-mail address doesn't have an associated " @@ -102,10 +116,61 @@ class TrueField(forms.BooleanField): widget = TrueCheckbox -_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") +def validate_username(username): + """ + Verifies a username is valid, raises a ValidationError otherwise. + Args: + username (unicode): The username to validate. + + This function is configurable with `ENABLE_UNICODE_USERNAME` feature. + """ + + username_re = slug_re + flags = None + message = USERNAME_INVALID_CHARS_ASCII + + if settings.FEATURES.get("ENABLE_UNICODE_USERNAME"): + username_re = r"^{regex}$".format(regex=settings.USERNAME_REGEX_PARTIAL) + flags = re.UNICODE + message = USERNAME_INVALID_CHARS_UNICODE + + validator = RegexValidator( + regex=username_re, + flags=flags, + message=message, + code='invalid', + ) + + validator(username) + + +class UsernameField(forms.CharField): + """ + A CharField that validates usernames based on the `ENABLE_UNICODE_USERNAME` feature. + """ + + default_validators = [validate_username] + + def __init__(self, *args, **kwargs): + super(UsernameField, self).__init__( + min_length=accounts_settings.USERNAME_MIN_LENGTH, + max_length=accounts_settings.USERNAME_MAX_LENGTH, + error_messages={ + "required": USERNAME_TOO_SHORT_MSG, + "min_length": USERNAME_TOO_SHORT_MSG, + "max_length": USERNAME_TOO_LONG_MSG, + } + ) + + def clean(self, value): + """ + Strips the spaces from the username. + + Similar to what `django.forms.SlugField` does. + """ + + value = self.to_python(value).strip() + return super(UsernameField, self).clean(value) class AccountCreationForm(forms.Form): @@ -113,20 +178,18 @@ class AccountCreationForm(forms.Form): A form to for account creation data. It is currently only used for validation, not rendering. """ + + _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") + # TODO: Resolve repetition - username = forms.SlugField( - min_length=2, - max_length=30, - error_messages={ - "required": _USERNAME_TOO_SHORT_MSG, - "invalid": _("Usernames can only contain Roman letters, western numerals (0-9), underscores (_), and " - "hyphens (-)."), - "min_length": _USERNAME_TOO_SHORT_MSG, - "max_length": _("Username cannot be more than %(limit_value)s characters long"), - } - ) + + username = UsernameField() + email = forms.EmailField( - max_length=254, # Limit per RFCs is 254 + max_length=accounts_settings.EMAIL_MAX_LENGTH, + min_length=accounts_settings.EMAIL_MIN_LENGTH, error_messages={ "required": _EMAIL_INVALID_MSG, "invalid": _EMAIL_INVALID_MSG, @@ -134,14 +197,14 @@ class AccountCreationForm(forms.Form): } ) password = forms.CharField( - min_length=2, + min_length=accounts_settings.PASSWORD_MIN_LENGTH, error_messages={ "required": _PASSWORD_INVALID_MSG, "min_length": _PASSWORD_INVALID_MSG, } ) name = forms.CharField( - min_length=2, + min_length=accounts_settings.NAME_MIN_LENGTH, error_messages={ "required": _NAME_TOO_SHORT_MSG, "min_length": _NAME_TOO_SHORT_MSG, diff --git a/common/djangoapps/student/tests/test_admin_views.py b/common/djangoapps/student/tests/test_admin_views.py index faa9456f18..d5886dfed0 100644 --- a/common/djangoapps/student/tests/test_admin_views.py +++ b/common/djangoapps/student/tests/test_admin_views.py @@ -1,8 +1,13 @@ """ Tests student admin.py """ +from django.contrib.admin.sites import AdminSite +from django.contrib.auth.models import User from django.core.urlresolvers import reverse +from django.test import TestCase +from mock import Mock +from student.admin import UserAdmin from student.tests.factories import UserFactory from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory @@ -150,3 +155,24 @@ class AdminCourseRolesPageTest(SharedModuleStoreTestCase): 'edxxx', 'edx' ) ) + + +class AdminUserPageTest(TestCase): + """ + Unit tests for the UserAdmin view. + """ + def setUp(self): + super(AdminUserPageTest, self).setUp() + self.admin = UserAdmin(User, AdminSite()) + + def test_username_is_readonly(self): + """ + Ensures that the username is readonly to skip Django validation in the `auth_user_change` view. + + Changing the username is still possible using the database or from the model directly. + + However, changing the username might cause issues with the logs and/or the cs_comments_service since it + stores the username in a different database. + """ + request = Mock() + self.assertIn('username', self.admin.get_readonly_fields(request)) diff --git a/common/djangoapps/student/tests/test_create_account.py b/common/djangoapps/student/tests/test_create_account.py index 98d5a8b8dc..fb9e922e42 100644 --- a/common/djangoapps/student/tests/test_create_account.py +++ b/common/djangoapps/student/tests/test_create_account.py @@ -1,3 +1,4 @@ +# -*- coding: utf-8 -*- """Tests for account creation""" import json import unittest @@ -22,6 +23,7 @@ from openedx.core.djangoapps.external_auth.models import ExternalAuthMap from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY from openedx.core.djangoapps.site_configuration.tests.mixins import SiteMixin from openedx.core.djangoapps.user_api.preferences.api import get_user_preference +from student.forms import USERNAME_INVALID_CHARS_ASCII, USERNAME_INVALID_CHARS_UNICODE from student.models import UserAttribute from student.views import REGISTRATION_AFFILIATE_ID, REGISTRATION_UTM_CREATED_AT, REGISTRATION_UTM_PARAMETERS @@ -487,8 +489,7 @@ class TestCreateAccountValidation(TestCase): # Invalid params["username"] = "invalid username" - assert_username_error("Usernames can only contain Roman letters, western numerals (0-9), underscores (_), and " - "hyphens (-).") + assert_username_error(str(USERNAME_INVALID_CHARS_ASCII)) def test_email(self): params = dict(self.minimal_params) @@ -735,3 +736,66 @@ class TestCreateCommentsServiceUser(TransactionTestCase): User.objects.get(username=self.username) self.assertTrue(register.called) self.assertFalse(request.called) + + +class TestUnicodeUsername(TestCase): + """ + Test for Unicode usernames which is an optional feature. + """ + + def setUp(self): + super(TestUnicodeUsername, self).setUp() + self.url = reverse('create_account') + + # The word below reads "Omar II", in Arabic. It also contains a space and + # an Eastern Arabic Number another option is to use the Esperanto fake + # language but this was used instead to test non-western letters. + self.username = u'عمر ٢' + + self.url_params = { + 'username': self.username, + 'email': 'unicode_user@example.com', + "password": "testpass", + 'name': 'unicode_user', + 'terms_of_service': 'true', + 'honor_code': 'true', + } + + @patch.dict(settings.FEATURES, {'ENABLE_UNICODE_USERNAME': False}) + def test_with_feature_disabled(self): + """ + Ensures backward-compatible defaults. + """ + response = self.client.post(self.url, self.url_params) + + self.assertEquals(response.status_code, 400) + obj = json.loads(response.content) + self.assertEquals(USERNAME_INVALID_CHARS_ASCII, obj['value']) + + with self.assertRaises(User.DoesNotExist): + User.objects.get(email=self.url_params['email']) + + @patch.dict(settings.FEATURES, {'ENABLE_UNICODE_USERNAME': True}) + def test_with_feature_enabled(self): + response = self.client.post(self.url, self.url_params) + self.assertEquals(response.status_code, 200) + + self.assertTrue(User.objects.get(email=self.url_params['email'])) + + @patch.dict(settings.FEATURES, {'ENABLE_UNICODE_USERNAME': True}) + def test_special_chars_with_feature_enabled(self): + """ + Ensures that special chars are still prevented. + """ + + invalid_params = self.url_params.copy() + invalid_params['username'] = '**john**' + + response = self.client.post(self.url, invalid_params) + self.assertEquals(response.status_code, 400) + + obj = json.loads(response.content) + self.assertEquals(USERNAME_INVALID_CHARS_UNICODE, obj['value']) + + with self.assertRaises(User.DoesNotExist): + User.objects.get(email=self.url_params['email']) diff --git a/common/djangoapps/third_party_auth/api/urls.py b/common/djangoapps/third_party_auth/api/urls.py index 39056c43e3..5e8c285d79 100644 --- a/common/djangoapps/third_party_auth/api/urls.py +++ b/common/djangoapps/third_party_auth/api/urls.py @@ -1,15 +1,23 @@ """ URL configuration for the third party auth API """ +from django.conf import settings from django.conf.urls import patterns, url from .views import UserMappingView, UserView -USERNAME_PATTERN = r'(?P[\w.+-]+)' + PROVIDER_PATTERN = r'(?P[\w.+-]+)(?:\:(?P[\w.+-]+))?' urlpatterns = patterns( '', - url(r'^v0/users/' + USERNAME_PATTERN + '$', UserView.as_view(), name='third_party_auth_users_api'), - url(r'^v0/providers/' + PROVIDER_PATTERN + '/users$', UserMappingView.as_view(), - name='third_party_auth_user_mapping_api'), + url( + r'^v0/users/{username_pattern}$'.format(username_pattern=settings.USERNAME_PATTERN), + UserView.as_view(), + name='third_party_auth_users_api', + ), + url( + r'^v0/providers/{provider_pattern}/users$'.format(provider_pattern=PROVIDER_PATTERN), + UserMappingView.as_view(), + name='third_party_auth_user_mapping_api', + ), ) diff --git a/common/test/acceptance/pages/common/auto_auth.py b/common/test/acceptance/pages/common/auto_auth.py index 8e265dd842..64773cc190 100644 --- a/common/test/acceptance/pages/common/auto_auth.py +++ b/common/test/acceptance/pages/common/auto_auth.py @@ -23,8 +23,8 @@ class AutoAuthPage(PageObject): # Internal cache for parsed user info. _user_info = None - def __init__(self, browser, username=None, email=None, password=None, full_name=XSS_INJECTION, staff=False, course_id=None, - enrollment_mode=None, roles=None, no_login=False, is_active=True, course_access_roles=None): + def __init__(self, browser, username=None, email=None, password=None, full_name=XSS_INJECTION, staff=False, superuser=None, + course_id=None, enrollment_mode=None, roles=None, no_login=False, is_active=True, course_access_roles=None): """ Auto-auth is an end-point for HTTP GET requests. By default, it will create accounts with random user credentials, @@ -33,6 +33,7 @@ class AutoAuthPage(PageObject): `username`, `email`, and `password` are the user's credentials (strings) 'full_name' is the profile full name value `staff` is a boolean indicating whether the user is global staff. + `superuser` is a boolean indicating whether the user is a super user. `course_id` is the ID of the course to enroll the student in. Currently, this has the form "org/number/run" @@ -49,6 +50,7 @@ class AutoAuthPage(PageObject): self._params = { 'full_name': full_name, 'staff': staff, + 'superuser': superuser, 'is_active': is_active, 'course_access_roles': course_access_roles, } @@ -62,6 +64,9 @@ class AutoAuthPage(PageObject): if password: self._params['password'] = password + if superuser is not None: + self._params['superuser'] = "true" if superuser else "false" + if course_id: self._params['course_id'] = course_id diff --git a/common/test/acceptance/pages/lms/admin.py b/common/test/acceptance/pages/lms/admin.py new file mode 100644 index 0000000000..ca55276b1b --- /dev/null +++ b/common/test/acceptance/pages/lms/admin.py @@ -0,0 +1,79 @@ +""" +Pages object for the Django's /admin/ views. +""" +from bok_choy.page_object import PageObject +from common.test.acceptance.pages.lms import BASE_URL + + +class ChangeUserAdminPage(PageObject): + """ + Change user page in Django's admin. + """ + def __init__(self, browser, user_pk): + super(ChangeUserAdminPage, self).__init__(browser) + self.user_pk = user_pk + + @property + def url(self): + """ + Returns the page URL for the page based on self.user_pk. + """ + + return u'{base}/admin/auth/user/{user_pk}/'.format( + base=BASE_URL, + user_pk=self.user_pk, + ) + + @property + def username(self): + """ + Reads the read-only username. + """ + return self.q(css='.field-username p').text[0] + + @property + def first_name_element(self): + """ + Selects the first name element. + """ + return self.q(css='[name="first_name"]') + + @property + def first_name(self): + """ + Reads the first name value from the input field. + """ + return self.first_name_element.attrs('value')[0] + + @property + def submit_element(self): + """ + Gets the "Save" submit element. + + Note that there are multiple submit elements in the change view. + """ + return self.q(css='input.default[type="submit"]') + + def submit(self): + """ + Submits the form. + """ + self.submit_element.click() + + def change_first_name(self, first_name): + """ + Changes the first name and submits the form. + + Args: + first_name: The first name as unicode. + + """ + + self.first_name_element.fill(first_name) + self.submit() + + def is_browser_on_page(self): + """ + Returns True if the browser is currently on the right page. + """ + return self.q(css='#user_form').present diff --git a/common/test/acceptance/tests/lms/test_unicode_username_admin.py b/common/test/acceptance/tests/lms/test_unicode_username_admin.py new file mode 100644 index 0000000000..dcd2a3d706 --- /dev/null +++ b/common/test/acceptance/tests/lms/test_unicode_username_admin.py @@ -0,0 +1,53 @@ +# -*- coding: utf-8 -*- +""" +End-to-end tests for admin change view. +""" + +from common.test.acceptance.pages.common.auto_auth import AutoAuthPage +from common.test.acceptance.pages.lms.admin import ChangeUserAdminPage +from common.test.acceptance.tests.helpers import AcceptanceTest + + +class UnicodeUsernameAdminTest(AcceptanceTest): + """ + Tests if it is possible to update users with unicode usernames in the admin. + """ + + # The word below reads "Omar II", in Arabic. It also contains a space and + # an Eastern Arabic Number another option is to use the Esperanto fake + # language but this was used instead to test non-western letters. + FIXTURE_USERNAME = u'عمر ٢' + + # From the db fixture `unicode_user.json` + FIXTURE_USER_ID = 1000 + + def setUp(self): + """ + Initializes and visits the change user admin page as a superuser. + """ + # Some state is constructed by the parent setUp() routine + super(UnicodeUsernameAdminTest, self).setUp() + + AutoAuthPage(self.browser, staff=True, superuser=True).visit() + + # Load page objects for use by the tests + self.page = ChangeUserAdminPage(self.browser, self.FIXTURE_USER_ID) + + # Navigate to the index page and get testing! + self.page.visit() + + def test_update_first_name(self): + """ + As a superuser I should be able to update the first name of a user with unicode username. + """ + self.assertNotEqual(self.page.first_name, 'John') + self.assertEquals(self.page.username, self.FIXTURE_USERNAME) + + self.page.change_first_name('John') + + self.assertFalse(self.page.is_browser_on_page(), 'Should redirect to the admin user list view on success') + + # Visit the page again to verify changes + self.page.visit() + + self.assertEquals(self.page.first_name, 'John', 'The first name should be updated') diff --git a/common/test/db_fixtures/unicode_user.json b/common/test/db_fixtures/unicode_user.json new file mode 100644 index 0000000000..fb0665832a --- /dev/null +++ b/common/test/db_fixtures/unicode_user.json @@ -0,0 +1,25 @@ +[ + { + "pk": 1000, + "model": "auth.user", + "fields": { + "date_joined": "2016-06-12 11:02:13.007790+00:00", + "username": "\u0639\u0645\u0631 \u0662", + "first_name": "Mike", + "last_name": "Doe", + "email":"unicode@example.com", + "password": "test", + "is_staff": false, + "is_active": true + } + }, + { + "pk": 1000, + "model": "student.userprofile", + "fields": { + "user": 1000, + "name": "John Doe", + "courseware": "course.xml" + } + } +] diff --git a/lms/djangoapps/teams/api_urls.py b/lms/djangoapps/teams/api_urls.py index 708057ed63..3ff1149847 100644 --- a/lms/djangoapps/teams/api_urls.py +++ b/lms/djangoapps/teams/api_urls.py @@ -13,7 +13,6 @@ from .views import ( ) TEAM_ID_PATTERN = r'(?P[a-z\d_-]+)' -USERNAME_PATTERN = r'(?P[\w.+-]+)' TOPIC_ID_PATTERN = r'(?P[A-Za-z\d_.-]+)' urlpatterns = patterns( @@ -24,7 +23,9 @@ urlpatterns = patterns( name="teams_list" ), url( - r'^v0/teams/' + TEAM_ID_PATTERN + '$', + r'^v0/teams/{team_id_pattern}$'.format( + team_id_pattern=TEAM_ID_PATTERN, + ), TeamsDetailView.as_view(), name="teams_detail" ), @@ -34,7 +35,10 @@ urlpatterns = patterns( name="topics_list" ), url( - r'^v0/topics/' + TOPIC_ID_PATTERN + ',' + settings.COURSE_ID_PATTERN + '$', + r'^v0/topics/{topic_id_pattern},{course_id_pattern}$'.format( + topic_id_pattern=TOPIC_ID_PATTERN, + course_id_pattern=settings.COURSE_ID_PATTERN, + ), TopicDetailView.as_view(), name="topics_detail" ), @@ -44,7 +48,10 @@ urlpatterns = patterns( name="team_membership_list" ), url( - r'^v0/team_membership/' + TEAM_ID_PATTERN + ',' + USERNAME_PATTERN + '$', + r'^v0/team_membership/{team_id_pattern},{username_pattern}$'.format( + team_id_pattern=TEAM_ID_PATTERN, + username_pattern=settings.USERNAME_PATTERN, + ), MembershipDetailView.as_view(), name="team_membership_detail" ) diff --git a/lms/envs/common.py b/lms/envs/common.py index 16f1c1bbc4..03bec41bbb 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -647,7 +647,13 @@ USAGE_KEY_PATTERN = r'(?P(?:i4x://?[^/]+/[^/]+/[^/]+/[^@]+(?:@ ASSET_KEY_PATTERN = r'(?P(?:/?c4x(:/)?/[^/]+/[^/]+/[^/]+/[^@]+(?:@[^/]+)?)|(?:[^/]+))' USAGE_ID_PATTERN = r'(?P(?:i4x://?[^/]+/[^/]+/[^/]+/[^@]+(?:@[^/]+)?)|(?:[^/]+))' -USERNAME_PATTERN = r'(?P[\w.@+-]+)' + +# The space is required for space-dependent languages like Arabic and Farsi. +# However, backward compatibility with Ficus older releases is still maintained (space is still not valid) +# in the AccountCreationForm and the user_api through the ENABLE_UNICODE_USERNAME feature flag. +USERNAME_REGEX_PARTIAL = r'[\w .@_+-]+' +USERNAME_PATTERN = r'(?P{regex})'.format(regex=USERNAME_REGEX_PARTIAL) + ############################## EVENT TRACKING ################################# LMS_SEGMENT_KEY = None diff --git a/lms/urls.py b/lms/urls.py index d98d716220..d0a41858a2 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -600,7 +600,9 @@ urlpatterns += ( # Student profile url( - r'^u/(?P[\w.@+-]+)$', + r'^u/{username_pattern}$'.format( + username_pattern=settings.USERNAME_PATTERN, + ), 'student_profile.views.learner_profile', name='learner_profile', ), diff --git a/openedx/core/djangoapps/user_api/accounts/__init__.py b/openedx/core/djangoapps/user_api/accounts/__init__.py index 02acd3f7b3..5441d168ff 100644 --- a/openedx/core/djangoapps/user_api/accounts/__init__.py +++ b/openedx/core/djangoapps/user_api/accounts/__init__.py @@ -12,7 +12,7 @@ USERNAME_MAX_LENGTH = 30 # The minimum and maximum length for the email account field EMAIL_MIN_LENGTH = 3 -EMAIL_MAX_LENGTH = 254 +EMAIL_MAX_LENGTH = 254 # Limit per RFCs is 254 # The minimum and maximum length for the password account field PASSWORD_MIN_LENGTH = 2 diff --git a/openedx/core/djangoapps/user_api/accounts/api.py b/openedx/core/djangoapps/user_api/accounts/api.py index 543a319a7d..852ac35c85 100644 --- a/openedx/core/djangoapps/user_api/accounts/api.py +++ b/openedx/core/djangoapps/user_api/accounts/api.py @@ -1,18 +1,19 @@ """ Programmatic integration point for User API Accounts sub-application """ -from django.utils.translation import ugettext as _ +from django.utils.translation import override as override_language, ugettext as _ from django.db import transaction, IntegrityError import datetime from pytz import UTC from django.core.exceptions import ObjectDoesNotExist from django.conf import settings -from django.core.validators import validate_email, validate_slug, ValidationError +from django.core.validators import validate_email, ValidationError from django.http import HttpResponseForbidden from openedx.core.djangoapps.user_api.preferences.api import update_user_preferences from openedx.core.djangoapps.user_api.errors import PreferenceValidationError from student.models import User, UserProfile, Registration +from student import forms as student_forms from student import views as student_views from util.model_utils import emit_setting_changed_event @@ -449,11 +450,12 @@ def _validate_username(username): ) ) try: - validate_slug(username) - except ValidationError: - raise AccountUsernameInvalid( - u"Username '{username}' must contain only A-Z, a-z, 0-9, -, or _ characters" - ) + with override_language('en'): + # `validate_username` provides a proper localized message, however the API needs only the English + # message by convention. + student_forms.validate_username(username) + except ValidationError as error: + raise AccountUsernameInvalid(error.message) def _validate_password(password, username): diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_api.py b/openedx/core/djangoapps/user_api/accounts/tests/test_api.py index 6b646be6e2..1fe87bc528 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_api.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_api.py @@ -460,3 +460,34 @@ class AccountCreationActivationAndPasswordChangeTest(TestCase): """ response = create_account(self.USERNAME, self.PASSWORD, self.EMAIL) self.assertEqual(response.status_code, 403) + + +@attr(shard=2) +@ddt.ddt +class AccountCreationUnicodeUsernameTest(TestCase): + """ + Test cases to cover the account initialization workflow + """ + PASSWORD = u'unicode-user-password' + EMAIL = u'unicode-user-username@example.com' + + UNICODE_USERNAMES = [ + u'Enchanté', + u'username_with_@', + u'username with spaces', + u'eastern_arabic_numbers_١٢٣', + ] + + @ddt.data(*UNICODE_USERNAMES) + def test_unicode_usernames(self, unicode_username): + with patch.dict(settings.FEATURES, {'ENABLE_UNICODE_USERNAME': False}): + with self.assertRaises(AccountUsernameInvalid): + create_account(unicode_username, self.PASSWORD, self.EMAIL) # Feature is disabled, therefore invalid. + + with patch.dict(settings.FEATURES, {'ENABLE_UNICODE_USERNAME': True}): + try: + create_account(unicode_username, self.PASSWORD, self.EMAIL) + except AccountUsernameInvalid: + self.fail(u'The API should accept Unicode username `{unicode_username}`.'.format( + unicode_username=unicode_username, + ))