From 8ffac2061d7291dd0f5267fa878cb4cdda495b83 Mon Sep 17 00:00:00 2001
From: uzairr <uzairr@yahoo.com>
Date: Wed, 6 Dec 2017 14:58:17 +0000
Subject: [PATCH] Verify 'Full Name' field does not allow HTML in Signup form

'Full Name' field in the signup form is allowing HTML as an input
which makes spoofing easily.To avoid it, validation is added
that will ensure 'Full Name' field does not allow HTML.

LEARNER-3385
---
 common/djangoapps/student/forms.py                 | 13 ++++++++++++-
 .../student/tests/test_long_username_email.py      |  8 ++++++++
 common/test/acceptance/pages/common/auto_auth.py   |  5 +++--
 .../acceptance/tests/lms/test_account_settings.py  |  8 ++++----
 openedx/core/djangoapps/user_api/accounts/api.py   | 14 +++++++++-----
 5 files changed, 36 insertions(+), 12 deletions(-)

diff --git a/common/djangoapps/student/forms.py b/common/djangoapps/student/forms.py
index 92d17ffa74..329d924510 100644
--- a/common/djangoapps/student/forms.py
+++ b/common/djangoapps/student/forms.py
@@ -128,6 +128,16 @@ def validate_username(username):
     validator(username)
 
 
+def validate_name(name):
+    """
+    Verifies a Full_Name is valid, raises a ValidationError otherwise.
+    Args:
+        name (unicode): The name to validate.
+    """
+    if accounts_settings.api.contains_html(name):
+        raise forms.ValidationError(_('Full Name cannot contain the following characters: < >'))
+
+
 class UsernameField(forms.CharField):
     """
     A CharField that validates usernames based on the `ENABLE_UNICODE_USERNAME` feature.
@@ -192,7 +202,8 @@ class AccountCreationForm(forms.Form):
         error_messages={
             "required": _NAME_TOO_SHORT_MSG,
             "min_length": _NAME_TOO_SHORT_MSG,
-        }
+        },
+        validators=[validate_name]
     )
 
     def __init__(
diff --git a/common/djangoapps/student/tests/test_long_username_email.py b/common/djangoapps/student/tests/test_long_username_email.py
index f949440ba2..73ffd882dc 100644
--- a/common/djangoapps/student/tests/test_long_username_email.py
+++ b/common/djangoapps/student/tests/test_long_username_email.py
@@ -39,6 +39,14 @@ class TestLongUsernameEmail(TestCase):
             USERNAME_BAD_LENGTH_MSG,
         )
 
+    def test_spoffed_name(self):
+        """
+        Test name cannot contains html.
+        """
+        self.url_params['name'] = '<p style="font-size:300px; color:green;"></br>Name<input type="text"></br>Content spoof'
+        response = self.client.post(self.url, self.url_params)
+        self.assertEqual(response.status_code, 400)
+
     def test_long_email(self):
         """
         Test email cannot be more than 254 characters long.
diff --git a/common/test/acceptance/pages/common/auto_auth.py b/common/test/acceptance/pages/common/auto_auth.py
index 64773cc190..f35e9c43f4 100644
--- a/common/test/acceptance/pages/common/auto_auth.py
+++ b/common/test/acceptance/pages/common/auto_auth.py
@@ -5,12 +5,13 @@ import json
 import os
 import urllib
 
-from bok_choy.page_object import XSS_INJECTION, PageObject, unguarded
+from bok_choy.page_object import PageObject, unguarded
 
 # The URL used for user auth in testing
 HOSTNAME = os.environ.get('BOK_CHOY_HOSTNAME', 'localhost')
 CMS_PORT = os.environ.get('BOK_CHOY_CMS_PORT', 8031)
 AUTH_BASE_URL = os.environ.get('test_url', 'http://{}:{}'.format(HOSTNAME, CMS_PORT))
+FULL_NAME = 'Test'
 
 
 class AutoAuthPage(PageObject):
@@ -23,7 +24,7 @@ class AutoAuthPage(PageObject):
     # Internal cache for parsed user info.
     _user_info = None
 
-    def __init__(self, browser, username=None, email=None, password=None, full_name=XSS_INJECTION, staff=False, superuser=None,
+    def __init__(self, browser, username=None, email=None, password=None, full_name=FULL_NAME, staff=False, superuser=None,
                  course_id=None, enrollment_mode=None, roles=None, no_login=False, is_active=True, course_access_roles=None):
         """
         Auto-auth is an end-point for HTTP GET requests.
diff --git a/common/test/acceptance/tests/lms/test_account_settings.py b/common/test/acceptance/tests/lms/test_account_settings.py
index 994b9c669d..11239f2c70 100644
--- a/common/test/acceptance/tests/lms/test_account_settings.py
+++ b/common/test/acceptance/tests/lms/test_account_settings.py
@@ -9,7 +9,7 @@ from bok_choy.page_object import XSS_INJECTION
 from nose.plugins.attrib import attr
 from pytz import timezone, utc
 
-from common.test.acceptance.pages.common.auto_auth import AutoAuthPage
+from common.test.acceptance.pages.common.auto_auth import AutoAuthPage, FULL_NAME
 from common.test.acceptance.pages.lms.account_settings import AccountSettingsPage
 from common.test.acceptance.pages.lms.dashboard import DashboardPage
 from common.test.acceptance.tests.helpers import AcceptanceTest, EventsTestMixin
@@ -123,7 +123,7 @@ class AccountSettingsPageTest(AccountSettingsTestMixin, AcceptanceTest):
         Initialize account and pages.
         """
         super(AccountSettingsPageTest, self).setUp()
-        self.full_name = XSS_INJECTION
+        self.full_name = FULL_NAME
         self.social_link = ''
         self.username, self.user_id = self.log_in_as_unique_user(full_name=self.full_name)
         self.visit_account_settings_page()
@@ -275,8 +275,8 @@ class AccountSettingsPageTest(AccountSettingsTestMixin, AcceptanceTest):
             u'Full Name',
             self.full_name,
             u'@',
-            [u'<h1>another name<h1>', self.full_name],
-            u'Full Name cannot contain the following characters: < >',
+            [u'<h1>another name<h1>', u'<script>'],
+            'Full Name cannot contain the following characters: < >',
             False
         )
 
diff --git a/openedx/core/djangoapps/user_api/accounts/api.py b/openedx/core/djangoapps/user_api/accounts/api.py
index 9043af0ac9..b6a847c50d 100644
--- a/openedx/core/djangoapps/user_api/accounts/api.py
+++ b/openedx/core/djangoapps/user_api/accounts/api.py
@@ -173,11 +173,15 @@ 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 the user asked to change full name, validate it
+    if changing_full_name:
+        try:
+            student_forms.validate_name(update['name'])
+        except ValidationError as err:
+            field_errors["name"] = {
+                "developer_message": u"Error thrown from validate_name: '{}'".format(err.message),
+                "user_message": err.message
+            }
 
     # If we have encountered any validation errors, return them to the user.
     if field_errors: