Closed
Bug 1126030
Opened 10 years ago
Closed 10 years ago
[heartbeat] add cors headers for heartbeat post
Categories
(Input :: General, defect, P1)
Input
General
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: glind, Assigned: rrosario)
References
Details
(Whiteboard: u=gregg c=heartbeat p= s=input.2015q1)
Attachments
(1 file)
1.20 KB,
text/plain
|
Details |
I need to POST cross origin from the self repair server endpoints. I think this needs CORS or JSONP.
This is a new issue, surfaced because addons do XHR from chrome privs, and thus have no xs restrictions.
Other solutions welcome, and my understanding may be incomplete.
+++ This bug was initially created as a clone of Bug #1030526 +++
Comment 1•10 years ago
|
||
JSONP isn't appropriate for POST requests. The CORS protection mechanisms prevent the request from going through at all without proper headers via the preflight mechanism. Also, JSONP works by injecting <script> tags into the page, and there isn't anyway to make <script> tags do POST.
In Kitsune I tried to do CORS for a while by hand. For simple GET requests it is pretty easy and worked for a while. For POSTs it is more complex, and I recommend using a library. I ended up using django-cors-headers, and it works well for Kitsune. This is the commit where I switched to it: https://github.com/mozilla/kitsune/commit/91c40c4954c5816b8d7e00a19105a7a7979541d7
Hopefully that saves you some pain in the future!
Comment 2•10 years ago
|
||
Just to clarify, are you posting and getting CORS-related errors right now? I thought you said that you didn't need CORS enabled because it was happening in-browser.
Comment 3•10 years ago
|
||
Mike: We have a cors_enabled decorator which has been working fine so far. Having said that, I forget what issues you hit with the one you hand-rolled and why I didn't think they applied to us.
https://github.com/mozilla/fjord/blob/4229113df885e8bf6c2c81e3781d5424b7e3e8f3/fjord/base/utils.py#L437
Anyhow, it's easy to add our cors_enabled decorator around the HB API.
Comment 4•10 years ago
|
||
Ooh shiny. That does seem to do all the things I think CORS needs. Carry on then.
Reporter | ||
Comment 5•10 years ago
|
||
willkg, if you feel comfy with this, then please CORS wrap that, and holler when it lands :) I need it for #1111016
Blocks: 1111016
Comment 6•10 years ago
|
||
Fjord has a cors_enabled decorator in fjord/base/utils.py . I think we can use that and wrap the heartbeat APIView's dispatch method:
class HeartbeatV2API(APIView):
...
dispatch = cors_enabled(HeartbeatV2API.dispatch, ...)
or the as_view classmethod:
class HeartbeatV2API(APIView):
...
as_view = cors_enabled(HeartbeatV2API.as_view, ...)
Pretty sure one of those will work. The as_view is already being wrapped with csrf_exempt, so it might be a better one to wrap. However, that's a classmethod and we might hit difficulties wrapping classmethods since they're "special" in Python.
Whichever is easier should be the one we do.
We also need to test this. Might be easy to write a test that does the pre-flight and then verifies the headers in the response.
Passing this to Ricky because I'm out this morning. I also made it a P1 because it needs to land and get deployed today.
Assignee: willkg → rrosario
Priority: -- → P1
Whiteboard: u=gregg c=heartbeat p= s=input.2015q1
Reporter | ||
Comment 7•10 years ago
|
||
linked test script (from browser) at: https://gist.github.com/gregglind/8749e3b0658e28d18554
Reporter | ||
Comment 8•10 years ago
|
||
Canonical version will be at: https://gist.github.com/gregglind/8749e3b0658e28d18554
Assignee | ||
Comment 9•10 years ago
|
||
In a pull request:
https://github.com/mozilla/fjord/pull/460
Comment 10•10 years ago
|
||
Comment 11•10 years ago
|
||
I verified Gregg's script failed in production, pushed the code and then verified that Gregg's script now succeeds.
We should be all set now.
Marking as FIXED.
Thank you, Ricky!
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Updated•10 years ago
|
Summary: add cors headers for heartbeat post → [heartbeat] add cors headers for heartbeat post
You need to log in
before you can comment on or make changes to this bug.
Description
•