Implement Initial LTI 1.3 Launch#1635
Merged
Merged
Conversation
- fix error for no cuds for non-admin instructor
20wildmanj
commented
Nov 8, 2022
Member
damianhxy
reviewed
Nov 11, 2022
damianhxy
left a comment
Member
There was a problem hiding this comment.
Left some clarifying comments
Contributor
|
Was able to follow the testing steps. PR looks good to me, besides the few things @damianhxy mentioned. |
najclark
approved these changes
Nov 12, 2022
- rework lti_launch_initialize to have more readable logic - add user-id to nonce to prevent overwrite from multiple launches by different users
Contributor
Author
| skip_before_action :authenticate_for_action | ||
|
|
||
| # have to do because we are making a POST request from Canvas | ||
| skip_before_action :verify_authenticity_token |
Check failure
Code scanning / CodeQL
CSRF protection weakened or disabled
…ab into joeywildman-init-lti-setup
5 tasks
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


Description
This PR implements the LTI Advantage launch flow described in LMS Integration Design Doc V2.0. It is recommended to view "Implementation and Technical Details" as well as "LTI Advantage Launch Flow" sections. The full LTI 1.3 launch specification for tools such as Autolab can be found here.
Most of the code in this PR is based off the LTI 1.3 PHP library and the pylti1.3 library, specifically LTI_OIDC_Login.php, and LTI_Message_Launch.php.
Specific changes:
controllers/lti_launch_controller.rb/lti_launch/oidc_login/endpoint handles Step 1 and Step 2 of the launch flow (see design doc for more details),issparameter from platform matches our configuration settings,lti_message_hintexistsstate,nonceparams usingSecureRandom, withstatestored as a short lived cookie andnonceas a cache entry (to prevent replay attacks)lti_settings.yml), and redirect to authorization url of platform./lti_launch/launch/endpoint handles Step 4 of the launch flow (see design doc for more details),lti_launch_initializestateandnoncereceived match the ones generated duringoidc_loginclient_idreceived from platform to that in configuration settingslti_launch_initializeinusers_controller.rbHow Has This Been Tested?
Go to /config and run:
cp lti_settings.yml.template lti_settings.ymlFill in
lti_settings.ymlenable cache by running
rails dev:cacheGo to the LTI Reference Tool main page
Go to "Manage Platforms"
Click on "Unclaimed Platforms"
Find the "Autolab Test" platform (ctrl-f)
Click on "View Platform". Make sure client id, OIDC Auth URL, platform public key matches the rails config file.
lti_settings.ymlinformation is correct, and try again.Types of changes
Checklist:
overcommit --install && overcommit --signto use pre-commit hook for linting-> will update documentation in a later PR
To do: figure out why "already logged in" auth error shows up and interrupts launch flow