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

Update validation messages for register endpoint (#27476)

Browse Source 
As part of authn redesign, validation messages have been updated.
- created a new endpoint for validations
- updated username/email conflict message in registration api based on
authn check

VAN-288
This commit is contained in:
Zainab Amir
2021-05-05 12:33:59 +05:00
committed by GitHub
parent 78c6be8bee
commit 8d4ccf950a
4 changed files with 128 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

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

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

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

57
openedx/core/djangoapps/user_authn/views/register.py
View File

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

53
openedx/core/djangoapps/user_authn/views/tests/test_register.py
View File

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