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

Proposal: generate default config with cors.allow_origin = "*" #1061

Open
masipcat opened this issue Dec 7, 2020 · 5 comments
Open

Proposal: generate default config with cors.allow_origin = "*" #1061

masipcat opened this issue Dec 7, 2020 · 5 comments

Comments

@masipcat
Copy link
Contributor

masipcat commented Dec 7, 2020

The idea is to make easier to start developing with guillotina and a frontend. Right now guillotina returns a 401 (Unauthorized) when origin is not allowed, and because this is returned from a OPTIONS it's not possible to provide a nice message to distinguish the cors error from a authentication error.

I think most frameworks doesn't have CORS enabled by default and probably the CORS would be managed by a reverse proxy when guillotina is deployed in production, so I think making this opt-in would be better.

What do you think?

@masipcat
Copy link
Contributor Author

masipcat commented Dec 7, 2020

This is related to this #1004

Also, maybe we could change the 401 to 403 (Forbidden)

@vangheem
Copy link
Member

vangheem commented Dec 7, 2020

We had this very long ago but thought it was a bad idea to have default values that were insecure. Maybe the cookie cutter that creates settings makes this setting explicit?

@masipcat
Copy link
Contributor Author

masipcat commented Dec 8, 2020

We had this very long ago but thought it was a bad idea to have default values that were insecure.

oh ok

Maybe the cookie cutter that creates settings makes this setting explicit?

yes, that's the case https://github.com/plone/guillotina/blob/master/guillotina/cookiecutter/application/%7B%7Bcookiecutter.package_name%7D%7D/config.yaml#L23-L25

@vangheem
Copy link
Member

vangheem commented Dec 8, 2020

Maybe have the default insecure with a logging message warning about it?

@masipcat
Copy link
Contributor Author

masipcat commented Dec 8, 2020

Sounds good to me! I'll open a PR later

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

2 participants