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

Add optional support for Unicode usernames

Browse Source 
Refactoring: Use format with named variables
This commit is contained in:
Omar Al-Ithawi
2017-03-20 19:13:28 +02:00
parent e6c94400d1
commit 1b46c3e646
16 changed files with 423 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
cms/envs/common.py
View File

@@ -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<username>[\w.@+-]+)'
# Partner support link for CMS footer
PARTNER_SUPPORT_EMAIL = ''

9
common/djangoapps/student/admin.py
View File

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

99
common/djangoapps/student/forms.py
View File

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

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

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

68
common/djangoapps/student/tests/test_create_account.py
View File

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

16
common/djangoapps/third_party_auth/api/urls.py
View File

@@ -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<username>[\w.+-]+)'
PROVIDER_PATTERN = r'(?P<provider_id>[\w.+-]+)(?:\:(?P<idp_slug>[\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',
),
)

9
common/test/acceptance/pages/common/auto_auth.py
View File

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

79
common/test/acceptance/pages/lms/admin.py Normal file
View File

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

53
common/test/acceptance/tests/lms/test_unicode_username_admin.py Normal file
View File

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

25
common/test/db_fixtures/unicode_user.json Normal file
View File

@@ -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"
}
}
]

15
lms/djangoapps/teams/api_urls.py
View File

@@ -13,7 +13,6 @@ from .views import (
)
TEAM_ID_PATTERN = r'(?P<team_id>[a-z\d_-]+)'
USERNAME_PATTERN = r'(?P<username>[\w.+-]+)'
TOPIC_ID_PATTERN = r'(?P<topic_id>[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"
)

8
lms/envs/common.py
View File

@@ -647,7 +647,13 @@ USAGE_KEY_PATTERN = r'(?P<usage_key_string>(?:i4x://?[^/]+/[^/]+/[^/]+/[^@]+(?:@
ASSET_KEY_PATTERN = r'(?P<asset_key_string>(?:/?c4x(:/)?/[^/]+/[^/]+/[^/]+/[^@]+(?:@[^/]+)?)|(?:[^/]+))'
USAGE_ID_PATTERN = r'(?P<usage_id>(?:i4x://?[^/]+/[^/]+/[^/]+/[^@]+(?:@[^/]+)?)|(?:[^/]+))'
USERNAME_PATTERN = r'(?P<username>[\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<username>{regex})'.format(regex=USERNAME_REGEX_PARTIAL)
############################## EVENT TRACKING #################################
LMS_SEGMENT_KEY = None

4
lms/urls.py
View File

@@ -600,7 +600,9 @@ urlpatterns += (
# Student profile
url(
r'^u/(?P<username>[\w.@+-]+)$',
r'^u/{username_pattern}$'.format(
username_pattern=settings.USERNAME_PATTERN,
),
'student_profile.views.learner_profile',
name='learner_profile',
),

2
openedx/core/djangoapps/user_api/accounts/__init__.py
View File

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

16
openedx/core/djangoapps/user_api/accounts/api.py
View File

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

31
openedx/core/djangoapps/user_api/accounts/tests/test_api.py
View File

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