hypothesis / lms

LTI app for integrating with learning management systems
BSD 2-Clause "Simplified" License
46 stars 14 forks source link

Allow for "scoped" dev keys in Canvas #256

Closed jeremydean closed 5 years ago

jeremydean commented 6 years ago

In order to give the LMS app access to PDFs housed in a Canvas course's "files" repository, administrators installing the app must provide a Canvas dev key (sometimes token) when they acquire consumer key/secret for our app. These keys or tokens provide a lot of unnecessary access to a course/student data/etc and so Canvas has recently allowed them to be scoped, for example, to allow use of only specified Canvas APIs.

https://community.canvaslms.com/docs/DOC-14977-4214937573

Right now, our app does not work with a scoped dev key. We had thought originally that simply providing these API endpoints used would be sufficient:

  1. The list files API (to show the teacher the list of Canvas files to choose from): https://canvas.instructure.com/doc/api/files.html#method.files.api_index

  2. The inline preview URL API (to get a "public" URL for the file so we can show the file to the user and let them annotate it): https://canvas.instructure.com/doc/api/files.html#method.files.public_url

But in fact changes need to be made to the app itself. @robertknight explains more below in response to a request for this feature from University of Texas at Austin. It is not sufficient :

The problem I believe is that Hypothesis does not specify which APIs (scopes) it is going to need when requesting an OAuth access token. According to the Canvas docs [1] the app needs to specify the scopes when requesting an access token and these need to be a subset of those registered with the developer key. This is what the Canvas error message from Mario is telling us.

On that basis, the current status is that we do not support scoped keys. In order to support them we will need to:

  1. Specify the scopes when requesting an access token from canvas
  2. Document these scopes somewhere for the benefit of other users

[1] https://hyp.is/gUGvasCWEeizaduERuybMg/canvas.instructure.com/doc/api/file.oauth_endpoints.html

jeremydean commented 5 years ago

Just off a conversation with folks at UPenn. It was stated that UPenn was highly unlikely to approve the use of a dev key that was not scoped.

jeremydean commented 5 years ago

@klemay I'm wondering if we should add the API endpoints used by the app to the KB (either as its own entry or part of the one on creating dev keys? I get asked about it regularly and it'd be nice to just be able to point somewhere.

klemay commented 5 years ago

@jeremydean since the LTI parameters are documented as part of this repo's wiki, I would say documentation re: Canvas API calls would belong there too (and we could certainly link to that from the KB article you suggested).

I would be happy to write this up if one of the devs could check it for clarity/accuracy, then I could drop a link in the KB article.

I've added a card to the upcoming sprint, which starts on Thursday.

EDIT: We wrote this up: https://github.com/hypothesis/lms/wiki/Canvas-API-Endpoints-Used-by-the-Hypothesis-LMS-App

robertknight commented 5 years ago

This will be partly addressed by #500 and #502 in which we'll remove any knowledge from the client-side part of the LMS app about the LMS's file API works.

seanh commented 5 years ago

Some Canvas docs for this:

https://canvas.instructure.com/doc/api/file.developer_keys.html

https://canvas.instructure.com/doc/api/api_token_scopes.html

seanh commented 5 years ago

More docs: http://www.testout.com/docs/lms/testout-canvas-eduapp-integration-guide.pdf

seanh commented 5 years ago

This issue got a bit messy so I've written a new one to replace it, with the dev details that we'll need to implement this: https://github.com/hypothesis/lms/issues/806