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

Drop unhelpful note about CSRF from CORS doc #4161

Merged
merged 1 commit into from
May 19, 2021

Conversation

sideshowbarker
Copy link
Member

@sideshowbarker sideshowbarker commented Apr 16, 2021

If we’re going to mention CSRF in the CORS article, we would need to say quite a lot more than what this note says. And since this note is wrongly saying that you don’t need to worry at all about CSRF in a specific subset of cases (simple requests), it may give the very wrong impression that CORS overall doesn’t create any additional CSRF risk.

So it’s better to mention nothing at all about CSRF in this CORS article than it is to have this one mention in this one odd note.

Fixes #4111

@sideshowbarker sideshowbarker requested a review from a team as a code owner April 16, 2021 09:29
@sideshowbarker sideshowbarker requested review from mirunacurtean and removed request for a team April 16, 2021 09:29
If we’re going to mention CSRF in the CORS article, we would need to say
quite a lot more than what this note says. And since this note is
wrongly saying that you don’t need to worry at all about CSRF in a
specific subset of cases (simple requests), it may give the very wrong
impression that CORS overall doesn’t create any additional CSRF risk.

So it’s better to mention nothing at all about CSRF in this CORS article
than it is to have this one mention in this one odd note.

Fixes #4111
@sideshowbarker sideshowbarker force-pushed the sideshowbarker/cors-drop-csrf-note branch from 07490f0 to d95f0fa Compare April 16, 2021 09:29
@sideshowbarker sideshowbarker requested review from a team and hamishwillee April 16, 2021 09:30
@sideshowbarker
Copy link
Member Author

@hamishwillee Pinging you because I know you care about the CORS article and have invested a lot of work in it. Would be good to get your call on whether this note is serving any useful purpose. I understand what the intent was of whoever added it — but a “you don’t need to worry about CSRF” message, even for a specific limited context, doesn’t seem prudent. And I don’t see any way to salvage the note without spending a good chunk of time to write further text to clarify things.

@Elchi3 Elchi3 merged commit 83fa84b into main May 19, 2021
@Elchi3 Elchi3 deleted the sideshowbarker/cors-drop-csrf-note branch May 19, 2021 13:36
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants