Feature: LTI third-party-auth providers

[wmono]

Based on top of PR #8140, this change permits Learning Tools Interoperability (LTI) tool consumer applications to perform a session transfer into edX instances.

As the LTI login process is significantly different from other python-social auth providers (in particular, logins cannot be initiated by the application as there is no login page to redirect to) this auth provider has been added to third-party-auth instead.

Background reading: Basic LTI implementation guide

To test this change, first configure a LTI tool consumer in the Django admin interface, making up a consumer key and consumer secret as one would for an OAuth auth provider, then use the LTI tool consumer reference implementation to launch https://yourserver.example.com/auth/login/lti/ . The trailing slash is mandatory.

Unfortunately, the consumer/provider names can be awkward: edX is acting as an LTI tool provider -- that is, it is providing itself as a tool to an external tool consumer application -- but it does so by accepting that tool consumer as a third-party-auth auth provider.

We've been working with @ebporter on this change. Please note that this change is distinct from the similar work in PR #8347 as this one creates/associates fully functional users; the intent here is to use LTI as a session transfer mechanism, whereas the intent there is to embed edX into another application. Further work to merge these two approaches is under discussion.

[openedx-webhooks]

Thanks for the pull request, @wmono! I've created OSPR-700 to keep track of it in JIRA. JIRA is a place for product owners to prioritize feature reviews by the engineering development teams.

Feel free to add as much of the following information to the ticket:

• supporting documentation
• edx-code email threads
• timeline information ('this must be merged by XX date', and why that is)
• partner information ('this is a course on edx.org')
• any other information that can help Product understand the context for the PR

All technical communication about the code itself will still be done via the Github pull request interface. As a reminder, our process documentation is here.

[wmono]

Please note that UBC has a blanket contributor agreement with edX and an individual contributor agreement is not required.

[sarina]

@wmono yep, we've got that noted! There's a merge conflict here and I strongly suspect it's with AUTHORS. Since that file changes so frequently, I would recommend you simply make a separate PR for the Update AUTHORS commit, so we can just quickly get that merged for you.

[sarina]

Nit: So that future import changes create a clean diff, I prefer doing

from some_lib import (
    thing1,
    thing2,
)

[sarina]

@wmono You have a few javascript test failures, and I know you're going to fail the quality checks.

See here for more info on quality testing

Here's how to run javascript tests locally

[wmono]

Er, how do I prevent this test timeout?

[wmono]

Jenkins, please rebuild?

[bradenmacdonald]

@wmono FYI you can now ask Jenkins to re-run each individual part (e.g. just re-run bok choy): https://openedx.atlassian.net/wiki/display/TE/Jenkins+Guide

[bradenmacdonald]

@wmono et al. I think this PR needs to be updated to not [necessarily] store the lti_consumer_secret in the DB, as was done recently for other third party auth providers - see the now-public security fix at https://github.com/edx/edx-platform/pull/9471 (CVE-2015-6671). There is concern that storing secrets in the DB increases surface area for attacks in the event that a DB/backup/replica is compromised. (If for some reason that's not a concern, it should be documented here.)

[cpennington]

@sarina @wmono I agree with @bradenmacdonald, and also would like the fixups squashed, and then I think it's ready to be merged.

[sarina]

@wmono Once you have responded to Braden and Cale's comments, please make a comment on this PR to ask them to look again - remember that we do not get notifications for new commits pushed, only for comments made.

[wmono]

@bradenmacdonald Does this change resolve your security concern?

[bradenmacdonald]

@wmono The code looks good. I'm not able to test it right now but I can do that later if needed.

[wmono]

jenkins run bokchoy

[wmono]

jenkins run bokchoy

[bradenmacdonald]

@wmono Just went to test this, but the security fix is not working for me. I think you need blank=True on the field in your model definition.

[bradenmacdonald]

@wmono I added blank=True locally and then everything was working fine, so my +1 stands if you fix that little issue and confirm yourself that it's working whether or not the key is in the database.

[wmono]

jenkins run bokchoy

[sarina]

jenkins run bokchoy last build failure

[cpennington]

@wmono: Can you write up a documentation on how this needs to be configured? @feanil, I'm going to merge this, since it's passing builds, but I think it's going to need ConfigModel and secure keys changes to go live.

[feanil]

Good to know. Thanks Cale. @ebporter I'll follow up with you to figure out the details of going live with this code.

[sarina]

@feanil can you tag me on any ops tickets re the configuration work needed to get this live?

[cpennington]

In retrospect, looking at this model, and running through the configuration on stage, could/should we replace these two fields with a foreign key to the existing LTI provider table?

[jibsheet]

If this is really just a copy/paste of the Key/Secret from the LTI Consumer page, then absolutely, this should be a dropdown list of LTI Consumers that you pick from. It also shouldn't generate a suggested lti_consumer_secret (default=long_token) since you're using one you already generated.

This lti_consumer_secret does all the work needed to save the secret to disk (rather than DB) but the other LTI Consumer does none of it, so we'd potentially have the secret in the DB for that table and on disk for this table, which seems strange.

@wmono Can you confirm if an LTI SSO secret/key pair should be identical to the keypair used for the consumer or if they must/should be different?

[wmono]

@jibsheet If you are absolutely certain that you will never have a different set of LTI TCs between the two ways of consuming edX, then yes, you can make this change if you like. Note that the level of trust involved is quite different: with this one, the TC causes a sign-in of a real user, while the other one creates a separate user.

[rukhsarahmad-gep]

need help on edx.org integration with litmos. please share link if you have. is it straight forward configuration on UI? or we have to call REST API to get authentication & display course list?

[nedbat]

@RukhsarAhmad Commenting on pull requests isn't a good way to get help. See here for options: https://open.edx.org/getting-help