From 4e66253c232b510683480ec51053cdc3fed1246c Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Mon, 31 Aug 2015 09:19:46 -0400 Subject: [PATCH 1/7] Upgrade uglify-js to 2.4.24 The latest version of uglify-js fixed a vulnerability which allows a specially crafted Javascript file to have altered functionality after minification. Changelog between the versions we are running can be found in the Readme file from this diff: https://github.com/mishoo/UglifyJS2/compare/v2.4.15...v2.4.24#diff-04c6e90faac2675aa89e2176d2eec7d8 We use the command line without any special arguments so I don't expect any issues. --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 6ff4a6c6bb..876b169897 100644 --- a/package.json +++ b/package.json @@ -3,7 +3,7 @@ "version": "0.1.0", "dependencies": { "coffee-script": "1.6.1", - "uglify-js": "2.4.15" + "uglify-js": "2.4.24" }, "devDependencies": { "jshint": "^2.7.0" From f099a7e9118611d4083605c24554db36d96bbc84 Mon Sep 17 00:00:00 2001 From: Adam Palay Date: Fri, 28 Aug 2015 12:26:00 -0400 Subject: [PATCH 2/7] fix issue where video player renders twice (TNL-3064) --- common/djangoapps/terrain/stubs/youtube.py | 5 ++-- .../xmodule/js/src/video/01_initialize.js | 6 +++- .../tests/video/test_video_module.py | 28 ++++++++++++++++++- 3 files changed, 34 insertions(+), 5 deletions(-) diff --git a/common/djangoapps/terrain/stubs/youtube.py b/common/djangoapps/terrain/stubs/youtube.py index 27f9d5ef2e..b6a00f8f87 100644 --- a/common/djangoapps/terrain/stubs/youtube.py +++ b/common/djangoapps/terrain/stubs/youtube.py @@ -92,12 +92,11 @@ class StubYouTubeHandler(StubHttpRequestHandler): self._send_video_response(youtube_id, "I'm youtube.") elif 'get_youtube_api' in self.path: + # Delay the response to simulate network latency + time.sleep(self.server.config.get('time_to_response', self.DEFAULT_DELAY_SEC)) if self.server.config.get('youtube_api_blocked'): self.send_response(404, content='', headers={'Content-type': 'text/plain'}) else: - # Delay the response to simulate network latency - time.sleep(self.server.config.get('time_to_response', self.DEFAULT_DELAY_SEC)) - # Get the response to send from YouTube. # We need to do this every time because Google sometimes sends different responses # as part of their own experiments, which has caused our tests to become "flaky" diff --git a/common/lib/xmodule/xmodule/js/src/video/01_initialize.js b/common/lib/xmodule/xmodule/js/src/video/01_initialize.js index cc43a23356..59b0133d24 100644 --- a/common/lib/xmodule/xmodule/js/src/video/01_initialize.js +++ b/common/lib/xmodule/xmodule/js/src/video/01_initialize.js @@ -596,7 +596,11 @@ function (VideoPlayer, i18n) { '[Video info]: YouTube returned an error for ' + 'video with id "' + self.id + '".' ); - self.loadHtmlPlayer(); + // If the video is already loaded in `_waitForYoutubeApi` by the + // time we get here, then we shouldn't load it again. + if (!self.htmlPlayerLoaded) { + self.loadHtmlPlayer(); + } }); window.Video.loadYouTubeIFrameAPI(scriptTag); diff --git a/common/test/acceptance/tests/video/test_video_module.py b/common/test/acceptance/tests/video/test_video_module.py index 77610440ad..6676c27d00 100644 --- a/common/test/acceptance/tests/video/test_video_module.py +++ b/common/test/acceptance/tests/video/test_video_module.py @@ -384,13 +384,36 @@ class YouTubeVideoTest(VideoBaseTest): self.assertTrue(self.video.is_video_rendered('html5')) - def test_video_with_youtube_blocked(self): + def test_video_with_youtube_blocked_with_default_response_time(self): + """ + Scenario: Video is rendered in HTML5 mode when the YouTube API is blocked + Given the YouTube API is blocked + And the course has a Video component in "Youtube_HTML5" mode + Then the video has rendered in "HTML5" mode + And only one video has rendered + """ + # configure youtube server + self.youtube_configuration.update({ + 'youtube_api_blocked': True, + }) + + self.metadata = self.metadata_for_mode('youtube_html5') + + self.navigate_to_video() + + self.assertTrue(self.video.is_video_rendered('html5')) + + # The video should only be loaded once + self.assertEqual(len(self.video.q(css='video')), 1) + + def test_video_with_youtube_blocked_delayed_response_time(self): """ Scenario: Video is rendered in HTML5 mode when the YouTube API is blocked Given the YouTube server response time is greater than 1.5 seconds And the YouTube API is blocked And the course has a Video component in "Youtube_HTML5" mode Then the video has rendered in "HTML5" mode + And only one video has rendered """ # configure youtube server self.youtube_configuration.update({ @@ -404,6 +427,9 @@ class YouTubeVideoTest(VideoBaseTest): self.assertTrue(self.video.is_video_rendered('html5')) + # The video should only be loaded once + self.assertEqual(len(self.video.q(css='video')), 1) + def test_html5_video_rendered_with_youtube_captions(self): """ Scenario: User should see Youtube captions for If there are no transcripts From 37cdb89a5e0d21e89516ce0b9ba540c09de0081e Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Fri, 28 Aug 2015 14:18:42 -0400 Subject: [PATCH 3/7] Allow LtiConsumer.instance_guid to be blank. The instructions say that this should be left blank until the initial launch, but Django Admin kicks that out because blanks are not allowed at the model level. --- lms/djangoapps/lti_provider/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/djangoapps/lti_provider/models.py b/lms/djangoapps/lti_provider/models.py index 61c024a107..dc5305bfe5 100644 --- a/lms/djangoapps/lti_provider/models.py +++ b/lms/djangoapps/lti_provider/models.py @@ -26,7 +26,7 @@ class LtiConsumer(models.Model): consumer_name = models.CharField(max_length=255, unique=True) consumer_key = models.CharField(max_length=32, unique=True, db_index=True) consumer_secret = models.CharField(max_length=32, unique=True) - instance_guid = models.CharField(max_length=255, null=True, unique=True) + instance_guid = models.CharField(max_length=255, blank=True, null=True, unique=True) @staticmethod def get_or_supplement(instance_guid, consumer_key): From 664544035a74a91dbfe3005d153b3c32acf22b27 Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Sat, 29 Aug 2015 20:22:27 -0400 Subject: [PATCH 4/7] Add explicit name for LTI provider outcome celery task. --- lms/djangoapps/lti_provider/tasks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/djangoapps/lti_provider/tasks.py b/lms/djangoapps/lti_provider/tasks.py index cf11489f5d..2db49f8c0d 100644 --- a/lms/djangoapps/lti_provider/tasks.py +++ b/lms/djangoapps/lti_provider/tasks.py @@ -44,7 +44,7 @@ def score_changed_handler(sender, **kwargs): # pylint: disable=unused-argument ) -@CELERY_APP.task +@CELERY_APP.task(name='lti_provider.tasks.send_outcome') def send_outcome(points_possible, points_earned, user_id, course_id, usage_id): """ Calculate the score for a given user in a problem and send it to the From f577a6d2781736ecc2c6bfe8fb7e1d80547ae54e Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Mon, 31 Aug 2015 15:22:37 -0400 Subject: [PATCH 5/7] Add body hash to LTI outcome message. This is necessary to properly implement the LTI outcome passback spec. It was not included previously because it was causing problems with Canvas, but Blackboard will not accept outcomes unless they are properly signed. The requests_oauthlib doesn't support the body hash spec out of the box, so BodyHashClient needed to be made. Fortunately, it's a pretty simple spec: https://oauth.googlecode.com/svn/spec/ext/body_hash/1.0/oauth-bodyhash.html --- lms/djangoapps/lti_provider/outcomes.py | 32 ++++++++++++- .../lti_provider/tests/test_outcomes.py | 47 ++++++++++++++++++- 2 files changed, 76 insertions(+), 3 deletions(-) diff --git a/lms/djangoapps/lti_provider/outcomes.py b/lms/djangoapps/lti_provider/outcomes.py index bcfd37a883..bc02bcb3ec 100644 --- a/lms/djangoapps/lti_provider/outcomes.py +++ b/lms/djangoapps/lti_provider/outcomes.py @@ -3,18 +3,39 @@ Helper functions for managing interactions with the LTI outcomes service defined in LTI v1.1. """ +from hashlib import sha1 +from base64 import b64encode import logging +import uuid + from lxml import etree from lxml.builder import ElementMaker +from oauthlib.oauth1 import Client +from oauthlib.common import to_unicode import requests import requests_oauthlib -import uuid from lti_provider.models import GradedAssignment, OutcomeService log = logging.getLogger("edx.lti_provider") +class BodyHashClient(Client): + """ + OAuth1 Client that adds body hash support (required by LTI). + + The default Client doesn't support body hashes, so we have to add it ourselves. + The spec: + https://oauth.googlecode.com/svn/spec/ext/body_hash/1.0/oauth-bodyhash.html + """ + def get_oauth_params(self, request): + """Override get_oauth_params to add the body hash.""" + params = super(BodyHashClient, self).get_oauth_params(request) + digest = b64encode(sha1(request.body.encode('UTF-8')).digest()) + params.append((u'oauth_body_hash', to_unicode(digest))) + return params + + def store_outcome_parameters(request_params, user, lti_consumer): """ Determine whether a set of LTI launch parameters contains information about @@ -112,7 +133,13 @@ def sign_and_send_replace_result(assignment, xml): # message. Testing with Canvas throws an error when this field is included. # This code may need to be revisited once we test with other LMS platforms, # and confirm whether there's a bug in Canvas. - oauth = requests_oauthlib.OAuth1(consumer_key, consumer_secret) + oauth = requests_oauthlib.OAuth1( + consumer_key, + consumer_secret, + signature_method='HMAC-SHA1', + client_class=BodyHashClient, + force_include_body=True + ) headers = {'content-type': 'application/xml'} response = requests.post( @@ -121,6 +148,7 @@ def sign_and_send_replace_result(assignment, xml): auth=oauth, headers=headers ) + return response diff --git a/lms/djangoapps/lti_provider/tests/test_outcomes.py b/lms/djangoapps/lti_provider/tests/test_outcomes.py index 981a56a704..4845e7ff3e 100644 --- a/lms/djangoapps/lti_provider/tests/test_outcomes.py +++ b/lms/djangoapps/lti_provider/tests/test_outcomes.py @@ -1,16 +1,20 @@ """ Tests for the LTI outcome service handlers, both in outcomes.py and in tasks.py """ +import unittest from django.test import TestCase from lxml import etree from mock import patch, MagicMock, ANY +import requests_oauthlib +import requests + +from opaque_keys.edx.locator import CourseLocator, BlockUsageLocator from student.tests.factories import UserFactory from lti_provider.models import GradedAssignment, LtiConsumer, OutcomeService import lti_provider.outcomes as outcomes import lti_provider.tasks as tasks -from opaque_keys.edx.locator import CourseLocator, BlockUsageLocator class StoreOutcomeParametersTest(TestCase): @@ -363,3 +367,44 @@ class XmlHandlingTest(TestCase): major_code='failure' ) self.assertFalse(outcomes.check_replace_result_response(response)) + + +class TestBodyHashClient(unittest.TestCase): + """ + Test our custom BodyHashClient + + This Client should do everything a normal oauthlib.oauth1.Client would do, + except it also adds oauth_body_hash to the Authorization headers. + """ + def test_simple_message(self): + oauth = requests_oauthlib.OAuth1( + '1000000000000000', # fake consumer key + '2000000000000000', # fake consumer secret + signature_method='HMAC-SHA1', + client_class=outcomes.BodyHashClient, + force_include_body=True + ) + headers = {'content-type': 'application/xml'} + req = requests.Request( + 'POST', + "http://example.edx.org/fake", + data="Hello world!", + auth=oauth, + headers=headers + ) + prepped_req = req.prepare() + + # Make sure that our body hash is now part of the test... + self.assertIn( + 'oauth_body_hash="00hq6RNueFa8QiEjhep5cJRHWAI%3D"', + prepped_req.headers['Authorization'] + ) + + # But make sure we haven't wiped out any of the other oauth values + # that we would expect to be in the Authorization header as well + expected_oauth_headers = [ + "oauth_nonce", "oauth_timestamp", "oauth_version", + "oauth_signature_method", "oauth_consumer_key", "oauth_signature", + ] + for oauth_header in expected_oauth_headers: + self.assertIn(oauth_header, prepped_req.headers['Authorization']) From 1c29763441a5bb343ddaeb8589b782a49ca9bb55 Mon Sep 17 00:00:00 2001 From: Chris Rodriguez Date: Thu, 3 Sep 2015 11:57:29 -0400 Subject: [PATCH 6/7] Overriding white color for messages on dashboard --- lms/static/sass/multicourse/_dashboard.scss | 1 + 1 file changed, 1 insertion(+) diff --git a/lms/static/sass/multicourse/_dashboard.scss b/lms/static/sass/multicourse/_dashboard.scss index 3066cc570c..f3f8c1bef4 100644 --- a/lms/static/sass/multicourse/_dashboard.scss +++ b/lms/static/sass/multicourse/_dashboard.scss @@ -574,6 +574,7 @@ padding: ($baseline/2) $baseline; background: $gray-l5; border: 1px solid $gray-l4; + color: $base-font-color; // Overrides the normal white color in this one case // STATE: shown &.is-shown { From dbfa6dafedd3b0f67341e73039d8ad16f04c7d68 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Thu, 3 Sep 2015 16:59:12 -0400 Subject: [PATCH 7/7] Resolve conflicts in merge to master --- lms/djangoapps/lti_provider/tasks.py | 7 +------ .../lti_provider/tests/test_outcomes.py | 20 ++----------------- 2 files changed, 3 insertions(+), 24 deletions(-) diff --git a/lms/djangoapps/lti_provider/tasks.py b/lms/djangoapps/lti_provider/tasks.py index 210422e22a..4b774950c9 100644 --- a/lms/djangoapps/lti_provider/tasks.py +++ b/lms/djangoapps/lti_provider/tasks.py @@ -53,12 +53,7 @@ def score_changed_handler(sender, **kwargs): # pylint: disable=unused-argument ) -<<<<<<< HEAD -@CELERY_APP.task(name='lti_provider.tasks.send_outcome') -def send_outcome(points_possible, points_earned, user_id, course_id, usage_id): -======= def increment_assignment_versions(course_key, usage_key, user_id): ->>>>>>> edx/master """ Update the version numbers for all assignments that are affected by a score change event. Returns a list of all affected assignments. @@ -77,7 +72,7 @@ def increment_assignment_versions(course_key, usage_key, user_id): return assignments -@CELERY_APP.task +@CELERY_APP.task(name='lti_provider.tasks.send_composite_outcome') def send_composite_outcome(user_id, course_id, assignment_id, version): """ Calculate and transmit the score for a composite module (such as a diff --git a/lms/djangoapps/lti_provider/tests/test_outcomes.py b/lms/djangoapps/lti_provider/tests/test_outcomes.py index 53582f7c42..e89e366d29 100644 --- a/lms/djangoapps/lti_provider/tests/test_outcomes.py +++ b/lms/djangoapps/lti_provider/tests/test_outcomes.py @@ -14,25 +14,10 @@ from student.tests.factories import UserFactory from lti_provider.models import GradedAssignment, LtiConsumer, OutcomeService import lti_provider.outcomes as outcomes -<<<<<<< HEAD -import lti_provider.tasks as tasks -======= -from opaque_keys.edx.locator import CourseLocator, BlockUsageLocator from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import ItemFactory, CourseFactory, check_mongo_calls -def create_score(earned, possible): - """ - Create a new mock Score object with specified earned and possible values - """ - score = MagicMock() - score.possible = possible - score.earned = earned - return score ->>>>>>> edx/master - - class StoreOutcomeParametersTest(TestCase): """ Tests for the store_outcome_parameters method in outcomes.py @@ -310,7 +295,6 @@ class XmlHandlingTest(TestCase): self.assertFalse(outcomes.check_replace_result_response(response)) -<<<<<<< HEAD class TestBodyHashClient(unittest.TestCase): """ Test our custom BodyHashClient @@ -350,7 +334,8 @@ class TestBodyHashClient(unittest.TestCase): ] for oauth_header in expected_oauth_headers: self.assertIn(oauth_header, prepped_req.headers['Authorization']) -======= + + class TestAssignmentsForProblem(ModuleStoreTestCase): """ Test cases for the assignments_for_problem method in outcomes.py @@ -471,4 +456,3 @@ class TestAssignmentsForProblem(ModuleStoreTestCase): self.assertEqual(assignments[1].lis_result_sourcedid, 'graded_unit2') self.assertEqual(assignments[0].outcome_service, self.outcome_service) self.assertEqual(assignments[1].outcome_service, other_outcome_service) ->>>>>>> edx/master