From d97495c8a8474b9a00fce784586629466100b5ad Mon Sep 17 00:00:00 2001 From: Andrew Gardener Date: Tue, 26 Sep 2017 11:39:15 -0700 Subject: [PATCH] Handle unicode in cas and lti membership CAS overrides should be handled better in the future --- compair/cas.py | 21 ++++++- compair/models/lti_models/lti_membership.py | 2 +- compair/tests/api/test_lti_launch.py | 66 +++++++++++---------- 3 files changed, 53 insertions(+), 36 deletions(-) diff --git a/compair/cas.py b/compair/cas.py index 8c80d3e15..72cc63161 100644 --- a/compair/cas.py +++ b/compair/cas.py @@ -1,5 +1,7 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals from flask import current_app, url_for -from caslib import SAMLClient, CASClient +from caslib import SAMLClient, CASClient, CASResponse from xml.dom.minidom import parseString import requests @@ -18,7 +20,7 @@ def _get_client(): auth_prefix=auth_prefix ) else: - return CASClient( + return CustomCASClient( server_url=server_url, service_url=service_url, auth_prefix=auth_prefix @@ -36,6 +38,18 @@ def get_cas_logout_url(): return _get_client()._logout_url(logout_service_url) +class CustomCASClient(CASClient): + def get_cas_response(self, url): + try: + # overwritten to allow development environment to use self signed certificates + verify = current_app.config.get('ENFORCE_SSL', True) + response = requests.get(url, verify=verify) + response_text = response.text.encode('utf-8') if response.text else None + return CASResponse(response_text) + except Exception: + current_app.logging.exception("CASLIB: Error retrieving a response") + return None + class CustomSAMLClient(SAMLClient): def get_saml_response(self, url, envelope): @@ -43,7 +57,8 @@ def get_saml_response(self, url, envelope): # overwritten to allow development environment to use self signed certificates verify = current_app.config.get('ENFORCE_SSL', True) response = requests.post(url, data=envelope, verify=verify) - return CustomSAMLResponse(response.text) + response_text = response.text.encode('utf-8') if response.text else None + return CustomSAMLResponse(response_text) except Exception: current_app.logger.error("SAML: Error retrieving a response") raise diff --git a/compair/models/lti_models/lti_membership.py b/compair/models/lti_models/lti_membership.py index b4a53e78d..fa6332fe7 100644 --- a/compair/models/lti_models/lti_membership.py +++ b/compair/models/lti_models/lti_membership.py @@ -265,7 +265,7 @@ def _get_membership_ext(cls, lti_context): params = parse_qs(signed_request.body.decode('utf-8')) data = LTIMembership._post_membership_request(memberships_url, params) - root = ElementTree.fromstring(data) + root = ElementTree.fromstring(data.encode('utf-8')) codemajor = root.find('statusinfo/codemajor') if codemajor is not None and codemajor.text in ['Failure', 'Unsupported']: diff --git a/compair/tests/api/test_lti_launch.py b/compair/tests/api/test_lti_launch.py index 7aaedc083..f1899bc8d 100644 --- a/compair/tests/api/test_lti_launch.py +++ b/compair/tests/api/test_lti_launch.py @@ -1,3 +1,5 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals import json import mock @@ -698,7 +700,7 @@ def test_lti_course_link_with_membership_ext(self, mocked_post_membership_reques Learner - compair_student_3 + compair_student_3è TeachingAssistant @@ -731,7 +733,7 @@ def test_lti_course_link_with_membership_ext(self, mocked_post_membership_reques self.assertEqual(len(lti_memberships), 5) for lti_membership in lti_memberships: self.assertIn(lti_membership.lti_user.user_id, [lti_user.user_id, "compair_student_1", "compair_student_2", - "compair_student_3", "compair_instructor_2"]) + "compair_student_3è", "compair_instructor_2"]) # test successful membership response (with user_id_override) with self.lti_launch(lti_consumer, lti_resource_link.resource_link_id, @@ -765,9 +767,9 @@ def test_lti_course_link_with_membership_ext(self, mocked_post_membership_reques compair_student_2 - ignore_4 + ignore_4è TeachingAssistant - compair_student_3 + compair_student_3è ignore_5 @@ -800,7 +802,7 @@ def test_lti_course_link_with_membership_ext(self, mocked_post_membership_reques self.assertEqual(len(lti_memberships), 5) for lti_membership in lti_memberships: self.assertIn(lti_membership.lti_user.user_id, [lti_user.user_id, "compair_student_1", "compair_student_2", - "compair_student_3", "compair_instructor_2"]) + "compair_student_3è", "compair_instructor_2"]) @mock.patch('compair.models.lti_models.lti_membership.LTIMembership._get_membership_request') @@ -903,7 +905,7 @@ def minimal_membership_requests(memberships_url, headers=None): "urn:lti:role:ims/lis/TeachingAssistant" ], "member":{ - "userId":"compair_student_3" + "userId":"compair_student_3è" } }, { @@ -995,11 +997,11 @@ def minimal_membership_requests(memberships_url, headers=None): "urn:lti:role:ims/lis/TeachingAssistant" ], "member":{ - "userId":"compair_student_3" + "userId":"compair_student_3è" }, "message": [{ "message_type": "basic-lti-launch-request", - "lis_result_sourcedid": "lis_result_sourcedid_compair_student_3" + "lis_result_sourcedid": "lis_result_sourcedid_compair_student_3è" },{ "message_type": "other-message-type" }] @@ -1076,7 +1078,7 @@ def minimal_membership_requests(memberships_url, headers=None): self.assertEqual(len(lti_memberships), 5) for lti_membership in lti_memberships: self.assertIn(lti_membership.lti_user.user_id, [lti_user.user_id, "compair_student_1", "compair_student_2", - "compair_student_3", "compair_instructor_2"]) + "compair_student_3è", "compair_instructor_2"]) # ensure the lti_user_resource_link is generated and stores the lis_result_sourcedid lti_user_resource_links = [lti_user_resource_link \ @@ -1180,10 +1182,10 @@ def user_id_override_membership_requests(memberships_url, headers=None): "message_type": "basic-lti-launch-request", "lis_result_sourcedid": "1234567890-3", "custom" : { - "puid": "compair_student_3_puid" + "puid": "compair_student_3è_puid" }, "ext" : { - "user_username": "compair_student_3_username", + "user_username": "compair_student_3è_username", } } ] @@ -1324,12 +1326,12 @@ def user_id_override_membership_requests(memberships_url, headers=None): "message" : [ { "message_type": "basic-lti-launch-request", - "lis_result_sourcedid": "lis_result_sourcedid_compair_student_3", + "lis_result_sourcedid": "lis_result_sourcedid_compair_student_3è", "custom" : { - "puid": "compair_student_3_puid" + "puid": "compair_student_3è_puid" }, "ext" : { - "user_username": "compair_student_3_username", + "user_username": "compair_student_3è_username", } } ] @@ -1420,10 +1422,10 @@ def user_id_override_membership_requests(memberships_url, headers=None): self.assertIn(lti_membership.lti_user.user_id, [lti_user.user_id, "userId_2", "userId_3", "userId_4", "userId_5"]) elif user_id_override == "custom_puid": self.assertIn(lti_membership.lti_user.user_id, [lti_user.user_id, "compair_student_1_puid", "compair_student_2_puid", - "compair_student_3_puid", "compair_instructor_2_puid"]) + "compair_student_3è_puid", "compair_instructor_2_puid"]) elif user_id_override == "ext_user_username": self.assertIn(lti_membership.lti_user.user_id, [lti_user.user_id, "compair_student_1_username", "compair_student_2_username", - "compair_student_3_username", "compair_instructor_2_username"]) + "compair_student_3è_username", "compair_instructor_2_username"]) elif user_id_override == "lis_result_sourcedid": self.assertIn(lti_membership.lti_user.user_id, [lti_user.user_id, "1234567890-1", "1234567890-2", "1234567890-3", "1234567890-4"]) @@ -1503,13 +1505,13 @@ def paginated_membership_requests(memberships_url, headers=None): elif memberships_url == "https://mockmembershipurl.com?page=4&per_page=1": result['nextPage'] = "https://mockmembershipurl.com?page=5&per_page=1" result['pageOf']['membershipSubject']['membership'][0]['role'] = ["urn:lti:role:ims/lis/TeachingAssistant"] - result['pageOf']['membershipSubject']['membership'][0]['member']['userId'] = "compair_student_3" + result['pageOf']['membershipSubject']['membership'][0]['member']['userId'] = "compair_student_3è" elif memberships_url == "https://mockmembershipurl.com?rlid="+rlid+"&page=4&per_page=1": result['nextPage'] = "https://mockmembershipurl.com?rlid="+rlid+"&page=5&per_page=1" result['pageOf']['membershipSubject']['membership'][0]['role'] = ["urn:lti:role:ims/lis/TeachingAssistant"] - result['pageOf']['membershipSubject']['membership'][0]['member']['userId'] = "compair_student_3" - result_launch_message['lis_result_sourcedid'] = "lis_result_sourcedid_compair_student_3" + result['pageOf']['membershipSubject']['membership'][0]['member']['userId'] = "compair_student_3è" + result_launch_message['lis_result_sourcedid'] = "lis_result_sourcedid_compair_student_3è" result['pageOf']['membershipSubject']['membership'][0]['message'] = [result_launch_message] elif memberships_url == "https://mockmembershipurl.com?page=5&per_page=1": @@ -1573,7 +1575,7 @@ def paginated_membership_requests(memberships_url, headers=None): self.assertEqual(len(lti_memberships), 5) for lti_membership in lti_memberships: self.assertIn(lti_membership.lti_user.user_id, [lti_user.user_id, "compair_student_1", "compair_student_2", - "compair_student_3", "compair_instructor_2"]) + "compair_student_3è", "compair_instructor_2"]) @mock.patch('compair.models.lti_models.lti_membership.LTIMembership._post_membership_request') def test_lti_membership_for_consumer_with_membership_ext(self, mocked_post_membership_request): @@ -1694,11 +1696,11 @@ def test_lti_membership_for_consumer_with_membership_ext(self, mocked_post_membe :_676_1::compai:compair_student_2 - compair_student_3 + compair_student_3è http://www.gravatar.com/avatar/4 TeachingAssistant - compair_student_3 - compair_student_3@test.com + compair_student_3è + compair_student_3è@test.com Student Six Student Six @@ -1739,7 +1741,7 @@ def test_lti_membership_for_consumer_with_membership_ext(self, mocked_post_membe self.assertEqual(len(lti_memberships), 5) for lti_membership in lti_memberships: self.assertIn(lti_membership.lti_user.user_id, [lti_user_instructor.user_id, lti_user_student_1.user_id, - "compair_student_2", "compair_student_3", "compair_instructor_2"]) + "compair_student_2", "compair_student_3è", "compair_instructor_2"]) # test minimual membership response mocked_post_membership_request.return_value = """ @@ -1765,7 +1767,7 @@ def test_lti_membership_for_consumer_with_membership_ext(self, mocked_post_membe Learner - compair_student_3 + compair_student_3è TeachingAssistant @@ -1798,7 +1800,7 @@ def test_lti_membership_for_consumer_with_membership_ext(self, mocked_post_membe self.assertEqual(len(lti_memberships), 5) for lti_membership in lti_memberships: self.assertIn(lti_membership.lti_user.user_id, [lti_user_instructor.user_id, lti_user_student_1.user_id, - "compair_student_2", "compair_student_3", "compair_instructor_2"]) + "compair_student_2", "compair_student_3è", "compair_instructor_2"]) # test ensure current user is not unenrolled from course on membership fetch @@ -1993,12 +1995,12 @@ def test_lti_membership_for_consumer_with_membership_service(self, mocked_get_me "@id":None, "name":"Student Six", "img":"http://www.gravatar.com/avatar/4", - "email":"compair_student_3@test.com", + "email":"compair_student_3è@test.com", "familyName":"Six", "givenName":"Student", "resultSourcedId":None, - "sourcedId":"compair_student_3", - "userId":"compair_student_3" + "sourcedId":"compair_student_3è", + "userId":"compair_student_3è" } }, { @@ -2067,7 +2069,7 @@ def test_lti_membership_for_consumer_with_membership_service(self, mocked_get_me self.assertEqual(len(lti_memberships), 5) for lti_membership in lti_memberships: self.assertIn(lti_membership.lti_user.user_id, [lti_user_instructor.user_id, lti_user_student_1.user_id, - "compair_student_2", "compair_student_3", "compair_instructor_2", "compair_student_100"]) + "compair_student_2", "compair_student_3è", "compair_instructor_2", "compair_student_100"]) # test minimual membership response mocked_get_membership_request.return_value = { @@ -2116,7 +2118,7 @@ def test_lti_membership_for_consumer_with_membership_service(self, mocked_get_me "urn:lti:role:ims/lis/TeachingAssistant" ], "member":{ - "userId":"compair_student_3" + "userId":"compair_student_3è" } }, { @@ -2166,7 +2168,7 @@ def test_lti_membership_for_consumer_with_membership_service(self, mocked_get_me self.assertEqual(len(lti_memberships), 5) for lti_membership in lti_memberships: self.assertIn(lti_membership.lti_user.user_id, [lti_user_instructor.user_id, lti_user_student_1.user_id, - "compair_student_2", "compair_student_3", "compair_instructor_2"]) + "compair_student_2", "compair_student_3è", "compair_instructor_2"]) # test ensure current user is not unenrolled from course on membership fetch