fix: Validates if 'lis_outcome_service_url' and 'lis_result_sourcedid…

…' fields exist (#95)

* Check if 'lis_outcome_service_url' and 'lis_result_sourcedid' fields exist before register them in control file
* Fix unit tests related to outcome_service_url
* Minor update to remove unused mock

src/illumidesk/authenticators/authenticator.py

@@ -223,14 +223,18 @@ async def authenticate(self, handler: BaseHandler, data: Dict[str, str] = None)
 
             # 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}')
+            lis_outcome_service_url = None
+            lis_result_sourcedid = None
+            if user_role == 'Learner':                
                 # the next fields must come in args
                 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)
+                # only if both values exist we can register them to submit grades later
+                if lis_outcome_service_url and lis_result_sourcedid:
+                    control_file = LTIGradesSenderControlFile(f'/home/grader-{course_id}/{course_id}')
+                    control_file.register_data(assignment_name, lis_outcome_service_url, lms_user_id, lis_result_sourcedid)
 
             return {
                 'name': username,

src/tests/illumidesk/authenticators/test_lti11_authenticator.py

@@ -45,9 +45,6 @@ async def test_authenticator_returns_auth_state_with_other_lms_vendor(lti11_auth
     """
     Do we get a valid username with lms vendors other than canvas?
     """
-    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()
         handler = Mock(
@@ -116,12 +113,9 @@ async def test_authenticator_returns_auth_state_with_missing_lis_outcome_service
     """
     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 = factory_lti11_complete_launch_args('canvas')
+        args = factory_lti11_complete_launch_args('canvas', 'Learner')        
         del args['lis_outcome_service_url']
         handler = Mock(
             spec=RequestHandler,
@@ -134,7 +128,7 @@ async def test_authenticator_returns_auth_state_with_missing_lis_outcome_service
             'auth_state': {
                 'course_id': 'intro101',
                 'lms_user_id': '185d6c59731a553009ca9b59ca3a885100000',
-                'user_role': 'Instructor',
+                'user_role': 'Learner',
             },
         }
         assert result == expected
@@ -146,12 +140,9 @@ async def test_authenticator_returns_auth_state_with_missing_lis_result_sourcedi
     """
     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 = factory_lti11_complete_launch_args('canvas')
+        args = factory_lti11_complete_launch_args('canvas', 'Learner')
         del args['lis_result_sourcedid']
         handler = Mock(
             spec=RequestHandler,
@@ -164,7 +155,7 @@ async def test_authenticator_returns_auth_state_with_missing_lis_result_sourcedi
             'auth_state': {
                 'course_id': 'intro101',
                 'lms_user_id': '185d6c59731a553009ca9b59ca3a885100000',
-                'user_role': 'Instructor',
+                'user_role': 'Learner',
             },
         }
         assert result == expected
@@ -176,12 +167,9 @@ async def test_authenticator_returns_auth_state_with_empty_lis_result_sourcedid(
     """
     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 = factory_lti11_complete_launch_args('canvas')
+        args = factory_lti11_complete_launch_args('canvas', 'Learner')
         args['lis_result_sourcedid'] = ''
         handler = Mock(
             spec=RequestHandler,
@@ -194,7 +182,7 @@ async def test_authenticator_returns_auth_state_with_empty_lis_result_sourcedid(
             'auth_state': {
                 'course_id': 'intro101',
                 'lms_user_id': '185d6c59731a553009ca9b59ca3a885100000',
-                'user_role': 'Instructor',
+                'user_role': 'Learner',
             },
         }
         assert result == expected
@@ -206,12 +194,9 @@ async def test_authenticator_returns_auth_state_with_empty_lis_outcome_service_u
     """
     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 = factory_lti11_complete_launch_args('canvas')
+        args = factory_lti11_complete_launch_args('canvas', 'Learner')
         args['lis_outcome_service_url'] = ''
         handler = Mock(
             spec=RequestHandler,
@@ -224,7 +209,7 @@ async def test_authenticator_returns_auth_state_with_empty_lis_outcome_service_u
             'auth_state': {
                 'course_id': 'intro101',
                 'lms_user_id': '185d6c59731a553009ca9b59ca3a885100000',
-                'user_role': 'Instructor',
+                'user_role': 'Learner',
             },
         }
         assert result == expected

src/tests/illumidesk/factory.py

@@ -45,7 +45,7 @@ def factory_lti11_basic_launch_args(oauth_consumer_key: str, oauth_consumer_secr
     return args
 
 
-def factory_lti11_complete_launch_args(lms_vendor: str) -> Dict[str, str]:
+def factory_lti11_complete_launch_args(lms_vendor: str, role: str='Instructor') -> Dict[str, str]:
     """
     Valid response when retrieving jwks from the platform.
     """
@@ -81,7 +81,7 @@ def factory_lti11_complete_launch_args(lms_vendor: str) -> Dict[str, str]:
         'oauth_callback': ['about:blank'.encode()],
         'resource_link_id': ['888efe72d4bbbdf90619353bb8ab5965ccbe9b3f'.encode()],
         'resource_link_title': ['IllumiDesk'.encode()],
-        'roles': ['Instructor'.encode()],
+        'roles': ['Instructor'.encode() if role == 'Instructor' else role.encode()],
         'tool_consumer_info_product_family_code': [lms_vendor.encode()],
         'tool_consumer_info_version': ['cloud'.encode()],
         'tool_consumer_instance_contact_email': ['[email protected]'.encode()],