From c4bb2ac75768f9fa054a0fea50d19078ee5d680f Mon Sep 17 00:00:00 2001 From: polesye Date: Thu, 10 Oct 2013 10:35:32 +0300 Subject: [PATCH 1/5] Remove adding prefix to custom parameters. --- common/lib/xmodule/xmodule/lti_module.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/common/lib/xmodule/xmodule/lti_module.py b/common/lib/xmodule/xmodule/lti_module.py index cea54e6e3d..e39d9d0a51 100644 --- a/common/lib/xmodule/xmodule/lti_module.py +++ b/common/lib/xmodule/xmodule/lti_module.py @@ -152,7 +152,7 @@ class LTIModule(LTIFields, XModule): except ValueError: raise LTIError('Could not parse LTI passport: {0!r}. \ Should be "id:key:secret" string.'.format(lti_passport)) - if lti_id == self.lti_id: + if lti_id == self.lti_id.strip(): client_key, client_secret = key, secret break @@ -165,8 +165,7 @@ class LTIModule(LTIFields, XModule): raise LTIError('Could not parse custom parameter: {0!r}. \ Should be "x=y" string.'.format(custom_parameter)) - # LTI specs: 'custom_' should be prepended before each custom parameter - custom_parameters[u'custom_' + unicode(param_name)] = unicode(param_value) + custom_parameters[unicode(param_name.strip())] = unicode(param_value.strip()) input_fields = self.oauth_params( custom_parameters, @@ -177,7 +176,7 @@ class LTIModule(LTIFields, XModule): 'input_fields': input_fields, # these params do not participate in oauth signing - 'launch_url': self.launch_url, + 'launch_url': self.launch_url.strip(), 'element_id': self.location.html_id(), 'element_class': self.category, 'open_in_a_new_page': self.open_in_a_new_page, @@ -227,7 +226,7 @@ class LTIModule(LTIFields, XModule): try: __, headers, __ = client.sign( - unicode(self.launch_url), + unicode(self.launch_url.strip()), http_method=u'POST', body=body, headers=headers) From e62c047fdf550a77c2d2ec341b3b401cb00d9605 Mon Sep 17 00:00:00 2001 From: polesye Date: Thu, 10 Oct 2013 11:34:34 +0300 Subject: [PATCH 2/5] Fix a malformed custom_parameter bug. --- common/lib/xmodule/xmodule/lti_module.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/common/lib/xmodule/xmodule/lti_module.py b/common/lib/xmodule/xmodule/lti_module.py index e39d9d0a51..5b6d5fb675 100644 --- a/common/lib/xmodule/xmodule/lti_module.py +++ b/common/lib/xmodule/xmodule/lti_module.py @@ -150,8 +150,9 @@ class LTIModule(LTIFields, XModule): try: lti_id, key, secret = lti_passport.split(':') except ValueError: - raise LTIError('Could not parse LTI passport: {0!r}. \ - Should be "id:key:secret" string.'.format(lti_passport)) + lti_id = key = secret = '' + log.error('Could not parse LTI passport: {0!r}. \ +Should be "id:key:secret" string.'.format(lti_passport)) if lti_id == self.lti_id.strip(): client_key, client_secret = key, secret break @@ -162,8 +163,9 @@ class LTIModule(LTIFields, XModule): try: param_name, param_value = custom_parameter.split('=', 1) except ValueError: - raise LTIError('Could not parse custom parameter: {0!r}. \ - Should be "x=y" string.'.format(custom_parameter)) + param_name = param_value = '' + log.error('Could not parse custom parameter: {0!r}. \ +Should be "x=y" string.'.format(custom_parameter)) custom_parameters[unicode(param_name.strip())] = unicode(param_value.strip()) From a6a5300b9078c16505b37e0dfe3092d09b6ba489 Mon Sep 17 00:00:00 2001 From: polesye Date: Tue, 15 Oct 2013 10:00:08 +0300 Subject: [PATCH 3/5] Fix custom parameter. --- common/lib/xmodule/xmodule/lti_module.py | 50 ++++++++++++++++++++++-- 1 file changed, 47 insertions(+), 3 deletions(-) diff --git a/common/lib/xmodule/xmodule/lti_module.py b/common/lib/xmodule/xmodule/lti_module.py index 5b6d5fb675..34f9bab551 100644 --- a/common/lib/xmodule/xmodule/lti_module.py +++ b/common/lib/xmodule/xmodule/lti_module.py @@ -141,14 +141,55 @@ class LTIModule(LTIFields, XModule): Renders parameters to template. """ + # LTI provides a list of default parameters that might be passed as + # part of the POST data. These parameters should not be prefixed. + # Likewise, The creator of an LTI link can add custom key/value parameters + # to a launch which are to be included with the launch of the LTI link. + # In this case, these parameters should be prefixed by 'custom_'. + # See http://www.imsglobal.org/LTI/v1p1p1/ltiIMGv1p1p1.html#_Toc316828520 + PARAMETERS = [ + "lti_message_type", + "lti_version", + "resource_link_id", + "resource_link_title", + "resource_link_description", + "user_id", + "user_image", + "roles", + "lis_person_name_given", + "lis_person_name_family", + "lis_person_name_full", + "lis_person_contact_email_primary", + "lis_person_sourcedid", + "role_scope_mentor", + "context_id", + "context_type", + "context_title", + "context_label", + "launch_presentation_locale", + "launch_presentation_document_target", + "launch_presentation_css_url", + "launch_presentation_width", + "launch_presentation_height", + "launch_presentation_return_url", + "tool_consumer_info_product_family_code", + "tool_consumer_info_version", + "tool_consumer_instance_guid", + "tool_consumer_instance_name", + "tool_consumer_instance_description", + "tool_consumer_instance_url", + "tool_consumer_instance_contact_email", + ] + # Obtains client_key and client_secret credentials from current course: course_id = self.runtime.course_id course_location = CourseDescriptor.id_to_location(course_id) course = self.descriptor.runtime.modulestore.get_item(course_location) client_key = client_secret = '' + for lti_passport in course.lti_passports: try: - lti_id, key, secret = lti_passport.split(':') + lti_id, key, secret = [i.strip() for i in lti_passport.split(':')] except ValueError: lti_id = key = secret = '' log.error('Could not parse LTI passport: {0!r}. \ @@ -161,13 +202,16 @@ Should be "id:key:secret" string.'.format(lti_passport)) custom_parameters = {} for custom_parameter in self.custom_parameters: try: - param_name, param_value = custom_parameter.split('=', 1) + param_name, param_value = [p.strip() for p in custom_parameter.split('=', 1)] except ValueError: param_name = param_value = '' log.error('Could not parse custom parameter: {0!r}. \ Should be "x=y" string.'.format(custom_parameter)) - custom_parameters[unicode(param_name.strip())] = unicode(param_value.strip()) + if param_name not in PARAMETERS: + param_name = 'custom_' + param_name + + custom_parameters[unicode(param_name)] = unicode(param_value) input_fields = self.oauth_params( custom_parameters, From 547dc11c99faf18b8b53a8e130eeb50967ccda2b Mon Sep 17 00:00:00 2001 From: polesye Date: Wed, 16 Oct 2013 08:38:03 +0300 Subject: [PATCH 4/5] Revert "Fix a malformed custom_parameter bug." This reverts commit 75bdc1eebfd444bf00102b486a881f5776bbc407. --- common/lib/xmodule/xmodule/lti_module.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/common/lib/xmodule/xmodule/lti_module.py b/common/lib/xmodule/xmodule/lti_module.py index 34f9bab551..1d7ca1f024 100644 --- a/common/lib/xmodule/xmodule/lti_module.py +++ b/common/lib/xmodule/xmodule/lti_module.py @@ -191,9 +191,8 @@ class LTIModule(LTIFields, XModule): try: lti_id, key, secret = [i.strip() for i in lti_passport.split(':')] except ValueError: - lti_id = key = secret = '' - log.error('Could not parse LTI passport: {0!r}. \ -Should be "id:key:secret" string.'.format(lti_passport)) + raise LTIError('Could not parse LTI passport: {0!r}. \ + Should be "id:key:secret" string.'.format(lti_passport)) if lti_id == self.lti_id.strip(): client_key, client_secret = key, secret break @@ -204,9 +203,8 @@ Should be "id:key:secret" string.'.format(lti_passport)) try: param_name, param_value = [p.strip() for p in custom_parameter.split('=', 1)] except ValueError: - param_name = param_value = '' - log.error('Could not parse custom parameter: {0!r}. \ -Should be "x=y" string.'.format(custom_parameter)) + raise LTIError('Could not parse custom parameter: {0!r}. \ + Should be "x=y" string.'.format(custom_parameter)) if param_name not in PARAMETERS: param_name = 'custom_' + param_name From 1ee1eb694607cae60d10ab85c0df6561f15dc71c Mon Sep 17 00:00:00 2001 From: polesye Date: Fri, 18 Oct 2013 11:59:25 +0300 Subject: [PATCH 5/5] Fix comments. --- common/lib/xmodule/xmodule/lti_module.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/common/lib/xmodule/xmodule/lti_module.py b/common/lib/xmodule/xmodule/lti_module.py index 1d7ca1f024..a9eda48299 100644 --- a/common/lib/xmodule/xmodule/lti_module.py +++ b/common/lib/xmodule/xmodule/lti_module.py @@ -8,7 +8,6 @@ http://www.imsglobal.org/LTI/v1p1p1/ltiIMGv1p1p1.html import logging import oauthlib.oauth1 import urllib -import json from xmodule.editing_module import MetadataOnlyEditingDescriptor from xmodule.raw_module import EmptyDataRawDescriptor @@ -145,7 +144,7 @@ class LTIModule(LTIFields, XModule): # part of the POST data. These parameters should not be prefixed. # Likewise, The creator of an LTI link can add custom key/value parameters # to a launch which are to be included with the launch of the LTI link. - # In this case, these parameters should be prefixed by 'custom_'. + # In this case, we will automatically add `custom_` prefix before this parameters. # See http://www.imsglobal.org/LTI/v1p1p1/ltiIMGv1p1p1.html#_Toc316828520 PARAMETERS = [ "lti_message_type", @@ -206,6 +205,7 @@ class LTIModule(LTIFields, XModule): raise LTIError('Could not parse custom parameter: {0!r}. \ Should be "x=y" string.'.format(custom_parameter)) + # LTI specs: 'custom_' should be prepended before each custom parameter, as pointed in link above. if param_name not in PARAMETERS: param_name = 'custom_' + param_name