From c68155f76f4c29034a25b5db71a50bc0ed299f08 Mon Sep 17 00:00:00 2001 From: uzairr Date: Tue, 1 Sep 2020 14:17:21 +0500 Subject: [PATCH] Modify the api response Update the api response so that it cannot contain the response in the form of HTML which may prove vulnerable for MFE in future. VAN-14 --- .../third_party_auth/tests/specs/base.py | 2 +- .../js/student_account/views/LoginView.js | 25 +++++++- .../core/djangoapps/user_authn/exceptions.py | 7 ++- .../core/djangoapps/user_authn/views/login.py | 61 +++++-------------- .../user_authn/views/tests/test_login.py | 10 +-- 5 files changed, 50 insertions(+), 55 deletions(-) diff --git a/common/djangoapps/third_party_auth/tests/specs/base.py b/common/djangoapps/third_party_auth/tests/specs/base.py index 4686e3ab13..5355af0a8c 100644 --- a/common/djangoapps/third_party_auth/tests/specs/base.py +++ b/common/djangoapps/third_party_auth/tests/specs/base.py @@ -126,7 +126,7 @@ class HelperMixin(object): self.assertEqual(400, response.status_code) payload = json.loads(response.content.decode('utf-8')) 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): """Asserts failure on /login for missing social auth looks right.""" diff --git a/lms/static/js/student_account/views/LoginView.js b/lms/static/js/student_account/views/LoginView.js index 3b0fa99785..ebbcb858f8 100644 --- a/lms/static/js/student_account/views/LoginView.js +++ b/lms/static/js/student_account/views/LoginView.js @@ -86,7 +86,7 @@ } }) ) - ) + ); this.postRender(); return this; @@ -212,6 +212,29 @@ msg = gettext('An error has occurred. Check your Internet connection and try again.'); } 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 + } 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('
'), + strong_start: HtmlUtils.HTML(''), + strong_end: HtmlUtils.HTML(''), + anchorStart: HtmlUtils.HTML( + StringUtils.interpolate( + '', { + SupportUrl: 'https://support.edx.org/' + } + ) + ), + anchorEnd: HtmlUtils.HTML('') + } + ); } else if (error.responseJSON !== undefined) { msg = error.responseJSON.value; errorCode = error.responseJSON.error_code; diff --git a/openedx/core/djangoapps/user_authn/exceptions.py b/openedx/core/djangoapps/user_authn/exceptions.py index 44dcf3d964..d4abc4377e 100644 --- a/openedx/core/djangoapps/user_authn/exceptions.py +++ b/openedx/core/djangoapps/user_authn/exceptions.py @@ -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 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__() self.value = Text(value) self.redirect = redirect self.redirect_url = redirect_url + self.error_code = error_code def get_response(self): """ Returns a dict representation of the error. """ resp = {'success': False} - for attr in ('value', 'redirect', 'redirect_url'): + for attr in ('value', 'redirect', 'redirect_url', 'error_code'): if self.__getattribute__(attr): resp[attr] = self.__getattribute__(attr) diff --git a/openedx/core/djangoapps/user_authn/views/login.py b/openedx/core/djangoapps/user_authn/views/login.py index 4fdf85a75d..75ec14e53d 100644 --- a/openedx/core/djangoapps/user_authn/views/login.py +++ b/openedx/core/djangoapps/user_authn/views/login.py @@ -72,23 +72,20 @@ def _do_third_party_auth(request): u"with backend_name {backend_name}".format( username=username, backend_name=backend_name) ) - message = Text(_( + message = _( 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"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"click {register_label_strong} at the top of the page." - )).format( - blank_lines=HTML('

'), + ).format( platform_name=platform_name, provider_name=requested_provider.name, - register_label_strong=HTML('{register_text}').format( - register_text=_('Register') - ) + register_label_strong='Register' ) - raise AuthFailedError(message) + raise AuthFailedError(message, error_code='third-party-auth-with-no-linked-account') def _get_user_by_email(request): @@ -144,39 +141,6 @@ def _enforce_password_policy_compliance(request, user): 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('

'), - email_strong=HTML('{email}').format(email=user.email), - link_start=HTML(u'').format( - support_url=support_url, - ), - link_end=HTML(""), - ) - - return not_activated_message - - 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 @@ -195,7 +159,7 @@ def _log_and_raise_inactive_user_auth_error(unauthenticated_user): profile = UserProfile.objects.get(user=unauthenticated_user) 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): @@ -358,7 +322,7 @@ def _check_user_auth_flow(site, user): @login_required @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. 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 response_content = e.get_response() - response_content['error_code'] = 'third-party-auth-with-no-linked-account' return JsonResponse(response_content, status=403) else: user = _get_user_by_email(request) @@ -496,8 +459,12 @@ def login_user(request): set_custom_metric('login_user_redirect_url', redirect_url) return response except AuthFailedError as error: - log.exception(error.get_response()) - response = JsonResponse(error.get_response(), status=400) + response_content = error.get_response() + 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_response_status', response.status_code) return response diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_login.py b/openedx/core/djangoapps/user_authn/views/tests/test_login.py index cc3cad810a..73a7a0de55 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_login.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_login.py @@ -236,8 +236,7 @@ class LoginTest(SiteMixin, CacheIsolationTestCase): self.user_email, self.password ) - self._assert_response(response, success=False, - value="In order to sign in, you need to activate your account.") + 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_not_in_audit_log(mock_audit_log, 'warning', [u'test']) @@ -253,7 +252,7 @@ class LoginTest(SiteMixin, CacheIsolationTestCase): self.user_email, 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']) @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) 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 JSON-parseable dict. @@ -618,6 +617,9 @@ class LoginTest(SiteMixin, CacheIsolationTestCase): if success is not None: 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: msg = (u"'%s' did not contain '%s'" % (six.text_type(response_dict['value']), six.text_type(value)))