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

Route level CORS config overrides connection level defaults #2980

Closed
csrl opened this issue Dec 17, 2015 · 1 comment
Closed

Route level CORS config overrides connection level defaults #2980

csrl opened this issue Dec 17, 2015 · 1 comment
Assignees
Milestone

Comments

@csrl
Copy link

@csrl csrl commented Dec 17, 2015

It seems that the server option's route defaults are no longer used when route specific config options are present. v10.5.0 worked as expected, and v11.0.0+ does not.

var Hapi = require('hapi');
var server = new Hapi.Server({
  connections:{routes:{cors:{credentials:true}}}
});
server.connection({host:'localhost', port:8080});
server.route({
  method:'GET',
  path:'/session',
  config:{
    handler: function (request, reply) { reply(); }
  },
});
server.start(function () {});
$ curl -X GET http://localhost:8080/session -H 'Origin: mydomain.com' -H 'Access-Control-Request-Method: "GET"' -v
Note: Unnecessary use of -X or --request, GET is already inferred.
*   Trying ::1...
* connect to ::1 port 8080 failed: Connection refused
*   Trying 127.0.0.1...
* Connected to localhost (127.0.0.1) port 8080 (#0)
> GET /session HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/7.45.0
> Accept: */*
> Origin: mydomain.com
> Access-Control-Request-Method: "GET"
> 
< HTTP/1.1 200 OK
< vary: origin
< access-control-allow-origin: mydomain.com
< access-control-allow-credentials: true
< access-control-expose-headers: WWW-Authenticate,Server-Authorization
< cache-control: no-cache
< content-length: 0
< Date: Thu, 17 Dec 2015 06:01:52 GMT
< Connection: keep-alive
< 
* Connection #0 to host localhost left intact

But adding a 'cors' configuration object to the route, causes the 'access-control-allow-credentials' header to be dropped

var Hapi = require('hapi');
var server = new Hapi.Server({
  connections:{routes:{cors:{credentials:true}}}
});
server.connection({host:'localhost', port:8080});
server.route({
  method:'GET',
  path:'/session',
  config:{
    cors:{origin:['mydomain.com']},
    handler: function (request, reply) { reply(); }
  },
});
server.start(function () {});
$ curl -X GET http://localhost:8080/session -H 'Origin: mydomain.com' -H 'Access-Control-Request-Method: "GET"' -v
Note: Unnecessary use of -X or --request, GET is already inferred.
*   Trying ::1...
* connect to ::1 port 8080 failed: Connection refused
*   Trying 127.0.0.1...
* Connected to localhost (127.0.0.1) port 8080 (#0)
> GET /session HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/7.45.0
> Accept: */*
> Origin: mydomain.com
> Access-Control-Request-Method: "GET"
> 
< HTTP/1.1 200 OK
< vary: origin
< access-control-allow-origin: mydomain.com
< access-control-expose-headers: WWW-Authenticate,Server-Authorization
< cache-control: no-cache
< content-length: 0
< Date: Thu, 17 Dec 2015 06:02:48 GMT
< Connection: keep-alive
< 
* Connection #0 to host localhost left intact

Is this intentional? I much prefer being able to have default values in place and explicitly override specific keys, rather than have to respecify all values for a route where only one of them needs customized.

@hueniverse hueniverse added the bug label Dec 27, 2015
@hueniverse hueniverse added this to the 12.0.0 milestone Dec 27, 2015
@hueniverse hueniverse self-assigned this Dec 27, 2015
@hueniverse hueniverse changed the title CORS regression in hapi v11 ? Route level CORS config overrides connection level defaults Dec 27, 2015
@hueniverse
Copy link
Contributor

@hueniverse hueniverse commented Dec 27, 2015

The flip scenario is much more problematic because allowing credentials on a single route will override the connection origin default and change it to '*' which is a security hole. It is not a critical one but in some configuration would open an API to CSRF attacks.

You should install v11.1.4 or newer if you combine server level, connection level, or route level CORS configuration to ensure the result is as-expected (each level applied to the previous). This bug caused each level to override the previous causing the higher level defaults to be ignored. When the higher level config includes security restrictions (like origin), those restrictions can be overridden by less restrictive defaults (e.g. origin defaults to all origins).

@hueniverse hueniverse removed this from the 12.0.0 milestone Dec 27, 2015
@hueniverse hueniverse added this to the 11.1.4 milestone Dec 27, 2015
hueniverse added a commit that referenced this issue Dec 27, 2015
Fix cors config cascade. Closes #2980
@lock lock bot locked as resolved and limited conversation to collaborators Jan 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants