diff --git a/common/lib/xmodule/xmodule/lti_module.py b/common/lib/xmodule/xmodule/lti_module.py index ff90d0525e..5257fd1247 100644 --- a/common/lib/xmodule/xmodule/lti_module.py +++ b/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): """ diff --git a/common/lib/xmodule/xmodule/tests/test_lti_unit.py b/common/lib/xmodule/xmodule/tests/test_lti_unit.py index 0c25bbafd2..718c738a8b 100644 --- a/common/lib/xmodule/xmodule/tests/test_lti_unit.py +++ b/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) diff --git a/lms/djangoapps/courseware/features/lti_setup.py b/lms/djangoapps/courseware/features/lti_setup.py index a201323ef9..51078102f6 100644 --- a/lms/djangoapps/courseware/features/lti_setup.py +++ b/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 diff --git a/lms/djangoapps/courseware/mock_lti_server/mock_lti_server.py b/lms/djangoapps/courseware/mock_lti_server/mock_lti_server.py index a788aed196..9b86592e16 100644 --- a/lms/djangoapps/courseware/mock_lti_server/mock_lti_server.py +++ b/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:
' + 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): """) 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(""" @@ -198,7 +202,7 @@ class MockLTIRequestHandler(BaseHTTPRequestHandler): - """).format(message, url="http://%s:%s" % self.server.server_address) + """).format(message, url=url) else: # lti can't be graded response_str = textwrap.dedent(""" diff --git a/lms/djangoapps/courseware/mock_lti_server/server_start.py b/lms/djangoapps/courseware/mock_lti_server/server_start.py index 77c384d46c..568d830472 100644 --- a/lms/djangoapps/courseware/mock_lti_server/server_start.py +++ b/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() diff --git a/lms/djangoapps/courseware/mock_lti_server/test_mock_lti_server.py b/lms/djangoapps/courseware/mock_lti_server/test_mock_lti_server.py index 3f6a7c57d2..d6f3c5df3c 100644 --- a/lms/djangoapps/courseware/mock_lti_server/test_mock_lti_server.py +++ b/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