Alex/lti fixes to release

CHANGELOG.rst

@@ -5,13 +5,16 @@ These are notable changes in edx-platform.  This is a rolling list of changes,
 in roughly chronological order, most recent first.  Add your entries at or near
 the top.  Include a label indicating the component affected.
 
+Blades: Make LTI module not send grade_back_url if has_score=False. BLD-561.
+Blades: LTI additional Python tests. LTI must use HTTPS for
+lis_outcome_service_url. BLD-564.
+
 Blades: Fix bug when Image mapping problems are not working for students in IE. BLD-413.
 
 Blades: Add template that displays the most up-to-date features of
 drag-and-drop. BLD-479.
 
-Blades: LTI additional Python tests. LTI fix bug e-reader error when popping
-out window. BLD-465.
+Blades: LTI fix bug e-reader error when popping out window. BLD-465.
 
 Common: Switch from mitx.db to edx.db for sqlite databases. This will effectively
   reset state for local instances of the code, unless you manually rename your

common/djangoapps/student/models.py

@@ -46,8 +46,8 @@ class AnonymousUserId(models.Model):
 
     Purpose of this table is to provide user by anonymous_user_id.
 
-    We are generating anonymous_user_id using md5 algorithm, so resulting length will always be 16 bytes.
-    http://docs.python.org/2/library/md5.html#md5.digest_size
+    We generate anonymous_user_id using md5 algorithm,
+    and use result in hex form, so its length is equal to 32 bytes.
     """
     user = models.ForeignKey(User, db_index=True)
     anonymous_user_id = models.CharField(unique=True, max_length=32)

common/lib/xmodule/xmodule/lti_module.py

@@ -259,37 +259,22 @@ def get_context(self):
             'element_class': self.category,
             'open_in_a_new_page': self.open_in_a_new_page,
             'display_name': self.display_name,
-            'form_url': self.get_form_path(),
+            'form_url': self.runtime.handler_url(self, 'preview_handler').rstrip('/?'),
         }
 
-
-    def get_form_path(self):
-        return  self.runtime.handler_url(self, 'preview_handler').rstrip('/?')
-
     def get_html(self):
         """
         Renders parameters to template.
         """
         return self.system.render_template('lti.html', self.get_context())
 
-    def get_form(self):
-        """
-        Renders parameters to form template.
-        """
-        return self.system.render_template('lti_form.html', self.get_context())
-
     @XBlock.handler
-    def preview_handler(self, request, dispatch):
+    def preview_handler(self, _, __):
         """
-        Ajax handler.
-
-        Args:
-            dispatch: string request slug
-
-        Returns:
-            json string
+        This is called to get context with new oauth params to iframe.
         """
-        return Response(self.get_form(), content_type='text/html')
+        template = self.system.render_template('lti_form.html', self.get_context())
+        return Response(template, content_type='text/html')
 
     def get_user_id(self):
         user_id = self.runtime.anonymous_student_id
@@ -299,11 +284,18 @@ def get_user_id(self):
     def get_outcome_service_url(self):
         """
         Return URL for storing grades.
+
+        To test LTI on sandbox we must use http scheme.
+
+        While testing locally and on Jenkins, mock_lti_server use http.referer
+        to obtain scheme, so it is ok to have http(s) anyway.
         """
-        uri = 'http://{host}{path}'.format(
-                host=self.system.hostname,
-                path=self.runtime.handler_url(self, 'grade_handler', thirdparty=True).rstrip('/?')
-            )
+        scheme = 'http' if 'sandbox' in self.system.hostname else 'https'
+        uri = '{scheme}://{host}{path}'.format(
+            scheme=scheme,
+            host=self.system.hostname,
+            path=self.runtime.handler_url(self, 'grade_handler', thirdparty=True).rstrip('/?')
+        )
         return uri
 
     def get_resource_link_id(self):
@@ -363,11 +355,15 @@ def oauth_params(self, custom_parameters, client_key, client_secret):
 
             # Parameters required for grading:
             u'resource_link_id': self.get_resource_link_id(),
-            u'lis_outcome_service_url': self.get_outcome_service_url(),
             u'lis_result_sourcedid': self.get_lis_result_sourcedid(),
 
         }
 
+        if self.has_score:
+            body.update({
+                u'lis_outcome_service_url': self.get_outcome_service_url()
+            })
+
         # Appending custom parameter for signing.
         body.update(custom_parameters)
 
@@ -449,7 +445,7 @@ def grade_handler(self, request, dispatch):
 
         Example of correct/incorrect answer XML body:: see response_xml_template.
         """
-        response_xml_template = textwrap.dedent("""
+        response_xml_template = textwrap.dedent("""\
             <?xml version="1.0" encoding="UTF-8"?>
             <imsx_POXEnvelopeResponse xmlns = "http://www.imsglobal.org/services/ltiv1p1/xsd/imsoms_v1p0">
                 <imsx_POXHeader>
@@ -491,17 +487,24 @@ def grade_handler(self, request, dispatch):
         try:
             imsx_messageIdentifier, sourcedId, score, action = self.parse_grade_xml_body(request.body)
         except Exception:
+            log.debug("[LTI]: Request body XML parsing error.")
+            failure_values['imsx_description'] = 'Request body XML parsing error.'
             return Response(response_xml_template.format(**failure_values), content_type="application/xml")
 
         # Verify OAuth signing.
         try:
             self.verify_oauth_body_sign(request)
         except (ValueError, LTIError):
             failure_values['imsx_messageIdentifier'] = escape(imsx_messageIdentifier)
+            failure_values['imsx_description'] = 'OAuth verification error.'
             return Response(response_xml_template.format(**failure_values), content_type="application/xml")
 
-
         real_user = self.system.get_real_user(urllib.unquote(sourcedId.split(':')[-1]))
+        if not real_user:  # that means we can't save to database, as we do not have real user id.
+            failure_values['imsx_messageIdentifier'] = escape(imsx_messageIdentifier)
+            failure_values['imsx_description'] = 'User not found.'
+            return Response(response_xml_template.format(**failure_values), content_type="application/xml")
+
         if action == 'replaceResultRequest':
             self.system.publish(
                 event={
@@ -518,9 +521,11 @@ def grade_handler(self, request, dispatch):
                 'imsx_messageIdentifier': escape(imsx_messageIdentifier),
                 'response': '<replaceResultResponse/>'
             }
+            log.debug("[LTI]: Grade is saved.")
             return Response(response_xml_template.format(**values), content_type="application/xml")
 
         unsupported_values['imsx_messageIdentifier'] = escape(imsx_messageIdentifier)
+        log.debug("[LTI]: Incorrect action.")
         return Response(response_xml_template.format(**unsupported_values), content_type='application/xml')
 
 
@@ -549,6 +554,7 @@ def parse_grade_xml_body(cls, body):
         # Raise exception if score is not float or not in range 0.0-1.0 regarding spec.
         score = float(score)
         if not 0 <= score <= 1:
+            log.debug("[LTI]: Score not in range.")
             raise LTIError
 
         return imsx_messageIdentifier, sourcedId, score, action
@@ -578,7 +584,7 @@ def verify_oauth_body_sign(self, request):
 
         sha1 = hashlib.sha1()
         sha1.update(request.body)
-        oauth_body_hash = base64.b64encode(sha1.hexdigest())
+        oauth_body_hash = base64.b64encode(sha1.digest())
 
         oauth_params = signature.collect_parameters(headers=headers, exclude_oauth_signature=False)
         oauth_headers =dict(oauth_params)
@@ -590,8 +596,11 @@ def verify_oauth_body_sign(self, request):
             params=oauth_headers.items(),
             signature=oauth_signature
         )
-        if (oauth_body_hash != oauth_headers.get('oauth_body_hash') or
-            not signature.verify_hmac_sha1(mock_request, client_secret)):
+        if oauth_body_hash != oauth_headers.get('oauth_body_hash'):
+            log.debug("[LTI]: OAuth body hash verification is failed.")
+            raise LTIError
+        if not signature.verify_hmac_sha1(mock_request, client_secret):
+            log.debug("[LTI]: OAuth signature verification is failed.")
             raise LTIError
 
     def get_client_key_secret(self):