From 0c9937889b47429e5dfbed427c322a48a70e47af Mon Sep 17 00:00:00 2001 From: cahrens Date: Mon, 1 Feb 2016 15:37:38 -0500 Subject: [PATCH] Enabling XSS vulnerability flag for bok choy tests --- common/test/acceptance/pages/lms/auto_auth.py | 7 +++++-- common/test/acceptance/tests/helpers.py | 3 ++- .../tests/lms/test_account_settings.py | 16 +++++++++------- .../tests/studio/base_studio_test.py | 19 +++++++++++++++++-- .../tests/studio/test_studio_course_team.py | 4 ++-- .../test_studio_settings_certificates.py | 2 +- .../tests/test_cohorted_courseware.py | 5 +++-- pavelib/bok_choy.py | 2 ++ .../paver_tests/test_paver_bok_choy_cmds.py | 18 +++++++++++++++++- pavelib/utils/test/suites/bokchoy_suite.py | 3 +++ requirements/edx/base.txt | 2 +- requirements/edx/github.txt | 2 ++ 12 files changed, 64 insertions(+), 19 deletions(-) diff --git a/common/test/acceptance/pages/lms/auto_auth.py b/common/test/acceptance/pages/lms/auto_auth.py index 1e4d0681ba..5576a90d39 100644 --- a/common/test/acceptance/pages/lms/auto_auth.py +++ b/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\S+) \((?P.+?)\) with password \S+ and user_id (?P\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 diff --git a/common/test/acceptance/tests/helpers.py b/common/test/acceptance/tests/helpers.py index c0f00b9b62..355d35ffb4 100644 --- a/common/test/acceptance/tests/helpers.py +++ b/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 diff --git a/common/test/acceptance/tests/lms/test_account_settings.py b/common/test/acceptance/tests/lms/test_account_settings.py index ede578d60e..e76e8179fb 100644 --- a/common/test/acceptance/tests/lms/test_account_settings.py +++ b/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 ) diff --git a/common/test/acceptance/tests/studio/base_studio_test.py b/common/test/acceptance/tests/studio/base_studio_test.py index cf41ca148b..96f1fb706e 100644 --- a/common/test/acceptance/tests/studio/base_studio_test.py +++ b/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 diff --git a/common/test/acceptance/tests/studio/test_studio_course_team.py b/common/test/acceptance/tests/studio/test_studio_course_team.py index fcc1d657bd..42580b00ae 100644 --- a/common/test/acceptance/tests/studio/test_studio_course_team.py +++ b/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'] ) diff --git a/common/test/acceptance/tests/studio/test_studio_settings_certificates.py b/common/test/acceptance/tests/studio/test_studio_settings_certificates.py index dea9e57ca7..9e6814e6f3 100644 --- a/common/test/acceptance/tests/studio/test_studio_settings_certificates.py +++ b/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'], diff --git a/common/test/acceptance/tests/test_cohorted_courseware.py b/common/test/acceptance/tests/test_cohorted_courseware.py index a893252f89..8bd178770b 100644 --- a/common/test/acceptance/tests/test_cohorted_courseware.py +++ b/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" diff --git a/pavelib/bok_choy.py b/pavelib/bok_choy.py index e43a149d71..a858446c3f 100644 --- a/pavelib/bok_choy.py +++ b/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', False), 'serversonly': getattr(options, 'serversonly', False), 'testsonly': getattr(options, 'testsonly', False), 'default_store': getattr(options, 'default_store', os.environ.get('DEFAULT_STORE', 'split')), diff --git a/pavelib/paver_tests/test_paver_bok_choy_cmds.py b/pavelib/paver_tests/test_paver_bok_choy_cmds.py index 87a0969cd6..849133ccb2 100644 --- a/pavelib/paver_tests/test_paver_bok_choy_cmds.py +++ b/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) diff --git a/pavelib/utils/test/suites/bokchoy_suite.py b/pavelib/utils/test/suites/bokchoy_suite.py index 9553d83805..d9c03ff1ef 100644 --- a/pavelib/utils/test/suites/bokchoy_suite.py +++ b/pavelib/utils/test/suites/bokchoy_suite.py @@ -37,6 +37,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 +54,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', 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 +225,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()) diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 965f003a4e..ea9a5eff10 100644 --- a/requirements/edx/base.txt +++ b/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.4.10 chrono==1.0.2 coverage==4.0.2 ddt==0.8.0 diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index 2e6cfb56f6..49a7439135 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -98,3 +98,5 @@ git+https://github.com/edx/xblock-lti-consumer.git@v1.0.3#egg=xblock-lti-consume -e git+https://github.com/mitodl/edx-sga@172a90fd2738f8142c10478356b2d9ed3e55334a#egg=edx-sga -e git+https://github.com/open-craft/xblock-poll@e7a6c95c300e95c51e42bfd1eba70489c05a6527#egg=xblock-poll git+https://github.com/edx-solutions/xblock-drag-and-drop-v2@v2.0.4#egg=xblock-drag-and-drop-v2==2.0.4 + +-e git+https://github.com/edx/bok-choy@christina/xss#egg=bok-choy