[LTI Provider] Basic LTI authentication - #8347
Conversation
This change cleans up the work in progress request at https://github.com/edx/edx-platform/pull/8176 This is an initial authentication implementation that allows LTI users to log in transparently to edX. The behavior is driven by pilot users at Harvard; this was the most requested feature. The patch creates a new database model that maps users' LTI identifiers to newly-created edX accounts. If an LTI launch comes in with a user_id field that is not in the database, a new edX account is created with a random user name and password. This account is then stored in the database, so that it is permanently associated with the LTI user ID. This patch takes a simplistic approach to session management. If a user is logged in with a different account when they perform an LTI launch, they will be logged out and then re-logged in using their LTI account. In order to keep the patch simple, I have split out some refactoring that needs to be done into a separate branch that I'll post once this has been merged. Since we no longer redirect to the login page, we don't need to maintain two separate LTI endpoints (one for the LTI launch and one for authenticated users), or deal with the session management that requires. There are also multiple fetches of the LtiConsumer object (one in the view, one in the signature validation) that the later patch will consolidate into one.
|
Thanks for the pull request, @mcgachey! I've created OSPR-628 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:
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. |
openedx-webhooks
added
open-source-contribution
|
@sarina - that's right. I'll close off 8176 and point it here instead. |
There was a problem hiding this comment.
If the above try block actually fails, doesn't this fail as well (because edx_user would be undefined)?
There was a problem hiding this comment.
The try block is in a loop which won't exit until edx_user is created.
There was a problem hiding this comment.
Ack. Brain did not parse indentation. Thank you.
|
If I'm logged into the system as myself, then later I view a problem via LTI, am I still logged in as that LTI user when I go back to edx.org in my browser? |
|
You will be. We don't know when a user's done with their work from an LTI launch, so we can't log them out reliably. For the use case on a dedicated system that's OK, but I'm hoping that the working group who are looking into the more general case will have a better solution. |
|
Is there anyway to run the links into a sub-domain (i.e. so that the session is local to something like lti.courses.edx.org) and doesn't trample sessions for typical users? |
|
I don't know enough about the deployment setup to say. Is that something that would be configured in the code or when the server is run? |
|
@chrisndodge: When you're logged into a microsite with a completely different URL from edX, the sessions don't carry over, do they? |
|
Based on my research, sometimes they do and sometimes they don't. I'll chat On Thu, Jun 11, 2015 at 1:34 PM, David Ormsbee notifications@github.com
|
|
If the micro site is on subdomain, then yes, if it's on its own fully qualified hostname like mitprofessionalx.mit.edu then no. Sent from my iPhone
|
|
👍 |
|
Not an issue for this PR, but in the future, could you please try wrapping your commit messages at 72 lines? It just shows up nicer in some tools and in GitHub. |
[LTI Provider] Basic LTI authentication
|
Apologies. But this merge has broken master. Here's a link to the build. https://build.testeng.edx.org/job/edx-platform-all-tests-master-flow/746/ I can't tell you what the issue is...whether a rebase will solve it (or expose it for you) or if there needs to be more resilience in the related tests, etc.... but I will be reverting it shortly. |
|
@benpatterson - Please do go ahead and revert it; I hadn't realized that it would be merged tonight. Some existing tests were refactored in PR https://github.com/edx/edx-platform/pull/8240 that went in after this PR was submitted. I'll rebase and fix the conflicts in the morning. |
|
Solid. Thanks, @mcgachey |
6 participants
This change cleans up the work in progress request at https://github.com/edx/edx-platform/pull/8176
This is an initial authentication implementation that allows LTI users to log in transparently to
edX. The behavior is driven by pilot users at Harvard; this was the most requested feature.
The patch creates a new database model that maps users' LTI identifiers to newly-created edX
accounts. If an LTI launch comes in with a user_id field that is not in the database, a new edX
account is created with a random user name and password. This account is then stored in the database,
so that it is permanently associated with the LTI user ID.
This patch takes a simplistic approach to session management. If a user is logged in with a
different account when they perform an LTI launch, they will be logged out and then re-logged
in using their LTI account.
In order to keep the patch simple, I have split out some refactoring that needs to be done into
a separate branch that I'll post once this has been merged. Since we no longer redirect to the
login page, we don't need to maintain two separate LTI endpoints (one for the LTI launch and
one for authenticated users), or deal with the session management that requires. There are
also multiple fetches of the LtiConsumer object (one in the view, one in the signature
validation) that the later patch will consolidate into one.