diff --git a/common/djangoapps/student/helpers.py b/common/djangoapps/student/helpers.py index a610ab6bc9..81f39a7ce6 100644 --- a/common/djangoapps/student/helpers.py +++ b/common/djangoapps/student/helpers.py @@ -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 diff --git a/lms/static/js/student_account/views/RegisterView.js b/lms/static/js/student_account/views/RegisterView.js index 95b668a235..db5f6ae855 100644 --- a/lms/static/js/student_account/views/RegisterView.js +++ b/lms/static/js/student_account/views/RegisterView.js @@ -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('
  • {error}
  • ', { - error: errorItem.user_message - }); - } - ); + function(errorList, key) { + if (key === 'error_code') { + return null; + } else { + return _.map( + errorList, + function(errorItem) { + return StringUtils.interpolate('
  • {error}
  • ', { + error: errorItem.user_message, + suppressAttr: ( + key === 'email' || key === 'username' + ) ? 'data-hj-suppress' : '' + }); + } + ); + } } ) ); diff --git a/openedx/core/djangoapps/user_authn/views/register.py b/openedx/core/djangoapps/user_authn/views/register.py index e9deffeaaf..860f8a4cbc 100644 --- a/openedx/core/djangoapps/user_authn/views/register.py +++ b/openedx/core/djangoapps/user_authn/views/register.py @@ -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): 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 900573582d..ccb089087b 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_register.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_register.py @@ -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)