Fix lis_outcome_service_url sending.

CHANGELOG.rst

@@ -5,6 +5,8 @@ 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: Show answer for imageresponse. BLD-21.
 
 Blades: LTI additional Python tests. LTI must use HTTPS for

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

@@ -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': '<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')
 
 
@@ -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):

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)

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

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 = """<html><head><title>TEST TITLE</title></head>
             <body>This is LTI Provider.</body></html>"""
-
         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("""
             <?xml version = "1.0" encoding = "UTF-8"?>
                 <imsx_POXEnvelopeRequest  xmlns="http://www.imsglobal.org/services/ltiv1p1/xsd/imsoms_v1p0">
@@ -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 = """<html><head><title>TEST TITLE</title></head>
-                <body>
-                <div><h2>Graded IFrame loaded</h2> \
-                <h3>Server response is:</h3>\
-                <h3 class="result">{}</h3></div>
-                <form action="{url}/grade" method="post">
-                <input type="submit" name="submit-button" value="Submit">
-                </form>
-
-                </body></html>""".format(message, url="http://%s:%s" % self.server.server_address)
-        else:
-            response_str = """<html><head><title>TEST TITLE</title></head>
-                <body>
-                <div><h2>IFrame loaded</h2> \
-                <h3>Server response is:</h3>\
-                <h3 class="result">{}</h3></div>
-                </body></html>""".format(message)
-
-        # Log the response
-        logger.debug("LTI: sent response {}".format(response_str))
+        if getattr(self.server, 'grade_data', False):  # lti can be graded
+            response_str = textwrap.dedent("""
+                <html>
+                    <head>
+                        <title>TEST TITLE</title>
+                    </head>
+                    <body>
+                        <div>
+                            <h2>Graded IFrame loaded</h2>
+                            <h3>Server response is:</h3>
+                            <h3 class="result">{}</h3>
+                        </div>
+                        <form action="{url}/grade" method="post">
+                            <input type="submit" name="submit-button" value="Submit">
+                        </form>
+                    </body>
+                </html>
+            """).format(message, url="http://%s:%s" % self.server.server_address)
+        else: # lti can't be graded
+            response_str = textwrap.dedent("""
+                <html>
+                    <head>
+                        <title>TEST TITLE</title>
+                    </head>
+                    <body>
+                        <div>
+                            <h2>IFrame loaded</h2>
+                            <h3>Server response is:</h3>
+                            <h3 class="result">{}</h3>
+                        </div>
+                    </body>
+                </html>
+            """).format(message)
 
+        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'])

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)

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,