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

Revert Graphql CORS from 2.4.1 #8399

Closed
Silarn opened this issue Feb 2, 2017 · 10 comments
Closed

Revert Graphql CORS from 2.4.1 #8399

Silarn opened this issue Feb 2, 2017 · 10 comments
Assignees
Labels
Component: Framework/Webapi USE ONLY for FRAMEWORK RELATED BUG! E.g If bug related to Catalog WEB API use just Catalog improvement Issue: Format is not valid Gate 1 Failed. Automatic verification of issue format is failed Priority: P0 This generally occurs in cases when the entire functionality is blocked. Progress: done Project: GraphQL

Comments

@Silarn
Copy link

Silarn commented Feb 2, 2017

Reverting Graphql CORS from 2.4.1 as it poses a security concern

I think the Admin Panel control is a business consideration, you guys like behavior, that's fine.
But my other comment on implementation is that the current implementation is wrong. (edited)

This implementation has some bugs:
We also check whether or not the domain is allowed, otherwise you'll have headers attached when you shouldn't./
Additionally some headers should be only be on OPTIONS some on the subsequent GraphQL request

These bugs can lead to security concerns so it's best to just revert and fix them in 2.4.2

#28561
#26425

@veloraven veloraven added 2.0.x bug report Component: Framework/Webapi USE ONLY for FRAMEWORK RELATED BUG! E.g If bug related to Catalog WEB API use just Catalog labels Feb 3, 2017
@misha-kotov
Copy link

Thanks for your feedback. I've added CORS headers support as an improvement item for our APIs (MAGETWO-64314)

In the meantime, I understand that headers are extensible. Pasting the following from elsewhere, but to be honest I'm not entirely sure if it'll help in this case or not:

app/code/Magento/Store/etc/di.xml:337
\Magento\Framework\App\Response\HeaderManager
Magento\Framework\App\Response\HeaderProvider\XssProtection

The idea is you inject a new "header provider" into the $headerProviderList parameter of Magento\Framework\App\Response\HeaderManager, and this header provider implements the interface \Magento\Framework\App\Response\HeaderProvider\HeaderProviderInterface, so it supplies the name and value of header and whether or not to add it.

@Silarn
Copy link
Author

Silarn commented Feb 8, 2017

Thanks for the tip. I've created a workaround in the Apache configuration for now, but this might be a better solution.

@piotrekkaminski
Copy link
Contributor

Thank you for your submission.

We recently made some changes to the way we process GitHub submissions to more quickly identify and respond to core code issues.

Feature Requests and Improvements should now be submitted to the new Magento 2 Feature Requests and Improvements forum (see details here).

We are closing this GitHub ticket and have moved your request to the new forum.

@meldron
Copy link

meldron commented Feb 13, 2017

@Silarn can you post the apache config workaround?

@Silarn
Copy link
Author

Silarn commented Feb 13, 2017

@meldron I'll post the workaround on the forum thread.

@thaddeusmt
Copy link

I have attempted to create an extension that allows CORS cross-domain requests without modifying your Apache and Nginx server configs. It handles the OPTIONS pre-flight request, and allows configuring the Origin domain in the Admin Configuration. Feel free to check it out, and let me know if I have implemented any part of the CORS protocol incorrectly (or if I have bugs in my Magento code!). Thanks

https://github.com/splashlab/magento-2-cors-requests

@Silarn
Copy link
Author

Silarn commented Jun 15, 2017

@thaddeusmt That looks like a great solution until/if/when they get around to implementing CORS in core, thanks!

I haven't pored over the module yet, but I plan to pull it and start testing soon.

@ghost
Copy link

ghost commented Feb 16, 2018

Is this actually closed in 2.3? I can't find any commits related to it.

@piotrekkaminski
Copy link
Contributor

@magento-engcom-team it was marked as Done however this was not addressed/fixed in 2.3.0 and
MAGETWO-64314 is still open.

@damienwebdev
Copy link
Member

I've followed @misha-kotov concept here and added a CORS package for both the GraphQl and REST APIs: https://github.com/graycoreio/magento2-cors

@magento-engcom-team magento-engcom-team changed the title API: CORS requests will fail without OPTIONS reponse Revert Graphql CORS from 2.4.1 Sep 3, 2020
@magento-engcom-team magento-engcom-team added the Priority: P0 This generally occurs in cases when the entire functionality is blocked. label Sep 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Framework/Webapi USE ONLY for FRAMEWORK RELATED BUG! E.g If bug related to Catalog WEB API use just Catalog improvement Issue: Format is not valid Gate 1 Failed. Automatic verification of issue format is failed Priority: P0 This generally occurs in cases when the entire functionality is blocked. Progress: done Project: GraphQL
Projects
None yet
Development

No branches or pull requests

9 participants