Merge pull request #1987 from edx/oleg/fix_mock_lti_protocols

Fixes and improvement to LTI.

common/lib/xmodule/xmodule/lti_module.py

@@ -486,23 +486,26 @@ oauth_consumer_key="", oauth_signature="frVp4JuvT1mVXlxktiAUjQ7%2F1cw%3D"'}
 
         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.'
+        except Exception as e:
+            error_message = "Request body XML parsing error: " + escape(e.message)
+            log.debug("[LTI]: " + error_message)
+            failure_values['imsx_description'] = error_message
             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):
+        except (ValueError, LTIError) as e:
             failure_values['imsx_messageIdentifier'] = escape(imsx_messageIdentifier)
-            failure_values['imsx_description'] = 'OAuth verification error.'
+            error_message = "OAuth verification error: " + escape(e.message)
+            failure_values['imsx_description'] = error_message
+            log.debug("[LTI]: " + error_message)
             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.'
+            failure_values['imsx_description'] = "User not found."
             return Response(response_xml_template.format(**failure_values), content_type="application/xml")
 
         if action == 'replaceResultRequest':
@@ -554,8 +557,7 @@ oauth_consumer_key="", oauth_signature="frVp4JuvT1mVXlxktiAUjQ7%2F1cw%3D"'}
         # 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
+            raise LTIError('score value outside the permitted range of 0-1.')
 
         return imsx_messageIdentifier, sourcedId, score, action
 
@@ -576,7 +578,6 @@ oauth_consumer_key="", oauth_signature="frVp4JuvT1mVXlxktiAUjQ7%2F1cw%3D"'}
         """
 
         client_key, client_secret = self.get_client_key_secret()
-
         headers = {
             'Authorization':unicode(request.headers.get('Authorization')),
             'Content-Type': 'application/x-www-form-urlencoded',
@@ -597,11 +598,9 @@ oauth_consumer_key="", oauth_signature="frVp4JuvT1mVXlxktiAUjQ7%2F1cw%3D"'}
             signature=oauth_signature
         )
         if oauth_body_hash != oauth_headers.get('oauth_body_hash'):
-            log.debug("[LTI]: OAuth body hash verification is failed.")
-            raise LTIError
+            raise LTIError("OAuth body hash verification is failed.")
         if not signature.verify_hmac_sha1(mock_request, client_secret):
-            log.debug("[LTI]: OAuth signature verification is failed.")
-            raise LTIError
+            raise LTIError("OAuth signature verification is failed.")
 
     def get_client_key_secret(self):
         """

common/lib/xmodule/xmodule/tests/test_lti_unit.py

@@ -99,7 +99,8 @@ class LTIModuleTest(LogicTest):
             'action': action
         }
 
-    def test_authorization_header_not_present(self):
+    @patch('xmodule.lti_module.LTIModule.get_client_key_secret', return_value=('test_client_key', u'test_client_secret'))
+    def test_authorization_header_not_present(self, get_key_secret):
         """
         Request has no Authorization header.
 
@@ -112,14 +113,15 @@ class LTIModuleTest(LogicTest):
         expected_response = {
             'action': None,
             'code_major': 'failure',
-            'description': 'OAuth verification error.',
+            'description': 'OAuth verification error: Malformed authorization header',
             'messageIdentifier': self.DEFAULTS['messageIdentifier'],
         }
 
         self.assertEqual(response.status_code, 200)
         self.assertDictEqual(expected_response, real_response)
 
-    def test_authorization_header_empty(self):
+    @patch('xmodule.lti_module.LTIModule.get_client_key_secret', return_value=('test_client_key', u'test_client_secret'))
+    def test_authorization_header_empty(self, get_key_secret):
         """
         Request Authorization header has no value.
 
@@ -133,7 +135,7 @@ class LTIModuleTest(LogicTest):
         expected_response = {
             'action': None,
             'code_major': 'failure',
-            'description': 'OAuth verification error.',
+            'description': 'OAuth verification error: Malformed authorization header',
             'messageIdentifier': self.DEFAULTS['messageIdentifier'],
         }
         self.assertEqual(response.status_code, 200)
@@ -171,7 +173,7 @@ class LTIModuleTest(LogicTest):
         expected_response = {
             'action': None,
             'code_major': 'failure',
-            'description': 'Request body XML parsing error.',
+            'description': 'Request body XML parsing error: score value outside the permitted range of 0-1.',
             'messageIdentifier': 'unknown',
         }
         self.assertEqual(response.status_code, 200)
@@ -189,7 +191,7 @@ class LTIModuleTest(LogicTest):
         expected_response = {
             'action': None,
             'code_major': 'failure',
-            'description': 'Request body XML parsing error.',
+            'description': 'Request body XML parsing error: invalid literal for float(): 0,5',
             'messageIdentifier': 'unknown',
         }
         self.assertEqual(response.status_code, 200)

lms/djangoapps/courseware/features/lti_setup.py

@@ -36,9 +36,8 @@ def setup_mock_lti_server():
         'lti_endpoint': 'correct_lti_endpoint'
     }
 
-    # Flag for acceptance tests used for creating right callback_url and sending
-    # graded result. Used in MockLTIRequestHandler.
-    server.test_mode = True
+    # For testing on localhost make callback url using referer host.
+    server.real_callback_url_on = False
 
     # Store the server instance in lettuce's world
     # so that other steps can access it

lms/djangoapps/courseware/mock_lti_server/mock_lti_server.py

@@ -61,7 +61,7 @@ class MockLTIRequestHandler(BaseHTTPRequestHandler):
         '''
         if 'grade' in self.path and self._send_graded_result().status_code == 200:
             status_message = 'LTI consumer (edX) responded with XML content:<br>' + self.server.grade_data['TC answer']
-            self.server.grade_data['callback_url'] = None
+            self.server.grade_data = None
             self._send_response(status_message, 200)
         # Respond to request with correct lti endpoint:
         elif self._is_correct_lti_request():
@@ -152,12 +152,14 @@ class MockLTIRequestHandler(BaseHTTPRequestHandler):
                 </imsx_POXEnvelopeRequest>
         """)
         data = payload.format(**values)
-        # get relative part, because host name is different in a) manual tests b) acceptance tests c) demos
-        if getattr(self.server, 'test_mode', None):
+        if getattr(self.server, 'use_real_callback_url', None):
+            # Use exact URL that was sent from TC when using this Stub LTI server
+            # as TP in real standalone environment.
+            url = self.server.grade_data['callback_url']
+        else:
+            # Use relative URL when using TP locally for manual testing or jenkins.
             relative_url = urlparse.urlparse(self.server.grade_data['callback_url']).path
             url = self.server.referer_host + relative_url
-        else:
-            url = self.server.grade_data['callback_url']
 
         headers = {'Content-Type': 'application/xml', 'X-Requested-With': 'XMLHttpRequest'}
         headers['Authorization'] = self.oauth_sign(url, data)
@@ -167,11 +169,12 @@ class MockLTIRequestHandler(BaseHTTPRequestHandler):
         if getattr(self.server, 'run_inside_unittest_flag', None):
             response = mock.Mock(status_code=200, url=url, data=data, headers=headers)
             return response
-
+        # Send request ignoring verification of SSL certificate
         response = requests.post(
             url,
             data=data,
-            headers=headers
+            headers=headers,
+            verify=False
         )
         self.server.grade_data['TC answer'] = response.content
         return response
@@ -182,6 +185,7 @@ class MockLTIRequestHandler(BaseHTTPRequestHandler):
         '''
         self._send_head(status_code)
         if getattr(self.server, 'grade_data', False):  # lti can be graded
+            url = "//{}:{}".format(self.server.server_host, self.server.server_port)
             response_str = textwrap.dedent("""
                 <html>
                     <head>
@@ -198,7 +202,7 @@ class MockLTIRequestHandler(BaseHTTPRequestHandler):
                         </form>
                     </body>
                 </html>
-            """).format(message, url="http://%s:%s" % self.server.server_address)
+            """).format(message, url=url)
         else: # lti can't be graded
             response_str = textwrap.dedent("""
                 <html>

lms/djangoapps/courseware/mock_lti_server/server_start.py

@@ -19,10 +19,10 @@ server.oauth_settings = {
     'lti_endpoint': 'correct_lti_endpoint'
 }
 server.server_host = server_host
+server.server_port = server_port
 
-# If in test mode mock lti server will make callback url using referer host.
-# Used in MockLTIRequestHandler when sending graded result.
-server.test_mode = True
+# For testing on localhost make callback url using referer host.
+server.use_real_callback_url = False
 
 try:
     server.serve_forever()

lms/djangoapps/courseware/mock_lti_server/test_mock_lti_server.py

@@ -36,6 +36,9 @@ class MockLTIServerTest(unittest.TestCase):
         #flag for creating right callback_url
         self.server.test_mode = True
 
+        self.server.server_host = server_host
+        self.server.server_port = server_port
+
         # Start the server in a separate daemon thread
         server_thread = threading.Thread(target=self.server.serve_forever)
         server_thread.daemon = True