diff --git a/common/lib/xmodule/xmodule/lti_module.py b/common/lib/xmodule/xmodule/lti_module.py index 85d4fb9300..1e02acefba 100644 --- a/common/lib/xmodule/xmodule/lti_module.py +++ b/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): """ diff --git a/common/lib/xmodule/xmodule/tests/test_lti_unit.py b/common/lib/xmodule/xmodule/tests/test_lti_unit.py index 13de6ceef9..fe3be39fa2 100644 --- a/common/lib/xmodule/xmodule/tests/test_lti_unit.py +++ b/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 = ( + '\n' + '' + 'V1.0' + 'edX_fix' + '' + 'MITxLTI/MITxLTI/201x:localhost%3A8000-i4x-MITxLTI-MITxLTI-lti-3751833a214a4f66a0d18f63234207f2:363979ef768ca171b50f9d1bfb322131' + 'en0.32' + '' + ) + + return mock_request def test_wrong_xml_namespace(self): """