Skip to content

Implement LTI Advantage NRPS GET request#1649

Merged
20wildmanj merged 56 commits into
masterfrom
joeywildman-nrps
Nov 27, 2022
Merged

Implement LTI Advantage NRPS GET request#1649
20wildmanj merged 56 commits into
masterfrom
joeywildman-nrps

Conversation

@20wildmanj

@20wildmanj 20wildmanj commented Nov 21, 2022

Copy link
Copy Markdown
Contributor

Description

  • Implements Oauth2 Access Token Request and subsequent LTI NRPS call in lti_nrps_controller.rb
  • update validate_jwt_signature in lti_launch_controller to call the platform_public_jwks_url if the configuration variable is set, to pull platform public jwks from an endpoint to get all possible public keys to verify the ID Token JWT from the platform
    • Platforms such as Canvas use a set of JWKs to sign their JWTs, so we need to have on hand all possible public JWKs
  • update .env.template to store Autolab's private key if on production
  • update misc HTTP headers, params to match what Canvas expects (the LTI testing tool is not comprehensive in checking all parameters)
  • Created lti_launch_initialize in users_controller to handle launching course linking page lti_launch_initialize.html.erb
  • Created lti_launch_link_course in users_controller to create a LtiCourseDatum representing the link between an LTI course and an Autolab course
  • Created LtiCourseDatum table to store course_id, context_id, last_synced, and membership_url to store data necessary for link between LTI course and Autolab course

Please take a look at the LMS Integration Design Doc, NRPS Query for more details.

How Has This Been Tested?

  • Go to /config and run:
    cp lti_settings.yml.template lti_settings.yml
  • Fill in lti_settings.yml for development
  • If you haven't linked the Autolab course you'd like to make the NRPS request from, execute a launch from the LTI testing tool
  • Once you have successfully launched, autolab, you should be redirected to a screen that allows you to choose a course:

Screen Shot 2022-11-25 at 3 08 02 PM

- After linking, you should get a confirmation flash

Screen Shot 2022-11-25 at 3 08 33 PM

  • Go to the course you linked, go to "Manage Course" and then "Manage Course Users"

Screen Shot 2022-11-25 at 3 09 33 PM

- Click the refresh button
  • You should see a JSON response similar to below
    Screen Shot 2022-11-23 at 14 56 02

NOTE: some things cannot be tested on the LTI Reference tool, such as filtering only for students, and for testing of pagination. These settings can only really be tested on Nightly with CMU Canvas as the platform.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have run rubocop for style check. If you haven't, run overcommit --install && overcommit --sign to use pre-commit hook for linting
  • I have updated the documentation accordingly, included in this PR

Other issues / help required

If unsure, feel free to submit first and we'll help you along.

- fix error for no cuds for non-admin instructor
…uest via Client-Credentials Grant

- Add lti_context_membership_url to course_user_data to store NRPS course membership url for NRPS requests
- Add placehold frontend to initiate requests
- remove amount of parameters sent to users controller
@20wildmanj 20wildmanj marked this pull request as ready for review November 25, 2022 22:52
@20wildmanj 20wildmanj requested a review from damianhxy November 25, 2022 22:52
@najclark najclark mentioned this pull request Nov 26, 2022
6 tasks
@damianhxy

damianhxy commented Nov 26, 2022

Copy link
Copy Markdown
Member

Successfully followed steps (However, had to set iss in lti_settings.yml to "https://lti-ri.imsglobal.org" instead (no trailing slash)
Screenshot 2022-11-26 at 15 44 20
Screenshot 2022-11-26 at 15 44 55

@damianhxy damianhxy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally lgtm, however unlink_course is currently vulnerable to CSRF attacks.

Left several comments, mostly consisting of nits

Comment thread .env.template Outdated
Comment thread .env.template Outdated
Comment thread config/routes.rb Outdated
Comment thread docs/installation/lti_integration.md
Comment thread docs/installation/lti_integration.md Outdated
Comment thread app/controllers/lti_launch_controller.rb Outdated
Comment thread app/controllers/lti_launch_controller.rb Outdated
Comment thread app/controllers/lti_nrps_controller.rb Outdated
Comment thread app/controllers/lti_nrps_controller.rb Outdated
Comment thread app/controllers/lti_nrps_controller.rb Outdated
@20wildmanj 20wildmanj changed the title [WIP] Implement LTI Advantage NRPS GET request Implement LTI Advantage NRPS GET request Nov 27, 2022
@najclark najclark requested a review from damianhxy November 27, 2022 22:01
Comment thread app/controllers/lti_launch_controller.rb Outdated

@damianhxy damianhxy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good, left one last comment but otherwise lgtm!

@20wildmanj 20wildmanj merged commit dff87e0 into master Nov 27, 2022
@20wildmanj 20wildmanj deleted the joeywildman-nrps branch November 27, 2022 23:28
@najclark najclark mentioned this pull request Nov 28, 2022
6 tasks
@damianhxy damianhxy mentioned this pull request Jan 4, 2024
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants