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

Add CORS middleware handler to the API. #1215

Merged
merged 2 commits into from May 12, 2015
Merged

Add CORS middleware handler to the API. #1215

merged 2 commits into from May 12, 2015

Conversation

NodeGuy
Copy link

@NodeGuy NodeGuy commented May 9, 2015

Fixes #1017 and #1049.

@jbenet jbenet added the backlog label May 9, 2015
@@ -8,6 +8,8 @@ import (
"strconv"
"strings"

"github.com/rs/cors"
Copy link
Member

Choose a reason for hiding this comment

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

needs to be vendored. run make vendor in project root.

you may have to godep restore or manually go get dependencies for it to work correctly, i believe.

Copy link
Author

Choose a reason for hiding this comment

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

Done, thanks.

@jbenet
Copy link
Member

jbenet commented May 9, 2015

👍 for tests! this is a good change IMO. i wonder about the handler wrapper-- feels clunky to me. wonder if maybe we can use the parts of the library instead of the handler wrapper.

@cryptix PTAL?

@jbenet
Copy link
Member

jbenet commented May 12, 2015

Okay, i'll just merge this in.

jbenet added a commit that referenced this pull request May 12, 2015
Add CORS middleware handler to the API.
@jbenet jbenet merged commit ad9d84d into ipfs:master May 12, 2015
@jbenet jbenet removed the backlog label May 12, 2015
@wking wking mentioned this pull request Jun 12, 2015
@opn
Copy link

opn commented Jun 21, 2015

So what is the current status regarding CORS in IPFS? I'd like to enable simple wildcard CORS headers in the daemon, until such time an authenticated API is available. In particular I want to be serving Federated Wiki from IPFS, mixing content freely with other CORS enabled Fedwiki sites.

@jbenet
Copy link
Member

jbenet commented Jun 22, 2015

@opn this should be working, but as i'm curling the gateway and my local node, i'm not seeing the proper headers:

WIP, hit enter too early.

@jbenet
Copy link
Member

jbenet commented Jun 22, 2015

@opn you're right, we're missing CORS headers on the gateway. The above CORS headers were added to the API route only.

We need to add a CORS handler thing similar to this PR but for the HTTP gateway.

Relevant code:


> curl -X HEAD -I http://127.0.0.1:8080/ipfs/QmRwsK1Cgq44gtuigfU8VHveTY3aazYro73j8uZakDjpsR
HTTP/1.1 200 OK
Accept-Ranges: bytes
Cache-Control: public, max-age=29030400
Content-Length: 10
Content-Type: text/plain; charset=utf-8
Etag: QmRwsK1Cgq44gtuigfU8VHveTY3aazYro73j8uZakDjpsR
Last-Modified: Thu, 01 Jan 1970 00:00:01 GMT
Suborigin: QmRwsK1Cgq44gtuigfU8VHveTY3aazYro73j8uZakDjpsR
X-Ipfs-Path: /ipfs/QmRwsK1Cgq44gtuigfU8VHveTY3aazYro73j8uZakDjpsR
Date: Mon, 22 Jun 2015 01:37:14 GMT

The CORS header code for the webui is here:

@jbenet
Copy link
Member

jbenet commented Jun 22, 2015

(Also @opn, 👍 on getting federated wiki going on ipfs! We should try to get this CORS thing figured out for you this week, let us know if we can help with anything else on that front!)

@jbenet
Copy link
Member

jbenet commented Jun 22, 2015

cc @NodeGuy @krl @travisperson

Do we want the same CORS header setup for the gateway? Or something else? what are all the relevant security concerns here?


Sidenote: the origin problem is tangential, but separate. We hope to address that with per-page suborigins once they land, using subdomains in the meantime.

@NodeGuy
Copy link
Author

NodeGuy commented Jun 22, 2015

I don't see why not.

@travisperson
Copy link
Member

I think it would be fine, but I'm not completely up to speed on the security issues around CORS. I understand what it is and what it does and how to use it, but that's about it really.

I think it would be good to get @diasdavid thoughts on this as well.

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.

API_ORIGIN doesn't work
4 participants