Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added CORS library #63

Closed
wants to merge 1 commit into from
Closed

Added CORS library #63

wants to merge 1 commit into from

Conversation

alvinsiu
Copy link
Contributor

What are the relevant tickets?

#56

What's this PR do?

Adds django-cors-middleware library to allow CORS iframe request. This theoretically fixes the Safari issue referenced in #56 (we cannot reproduce this on our end).

Where should the reviewer start?

How should this be manually tested?

Any background context you want to provide?

Screenshots (if appropriate)

What GIF best describes this PR or how it makes you feel?

@bdero bdero temporarily deployed to mitodl-sga-lti-pr-63 August 10, 2016 14:07 Inactive
@pdpinch
Copy link
Member

pdpinch commented Aug 18, 2016

I restarted the build, since there were some issues with Travis last week.

@odlbot odlbot temporarily deployed to mitodl-sga-lti-staging-pr-63 August 22, 2016 13:30 Inactive
@odlbot odlbot temporarily deployed to mitodl-sga-lti-staging-pr-63 August 22, 2016 13:55 Inactive
@odlbot odlbot temporarily deployed to mitodl-sga-lti-staging-pr-63 August 22, 2016 13:56 Inactive
@odlbot odlbot temporarily deployed to mitodl-sga-lti-staging-pr-63 August 22, 2016 14:16 Inactive
@pdpinch
Copy link
Member

pdpinch commented Aug 22, 2016

Hmm, this still is failing for me in Safari (with a clean session).

Here's the server log when I request https://edge.edx.org/courses/MITx/TEST101/2013_Fall/courseware/487dae5b80404631a8b43475f3839cd5/3ed699ea56e344069ea06a2c74f4a125/ from Safari:

2016-08-22T17:56:25.073945+00:00 app[web.1]: {address space usage: 483778560 bytes/461MB} {rss usage: 42815488 bytes/40MB} [pid: 12|app: 0|req: 2/32] 10.95.237.20 () {54 vars in 1101 bytes} [Mon Aug 22 17:56:24 2016] POST / => generated 0 bytes in 891 msecs (HTTP/1.1 302) 6 headers in 412 bytes (1 switches on core 0)
2016-08-22T17:56:25.171937+00:00 app[web.1]: Could not find LTI launch parameters
2016-08-22T17:56:25.206426+00:00 app[web.1]: {address space usage: 486416384 bytes/463MB} {rss usage: 43581440 bytes/41MB} [pid: 20|app: 0|req: 7/33] 10.51.167.27 () {50 vars in 1067 bytes} [Mon Aug 22 17:56:25 2016] GET /view-assignment/1/1 => generated 0 bytes in 36 msecs (HTTP/1.1 403) 4 headers in 262 bytes (1 switches on core 0)
2016-08-22T17:56:25.076863+00:00 heroku[router]: at=info method=POST path="/" host=mitodl-sga-lti-staging-pr-63.herokuapp.com request_id=8425aba8-5dca-49ff-bc3a-b3c44338a90a fwd="18.189.18.73" dyno=web.1 connect=0ms service=893ms status=302 bytes=412
2016-08-22T17:56:25.227218+00:00 heroku[router]: at=info method=GET path="/view-assignment/1/1" host=mitodl-sga-lti-staging-pr-63.herokuapp.com request_id=fe5b2275-db82-4b1c-a738-a0782133e258 fwd="18.189.18.73" dyno=web.1 connect=0ms service=37ms status=403 bytes=262

On the other hand, this is the server log when I request the same URL from Chrome (private window):

2016-08-22T17:58:46.890299+00:00 app[web.1]: {address space usage: 486252544 bytes/463MB} {rss usage: 43282432 bytes/41MB} [pid: 18|app: 0|req: 8/34] 10.137.144.23 () {60 vars in 1210 bytes} [Mon Aug 22 17:58:46 2016] POST / => generated 0 bytes in 187 msecs (HTTP/1.1 302) 6 headers in 412 bytes (1 switches on core 0)
2016-08-22T17:58:46.992750+00:00 app[web.1]: {address space usage: 486379520 bytes/463MB} {rss usage: 43393024 bytes/41MB} [pid: 19|app: 0|req: 8/35] 10.13.217.57 () {56 vars in 1247 bytes} [Mon Aug 22 17:58:46 2016] GET /view-assignment/1/1 => generated 6552 bytes in 64 msecs (HTTP/1.1 200) 2 headers in 73 bytes (1 switches on core 0)

@pdpinch
Copy link
Member

pdpinch commented Aug 2, 2017

This didn't appear to fix this issue, but in the meantime we've instituted a workaround for Safari users.

@pdpinch pdpinch closed this Aug 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants