fix allowed origins to make webui work again #1544
Conversation
|
I would also love to have some tests to somehow make sure the webui works against each ipfs commit so we dont accidentaly break things as we move along. |
| "http://localhost", | ||
| "https://localhost", | ||
| "127.0.0.1:5001", | ||
| "localhost:5001", | ||
| } |
There was a problem hiding this comment.
We should probably get the API port from the config rather than hardcoding port 5001.
There was a problem hiding this comment.
👍 to @mappum's comment. should try to grab it from config.
i think this is the key part: adding the API port, not removing the protocol. this should work then:
var localhostOrigins = []string{
"http://127.0.0.1:<api-port>",
"https://127.0.0.1:<api-port>",
"http://localhost:<api-port>",
"https://localhost:<api-port>",
}There was a problem hiding this comment.
but the protocol isnt part of the url host
|
Other than my inline comment, this looks good. It's important to restrict access only to the API server, instead of all localhost origins/referers, otherwise attacks can still come from pages served over the local gateway (http://localhost:8080/ipfs/EVIL_PAGE). (This PR does that, the previous change did not). |
| for _, o := range cfg.CORSOpts.AllowedOrigins { | ||
| if o == "*" { // ok! you asked for it! | ||
| return true | ||
| } | ||
|
|
||
| if o == origin { // allowed explicitly |
There was a problem hiding this comment.
no, i think the protocol matters to CORS, one should be able to set CORS on https only 👎
jbenet
force-pushed
the
fix/allowed-origins
branch
3 times, most recently
from
9d06375
to
1d94d6f
Compare
ServeOptions take the node and muxer, they should get the listener too as sometimes they need to operate on the listener address. License: MIT Signed-off-by: Juan Batiz-Benet <[email protected]>
need to do it this way to avoid VERY confusing situations where the user would change the API port (to another port, or maybe even to :0). this way things dont break on the user, and by default, users only need to change the API address and things should still "just work" License: MIT Signed-off-by: Juan Batiz-Benet <[email protected]>
jbenet
force-pushed
the
fix/allowed-origins
branch
from
1d94d6f
to
3ee83a7
Compare
fix allowed origins to make webui work again
|
thanks for helping fix this @whyrusleeping 👍 |
|
@jbenet thanks for actually fixing it 😄 |
|
@whyrusleeping the api has a test suite that should probably be run on go-ipfs changes as well, i made the tests use browserify/phantomjs as well |
|
@krl that would be pretty sweet to have. How would we hook up those tests to the go-ipfs CI? |
fix allowed origins to make webui work again This commit was moved from ipfs/kubo@b19dd47
I'm not really sure what these changes mean, All I know is that the webui now works and I can add files through that interface again.
cc @mappum @diasdavid @jbenet and anyone else who can tell me if what i've done here is right.
License: MIT
Signed-off-by: Jeromy [email protected]