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

VAN-113: Add error code for registration workflow (#26821)

Browse Source
This commit is contained in:
Zainab Amir
2021-03-08 18:09:26 +05:00
committed by GitHub
parent b8da9fb59b
commit a39befb548
4 changed files with 83 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -430,9 +430,10 @@ class AccountValidationError(Exception):
"""
Used in account creation views to raise exceptions with details about specific invalid fields
"""
def __init__(self, message, field):
def __init__(self, message, field, error_code=None):
super(AccountValidationError, self).__init__(message) # lint-amnesty, pylint: disable=super-with-arguments
self.field = field
self.error_code = error_code
def cert_info(user, course_overview):
@@ -652,12 +653,14 @@ def do_create_account(form, custom_form=None):
if username_exists_or_retired(user.username): # lint-amnesty, pylint: disable=no-else-raise
raise AccountValidationError( # lint-amnesty, pylint: disable=raise-missing-from
USERNAME_EXISTS_MSG_FMT.format(username=proposed_username),
field="username"
field="username",
error_code='duplicate-username',
)
elif email_exists_or_retired(user.email):
raise AccountValidationError( # lint-amnesty, pylint: disable=raise-missing-from
_("An account with the Email '{email}' already exists.").format(email=user.email),
field="email"
field="email",
error_code='duplicate-email'
)
else:
raise

25
lms/static/js/student_account/views/RegisterView.js
View File

@@ -454,15 +454,22 @@
_.map(
// Something is passing this 'undefined'. Protect against this.
JSON.parse(error.responseText || '[]'),
function(errorList) {
return _.map(
errorList,
function(errorItem) {
return StringUtils.interpolate('<li>{error}</li>', {
error: errorItem.user_message
});
}
);
function(errorList, key) {
if (key === 'error_code') {
return null;
} else {
return _.map(
errorList,
function(errorItem) {
return StringUtils.interpolate('<li {suppressAttr} >{error}</li>', {
error: errorItem.user_message,
suppressAttr: (
key === 'email' || key === 'username'
) ? 'data-hj-suppress' : ''
});
}
);
}
}
)
);

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

@@ -178,10 +178,13 @@ def create_account_with_params(request, params):
# error message
if is_third_party_auth_enabled and ('social_auth_provider' in params and not pipeline.running(request)):
raise ValidationError(
{'session_expired': [
_(u"Registration using {provider} has timed out.").format(
provider=params.get('social_auth_provider'))
]}
{
'session_expired': [
_(u"Registration using {provider} has timed out.").format(
provider=params.get('social_auth_provider'))
],
'error_code': 'tpa-session-expired',
}
)
if is_third_party_auth_enabled:
@@ -288,21 +291,25 @@ def _link_user_to_third_party_provider(
_(u"An access_token is required when passing value ({}) for provider.").format(
params['provider']
)
]
],
'error_code': 'tpa-missing-access-token'
})
request.session[pipeline.AUTH_ENTRY_KEY] = pipeline.AUTH_ENTRY_REGISTER_API
pipeline_user = None
error_message = ""
error_code = None
try:
pipeline_user = request.backend.do_auth(social_access_token, user=user)
except AuthAlreadyAssociated:
error_message = _("The provided access_token is already associated with another user.")
error_code = 'tpa-token-already-associated'
except (HTTPError, AuthException):
error_message = _("The provided access_token is not valid.")
error_code = 'tpa-invalid-access-token'
if not pipeline_user or not isinstance(pipeline_user, User):
# Ensure user does not re-enter the pipeline
request.social_strategy.clean_partial_pipeline(social_access_token)
raise ValidationError({'access_token': [error_message]})
raise ValidationError({'access_token': [error_message], 'error_code': error_code})
# If the user is registering via 3rd party auth, track which provider they use
if is_third_party_auth_enabled and pipeline.running(request):
@@ -523,14 +530,17 @@ class RegistrationView(APIView):
username = data.get('username')
errors = {}
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)}]
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)}]
if errors:
return self._create_response(request, errors, status_code=409)
return self._create_response(request, errors, status_code=409, error_code=error_code)
def _handle_terms_of_service(self, data):
# Backwards compatibility: the student view expects both
@@ -551,22 +561,26 @@ class RegistrationView(APIView):
errors = {
err.field: [{"user_message": text_type(err)}]
}
response = self._create_response(request, errors, status_code=409)
response = self._create_response(request, errors, status_code=409, error_code=err.error_code)
except ValidationError as err:
# Should only get field errors from this exception
assert NON_FIELD_ERRORS not in err.message_dict
# Error messages are returned as arrays from ValidationError
error_code = err.message_dict.get('error_code', ['validation-error'])[0]
# Only return first error for each field
errors = {
field: [{"user_message": error} for error in error_list]
for field, error_list in err.message_dict.items()
for field, error_list in err.message_dict.items() if field != 'error_code'
}
response = self._create_response(request, errors, status_code=400)
response = self._create_response(request, errors, status_code=400, error_code=error_code)
except PermissionDenied:
response = HttpResponseForbidden(_("Account creation not allowed."))
return response, user
def _create_response(self, request, response_dict, status_code, redirect_url=None):
def _create_response(self, request, response_dict, status_code, redirect_url=None, error_code=None):
if status_code == 200:
# keeping this `success` field in for now, as we have outstanding clients expecting this
response_dict['success'] = True
@@ -574,6 +588,9 @@ class RegistrationView(APIView):
self._log_validation_errors(request, response_dict, status_code)
if redirect_url:
response_dict['redirect_url'] = redirect_url
if error_code:
response_dict['error_code'] = error_code
set_custom_attribute('register_error_code', error_code)
return JsonResponse(response_dict, status=status_code)
def _log_validation_errors(self, request, errors, status_code):

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

@@ -148,7 +148,8 @@ class RegistrationViewValidationErrorTest(ThirdPartyAuthTestMixin, UserAPITestCa
).format(
self.EMAIL
)
}]
}],
"error_code": "duplicate-email"
}
)
@@ -192,7 +193,8 @@ class RegistrationViewValidationErrorTest(ThirdPartyAuthTestMixin, UserAPITestCa
).format(
self.USERNAME
)
}]
}],
"error_code": "duplicate-username"
}
)
@@ -229,7 +231,8 @@ class RegistrationViewValidationErrorTest(ThirdPartyAuthTestMixin, UserAPITestCa
).format(
self.EMAIL
)
}]
}],
"error_code": "duplicate-email"
}
)
@@ -266,7 +269,8 @@ class RegistrationViewValidationErrorTest(ThirdPartyAuthTestMixin, UserAPITestCa
).format(
self.EMAIL
)
}]
}],
"error_code": "duplicate-email"
}
)
@@ -302,7 +306,8 @@ class RegistrationViewValidationErrorTest(ThirdPartyAuthTestMixin, UserAPITestCa
).format(
self.USERNAME
)
}]
}],
"error_code": "duplicate-username"
}
)
@@ -346,7 +351,8 @@ class RegistrationViewValidationErrorTest(ThirdPartyAuthTestMixin, UserAPITestCa
).format(
self.EMAIL
)
}]
}],
"error_code": "duplicate-email-username"
}
)
@@ -1460,7 +1466,8 @@ class RegistrationViewTestV1(ThirdPartyAuthTestMixin, UserAPITestCase):
).format(
self.EMAIL
)
}]
}],
"error_code": "duplicate-email"
}
)
@@ -1496,7 +1503,8 @@ class RegistrationViewTestV1(ThirdPartyAuthTestMixin, UserAPITestCase):
).format(
self.USERNAME
)
}]
}],
"error_code": "duplicate-username"
}
)
@@ -1540,7 +1548,8 @@ class RegistrationViewTestV1(ThirdPartyAuthTestMixin, UserAPITestCase):
).format(
self.EMAIL
)
}]
}],
"error_code": "duplicate-email-username"
}
)
@@ -1568,8 +1577,9 @@ class RegistrationViewTestV1(ThirdPartyAuthTestMixin, UserAPITestCase):
self.assertDictEqual(
response_json,
{
u"username": [{u"user_message": USERNAME_BAD_LENGTH_MSG}],
u"password": [{u"user_message": u"This field is required."}],
"username": [{u"user_message": USERNAME_BAD_LENGTH_MSG}],
"password": [{u"user_message": u"This field is required."}],
"error_code": "validation-error"
}
)
@@ -1974,7 +1984,7 @@ class ThirdPartyRegistrationTestMixin(ThirdPartyOAuthTestMixin, CacheIsolationTe
assert conflict_attribute in errors
assert "belongs to an existing account" in errors[conflict_attribute][0]["user_message"]
def _assert_access_token_error(self, response, expected_error_message):
def _assert_access_token_error(self, response, expected_error_message, error_code):
"""Assert that the given response was an error for the access_token field with the given error message."""
assert response.status_code == 400
response_json = json.loads(response.content.decode('utf-8'))
@@ -1982,6 +1992,7 @@ class ThirdPartyRegistrationTestMixin(ThirdPartyOAuthTestMixin, CacheIsolationTe
response_json,
{
"access_token": [{"user_message": expected_error_message}],
"error_code": error_code
}
)
@@ -1993,6 +2004,7 @@ class ThirdPartyRegistrationTestMixin(ThirdPartyOAuthTestMixin, CacheIsolationTe
response_json,
{
"session_expired": [{"user_message": expected_error_message}],
"error_code": "tpa-session-expired",
}
)
@@ -2049,7 +2061,11 @@ class ThirdPartyRegistrationTestMixin(ThirdPartyOAuthTestMixin, CacheIsolationTe
user = UserFactory()
UserSocialAuth.objects.create(user=user, provider=self.BACKEND, uid=self.social_uid)
response = self.client.post(self.url, self.data())
self._assert_access_token_error(response, "The provided access_token is already associated with another user.")
self._assert_access_token_error(
response,
"The provided access_token is already associated with another user.",
"tpa-token-already-associated"
)
self._verify_user_existence(
user_exists=True, social_link_exists=True, user_is_active=True, username=user.username
)
@@ -2057,7 +2073,7 @@ class ThirdPartyRegistrationTestMixin(ThirdPartyOAuthTestMixin, CacheIsolationTe
def test_invalid_token(self):
self._setup_provider_response(success=False)
response = self.client.post(self.url, self.data())
self._assert_access_token_error(response, "The provided access_token is not valid.")
self._assert_access_token_error(response, "The provided access_token is not valid.", "tpa-invalid-access-token")
self._verify_user_existence(user_exists=False, social_link_exists=False)
def test_missing_token(self):
@@ -2066,7 +2082,8 @@ class ThirdPartyRegistrationTestMixin(ThirdPartyOAuthTestMixin, CacheIsolationTe
response = self.client.post(self.url, data)
self._assert_access_token_error(
response,
u"An access_token is required when passing value ({}) for provider.".format(self.BACKEND)
u"An access_token is required when passing value ({}) for provider.".format(self.BACKEND),
"tpa-missing-access-token"
)
self._verify_user_existence(user_exists=False, social_link_exists=False)
@@ -2106,7 +2123,7 @@ class TestFacebookRegistrationView(
"""
self._setup_provider_response_with_body(200, json.dumps("false"))
response = self.client.post(self.url, self.data())
self._assert_access_token_error(response, "The provided access_token is not valid.")
self._assert_access_token_error(response, "The provided access_token is not valid.", "tpa-invalid-access-token")
self._verify_user_existence(user_exists=False, social_link_exists=False)