Commit 7c070f29 by Oleg Marshev

Add better messaging, turn off SSL verification when sending grade back.

parent e24ddaa3
...@@ -486,23 +486,26 @@ oauth_consumer_key="", oauth_signature="frVp4JuvT1mVXlxktiAUjQ7%2F1cw%3D"'} ...@@ -486,23 +486,26 @@ oauth_consumer_key="", oauth_signature="frVp4JuvT1mVXlxktiAUjQ7%2F1cw%3D"'}
try: try:
imsx_messageIdentifier, sourcedId, score, action = self.parse_grade_xml_body(request.body) imsx_messageIdentifier, sourcedId, score, action = self.parse_grade_xml_body(request.body)
except Exception: except Exception as e:
log.debug("[LTI]: Request body XML parsing error.") error_message = "Request body XML parsing error: " + escape(e.message)
failure_values['imsx_description'] = 'Request body XML parsing error.' log.debug("[LTI]: " + error_message)
failure_values['imsx_description'] = error_message
return Response(response_xml_template.format(**failure_values), content_type="application/xml") return Response(response_xml_template.format(**failure_values), content_type="application/xml")
# Verify OAuth signing. # Verify OAuth signing.
try: try:
self.verify_oauth_body_sign(request) self.verify_oauth_body_sign(request)
except (ValueError, LTIError): except (ValueError, LTIError) as e:
failure_values['imsx_messageIdentifier'] = escape(imsx_messageIdentifier) 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") return Response(response_xml_template.format(**failure_values), content_type="application/xml")
real_user = self.system.get_real_user(urllib.unquote(sourcedId.split(':')[-1])) 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. 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_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") return Response(response_xml_template.format(**failure_values), content_type="application/xml")
if action == 'replaceResultRequest': if action == 'replaceResultRequest':
...@@ -554,8 +557,7 @@ oauth_consumer_key="", oauth_signature="frVp4JuvT1mVXlxktiAUjQ7%2F1cw%3D"'} ...@@ -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. # Raise exception if score is not float or not in range 0.0-1.0 regarding spec.
score = float(score) score = float(score)
if not 0 <= score <= 1: if not 0 <= score <= 1:
log.debug("[LTI]: Score not in range.") raise LTIError('score value outside the permitted range of 0-1.')
raise LTIError
return imsx_messageIdentifier, sourcedId, score, action return imsx_messageIdentifier, sourcedId, score, action
...@@ -576,7 +578,6 @@ oauth_consumer_key="", oauth_signature="frVp4JuvT1mVXlxktiAUjQ7%2F1cw%3D"'} ...@@ -576,7 +578,6 @@ oauth_consumer_key="", oauth_signature="frVp4JuvT1mVXlxktiAUjQ7%2F1cw%3D"'}
""" """
client_key, client_secret = self.get_client_key_secret() client_key, client_secret = self.get_client_key_secret()
headers = { headers = {
'Authorization':unicode(request.headers.get('Authorization')), 'Authorization':unicode(request.headers.get('Authorization')),
'Content-Type': 'application/x-www-form-urlencoded', 'Content-Type': 'application/x-www-form-urlencoded',
...@@ -597,11 +598,9 @@ oauth_consumer_key="", oauth_signature="frVp4JuvT1mVXlxktiAUjQ7%2F1cw%3D"'} ...@@ -597,11 +598,9 @@ oauth_consumer_key="", oauth_signature="frVp4JuvT1mVXlxktiAUjQ7%2F1cw%3D"'}
signature=oauth_signature signature=oauth_signature
) )
if oauth_body_hash != oauth_headers.get('oauth_body_hash'): if oauth_body_hash != oauth_headers.get('oauth_body_hash'):
log.debug("[LTI]: OAuth body hash verification is failed.") raise LTIError("OAuth body hash verification is failed.")
raise LTIError
if not signature.verify_hmac_sha1(mock_request, client_secret): if not signature.verify_hmac_sha1(mock_request, client_secret):
log.debug("[LTI]: OAuth signature verification is failed.") raise LTIError("OAuth signature verification is failed.")
raise LTIError
def get_client_key_secret(self): def get_client_key_secret(self):
""" """
......
...@@ -99,7 +99,8 @@ class LTIModuleTest(LogicTest): ...@@ -99,7 +99,8 @@ class LTIModuleTest(LogicTest):
'action': action '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. Request has no Authorization header.
...@@ -112,14 +113,15 @@ class LTIModuleTest(LogicTest): ...@@ -112,14 +113,15 @@ class LTIModuleTest(LogicTest):
expected_response = { expected_response = {
'action': None, 'action': None,
'code_major': 'failure', 'code_major': 'failure',
'description': 'OAuth verification error.', 'description': 'OAuth verification error: Malformed authorization header',
'messageIdentifier': self.DEFAULTS['messageIdentifier'], 'messageIdentifier': self.DEFAULTS['messageIdentifier'],
} }
self.assertEqual(response.status_code, 200) self.assertEqual(response.status_code, 200)
self.assertDictEqual(expected_response, real_response) 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. Request Authorization header has no value.
...@@ -133,7 +135,7 @@ class LTIModuleTest(LogicTest): ...@@ -133,7 +135,7 @@ class LTIModuleTest(LogicTest):
expected_response = { expected_response = {
'action': None, 'action': None,
'code_major': 'failure', 'code_major': 'failure',
'description': 'OAuth verification error.', 'description': 'OAuth verification error: Malformed authorization header',
'messageIdentifier': self.DEFAULTS['messageIdentifier'], 'messageIdentifier': self.DEFAULTS['messageIdentifier'],
} }
self.assertEqual(response.status_code, 200) self.assertEqual(response.status_code, 200)
...@@ -171,7 +173,7 @@ class LTIModuleTest(LogicTest): ...@@ -171,7 +173,7 @@ class LTIModuleTest(LogicTest):
expected_response = { expected_response = {
'action': None, 'action': None,
'code_major': 'failure', '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', 'messageIdentifier': 'unknown',
} }
self.assertEqual(response.status_code, 200) self.assertEqual(response.status_code, 200)
...@@ -189,7 +191,7 @@ class LTIModuleTest(LogicTest): ...@@ -189,7 +191,7 @@ class LTIModuleTest(LogicTest):
expected_response = { expected_response = {
'action': None, 'action': None,
'code_major': 'failure', 'code_major': 'failure',
'description': 'Request body XML parsing error.', 'description': 'Request body XML parsing error: invalid literal for float(): 0,5',
'messageIdentifier': 'unknown', 'messageIdentifier': 'unknown',
} }
self.assertEqual(response.status_code, 200) self.assertEqual(response.status_code, 200)
......
...@@ -36,9 +36,8 @@ def setup_mock_lti_server(): ...@@ -36,9 +36,8 @@ def setup_mock_lti_server():
'lti_endpoint': 'correct_lti_endpoint' 'lti_endpoint': 'correct_lti_endpoint'
} }
# Flag for acceptance tests used for creating right callback_url and sending # For testing on localhost make callback url using referer host.
# graded result. Used in MockLTIRequestHandler. server.real_callback_url_on = False
server.test_mode = True
# Store the server instance in lettuce's world # Store the server instance in lettuce's world
# so that other steps can access it # so that other steps can access it
......
...@@ -61,7 +61,7 @@ class MockLTIRequestHandler(BaseHTTPRequestHandler): ...@@ -61,7 +61,7 @@ class MockLTIRequestHandler(BaseHTTPRequestHandler):
''' '''
if 'grade' in self.path and self._send_graded_result().status_code == 200: 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'] 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) self._send_response(status_message, 200)
# Respond to request with correct lti endpoint: # Respond to request with correct lti endpoint:
elif self._is_correct_lti_request(): elif self._is_correct_lti_request():
...@@ -152,12 +152,14 @@ class MockLTIRequestHandler(BaseHTTPRequestHandler): ...@@ -152,12 +152,14 @@ class MockLTIRequestHandler(BaseHTTPRequestHandler):
</imsx_POXEnvelopeRequest> </imsx_POXEnvelopeRequest>
""") """)
data = payload.format(**values) 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, 'use_real_callback_url', None):
if getattr(self.server, 'test_mode', 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 relative_url = urlparse.urlparse(self.server.grade_data['callback_url']).path
url = self.server.referer_host + relative_url 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 = {'Content-Type': 'application/xml', 'X-Requested-With': 'XMLHttpRequest'}
headers['Authorization'] = self.oauth_sign(url, data) headers['Authorization'] = self.oauth_sign(url, data)
...@@ -167,11 +169,12 @@ class MockLTIRequestHandler(BaseHTTPRequestHandler): ...@@ -167,11 +169,12 @@ class MockLTIRequestHandler(BaseHTTPRequestHandler):
if getattr(self.server, 'run_inside_unittest_flag', None): if getattr(self.server, 'run_inside_unittest_flag', None):
response = mock.Mock(status_code=200, url=url, data=data, headers=headers) response = mock.Mock(status_code=200, url=url, data=data, headers=headers)
return response return response
# Send request ignoring verification of SSL certificate
response = requests.post( response = requests.post(
url, url,
data=data, data=data,
headers=headers headers=headers,
verify=False
) )
self.server.grade_data['TC answer'] = response.content self.server.grade_data['TC answer'] = response.content
return response return response
...@@ -182,6 +185,7 @@ class MockLTIRequestHandler(BaseHTTPRequestHandler): ...@@ -182,6 +185,7 @@ class MockLTIRequestHandler(BaseHTTPRequestHandler):
''' '''
self._send_head(status_code) self._send_head(status_code)
if getattr(self.server, 'grade_data', False): # lti can be graded 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(""" response_str = textwrap.dedent("""
<html> <html>
<head> <head>
...@@ -198,7 +202,7 @@ class MockLTIRequestHandler(BaseHTTPRequestHandler): ...@@ -198,7 +202,7 @@ class MockLTIRequestHandler(BaseHTTPRequestHandler):
</form> </form>
</body> </body>
</html> </html>
""").format(message, url="http://%s:%s" % self.server.server_address) """).format(message, url=url)
else: # lti can't be graded else: # lti can't be graded
response_str = textwrap.dedent(""" response_str = textwrap.dedent("""
<html> <html>
......
...@@ -19,10 +19,10 @@ server.oauth_settings = { ...@@ -19,10 +19,10 @@ server.oauth_settings = {
'lti_endpoint': 'correct_lti_endpoint' 'lti_endpoint': 'correct_lti_endpoint'
} }
server.server_host = server_host server.server_host = server_host
server.server_port = server_port
# If in test mode mock lti server will make callback url using referer host. # For testing on localhost make callback url using referer host.
# Used in MockLTIRequestHandler when sending graded result. server.use_real_callback_url = False
server.test_mode = True
try: try:
server.serve_forever() server.serve_forever()
......
...@@ -36,6 +36,9 @@ class MockLTIServerTest(unittest.TestCase): ...@@ -36,6 +36,9 @@ class MockLTIServerTest(unittest.TestCase):
#flag for creating right callback_url #flag for creating right callback_url
self.server.test_mode = True 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 # Start the server in a separate daemon thread
server_thread = threading.Thread(target=self.server.serve_forever) server_thread = threading.Thread(target=self.server.serve_forever)
server_thread.daemon = True server_thread.daemon = True
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment