diff --git a/lms/djangoapps/lti_provider/migrations/0001_create_lti_consumer_model.py b/lms/djangoapps/lti_provider/migrations/0001_create_lti_consumer_model.py index 0ad7ce6c6e..c3d313afaa 100644 --- a/lms/djangoapps/lti_provider/migrations/0001_create_lti_consumer_model.py +++ b/lms/djangoapps/lti_provider/migrations/0001_create_lti_consumer_model.py @@ -34,4 +34,4 @@ class Migration(SchemaMigration): } } - complete_apps = ['lti_provider'] \ No newline at end of file + complete_apps = ['lti_provider'] diff --git a/lms/djangoapps/lti_provider/tests/test_views.py b/lms/djangoapps/lti_provider/tests/test_views.py index add8b13e14..2127807897 100644 --- a/lms/djangoapps/lti_provider/tests/test_views.py +++ b/lms/djangoapps/lti_provider/tests/test_views.py @@ -74,7 +74,7 @@ class LtiLaunchTest(TestCase): Verifies that the LTI launch succeeds when passed a valid request. """ request = build_launch_request() - views.lti_launch(request, str(COURSE_PARAMS['course_key']), str(COURSE_PARAMS['usage_key'])) + views.lti_launch(request, unicode(COURSE_KEY), unicode(USAGE_KEY)) render.assert_called_with(request, ALL_PARAMS) def launch_with_missing_parameter(self, missing_param): @@ -114,7 +114,7 @@ class LtiLaunchTest(TestCase): properly stored in the session """ request = build_launch_request() - views.lti_launch(request, str(COURSE_PARAMS['course_key']), str(COURSE_PARAMS['usage_key'])) + views.lti_launch(request, unicode(COURSE_KEY), unicode(USAGE_KEY)) session = request.session[views.LTI_SESSION_KEY] self.assertEqual(session['course_key'], COURSE_KEY, 'Course key not set in the session') self.assertEqual(session['usage_key'], USAGE_KEY, 'Usage key not set in the session') @@ -128,9 +128,7 @@ class LtiLaunchTest(TestCase): URL """ request = build_launch_request(False) - response = views.lti_launch( - request, str(COURSE_PARAMS['course_key']), str(COURSE_PARAMS['usage_key']) - ) + response = views.lti_launch(request, unicode(COURSE_KEY), unicode(USAGE_KEY)) self.assertEqual(response.status_code, 302) self.assertEqual(response['Location'], '/accounts/login?next=/lti_provider/lti_run') @@ -143,6 +141,7 @@ class LtiLaunchTest(TestCase): request = build_launch_request() response = views.lti_launch(request, None, None) self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, 403) class LtiRunTest(TestCase): @@ -253,7 +252,7 @@ class RenderCoursewareTest(TestCase): """ request = build_run_request() views.render_courseware(request, ALL_PARAMS.copy()) - self.module_mock.assert_called_with(request, str(ALL_PARAMS['course_key']), str(ALL_PARAMS['usage_key'])) + self.module_mock.assert_called_with(request, unicode(COURSE_KEY), unicode(USAGE_KEY)) def test_render(self): """ diff --git a/lms/djangoapps/lti_provider/views.py b/lms/djangoapps/lti_provider/views.py index bd7d73600c..2171bd8b58 100644 --- a/lms/djangoapps/lti_provider/views.py +++ b/lms/djangoapps/lti_provider/views.py @@ -66,9 +66,12 @@ def lti_launch(request, course_id, usage_id): # Store the course, and usage ID in the session to prevent privilege # escalation if a staff member in one course tries to access material in # another. - course_key, usage_key = parse_course_and_usage_keys(course_id, usage_id) - if not course_key: - raise Http404('Invalid course or usage key') + try: + course_key, usage_key = parse_course_and_usage_keys(course_id, usage_id) + except InvalidKeyError: + raise Http404( + 'Invalid course key {} or usage key {}'.format(course_id, usage_id) + ) params['course_key'] = course_key params['usage_key'] = usage_key request.session[LTI_SESSION_KEY] = params @@ -164,7 +167,11 @@ def render_courseware(request, lti_params): user = request.user course = get_course_with_access(user, 'load', course_key) staff = has_access(request.user, 'staff', course) - instance, _ = get_module_by_usage_id(request, str(course_key), str(usage_key)) + instance, _dummy = get_module_by_usage_id( + request, + unicode(course_key), + unicode(usage_key) + ) fragment = instance.render('student_view', context=request.GET) @@ -188,15 +195,7 @@ def parse_course_and_usage_keys(course_id, usage_id): Convert course and usage ID strings into key objects. Return a tuple of (course_key, usage_key), or (None, None) if the translation fails. """ - try: - course_key = CourseKey.from_string(course_id) - except InvalidKeyError: - return None, None - if not course_key: - return None, None - try: - usage_id = unquote_slashes(usage_id) - usage_key = UsageKey.from_string(usage_id).map_into_course(course_key) - except InvalidKeyError: - return None, None + course_key = CourseKey.from_string(course_id) + usage_id = unquote_slashes(usage_id) + usage_key = UsageKey.from_string(usage_id).map_into_course(course_key) return course_key, usage_key