fix: checkOrigin headers check - #12632
Conversation
* fix: enforce check origin logic * address feedback
🦋 Changeset detectedLatest commit: 56597ff The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Before merging this, we might want to first prepare the That way, changesets will create the correct PR for releasing the backport, and we don't clash the We could name this backport tag as |
|
I don't think I understand what I have to do. I thought that adding |
| request.method === 'PUT' || | ||
| request.method === 'PATCH' || | ||
| request.method === 'DELETE') && | ||
| request.headers.get('origin') === url.origin; |
There was a problem hiding this comment.
Should we allow empty origins too?
There was a problem hiding this comment.
I thought checking the origin was the whole point of CSRP protection 😅 Regardless, this port from v5 is a security fix. We shouldn't change the logic in v4, but we can enhance the v5.
There was a problem hiding this comment.
It's checking that the origin matches to prevent CSRF. A browser will always send the origin header for those requests. The only reason I ask is because of webhooks and APIs that might be called directly, and users might want to still have available while protecting forms from CSRF. However I realise the webhooks could just send a different content type, so would be fine
I updated the |
Changes
This PR ports a fix of
checkOriginto v4Testing
CI should pass
Docs
N/A