Merge pull request #11758 from edx/christina/xss-tests

Bok choy XSS changes

common/test/acceptance/pages/lms/auto_auth.py

@@ -4,7 +4,7 @@ Auto-auth page (used to automatically log in during testing).
 
 import re
 import urllib
-from bok_choy.page_object import PageObject, unguarded
+from bok_choy.page_object import PageObject, unguarded, XSS_INJECTION
 from . import AUTH_BASE_URL
 
 
@@ -17,7 +17,7 @@ class AutoAuthPage(PageObject):
 
     CONTENT_REGEX = r'.+? user (?P<username>\S+) \((?P<email>.+?)\) with password \S+ and user_id (?P<user_id>\d+)$'
 
-    def __init__(self, browser, username=None, email=None, password=None, staff=None, course_id=None,
+    def __init__(self, browser, username=None, email=None, password=None, full_name=None, staff=None, course_id=None,
                  enrollment_mode=None, roles=None):
         """
         Auto-auth is an end-point for HTTP GET requests.
@@ -25,6 +25,7 @@ class AutoAuthPage(PageObject):
         but you can also specify credentials using querystring parameters.
 
         `username`, `email`, and `password` are the user's credentials (strings)
+        'full_name' is the profile full name value
         `staff` is a boolean indicating whether the user is global staff.
         `course_id` is the ID of the course to enroll the student in.
         Currently, this has the form "org/number/run"
@@ -42,6 +43,8 @@ class AutoAuthPage(PageObject):
         if username is not None:
             self._params['username'] = username
 
+        self._params['full_name'] = full_name if full_name is not None else XSS_INJECTION
+
         if email is not None:
             self._params['email'] = email
 

common/test/acceptance/tests/helpers.py

@@ -16,6 +16,7 @@ from path import Path as path
 from bok_choy.javascript import js_defined
 from bok_choy.web_app_test import WebAppTest
 from bok_choy.promise import EmptyPromise, Promise
+from bok_choy.page_object import XSS_INJECTION
 from opaque_keys.edx.locator import CourseLocator
 from pymongo import MongoClient, ASCENDING
 from openedx.core.lib.tests.assertions.events import assert_event_matches, is_matching_event, EventMatchTolerates
@@ -640,7 +641,7 @@ class UniqueCourseTest(WebAppTest):
             'org': 'test_org',
             'number': self.unique_id,
             'run': 'test_run',
-            'display_name': 'Test Course' + self.unique_id
+            'display_name': 'Test Course' + XSS_INJECTION + self.unique_id
         }
 
     @property

common/test/acceptance/tests/lms/test_account_settings.py

@@ -6,6 +6,7 @@ from unittest import skip
 from nose.plugins.attrib import attr
 
 from bok_choy.web_app_test import WebAppTest
+from bok_choy.page_object import XSS_INJECTION
 
 from ...pages.lms.account_settings import AccountSettingsPage
 from ...pages.lms.auto_auth import AutoAuthPage
@@ -33,12 +34,12 @@ class AccountSettingsTestMixin(EventsTestMixin, WebAppTest):
         self.account_settings_page.visit()
         self.account_settings_page.wait_for_ajax()
 
-    def log_in_as_unique_user(self, email=None):
+    def log_in_as_unique_user(self, email=None, full_name=None):
         """
         Create a unique user and return the account's username and id.
         """
         username = "test_{uuid}".format(uuid=self.unique_id[0:6])
-        auto_auth_page = AutoAuthPage(self.browser, username=username, email=email).visit()
+        auto_auth_page = AutoAuthPage(self.browser, username=username, email=email, full_name=full_name).visit()
         user_id = auto_auth_page.get_user_id()
         return username, user_id
 
@@ -122,7 +123,8 @@ class AccountSettingsPageTest(AccountSettingsTestMixin, WebAppTest):
         Initialize account and pages.
         """
         super(AccountSettingsPageTest, self).setUp()
-        self.username, self.user_id = self.log_in_as_unique_user()
+        self.full_name = XSS_INJECTION
+        self.username, self.user_id = self.log_in_as_unique_user(full_name=self.full_name)
         self.visit_account_settings_page()
 
     def test_page_view_event(self):
@@ -259,16 +261,16 @@ class AccountSettingsPageTest(AccountSettingsTestMixin, WebAppTest):
         self._test_text_field(
             u'name',
             u'Full Name',
-            self.username,
+            self.full_name,
             u'@',
-            [u'another name', self.username],
+            [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.username, 'another name'),
-                self.expected_settings_changed_event('name', 'another name', self.username),
+                self.expected_settings_changed_event('name', self.full_name, 'another name'),
+                self.expected_settings_changed_event('name', 'another name', self.full_name),
             ],
             actual_events
         )

common/test/acceptance/tests/studio/base_studio_test.py

@@ -2,6 +2,7 @@
 Base classes used by studio tests.
 """
 from bok_choy.web_app_test import WebAppTest
+from bok_choy.page_object import XSS_INJECTION
 from ...pages.studio.auto_auth import AutoAuthPage
 from ...fixtures.course import CourseFixture
 from ...fixtures.library import LibraryFixture
@@ -15,11 +16,12 @@ class StudioCourseTest(UniqueCourseTest):
     Base class for all Studio course tests.
     """
 
-    def setUp(self, is_staff=False):
+    def setUp(self, is_staff=False, test_xss=True):  # pylint: disable=arguments-differ
         """
         Install a course with no content using a fixture.
         """
         super(StudioCourseTest, self).setUp()
+        self.test_xss = test_xss
         self.install_course_fixture(is_staff)
 
     def install_course_fixture(self, is_staff=False):
@@ -30,8 +32,21 @@ class StudioCourseTest(UniqueCourseTest):
             self.course_info['org'],
             self.course_info['number'],
             self.course_info['run'],
-            self.course_info['display_name']
+            self.course_info['display_name'],
         )
+        if self.test_xss:
+            xss_injected_unique_id = XSS_INJECTION + self.unique_id
+            test_improper_escaping = {u"value": xss_injected_unique_id}
+            self.course_fixture.add_advanced_settings({
+                "advertised_start": test_improper_escaping,
+                "info_sidebar_name": test_improper_escaping,
+                "cert_name_short": test_improper_escaping,
+                "cert_name_long": test_improper_escaping,
+                "display_organization": test_improper_escaping,
+                "display_coursenumber": test_improper_escaping,
+            })
+            self.course_info['display_organization'] = xss_injected_unique_id
+            self.course_info['display_coursenumber'] = xss_injected_unique_id
         self.populate_course_fixture(self.course_fixture)
         self.course_fixture.install()
         self.user = self.course_fixture.user

common/test/acceptance/tests/studio/test_studio_course_team.py

@@ -61,8 +61,8 @@ class CourseTeamPageTest(StudioCourseTest):
         def check_course_equality(course1, course2):
             """ Compares to course dictionaries using org, number and run as keys"""
             return (
-                course1['org'] == course2['org'] and
-                course1['number'] == course2['number'] and
+                course1['org'] == course2['display_organization'] and
+                course1['number'] == course2['display_coursenumber'] and
                 course1['run'] == course2['run']
             )
 

common/test/acceptance/tests/studio/test_studio_settings_certificates.py

@@ -19,7 +19,7 @@ class CertificatesTest(StudioCourseTest):
     Tests for settings/certificates Page.
     """
     def setUp(self):  # pylint: disable=arguments-differ
-        super(CertificatesTest, self).setUp(is_staff=True)
+        super(CertificatesTest, self).setUp(is_staff=True, test_xss=False)
         self.certificates_page = CertificatesPage(
             self.browser,
             self.course_info['org'],

common/test/acceptance/tests/test_cohorted_courseware.py

@@ -18,6 +18,7 @@ from ..pages.lms.auto_auth import AutoAuthPage as LmsAutoAuthPage
 from ..tests.lms.test_lms_user_preview import verify_expected_problem_visibility
 
 from bok_choy.promise import EmptyPromise
+from bok_choy.page_object import XSS_INJECTION
 
 
 @attr('shard_5')
@@ -28,8 +29,8 @@ class EndToEndCohortedCoursewareTest(ContainerBase):
         super(EndToEndCohortedCoursewareTest, self).setUp(is_staff=is_staff)
         self.staff_user = self.user
 
-        self.content_group_a = "Content Group A"
-        self.content_group_b = "Content Group B"
+        self.content_group_a = "Content Group A" + XSS_INJECTION
+        self.content_group_b = "Content Group B" + XSS_INJECTION
 
         # Create a student who will be in "Cohort A"
         self.cohort_a_student_username = "cohort_a_student"

pavelib/bok_choy.py

@@ -25,6 +25,7 @@ BOKCHOY_OPTS = [
     ('default_store=', 's', 'Default modulestore'),
     ('test_dir=', 'd', 'Directory for finding tests (relative to common/test/acceptance)'),
     ('num_processes=', 'n', 'Number of test threads (for multiprocessing)'),
+    ('verify_xss', 'x', 'Run XSS vulnerability tests'),
     make_option("--verbose", action="store_const", const=2, dest="verbosity"),
     make_option("-q", "--quiet", action="store_const", const=0, dest="verbosity"),
     make_option("-v", "--verbosity", action="count", dest="verbosity"),
@@ -43,6 +44,7 @@ def parse_bokchoy_opts(options):
         'test_spec': getattr(options, 'test_spec', None),
         'fasttest': getattr(options, 'fasttest', False),
         'num_processes': int(getattr(options, 'num_processes', 1)),
+        'verify_xss': getattr(options, 'verify_xss', os.environ.get('VERIFY_XSS', False)),
         'serversonly': getattr(options, 'serversonly', False),
         'testsonly': getattr(options, 'testsonly', False),
         'default_store': getattr(options, 'default_store', os.environ.get('DEFAULT_STORE', 'split')),

pavelib/paver_tests/test_paver_bok_choy_cmds.py

@@ -4,6 +4,7 @@ Run just this test with: paver test_lib -t pavelib/paver_tests/test_paver_bok_ch
 """
 import os
 import unittest
+from test.test_support import EnvironmentVarGuard
 from paver.easy import BuildFailure
 from pavelib.utils.test.suites import BokChoyTestSuite
 
@@ -15,7 +16,7 @@ class TestPaverBokChoyCmd(unittest.TestCase):
     Paver Bok Choy Command test cases
     """
 
-    def _expected_command(self, name, store=None):
+    def _expected_command(self, name, store=None, verify_xss=False):
         """
         Returns the command that is expected to be run for the given test spec
         and store.
@@ -27,6 +28,7 @@ class TestPaverBokChoyCmd(unittest.TestCase):
             "BOK_CHOY_HAR_DIR='{repo_dir}/test_root/log{shard_str}/hars' "
             "BOKCHOY_A11Y_CUSTOM_RULES_FILE='{repo_dir}/{a11y_custom_file}' "
             "SELENIUM_DRIVER_LOG_DIR='{repo_dir}/test_root/log{shard_str}' "
+            "VERIFY_XSS='{verify_xss}' "
             "nosetests {repo_dir}/common/test/acceptance/{exp_text} "
             "--with-xunit "
             "--xunit-file={repo_dir}/reports/bok_choy{shard_str}/xunit.xml "
@@ -37,12 +39,14 @@ class TestPaverBokChoyCmd(unittest.TestCase):
             shard_str='/shard_' + self.shard if self.shard else '',
             exp_text=name,
             a11y_custom_file='node_modules/edx-custom-a11y-rules/lib/custom_a11y_rules.js',
+            verify_xss=verify_xss
         )
         return expected_statement
 
     def setUp(self):
         super(TestPaverBokChoyCmd, self).setUp()
         self.shard = os.environ.get('SHARD')
+        self.env_var_override = EnvironmentVarGuard()
 
     def test_default(self):
         suite = BokChoyTestSuite('')
@@ -89,6 +93,18 @@ class TestPaverBokChoyCmd(unittest.TestCase):
         suite = BokChoyTestSuite('', serversonly=True)
         self.assertEqual(suite.cmd, "")
 
+    def test_verify_xss(self):
+        suite = BokChoyTestSuite('', verify_xss=True)
+        name = 'tests'
+        self.assertEqual(suite.cmd, self._expected_command(name=name, verify_xss=True))
+
+    def test_verify_xss_env_var(self):
+        self.env_var_override.set('VERIFY_XSS', 'True')
+        with self.env_var_override:
+            suite = BokChoyTestSuite('')
+            name = 'tests'
+            self.assertEqual(suite.cmd, self._expected_command(name=name, verify_xss=True))
+
     def test_test_dir(self):
         test_dir = 'foo'
         suite = BokChoyTestSuite('', test_dir=test_dir)

pavelib/utils/test/suites/bokchoy_suite.py

@@ -11,6 +11,8 @@ from pavelib.utils.envs import Env
 from pavelib.utils.test import bokchoy_utils
 from pavelib.utils.test import utils as test_utils
 
+import os
+
 try:
     from pygments.console import colorize
 except ImportError:
@@ -37,6 +39,7 @@ class BokChoyTestSuite(TestSuite):
       default_store - modulestore to use when running tests (split or draft)
       num_processes - number of processes or threads to use in tests. Recommendation is that this
       is less than or equal to the number of available processors.
+      verify_xss - when set, check for XSS vulnerabilities in the page HTML.
       See nosetest documentation: http://nose.readthedocs.org/en/latest/usage.html
     """
     def __init__(self, *args, **kwargs):
@@ -53,6 +56,7 @@ class BokChoyTestSuite(TestSuite):
         self.default_store = kwargs.get('default_store', None)
         self.verbosity = kwargs.get('verbosity', DEFAULT_VERBOSITY)
         self.num_processes = kwargs.get('num_processes', DEFAULT_NUM_PROCESSES)
+        self.verify_xss = kwargs.get('verify_xss', os.environ.get('VERIFY_XSS', False))
         self.extra_args = kwargs.get('extra_args', '')
         self.har_dir = self.log_dir / 'hars'
         self.a11y_file = Env.BOK_CHOY_A11Y_CUSTOM_RULES_FILE
@@ -223,6 +227,7 @@ class BokChoyTestSuite(TestSuite):
             "BOK_CHOY_HAR_DIR='{}'".format(self.har_dir),
             "BOKCHOY_A11Y_CUSTOM_RULES_FILE='{}'".format(self.a11y_file),
             "SELENIUM_DRIVER_LOG_DIR='{}'".format(self.log_dir),
+            "VERIFY_XSS='{}'".format(self.verify_xss),
             "nosetests",
             test_spec,
             "{}".format(self.verbosity_processes_string())

requirements/edx/base.txt

@@ -136,7 +136,7 @@ django_debug_toolbar==1.3.2
 
 # Used for testing
 before_after==0.1.3
-bok-choy==0.4.10
+bok-choy==0.5.0
 chrono==1.0.2
 coverage==4.0.2
 ddt==0.8.0