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

Contribution offering: abstracted CORS configuration providers #15

Closed
bdunogier opened this issue Dec 12, 2013 · 5 comments
Closed

Contribution offering: abstracted CORS configuration providers #15

bdunogier opened this issue Dec 12, 2013 · 5 comments

Comments

@bdunogier
Copy link
Contributor

Contributed as #15

We are currently implementing CORS support into our REST API at http://github.com/ezsystems/ezpublish-kernel.

As it turns out, this bundle really does what needs to be done, and it does it well. We have a partially working implementation, but I'm thinking that I'd like to use NelmioCorsBundle instead of writing our own thing.

But the thing is: we need to make some stuff dynamic (methods list per URI, etc).

I was thinking that we could propose support for some cors_request_matcher service tag. Those services would provide a couple methods used to check a CORS request. A default one would be implemented that uses the bundle's semantical configuration as a source.

What do you think ? Would you be willing to accept such a contribution.

NB: there is also a need for a security request matcher in order to ignore auth on preflight requests. We should be able to contribute that as well.

@Seldaek
Copy link
Member

Seldaek commented Dec 13, 2013

Sounds reasonable so I'd say go for it. Not sure I got the point about the security request matcher though, IIRC the security component is not an issue because we register our listener with a higher prio than the security one, but I could be wrong on that.

@bdunogier
Copy link
Contributor Author

Nice, i'll work on the PR then !

About Security, there is a reasonable chance that we missed something. I'll see once the CorsBundle is integrated.

@lolautruche
Copy link

@bdunogier, @Seldaek is right. CorsListener has a priority of 10000, so far more than the security firewall 😄 . Furthermore, it does return a response when needed, so it will never go through the firewall. 👍

@stof
Copy link

stof commented Dec 13, 2013

regarding the CorsListener priority, it should be lower than that: it should run after the ProfilerListener, otherwise it could break it (this is not an issue anymore in 2.4 because the ProfilerListener has been refactored to use the RequestStack instead of managing its own stack)

@bdunogier
Copy link
Contributor Author

See #15 for the pull-request.

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

No branches or pull requests

4 participants