Closed Bug 986057 Opened 9 years ago Closed 9 years ago

Loop Server — Should accept cross origin requests

Categories

(Hello (Loop) :: Server, defect, P1)

x86_64
Linux
defect

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: alexis+bugs, Assigned: alexis+bugs)

References

Details

(Whiteboard: [s=mlpnightly1, p=.25][qa!])

Attachments

(2 files)

55 bytes, text/x-github-pull-request
rhubscher
: review+
Details | Review
55 bytes, text/x-github-pull-request
standard8
: review+
Details | Review
We should allow cross origin resource sharing for some URLs, that will be called from the WebApp.

To do that, add the following headers:

res.set("Access-Control-Allow-Origin", "*");
res.set("Access-Control-Allow-Methods", "GET,POST");
Attached file link to github PR
Attachment #8394445 - Flags: review?(rhubscher)
Assignee: nobody → alexis+bugs
Status: NEW → ASSIGNED
Uh, this is generally not a safe practice, since effectively this
removes all CSRF protection. Why can't you identify the correct
friendly origin URLs?

Please address this question prior to landing this patch.
Flags: needinfo?(alexis+bugs)
Basically, I was considering this URL unique and as one that can only be accessed by people who know it exists, so CSRF doesn't really seem a treat to me here.

Nevertheless, I agree it is possible to narrow the list of hosts that will expose the pages (theses are the CDNs) and probably that should be good practice, too.

I've updated the patch with a way to define the list of authorized domains in the settings file.
Flags: needinfo?(alexis+bugs)
Comment on attachment 8394445 [details] [review]
link to github PR

For now it is good enough, but we should add a list to decide how we want to handle multiple domain authorization.

See http://stackoverflow.com/questions/1653308/access-control-allow-origin-multiple-origin-domains
Attachment #8394445 - Flags: review?(rhubscher) → review+
https://github.com/mozilla-services/loop-server/commit/7698ebce3cc923fd1fb0fb433649b7692c016ba3
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
This doesn't seem like the right way to accept multiple domains, so I'm reopening this one until we fix that.

The link you pointed to says we should have the Allow-Origin match the Origin of the request that had been made, and now I remember that's the way it should be implemented.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
We should define the OPTION HTTP verb as well.

As NiKo` noticed they might be interresting though here: https://gist.github.com/cuppster/2344435
For GET / POST with JSON we should't need a preflight (OPTION) request to be made.

Why are you suggesting to answer to OPTION for this specific case? What's the request that's not working okay?
Actually, after reading back the CORS spec, it seems that we need to support OPTIONS if we want an accept-header different than one of application/x-www-form-urlencoded, multipart/form-data, or text/plain.
Severity: normal → blocker
Right, this bug is currently preventing from testing the Loop webapp using the server. Blocker.
Priority: -- → P1
Yes looks to be a great as possible :+1:
Attached file link to github PR
Attachment #8395618 - Flags: review?(standard8)
Comment on attachment 8395618 [details] [review]
link to github PR

Looks good, r=Standard8
Attachment #8395618 - Flags: review?(standard8) → review+
https://github.com/mozilla-services/loop-server/commit/83b58df712d15bab290f51977077b352f52b0423
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
Whiteboard: [s=mlpnightly1, p=.25]
Verified in code.
Status: RESOLVED → VERIFIED
QA Contact: jbonacci
Whiteboard: [s=mlpnightly1, p=.25] → [s=mlpnightly1, p=.25][qa!]
You need to log in before you can comment on or make changes to this bug.