Merge pull request #16807 from edx/LEARNER-3385/content-injection-on-registration

Verify 'Full Name' field does not allow HTML in Signup form

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__(

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.

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.

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
         )
 

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: