From 333e3315cbc12e2b4c9d15d2bf1296f4715c13f5 Mon Sep 17 00:00:00 2001
From: uzairr <uzairr@yahoo.com>
Date: Tue, 28 Nov 2017 11:29:43 +0000
Subject: [PATCH] Verify 'Full Name' field does not allow HTML.

'Full Name' field in the student account settings is allowing HTML
as an input which makes spoofing easily.To avoid it, validation is
added that ensures 'Full Name' field does not allow HTML as input.

LEARNER-3337
---
 .../tests/lms/test_account_settings.py        | 13 +++--------
 .../core/djangoapps/user_api/accounts/api.py  | 22 +++++++++++++++++--
 .../user_api/accounts/tests/test_api.py       |  7 ++++--
 3 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/common/test/acceptance/tests/lms/test_account_settings.py b/common/test/acceptance/tests/lms/test_account_settings.py
index 3a79f57955..044dfa90fb 100644
--- a/common/test/acceptance/tests/lms/test_account_settings.py
+++ b/common/test/acceptance/tests/lms/test_account_settings.py
@@ -275,16 +275,9 @@ class AccountSettingsPageTest(AccountSettingsTestMixin, AcceptanceTest):
             u'Full Name',
             self.full_name,
             u'@',
-            [u'another name', self.full_name],
-        )
-
-        actual_events = self.wait_for_events(event_filter=self.settings_changed_event_filter, number_of_matches=2)
-        self.assert_events_match(
-            [
-                self.expected_settings_changed_event('name', self.full_name, 'another name'),
-                self.expected_settings_changed_event('name', 'another name', self.full_name),
-            ],
-            actual_events
+            [u'<h1>another name<h1>', self.full_name],
+            u'Full Name cannot contain the following characters: < >',
+            False
         )
 
     def test_email_field(self):
diff --git a/openedx/core/djangoapps/user_api/accounts/api.py b/openedx/core/djangoapps/user_api/accounts/api.py
index 1bda1ee67d..9043af0ac9 100644
--- a/openedx/core/djangoapps/user_api/accounts/api.py
+++ b/openedx/core/djangoapps/user_api/accounts/api.py
@@ -2,10 +2,12 @@
 """
 Programmatic integration point for User API Accounts sub-application
 """
-from django.utils.translation import override as override_language, ugettext as _
-from django.db import transaction, IntegrityError
+import re
 import datetime
 from pytz import UTC
+
+from django.utils.translation import override as override_language, ugettext as _
+from django.db import transaction, IntegrityError
 from django.core.exceptions import ObjectDoesNotExist
 from django.conf import settings
 from django.core.validators import validate_email, ValidationError
@@ -133,8 +135,10 @@ def update_account_settings(requesting_user, update, username=None):
 
     # If user has requested to change name, store old name because we must update associated metadata
     # after the save process is complete.
+    changing_full_name = False
     old_name = None
     if "name" in update:
+        changing_full_name = True
         old_name = existing_user_profile.name
 
     # Check for fields that are not editable. Marking them read-only causes them to be ignored, but we wish to 400.
@@ -169,6 +173,12 @@ def update_account_settings(requesting_user, update, username=None):
                 "user_message": err.message
             }
 
+    if changing_full_name and contains_html(update['name']):
+        field_errors["name"] = {
+            "developer_message": u"Error thrown from validate_full_name: '{}'".format('Full Name is in-valid'),
+            "user_message": _(u"Full Name cannot contain the following characters: < >")
+        }
+
     # If we have encountered any validation errors, return them to the user.
     if field_errors:
         raise errors.AccountValidationError(field_errors)
@@ -514,6 +524,14 @@ def _get_user_and_profile(username):
     return existing_user, existing_user_profile
 
 
+def contains_html(value):
+    """
+    Validator method to check whether name contains html tags
+    """
+    regex = re.compile('(<|>)', re.UNICODE)
+    return bool(regex.search(value))
+
+
 def _validate(validation_func, err, *args):
     """Generic validation function that returns default on
     no errors, but the message associated with the err class
diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_api.py b/openedx/core/djangoapps/user_api/accounts/tests/test_api.py
index 8362da611a..b67484f603 100644
--- a/openedx/core/djangoapps/user_api/accounts/tests/test_api.py
+++ b/openedx/core/djangoapps/user_api/accounts/tests/test_api.py
@@ -179,22 +179,25 @@ class TestAccountApi(UserSettingsEventTestMixin, TestCase):
     def test_update_multiple_validation_errors(self):
         """Test that all validation errors are built up and returned at once"""
         # Send a read-only error, serializer error, and email validation error.
+
         naughty_update = {
             "username": "not_allowed",
             "gender": "undecided",
-            "email": "not an email address"
+            "email": "not an email address",
+            "name": "<p style=\"font-size:300px; color:green;\"></br>Name<input type=\"text\"></br>Content spoof"
         }
 
         with self.assertRaises(AccountValidationError) as context_manager:
             update_account_settings(self.user, naughty_update)
         field_errors = context_manager.exception.field_errors
-        self.assertEqual(3, len(field_errors))
+        self.assertEqual(4, len(field_errors))
         self.assertEqual("This field is not editable via this API", field_errors["username"]["developer_message"])
         self.assertIn(
             "Value \'undecided\' is not valid for field \'gender\'",
             field_errors["gender"]["developer_message"]
         )
         self.assertIn("Valid e-mail address required.", field_errors["email"]["developer_message"])
+        self.assertIn("Full Name cannot contain the following characters: < >", field_errors["name"]["user_message"])
 
     @patch('django.core.mail.send_mail')
     @patch('student.views.render_to_string', Mock(side_effect=mock_render_to_string, autospec=True))