a-plus icon indicating copy to clipboard operation
a-plus copied to clipboard

LTI Tool v1.3: Launch view crashes if expected LTI parameters are missing

Open PasiSa opened this issue 3 years ago • 3 comments

The LTI launch request processing assumes that the LTI platform always sends the expected parameters in the JWT claims. In practice the A+ sysadmins would operate with well known platforms and can "know" that the platform should work as expected before configuring a new platform, but at least the "custom" field can easily be missing or have invalid structure because of misconfiguration (here).

Probably it would be best to have a common handler for all parameters that produces a nice error notification if LTI platform is not delivering the expected parameters.

PasiSa avatar Jan 21 '23 17:01 PasiSa

I think this is the same thing I wrote in the comment here (in the file lti_tool/views.py): https://github.com/apluslms/a-plus/pull/1104#discussion_r1053586866

I suppose it is possible that the teacher modifies the custom parameters in the Moodle external activity settings. Then, if they remove, e.g., the "module_slug" parameter, this line crashes to KeyError since the launch_params dictionary does not contain the key. At least, the students can't do that to cause crashes on purpose.

Catching the exceptions would be good so that the platform does not crash when somebody does something wrong. In practice, we assume that teachers understand they are not supposed to manually modify the custom LTI parameters in Moodle. The custom LTI parameters are automatically set by the "deep linking", which is basically the pop-up window in the Moodle activity settings for selecting the learning object in A+ to connect to the Moodle LTI activity.

markkuriekkinen avatar Feb 03 '23 17:02 markkuriekkinen

It just occurred to me that LTI 1.3 does not require that email, first name and last name are provided from the platform, and commonly a platform (also Moodle) can be configured to not send these (not necessarily an uncommon scenario in the GDPR world). Supporting anonymous case would require quite some redesign, but at least we should be able to handle the case where email, etc. are not received from the platform, e.g. here: https://github.com/apluslms/a-plus/blob/4def5390af7d6df0ee14b48c1ee2ee4cfc0312da/lti_tool/views.py#L56

I understand that our main use case is to interact with Aalto Moodle with specific exercise type, but if A+ was to support LTI tool specification more generally, these things should be considered.

PasiSa avatar Mar 22 '23 09:03 PasiSa

Just to confirm the above comment, when testing with opendev8, I got the following errors when trying to add a new activity with minus-based tool.

[2023-04-25 10:39:32,463: ERROR/log] Internal Server Error: /lti/launch/ Traceback (most recent call last): File "/a-plus/lti_tool/views.py", line 56, in post username = self.message_launch_data['https://purl.imsglobal.org/spec/lti/claim/ext']['user_username'] KeyError: 'user_username'

During handling of the above exception, another exception occurred:

Traceback (most recent call last): File "/usr/local/lib/python3.8/dist-packages/django/core/handlers/exception.py", line 47, in inner response = get_response(request) File "/usr/local/lib/python3.8/dist-packages/django/core/handlers/base.py", line 181, in _get_response response = wrapped_callback(request, *callback_args, **callback_kwargs) File "/usr/local/lib/python3.8/dist-packages/django/views/generic/base.py", line 70, in view return self.dispatch(request, *args, **kwargs) File "/usr/local/lib/python3.8/dist-packages/django/utils/decorators.py", line 43, in _wrapper return bound_method(*args, **kwargs) File "/usr/local/lib/python3.8/dist-packages/django/views/decorators/csrf.py", line 54, in wrapped_view return view_func(*args, **kwargs) File "/usr/local/lib/python3.8/dist-packages/django/utils/decorators.py", line 43, in _wrapper return bound_method(*args, **kwargs) File "/usr/local/lib/python3.8/dist-packages/django/views/decorators/clickjacking.py", line 50, in wrapped_view resp = view_func(*args, **kwargs) File "/a-plus/authorization/views.py", line 38, in dispatch response = self.handle_exception(exc) File "/a-plus/authorization/views.py", line 21, in handle_exception raise exc File "/a-plus/authorization/views.py", line 36, in dispatch response = super().dispatch(request, *args, **kwargs) File "/usr/local/lib/python3.8/dist-packages/django/views/generic/base.py", line 98, in dispatch return handler(request, *args, **kwargs) File "/a-plus/lti_tool/views.py", line 59, in post username = self.message_launch_data['email'] KeyError: 'email'

What puzzles me is that I had "Share launcher's email" and "Share launcher's name" enabled in the configuration, but still in this case (when entering deep linking content browser while configuring activity), I got these errors.

PasiSa avatar Apr 25 '23 07:04 PasiSa