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

CORS pre-fetch not respecting per-route config #2491

Closed
jgauna opened this issue Apr 6, 2015 · 12 comments
Closed

CORS pre-fetch not respecting per-route config #2491

jgauna opened this issue Apr 6, 2015 · 12 comments
Assignees
Labels
breaking changes Change that can breaking existing code bug Bug or defect security Issue with security impact
Milestone

Comments

@jgauna
Copy link

jgauna commented Apr 6, 2015

I've been using HapiJs for some weeks and it's really great but I can't make it work with CORS. Right now I have method (POST) with:

        cors: {
            origin: ['*']
        },

And it's working great. I've added a new one (PUT) and it's not working:

XMLHttpRequest cannot load http://192.168.0.21:3000/updateOrder.... No 'Access-Control-Allow-Origin' header is present on the requested resource. Origin 'http://localhost' is therefore not allowed access. The response had HTTP status code 404

Also, I've opened this issue days ago in StackOverflow and didn't get any answer:

http://stackoverflow.com/questions/29424735/cant-use-authorization-in-hapi-js

Any ideas guys?

Thanks in advance

@MathieuLoutre
Copy link
Contributor

It seems to be a configuration problem. Can you post some code? Have you tried cors: true?

@Marsup
Copy link
Contributor

Marsup commented Apr 9, 2015

Hard to tell without more code.

@hueniverse
Copy link
Contributor

Not enough information.

@hueniverse hueniverse added the support Questions, discussions, and general support label Apr 16, 2015
@hueniverse hueniverse self-assigned this Apr 16, 2015
@dypsilon
Copy link
Contributor

Just for the history: the problem here seems to be that the browser is trying to prefly the CORS request and sends an OPTIONS request. Which is answered by Hapi with a 404. The solution is to enable CORS for the entire connection. See this #2256

@Marsup
Copy link
Contributor

Marsup commented Apr 20, 2015

I'd say having to enable CORS globally is not a good solution since you'd have to disable it on all the routes you don't want it on, this can lead to security issues.

@dypsilon
Copy link
Contributor

@Marsup this is a fair comment. Is there any other way to have automatic preflights with 8.x?

@Marsup
Copy link
Contributor

Marsup commented Apr 20, 2015

Based on

hapi/lib/connection.js

Lines 391 to 402 in 16c6373

this._router.special('options', new Route({
path: '/{p*}',
method: 'options',
config: {
auth: false, // Override any defaults
cors: this.settings.routes.cors,
handler: function (request, reply) {
return reply();
}
}
}, this, this.server.realm));
, I'd say something similar to that : a route with the same path, method OPTIONS, your CORS, and just a reply should work. Maybe hapi should take care of that if a route has CORS but no global CORS, thoughts @hueniverse ?

@saidimu
Copy link

saidimu commented Oct 6, 2015

@hueniverse This issue needs to be re-opened? Based on this thread I see no way to have per-route CORS. As @Marsup mentioned, global CORS doesn't seem like a good idea.

@devinivy
Copy link
Member

devinivy commented Oct 6, 2015

CORS should be configurable per connection and per route: http://hapijs.com/api#route-options

@saidimu
Copy link

saidimu commented Oct 6, 2015

This doesn't work in v10.3.0:

server.route({
  method: 'GET',
  path: '/books/get/{id?}',
  config: {
    cors: true,
  },
  handler: function(request, reply) {
    ...
  }
});

@hueniverse hueniverse reopened this Oct 6, 2015
@hueniverse hueniverse added bug Bug or defect and removed support Questions, discussions, and general support labels Oct 14, 2015
@hueniverse hueniverse changed the title CORS not working CORS pre-fetch not respecting per-route config Oct 14, 2015
@hueniverse hueniverse added security Issue with security impact breaking changes Change that can breaking existing code labels Oct 14, 2015
@hueniverse hueniverse added this to the 10.5.0 milestone Oct 14, 2015
@hueniverse
Copy link
Contributor

@Marsup can you review the change?

@Marsup
Copy link
Contributor

Marsup commented Oct 14, 2015

@hueniverse LGTM but I think we should handle Access-Control-Allow-Methods automatically.

@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.
Labels
breaking changes Change that can breaking existing code bug Bug or defect security Issue with security impact
Projects
None yet
Development

No branches or pull requests

7 participants