Skip to content

[LTI Provider] Basic LTI authentication - #8501

Merged
ormsbee merged 1 commit into
openedx:masterfrom
mcgachey:lti-auth-fix
Jun 12, 2015
Merged

[LTI Provider] Basic LTI authentication#8501
ormsbee merged 1 commit into
openedx:masterfrom
mcgachey:lti-auth-fix

Conversation

@mcgachey

Copy link
Copy Markdown
Contributor

This change cleans up the work in progress request at #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.

This branch fixes the previous conflicts with the test refactoring
carried out in PR #8240.

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.

This branch fixes the previous conflicts with the test refactoring
carried out in PR 8240.
@openedx-webhooks

Copy link
Copy Markdown

Thanks for the pull request, @mcgachey! I've created OSPR-646 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.

@sarina

sarina commented Jun 12, 2015

Copy link
Copy Markdown
Contributor

@mcgachey to confirm this is now rebased atop master and resolves hopefully the failures that were seen last night?

cc @ormsbee @benpatterson

@mcgachey

Copy link
Copy Markdown
Contributor Author

@sarina - That's right. This is a new branch spun off this morning with the same changes applied, and with fixes to the refactored tests.

@sarina

sarina commented Jun 12, 2015

Copy link
Copy Markdown
Contributor

LGTM. @ormsbee can you take a glance and then be responsible for merging? Thanks

@ormsbee

ormsbee commented Jun 12, 2015

Copy link
Copy Markdown
Contributor

Yup, 👍 again. I'll merge.

ormsbee pushed a commit that referenced this pull request Jun 12, 2015
[LTI Provider] Basic LTI authentication
@ormsbee
ormsbee merged commit 3ec2206 into openedx:master Jun 12, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

engineering review open-source-contribution PR author is not from Axim or 2U

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants