Merge pull request #6266 from mitocw/fix_lti_callback

OAuth signature should be verified against URL provided to the tool.  

common/lib/xmodule/xmodule/lti_module.py

@@ -809,18 +809,39 @@ oauth_consumer_key="", oauth_signature="frVp4JuvT1mVXlxktiAUjQ7%2F1cw%3D"'}
         oauth_params = signature.collect_parameters(headers=headers, exclude_oauth_signature=False)
         oauth_headers = dict(oauth_params)
         oauth_signature = oauth_headers.pop('oauth_signature')
-        mock_request = mock.Mock(
+        mock_request_lti_1 = mock.Mock(
+            uri=unicode(urllib.unquote(self.get_outcome_service_url())),
+            http_method=unicode(request.method),
+            params=oauth_headers.items(),
+            signature=oauth_signature
+        )
+        mock_request_lti_2 = mock.Mock(
             uri=unicode(urllib.unquote(request.url)),
             http_method=unicode(request.method),
             params=oauth_headers.items(),
             signature=oauth_signature
         )
-
         if oauth_body_hash != oauth_headers.get('oauth_body_hash'):
+            log.error(
+                "OAuth body hash verification failed, provided: {}, "
+                "calculated: {}, for url: {}, body is: {}".format(
+                    oauth_headers.get('oauth_body_hash'),
+                    oauth_body_hash,
+                    self.get_outcome_service_url(),
+                    request.body
+                )
+            )
             raise LTIError("OAuth body hash verification is failed.")
 
-        if not signature.verify_hmac_sha1(mock_request, client_secret):
-            raise LTIError("OAuth signature verification is failed.")
+        if (not signature.verify_hmac_sha1(mock_request_lti_1, client_secret) and not
+                signature.verify_hmac_sha1(mock_request_lti_2, client_secret)):
+            log.error("OAuth signature verification failed, for "
+                      "headers:{} url:{} method:{}".format(
+                          oauth_headers,
+                          self.get_outcome_service_url(),
+                          unicode(request.method)
+                      ))
+            raise LTIError("OAuth signature verification has failed.")
 
     def get_client_key_secret(self):
         """

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

@@ -296,13 +296,13 @@ class LTIModuleTest(LogicTest):
         There are key and secret but not for specific LTI.
         """
 
-        #this adds lti passports to system
+        # this adds lti passports to system
         mocked_course = Mock(lti_passports=['test_id:test_client:test_secret'])
         modulestore = Mock()
         modulestore.get_course.return_value = mocked_course
         runtime = Mock(modulestore=modulestore)
         self.xmodule.descriptor.runtime = runtime
-        #set another lti_id
+        # set another lti_id
         self.xmodule.lti_id = "another_lti_id"
         key_secret = self.xmodule.get_client_key_secret()
         expected = ('', '')
@@ -314,7 +314,7 @@ class LTIModuleTest(LogicTest):
 
         There are key and secret provided in wrong format.
         """
-        #this adds lti passports to system
+        # this adds lti passports to system
         mocked_course = Mock(lti_passports=['test_id_test_client_test_secret'])
         modulestore = Mock()
         modulestore.get_course.return_value = mocked_course
@@ -330,10 +330,54 @@ class LTIModuleTest(LogicTest):
         """
         Test if OAuth signing was successful.
         """
-        try:
-            self.xmodule.verify_oauth_body_sign(self.get_signed_grade_mock_request())
-        except LTIError as err:
-            self.fail("verify_oauth_body_sign() raised LTIError: " + err.message)
+        self.xmodule.verify_oauth_body_sign(self.get_signed_grade_mock_request())
+
+    @patch('xmodule.lti_module.LTIModule.get_outcome_service_url', Mock(return_value=u'https://testurl/'))
+    @patch('xmodule.lti_module.LTIModule.get_client_key_secret',
+           Mock(return_value=(u'__consumer_key__', u'__lti_secret__')))
+    def test_failed_verify_oauth_body_sign_proxy_mangle_url(self):
+        """
+        Oauth signing verify fail.
+        """
+        request = self.get_signed_grade_mock_request_with_correct_signature()
+        self.xmodule.verify_oauth_body_sign(request)
+        # we should verify against get_outcome_service_url not
+        # request url proxy and load balancer along the way may
+        # change url presented to the method
+        request.url = 'http://testurl/'
+        self.xmodule.verify_oauth_body_sign(request)
+
+    def get_signed_grade_mock_request_with_correct_signature(self):
+        """
+        Generate a proper LTI request object
+        """
+        mock_request = Mock()
+        mock_request.headers = {
+            'X-Requested-With': 'XMLHttpRequest',
+            'Content-Type': 'application/x-www-form-urlencoded',
+            'Authorization': (
+                u'OAuth realm="https://testurl/", oauth_body_hash="wwzA3s8gScKD1VpJ7jMt9b%2BMj9Q%3D",'
+                'oauth_nonce="18821463", oauth_timestamp="1409321145", '
+                'oauth_consumer_key="__consumer_key__", oauth_signature_method="HMAC-SHA1", '
+                'oauth_version="1.0", oauth_signature="fHsE1hhIz76/msUoMR3Lyb7Aou4%3D"'
+            )
+        }
+        mock_request.url = u'https://testurl'
+        mock_request.http_method = u'POST'
+        mock_request.method = mock_request.http_method
+
+        mock_request.body = (
+            '<?xml version=\'1.0\' encoding=\'utf-8\'?>\n'
+            '<imsx_POXEnvelopeRequest xmlns="http://www.imsglobal.org/services/ltiv1p1/xsd/imsoms_v1p0">'
+            '<imsx_POXHeader><imsx_POXRequestHeaderInfo><imsx_version>V1.0</imsx_version>'
+            '<imsx_messageIdentifier>edX_fix</imsx_messageIdentifier></imsx_POXRequestHeaderInfo>'
+            '</imsx_POXHeader><imsx_POXBody><replaceResultRequest><resultRecord><sourcedGUID>'
+            '<sourcedId>MITxLTI/MITxLTI/201x:localhost%3A8000-i4x-MITxLTI-MITxLTI-lti-3751833a214a4f66a0d18f63234207f2:363979ef768ca171b50f9d1bfb322131</sourcedId>'
+            '</sourcedGUID><result><resultScore><language>en</language><textString>0.32</textString></resultScore>'
+            '</result></resultRecord></replaceResultRequest></imsx_POXBody></imsx_POXEnvelopeRequest>'
+        )
+
+        return mock_request
 
     def test_wrong_xml_namespace(self):
         """