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

Generate function never called when Vision route has CORS enabled #90

Closed
stongo opened this issue Aug 9, 2016 · 11 comments
Closed

Generate function never called when Vision route has CORS enabled #90

stongo opened this issue Aug 9, 2016 · 11 comments
Milestone

Comments

@stongo
Copy link
Contributor

@stongo stongo commented Aug 9, 2016

So I think found the reason why the crumb cookie isn't being set sometimes.

If you setup your server with CORS enabled globally but also using Vision like this:

const server = new Hapi.Server({
    connections: {
      routes: {
        cors: true
      }
    }
});
server.connection();
server.register([{
    register: require('vision'),
    options: Config.vision
}, {
    register: require('crumb'),
    options: Config.crumb
}])
...

And then proceed to create a few routes returning views, the view routes will never call Crumb's generate function, because https://github.com/hapijs/crumb/blob/master/lib/index.js#L83 will always fail unless CORS is explicitly turned off for the view route.
The reason being that request.route.settings.cors evaluates to true, but then no CORS headers are actually set with the view, so the origin header isn't set making request.info.cors.isOriginMatch fail here https://github.com/hapijs/hapi/blob/ed195fad213a9da0f0762271c4907f4218e2abaf/lib/cors.js#L177-L179

As far as I can see, it comes down to the user being aware that a view route can't have CORS enabled. @hueniverse do you think there's any way to work around this in code, or will the best solution be to document the heck out of it in Crumb?

@stongo stongo added this to the 6.0.3 milestone Aug 9, 2016
@hueniverse

This comment has been minimized.

Copy link
Member

@hueniverse hueniverse commented Aug 9, 2016

I'm not following the flow. Why are vision routes acting differently?

@stongo

This comment has been minimized.

Copy link
Contributor Author

@stongo stongo commented Aug 9, 2016

sorry @hueniverse you're right, it's not limited to vision routes, but to any route being access directly from browser when CORS is enabled.

so if CORS is set globally and one tries to access a route directly in browser for example, request.info.cors.isOriginMatch is always going to be false because the origin header isn't there.

this makes it so that a route can't be used in both CORS and non-CORS contexts when using crumb. maybe this is okay though, and I just need to clearly state that in crumb readme?

@hueniverse

This comment has been minimized.

Copy link
Member

@hueniverse hueniverse commented Aug 9, 2016

Directly you mean CURL?

@stongo

This comment has been minimized.

Copy link
Contributor Author

@stongo stongo commented Aug 9, 2016

CURL or in a browser

Here's the headers from chrome dev tools from a Vision route with CORS enabled

Request URL:http://localhost:8001/
Request Method:GET
Status Code:200 OK
Remote Address:127.0.0.1:8001

Response Headers
cache-control:no-cache
Connection:keep-alive
content-encoding:gzip
content-type:text/html; charset=utf-8
Date:Tue, 09 Aug 2016 18:02:02 GMT
Transfer-Encoding:chunked
vary:origin,accept-encoding

Request Headers
Accept:text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8
Accept-Encoding:gzip, deflate, sdch
Accept-Language:en-US,en;q=0.8
Cache-Control:max-age=0
Connection:keep-alive
Host:localhost:8001
Upgrade-Insecure-Requests:1
User-Agent:Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.103 Safari/537.36

vs

curl -vv -H "origin: http://localhost" 127.0.0.1:8001
* Rebuilt URL to: 127.0.0.1:8001/
*   Trying 127.0.0.1...
* Connected to 127.0.0.1 (127.0.0.1) port 8001 (#0)
> GET / HTTP/1.1
> Host: 127.0.0.1:8001
> User-Agent: curl/7.43.0
> Accept: */*
> origin: http://localhost
>
< HTTP/1.1 200 OK
< vary: origin
< access-control-allow-origin: http://localhost
< access-control-expose-headers: WWW-Authenticate,Server-Authorization
< set-cookie: crumb=xxxxxxxxxxxxxxxxxxxxxxxx; HttpOnly; Path=/
< cache-control: no-cache
< content-type: text/html; charset=utf-8
< content-length: 1638
< accept-ranges: bytes
< Date: Tue, 09 Aug 2016 17:38:16 GMT
< Connection: keep-alive
<
@stongo

This comment has been minimized.

Copy link
Contributor Author

@stongo stongo commented Aug 9, 2016

If I disable CORS for the same route, then crumb generate is called and crumb works as expected.

This came up when I was debugging someone's setup where the crumb cookie wasn't being set or added to the view context, and it was because CORS was enabled globally

@hueniverse

This comment has been minimized.

Copy link
Member

@hueniverse hueniverse commented Aug 9, 2016

Why isn't the browser sending the CORS headers?

@stongo

This comment has been minimized.

Copy link
Contributor Author

@stongo stongo commented Aug 9, 2016

I don't know honestly. Probably the root issue. I'll dig in to that

@stongo

This comment has been minimized.

Copy link
Contributor Author

@stongo stongo commented Aug 10, 2016

@hueniverse the browser will never use CORS for a single api request or for an initial html page as served by a Vision route for example.
"A resource makes a cross-origin HTTP request when it requests a resource from a different domain than the one which the first resource itself serves" - https://developer.mozilla.org/en-US/docs/Web/HTTP/Access_control_CORS
So that explains a route not being able to qualify a valid CORS request vs a same-origin request, as currently implemented.
I'm wondering if the lack of a origin header is enough to confirm it's a same-origin request in the context of crumb, but need to dig in to the implications of that a bit more.

@stongo

This comment has been minimized.

Copy link
Contributor Author

@stongo stongo commented Aug 10, 2016

this page outlines when the header origin is served https://wiki.mozilla.org/Security/Origin

@hueniverse

This comment has been minimized.

Copy link
Member

@hueniverse hueniverse commented Aug 12, 2016

I think that's the right change. Enforce CORS where Origin is present.

stongo added a commit that referenced this issue Aug 12, 2016
@stongo

This comment has been minimized.

Copy link
Contributor Author

@stongo stongo commented Aug 12, 2016

Closed by a11b358

@stongo stongo closed this Aug 12, 2016
@Marsup Marsup added support and removed question labels Sep 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.