Do not send CSRF token for external requests #46
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This library could be used for any AJAX request, however it was reported that some 3rd party endpoints reject the request if the CSRF token is included in the headers. This change excludes the CSRF token from the headers by comparing the request URL and `window.location.hostname`. - If the URL is a relative path (ie doesn't begin with "http:"), include the token. - If the URL is not parseable with `new URL()`, then we'll continue on and include the token. - If the hostname of the URL and from `window.location` are equal, include the token. - If the hostname of the URL and from `window.location` are different, do not include the token. I beleive this covers and maintains the existing expectations of the library so existing applications shouldn't be caught off gaurd as we are including the token more often than not.
t27duck
force-pushed
the
no_csrf_for_external
branch
from
909de71 to
3c898a0
Compare
t27duck
commented
Jun 16, 2022
t27duck
commented
Jun 16, 2022
| return new URL(this.originalUrl).hostname === window.location.hostname | ||
| } catch (_) { | ||
| return true | ||
| } |
There was a problem hiding this comment.
This feels ugly, but new URL() bombs on relative paths and invalid URLs. Rather have soemthing further down the line fail because of a bad URL than it throwing an error here.
marcelolx
approved these changes
Jun 20, 2022
There was a problem hiding this comment.
@adrienpoly Did you have a chance to test @t27duck changes if they solve your problem? I believe it does, but let us know if it doesn't.
Thanks for the PR @t27duck!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This library could be used for any AJAX request, however it was reported that some 3rd party endpoints reject the request if the CSRF token is included in the headers.
This change excludes the CSRF token from the headers by comparing the request URL and
window.location.hostname.new URL(), then we'll continue on and include the token.window.locationare equal, include the token.window.locationare different, do not include the token.I beleive this covers and maintains the existing expectations of the library so existing applications shouldn't be caught off gaurd as we are including the token more often than not.
Closes #45