Handle missing lis arguments with learner launch requests

src/illumidesk/authenticators/authenticator.py

@@ -28,7 +28,7 @@ class LTI11Authenticator(LTIAuthenticator):
     def get_handlers(self, app):
         return [('/lti/launch', LTI11AuthenticateHandler)]
 
-    async def authenticate(self, handler, data=None):
+    async def authenticate(self, handler, data=None):  # noqa: C901
         """
         LTI 1.1 authenticator which overrides authenticate function from base LTIAuthenticator.
         After validating the LTI 1.1 signuature, this function decodes the dictionary object
@@ -135,12 +135,15 @@ async def authenticate(self, handler, data=None):
             # use the user_id as the lms_user_id, used to map usernames to lms user ids
             lms_user_id = args['user_id']
 
-            # with all info extracted from lms request, register info for grades sender only if user is a STUDENT
+            # with all info extracted from lms request, register info for grades sender only if the user has
+            # the Learner role
             if user_role == 'Learner':
                 control_file = LTIGradesSenderControlFile(f'/home/grader-{course_id}/{course_id}')
                 # the next fields must come in args
-                lis_outcome_service_url = args['lis_outcome_service_url']
-                lis_result_sourcedid = args['lis_result_sourcedid']
+                if 'lis_outcome_service_url' in args and args['lis_outcome_service_url'] is not None:
+                    lis_outcome_service_url = args['lis_outcome_service_url']
+                if 'lis_result_sourcedid' in args and args['lis_result_sourcedid'] is not None:
+                    lis_result_sourcedid = args['lis_result_sourcedid']
                 control_file.register_data(assignment_name, lis_outcome_service_url, lms_user_id, lis_result_sourcedid)
 
             return {

tests/illumidesk/authenticators/test_lti11_authenticator.py

@@ -36,11 +36,15 @@ def mock_lti11_instructor_args(lms_vendor: str) -> Dict[str, str]:
             'https: //illumidesk.instructure.com/courses/161/external_content/success/external_tool_redirect'.encode()
         ],
         'launch_presentation_width': ['1000'.encode()],
-        'lis_person_contact_email_primary': ['[email protected]'.encode()],
+        'lis_outcome_service_url': [
+            'http://www.imsglobal.org/developers/LTI/test/v1p1/common/tool_consumer_outcome.php?b64=MTIzNDU6OjpzZWNyZXQ='.encode()
+        ],
+        'lis_person_contact_email_primary': ['[email protected]'.encode()],
         'lis_person_name_family': ['Bar'.encode()],
         'lis_person_name_full': ['Foo Bar'.encode()],
         'lis_person_name_given': ['Foo'.encode()],
         'lti_message_type': ['basic-lti-launch-request'.encode()],
+        'lis_result_sourcedid': ['feb-123-456-2929::28883'.encode()],
         'lti_version': ['LTI-1p0'.encode()],
         'oauth_callback': ['about:blank'.encode()],
         'resource_link_id': ['888efe72d4bbbdf90619353bb8ab5965ccbe9b3f'.encode()],
@@ -101,7 +105,7 @@ async def test_authenticator_returns_auth_state_with_other_lms_vendor(lti11_auth
         )
         result = await authenticator.authenticate(handler, None)
         expected = {
-            'name': 'foo',
+            'name': 'student1',
             'auth_state': {
                 'course_id': 'intro101',
                 'lms_user_id': '185d6c59731a553009ca9b59ca3a885100000',
@@ -151,20 +155,25 @@ async def test_authenticator_invokes_validator_with_decoded_dict():
 
 @pytest.mark.asyncio
 @patch('illumidesk.authenticators.authenticator.LTI11LaunchValidator')
-async def test_authenticator_username_with_lis_person_contact_email_primary(lti11_authenticator):
+async def test_authenticator_returns_auth_state_with_missing_lis_outcome_service_url(lti11_authenticator,):
     """
-    Do we get a valid username when sending an argument with lis_person_contact_email_primary?
+    Are we able to handle requests with a missing lis_outcome_service_url key?
     """
+    utils = LTIUtils()
+    utils.convert_request_to_dict = MagicMock(name='convert_request_to_dict')
+    utils.convert_request_to_dict(3, 4, 5, key='value')
     with patch.object(LTI11LaunchValidator, 'validate_launch_request', return_value=True):
         authenticator = LTI11Authenticator()
+        args = mock_lti11_instructor_args('canvas')
+        del args['lis_outcome_service_url']
         handler = Mock(
             spec=RequestHandler,
             get_secure_cookie=Mock(return_value=json.dumps(['key', 'secret'])),
-            request=Mock(arguments=mock_lti11_instructor_args('moodle'), headers={}, items=[],),
+            request=Mock(arguments=args, headers={}, items=[],),
         )
         result = await authenticator.authenticate(handler, None)
         expected = {
-            'name': 'foo',
+            'name': 'student1',
             'auth_state': {
                 'course_id': 'intro101',
                 'lms_user_id': '185d6c59731a553009ca9b59ca3a885100000',
@@ -176,16 +185,81 @@ async def test_authenticator_username_with_lis_person_contact_email_primary(lti1
 
 @pytest.mark.asyncio
 @patch('illumidesk.authenticators.authenticator.LTI11LaunchValidator')
-async def test_authenticator_username_with_lis_person_name_given(lti11_authenticator):
+async def test_authenticator_returns_auth_state_with_missing_lis_result_sourcedid(lti11_authenticator,):
     """
-    Do we get a valid username when sending an argument with lis_person_name_given?
+    Are we able to handle requests with a missing lis_result_sourcedid key?
     """
+    utils = LTIUtils()
+    utils.convert_request_to_dict = MagicMock(name='convert_request_to_dict')
+    utils.convert_request_to_dict(3, 4, 5, key='value')
     with patch.object(LTI11LaunchValidator, 'validate_launch_request', return_value=True):
         authenticator = LTI11Authenticator()
+        args = mock_lti11_instructor_args('canvas')
+        del args['lis_result_sourcedid']
         handler = Mock(
             spec=RequestHandler,
             get_secure_cookie=Mock(return_value=json.dumps(['key', 'secret'])),
-            request=Mock(arguments=mock_lti11_instructor_args('canvas'), headers={}, items=[],),
+            request=Mock(arguments=args, headers={}, items=[],),
+        )
+        result = await authenticator.authenticate(handler, None)
+        expected = {
+            'name': 'student1',
+            'auth_state': {
+                'course_id': 'intro101',
+                'lms_user_id': '185d6c59731a553009ca9b59ca3a885100000',
+                'user_role': 'Instructor',
+            },
+        }
+        assert result == expected
+
+
+@pytest.mark.asyncio
+@patch('illumidesk.authenticators.authenticator.LTI11LaunchValidator')
+async def test_authenticator_returns_auth_state_with_empty_lis_result_sourcedid(lti11_authenticator,):
+    """
+    Are we able to handle requests with lis_result_sourcedid set to an empty value?
+    """
+    utils = LTIUtils()
+    utils.convert_request_to_dict = MagicMock(name='convert_request_to_dict')
+    utils.convert_request_to_dict(3, 4, 5, key='value')
+    with patch.object(LTI11LaunchValidator, 'validate_launch_request', return_value=True):
+        authenticator = LTI11Authenticator()
+        args = mock_lti11_instructor_args('canvas')
+        args['lis_result_sourcedid'] = ''
+        handler = Mock(
+            spec=RequestHandler,
+            get_secure_cookie=Mock(return_value=json.dumps(['key', 'secret'])),
+            request=Mock(arguments=args, headers={}, items=[],),
+        )
+        result = await authenticator.authenticate(handler, None)
+        expected = {
+            'name': 'student1',
+            'auth_state': {
+                'course_id': 'intro101',
+                'lms_user_id': '185d6c59731a553009ca9b59ca3a885100000',
+                'user_role': 'Instructor',
+            },
+        }
+        assert result == expected
+
+
+@pytest.mark.asyncio
+@patch('illumidesk.authenticators.authenticator.LTI11LaunchValidator')
+async def test_authenticator_returns_auth_state_with_empty_lis_outcome_service_url(lti11_authenticator,):
+    """
+    Are we able to handle requests with lis_outcome_service_url set to an empty value?
+    """
+    utils = LTIUtils()
+    utils.convert_request_to_dict = MagicMock(name='convert_request_to_dict')
+    utils.convert_request_to_dict(3, 4, 5, key='value')
+    with patch.object(LTI11LaunchValidator, 'validate_launch_request', return_value=True):
+        authenticator = LTI11Authenticator()
+        args = mock_lti11_instructor_args('canvas')
+        args['lis_outcome_service_url'] = ''
+        handler = Mock(
+            spec=RequestHandler,
+            get_secure_cookie=Mock(return_value=json.dumps(['key', 'secret'])),
+            request=Mock(arguments=args, headers={}, items=[],),
         )
         result = await authenticator.authenticate(handler, None)
         expected = {

tests/illumidesk/authenticators/test_lti11_validator.py

@@ -15,9 +15,7 @@
 from illumidesk.authenticators.utils import LTIUtils
 
 
-def mock_lti11_args(
-    oauth_consumer_key: str, oauth_consumer_secret: str,
-) -> Dict[str, str]:
+def mock_lti11_args(oauth_consumer_key: str, oauth_consumer_secret: str,) -> Dict[str, str]:
 
     utils = LTIUtils()
     oauth_timestamp = str(int(time.time()))
@@ -43,14 +41,10 @@ def mock_lti11_args(
     base_string = signature.signature_base_string(
         'POST',
         signature.base_string_uri(launch_url),
-        signature.normalize_parameters(
-            signature.collect_parameters(body=args, headers=headers)
-        ),
+        signature.normalize_parameters(signature.collect_parameters(body=args, headers=headers)),
     )
 
-    args['oauth_signature'] = signature.sign_hmac_sha1(
-        base_string, oauth_consumer_secret, None
-    )
+    args['oauth_signature'] = signature.sign_hmac_sha1(base_string, oauth_consumer_secret, None)
 
     return args
 
@@ -496,7 +490,7 @@ def test_launch_with_none_or_empty_lti_version():
 
 def test_launch_with_missing_resource_link_id():
     """
-    Does the launch request work with a missing oauth_signature key?
+    Does the launch request work with a missing resource_link_id key?
     """
     oauth_consumer_key = 'my_consumer_key'
     oauth_consumer_secret = 'my_shared_secret'
@@ -515,7 +509,7 @@ def test_launch_with_missing_resource_link_id():
 
 def test_launch_with_none_or_empty_resource_link_id():
     """
-    Does the launch request work with an empty or None oauth_signature value?
+    Does the launch request work with an empty or None resource_link_id value?
     """
     oauth_consumer_key = 'my_consumer_key'
     oauth_consumer_secret = 'my_shared_secret'