[heartbeat] add cors headers for heartbeat post

RESOLVED FIXED

Status

Input
General
P1
normal
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: Gregg Lind (Fx Strategy and Insights - Shield - Heartbeat ), Assigned: rrosario)

Tracking

Details

(Whiteboard: u=gregg c=heartbeat p= s=input.2015q1)

Attachments

(1 attachment)

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 +++
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!
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.
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.
Ooh shiny. That does seem to do all the things I think CORS needs. Carry on then.
willkg, if you feel comfy with this, then please CORS wrap that, and holler when it lands :)  I need it for #1111016
Blocks: 1111016
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
linked test script (from browser) at:  https://gist.github.com/gregglind/8749e3b0658e28d18554
Created attachment 8555895 [details]
cors test https://gist.github.com/gregglind/8749e3b0658e28d18554

Canonical version will be at:  https://gist.github.com/gregglind/8749e3b0658e28d18554
(Assignee)

Comment 9

3 years ago
In a pull request:
https://github.com/mozilla/fjord/pull/460
https://github.com/mozilla/fjord/commit/ba498e36734fc2f78a98d7af1faef341ef08f065
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
Last Resolved: 3 years ago
Resolution: --- → FIXED
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.