From f577a6d2781736ecc2c6bfe8fb7e1d80547ae54e Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Mon, 31 Aug 2015 15:22:37 -0400 Subject: [PATCH] Add body hash to LTI outcome message. This is necessary to properly implement the LTI outcome passback spec. It was not included previously because it was causing problems with Canvas, but Blackboard will not accept outcomes unless they are properly signed. The requests_oauthlib doesn't support the body hash spec out of the box, so BodyHashClient needed to be made. Fortunately, it's a pretty simple spec: https://oauth.googlecode.com/svn/spec/ext/body_hash/1.0/oauth-bodyhash.html --- lms/djangoapps/lti_provider/outcomes.py | 32 ++++++++++++- .../lti_provider/tests/test_outcomes.py | 47 ++++++++++++++++++- 2 files changed, 76 insertions(+), 3 deletions(-) diff --git a/lms/djangoapps/lti_provider/outcomes.py b/lms/djangoapps/lti_provider/outcomes.py index bcfd37a883..bc02bcb3ec 100644 --- a/lms/djangoapps/lti_provider/outcomes.py +++ b/lms/djangoapps/lti_provider/outcomes.py @@ -3,18 +3,39 @@ Helper functions for managing interactions with the LTI outcomes service defined in LTI v1.1. """ +from hashlib import sha1 +from base64 import b64encode import logging +import uuid + from lxml import etree from lxml.builder import ElementMaker +from oauthlib.oauth1 import Client +from oauthlib.common import to_unicode import requests import requests_oauthlib -import uuid from lti_provider.models import GradedAssignment, OutcomeService 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): """ Determine whether a set of LTI launch parameters contains information about @@ -112,7 +133,13 @@ def sign_and_send_replace_result(assignment, xml): # 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(consumer_key, consumer_secret) + oauth = requests_oauthlib.OAuth1( + consumer_key, + consumer_secret, + signature_method='HMAC-SHA1', + client_class=BodyHashClient, + force_include_body=True + ) headers = {'content-type': 'application/xml'} response = requests.post( @@ -121,6 +148,7 @@ def sign_and_send_replace_result(assignment, xml): auth=oauth, headers=headers ) + return response diff --git a/lms/djangoapps/lti_provider/tests/test_outcomes.py b/lms/djangoapps/lti_provider/tests/test_outcomes.py index 981a56a704..4845e7ff3e 100644 --- a/lms/djangoapps/lti_provider/tests/test_outcomes.py +++ b/lms/djangoapps/lti_provider/tests/test_outcomes.py @@ -1,16 +1,20 @@ """ Tests for the LTI outcome service handlers, both in outcomes.py and in tasks.py """ +import unittest from django.test import TestCase from lxml import etree from mock import patch, MagicMock, ANY +import requests_oauthlib +import requests + +from opaque_keys.edx.locator import CourseLocator, BlockUsageLocator from student.tests.factories import UserFactory from lti_provider.models import GradedAssignment, LtiConsumer, OutcomeService import lti_provider.outcomes as outcomes import lti_provider.tasks as tasks -from opaque_keys.edx.locator import CourseLocator, BlockUsageLocator class StoreOutcomeParametersTest(TestCase): @@ -363,3 +367,44 @@ class XmlHandlingTest(TestCase): major_code='failure' ) 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'])