tl-its-umich-edu / canvas-app-explorer

A Web application that presents a list of Canvas external (LTI) tools with details. When integrated within Canvas, the user can search for specific LTI tool(s), and add or remove those tools from Canvas courses.
Apache License 2.0
4 stars 6 forks source link

Possibly rely on LTI launch roles over Canvas roles for launch restrictions #254

Open ssciolla opened 2 years ago

ssciolla commented 2 years ago

As part of #247, in discussion with @jonespm, I tried to rely on the roles LTI claim instead of a custom variable from Canvas for determining who was a student, instructor, and/or admin in the course context. I ran up against what I think may be a bug in the PyLTI1p3 library. In order to use this more general approach, we may need to contribute to the library to improve context identifier parsing. See the comments below.


It looks like it still sends Instructor as an institution role and the pylti1.3 includes this as a common role for a Teacher.

I'd think if you wanted a non masqueraded common role here you'd have to have only a context role. Though I don't think this "View As" edge case is going to really be a problem. They still do technically have the role as an Instructor and a Student in the same course.

class TeacherRoleContextOnly(AbstractRole):
    _context_roles = ('Instructor', 'Administrator')  # type: tuple

_Originally posted by @jonespm in https://github.com/tl-its-umich-edu/canvas-app-explorer/pull/247#discussion_r925813728_


I tried something like this class, and it's not really working as I expected. Code below:

class StaffRoleContextOnly(AbstractRole):
    _institution_roles = ('Administrator',)
    _context_roles = ('Instructor', 'Administrator')
    logger.info(roles)
    staff_role_obj = StaffRoleContextOnly(message_launch._get_jwt_body())
    for role in roles:
        logger.info(staff_role_obj.parse_role_str(role))

    user_is_staff_in_course = StaffRoleContextOnly(message_launch._get_jwt_body()).check()

    if not user_is_staff_in_course:
        logger.warn(f'User {username} does not have a staff role.')
        raise PermissionDenied()

This will work for the admin and student cases, but I couldn't launch when I was masquerading as an instructor. My initial impression is that the context roles generated by Canvas are malformed, or there's a bug in this library, but not sure exactly. When I parse the strings Canvas generates as the library does, I get this:

[INFO] [lti1p3.py:145] ['http://purl.imsglobal.org/vocab/lis/v2/institution/person#Instructor', 'http://purl.imsglobal.org/vocab/lis/v2/institution/person#Student', 'http://purl.imsglobal.org/vocab/lis/v2/membership#Instructor', 'http://purl.imsglobal.org/vocab/lis/v2/system/person#User']
[INFO] [lti1p3.py:148] ('Instructor', 'institution')
[INFO] [lti1p3.py:148] ('Student', 'institution')
[INFO] [lti1p3.py:148] ('Instructor', None)
[INFO] [lti1p3.py:148] ('User', 'system')

_Originally posted by @ssciolla in https://github.com/tl-its-umich-edu/canvas-app-explorer/pull/247#discussion_r925881753_


@jonespm, it looks there may be an issue with the pylti1.3 source code: https://github.com/dmitry-viskov/pylti1.3/blob/7474685aecc35d528b944caf05b28ea66ac6d204/pylti1p3/roles.py#L44-L57

http://www.imsglobal.org/spec/lti/v1p3/#lis-vocabulary-for-context-roles

The membership roles just don't have the same structure as the institution or system roles; there is no slash, so the splits don't result in ('Instructor', 'membership') as expected.

I think I'm going to stick with the custom variable roles approach for now, but sure, we can create an issue to do this. It'd be interesting to look into this for CCM as well, but I don't think ltijs has an equivalent role class. Then again, these tools are pretty reliant on Canvas, and we're not getting much from the LMS-agnostic aspect of the LTI/LIS roles.

_Originally posted by @ssciolla in https://github.com/tl-its-umich-edu/canvas-app-explorer/pull/247#discussion_r926099256_