diff --git a/common/djangoapps/student/forms.py b/common/djangoapps/student/forms.py index 0bc46f80cb..89f802c28d 100644 --- a/common/djangoapps/student/forms.py +++ b/common/djangoapps/student/forms.py @@ -23,18 +23,6 @@ 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 " @@ -127,12 +115,12 @@ def validate_username(username): username_re = slug_re flags = None - message = USERNAME_INVALID_CHARS_ASCII + message = accounts_settings.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 + message = accounts_settings.USERNAME_INVALID_CHARS_UNICODE validator = RegexValidator( regex=username_re, @@ -156,9 +144,9 @@ class UsernameField(forms.CharField): 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, + "required": accounts_settings.USERNAME_BAD_LENGTH_MSG, + "min_length": accounts_settings.USERNAME_BAD_LENGTH_MSG, + "max_length": accounts_settings.USERNAME_BAD_LENGTH_MSG, } ) diff --git a/common/djangoapps/student/tests/test_create_account.py b/common/djangoapps/student/tests/test_create_account.py index 7eb9b55772..2195970574 100644 --- a/common/djangoapps/student/tests/test_create_account.py +++ b/common/djangoapps/student/tests/test_create_account.py @@ -22,8 +22,8 @@ from notification_prefs import NOTIFICATION_PREF_KEY 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.accounts import USERNAME_INVALID_CHARS_ASCII, USERNAME_INVALID_CHARS_UNICODE 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 diff --git a/lms/djangoapps/student_account/test/test_views.py b/lms/djangoapps/student_account/test/test_views.py index e82d4b4ab0..20ae639da0 100644 --- a/lms/djangoapps/student_account/test/test_views.py +++ b/lms/djangoapps/student_account/test/test_views.py @@ -36,7 +36,6 @@ from openedx.core.djangoapps.oauth_dispatch.tests import factories as dot_factor from openedx.core.djangoapps.programs.tests.mixins import ProgramsApiConfigMixin from openedx.core.djangoapps.site_configuration.tests.mixins import SiteMixin from openedx.core.djangoapps.theming.tests.test_util import with_comprehensive_theme_context -from openedx.core.djangoapps.user_api.accounts import EMAIL_MAX_LENGTH from openedx.core.djangoapps.user_api.accounts.api import activate_account, create_account from openedx.core.djangolib.js_utils import dump_js_escaped_json from openedx.core.djangolib.testing.utils import CacheIsolationTestCase @@ -62,24 +61,6 @@ class StudentAccountUpdateTest(CacheIsolationTestCase, UrlResetMixin): NEW_EMAIL = u"walt@savewalterwhite.com" INVALID_ATTEMPTS = 100 - - INVALID_EMAILS = [ - None, - u"", - u"a", - "no_domain", - "no+domain", - "@", - "@domain.com", - "test@no_extension", - - # Long email -- subtract the length of the @domain - # except for one character (so we exceed the max length limit) - u"{user}@example.com".format( - user=(u'e' * (EMAIL_MAX_LENGTH - 11)) - ) - ] - INVALID_KEY = u"123abc" URLCONF_MODULES = ['student_accounts.urls'] diff --git a/openedx/core/djangoapps/user_api/accounts/__init__.py b/openedx/core/djangoapps/user_api/accounts/__init__.py index 5441d168ff..ec47c6a2b6 100644 --- a/openedx/core/djangoapps/user_api/accounts/__init__.py +++ b/openedx/core/djangoapps/user_api/accounts/__init__.py @@ -2,6 +2,9 @@ Account constants """ +from django.utils.translation import ugettext as _ + + # The minimum and maximum length for the name ("full name") account field NAME_MIN_LENGTH = 2 NAME_MAX_LENGTH = 255 @@ -25,3 +28,47 @@ ALL_USERS_VISIBILITY = 'all_users' # Indicates the user's preference that all their account information be private. PRIVATE_VISIBILITY = 'private' + +# Translators: This message is shown when the Unicode usernames are NOT allowed. +# It is shown to users who attempt to create a new account using invalid characters +# in the username. +USERNAME_INVALID_CHARS_ASCII = _( + u"Usernames can only contain letters (A-Z, a-z), numerals (0-9), underscores (_), and hyphens (-)." +) + +# Translators: This message is shown only when the Unicode usernames are allowed. +# It is shown to users who attempt to create a new account using invalid characters +# in the username. +USERNAME_INVALID_CHARS_UNICODE = _( + u"Usernames can only contain letters, numerals, and @/./+/-/_ characters." +) + +# Translators: This message is shown to users who attempt to create a new account using +# an invalid email format. +EMAIL_INVALID_MSG = _(u"Email '{email}' format is not valid") + +# Translators: This message is shown to users who attempt to create a new +# account using an username/email associated with an existing account. +EMAIL_CONFLICT_MSG = _( + u"It looks like {email_address} belongs to an existing account. " + u"Try again with a different email address." +) +USERNAME_CONFLICT_MSG = _( + u"It looks like {username} belongs to an existing account. " + u"Try again with a different username." +) + +# Translators: This message is shown to users who enter a username/email/password +# with an inappropriate length (too short or too long). +USERNAME_BAD_LENGTH_MSG = _(u"Username '{username}' must be between {min} and {max} characters long") +EMAIL_BAD_LENGTH_MSG = _(u"Email '{email}' must be between {min} and {max} characters long") +PASSWORD_BAD_LENGTH_MSG = _(u"Password must be between {min} and {max} characters long") + +# These strings are normally not user-facing. +USERNAME_BAD_TYPE_MSG = u"Username must be a string" +EMAIL_BAD_TYPE_MSG = u"Email must be a string" +PASSWORD_BAD_TYPE_MSG = u"Password must be a string" + +# Translators: This message is shown to users who enter a password matching +# the username they enter(ed). +PASSWORD_CANT_EQUAL_USERNAME_MSG = _(u"Password cannot be the same as the username") diff --git a/openedx/core/djangoapps/user_api/accounts/api.py b/openedx/core/djangoapps/user_api/accounts/api.py index 8fc3fcad65..1d503dabb7 100644 --- a/openedx/core/djangoapps/user_api/accounts/api.py +++ b/openedx/core/djangoapps/user_api/accounts/api.py @@ -1,3 +1,4 @@ +# -*- coding: utf-8 -*- """ Programmatic integration point for User API Accounts sub-application """ @@ -20,16 +21,23 @@ from util.model_utils import emit_setting_changed_event from openedx.core.lib.api.view_utils import add_serializer_errors from ..errors import ( - AccountUpdateError, AccountValidationError, AccountUsernameInvalid, AccountPasswordInvalid, - AccountEmailInvalid, AccountUserAlreadyExists, + AccountUpdateError, AccountValidationError, + AccountDataBadLength, AccountDataBadType, + AccountUsernameInvalid, AccountPasswordInvalid, AccountEmailInvalid, + AccountUserAlreadyExists, AccountUsernameAlreadyExists, AccountEmailAlreadyExists, UserAPIInternalError, UserAPIRequestError, UserNotFound, UserNotAuthorized ) from ..forms import PasswordResetFormNoActive from ..helpers import intercept_errors from . import ( - EMAIL_MIN_LENGTH, EMAIL_MAX_LENGTH, PASSWORD_MIN_LENGTH, PASSWORD_MAX_LENGTH, - USERNAME_MIN_LENGTH, USERNAME_MAX_LENGTH + EMAIL_BAD_LENGTH_MSG, PASSWORD_BAD_LENGTH_MSG, USERNAME_BAD_LENGTH_MSG, + EMAIL_BAD_TYPE_MSG, PASSWORD_BAD_TYPE_MSG, USERNAME_BAD_TYPE_MSG, + EMAIL_CONFLICT_MSG, USERNAME_CONFLICT_MSG, + EMAIL_INVALID_MSG, USERNAME_INVALID_MSG, + EMAIL_MIN_LENGTH, PASSWORD_MIN_LENGTH, USERNAME_MIN_LENGTH, + EMAIL_MAX_LENGTH, PASSWORD_MAX_LENGTH, USERNAME_MAX_LENGTH, + PASSWORD_CANT_EQUAL_USERNAME_MSG ) from .serializers import ( AccountLegacyProfileSerializer, AccountUserSerializer, @@ -70,6 +78,7 @@ def get_account_settings(request, usernames=None, configuration=None, view=None) UserNotFound: no user with username `username` exists (or `request.user.username` if `username` is not specified) UserAPIInternalError: the operation failed due to an unexpected error. + """ requesting_user = request.user usernames = usernames or [requesting_user.username] @@ -122,6 +131,7 @@ def update_account_settings(requesting_user, update, username=None): in particular, the user account (not including e-mail address) may have successfully been updated, but then the e-mail change request, which is processed last, may throw an error. UserAPIInternalError: the operation failed due to an unexpected error. + """ if username is None: username = requesting_user.username @@ -243,20 +253,6 @@ def update_account_settings(requesting_user, update, username=None): ) -def _get_user_and_profile(username): - """ - Helper method to return the legacy user and profile objects based on username. - """ - try: - existing_user = User.objects.get(username=username) - except ObjectDoesNotExist: - raise UserNotFound() - - existing_user_profile, _ = UserProfile.objects.get_or_create(user=existing_user) - - return existing_user, existing_user_profile - - @intercept_errors(UserAPIInternalError, ignore_errors=[UserAPIRequestError]) @transaction.atomic def create_account(username, password, email): @@ -296,6 +292,7 @@ def create_account(username, password, email): AccountEmailInvalid AccountPasswordInvalid UserAPIInternalError: the operation failed due to an unexpected error. + """ # Check if ALLOW_PUBLIC_ACCOUNT_CREATION flag turned off to restrict user account creation if not configuration_helpers.get_value( @@ -350,10 +347,13 @@ def check_account_exists(username=None, email=None): """ conflicts = [] - if email is not None and User.objects.filter(email=email).exists(): + try: + _validate_email_doesnt_exist(email) + except AccountEmailAlreadyExists: conflicts.append("email") - - if username is not None and User.objects.filter(username=username).exists(): + try: + _validate_username_doesnt_exist(username) + except AccountUsernameAlreadyExists: conflicts.append("username") return conflicts @@ -372,6 +372,7 @@ def activate_account(activation_key): Raises: UserNotAuthorized UserAPIInternalError: the operation failed due to an unexpected error. + """ try: registration = Registration.objects.get(activation_key=activation_key) @@ -400,6 +401,7 @@ def request_password_change(email, orig_host, is_secure): UserNotFound AccountRequestError UserAPIInternalError: the operation failed due to an unexpected error. + """ # Binding data to a form requires that the data be passed as a dictionary # to the Form class constructor. @@ -419,6 +421,101 @@ def request_password_change(email, orig_host, is_secure): raise UserNotFound +def get_username_validation_error(username, default=''): + """Get the built-in validation error message for when + the username is invalid in some way. + + :param username: The proposed username (unicode). + :param default: The message to default to in case of no error. + :return: Validation error message. + + """ + try: + _validate_username(username) + except AccountUsernameInvalid as invalid_username_err: + return invalid_username_err.message + return default + + +def get_email_validation_error(email, default=''): + """Get the built-in validation error message for when + the email is invalid in some way. + + :param email: The proposed email (unicode). + :param default: The message to default to in case of no error. + :return: Validation error message. + + """ + try: + _validate_email(email) + except AccountEmailInvalid as invalid_email_err: + return invalid_email_err.message + return default + + +def get_password_validation_error(password, username=None, default=''): + """Get the built-in validation error message for when + the password is invalid in some way. + + :param password: The proposed password (unicode). + :param username: The username associated with the user's account (unicode). + :param default: The message to default to in case of no error. + :return: Validation error message. + + """ + try: + _validate_password(password, username) + except AccountPasswordInvalid as invalid_password_err: + return invalid_password_err.message + return default + + +def get_username_existence_validation_error(username, default=''): + """Get the built-in validation error message for when + the username has an existence conflict. + + :param username: The proposed username (unicode). + :param default: The message to default to in case of no error. + :return: Validation error message. + + """ + try: + _validate_username_doesnt_exist(username) + except AccountUsernameAlreadyExists as username_exists_err: + return username_exists_err.message + return default + + +def get_email_existence_validation_error(email, default=''): + """Get the built-in validation error message for when + the email has an existence conflict. + + :param email: The proposed email (unicode). + :param default: The message to default to in case of no error. + :return: Validation error message. + + """ + try: + _validate_email_doesnt_exist(email) + except AccountEmailAlreadyExists as email_exists_err: + return email_exists_err.message + return default + + +def _get_user_and_profile(username): + """ + Helper method to return the legacy user and profile objects based on username. + """ + try: + existing_user = User.objects.get(username=username) + except ObjectDoesNotExist: + raise UserNotFound() + + existing_user_profile, _ = UserProfile.objects.get_or_create(user=existing_user) + + return existing_user, existing_user_profile + + def _validate_username(username): """Validate the username. @@ -432,33 +529,54 @@ def _validate_username(username): AccountUsernameInvalid """ - if not isinstance(username, basestring): - raise AccountUsernameInvalid(u"Username must be a string") - - if len(username) < USERNAME_MIN_LENGTH: - raise AccountUsernameInvalid( - u"Username '{username}' must be at least {min} characters long".format( - username=username, - min=USERNAME_MIN_LENGTH - ) - ) - if len(username) > USERNAME_MAX_LENGTH: - raise AccountUsernameInvalid( - u"Username '{username}' must be at most {max} characters long".format( + try: + _validate_unicode(username) + _validate_type(username, basestring, USERNAME_BAD_TYPE_MSG) + _validate_length( + username, USERNAME_MIN_LENGTH, USERNAME_MAX_LENGTH, USERNAME_BAD_LENGTH_MSG.format( username=username, + min=USERNAME_MIN_LENGTH, max=USERNAME_MAX_LENGTH ) ) - try: 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) + except (UnicodeError, AccountDataBadType, AccountDataBadLength, ValidationError) as invalid_username_err: + raise AccountUsernameInvalid(invalid_username_err.message) -def _validate_password(password, username): +def _validate_email(email): + """Validate the format of the email address. + + Arguments: + email (unicode): The proposed email. + + Returns: + None + + Raises: + AccountEmailInvalid + + """ + try: + _validate_unicode(email) + _validate_type(email, basestring, EMAIL_BAD_TYPE_MSG) + _validate_length( + email, EMAIL_MIN_LENGTH, EMAIL_MAX_LENGTH, EMAIL_BAD_LENGTH_MSG.format( + email=email, + min=EMAIL_MIN_LENGTH, + max=EMAIL_MAX_LENGTH + ) + ) + validate_email.message = EMAIL_INVALID_MSG.format(email=email) + validate_email(email) + except (UnicodeError, AccountDataBadType, AccountDataBadLength, ValidationError) as invalid_email_err: + raise AccountEmailInvalid(invalid_email_err.message) + + +def _validate_password(password, username=None): """Validate the format of the user's password. Passwords cannot be the same as the username of the account, @@ -475,62 +593,99 @@ def _validate_password(password, username): AccountPasswordInvalid """ - if not isinstance(password, basestring): - raise AccountPasswordInvalid(u"Password must be a string") - - if len(password) < PASSWORD_MIN_LENGTH: - raise AccountPasswordInvalid( - u"Password must be at least {min} characters long".format( - min=PASSWORD_MIN_LENGTH - ) - ) - - if len(password) > PASSWORD_MAX_LENGTH: - raise AccountPasswordInvalid( - u"Password must be at most {max} characters long".format( + try: + _validate_type(password, basestring, PASSWORD_BAD_TYPE_MSG) + _validate_length( + password, PASSWORD_MIN_LENGTH, PASSWORD_MAX_LENGTH, PASSWORD_BAD_LENGTH_MSG.format( + min=PASSWORD_MIN_LENGTH, max=PASSWORD_MAX_LENGTH ) ) + _validate_password_works_with_username(password, username) + except (AccountDataBadType, AccountDataBadLength) as invalid_password_err: + raise AccountPasswordInvalid(invalid_password_err.message) + +def _validate_username_doesnt_exist(username): + """Validate that the username is not associated with an existing user. + + :param username: The proposed username (unicode). + :return: None + :raises: AccountUsernameAlreadyExists + """ + if username is not None and User.objects.filter(username=username).exists(): + raise AccountUsernameAlreadyExists(_(USERNAME_CONFLICT_MSG).format(username=username)) + + +def _validate_email_doesnt_exist(email): + """Validate that the email is not associated with an existing user. + + :param email: The proposed email (unicode). + :return: None + :raises: AccountEmailAlreadyExists + """ + if email is not None and User.objects.filter(email=email).exists(): + raise AccountEmailAlreadyExists(_(EMAIL_CONFLICT_MSG).format(email_address=email)) + + +def _validate_password_works_with_username(password, username=None): + """Run validation checks on whether the password and username + go well together. + + An example check is to see whether they are the same. + + :param password: The proposed password (unicode). + :param username: The username associated with the user's account (unicode). + :return: None + :raises: AccountPasswordInvalid + """ if password == username: - raise AccountPasswordInvalid(u"Password cannot be the same as the username") + raise AccountPasswordInvalid(PASSWORD_CANT_EQUAL_USERNAME_MSG) -def _validate_email(email): - """Validate the format of the email address. +def _validate_type(data, type, err): + """Checks whether the input data is of type. If not, + throws a generic error message. - Arguments: - email (unicode): The proposed email. - - Returns: - None - - Raises: - AccountEmailInvalid + :param data: The data to check. + :param type: The type to check against. + :param err: The error message to throw back if data is not of type. + :return: None + :raises: AccountDataBadType """ - if not isinstance(email, basestring): - raise AccountEmailInvalid(u"Email must be a string") + if not isinstance(data, type): + raise AccountDataBadType(err) - if len(email) < EMAIL_MIN_LENGTH: - raise AccountEmailInvalid( - u"Email '{email}' must be at least {min} characters long".format( - email=email, - min=EMAIL_MIN_LENGTH - ) - ) - if len(email) > EMAIL_MAX_LENGTH: - raise AccountEmailInvalid( - u"Email '{email}' must be at most {max} characters long".format( - email=email, - max=EMAIL_MAX_LENGTH - ) - ) +def _validate_length(data, min, max, err): + """Validate that the data's length is less than or equal to max, + and greater than or equal to min. + :param data: The data to do the test on. + :param min: The minimum allowed length. + :param max: The maximum allowed length. + :return: None + :raises: AccountDataBadLength + + """ + if len(data) < min or len(data) > max: + raise AccountDataBadLength(err) + + +def _validate_unicode(data, err=u"Input not valid unicode"): + """Checks whether the input data is valid unicode or not. + + :param data: The data to check for unicode validity. + :param err: The error message to throw back if unicode is invalid. + :return: None + :raises: UnicodeError + + """ try: - validate_email(email) - except ValidationError: - raise AccountEmailInvalid( - u"Email '{email}' format is not valid".format(email=email) - ) + if not isinstance(data, str) and not isinstance(data, unicode): + raise UnicodeError + # In some cases we pass the above, but it's still inappropriate utf-8. + str(data) + except UnicodeError: + raise UnicodeError(err) 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 129cf9883e..2928807f59 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_api.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_api.py @@ -3,6 +3,7 @@ Unit tests for behavior that is specific to the api methods (vs. the view methods). Most of the functionality is covered in test_views.py. """ + import re import ddt from dateutil.parser import parse as parse_datetime @@ -17,17 +18,29 @@ from django.conf import settings from django.contrib.auth.models import User from django.core import mail from django.test.client import RequestFactory +from openedx.core.djangoapps.user_api.accounts import ( + USERNAME_MAX_LENGTH, + PRIVATE_VISIBILITY +) +from openedx.core.djangoapps.user_api.accounts.api import ( + get_account_settings, + update_account_settings, + create_account, + activate_account, + request_password_change +) +from openedx.core.djangoapps.user_api.errors import ( + UserNotFound, UserNotAuthorized, + AccountUpdateError, AccountValidationError, AccountUserAlreadyExists, + AccountUsernameInvalid, AccountEmailInvalid, AccountPasswordInvalid, + AccountRequestError +) +from openedx.core.djangoapps.user_api.accounts.tests.testutils import ( + INVALID_EMAILS, INVALID_PASSWORDS, INVALID_USERNAMES +) from openedx.core.djangolib.testing.utils import skip_unless_lms from student.models import PendingEmailChange from student.tests.tests import UserSettingsEventTestMixin -from ...errors import ( - UserNotFound, UserNotAuthorized, AccountUpdateError, AccountValidationError, - AccountUserAlreadyExists, AccountUsernameInvalid, AccountEmailInvalid, AccountPasswordInvalid, AccountRequestError -) -from ..api import ( - get_account_settings, update_account_settings, create_account, activate_account, request_password_change -) -from .. import USERNAME_MAX_LENGTH, EMAIL_MAX_LENGTH, PASSWORD_MAX_LENGTH, PRIVATE_VISIBILITY def mock_render_to_string(template_name, context): @@ -310,40 +323,6 @@ class AccountCreationActivationAndPasswordChangeTest(TestCase): ORIG_HOST = 'example.com' IS_SECURE = False - INVALID_USERNAMES = [ - None, - u'', - u'a', - u'a' * (USERNAME_MAX_LENGTH + 1), - u'invalid_symbol_@', - u'invalid-unicode_fŕáńḱ', - ] - - INVALID_EMAILS = [ - None, - u'', - u'a', - 'no_domain', - 'no+domain', - '@', - '@domain.com', - 'test@no_extension', - u'fŕáńḱ@example.com', - - # Long email -- subtract the length of the @domain - # except for one character (so we exceed the max length limit) - u'{user}@example.com'.format( - user=(u'e' * (EMAIL_MAX_LENGTH - 11)) - ) - ] - - INVALID_PASSWORDS = [ - None, - u'', - u'a', - u'a' * (PASSWORD_MAX_LENGTH + 1) - ] - @skip_unless_lms def test_activate_account(self): # Create the account, which is initially inactive diff --git a/openedx/core/djangoapps/user_api/accounts/tests/testutils.py b/openedx/core/djangoapps/user_api/accounts/tests/testutils.py new file mode 100644 index 0000000000..b5ab4cb187 --- /dev/null +++ b/openedx/core/djangoapps/user_api/accounts/tests/testutils.py @@ -0,0 +1,74 @@ +# -*- coding: utf-8 -*- +""" +Utility functions, constants, etc. for testing. +""" + +from openedx.core.djangoapps.user_api.accounts import ( + USERNAME_MIN_LENGTH, USERNAME_MAX_LENGTH, + EMAIL_MAX_LENGTH, + PASSWORD_MIN_LENGTH, PASSWORD_MAX_LENGTH +) + + +INVALID_USERNAMES_ASCII = [ + '$invalid-ascii$', + 'invalid-fŕáńḱ', + '@invalid-ascii@' +] + +INVALID_USERNAMES_UNICODE = [ + u'invalid-unicode_fŕáńḱ', +] + +INVALID_USERNAMES = [ + None, + u'', + u'a', + u'a' * (USERNAME_MAX_LENGTH + 1), +] + INVALID_USERNAMES_ASCII + INVALID_USERNAMES_UNICODE + +INVALID_EMAILS = [ + None, + u'', + u'a', + 'no_domain', + 'no+domain', + '@', + '@domain.com', + 'test@no_extension', + u'fŕáńḱ@example.com', + + # Long email -- subtract the length of the @domain + # except for one character (so we exceed the max length limit) + u'{user}@example.com'.format( + user=(u'e' * (EMAIL_MAX_LENGTH - 11)) + ) +] + +INVALID_PASSWORDS = [ + None, + u'', + u'a', + u'a' * (PASSWORD_MAX_LENGTH + 1) +] + +VALID_USERNAMES = [ + u'username', + u'a' * USERNAME_MIN_LENGTH, + u'a' * USERNAME_MAX_LENGTH, + u'-' * USERNAME_MIN_LENGTH, + u'-' * USERNAME_MAX_LENGTH, + u'_username_', + u'-username-', + u'-_username_-' +] + +VALID_EMAILS = [ + 'has@domain.com' +] + +VALID_PASSWORDS = [ + u'password', # :) + u'a' * PASSWORD_MIN_LENGTH, + u'a' * PASSWORD_MAX_LENGTH +] diff --git a/openedx/core/djangoapps/user_api/errors.py b/openedx/core/djangoapps/user_api/errors.py index 1930d31365..9bd6be4a00 100644 --- a/openedx/core/djangoapps/user_api/errors.py +++ b/openedx/core/djangoapps/user_api/errors.py @@ -33,6 +33,16 @@ class AccountUserAlreadyExists(AccountRequestError): pass +class AccountUsernameAlreadyExists(AccountRequestError): + """User with the same username already exists. """ + pass + + +class AccountEmailAlreadyExists(AccountRequestError): + """User with the same email already exists. """ + pass + + class AccountUsernameInvalid(AccountRequestError): """The requested username is not in a valid format. """ pass @@ -48,6 +58,16 @@ class AccountPasswordInvalid(AccountRequestError): pass +class AccountDataBadLength(AccountRequestError): + """The requested account data is either too short or too long. """ + pass + + +class AccountDataBadType(AccountRequestError): + """The requested account data is of the wrong type. """ + pass + + class AccountUpdateError(AccountRequestError): """ An update to the account failed. More detailed information is present in developer_message, diff --git a/openedx/core/djangoapps/user_api/tests/test_views.py b/openedx/core/djangoapps/user_api/tests/test_views.py index 389407bb6b..a5d7ed3b36 100644 --- a/openedx/core/djangoapps/user_api/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/tests/test_views.py @@ -31,7 +31,6 @@ from third_party_auth.tests.utils import ( from .test_helpers import TestCaseForm from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory -from ..helpers import FormDescription from ..accounts import ( NAME_MAX_LENGTH, EMAIL_MIN_LENGTH, EMAIL_MAX_LENGTH, PASSWORD_MIN_LENGTH, PASSWORD_MAX_LENGTH, USERNAME_MIN_LENGTH, USERNAME_MAX_LENGTH diff --git a/openedx/core/djangoapps/user_api/urls.py b/openedx/core/djangoapps/user_api/urls.py index 38891f488e..ad11c09998 100644 --- a/openedx/core/djangoapps/user_api/urls.py +++ b/openedx/core/djangoapps/user_api/urls.py @@ -9,6 +9,7 @@ from ..profile_images.views import ProfileImageView from .accounts.views import AccountDeactivationView, AccountViewSet from .preferences.views import PreferencesDetailView, PreferencesView from .verification_api.views import PhotoVerificationStatusView +from .validation.views import RegistrationValidationView ME = AccountViewSet.as_view({ 'get': 'get', @@ -25,9 +26,21 @@ ACCOUNT_DETAIL = AccountViewSet.as_view({ urlpatterns = patterns( '', - url(r'^v1/me$', ME, name='own_username_api'), - url(r'^v1/accounts/{}$'.format(settings.USERNAME_PATTERN), ACCOUNT_DETAIL, name='accounts_api'), - url(r'^v1/accounts$', ACCOUNT_LIST, name='accounts_detail_api'), + url( + r'^v1/me$', + ME, + name='own_username_api' + ), + url( + r'^v1/accounts$', + ACCOUNT_LIST, + name='accounts_detail_api' + ), + url( + r'^v1/accounts/{}$'.format(settings.USERNAME_PATTERN), + ACCOUNT_DETAIL, + name='accounts_api' + ), url( r'^v1/accounts/{}/image$'.format(settings.USERNAME_PATTERN), ProfileImageView.as_view(), @@ -43,6 +56,11 @@ urlpatterns = patterns( PhotoVerificationStatusView.as_view(), name='verification_status' ), + url( + r'^v1/validation/registration$', + RegistrationValidationView.as_view(), + name='registration_validation' + ), url( r'^v1/preferences/{}$'.format(settings.USERNAME_PATTERN), PreferencesView.as_view(), diff --git a/openedx/core/djangoapps/user_api/validation/__init__.py b/openedx/core/djangoapps/user_api/validation/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/openedx/core/djangoapps/user_api/validation/tests/__init__.py b/openedx/core/djangoapps/user_api/validation/tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/openedx/core/djangoapps/user_api/validation/tests/test_views.py b/openedx/core/djangoapps/user_api/validation/tests/test_views.py new file mode 100644 index 0000000000..566c1b1356 --- /dev/null +++ b/openedx/core/djangoapps/user_api/validation/tests/test_views.py @@ -0,0 +1,171 @@ +# -*- coding: utf-8 -*- +""" +Tests for an API endpoint for client-side user data validation. +""" + +import unittest + +import ddt +from django.conf import settings +from django.contrib.auth.models import User +from django.core.urlresolvers import reverse + +from openedx.core.djangoapps.user_api.accounts import ( + EMAIL_BAD_LENGTH_MSG, EMAIL_INVALID_MSG, + EMAIL_CONFLICT_MSG, EMAIL_MAX_LENGTH, EMAIL_MIN_LENGTH, + PASSWORD_BAD_LENGTH_MSG, PASSWORD_CANT_EQUAL_USERNAME_MSG, + PASSWORD_MAX_LENGTH, PASSWORD_MIN_LENGTH, + USERNAME_BAD_LENGTH_MSG, USERNAME_INVALID_CHARS_ASCII, USERNAME_INVALID_CHARS_UNICODE, + USERNAME_CONFLICT_MSG, USERNAME_MAX_LENGTH, USERNAME_MIN_LENGTH +) +from openedx.core.djangoapps.user_api.accounts.tests.testutils import ( + VALID_EMAILS, VALID_PASSWORDS, VALID_USERNAMES, + INVALID_EMAILS, INVALID_PASSWORDS, INVALID_USERNAMES, + INVALID_USERNAMES_ASCII, INVALID_USERNAMES_UNICODE +) +from openedx.core.lib.api import test_utils + + +@ddt.ddt +class RegistrationValidationViewTests(test_utils.ApiTestCase): + """ + Tests for validity of user data in registration forms. + """ + + endpoint_name = 'registration_validation' + path = reverse(endpoint_name) + + def get_validation_decision(self, data): + response = self.client.post(self.path, data) + return response.data.get('validation_decisions', {}) + + def assertValidationDecision(self, data, decision): + self.assertEqual( + self.get_validation_decision(data), + decision + ) + + def test_no_decision_for_empty_request(self): + self.assertValidationDecision({}, {}) + + def test_no_decision_for_invalid_request(self): + self.assertValidationDecision({'invalid_field': 'random_user_data'}, {}) + + @ddt.data( + ['email', (email for email in VALID_EMAILS)], + ['password', (password for password in VALID_PASSWORDS)], + ['username', (username for username in VALID_USERNAMES)] + ) + @ddt.unpack + def test_positive_validation_decision(self, form_field_name, user_data): + """ + Test if {0} as any item in {1} gives a positive validation decision. + """ + self.assertValidationDecision( + {form_field_name: user_data}, + {form_field_name: ''} + ) + + @ddt.data( + # Skip None type for invalidity checks. + ['email', (email for email in INVALID_EMAILS[1:])], + ['password', (password for password in INVALID_PASSWORDS[1:])], + ['username', (username for username in INVALID_USERNAMES[1:])] + ) + @ddt.unpack + def test_negative_validation_decision(self, form_field_name, user_data): + """ + Test if {0} as any item in {1} gives a negative validation decision. + """ + self.assertNotEqual( + self.get_validation_decision({form_field_name: user_data}), + {form_field_name: ''} + ) + + @ddt.data( + ['username', 'username@email.com'], # No conflict + ['user', 'username@email.com'], # Username conflict + ['username', 'user@email.com'], # Email conflict + ['user', 'user@email.com'] # Both conflict + ) + @ddt.unpack + def test_existence_conflict(self, username, email): + """ + Test if username '{0}' and email '{1}' have conflicts with + username 'user' and email 'user@email.com'. + """ + user = User.objects.create_user(username='user', email='user@email.com') + self.assertValidationDecision( + { + 'username': username, + 'email': email + }, + { + "username": USERNAME_CONFLICT_MSG.format(username=user.username) if username == user.username else '', + "email": EMAIL_CONFLICT_MSG.format(email_address=user.email) if email == user.email else '' + } + ) + + @ddt.data('', ('e' * EMAIL_MAX_LENGTH) + '@email.com') + def test_email_less_than_min_length_validation_decision(self, email): + self.assertValidationDecision( + {'email': email}, + {'email': EMAIL_BAD_LENGTH_MSG.format(email=email, min=EMAIL_MIN_LENGTH, max=EMAIL_MAX_LENGTH)} + ) + + def test_email_generically_invalid_validation_decision(self): + email = 'email' + self.assertValidationDecision( + {'email': email}, + {'email': EMAIL_INVALID_MSG.format(email=email)} + ) + + @ddt.data( + 'u' * (USERNAME_MIN_LENGTH - 1), + 'u' * (USERNAME_MAX_LENGTH + 1) + ) + def test_username_less_than_min_length_validation_decision(self, username): + self.assertValidationDecision( + {'username': username}, + { + 'username': USERNAME_BAD_LENGTH_MSG.format( + username=username, + min=USERNAME_MIN_LENGTH, + max=USERNAME_MAX_LENGTH + ) + } + ) + + @unittest.skipUnless(settings.FEATURES.get("ENABLE_UNICODE_USERNAME"), "Unicode usernames disabled.") + @ddt.data(*INVALID_USERNAMES_UNICODE) + @ddt.unpack + def test_username_invalid_unicode_validation_decision(self, username): + self.assertValidationDecision( + {'username': username}, + {'username': USERNAME_INVALID_CHARS_UNICODE} + ) + + @unittest.skipIf(settings.FEATURES.get("ENABLE_UNICODE_USERNAME"), "Unicode usernames enabled.") + @ddt.data(*INVALID_USERNAMES_ASCII) + @ddt.unpack + def test_username_invalid_ascii_validation_decision(self, username): + self.assertValidationDecision( + {'username': username}, + {"username": USERNAME_INVALID_CHARS_ASCII} + ) + + @ddt.data( + 'p' * (PASSWORD_MIN_LENGTH - 1), + 'p' * (PASSWORD_MAX_LENGTH + 1) + ) + def test_password_less_than_min_length_validation_decision(self, password): + self.assertValidationDecision( + {'password': password}, + {"password": PASSWORD_BAD_LENGTH_MSG.format(min=PASSWORD_MIN_LENGTH, max=PASSWORD_MAX_LENGTH)} + ) + + def test_password_equals_username_validation_decision(self): + self.assertValidationDecision( + {"username": "somephrase", "password": "somephrase"}, + {"username": "", "password": PASSWORD_CANT_EQUAL_USERNAME_MSG} + ) diff --git a/openedx/core/djangoapps/user_api/validation/views.py b/openedx/core/djangoapps/user_api/validation/views.py new file mode 100644 index 0000000000..8d90fb1617 --- /dev/null +++ b/openedx/core/djangoapps/user_api/validation/views.py @@ -0,0 +1,146 @@ +# -*- coding: utf-8 -*- +""" +An API for client-side validation of (potential) user data. +""" + +from rest_framework.response import Response +from rest_framework.views import APIView + +from openedx.core.djangoapps.user_api.accounts.api import ( + get_email_validation_error, + get_email_existence_validation_error, + get_password_validation_error, + get_username_validation_error, + get_username_existence_validation_error +) + + +class RegistrationValidationView(APIView): + """ + **Use Cases** + + Get validation information about user data during registration. + Client-side may request validation for any number of form fields, + and the API will return a conclusion from its analysis for each + input (i.e. valid or not valid, or a custom, detailed message). + + **Example Requests and Responses** + + - Checks the validity of the username and email inputs separately. + POST /api/user/v1/validation/registration/ + >>> { + >>> "username": "hi_im_new", + >>> "email": "newguy101@edx.org" + >>> } + RESPONSE + >>> { + >>> "validation_decisions": { + >>> "username": "", + >>> "email": "" + >>> } + >>> } + Empty strings indicate that there was no problem with the input. + + - Checks the validity of the password field (its validity depends + upon both the username and password fields, so we need both). If + only password is input, we don't check for password/username + compatibility issues. + POST /api/user/v1/validation/registration/ + >>> { + >>> "username": "myname", + >>> "password": "myname" + >>> } + RESPONSE + >>> { + >>> "validation_decisions": { + >>> "username": "", + >>> "password": "Password cannot be the same as the username" + >>> } + >>> } + + - Checks the validity of the username, email, and password fields + separately, and also tells whether an account exists. The password + field's validity depends upon both the username and password, and + the account's existence depends upon both the username and email. + POST /api/user/v1/validation/registration/ + >>> { + >>> "username": "hi_im_new", + >>> "email": "cto@edx.org", + >>> "password": "p" + >>> } + RESPONSE + >>> { + >>> "validation_decisions": { + >>> "username": "", + >>> "email": "It looks like cto@edx.org belongs to an existing account. Try again with a different email address.", + >>> "password": "Password must be at least 2 characters long", + >>> } + >>> } + In this example, username is valid and (we assume) there is + a preexisting account with that email. The password also seems + to contain the username. + + Note that a validation decision is returned *for all* inputs, whether + positive or negative. + + **Available Handlers** + + "username": + A handler to check the validity of usernames. + "email": + A handler to check the validity of emails. + "password": + A handler to check the validity of passwords; a compatibility + decision with the username is made if it exists in the input. + """ + + def username_handler(self, request): + username = request.data.get('username') + invalid_username_error = get_username_validation_error(username) + username_exists_error = get_username_existence_validation_error(username) + # Existing usernames are already valid, so we prefer that error. + return username_exists_error or invalid_username_error + + def email_handler(self, request): + email = request.data.get('email') + invalid_email_error = get_email_validation_error(email) + email_exists_error = get_email_existence_validation_error(email) + # Existing emails are already valid, so we prefer that error. + return email_exists_error or invalid_email_error + + def password_handler(self, request): + username = request.data.get('username') or None + password = request.data.get('password') + return get_password_validation_error(password, username) + + validation_handlers = { + "username": username_handler, + "email": email_handler, + "password": password_handler, + } + + def post(self, request): + """ + POST /api/user/v1/validation/registration/ + + Expects request of the form + >>> { + >>> "username": "mslm", + >>> "email": "mslm@gmail.com", + >>> "password": "password123" + >>> } + where each key is the appropriate form field name and the value is + user input. One may enter individual inputs if needed. Some inputs + can get extra verification checks if entered along with others, + like when the password may not equal the username. + """ + validation_decisions = {} + for form_field_key in self.validation_handlers: + # For every field requiring validation from the client, + # request a decision for it from the appropriate handler. + if form_field_key in request.data: + handler = self.validation_handlers[form_field_key] + validation_decisions.update({ + form_field_key: handler(self, request) + }) + return Response({"validation_decisions": validation_decisions}) diff --git a/openedx/core/djangoapps/user_api/views.py b/openedx/core/djangoapps/user_api/views.py index 487a31477b..61735b417f 100644 --- a/openedx/core/djangoapps/user_api/views.py +++ b/openedx/core/djangoapps/user_api/views.py @@ -32,13 +32,12 @@ from student.views import create_account_with_params, AccountValidationError from util.json_request import JsonResponse from .accounts import ( - EMAIL_MAX_LENGTH, - EMAIL_MIN_LENGTH, + EMAIL_MAX_LENGTH, EMAIL_MIN_LENGTH, NAME_MAX_LENGTH, - PASSWORD_MAX_LENGTH, - PASSWORD_MIN_LENGTH, - USERNAME_MAX_LENGTH, - USERNAME_MIN_LENGTH + PASSWORD_MAX_LENGTH, PASSWORD_MIN_LENGTH, + USERNAME_MAX_LENGTH, USERNAME_MIN_LENGTH, + EMAIL_CONFLICT_MSG, + USERNAME_CONFLICT_MSG ) from .accounts.api import check_account_exists from .helpers import FormDescription, require_post_params, shim_student_view @@ -340,18 +339,8 @@ class RegistrationView(APIView): conflicts = check_account_exists(email=email, username=username) if conflicts: conflict_messages = { - "email": _( - # Translators: This message is shown to users who attempt to create a new - # account using an email address associated with an existing account. - u"It looks like {email_address} belongs to an existing account. " - u"Try again with a different email address." - ).format(email_address=email), - "username": _( - # Translators: This message is shown to users who attempt to create a new - # account using a username associated with an existing account. - u"It looks like {username} belongs to an existing account. " - u"Try again with a different username." - ).format(username=username), + "email": EMAIL_CONFLICT_MSG.format(email_address=email), + "username": USERNAME_CONFLICT_MSG.format(username=username), } errors = { field: [{"user_message": conflict_messages[field]}] diff --git a/openedx/core/lib/api/test_utils.py b/openedx/core/lib/api/test_utils.py index 181429add3..eb8c2a2aa3 100644 --- a/openedx/core/lib/api/test_utils.py +++ b/openedx/core/lib/api/test_utils.py @@ -28,7 +28,7 @@ class ApiTestCase(TestCase): return getattr(self.client, method)(*args, HTTP_X_EDX_API_KEY=TEST_API_KEY, **kwargs) def get_json(self, *args, **kwargs): - """Make a request with the given args and return the parsed JSON repsonse""" + """Make a request with the given args and return the parsed JSON response""" resp = self.request_with_auth("get", *args, **kwargs) self.assertHttpOK(resp) self.assertTrue(resp["Content-Type"].startswith("application/json"))