Commit 18648b52 by Pan Luo Committed by Nimisha Asthagiri

Fix the duplicate oauth_body_hash in the outcomes request

The "oauth_body_hash" appeared twice in the auth header in the request
when posting grade back to tool consumer. However, the signature sent
from edX is calculated based on only one oauth_body_hash.
On the tool consumer side, the signature is calculated based on the auth
header and will use the duplicated fields. So the signatures will not match.
And request will fail the signature validation.

The bug was introduced in this commit:
https://github.com/edx/edx-platform/commit/03cee389e0869263f5f9977098770495b5216733
on July 12th by updating the oauthlib.

Because 0.7.2(original version) doesn't have oauth_body_hash support, so a custom
OAuth1 client was implemented to add oauth_body_hash to the headers:
https://github.com/edx/edx-platform/blob/f5d0f3ff55d264ed4b68a24808b255aef5d8dad2/lms/djangoapps/lti_provider/outcomes.py#L24.
However, the new oauthlib 1.0.3 has support for oauth_body_hash
(https://github.com/idan/oauthlib/commit/51675237c410b413a11091926436420493c52866#diff-c2a1e5f1ddfe8e48ff62b59eb952644eR180).
So after updating library, oauth_body_hash is added twice.

This fixes the bug by removing the custom client and use the oauthlib
default client to generate the auth header.
parent c70ebfbb
...@@ -3,15 +3,11 @@ Helper functions for managing interactions with the LTI outcomes service defined ...@@ -3,15 +3,11 @@ Helper functions for managing interactions with the LTI outcomes service defined
in LTI v1.1. in LTI v1.1.
""" """
from hashlib import sha1
from base64 import b64encode
import logging import logging
import uuid import uuid
from lxml import etree from lxml import etree
from lxml.builder import ElementMaker from lxml.builder import ElementMaker
from oauthlib.oauth1 import Client
from oauthlib.common import to_unicode
import requests import requests
from requests.exceptions import RequestException from requests.exceptions import RequestException
import requests_oauthlib import requests_oauthlib
...@@ -21,22 +17,6 @@ from lti_provider.models import GradedAssignment, OutcomeService ...@@ -21,22 +17,6 @@ from lti_provider.models import GradedAssignment, OutcomeService
log = logging.getLogger("edx.lti_provider") log = logging.getLogger("edx.lti_provider")
class BodyHashClient(Client):
"""
OAuth1 Client that adds body hash support (required by LTI).
The default Client doesn't support body hashes, so we have to add it ourselves.
The spec:
https://oauth.googlecode.com/svn/spec/ext/body_hash/1.0/oauth-bodyhash.html
"""
def get_oauth_params(self, request):
"""Override get_oauth_params to add the body hash."""
params = super(BodyHashClient, self).get_oauth_params(request)
digest = b64encode(sha1(request.body.encode('UTF-8')).digest())
params.append((u'oauth_body_hash', to_unicode(digest)))
return params
def store_outcome_parameters(request_params, user, lti_consumer): def store_outcome_parameters(request_params, user, lti_consumer):
""" """
Determine whether a set of LTI launch parameters contains information about Determine whether a set of LTI launch parameters contains information about
...@@ -183,16 +163,10 @@ def sign_and_send_replace_result(assignment, xml): ...@@ -183,16 +163,10 @@ def sign_and_send_replace_result(assignment, xml):
consumer_secret = consumer.consumer_secret consumer_secret = consumer.consumer_secret
# Calculate the OAuth signature for the replace_result message. # Calculate the OAuth signature for the replace_result message.
# TODO: According to the LTI spec, there should be an additional
# oauth_body_hash field that contains a digest of the replace_result
# message. Testing with Canvas throws an error when this field is included.
# This code may need to be revisited once we test with other LMS platforms,
# and confirm whether there's a bug in Canvas.
oauth = requests_oauthlib.OAuth1( oauth = requests_oauthlib.OAuth1(
consumer_key, consumer_key,
consumer_secret, consumer_secret,
signature_method='HMAC-SHA1', signature_method='HMAC-SHA1',
client_class=BodyHashClient,
force_include_body=True force_include_body=True
) )
......
...@@ -295,47 +295,6 @@ class XmlHandlingTest(TestCase): ...@@ -295,47 +295,6 @@ class XmlHandlingTest(TestCase):
self.assertFalse(outcomes.check_replace_result_response(response)) self.assertFalse(outcomes.check_replace_result_response(response))
class TestBodyHashClient(unittest.TestCase):
"""
Test our custom BodyHashClient
This Client should do everything a normal oauthlib.oauth1.Client would do,
except it also adds oauth_body_hash to the Authorization headers.
"""
def test_simple_message(self):
oauth = requests_oauthlib.OAuth1(
'1000000000000000', # fake consumer key
'2000000000000000', # fake consumer secret
signature_method='HMAC-SHA1',
client_class=outcomes.BodyHashClient,
force_include_body=True
)
headers = {'content-type': 'application/xml'}
req = requests.Request(
'POST',
"http://example.edx.org/fake",
data="Hello world!",
auth=oauth,
headers=headers
)
prepped_req = req.prepare()
# Make sure that our body hash is now part of the test...
self.assertIn(
'oauth_body_hash="00hq6RNueFa8QiEjhep5cJRHWAI%3D"',
prepped_req.headers['Authorization']
)
# But make sure we haven't wiped out any of the other oauth values
# that we would expect to be in the Authorization header as well
expected_oauth_headers = [
"oauth_nonce", "oauth_timestamp", "oauth_version",
"oauth_signature_method", "oauth_consumer_key", "oauth_signature",
]
for oauth_header in expected_oauth_headers:
self.assertIn(oauth_header, prepped_req.headers['Authorization'])
class TestAssignmentsForProblem(ModuleStoreTestCase): class TestAssignmentsForProblem(ModuleStoreTestCase):
""" """
Test cases for the assignments_for_problem method in outcomes.py Test cases for the assignments_for_problem method in outcomes.py
......
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