diff --git a/cms/djangoapps/contentstore/management/commands/delete_course.py b/cms/djangoapps/contentstore/management/commands/delete_course.py index 61f99b1dab..da2bf1713b 100644 --- a/cms/djangoapps/contentstore/management/commands/delete_course.py +++ b/cms/djangoapps/contentstore/management/commands/delete_course.py @@ -57,7 +57,11 @@ class Command(BaseCommand): def handle(self, *args, **options): try: # a course key may have unicode chars in it - course_key = text_type(options['course_key'], 'utf8') + try: + course_key = text_type(options['course_key'], 'utf8') + # May already be decoded to unicode if coming in through tests, this is ok. + except TypeError: + course_key = text_type(options['course_key']) course_key = CourseKey.from_string(course_key) except InvalidKeyError: raise CommandError('Invalid course_key: {}'.format(options['course_key'])) diff --git a/cms/djangoapps/contentstore/tasks.py b/cms/djangoapps/contentstore/tasks.py index fd3761f520..5c24e8f585 100644 --- a/cms/djangoapps/contentstore/tasks.py +++ b/cms/djangoapps/contentstore/tasks.py @@ -254,7 +254,7 @@ def export_olx(self, user_id, course_key_string, language): self.status.set_state(u'Exporting') tarball = create_export_tarball(courselike_module, courselike_key, {}, self.status) artifact = UserTaskArtifact(status=self.status, name=u'Output') - artifact.file.save(name=tarball.name, content=File(tarball)) # pylint: disable=no-member + artifact.file.save(name=os.path.basename(tarball.name), content=File(tarball)) # pylint: disable=no-member artifact.save() # catch all exceptions so we can record useful error messages except Exception as exception: # pylint: disable=broad-except diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index d28de5216a..5c388b0609 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -1,5 +1,7 @@ # -*- coding: utf-8 -*- +from __future__ import print_function + import copy import shutil from datetime import timedelta @@ -10,6 +12,7 @@ from unittest import SkipTest from uuid import uuid4 import ddt +import django import lxml.html import mock from django.conf import settings @@ -156,7 +159,7 @@ class ImportRequiredTestCases(ContentStoreTestCase): # Test course export does not fail root_dir = path(mkdtemp_clean()) - print 'Exporting to tempdir = {0}'.format(root_dir) + print('Exporting to tempdir = {0}'.format(root_dir)) export_course_to_xml(self.store, content_store, course.id, root_dir, u'test_export') filesystem = OSFS(text_type(root_dir / 'test_export/static')) @@ -171,11 +174,11 @@ class ImportRequiredTestCases(ContentStoreTestCase): shutil.rmtree(root_dir) def test_about_overrides(self): - ''' + """ This test case verifies that a course can use specialized override for about data, e.g. /about/Fall_2012/effort.html while there is a base definition in /about/effort.html - ''' + """ course_items = import_course_from_xml( self.store, self.user.id, TEST_DATA_DIR, ['toy'], create_if_not_present=True ) @@ -189,9 +192,9 @@ class ImportRequiredTestCases(ContentStoreTestCase): @requires_pillow_jpeg def test_asset_import(self): - ''' + """ This test validates that an image asset is imported and a thumbnail was generated for a .gif - ''' + """ content_store = contentstore() import_course_from_xml( @@ -255,7 +258,7 @@ class ImportRequiredTestCases(ContentStoreTestCase): # now export the course to a tempdir and test that it contains files 'updates.html' and 'updates.items.json' # with same content as in course 'info' directory root_dir = path(mkdtemp_clean()) - print 'Exporting to tempdir = {0}'.format(root_dir) + print('Exporting to tempdir = {0}'.format(root_dir)) export_course_to_xml(self.store, content_store, course.id, root_dir, u'test_export') # check that exported course has files 'updates.html' and 'updates.items.json' @@ -313,7 +316,7 @@ class ImportRequiredTestCases(ContentStoreTestCase): course_id = self.import_and_populate_course() root_dir = path(mkdtemp_clean()) - print 'Exporting to tempdir = {0}'.format(root_dir) + print('Exporting to tempdir = {0}'.format(root_dir)) # export out to a tempdir export_course_to_xml(self.store, content_store, course_id, root_dir, u'test_export') @@ -415,7 +418,7 @@ class ImportRequiredTestCases(ContentStoreTestCase): root_dir = path(mkdtemp_clean()) - print 'Exporting to tempdir = {0}'.format(root_dir) + print('Exporting to tempdir = {0}'.format(root_dir)) # export out to a tempdir export_course_to_xml(self.store, content_store, course_id, root_dir, u'test_export') @@ -441,7 +444,7 @@ class ImportRequiredTestCases(ContentStoreTestCase): root_dir = path(mkdtemp_clean()) - print 'Exporting to tempdir = {0}'.format(root_dir) + print('Exporting to tempdir = {0}'.format(root_dir)) # export out to a tempdir export_course_to_xml(self.store, content_store, course_id, root_dir, u'test_export') @@ -541,7 +544,7 @@ class ImportRequiredTestCases(ContentStoreTestCase): root_dir = path(mkdtemp_clean()) - print 'Exporting to tempdir = {0}'.format(root_dir) + print('Exporting to tempdir = {0}'.format(root_dir)) export_course_to_xml(self.store, None, course_id, root_dir, u'test_export_no_content_store') # Delete the course from module store and reimport it @@ -684,8 +687,8 @@ class MiscCourseTests(ContentStoreTestCase): self.assertNotIn(malicious_code, resp.content) def test_advanced_components_in_edit_unit(self): - # This could be made better, but for now let's just assert that we see the advanced modules mentioned in the page - # response HTML + # This could be made better, but for now let's just assert that we see the advanced modules mentioned in the + # page response HTML self.check_components_on_page( ADVANCED_COMPONENT_TYPES, ['Word cloud', 'Annotation', 'Text Annotation', 'Video Annotation', 'Image Annotation', @@ -713,7 +716,7 @@ class MiscCourseTests(ContentStoreTestCase): # Now export the course to a tempdir and test that it contains assets. The export should pass root_dir = path(mkdtemp_clean()) - print 'Exporting to tempdir = {0}'.format(root_dir) + print('Exporting to tempdir = {0}'.format(root_dir)) export_course_to_xml(self.store, content_store, self.course.id, root_dir, u'test_export') filesystem = OSFS(root_dir / 'test_export/static') @@ -773,7 +776,7 @@ class MiscCourseTests(ContentStoreTestCase): # Now export the course to a tempdir and test that it contains assets. root_dir = path(mkdtemp_clean()) - print 'Exporting to tempdir = {0}'.format(root_dir) + print('Exporting to tempdir = {0}'.format(root_dir)) export_course_to_xml(self.store, content_store, self.course.id, root_dir, u'test_export') # Verify that asset have been overwritten during export. @@ -807,11 +810,11 @@ class MiscCourseTests(ContentStoreTestCase): return cnt def test_get_items(self): - ''' + """ This verifies a bug we had where the None setting in get_items() meant 'wildcard' Unfortunately, None = published for the revision field, so get_items() would return both draft and non-draft copies. - ''' + """ self.store.convert_to_draft(self.problem.location, self.user.id) # Query get_items() and find the html item. This should just return back a single item (not 2). @@ -832,11 +835,11 @@ class MiscCourseTests(ContentStoreTestCase): self.assertTrue(getattr(items_from_draft_store[0], 'is_draft', False)) def test_draft_metadata(self): - ''' + """ This verifies a bug we had where inherited metadata was getting written to the module as 'own-metadata' when publishing. Also verifies the metadata inheritance is properly computed - ''' + """ # refetch course so it has all the children correct course = self.store.update_item(self.course, self.user.id) course.graceperiod = timedelta(days=1, hours=5, minutes=59, seconds=59) @@ -935,7 +938,7 @@ class MiscCourseTests(ContentStoreTestCase): """ Tests the ajax callback to render an XModule """ - with override_settings(COURSES_WITH_UNSAFE_CODE=[unicode(self.course.id)]): + with override_settings(COURSES_WITH_UNSAFE_CODE=[text_type(self.course.id)]): # also try a custom response which will trigger the 'is this course in whitelist' logic resp = self.client.get_json( get_url('xblock_view_handler', self.vert_loc, kwargs={'view_name': 'container_preview'}) @@ -944,7 +947,7 @@ class MiscCourseTests(ContentStoreTestCase): vertical = self.store.get_item(self.vert_loc) for child in vertical.children: - self.assertContains(resp, unicode(child)) + self.assertContains(resp, text_type(child)) def test_delete(self): # make sure the parent points to the child object which is to be deleted @@ -963,9 +966,9 @@ class MiscCourseTests(ContentStoreTestCase): self.assertNotIn(self.seq_loc, chapter.children) def test_asset_delete_and_restore(self): - ''' + """ This test will exercise the soft delete/restore functionality of the assets - ''' + """ asset_key = self._delete_asset_in_course() # now try to find it in store, but they should not be there any longer @@ -977,7 +980,7 @@ class MiscCourseTests(ContentStoreTestCase): self.assertIsNotNone(content) # let's restore the asset - restore_asset_from_trashcan(unicode(asset_key)) + restore_asset_from_trashcan(text_type(asset_key)) # now try to find it in courseware store, and they should be back after restore content = contentstore('trashcan').find(asset_key, throw_on_not_found=False) @@ -1001,7 +1004,7 @@ class MiscCourseTests(ContentStoreTestCase): url = reverse_course_url( 'assets_handler', self.course.id, - kwargs={'asset_key_string': unicode(asset_key)} + kwargs={'asset_key_string': text_type(asset_key)} ) resp = self.client.delete(url) self.assertEqual(resp.status_code, 204) @@ -1009,9 +1012,9 @@ class MiscCourseTests(ContentStoreTestCase): return asset_key def test_empty_trashcan(self): - ''' + """ This test will exercise the emptying of the asset trashcan - ''' + """ self._delete_asset_in_course() # make sure there's something in the trashcan @@ -1115,7 +1118,7 @@ class MiscCourseTests(ContentStoreTestCase): # check that /static/ has been converted to the full path # note, we know the link it should be because that's what in the 'toy' course in the test data asset_key = self.course.id.make_asset_key('asset', 'handouts_sample_handout.txt') - self.assertContains(resp, unicode(asset_key)) + self.assertContains(resp, text_type(asset_key)) def test_prefetch_children(self): # make sure we haven't done too many round trips to DB: @@ -1492,8 +1495,8 @@ class ContentStoreTest(ContentStoreTestCase): self.assertContains( resp, '
'.format( - locator=unicode(course.location), - course_key=unicode(course.id), + locator=text_type(course.location), + course_key=text_type(course.id), ), status_code=200, html=True @@ -1504,7 +1507,7 @@ class ContentStoreTest(ContentStoreTestCase): course = CourseFactory.create() section_data = { - 'parent_locator': unicode(course.location), + 'parent_locator': text_type(course.location), 'category': 'chapter', 'display_name': 'Section One', } @@ -1513,7 +1516,7 @@ class ContentStoreTest(ContentStoreTestCase): self.assertEqual(resp.status_code, 200) data = parse_json(resp) - retarget = unicode(course.id.make_usage_key('chapter', 'REPLACE')).replace('REPLACE', r'([0-9]|[a-f]){3,}') + retarget = text_type(course.id.make_usage_key('chapter', 'REPLACE')).replace('REPLACE', r'([0-9]|[a-f]){3,}') self.assertRegexpMatches(data['locator'], retarget) def test_capa_module(self): @@ -1521,7 +1524,7 @@ class ContentStoreTest(ContentStoreTestCase): course = CourseFactory.create() problem_data = { - 'parent_locator': unicode(course.location), + 'parent_locator': text_type(course.location), 'category': 'problem' } @@ -1808,7 +1811,7 @@ class MetadataSaveTestCase(ContentStoreTestCase): course = CourseFactory.create() - video_sample_xml = ''' + video_sample_xml = """ - ''' + """ self.video_descriptor = ItemFactory.create( parent_location=course.location, category='video', data={'data': video_sample_xml} @@ -1878,7 +1881,7 @@ class RerunCourseTest(ContentStoreTestCase): """Create and send an ajax post for the rerun request""" # create data to post - rerun_course_data = {'source_course_key': unicode(source_course_key)} + rerun_course_data = {'source_course_key': text_type(source_course_key)} if not destination_course_data: destination_course_data = self.destination_course_data rerun_course_data.update(destination_course_data) @@ -1898,7 +1901,7 @@ class RerunCourseTest(ContentStoreTestCase): def get_unsucceeded_course_action_elements(self, html, course_key): """Returns the elements in the unsucceeded course action section that have the given course_key""" - return html.cssselect('.courses-processing li[data-course-key="{}"]'.format(unicode(course_key))) + return html.cssselect('.courses-processing li[data-course-key="{}"]'.format(text_type(course_key))) def assertInCourseListing(self, course_key): """ @@ -1943,7 +1946,7 @@ class RerunCourseTest(ContentStoreTestCase): source_course = CourseFactory.create() destination_course_key = self.post_rerun_request(source_course.id) self.verify_rerun_course(source_course.id, destination_course_key, self.destination_course_data['display_name']) - videos = list(get_videos_for_course(unicode(destination_course_key))) + videos = list(get_videos_for_course(text_type(destination_course_key))) self.assertEqual(0, len(videos)) self.assertInCourseListing(destination_course_key) @@ -1952,7 +1955,7 @@ class RerunCourseTest(ContentStoreTestCase): create_video( dict( edx_video_id="tree-hugger", - courses=[unicode(source_course.id)], + courses=[text_type(source_course.id)], status='test', duration=2, encoded_videos=[] @@ -1962,8 +1965,8 @@ class RerunCourseTest(ContentStoreTestCase): self.verify_rerun_course(source_course.id, destination_course_key, self.destination_course_data['display_name']) # Verify that the VAL copies videos to the rerun - source_videos = list(get_videos_for_course(unicode(source_course.id))) - target_videos = list(get_videos_for_course(unicode(destination_course_key))) + source_videos = list(get_videos_for_course(text_type(source_course.id))) + target_videos = list(get_videos_for_course(text_type(destination_course_key))) self.assertEqual(1, len(source_videos)) self.assertEqual(source_videos, target_videos) @@ -2188,7 +2191,7 @@ class SigninPageTestCase(TestCase): # ... # # ... - # + # response = self.client.get("/signin") csrf_token = response.cookies.get("csrftoken") form = lxml.html.fromstring(response.content).get_element_by_id("login_form") @@ -2197,7 +2200,14 @@ class SigninPageTestCase(TestCase): self.assertIsNotNone(csrf_token) self.assertIsNotNone(csrf_token.value) self.assertIsNotNone(csrf_input_field) - self.assertEqual(csrf_token.value, csrf_input_field.attrib["value"]) + + # TODO: Remove Django 1.11 upgrade shim + # SHIM: _compare_salted_tokens was introduced in 1.10. Move the import and use only that branch post-upgrade. + if django.VERSION < (1, 10): + self.assertEqual(csrf_token.value, csrf_input_field.attrib["value"]) + else: + from django.middleware.csrf import _compare_salted_tokens + self.assertTrue(_compare_salted_tokens(csrf_token.value, csrf_input_field.attrib["value"])) def _create_course(test, course_key, course_data): diff --git a/cms/djangoapps/contentstore/views/tests/test_library.py b/cms/djangoapps/contentstore/views/tests/test_library.py index dcfb0afaaf..042153004d 100644 --- a/cms/djangoapps/contentstore/views/tests/test_library.py +++ b/cms/djangoapps/contentstore/views/tests/test_library.py @@ -8,6 +8,7 @@ import mock from django.conf import settings from mock import patch from opaque_keys.edx.locator import CourseKey, LibraryLocator +from six import binary_type, text_type from contentstore.tests.utils import AjaxEnabledTestClient, CourseTestCase, parse_json from contentstore.utils import reverse_course_url, reverse_library_url @@ -23,7 +24,7 @@ LIBRARY_REST_URL = '/library/' # URL for GET/POST requests involving libraries def make_url_for_lib(key): """ Get the RESTful/studio URL for testing the given library """ if isinstance(key, LibraryLocator): - key = unicode(key) + key = text_type(key) return LIBRARY_REST_URL + key @@ -239,11 +240,11 @@ class UnitTestLibraries(CourseTestCase): self.assertEqual(response.status_code, 200) info = parse_json(response) self.assertEqual(info['display_name'], lib.display_name) - self.assertEqual(info['library_id'], unicode(lib_key)) + self.assertEqual(info['library_id'], text_type(lib_key)) self.assertEqual(info['previous_version'], None) self.assertNotEqual(info['version'], None) self.assertNotEqual(info['version'], '') - self.assertEqual(info['version'], unicode(version)) + self.assertEqual(info['version'], text_type(version)) def test_get_lib_edit_html(self): """ @@ -319,12 +320,12 @@ class UnitTestLibraries(CourseTestCase): """ library = LibraryFactory.create() extra_user, _ = self.create_non_staff_user() - manage_users_url = reverse_library_url('manage_library_users', unicode(library.location.library_key)) + manage_users_url = reverse_library_url('manage_library_users', text_type(library.location.library_key)) response = self.client.get(manage_users_url) self.assertEqual(response.status_code, 200) # extra_user has not been assigned to the library so should not show up in the list: - self.assertNotIn(extra_user.username, response.content) + self.assertNotIn(binary_type(extra_user.username), response.content) # Now add extra_user to the library: user_details_url = reverse_course_url( @@ -337,4 +338,4 @@ class UnitTestLibraries(CourseTestCase): # Now extra_user should apear in the list: response = self.client.get(manage_users_url) self.assertEqual(response.status_code, 200) - self.assertIn(extra_user.username, response.content) + self.assertIn(binary_type(extra_user.username), response.content) diff --git a/cms/djangoapps/course_creators/tests/test_admin.py b/cms/djangoapps/course_creators/tests/test_admin.py index 949fa45099..438b2e6bf8 100644 --- a/cms/djangoapps/course_creators/tests/test_admin.py +++ b/cms/djangoapps/course_creators/tests/test_admin.py @@ -3,6 +3,7 @@ Tests course_creators.admin.py. """ import mock +import django from django.contrib.admin.sites import AdminSite from django.contrib.auth.models import User from django.core import mail @@ -105,7 +106,13 @@ class CourseCreatorAdminTest(TestCase): # message sent. Admin message will follow. base_num_emails = 1 if expect_sent_to_user else 0 if expect_sent_to_admin: - context = {'user_name': "test_user", 'user_email': u'test_user+courses@edx.org'} + # TODO: Remove Django 1.11 upgrade shim + # SHIM: Usernames come back as unicode in 1.10+, remove this shim post-upgrade + if django.VERSION < (1, 10): + context = {'user_name': 'test_user', 'user_email': u'test_user+courses@edx.org'} + else: + context = {'user_name': u'test_user', 'user_email': u'test_user+courses@edx.org'} + self.assertEquals(base_num_emails + 1, len(mail.outbox), 'Expected admin message to be sent') sent_mail = mail.outbox[base_num_emails] self.assertEquals( diff --git a/cms/envs/test.py b/cms/envs/test.py index ca401048c3..ebf44c35fd 100644 --- a/cms/envs/test.py +++ b/cms/envs/test.py @@ -46,6 +46,9 @@ ALLOWED_HOSTS = [ '.testserver.fake', 'test-site.testserver', 'testserver.fakeother', + 'edx.org', + 'microsite.example.com', + 'testserver2', ] # mongo connection settings diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 1b5fcdb610..98829a79ed 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -1576,6 +1576,32 @@ def login_user(request, error=""): # pylint: disable=too-many-statements,unused if user_found_by_email_lookup and LoginFailures.is_feature_enabled(): LoginFailures.increment_lockout_counter(user_found_by_email_lookup) + # TODO: Remove Django 1.11 upgrade shim + # SHIM: In Django 1.10+ the user will not authenticate if it is not active (after the initial account creation, + # see NEW_USER_AUTH_BACKEND for how we handle that). This code is duplicated below for the < 1.10 case where + # the user is successfully authenticated, but not active. To remove this shim, remove the version check here + # and the duplicated code at the end of this function. + # + # Note that this will be triggered for ALL inactive users, regardless of whether the password is correct. In + # either case they need to activate before continuing so password status is moot. + if django.VERSION >= (1, 10) and user_found_by_email_lookup and not user_found_by_email_lookup.is_active: + if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + AUDIT_LOG.warning( + u"Login failed - Account not active for user.id: {0}, resending activation".format( + user_found_by_email_lookup.id) + ) + else: + AUDIT_LOG.warning(u"Login failed - Account not active for user {0}, resending activation".format( + user_found_by_email_lookup.username) + ) + + reactivation_email_for_user(user_found_by_email_lookup) + + return JsonResponse({ + "success": False, + "value": _generate_not_activated_message(user_found_by_email_lookup), + }) # TODO: this should be status code 400 # pylint: disable=fixme + # if we didn't find this username earlier, the account for this email # doesn't exist, and doesn't have a corresponding password if username != "": @@ -1654,17 +1680,20 @@ def login_user(request, error=""): # pylint: disable=too-many-statements,unused # detect that the user is logged in. return set_logged_in_cookies(request, response, user) - if settings.FEATURES['SQUELCH_PII_IN_LOGS']: - AUDIT_LOG.warning(u"Login failed - Account not active for user.id: {0}, resending activation".format(user.id)) - else: - AUDIT_LOG.warning(u"Login failed - Account not active for user {0}, resending activation".format(username)) + # TODO: Remove Django 1.11 upgrade shim + # SHIM: See above SHIM for details and removal instructions. + if django.VERSION < (1, 10): + if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + AUDIT_LOG.warning(u"Login failed - Account not active for user.id: {0}, resending activation".format(user.id)) + else: + AUDIT_LOG.warning(u"Login failed - Account not active for user {0}, resending activation".format(username)) - reactivation_email_for_user(user) + reactivation_email_for_user(user) - return JsonResponse({ - "success": False, - "value": _generate_not_activated_message(user), - }) # TODO: this should be status code 400 # pylint: disable=fixme + return JsonResponse({ + "success": False, + "value": _generate_not_activated_message(user), + }) # TODO: this should be status code 400 # pylint: disable=fixme @csrf_exempt diff --git a/common/djangoapps/track/management/commands/__init__.py b/common/djangoapps/track/management/commands/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/common/djangoapps/track/management/commands/tracked_dummy_command.py b/common/djangoapps/track/management/commands/tracked_dummy_command.py new file mode 100644 index 0000000000..62c6ae6bc5 --- /dev/null +++ b/common/djangoapps/track/management/commands/tracked_dummy_command.py @@ -0,0 +1,19 @@ +""" +Command used for testing TrackedCommands +""" + +import json + +from eventtracking import tracker as eventtracker +from track.management.tracked_command import TrackedCommand + + +class Command(TrackedCommand): + """A locally-defined command, for testing, that returns the current context as a JSON string.""" + def add_arguments(self, parser): + parser.add_argument('dummy_arg') + parser.add_argument('--key1') + parser.add_argument('--key2') + + def handle(self, *args, **options): + return json.dumps(eventtracker.get_tracker().resolve_context()) diff --git a/common/djangoapps/track/management/tests/test_tracked_command.py b/common/djangoapps/track/management/tests/test_tracked_command.py index 695703a192..94a8c7b1d3 100644 --- a/common/djangoapps/track/management/tests/test_tracked_command.py +++ b/common/djangoapps/track/management/tests/test_tracked_command.py @@ -1,24 +1,20 @@ import json from StringIO import StringIO + +from django.core.management import call_command from django.test import TestCase -from eventtracking import tracker as eventtracker - -from track.management.tracked_command import TrackedCommand - - -class DummyCommand(TrackedCommand): - """A locally-defined command, for testing, that returns the current context as a JSON string.""" - def handle(self, *args, **options): - return json.dumps(eventtracker.get_tracker().resolve_context()) - class CommandsTestBase(TestCase): - + """ + Command for testing track functionality + """ def _run_dummy_command(self, *args, **kwargs): - """Runs the test command's execute method directly, and outputs a dict of the current context.""" + """ + Calls the test command and outputs a dict of the current context. + """ out = StringIO() - DummyCommand().execute(*args, stdout=out, **kwargs) + call_command('tracked_dummy_command', *args, stdout=out, **kwargs) out.seek(0) return json.loads(out.read()) @@ -26,4 +22,4 @@ class CommandsTestBase(TestCase): args = ['whee'] kwargs = {'key1': 'default', 'key2': True} json_out = self._run_dummy_command(*args, **kwargs) - self.assertEquals(json_out['command'], 'unknown') + self.assertEquals(json_out['command'].strip(), 'tracked_dummy_command') diff --git a/lms/envs/common.py b/lms/envs/common.py index 6f1e33a2c4..0d77b8ba99 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -665,7 +665,7 @@ derived_collection_entry('DEFAULT_TEMPLATE_ENGINE', 'DIRS') ############################################################################################### # use the ratelimit backend to prevent brute force attacks -AUTHENTICATION_BACKENDS = ['ratelimitbackend.backends.RateLimitModelBackend'] +AUTHENTICATION_BACKENDS = ['ratelimitbackend.backends.RateLimitModelBackend', ] STUDENT_FILEUPLOAD_MAX_SIZE = 4 * 1000 * 1000 # 4 MB MAX_FILEUPLOADS_PER_INPUT = 20 diff --git a/lms/envs/test.py b/lms/envs/test.py index e1c4923027..adcca3ae91 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -40,7 +40,9 @@ ALLOWED_HOSTS = [ '.testserver.fake', 'test-site.testserver', 'testserver.fakeother', - 'edx.org' + 'edx.org', + 'microsite.example.com', + 'testserver2', ] # Silence noisy logs to make troubleshooting easier when tests fail. diff --git a/openedx/core/djangoapps/cors_csrf/authentication.py b/openedx/core/djangoapps/cors_csrf/authentication.py index 6083fee5d3..55bffe33cb 100644 --- a/openedx/core/djangoapps/cors_csrf/authentication.py +++ b/openedx/core/djangoapps/cors_csrf/authentication.py @@ -8,7 +8,8 @@ from .helpers import is_cross_domain_request_allowed, skip_cross_domain_referer_ class SessionAuthenticationCrossDomainCsrf(authentication.SessionAuthentication): - """Session authentication that skips the referer check over secure connections. + """ + Session authentication that skips the referer check over secure connections. Django Rest Framework's `SessionAuthentication` class calls Django's CSRF middleware implementation directly, which bypasses the middleware @@ -21,11 +22,12 @@ class SessionAuthenticationCrossDomainCsrf(authentication.SessionAuthentication) Since this subclass overrides only the `enforce_csrf()` method, it can be mixed in with other `SessionAuthentication` subclasses. - """ def enforce_csrf(self, request): - """Skip the referer check if the cross-domain request is allowed. """ + """ + Skip the referer check if the cross-domain request is allowed. + """ if is_cross_domain_request_allowed(request): with skip_cross_domain_referer_check(request): return super(SessionAuthenticationCrossDomainCsrf, self).enforce_csrf(request) diff --git a/openedx/core/djangoapps/cors_csrf/tests/test_authentication.py b/openedx/core/djangoapps/cors_csrf/tests/test_authentication.py index 5f9001ca10..4adbe2aa5a 100644 --- a/openedx/core/djangoapps/cors_csrf/tests/test_authentication.py +++ b/openedx/core/djangoapps/cors_csrf/tests/test_authentication.py @@ -1,6 +1,7 @@ """Tests for the CORS CSRF version of Django Rest Framework's SessionAuthentication.""" from mock import patch +from django.middleware.csrf import get_token from django.test import TestCase from django.test.utils import override_settings from django.test.client import RequestFactory @@ -11,16 +12,21 @@ from rest_framework.exceptions import PermissionDenied from ..authentication import SessionAuthenticationCrossDomainCsrf +# A class to pass into django.middleware.csrf.get_token() so we can easily get a valid CSRF token to use. +class FakeRequest(object): + META = {} + + class CrossDomainAuthTest(TestCase): """Tests for the CORS CSRF version of Django Rest Framework's SessionAuthentication. """ URL = "/dummy_url" REFERER = "https://www.edx.org" - CSRF_TOKEN = 'abcd1234' def setUp(self): super(CrossDomainAuthTest, self).setUp() self.auth = SessionAuthenticationCrossDomainCsrf() + self.csrf_token = get_token(FakeRequest()) def test_perform_csrf_referer_check(self): request = self._fake_request() @@ -45,12 +51,14 @@ class CrossDomainAuthTest(TestCase): def _fake_request(self): """Construct a fake request with a referer and CSRF token over a secure connection. """ factory = RequestFactory() - factory.cookies[settings.CSRF_COOKIE_NAME] = self.CSRF_TOKEN - + factory.cookies[settings.CSRF_COOKIE_NAME] = self.csrf_token request = factory.post( self.URL, HTTP_REFERER=self.REFERER, - HTTP_X_CSRFTOKEN=self.CSRF_TOKEN + HTTP_X_CSRFTOKEN=self.csrf_token ) request.is_secure = lambda: True + + # The way we're testing this skips django.middleware.csrf's process_request, which copies this from the cookie + request.META['CSRF_COOKIE'] = self.csrf_token return request diff --git a/openedx/core/djangoapps/cors_csrf/tests/test_decorators.py b/openedx/core/djangoapps/cors_csrf/tests/test_decorators.py index 8e2e1b86ab..1fa1daf6f0 100644 --- a/openedx/core/djangoapps/cors_csrf/tests/test_decorators.py +++ b/openedx/core/djangoapps/cors_csrf/tests/test_decorators.py @@ -18,6 +18,7 @@ class TestEnsureCsrfCookieCrossDomain(TestCase): def test_ensure_csrf_cookie_cross_domain(self): request = mock.Mock() request.META = {} + request.COOKIES = {} wrapped_view = ensure_csrf_cookie_cross_domain(fake_view) response = wrapped_view(request) response_meta = json.loads(response.content)