Merge pull request #24883 from edx/VAN-14/convert-api-resp

Modify the api response
This commit is contained in:
Uzair Rasheed
2020-09-10 14:16:18 +05:00
committed by GitHub
5 changed files with 50 additions and 55 deletions

View File

@@ -126,7 +126,7 @@ class HelperMixin(object):
self.assertEqual(400, response.status_code) self.assertEqual(400, response.status_code)
payload = json.loads(response.content.decode('utf-8')) payload = json.loads(response.content.decode('utf-8'))
self.assertFalse(payload.get('success')) self.assertFalse(payload.get('success'))
self.assertIn('In order to sign in, you need to activate your account.', payload.get('value')) self.assertIn('inactive-user', payload.get('error_code'))
def assert_json_failure_response_is_missing_social_auth(self, response): def assert_json_failure_response_is_missing_social_auth(self, response):
"""Asserts failure on /login for missing social auth looks right.""" """Asserts failure on /login for missing social auth looks right."""

View File

@@ -86,7 +86,7 @@
} }
}) })
) )
) );
this.postRender(); this.postRender();
return this; return this;
@@ -212,6 +212,29 @@
msg = gettext('An error has occurred. Check your Internet connection and try again.'); msg = gettext('An error has occurred. Check your Internet connection and try again.');
} else if (error.status === 500) { } else if (error.status === 500) {
msg = gettext('An error has occurred. Try refreshing the page, or check your Internet connection.'); // eslint-disable-line max-len msg = gettext('An error has occurred. Try refreshing the page, or check your Internet connection.'); // eslint-disable-line max-len
} else if (error.responseJSON !== undefined && error.responseJSON.error_code === 'inactive-user') {
msg = HtmlUtils.interpolateHtml(
gettext('In order to sign in, you need to activate your account.{line_break}{line_break}' +
'We just sent an activation link to {strong_start} {email} {strong_end}. If ' +
' you do not receive an email, check your spam folders or ' +
' {anchorStart}contact {platform_name} Support{anchorEnd}.'),
{
email: error.responseJSON.email,
platform_name: this.platform_name,
support_url: 'https://support.edx.org/',
line_break: HtmlUtils.HTML('<br/>'),
strong_start: HtmlUtils.HTML('<strong>'),
strong_end: HtmlUtils.HTML('</strong>'),
anchorStart: HtmlUtils.HTML(
StringUtils.interpolate(
'<a href="{SupportUrl}">', {
SupportUrl: 'https://support.edx.org/'
}
)
),
anchorEnd: HtmlUtils.HTML('</a>')
}
);
} else if (error.responseJSON !== undefined) { } else if (error.responseJSON !== undefined) {
msg = error.responseJSON.value; msg = error.responseJSON.value;
errorCode = error.responseJSON.error_code; errorCode = error.responseJSON.error_code;

View File

@@ -9,16 +9,19 @@ class AuthFailedError(Exception):
This is a helper for the login view, allowing the various sub-methods to error out with an appropriate failure This is a helper for the login view, allowing the various sub-methods to error out with an appropriate failure
message. message.
""" """
def __init__(self, value=None, redirect=None, redirect_url=None): def __init__(
self, value=None, redirect=None, redirect_url=None, error_code=None
):
super(AuthFailedError, self).__init__() super(AuthFailedError, self).__init__()
self.value = Text(value) self.value = Text(value)
self.redirect = redirect self.redirect = redirect
self.redirect_url = redirect_url self.redirect_url = redirect_url
self.error_code = error_code
def get_response(self): def get_response(self):
""" Returns a dict representation of the error. """ """ Returns a dict representation of the error. """
resp = {'success': False} resp = {'success': False}
for attr in ('value', 'redirect', 'redirect_url'): for attr in ('value', 'redirect', 'redirect_url', 'error_code'):
if self.__getattribute__(attr): if self.__getattribute__(attr):
resp[attr] = self.__getattribute__(attr) resp[attr] = self.__getattribute__(attr)

View File

@@ -72,23 +72,20 @@ def _do_third_party_auth(request):
u"with backend_name {backend_name}".format( u"with backend_name {backend_name}".format(
username=username, backend_name=backend_name) username=username, backend_name=backend_name)
) )
message = Text(_( message = _(
u"You've successfully signed in to your {provider_name} account, " u"You've successfully signed in to your {provider_name} account, "
u"but this account isn't linked with your {platform_name} account yet. {blank_lines}" u"but this account isn't linked with your {platform_name} account yet."
u"Use your {platform_name} username and password to sign in to {platform_name} below, " u"Use your {platform_name} username and password to sign in to {platform_name} below, "
u"and then link your {platform_name} account with {provider_name} from your dashboard. {blank_lines}" u"and then link your {platform_name} account with {provider_name} from your dashboard."
u"If you don't have an account on {platform_name} yet, " u"If you don't have an account on {platform_name} yet, "
u"click {register_label_strong} at the top of the page." u"click {register_label_strong} at the top of the page."
)).format( ).format(
blank_lines=HTML('<br/><br/>'),
platform_name=platform_name, platform_name=platform_name,
provider_name=requested_provider.name, provider_name=requested_provider.name,
register_label_strong=HTML('<strong>{register_text}</strong>').format( register_label_strong='Register'
register_text=_('Register')
)
) )
raise AuthFailedError(message) raise AuthFailedError(message, error_code='third-party-auth-with-no-linked-account')
def _get_user_by_email(request): def _get_user_by_email(request):
@@ -144,39 +141,6 @@ def _enforce_password_policy_compliance(request, user):
raise AuthFailedError(HTML(six.text_type(e))) raise AuthFailedError(HTML(six.text_type(e)))
def _generate_not_activated_message(user):
"""
Generates the message displayed on the sign-in screen when a learner attempts to access the
system with an inactive account.
"""
support_url = configuration_helpers.get_value(
'SUPPORT_SITE_LINK',
settings.SUPPORT_SITE_LINK
)
platform_name = configuration_helpers.get_value(
'PLATFORM_NAME',
settings.PLATFORM_NAME
)
not_activated_message = Text(_(
u'In order to sign in, you need to activate your account.{blank_lines}'
u'We just sent an activation link to {email_strong}. If '
u'you do not receive an email, check your spam folders or '
u'{link_start}contact {platform_name} Support{link_end}.'
)).format(
platform_name=platform_name,
blank_lines=HTML('<br/><br/>'),
email_strong=HTML('<strong>{email}</strong>').format(email=user.email),
link_start=HTML(u'<a href="{support_url}">').format(
support_url=support_url,
),
link_end=HTML("</a>"),
)
return not_activated_message
def _log_and_raise_inactive_user_auth_error(unauthenticated_user): def _log_and_raise_inactive_user_auth_error(unauthenticated_user):
""" """
Depending on Django version we can get here a couple of ways, but this takes care of logging an auth attempt Depending on Django version we can get here a couple of ways, but this takes care of logging an auth attempt
@@ -195,7 +159,7 @@ def _log_and_raise_inactive_user_auth_error(unauthenticated_user):
profile = UserProfile.objects.get(user=unauthenticated_user) profile = UserProfile.objects.get(user=unauthenticated_user)
compose_and_send_activation_email(unauthenticated_user, profile) compose_and_send_activation_email(unauthenticated_user, profile)
raise AuthFailedError(_generate_not_activated_message(unauthenticated_user)) raise AuthFailedError(error_code='inactive-user')
def _authenticate_first_party(request, unauthenticated_user, third_party_auth_requested): def _authenticate_first_party(request, unauthenticated_user, third_party_auth_requested):
@@ -358,7 +322,7 @@ def _check_user_auth_flow(site, user):
@login_required @login_required
@require_http_methods(['GET']) @require_http_methods(['GET'])
def finish_auth(request): # pylint: disable=unused-argument def finish_auth(request):
""" Following logistration (1st or 3rd party), handle any special query string params. """ Following logistration (1st or 3rd party), handle any special query string params.
See FinishAuthView.js for details on the query string params. See FinishAuthView.js for details on the query string params.
@@ -456,7 +420,6 @@ def login_user(request):
# user successfully authenticated with a third party provider, but has no linked Open edX account # user successfully authenticated with a third party provider, but has no linked Open edX account
response_content = e.get_response() response_content = e.get_response()
response_content['error_code'] = 'third-party-auth-with-no-linked-account'
return JsonResponse(response_content, status=403) return JsonResponse(response_content, status=403)
else: else:
user = _get_user_by_email(request) user = _get_user_by_email(request)
@@ -496,8 +459,12 @@ def login_user(request):
set_custom_metric('login_user_redirect_url', redirect_url) set_custom_metric('login_user_redirect_url', redirect_url)
return response return response
except AuthFailedError as error: except AuthFailedError as error:
log.exception(error.get_response()) response_content = error.get_response()
response = JsonResponse(error.get_response(), status=400) log.exception(response_content)
if response_content.get('error_code') == 'inactive-user':
response_content['email'] = user.email
response = JsonResponse(response_content, status=400)
set_custom_metric('login_user_auth_failed_error', True) set_custom_metric('login_user_auth_failed_error', True)
set_custom_metric('login_user_response_status', response.status_code) set_custom_metric('login_user_response_status', response.status_code)
return response return response

View File

@@ -236,8 +236,7 @@ class LoginTest(SiteMixin, CacheIsolationTestCase):
self.user_email, self.user_email,
self.password self.password
) )
self._assert_response(response, success=False, self._assert_response(response, success=False, error_code="inactive-user")
value="In order to sign in, you need to activate your account.")
self._assert_audit_log(mock_audit_log, 'warning', [u'Login failed', u'Account not active for user']) self._assert_audit_log(mock_audit_log, 'warning', [u'Login failed', u'Account not active for user'])
self._assert_not_in_audit_log(mock_audit_log, 'warning', [u'test']) self._assert_not_in_audit_log(mock_audit_log, 'warning', [u'test'])
@@ -253,7 +252,7 @@ class LoginTest(SiteMixin, CacheIsolationTestCase):
self.user_email, self.user_email,
self.password, self.password,
) )
self._assert_response(response, success=False, value=self.ACTIVATE_ACCOUNT_WARNING) self._assert_response(response, success=False, error_code="inactive-user")
self._assert_audit_log(mock_audit_log, 'warning', [u'Login failed', u'Account not active for user']) self._assert_audit_log(mock_audit_log, 'warning', [u'Login failed', u'Account not active for user'])
@patch('openedx.core.djangoapps.user_authn.views.login._log_and_raise_inactive_user_auth_error') @patch('openedx.core.djangoapps.user_authn.views.login._log_and_raise_inactive_user_auth_error')
@@ -595,7 +594,7 @@ class LoginTest(SiteMixin, CacheIsolationTestCase):
result = self.client.post(self.url, post_params, **extra) result = self.client.post(self.url, post_params, **extra)
return result, mock_audit_log return result, mock_audit_log
def _assert_response(self, response, success=None, value=None, status_code=None): def _assert_response(self, response, success=None, value=None, status_code=None, error_code=None):
""" """
Assert that the response has the expected status code and returned a valid Assert that the response has the expected status code and returned a valid
JSON-parseable dict. JSON-parseable dict.
@@ -618,6 +617,9 @@ class LoginTest(SiteMixin, CacheIsolationTestCase):
if success is not None: if success is not None:
self.assertEqual(response_dict['success'], success) self.assertEqual(response_dict['success'], success)
if error_code is not None:
self.assertEqual(response_dict['error_code'], error_code)
if value is not None: if value is not None:
msg = (u"'%s' did not contain '%s'" % msg = (u"'%s' did not contain '%s'" %
(six.text_type(response_dict['value']), six.text_type(value))) (six.text_type(response_dict['value']), six.text_type(value)))