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

Fixes #2 - Initial commit to add OAuth support #98

Closed jonespm closed 2 years ago

jonespm commented 3 years ago

This adds support for OAuth. There is some configuration needed for this to work.

Testing this app for LTI locally if you were doing that is kind of a pain. If you use the development mode. You need ngrok with 2 URL's as described on https://github.com/tl-its-umich-edu/canvas-app-explorer/wiki/Configuring-LTI-1.3. If you restart Ngrok you'll have to both change Canvas and the value in the .env. Alternatively you can use the docker-compose-openshift-test-yml which only needs a single port but doesn't auto reload.

To test the OAuth you'll have to generate an API key (scopes don't matter yet) and have the redirect URL setup as https:///oauth/oauth-callback

Then add the values of the developer key to settings the env in

# CANVAS_OAUTH_CLIENT_ID=

# (required) The client secret is the random string (secret) value of your Canvas developer key.
# CANVAS_OAUTH_CLIENT_SECRET=

# (required) The domain of your canvas instance (e.g. canvas.instructure.com)
# CANVAS_OAUTH_CANVAS_DOMAIN=

There are at least 2 big improvements than can be made on this

1) Write a custom AUTHENTICATION_BACKEND. Currently this uses the default ['django.contrib.auth.backends.ModelBackend'] and logs in the user through this backend. We should have a custom one for LTI. This will likely be a separate issue and is one we've wanted for other apps like MYLA as well. #119

This was a sample from one LTI project. https://github.com/wachjose88/django-lti-provider-auth/blob/master/lti_provider/backends.py Here was another: https://github.com/ccnmtl/django-lti-provider/blob/master/lti_provider/auth.py

2) Store the OAuth configuration in the database along with the LTI Tools. This app's LTI config in the database can support multi-tenant, however the OAuth config is only a single value defined in the settings. For this to be multi-tenant this will need to extend and reference the LTI's tool config model. #120

ssciolla commented 2 years ago

I think I'll have some time to look at this tomorrow.

jonespm commented 2 years ago

Thanks @ssciolla ! We're frozen for new features for user testing next week so no rush, whenever you have time. I'm available most of tomorrow too if you wanted to chat about anything. This looks simple but it was a lot of trial and error to get here and it very likely has a few adjustments beyond the issues I mentioned.

The next step of this is going to be using the token against the API #32 which I'm going to be working on next week.

zqian commented 2 years ago

@jonespm l'd like to join the chat on this. I had some problem getting the custom field values in #32