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

Add Access Control, CORS (Cross Origin Request Sharing) header methods #1699

Merged
merged 2 commits into from Jan 14, 2020

Conversation

pgjones
Copy link
Member

@pgjones pgjones commented Jan 11, 2020

This should make it a little easier to get and set access control headers as it ensures the types and naming is correct. It is also intentionally very minimal like the other header accessors.

closes #131

@pgjones pgjones mentioned this pull request Jan 11, 2020
src/werkzeug/wrappers/cors.py Outdated Show resolved Hide resolved
src/werkzeug/wrappers/cors.py Outdated Show resolved Hide resolved
src/werkzeug/wrappers/request.py Outdated Show resolved Hide resolved
tests/test_wrappers.py Outdated Show resolved Hide resolved
@davidism davidism added this to the 1.0.0 milestone Jan 12, 2020
src/werkzeug/wrappers/cors.py Outdated Show resolved Hide resolved
@davidism
Copy link
Member

@davidism davidism commented Jan 12, 2020

Similar to the CSP implementation, I'm wondering if we should have a single access_control property that returns an AccessControl object instead. Or maybe we need some sort of equivalent to EtagMixin.make_conditional that adjusts the response based on the request, since that's where the complexity lies.

I checked out Flask-CORS, somewhat discouraging to see that they're using str.split instead of the Werkzeug header parsing utilities already. While I'm fine with adding the header properties, I'm not sure how useful on its own that will be.

@pgjones
Copy link
Member Author

@pgjones pgjones commented Jan 14, 2020

I went for a single access_control property and object in Quart, however I don't think it is the right decision as it isn't consistent with how I think Werkzeug works (headers are only combined within a wrapper object, including CSP which is a single header) and the AccessControl instance itself isn't very useful on its own (Quart usage is always [wrapper].access_control.[header]).

The correct header parsing and setting is in itself useful (in my view). Whilst I like something like make_cors, I don't think it fits with Werkzeug. I think make_conditional is possible using the information in the wrappers alone whereas cors needs some application decisions/inputs.

This should make it a little easier to get and set access control
headers as it ensures the types and naming is correct. It is also
intentionally very minimal like the other header accessors.
@davidism davidism merged commit 2b2663b into pallets:master Jan 14, 2020
12 checks passed
@pgjones pgjones deleted the access_control branch Jan 19, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants