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
This commit is contained in:
@@ -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)
|
||||
|
||||
|
||||
@@ -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('<br/><br/>'),
|
||||
).format(
|
||||
platform_name=platform_name,
|
||||
provider_name=requested_provider.name,
|
||||
register_label_strong=HTML('<strong>{register_text}</strong>').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('<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):
|
||||
"""
|
||||
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
|
||||
|
||||
@@ -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)))
|
||||
|
||||
Reference in New Issue
Block a user