diff --git a/openedx/core/djangoapps/user_api/accounts/__init__.py b/openedx/core/djangoapps/user_api/accounts/__init__.py index 04080e1776..7cc88ec3ca 100644 --- a/openedx/core/djangoapps/user_api/accounts/__init__.py +++ b/openedx/core/djangoapps/user_api/accounts/__init__.py @@ -53,6 +53,7 @@ USERNAME_INVALID_CHARS_UNICODE = _( # Translators: This message is shown to users who attempt to create a new account using # an invalid email format. EMAIL_INVALID_MSG = _('"{email}" is not a valid email address.') +AUTHN_EMAIL_INVALID_MSG = _('Enter a valid email address') # Translators: This message is shown to users who attempt to create a new # account using an username/email associated with an existing account. @@ -60,10 +61,12 @@ EMAIL_CONFLICT_MSG = _( "It looks like {email_address} belongs to an existing account. " "Try again with a different email address." ) +AUTHN_EMAIL_CONFLICT_MSG = _("It looks like this email address is already registered") USERNAME_CONFLICT_MSG = _( "It looks like {username} belongs to an existing account. " "Try again with a different username." ) +AUTHN_USERNAME_CONFLICT_MSG = _("It looks like this username is already taken") # Translators: This message is shown to users who enter a username/email/password # with an inappropriate length (too short or too long). diff --git a/openedx/core/djangoapps/user_api/accounts/api.py b/openedx/core/djangoapps/user_api/accounts/api.py index 6713098ce2..fc46984932 100644 --- a/openedx/core/djangoapps/user_api/accounts/api.py +++ b/openedx/core/djangoapps/user_api/accounts/api.py @@ -363,16 +363,17 @@ def get_username_validation_error(username): return _validate(_validate_username, errors.AccountUsernameInvalid, username) -def get_email_validation_error(email): +def get_email_validation_error(email, api_version='v1'): """Get the built-in validation error message for when the email is invalid in some way. :param email: The proposed email (unicode). + :param api_version: registration validation api version :param default: The message to default to in case of no error. :return: Validation error message. """ - return _validate(_validate_email, errors.AccountEmailInvalid, email) + return _validate(_validate_email, errors.AccountEmailInvalid, email, api_version) def get_secondary_email_validation_error(email): @@ -425,28 +426,30 @@ def get_country_validation_error(country): return _validate(_validate_country, errors.AccountCountryInvalid, country) -def get_username_existence_validation_error(username): +def get_username_existence_validation_error(username, api_version='v1'): """Get the built-in validation error message for when the username has an existence conflict. :param username: The proposed username (unicode). + :param api_version: registration validation api version :param default: The message to default to in case of no error. :return: Validation error message. """ - return _validate(_validate_username_doesnt_exist, errors.AccountUsernameAlreadyExists, username) + return _validate(_validate_username_doesnt_exist, errors.AccountUsernameAlreadyExists, username, api_version) -def get_email_existence_validation_error(email): +def get_email_existence_validation_error(email, api_version='v1'): """Get the built-in validation error message for when the email has an existence conflict. :param email: The proposed email (unicode). + :param api_version: registration validation api version :param default: The message to default to in case of no error. :return: Validation error message. """ - return _validate(_validate_email_doesnt_exist, errors.AccountEmailAlreadyExists, email) + return _validate(_validate_email_doesnt_exist, errors.AccountEmailAlreadyExists, email, api_version) def _get_user_and_profile(username): @@ -513,11 +516,12 @@ def _validate_username(username): raise errors.AccountUsernameInvalid(validation_err.message) -def _validate_email(email): +def _validate_email(email, api_version='v1'): """Validate the format of the email address. Arguments: email (unicode): The proposed email. + api_version(str): Validation API version; it is used to determine the error message Returns: None @@ -530,7 +534,9 @@ def _validate_email(email): _validate_unicode(email) _validate_type(email, str, accounts.EMAIL_BAD_TYPE_MSG) _validate_length(email, accounts.EMAIL_MIN_LENGTH, accounts.EMAIL_MAX_LENGTH, accounts.EMAIL_BAD_LENGTH_MSG) - validate_email.message = accounts.EMAIL_INVALID_MSG.format(email=email) + validate_email.message = ( + accounts.EMAIL_INVALID_MSG.format(email=email) if api_version == 'v1' else accounts.AUTHN_EMAIL_INVALID_MSG + ) validate_email(email) except (UnicodeError, errors.AccountDataBadType, errors.AccountDataBadLength) as invalid_email_err: raise errors.AccountEmailInvalid(str(invalid_email_err)) @@ -590,26 +596,37 @@ def _validate_country(country): raise errors.AccountCountryInvalid(accounts.REQUIRED_FIELD_COUNTRY_MSG) -def _validate_username_doesnt_exist(username): +def _validate_username_doesnt_exist(username, api_version='v1'): """Validate that the username is not associated with an existing user. :param username: The proposed username (unicode). + :param api_version: Validation API version; it is used to determine the error message :return: None :raises: errors.AccountUsernameAlreadyExists """ + if api_version == 'v1': + error_message = accounts.USERNAME_CONFLICT_MSG.format(username=username) + else: + error_message = accounts.AUTHN_USERNAME_CONFLICT_MSG if username is not None and username_exists_or_retired(username): - raise errors.AccountUsernameAlreadyExists(_(accounts.USERNAME_CONFLICT_MSG).format(username=username)) # lint-amnesty, pylint: disable=translation-of-non-string + raise errors.AccountUsernameAlreadyExists(_(error_message)) # lint-amnesty, pylint: disable=translation-of-non-string -def _validate_email_doesnt_exist(email): +def _validate_email_doesnt_exist(email, api_version='v1'): """Validate that the email is not associated with an existing user. :param email: The proposed email (unicode). + :param api_version: Validation API version; it is used to determine the error message :return: None :raises: errors.AccountEmailAlreadyExists """ + if api_version == 'v1': + error_message = accounts.EMAIL_CONFLICT_MSG.format(email_address=email) + else: + error_message = accounts.AUTHN_EMAIL_CONFLICT_MSG + if email is not None and email_exists_or_retired(email): - raise errors.AccountEmailAlreadyExists(_(accounts.EMAIL_CONFLICT_MSG).format(email_address=email)) # lint-amnesty, pylint: disable=translation-of-non-string + raise errors.AccountEmailAlreadyExists(_(error_message)) # lint-amnesty, pylint: disable=translation-of-non-string def _validate_secondary_email_doesnt_exist(email): diff --git a/openedx/core/djangoapps/user_authn/views/register.py b/openedx/core/djangoapps/user_authn/views/register.py index 74adfef418..7730a7ac49 100644 --- a/openedx/core/djangoapps/user_authn/views/register.py +++ b/openedx/core/djangoapps/user_authn/views/register.py @@ -545,14 +545,23 @@ class RegistrationView(APIView): username = data.get('username') errors = {} + # TODO: remove the is_authn_mfe check and use the new error message as default after redesign + is_authn_mfe = data.get('is_authn_mfe', False) + error_code = 'duplicate' if email is not None and email_exists_or_retired(email): error_code += '-email' - errors['email'] = [{'user_message': accounts_settings.EMAIL_CONFLICT_MSG.format(email_address=email)}] + error_message = accounts_settings.AUTHN_EMAIL_CONFLICT_MSG if is_authn_mfe else ( + accounts_settings.EMAIL_CONFLICT_MSG.format(email_address=email) + ) + errors['email'] = [{'user_message': error_message}] if username is not None and username_exists_or_retired(username): error_code += '-username' - errors['username'] = [{'user_message': accounts_settings.USERNAME_CONFLICT_MSG.format(username=username)}] + error_message = accounts_settings.AUTHN_USERNAME_CONFLICT_MSG if is_authn_mfe else ( + accounts_settings.USERNAME_CONFLICT_MSG.format(username=username) + ) + errors['username'] = [{'user_message': error_message}] errors['username_suggestions'] = generate_username_suggestions(username) if errors: @@ -718,8 +727,10 @@ class RegistrationValidationView(APIView): # This end-point is available to anonymous users, so no authentication is needed. authentication_classes = [] username_suggestions = [] + api_version = 'v1' def name_handler(self, request): + """ Validates whether fullname is valid """ name = request.data.get('name') return get_name_validation_error(name) @@ -727,7 +738,7 @@ class RegistrationValidationView(APIView): """ Validates whether the username is valid. """ username = request.data.get('username') invalid_username_error = get_username_validation_error(username) - username_exists_error = get_username_existence_validation_error(username) + username_exists_error = get_username_existence_validation_error(username, self.api_version) if username_exists_error: self.username_suggestions = generate_username_suggestions(username) # We prefer seeing for invalidity first. @@ -737,24 +748,27 @@ class RegistrationValidationView(APIView): def email_handler(self, request): """ Validates whether the email address is valid. """ email = request.data.get('email') - invalid_email_error = get_email_validation_error(email) - email_exists_error = get_email_existence_validation_error(email) + invalid_email_error = get_email_validation_error(email, self.api_version) + email_exists_error = get_email_existence_validation_error(email, self.api_version) # We prefer seeing for invalidity first. # Some invalid emails (like a blank one for superusers) may exist. return invalid_email_error or email_exists_error def confirm_email_handler(self, request): + """ Confirm email validator """ email = request.data.get('email') confirm_email = request.data.get('confirm_email') return get_confirm_email_validation_error(confirm_email, email) def password_handler(self, request): + """ Password validator """ username = request.data.get('username') email = request.data.get('email') password = request.data.get('password') return get_password_validation_error(password, username, email) def country_handler(self, request): + """ Country validator """ country = request.data.get('country') return get_country_validation_error(country) @@ -790,17 +804,32 @@ class RegistrationValidationView(APIView): can get extra verification checks if entered along with others, like when the password may not equal the username. """ + # TODO: remove is_authn_mfe after redesign-master is merged in frontend-app-authn + # and use the new messages as default + is_auth_mfe = request.data.get('is_authn_mfe') + field_key = request.data.get('form_field_key') 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) - }) - response_dict = {"validation_decisions": validation_decisions} + def update_validations(field_name): + """ + Updates the validation decisions + """ + validation = self.validation_handlers[field_name](self, request) + validation_decisions[field_name] = validation + + if is_auth_mfe: + self.api_version = 'v2' + + if field_key and field_key in self.validation_handlers: + update_validations(field_key) + else: + 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: + update_validations(form_field_key) + + response_dict = {'validation_decisions': validation_decisions} if self.username_suggestions: response_dict['username_suggestions'] = self.username_suggestions diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_register.py b/openedx/core/djangoapps/user_authn/views/tests/test_register.py index f671c1f825..c242b2c8a1 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_register.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_register.py @@ -22,6 +22,8 @@ from edx_toggles.toggles.testutils import override_waffle_flag from openedx.core.djangoapps.site_configuration.helpers import get_value from openedx.core.djangoapps.site_configuration.tests.test_util import with_site_configuration from openedx.core.djangoapps.user_api.accounts import ( + AUTHN_EMAIL_CONFLICT_MSG, + AUTHN_USERNAME_CONFLICT_MSG, EMAIL_BAD_LENGTH_MSG, EMAIL_CONFLICT_MSG, EMAIL_INVALID_MSG, @@ -359,6 +361,44 @@ class RegistrationViewValidationErrorTest(ThirdPartyAuthTestMixin, UserAPITestCa } ) + def test_duplicate_email_username_error_with_is_authn_check(self): + # Register the first user + response = self.client.post(self.url, { + "email": self.EMAIL, + "name": self.NAME, + "username": self.USERNAME, + "password": self.PASSWORD, + "honor_code": "true", + }) + self.assertHttpOK(response) + + # Try to create a second user with the same username and email + response = self.client.post(self.url, { + "is_authn_mfe": True, + "email": self.EMAIL, + "name": "Someone Else", + "username": self.USERNAME, + "password": self.PASSWORD, + "honor_code": "true", + }) + + response_json = json.loads(response.content.decode('utf-8')) + assert response.status_code == 409 + username_suggestions = response_json.pop('username_suggestions') + assert len(username_suggestions) == 3 + self.assertDictEqual( + response_json, + { + "username": [{ + "user_message": AUTHN_USERNAME_CONFLICT_MSG, + }], + "email": [{ + "user_message": AUTHN_EMAIL_CONFLICT_MSG + }], + "error_code": "duplicate-email-username" + } + ) + @ddt.ddt @skip_unless_lms @@ -2393,3 +2433,16 @@ class RegistrationValidationViewTests(test_utils.ApiTestCase): assert response.status_code != 403 response = self.request_without_auth('post', self.path) assert response.status_code == 403 + + def test_single_field_validation(self): + """ + Test that if `is_authn_mfe` is provided in request along with form_field_key, only + error message for that field is returned. + """ + User.objects.create_user(username='user', email='user@email.com') + # using username and email that have conflicts but sending form_field_key will return + # validation for only email + self.assertValidationDecision( + {'username': 'user', 'email': 'user@email.com', 'is_authn_mfe': True, 'form_field_key': 'email'}, + {'email': AUTHN_EMAIL_CONFLICT_MSG} + )