Skip to content

[LTI Provider] Fix bug preventing unenrolled users from accessing content - #8426

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

[LTI Provider] Fix bug preventing unenrolled users from accessing content#8426
ormsbee merged 1 commit into
openedx:masterfrom
mcgachey:mcgachey-lti-enrollment-fix

Conversation

@mcgachey

@mcgachey mcgachey commented Jun 9, 2015

Copy link
Copy Markdown
Contributor

Change https://github.com/edx/edx-platform/pull/8240 refactored the LTI
provider template rendering code and introduced an issue where a user was
required to be enrolled in a course before that course's content could be
accessed over LTI. According to the LTI design spec:
https://docs.google.com/document/d/185hdPvIxcKtiDOLjb4sTGovA_WYXWz5Cd79gCzQwBms
we delegate access control over LTI content to the LTI consumer, rather
than requiring that users enroll in edX courses explicitly (and that admins
keep edX and LTI provider enrollments consistent when students add or drop
courses).

This change fixes the immediate issue, which is disrupting the LTI Provider
pilot currently running at Harvard.

@openedx-webhooks

Copy link
Copy Markdown

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

@openedx-webhooks openedx-webhooks added open-source-contribution PR author is not from Axim or 2U needs triage labels Jun 9, 2015
@mcgachey

mcgachey commented Jun 9, 2015

Copy link
Copy Markdown
Contributor Author

Pinging @ormsbee and @nasthagiri

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make sure we don't break LTI for unenrolled students again, can you:

  1. rename this test to test_unenrolled_student (both here and in RenderXBlockTestMixin)
  2. update the implementation of this test to verify that an LTI unenrolled student can successfully access the endpoint. Something like:
        self.setup_course()
        self.setup_user(admin=False, enroll=False, login=True)
        self.verify_response()

Thanks.

…tent

Change https://github.com/edx/edx-platform/pull/8240 refactored the LTI
provider template rendering code and introduced an issue where a user was
required to be enrolled in a course before that course's content could be
accessed over LTI. According to the LTI design spec:
    https://docs.google.com/document/d/185hdPvIxcKtiDOLjb4sTGovA_WYXWz5Cd79gCzQwBms
we delegate access control over LTI content to the LTI consumer, rather
than requiring that users enroll in edX courses explicitly (and that admins
keep edX and LTI provider enrollemnts consistent when students add or drop
courses).

This change fixes the immediate issue, which is disrupting the LTI Provider
pilot currently running at Harvard.
@mcgachey
mcgachey force-pushed the mcgachey-lti-enrollment-fix branch from 1975037 to 0c7623d Compare June 9, 2015 03:02
@nasthagiri

Copy link
Copy Markdown
Contributor

👍 This is Ok for a short-term workaround to unblock further LTI work.
However, we need to address the inconsistent authorization check that is now introduced between the 2 xblock rendering endpoints. I will speak with Product to understand their perspective on course-enrollment checks. In the meantime, can you add a TODO statement in the code to this effect?

@ormsbee

ormsbee commented Jun 9, 2015

Copy link
Copy Markdown
Contributor

👍

@ormsbee

ormsbee commented Jun 9, 2015

Copy link
Copy Markdown
Contributor

test this please

@mcgachey

mcgachey commented Jun 9, 2015

Copy link
Copy Markdown
Contributor Author

Are we good to go on this change?

ormsbee pushed a commit that referenced this pull request Jun 9, 2015
[LTI Provider] Fix bug preventing unenrolled users from accessing content
@ormsbee
ormsbee merged commit 73b3af6 into openedx:master Jun 9, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs triage 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