no need for csrf cookie anymore
Massimo Melina committed
Aug 9, 2023 at 18:24 UTC
82b1f458f0ed508b270e152296d3bb2117320ad0
4 files changed
+45
-74
shared/api.ts
+1
-8
@@ -26,15 +26,12 @@ export function setDefaultApiCallOptions(options: Partial<ApiCallOptions>) {
26
export function apiCall<T=any>(cmd: string, params?: Dict, options: ApiCallOptions={}) {
27
_.defaults(options, defaultApiCallOptions)
28
const stop = options.modal?.(cmd, params)
29
- const csrf = getCsrf()
30
- if (csrf)
31
- params = { csrf, ...params }
29
const controller = new AbortController()
30
if (options.timeout !== false)
31
setTimeout(() => controller.abort('timeout'), 1000*(timeoutByApi[cmd] ?? options.timeout ?? 10))
32
return Object.assign(fetch(getPrefixUrl() + API_URL + cmd, {
33
method: 'POST',
37
- headers: { 'content-type': 'application/json' },
34
+ headers: { 'content-type': 'application/json', 'x-hfs-anti-csrf': '1' },
35
signal: controller.signal,
36
body: params && JSON.stringify(params),
37
}).then(async res => {
@@ -122,10 +119,6 @@ export function apiEvents(cmd: string, params: Dict, cb:EventHandler) {
119
return source
120
}
121
125
-function getCsrf() {
126
- return getCookie('csrf')
127
-}
128
-
122
export function useApiEvents(cmd: string, params: Dict={}) {
123
const [data, setData] = useStateMounted<any>(undefined)
124
const [error, setError] = useStateMounted<any>(undefined)
src/api.auth.ts
-3
@@ -8,7 +8,6 @@ import {
8
ADMIN_URI,
9
HTTP_UNAUTHORIZED, HTTP_BAD_REQUEST, HTTP_SERVER_ERROR, HTTP_NOT_ACCEPTABLE, HTTP_CONFLICT, HTTP_NOT_FOUND
10
} from './const'
11
-import { randomId } from './misc'
11
import Koa from 'koa'
12
import { changeSrpHelper, changePasswordHelper } from './api.helpers'
13
import { ctxAdminAccess } from './adminApis'
@@ -26,12 +25,10 @@ async function loggedIn(ctx:Koa.Context, username: string | false) {
25
return ctx.throw(HTTP_SERVER_ERROR,'session')
26
if (username === false) {
27
delete s.username
29
- ctx.cookies.set('csrf', '')
28
return
29
}
30
s.username = normalizeUsername(username)
31
await prepareState(ctx, async ()=>{}) // updating the state is necessary to send complete session data so that frontend shows admin button
34
- ctx.cookies.set('csrf', randomId(), { signed:false, httpOnly: false })
32
}
33
34
function makeExp() {
src/apiMiddleware.ts
+1
-10
@@ -28,15 +28,13 @@ export function apiMiddleware(apis: ApiHandlers) : Koa.Middleware {
28
const params = isPost ? ctx.params || {} : ctx.query
29
const apiName = ctx.path
30
console.debug('API', ctx.method, apiName, { ...params })
31
- const safe = postWithOriginMatchingHost() // POST is safe because browser will enforce SameSite cookie
31
+ const safe = isPost && ctx.get('x-hfs-anti-csrf') // POST is safe because browser will enforce SameSite cookie
32
|| apiName.startsWith('get_') // "get_" apis are safe because they make no change
33
if (!safe)
34
return send(HTTP_FOOL)
35
const apiFun = apis.hasOwnProperty(apiName) && apis[apiName]!
36
if (!apiFun)
37
return send(HTTP_NOT_FOUND, 'invalid api')
38
- if (isPost && ctx.cookies.get('csrf') !== params.csrf)
39
- return send(HTTP_UNAUTHORIZED, 'csrf')
38
// we don't rely on SameSite cookie option because it's https-only
39
let res
40
try {
@@ -75,13 +73,6 @@ export function apiMiddleware(apis: ApiHandlers) : Koa.Middleware {
73
ctx.body = body
74
ctx.status = status
75
}
78
-
79
- function postWithOriginMatchingHost() {
80
- if (!isPost) return false
81
- const origin = ctx.get('origin')
82
- return !origin // not a browser
83
- || origin.split('//')[1] === ctx.get('host') // browser's requests must come from the inside. Even when no credentials are necessary, we don't want other website to issue non-get actions without the user knowing
84
- }
76
}
77
}
78
tests/test.ts
+43
-53
@@ -1,7 +1,6 @@
1
import axios, { AxiosRequestConfig } from 'axios'
2
import { wrapper } from 'axios-cookiejar-support'
3
import { CookieJar } from 'tough-cookie'
4
-import { Done } from 'mocha'
4
import { srpSequence } from '@hfs/shared/srp'
5
import { createReadStream, rmSync } from 'fs'
6
import { join } from 'path'
@@ -24,7 +23,7 @@ const client = wrapper(axios.create({ jar, maxRedirects: 0 }))
23
24
describe('basics', () => {
25
//before(async () => appStarted)
27
- it('frontend', req('/', /<body>/))
26
+ it('frontend', req('/', /<body>/, { headers: { accept: '*/*' } })) // workaround: 'accept' is necessary when running server-for-test-dev, still don't know why
27
it('force slash', req('/f1', 302))
28
it('list', reqList('/f1/', { inList:['f2/', 'page'] }))
29
it('search', reqList('f1', { inList:['f2/'], outList:['page'] }, { search:'2' }))
@@ -110,7 +109,7 @@ describe('accounts', () => {
109
describe('after-login', () => {
110
before(() =>
111
srpSequence(username, password, (cmd: string, params: any) =>
113
- client.post(API+cmd, params).then(x => x.data))
112
+ reqApi(cmd, params, ()=>true)())
113
)
114
it('list protected', reqList('/for-admins/', { inList:['alfa.txt'] }))
115
testUpload('upload', 200)
@@ -142,61 +141,52 @@ type Tester = number
141
}
142
143
function req(methodUrl: string, test:Tester, requestOptions: AxiosRequestConfig<any>={}) {
145
- return (done:Done) => {
146
- const csrf = getCookie('csrf')
147
- if (csrf)
148
- Object.assign(requestOptions.data, { csrf })
149
-
150
- // all url starts with /, so if one doesn't it's because the method is prefixed
151
- const i = methodUrl.indexOf('/')
152
- const method = methodUrl.slice(0,i) || requestOptions?.data && 'POST' || 'GET'
153
- const url = BASE_URL+methodUrl.slice(i)
154
- client.request({ method, url, ...requestOptions })
155
- .then(process, process)
156
- .catch(err => {
157
- done(err)
158
- })
159
-
160
- function process(res:any) {
161
- //console.debug('sent', requestOptions, 'got', res instanceof Error ? String(res) : [res.status])
162
- if (test && test instanceof RegExp)
163
- test = { re:test }
164
- if (typeof test === 'number')
165
- test = { status: test }
166
- if (typeof test === 'object') {
167
- const { status, mime, re, inList, outList, length, permInList } = test
168
- const gotMime = res.headers?.['content-type']
169
- const gotStatus = (res.status|| res.response.status)
170
- const gotLength = res.headers?.['content-length']
171
- const err = mime && !gotMime?.startsWith(mime) && 'expected mime ' + mime + ' got ' + gotMime
172
- || status && gotStatus !== status && 'expected status ' + status + ' got ' + gotStatus
173
- || re && !(typeof res.data === 'string' && re.test(res.data)) && 'expected content '+String(re)+' got '+res.data
174
- || inList && !inList.every(x => isInList(res.data, x)) && 'expected in list '+inList
175
- || outList && !outList.every(x => !isInList(res.data, x)) && 'expected not in list '+outList
176
- || permInList && findFirst(permInList, (v, k) => {
177
- const got = _.find(res.data.list, { n: k })?.p
178
- const negate = v[0] === '!'
179
- return findFirst(v.slice(negate ? 1 : 0).split(''), char =>
180
- got?.includes(char) === negate ? `expected perm ${v} on ${k}, got ${got}` : undefined)
181
- })
182
- || test.empty && res.data && 'expected empty body'
183
- || length !== undefined && gotLength !== String(length) && "expected content-length " + length + " got " + gotLength
184
- || test.cb?.(res.data, res) === false && 'error'
185
- || ''
186
- return done(err && Error(err))
187
- }
188
- const ok = test(res.data, res)
189
- done(!ok && Error())
144
+ // all url starts with /, so if one doesn't it's because the method is prefixed
145
+ const i = methodUrl.indexOf('/')
146
+ const method = methodUrl.slice(0,i) || requestOptions?.data && 'POST' || 'GET'
147
+ const url = BASE_URL+methodUrl.slice(i)
148
+ return () => client.request({ method, url, ...requestOptions })
149
+ .then(process, process)
150
+
151
+ function process(res:any) {
152
+ //console.debug('sent', requestOptions, 'got', res instanceof Error ? String(res) : [res.status])
153
+ if (test && test instanceof RegExp)
154
+ test = { re:test }
155
+ if (typeof test === 'number')
156
+ test = { status: test }
157
+ const { data } = res.response || res
158
+ if (typeof test === 'object') {
159
+ const { status, mime, re, inList, outList, length, permInList } = test
160
+ const gotMime = res.headers?.['content-type']
161
+ const gotStatus = (res.status|| res.response.status)
162
+ const gotLength = res.headers?.['content-length']
163
+ const err = mime && !gotMime?.startsWith(mime) && 'expected mime ' + mime + ' got ' + gotMime
164
+ || status && gotStatus !== status && 'expected status ' + status + ' got ' + gotStatus
165
+ || re && !(typeof data === 'string' && re.test(data)) && 'expected content '+String(re)+' got '+(data || '-empty-')
166
+ || inList && !inList.every(x => isInList(data, x)) && 'expected in list '+inList
167
+ || outList && !outList.every(x => !isInList(data, x)) && 'expected not in list '+outList
168
+ || permInList && findFirst(permInList, (v, k) => {
169
+ const got = _.find(data.list, { n: k })?.p
170
+ const negate = v[0] === '!'
171
+ return findFirst(v.slice(negate ? 1 : 0).split(''), char =>
172
+ got?.includes(char) === negate ? `expected perm ${v} on ${k}, got ${got}` : undefined)
173
+ })
174
+ || test.empty && data && 'expected empty body'
175
+ || length !== undefined && gotLength !== String(length) && "expected content-length " + length + " got " + gotLength
176
+ || test.cb?.(data, res) === false && 'error'
177
+ || ''
178
+ if (err)
179
+ throw Error(err)
180
}
181
+ if (typeof test === 'function')
182
+ if (!test(data, res))
183
+ throw Error()
184
+ return data
185
}
186
}
187
194
-function getCookie(k: string) {
195
- return jar.getCookiesSync(BASE_URL).find(c => c.key === k)?.value
196
-}
197
-
188
function reqApi(api: string, params: object, test:Tester) {
199
- return req(API+api, test, { data: params })
189
+ return req(API+api, test, { data: params, headers: { 'x-hfs-anti-csrf': '1'} })
190
}
191
192
function reqList(uri:string, tester:Tester, params?: object) {