From d43ffd3a5683b55be901e4f5d0888ad3988531c9 Mon Sep 17 00:00:00 2001
From: Phil McGachey <phil_mcgachey@harvard.edu>
Date: Wed, 20 May 2015 08:44:58 -0400
Subject: [PATCH 1/4] [LTI Provider] Refactoring and clean-up

---
 .../0001_create_lti_consumer_model.py         | 37 ++++++++++++++++
 .../lti_provider/migrations/__init__.py       |  0
 lms/djangoapps/lti_provider/models.py         | 12 +++++-
 .../lti_provider/signature_validator.py       |  4 +-
 .../tests/test_signature_validator.py         |  4 +-
 .../lti_provider/tests/test_views.py          | 37 +++++++---------
 lms/djangoapps/lti_provider/views.py          | 43 ++++++++++++++-----
 7 files changed, 100 insertions(+), 37 deletions(-)
 create mode 100644 lms/djangoapps/lti_provider/migrations/0001_create_lti_consumer_model.py
 create mode 100644 lms/djangoapps/lti_provider/migrations/__init__.py

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
new file mode 100644
index 0000000000..0ad7ce6c6e
--- /dev/null
+++ b/lms/djangoapps/lti_provider/migrations/0001_create_lti_consumer_model.py
@@ -0,0 +1,37 @@
+# -*- coding: utf-8 -*-
+# pylint: disable=invalid-name, missing-docstring, unused-argument, unused-import, line-too-long
+from south.utils import datetime_utils as datetime
+from south.db import db
+from south.v2 import SchemaMigration
+from django.db import models
+
+
+class Migration(SchemaMigration):
+
+    def forwards(self, orm):
+        # Adding model 'LtiConsumer'
+        db.create_table('lti_provider_lticonsumer', (
+            ('id', self.gf('django.db.models.fields.AutoField')(primary_key=True)),
+            ('consumer_name', self.gf('django.db.models.fields.CharField')(max_length=255)),
+            ('consumer_key', self.gf('django.db.models.fields.CharField')(unique=True, max_length=32, db_index=True)),
+            ('consumer_secret', self.gf('django.db.models.fields.CharField')(unique=True, max_length=32)),
+        ))
+        db.send_create_signal('lti_provider', ['LtiConsumer'])
+
+
+    def backwards(self, orm):
+        # Deleting model 'LtiConsumer'
+        db.delete_table('lti_provider_lticonsumer')
+
+
+    models = {
+        'lti_provider.lticonsumer': {
+            'Meta': {'object_name': 'LtiConsumer'},
+            'consumer_key': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '32', 'db_index': 'True'}),
+            'consumer_name': ('django.db.models.fields.CharField', [], {'max_length': '255'}),
+            'consumer_secret': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '32'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'})
+        }
+    }
+
+    complete_apps = ['lti_provider']
\ No newline at end of file
diff --git a/lms/djangoapps/lti_provider/migrations/__init__.py b/lms/djangoapps/lti_provider/migrations/__init__.py
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/lms/djangoapps/lti_provider/models.py b/lms/djangoapps/lti_provider/models.py
index 8b0e929659..50eec8f254 100644
--- a/lms/djangoapps/lti_provider/models.py
+++ b/lms/djangoapps/lti_provider/models.py
@@ -1,5 +1,12 @@
 """
 Database models for the LTI provider feature.
+
+This app uses migrations. If you make changes to this model, be sure to create
+an appropriate migration file and check it in at the same time as your model
+changes. To do that,
+
+1. Go to the edx-platform dir
+2. ./manage.py lms schemamigration lti_provider --auto "description" --settings=devstack
 """
 from django.db import models
 from django.dispatch import receiver
@@ -13,8 +20,9 @@ class LtiConsumer(models.Model):
     specific settings, such as the OAuth key/secret pair and any LTI fields
     that must be persisted.
     """
-    key = models.CharField(max_length=32, unique=True, db_index=True)
-    secret = models.CharField(max_length=32, unique=True)
+    consumer_name = models.CharField(max_length=255)
+    consumer_key = models.CharField(max_length=32, unique=True, db_index=True)
+    consumer_secret = models.CharField(max_length=32, unique=True)
 
 
 @receiver(SCORE_CHANGED)
diff --git a/lms/djangoapps/lti_provider/signature_validator.py b/lms/djangoapps/lti_provider/signature_validator.py
index 7376666cb9..59207ea9e0 100644
--- a/lms/djangoapps/lti_provider/signature_validator.py
+++ b/lms/djangoapps/lti_provider/signature_validator.py
@@ -79,7 +79,7 @@ class SignatureValidator(RequestValidator):
 
         :return: True if the key is valid, False if it is not.
         """
-        return LtiConsumer.objects.filter(key=client_key).count() == 1
+        return LtiConsumer.objects.filter(consumer_key=client_key).count() == 1
 
     def get_client_secret(self, client_key, request):
         """
@@ -90,7 +90,7 @@ class SignatureValidator(RequestValidator):
         present, or None if the key does not exist in the database.
         """
         try:
-            return LtiConsumer.objects.get(key=client_key).secret
+            return LtiConsumer.objects.get(consumer_key=client_key).consumer_secret
         except ObjectDoesNotExist:
             return None
 
diff --git a/lms/djangoapps/lti_provider/tests/test_signature_validator.py b/lms/djangoapps/lti_provider/tests/test_signature_validator.py
index 9b19c6896f..4e0e9c03bf 100644
--- a/lms/djangoapps/lti_provider/tests/test_signature_validator.py
+++ b/lms/djangoapps/lti_provider/tests/test_signature_validator.py
@@ -78,7 +78,7 @@ class SignatureValidatorTest(TestCase):
         Verify that validate_client_key succeeds if the client key exists in the
         database
         """
-        LtiConsumer.objects.create(key='client_key', secret='client_secret')
+        LtiConsumer.objects.create(consumer_key='client_key', consumer_secret='client_secret')
         self.assertTrue(SignatureValidator().validate_client_key('client_key', None))
 
     def test_validate_missing_key(self):
@@ -93,7 +93,7 @@ class SignatureValidatorTest(TestCase):
         Verify that get_client_secret returns the right value if the key is in
         the database
         """
-        LtiConsumer.objects.create(key='client_key', secret='client_secret')
+        LtiConsumer.objects.create(consumer_key='client_key', consumer_secret='client_secret')
         secret = SignatureValidator().get_client_secret('client_key', None)
         self.assertEqual(secret, 'client_secret')
 
diff --git a/lms/djangoapps/lti_provider/tests/test_views.py b/lms/djangoapps/lti_provider/tests/test_views.py
index 10c622bd28..add8b13e14 100644
--- a/lms/djangoapps/lti_provider/tests/test_views.py
+++ b/lms/djangoapps/lti_provider/tests/test_views.py
@@ -8,6 +8,7 @@ from mock import patch, MagicMock
 
 from lti_provider import views
 from lti_provider.signature_validator import SignatureValidator
+from opaque_keys.edx.keys import CourseKey, UsageKey
 from student.tests.factories import UserFactory
 
 
@@ -22,10 +23,11 @@ LTI_DEFAULT_PARAMS = {
     'oauth_nonce': u'OAuth Nonce',
 }
 
-
+COURSE_KEY = CourseKey.from_string('some/course/id')
+USAGE_KEY = UsageKey.from_string('i4x://some/course/problem/uuid').map_into_course(COURSE_KEY)
 COURSE_PARAMS = {
-    'course_id': 'CourseID',
-    'usage_id': 'UsageID'
+    'course_key': COURSE_KEY,
+    'usage_key': USAGE_KEY
 }
 
 
@@ -72,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, COURSE_PARAMS['course_id'], COURSE_PARAMS['usage_id'])
+        views.lti_launch(request, str(COURSE_PARAMS['course_key']), str(COURSE_PARAMS['usage_key']))
         render.assert_called_with(request, ALL_PARAMS)
 
     def launch_with_missing_parameter(self, missing_param):
@@ -112,10 +114,10 @@ class LtiLaunchTest(TestCase):
         properly stored in the session
         """
         request = build_launch_request()
-        views.lti_launch(request, COURSE_PARAMS['course_id'], COURSE_PARAMS['usage_id'])
+        views.lti_launch(request, str(COURSE_PARAMS['course_key']), str(COURSE_PARAMS['usage_key']))
         session = request.session[views.LTI_SESSION_KEY]
-        self.assertEqual(session['course_id'], 'CourseID', 'Course ID not set in the session')
-        self.assertEqual(session['usage_id'], 'UsageID', 'Usage ID not set in the session')
+        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')
         for key in views.REQUIRED_PARAMETERS:
             self.assertEqual(session[key], request.POST[key], key + ' not set in the session')
 
@@ -126,7 +128,9 @@ class LtiLaunchTest(TestCase):
         URL
         """
         request = build_launch_request(False)
-        response = views.lti_launch(request, None, None)
+        response = views.lti_launch(
+            request, str(COURSE_PARAMS['course_key']), str(COURSE_PARAMS['usage_key'])
+        )
         self.assertEqual(response.status_code, 302)
         self.assertEqual(response['Location'], '/accounts/login?next=/lti_provider/lti_run')
 
@@ -170,7 +174,7 @@ class LtiRunTest(TestCase):
         Verifies that the lti_run view returns a Forbidden status if the session
         is missing any of the required LTI parameters or course information.
         """
-        extra_keys = ['course_id', 'usage_id']
+        extra_keys = ['course_key', 'usage_key']
         for key in views.REQUIRED_PARAMETERS + extra_keys:
             request = build_run_request()
             del request.session[views.LTI_SESSION_KEY][key]
@@ -208,7 +212,6 @@ class RenderCoursewareTest(TestCase):
         self.module_mock = self.setup_patch('lti_provider.views.get_module_by_usage_id', (self.module_instance, None))
         self.access_mock = self.setup_patch('lti_provider.views.has_access', 'StaffAccess')
         self.course_mock = self.setup_patch('lti_provider.views.get_course_with_access', 'CourseWithAccess')
-        self.key_mock = self.setup_patch('lti_provider.views.CourseKey.from_string', 'CourseKey')
 
     def setup_patch(self, function_name, return_value):
         """
@@ -228,21 +231,13 @@ class RenderCoursewareTest(TestCase):
         response = views.render_courseware(request, ALL_PARAMS.copy())
         self.assertEqual(response, 'Rendered page')
 
-    def test_course_key(self):
-        """
-        Verify that the correct course key is requested
-        """
-        request = build_run_request()
-        views.render_courseware(request, ALL_PARAMS.copy())
-        self.key_mock.assert_called_with(ALL_PARAMS['course_id'])
-
     def test_course_with_access(self):
         """
         Verify that get_course_with_access is called with the right parameters
         """
         request = build_run_request()
         views.render_courseware(request, ALL_PARAMS.copy())
-        self.course_mock.assert_called_with(request.user, 'load', 'CourseKey')
+        self.course_mock.assert_called_with(request.user, 'load', COURSE_KEY)
 
     def test_has_access(self):
         """
@@ -258,7 +253,7 @@ class RenderCoursewareTest(TestCase):
         """
         request = build_run_request()
         views.render_courseware(request, ALL_PARAMS.copy())
-        self.module_mock.assert_called_with(request, ALL_PARAMS['course_id'], ALL_PARAMS['usage_id'])
+        self.module_mock.assert_called_with(request, str(ALL_PARAMS['course_key']), str(ALL_PARAMS['usage_key']))
 
     def test_render(self):
         """
@@ -278,7 +273,7 @@ class RenderCoursewareTest(TestCase):
             'disable_footer': True,
             'disable_tabs': True,
             'staff_access': 'StaffAccess',
-            'xqa_server': 'http://your_xqa_server.com',
+            'xqa_server': 'http://example.com/xqa',
         }
         request = build_run_request()
         views.render_courseware(request, ALL_PARAMS.copy())
diff --git a/lms/djangoapps/lti_provider/views.py b/lms/djangoapps/lti_provider/views.py
index cf49289af7..bd7d73600c 100644
--- a/lms/djangoapps/lti_provider/views.py
+++ b/lms/djangoapps/lti_provider/views.py
@@ -6,7 +6,7 @@ from django.conf import settings
 from django.contrib.auth.decorators import login_required
 from django.contrib.auth.views import redirect_to_login
 from django.core.urlresolvers import reverse
-from django.http import HttpResponseBadRequest, HttpResponseForbidden
+from django.http import HttpResponseBadRequest, HttpResponseForbidden, Http404
 from django.views.decorators.csrf import csrf_exempt
 
 from courseware.access import has_access
@@ -14,7 +14,9 @@ from courseware.courses import get_course_with_access
 from courseware.module_render import get_module_by_usage_id
 from edxmako.shortcuts import render_to_response
 from lti_provider.signature_validator import SignatureValidator
-from opaque_keys.edx.keys import CourseKey
+from lms_xblock.runtime import unquote_slashes
+from opaque_keys.edx.keys import CourseKey, UsageKey
+from opaque_keys import InvalidKeyError
 
 # LTI launch parameters that must be present for a successful launch
 REQUIRED_PARAMETERS = [
@@ -64,8 +66,11 @@ 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.
-    params['course_id'] = course_id
-    params['usage_id'] = usage_id
+    course_key, usage_key = parse_course_and_usage_keys(course_id, usage_id)
+    if not course_key:
+        raise Http404('Invalid course or usage key')
+    params['course_key'] = course_key
+    params['usage_key'] = usage_key
     request.session[LTI_SESSION_KEY] = params
 
     if not request.user.is_authenticated():
@@ -140,7 +145,7 @@ def restore_params_from_session(request):
     if LTI_SESSION_KEY not in request.session:
         return None
     session_params = request.session[LTI_SESSION_KEY]
-    additional_params = ['course_id', 'usage_id']
+    additional_params = ['course_key', 'usage_key']
     return get_required_parameters(session_params, additional_params)
 
 
@@ -154,13 +159,12 @@ def render_courseware(request, lti_params):
     :return: an HttpResponse object that contains the template and necessary
     context to render the courseware.
     """
-    usage_id = lti_params['usage_id']
-    course_id = lti_params['course_id']
-    course_key = CourseKey.from_string(course_id)
+    usage_key = lti_params['usage_key']
+    course_key = lti_params['course_key']
     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, course_id, usage_id)
+    instance, _ = get_module_by_usage_id(request, str(course_key), str(usage_key))
 
     fragment = instance.render('student_view', context=request.GET)
 
@@ -173,7 +177,26 @@ def render_courseware(request, lti_params):
         'disable_footer': True,
         'disable_tabs': True,
         'staff_access': staff,
-        'xqa_server': settings.FEATURES.get('XQA_SERVER', 'http://your_xqa_server.com'),
+        'xqa_server': settings.FEATURES.get('XQA_SERVER', 'http://example.com/xqa'),
     }
 
     return render_to_response('courseware/courseware.html', context)
+
+
+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
+    return course_key, usage_key

From 577438d08536b90ab9b1a47b8b473db665d2d7ff Mon Sep 17 00:00:00 2001
From: Phil McGachey <phil_mcgachey@harvard.edu>
Date: Wed, 27 May 2015 12:22:34 -0400
Subject: [PATCH 2/4] Addressing review comments

---
 .../0001_create_lti_consumer_model.py         |  2 +-
 .../lti_provider/tests/test_views.py          | 11 ++++---
 lms/djangoapps/lti_provider/views.py          | 29 +++++++++----------
 3 files changed, 20 insertions(+), 22 deletions(-)

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

From 1ea4b8a026a84757e3ec2b865f3e3464d37d989e Mon Sep 17 00:00:00 2001
From: Phil McGachey <phil_mcgachey@harvard.edu>
Date: Wed, 27 May 2015 13:56:10 -0400
Subject: [PATCH 3/4] Logging invalid key errors

---
 lms/djangoapps/lti_provider/views.py | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/lms/djangoapps/lti_provider/views.py b/lms/djangoapps/lti_provider/views.py
index 2171bd8b58..5df3afa232 100644
--- a/lms/djangoapps/lti_provider/views.py
+++ b/lms/djangoapps/lti_provider/views.py
@@ -8,6 +8,7 @@ from django.contrib.auth.views import redirect_to_login
 from django.core.urlresolvers import reverse
 from django.http import HttpResponseBadRequest, HttpResponseForbidden, Http404
 from django.views.decorators.csrf import csrf_exempt
+import logging
 
 from courseware.access import has_access
 from courseware.courses import get_course_with_access
@@ -18,6 +19,9 @@ from lms_xblock.runtime import unquote_slashes
 from opaque_keys.edx.keys import CourseKey, UsageKey
 from opaque_keys import InvalidKeyError
 
+log = logging.getLogger("edx.lti_provider")
+
+
 # LTI launch parameters that must be present for a successful launch
 REQUIRED_PARAMETERS = [
     'roles', 'context_id', 'oauth_version', 'oauth_consumer_key',
@@ -69,9 +73,13 @@ def lti_launch(request, course_id, usage_id):
     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)
+        log.error(
+            'Invalid course key %s or usage key %s from request %s',
+            course_id,
+            usage_id,
+            request
         )
+        raise Http404()
     params['course_key'] = course_key
     params['usage_key'] = usage_key
     request.session[LTI_SESSION_KEY] = params

From a7b8b3f503ca9f2cb458e87a706c074ecbb492e5 Mon Sep 17 00:00:00 2001
From: Phil McGachey <phil_mcgachey@harvard.edu>
Date: Wed, 27 May 2015 15:37:30 -0400
Subject: [PATCH 4/4] [LTI Provider] Documentation fix

---
 lms/djangoapps/lti_provider/views.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lms/djangoapps/lti_provider/views.py b/lms/djangoapps/lti_provider/views.py
index 5df3afa232..8e0cb0925c 100644
--- a/lms/djangoapps/lti_provider/views.py
+++ b/lms/djangoapps/lti_provider/views.py
@@ -201,7 +201,8 @@ def render_courseware(request, lti_params):
 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.
+    (course_key, usage_key), or throw an InvalidKeyError if the translation
+    fails.
     """
     course_key = CourseKey.from_string(course_id)
     usage_id = unquote_slashes(usage_id)