Skip to content

Commit 3c898a0

Browse files
committed
Do not send CSRF token for external requests
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.
1 parent a2d1ae4 commit 3c898a0

File tree

2 files changed

+56
-17
lines changed

2 files changed

+56
-17
lines changed

__tests__/fetch_request.js

Lines changed: 33 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,11 @@ describe('perform', () => {
2222

2323
const testRequest = new FetchRequest("get", "localhost")
2424
const testResponse = await testRequest.perform()
25-
25+
2626
expect(window.fetch).toHaveBeenCalledTimes(1)
2727
expect(window.fetch).toHaveBeenCalledWith("localhost", testRequest.fetchOptions)
2828
expect(testResponse).toStrictEqual(new FetchResponse(mockResponse))
29-
})
29+
})
3030

3131
test('request is performed with 401', async () => {
3232
const mockResponse = new Response(undefined, { status: 401, headers: {'WWW-Authenticate': 'https://localhost/login'}})
@@ -111,20 +111,20 @@ describe('header handling', () => {
111111
expect(customRequest.fetchOptions.headers)
112112
.toStrictEqual({ ...defaultHeaders, "Content-Type": 'any/thing'})
113113
})
114-
test('is not set by formData', () => {
114+
test('is not set by formData', () => {
115115
const formData = new FormData()
116116
formData.append("this", "value")
117117
const formDataRequest = new FetchRequest("get", "localhost", { body: formData })
118118
expect(formDataRequest.fetchOptions.headers)
119119
.toStrictEqual(defaultHeaders)
120120
})
121-
test('is set by file body', () => {
121+
test('is set by file body', () => {
122122
const file = new File(["contenxt"], "file.txt", { type: "text/plain" })
123123
const fileRequest = new FetchRequest("get", "localhost", { body: file })
124124
expect(fileRequest.fetchOptions.headers)
125-
.toStrictEqual({ ...defaultHeaders, "Content-Type": "text/plain"})
125+
.toStrictEqual({ ...defaultHeaders, "Content-Type": "text/plain"})
126126
})
127-
test('is set by json body', () => {
127+
test('is set by json body', () => {
128128
const jsonRequest = new FetchRequest("get", "localhost", { body: { some: "json"} })
129129
expect(jsonRequest.fetchOptions.headers)
130130
.toStrictEqual({ ...defaultHeaders, "Content-Type": "application/json"})
@@ -138,13 +138,13 @@ describe('header handling', () => {
138138
request.addHeader("test", "header")
139139
expect(request.fetchOptions.headers)
140140
.toStrictEqual({ ...defaultHeaders, custom: "Header", "Content-Type": "application/json", "test": "header"})
141-
})
141+
})
142142

143143
test('headers win over contentType', () => {
144144
const request = new FetchRequest("get", "localhost", { contentType: "application/json", headers: { "Content-Type": "this/overwrites" } })
145145
expect(request.fetchOptions.headers)
146146
.toStrictEqual({ ...defaultHeaders, "Content-Type": "this/overwrites"})
147-
})
147+
})
148148

149149
test('serializes JSON to String', () => {
150150
const jsonBody = { some: "json" }
@@ -169,7 +169,7 @@ describe('header handling', () => {
169169
request = new FetchRequest("get", "localhost", { redirect })
170170
expect(request.fetchOptions.redirect).toBe(redirect)
171171
}
172-
172+
173173
request = new FetchRequest("get", "localhost")
174174
expect(request.fetchOptions.redirect).toBe("follow")
175175
})
@@ -191,7 +191,30 @@ describe('header handling', () => {
191191
// has no effect
192192
request = new FetchRequest("get", "localhost", { credentials: "omit"})
193193
expect(request.fetchOptions.credentials).toBe('same-origin')
194-
})
194+
})
195+
196+
describe('csrf token inclusion', () => {
197+
// window.location.hostname is "localhost" in the test suite
198+
test('csrf token is not included in headers if url hostname is not the same as window.location', () => {
199+
const request = new FetchRequest("get", "http://removeservice.com/test.json")
200+
expect(request.fetchOptions.headers).not.toHaveProperty("X-CSRF-Token")
201+
})
202+
203+
test('csrf token is included in headers if url hostname is the same as window.location', () => {
204+
const request = new FetchRequest("get", "http://localhost/test.json")
205+
expect(request.fetchOptions.headers).toHaveProperty("X-CSRF-Token")
206+
})
207+
208+
test('csrf token is included if url is a realative path', async () => {
209+
const defaultRequest = new FetchRequest("get", "/somepath")
210+
expect(defaultRequest.fetchOptions.headers).toHaveProperty("X-CSRF-Token")
211+
})
212+
213+
test('csrf token is included if url is not parseable', async () => {
214+
const defaultRequest = new FetchRequest("get", "not-a-url")
215+
expect(defaultRequest.fetchOptions.headers).toHaveProperty("X-CSRF-Token")
216+
})
217+
})
195218
})
196219

197220
describe('query params are parsed', () => {

src/fetch_request.js

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,18 @@ export class FetchRequest {
3838
this.options.headers = headers
3939
}
4040

41+
sameHostname () {
42+
if (!this.originalUrl.startsWith('http:')) {
43+
return true
44+
}
45+
46+
try {
47+
return new URL(this.originalUrl).hostname === window.location.hostname
48+
} catch (_) {
49+
return true
50+
}
51+
}
52+
4153
get fetchOptions () {
4254
return {
4355
method: this.method.toUpperCase(),
@@ -50,14 +62,18 @@ export class FetchRequest {
5062
}
5163

5264
get headers () {
65+
const baseHeaders = {
66+
'X-Requested-With': 'XMLHttpRequest',
67+
'Content-Type': this.contentType,
68+
Accept: this.accept
69+
}
70+
71+
if (this.sameHostname()) {
72+
baseHeaders['X-CSRF-Token'] = this.csrfToken
73+
}
74+
5375
return compact(
54-
Object.assign({
55-
'X-Requested-With': 'XMLHttpRequest',
56-
'X-CSRF-Token': this.csrfToken,
57-
'Content-Type': this.contentType,
58-
Accept: this.accept
59-
},
60-
this.additionalHeaders)
76+
Object.assign(baseHeaders, this.additionalHeaders)
6177
)
6278
}
6379

0 commit comments

Comments
 (0)