diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 3297f426a0..cf288721fb 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -5,6 +5,7 @@ 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.
diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py
index 8921a078ba..935abbdd2e 100644
--- a/common/djangoapps/student/models.py
+++ b/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)
diff --git a/common/lib/xmodule/xmodule/lti_module.py b/common/lib/xmodule/xmodule/lti_module.py
index 45ec5b29bc..ff90d0525e 100644
--- a/common/lib/xmodule/xmodule/lti_module.py
+++ b/common/lib/xmodule/xmodule/lti_module.py
@@ -355,11 +355,15 @@ class LTIModule(LTIFields, XModule):
# 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)
@@ -483,6 +487,8 @@ 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.'
return Response(response_xml_template.format(**failure_values), content_type="application/xml")
# Verify OAuth signing.
@@ -490,10 +496,15 @@ oauth_consumer_key="", oauth_signature="frVp4JuvT1mVXlxktiAUjQ7%2F1cw%3D"'}
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={
@@ -510,9 +521,11 @@ oauth_consumer_key="", oauth_signature="frVp4JuvT1mVXlxktiAUjQ7%2F1cw%3D"'}
'imsx_messageIdentifier': escape(imsx_messageIdentifier),
'response': ''
}
+ 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')
@@ -541,6 +554,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
return imsx_messageIdentifier, sourcedId, score, action
@@ -582,8 +596,11 @@ oauth_consumer_key="", oauth_signature="frVp4JuvT1mVXlxktiAUjQ7%2F1cw%3D"'}
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):
diff --git a/common/lib/xmodule/xmodule/tests/test_lti_unit.py b/common/lib/xmodule/xmodule/tests/test_lti_unit.py
index 63a4e49f6c..0c25bbafd2 100644
--- a/common/lib/xmodule/xmodule/tests/test_lti_unit.py
+++ b/common/lib/xmodule/xmodule/tests/test_lti_unit.py
@@ -112,7 +112,7 @@ class LTIModuleTest(LogicTest):
expected_response = {
'action': None,
'code_major': 'failure',
- 'description': 'The request has failed.',
+ 'description': 'OAuth verification error.',
'messageIdentifier': self.DEFAULTS['messageIdentifier'],
}
@@ -133,7 +133,27 @@ class LTIModuleTest(LogicTest):
expected_response = {
'action': None,
'code_major': 'failure',
- 'description': 'The request has failed.',
+ 'description': 'OAuth verification error.',
+ 'messageIdentifier': self.DEFAULTS['messageIdentifier'],
+ }
+ self.assertEqual(response.status_code, 200)
+ self.assertDictEqual(expected_response, real_response)
+
+ def test_real_user_is_none(self):
+ """
+ If we have no real user, we should send back failure response.
+ """
+ self.xmodule.verify_oauth_body_sign = Mock()
+ self.xmodule.has_score = True
+ self.system.get_real_user = Mock(return_value=None)
+ request = Request(self.environ)
+ request.body = self.get_request_body()
+ response = self.xmodule.grade_handler(request, '')
+ real_response = self.get_response_values(response)
+ expected_response = {
+ 'action': None,
+ 'code_major': 'failure',
+ 'description': 'User not found.',
'messageIdentifier': self.DEFAULTS['messageIdentifier'],
}
self.assertEqual(response.status_code, 200)
@@ -151,7 +171,7 @@ class LTIModuleTest(LogicTest):
expected_response = {
'action': None,
'code_major': 'failure',
- 'description': 'The request has failed.',
+ 'description': 'Request body XML parsing error.',
'messageIdentifier': 'unknown',
}
self.assertEqual(response.status_code, 200)
@@ -169,7 +189,7 @@ class LTIModuleTest(LogicTest):
expected_response = {
'action': None,
'code_major': 'failure',
- 'description': 'The request has failed.',
+ 'description': 'Request body XML parsing error.',
'messageIdentifier': 'unknown',
}
self.assertEqual(response.status_code, 200)
diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py
index 9154952ace..8f8674b615 100644
--- a/common/lib/xmodule/xmodule/x_module.py
+++ b/common/lib/xmodule/xmodule/x_module.py
@@ -1016,6 +1016,9 @@ class ModuleSystem(ConfigurableFragmentWrapper, Runtime): # pylint: disable=abs
error_descriptor_class - The class to use to render XModules with errors
+ get_real_user - function that takes `anonymous_student_id` and returns real user_id,
+ associated with `anonymous_student_id`.
+
"""
# Right now, usage_store is unused, and field_data is always supplanted
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 9d69ec4152..a788aed196 100644
--- a/lms/djangoapps/courseware/mock_lti_server/mock_lti_server.py
+++ b/lms/djangoapps/courseware/mock_lti_server/mock_lti_server.py
@@ -33,7 +33,6 @@ class MockLTIRequestHandler(BaseHTTPRequestHandler):
protocol = "HTTP/1.0"
callback_url = None
-
def log_message(self, format, *args):
"""Log an arbitrary message."""
# Code copied from BaseHTTPServer.py. Changed to write to sys.stdout
@@ -49,18 +48,13 @@ class MockLTIRequestHandler(BaseHTTPRequestHandler):
Used for checking LTI Provider started correctly.
'''
-
self.send_response(200, 'OK')
self.send_header('Content-type', 'html')
self.end_headers()
-
response_str = """TEST TITLE
This is LTI Provider."""
-
self.wfile.write(response_str)
-
-
def do_POST(self):
'''
Handle a POST request from the client and sends response back.
@@ -72,38 +66,17 @@ class MockLTIRequestHandler(BaseHTTPRequestHandler):
# Respond to request with correct lti endpoint:
elif self._is_correct_lti_request():
self.post_dict = self._post_dict()
- correct_keys = [
- 'user_id',
- 'role',
- 'oauth_nonce',
- 'oauth_timestamp',
- 'oauth_consumer_key',
- 'lti_version',
- 'oauth_signature_method',
- 'oauth_version',
- 'oauth_signature',
- 'lti_message_type',
- 'oauth_callback',
- 'lis_outcome_service_url',
- 'lis_result_sourcedid',
- 'launch_presentation_return_url',
- # 'lis_person_sourcedid', optional, not used now.
- 'resource_link_id',
- ]
- if sorted(correct_keys) != sorted(self.post_dict.keys()):
- status_message = "Incorrect LTI header"
+ params = {k: v for k, v in self.post_dict.items() if k != 'oauth_signature'}
+ if self.server.check_oauth_signature(params, self.post_dict.get('oauth_signature', "")):
+ status_message = "This is LTI tool. Success."
+ # set data for grades what need to be stored as server data
+ if 'lis_outcome_service_url' in self.post_dict:
+ self.server.grade_data = {
+ 'callback_url': self.post_dict.get('lis_outcome_service_url'),
+ 'sourcedId': self.post_dict.get('lis_result_sourcedid')
+ }
else:
- params = {k: v for k, v in self.post_dict.items() if k != 'oauth_signature'}
- if self.server.check_oauth_signature(params, self.post_dict['oauth_signature']):
- status_message = "This is LTI tool. Success."
- else:
- status_message = "Wrong LTI signature"
- # set data for grades
- # what need to be stored as server data
- self.server.grade_data = {
- 'callback_url': self.post_dict.get('lis_outcome_service_url'),
- 'sourcedId': self.post_dict.get('lis_result_sourcedid')
- }
+ status_message = "Wrong LTI signature"
self._send_response(status_message, 200)
else:
status_message = "Invalid request URL"
@@ -141,17 +114,17 @@ class MockLTIRequestHandler(BaseHTTPRequestHandler):
self.server.cookie = {}
referer = urlparse.urlparse(self.headers.getheader('referer'))
self.server.referer_host = "{}://{}".format(referer.scheme, referer.netloc)
- self.server.referer_netloc = referer.netloc
return post_dict
def _send_graded_result(self):
-
+ """
+ Send grade request.
+ """
values = {
'textString': 0.5,
'sourcedId': self.server.grade_data['sourcedId'],
'imsx_messageIdentifier': uuid4().hex,
}
-
payload = textwrap.dedent("""
@@ -208,40 +181,53 @@ class MockLTIRequestHandler(BaseHTTPRequestHandler):
Send message back to the client
'''
self._send_head(status_code)
- if self.server.grade_data['callback_url']:
- response_str = """TEST TITLE
-
- Graded IFrame loaded
\
- Server response is:
\
- {}
-
+ if getattr(self.server, 'grade_data', False): # lti can be graded
+ response_str = textwrap.dedent("""
+
+
+ TEST TITLE
+
+
+
+
Graded IFrame loaded
+ Server response is:
+ {}
+
+
+
+
+ """).format(message, url="http://%s:%s" % self.server.server_address)
+ else: # lti can't be graded
+ response_str = textwrap.dedent("""
+
+
+ TEST TITLE
+
+
+
+
IFrame loaded
+ Server response is:
+ {}
+
+
+
+ """).format(message)
- """.format(message, url="http://%s:%s" % self.server.server_address)
- else:
- response_str = """TEST TITLE
-
- IFrame loaded
\
- Server response is:
\
- {}
- """.format(message)
-
- # Log the response
logger.debug("LTI: sent response {}".format(response_str))
-
self.wfile.write(response_str)
def _is_correct_lti_request(self):
- '''If url to LTI tool is correct.'''
+ '''
+ If url to LTI tool is correct.
+ '''
return self.server.oauth_settings['lti_endpoint'] in self.path
def oauth_sign(self, url, body):
"""
Signs request and returns signed body and headers.
-
"""
-
client = oauthlib.oauth1.Client(
client_key=unicode(self.server.oauth_settings['client_key']),
client_secret=unicode(self.server.oauth_settings['client_secret'])
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 282cbcc039..3f6a7c57d2 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
@@ -49,10 +49,9 @@ class MockLTIServerTest(unittest.TestCase):
def test_wrong_header(self):
"""
- Tests that LTI server processes request with right program
- path and responses with wrong header.
+ Tests that LTI server processes request with right program path but with wrong header.
"""
- #wrong number of params
+ #wrong number of params and no signature
payload = {
'user_id': 'default_user_id',
'role': 'student',
@@ -62,7 +61,7 @@ class MockLTIServerTest(unittest.TestCase):
uri = self.server.oauth_settings['lti_base'] + self.server.oauth_settings['lti_endpoint']
headers = {'referer': 'http://localhost:8000/'}
response = requests.post(uri, data=payload, headers=headers)
- self.assertIn('Incorrect LTI header', response.content)
+ self.assertIn('Wrong LTI signature', response.content)
def test_wrong_signature(self):
"""
@@ -74,7 +73,7 @@ class MockLTIServerTest(unittest.TestCase):
'role': 'student',
'oauth_nonce': '',
'oauth_timestamp': '',
- 'oauth_consumer_key': 'client_key',
+ 'oauth_consumer_key': 'test_client_key',
'lti_version': 'LTI-1p0',
'oauth_signature_method': 'HMAC-SHA1',
'oauth_version': '1.0',
@@ -101,7 +100,7 @@ class MockLTIServerTest(unittest.TestCase):
'role': 'student',
'oauth_nonce': '',
'oauth_timestamp': '',
- 'oauth_consumer_key': 'client_key',
+ 'oauth_consumer_key': 'test_client_key',
'lti_version': 'LTI-1p0',
'oauth_signature_method': 'HMAC-SHA1',
'oauth_version': '1.0',
@@ -112,7 +111,6 @@ class MockLTIServerTest(unittest.TestCase):
'lis_outcome_service_url': '',
'lis_result_sourcedid': '',
'resource_link_id':'',
- "lis_outcome_service_url": '',
}
self.server.check_oauth_signature = Mock(return_value=True)
@@ -128,7 +126,7 @@ class MockLTIServerTest(unittest.TestCase):
'role': 'student',
'oauth_nonce': '',
'oauth_timestamp': '',
- 'oauth_consumer_key': 'client_key',
+ 'oauth_consumer_key': 'test_client_key',
'lti_version': 'LTI-1p0',
'oauth_signature_method': 'HMAC-SHA1',
'oauth_version': '1.0',
@@ -139,7 +137,6 @@ class MockLTIServerTest(unittest.TestCase):
'lis_outcome_service_url': '',
'lis_result_sourcedid': '',
'resource_link_id':'',
- "lis_outcome_service_url": '',
}
self.server.check_oauth_signature = Mock(return_value=True)
@@ -147,8 +144,8 @@ class MockLTIServerTest(unittest.TestCase):
#this is the uri for sending grade from lti
headers = {'referer': 'http://localhost:8000/'}
response = requests.post(uri, data=payload, headers=headers)
+ self.assertIn('This is LTI tool. Success.', response.content)
- self.assertTrue('This is LTI tool. Success.' in response.content)
self.server.grade_data['TC answer'] = "Test response"
graded_response = requests.post('http://127.0.0.1:8034/grade')
self.assertIn('Test response', graded_response.content)
diff --git a/lms/djangoapps/courseware/tests/test_lti_integration.py b/lms/djangoapps/courseware/tests/test_lti_integration.py
index d792285f6e..178cef57f1 100644
--- a/lms/djangoapps/courseware/tests/test_lti_integration.py
+++ b/lms/djangoapps/courseware/tests/test_lti_integration.py
@@ -46,7 +46,6 @@ class TestLTI(BaseTestXmodule):
u'role': u'student',
u'resource_link_id': module_id,
- u'lis_outcome_service_url': lis_outcome_service_url,
u'lis_result_sourcedid': sourcedId,
u'oauth_nonce': mocked_nonce,