Skip to content

Commit e6d1c01

Browse files
authored
Merge pull request #46 from t27duck/no_csrf_for_external
Do not send CSRF token for external requests
2 parents a2d1ae4 + 3c898a0 commit e6d1c01

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)