Skip to content

Commit

Permalink
Handle unicode in cas and lti membership
Browse files Browse the repository at this point in the history 
CAS overrides should be handled better in the future
  • Loading branch information
@andrew-gardener
andrew-gardener committed Sep 26, 2017
1 parent 5589eb3 commit d97495c
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 36 deletions.
21 changes: 18 additions & 3 deletions compair/cas.py
View file
Original file line number Diff line number Diff line change
@@ -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

Expand All @@ -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
Expand All @@ -36,14 +38,27 @@ 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):
try:
# 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
Expand Down
2 changes: 1 addition & 1 deletion compair/models/lti_models/lti_membership.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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']:
Expand Down
66 changes: 34 additions & 32 deletions compair/tests/api/test_lti_launch.py
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# -*- coding: utf-8 -*-
from __future__ import unicode_literals
import json
import mock

Expand Down Expand Up @@ -698,7 +700,7 @@ def test_lti_course_link_with_membership_ext(self, mocked_post_membership_reques
<roles>Learner</roles>
</member>
<member>
<user_id>compair_student_3</user_id>
<user_id>compair_student_3è</user_id>
<roles>TeachingAssistant</roles>
</member>
<member>
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -765,9 +767,9 @@ def test_lti_course_link_with_membership_ext(self, mocked_post_membership_reques
<custom_puid>compair_student_2</custom_puid>
</member>
<member>
<user_id>ignore_4</user_id>
<user_id>ignore_4è</user_id>
<roles>TeachingAssistant</roles>
<custom_puid>compair_student_3</custom_puid>
<custom_puid>compair_student_3è</custom_puid>
</member>
<member>
<user_id>ignore_5</user_id>
Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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è"
}
},
{
Expand Down Expand Up @@ -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"
}]
Expand Down Expand Up @@ -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 \
Expand Down Expand Up @@ -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",
}
}
]
Expand Down Expand Up @@ -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",
}
}
]
Expand Down Expand Up @@ -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"])
Expand Down Expand Up @@ -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":
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -1694,11 +1696,11 @@ def test_lti_membership_for_consumer_with_membership_ext(self, mocked_post_membe
<lis_result_sourcedid>:_676_1::compai:compair_student_2</lis_result_sourcedid>
</member>
<member>
<user_id>compair_student_3</user_id>
<user_id>compair_student_3è</user_id>
<user_image>http://www.gravatar.com/avatar/4</user_image>
<roles>TeachingAssistant</roles>
<person_sourcedid>compair_student_3</person_sourcedid>
<person_contact_email_primary>compair_student_3@test.com</person_contact_email_primary>
<person_sourcedid>compair_student_3è</person_sourcedid>
<person_contact_email_primary>compair_student_3è@test.com</person_contact_email_primary>
<person_name_given>Student</person_name_given>
<person_name_family>Six</person_name_family>
<person_name_full>Student Six</person_name_full>
Expand Down Expand Up @@ -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 = """
Expand All @@ -1765,7 +1767,7 @@ def test_lti_membership_for_consumer_with_membership_ext(self, mocked_post_membe
<roles>Learner</roles>
</member>
<member>
<user_id>compair_student_3</user_id>
<user_id>compair_student_3è</user_id>
<roles>TeachingAssistant</roles>
</member>
<member>
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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è"
}
},
{
Expand Down Expand Up @@ -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 = {
Expand Down Expand Up @@ -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è"
}
},
{
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit d97495c

Please sign in to comment.