avalonmediasystem / avalon

Avalon Media System – Samvera Application
http://www.avalonmediasystem.org/
Apache License 2.0
93 stars 51 forks source link

Upgrade to LTI 1.3 #5980

Closed joncameron closed 3 hours ago

joncameron commented 1 month ago

Description

Avalon is currently using the LTI 1.1 spec for auth with Learning Management Systems, which is no longer supported and officially deprecated.

LTI 1.3 and LTI Advantage are the current specs, and Avalon should be updated to use at least LTI 1.3 to stay current and avoid potentially breaking changes that could come at any time. LTI Advantage may not be necessary or at least out of scope for this particular upgrade.

LTI Spec Page: https://www.1edtech.org/standards/lti

Done Looks Like

masaball commented 3 weeks ago

Currently we use a fork of the omniauth-lti gem to handle authentication. The gem is set up for an OAuth1 workflow. It does not look like there is currently a replacement for this gem. Potentially easiest approach would be to update our fork to handle OAuth2/OpenID workflows? Though changes will probably be significant, so reworking the existing fork may not be that much less work than creating a new omniauth strategy from scratch. We would also need to update parameter names to reflect the new designations in LTI 1.3 (e.x. "lis_person_content_email_primary" becomes "email") There is a list of omniauth strategy gems that include examples of OAuth2/OpenID workflows: https://github.com/omniauth/omniauth/wiki/List-of-Strategies

Migration guide with param changes: https://www.imsglobal.org/spec/lti/v1p3/migr

The omniauth-lti gem relies on the ims-lti gem created/maintained by instructure. The ims-lti gem appears to be getting used for the actual OAuth/LTI request validation? It does not say anything in the README about LTI 1.3 support and there has been no response to this issue, but there is logic in lib/ims/lti/services/message_authenticator.rb that checks JWT and there is a service for an OAuth2 client. So the most recent version of ims-lti may support minimal processing of LTI 1.3, or they may be working on it.

We will need to update config/lti.yml to reflect the new configuration expectations for parameter names and auth flow. We will need to update the Devise initializer, /config/initializer/devise.rb. We may need to change lib/avalon/authentication.rb. We will likely need to update how LTI auth is configured in config/settings.yml to match the OAuth2 changes necessary in omniauth-lti.

I think that is what will be required for basic 1.3 authorization/launch. I am unsure at this point if we will need to have any of the functionality from LTI Advantage. If so, that will add more in depth work on handling Resource messages. If anything, we may need to support Deep Linking. Instructure has an internal gem for lti-advantage for Canvas that we could potentially take inspiration from when it comes to that, or use if they eventually spin it into its own repo: https://github.com/instructure/canvas-lms/tree/master/gems/lti-advantage/lib/lti_advantage.

joncameron commented 3 weeks ago

Investigation pretty much done; can be reviewed by @cjcolvar for a sanity check about Avalon's current implementation.

elynema commented 2 weeks ago

Response from edshelp@iu.edu:

cjcolvar commented 2 weeks ago

I think it might make sense to make a new omniauth gem with a new strategy that way it can be configured alongside the existing :lti strategy and wouldn't need to be a forced migration in the Avalon code but an opt-in to upgrade. This might mean more branching in Avalon code to handle both strategies/gems but I think it will make for easier migration instead of having to deal with it at the same time as upgrading Avalon.

joncameron commented 22 hours ago

New issue:

elynema commented 22 hours ago

This is probably not a requirement for implementation in 8.0. It's great that campus is excited to support us in the upgrade, and it would be great to leverage that. However, with support for 1.1 remaining indefinitely, this could be delayed if needed.

joncameron commented 3 hours ago

Created #6031 to represent the implementation work.