don't require csrf token for GET requests #310
Massimo Melina committed
Aug 8, 2023 at 19:15 UTC
a1c1140544f2a5157fcf9b2ab5602364efbbd72d
16 files changed
+63
-66
admin/src/FilePicker.ts
+1
-1
@@ -42,7 +42,7 @@ export default function FilePicker({ onSelect, multiple=true, files=true, folder
42
}
43
}).finally(() => setReady(true))
44
}, [from])
45
- const { list, error, connecting } = useApiList<DirEntry>(ready && 'ls', { path: cwd, files, fileMask })
45
+ const { list, error, connecting } = useApiList<DirEntry>(ready && 'get_ls', { path: cwd, files, fileMask })
46
useEffect(() => {
47
setSel([])
48
setFilter('')
admin/src/LangPage.ts
+1
-1
@@ -11,7 +11,7 @@ import { alertDialog, toast } from './dialog'
11
import { Field, SelectField } from '@hfs/mui-grid-form';
12
13
export default function LangPage() {
14
- const { list, error, connecting, reload } = useApiList('list_langs')
14
+ const { list, error, connecting, reload } = useApiList('get_langs')
15
const langs = useMemo(() => ['en', ..._.uniq(list.map(x => x.code))], [list])
16
const large = useBreakpoint('md')
17
return error || h(Fragment, {},
admin/src/OnlinePlugins.ts
+1
-1
@@ -15,7 +15,7 @@ import { alertDialog } from './dialog'
15
export default function OnlinePlugins() {
16
const [search, setSearch] = useState('')
17
const debouncedSearch = useDebounce(search, 1000)
18
- const { list, error, initializing, updateList } = useApiList('search_online_plugins', { text: debouncedSearch })
18
+ const { list, error, initializing, updateList } = useApiList('get_online_plugins', { text: debouncedSearch })
19
const snap = useSnapState()
20
if (error)
21
return showError(error)
frontend/src/useFetchList.ts
+15
-20
@@ -11,41 +11,36 @@ import { ERRORS } from './misc'
11
import { t } from './i18n'
12
import { useLocation, useNavigate } from 'react-router-dom'
13
14
-const RELOADER_PROP = Symbol('reloader')
15
-
14
export function usePath() {
15
return useLocation().pathname
16
}
17
18
export default function useFetchList() {
19
const snap = useSnapState()
22
- const desiredPath = usePath()
20
+ const uri = usePath()
21
const search = snap.remoteSearch || undefined
24
- const lastPath = useRef('')
22
+ const lastUri = useRef('')
23
const lastReq = useRef<any>()
24
+ const lastReloader = useRef(snap.listReloader)
25
const isMounted = useIsMounted()
26
const navigate = useNavigate()
27
useEffect(()=>{
29
- const previous = lastPath.current
30
- lastPath.current = desiredPath
31
- if (previous !== desiredPath) {
28
+ const previous = lastUri.current
29
+ lastUri.current = uri
30
+ if (previous !== uri) {
31
state.showFilter = false
32
state.stopSearch?.()
33
}
34
state.stoppedSearch = false
36
- if (previous !== desiredPath && search) {
35
+ if (previous !== uri && search) {
36
state.remoteSearch = ''
37
return
38
}
39
41
- const params = {
42
- uri: desiredPath,
43
- search,
44
- sse: true,
45
- [RELOADER_PROP]: snap.listReloader, // symbol, so it won't be serialized, but will force reloading
46
- }
47
- if (_.isEqual(params, lastReq.current)) return
40
+ const params = { uri, search }
41
+ if (snap.listReloader === lastReloader.current && _.isEqual(params, lastReq.current)) return
42
lastReq.current = params
43
+ lastReloader.current = snap.listReloader
44
45
state.list = []
46
state.filteredList = undefined
@@ -62,7 +57,7 @@ export default function useFetchList() {
57
state.list = sort([...state.list, ...chunk])
58
}
59
const timer = setInterval(flush, 1000)
65
- const src = apiEvents('file_list', params, (type, data) => {
60
+ const src = apiEvents('get_file_list', params, (type, data) => {
61
if (!isMounted()) return
62
switch (type) {
63
case 'error':
@@ -80,7 +75,7 @@ export default function useFetchList() {
75
for (const entry of data) {
76
const [op, par] = entry
77
const error = op === 'error' && par
83
- if (error === 405) { // "method not allowed" happens when we try to directly access an unauthorized file, and we get a login prompt, and then file_list the file (because we didn't know it was file or folder)
78
+ if (error === 405) { // "method not allowed" happens when we try to directly access an unauthorized file, and we get a login prompt, and then get_file_list the file (because we didn't know it was file or folder)
79
state.messageOnly = t('upload_starting', "Your download should now start")
80
window.location.reload() // reload will start the download, because now we got authenticated
81
continue
@@ -94,8 +89,8 @@ export default function useFetchList() {
89
lastReq.current = null
90
continue
91
}
97
- if (!desiredPath.endsWith('/')) // now we know it was a folder for sure
98
- return navigate(desiredPath + '/')
92
+ if (!uri.endsWith('/')) // now we know it was a folder for sure
93
+ return navigate(uri + '/')
94
if (op === 'props') {
95
Object.assign(state, _.pick(par, ['can_upload', 'can_delete', 'accept']))
96
continue
@@ -116,7 +111,7 @@ export default function useFetchList() {
111
clearInterval(timer)
112
src.close()
113
}
119
- }, [desiredPath, search, snap.username, snap.listReloader, snap.loginRequired])
114
+ }, [uri, search, snap.username, snap.listReloader, snap.loginRequired])
115
}
116
117
export function reloadList() {
plugins/vhosting/plugin.js
+2
-2
@@ -32,7 +32,7 @@ exports.init = api => {
32
let { referer } = ctx.headers
33
referer &&= new URL(referer).pathname
34
if (referer?.startsWith(ctx.state.revProxyPath + api.const.ADMIN_URI)) return // exclude apis for admin-panel
35
- params = ctx.params
35
+ params = ctx.params || ctx.query
36
}
37
38
const hosts = api.getConfig('hosts')
@@ -51,7 +51,7 @@ exports.init = api => {
51
if (root && root[0] !== '/') // normalize
52
root = '/' + root
53
if (!root) return
54
- if (!params)
54
+ if (params === undefined)
55
ctx.path = root + ctx.path
56
else
57
for (const [k,v] of Object.entries(params))
shared/api.ts
+2
-10
@@ -102,17 +102,9 @@ export function useApi<T=any>(cmd: string | Falsy, params?: object) : [T | undef
102
type EventHandler = (type:string, data?:any) => void
103
104
export function apiEvents(cmd: string, params: Dict, cb:EventHandler) {
105
+ params = _.omitBy(params, _.isUndefined)
106
console.debug('API EVENTS', cmd, params)
106
- const processed: Record<string,string> = {}
107
- for (const k in params) {
108
- const v = params[k]
109
- if (v !== undefined)
110
- processed[k] = JSON.stringify(v)
111
- }
112
- const csrf = getCsrf()
113
- if (csrf)
114
- processed.csrf = JSON.stringify(csrf)
115
- const source = new EventSource(getPrefixUrl() + API_URL + cmd + '?' + new URLSearchParams(processed))
107
+ const source = new EventSource(getPrefixUrl() + API_URL + cmd + '?' + new URLSearchParams(params))
108
source.onopen = () => cb('connected')
109
source.onerror = err => cb('error', err)
110
source.onmessage = ({ data }) => {
src/api.file_list.ts
+2
-1
@@ -19,7 +19,7 @@ import _ from 'lodash'
19
import { HTTP_FOOL, HTTP_METHOD_NOT_ALLOWED, HTTP_NOT_FOUND } from './const'
20
import Koa from 'koa'
21
22
-export const file_list: ApiHandler = async ({ uri, offset, limit, search, c, sse }, ctx) => {
22
+export const get_file_list: ApiHandler = async ({ uri, offset, limit, search, c }, ctx) => {
23
const node = await urlToNode( uri || '/', ctx)
24
const list = new SendListReadable()
25
if (!node)
@@ -28,6 +28,7 @@ export const file_list: ApiHandler = async ({ uri, offset, limit, search, c, sse
28
return fail()
29
if (dirTraversal(search))
30
return fail(HTTP_FOOL)
31
+ const sse = ctx.get('accept') === 'text/event-stream'
32
if (node.default)
33
return (sse ? list.custom : _.identity)({ // sse will wrap the object in a 'custom' message, otherwise we plainly return the object
34
redirect: uri // tell the browser to access the folder (instead of using this api), so it will get the default file
src/api.lang.ts
+1
-1
@@ -11,7 +11,7 @@ import EMBEDDED_TRANSLATIONS from './langs/embedded'
11
12
const apis: ApiHandlers = {
13
14
- list_langs() {
14
+ get_langs() {
15
return new SendListReadable({
16
doAtStart: async list => {
17
for await (let name of glob.stream(code2file('*'))) {
src/api.plugins.ts
+1
-1
@@ -105,7 +105,7 @@ const apis: ApiHandlers = {
105
}
106
},
107
108
- search_online_plugins({ text }, ctx) {
108
+ get_online_plugins({ text }, ctx) {
109
return new SendListReadable({
110
async doAtStart(list) {
111
try {
src/api.vfs.ts
+1
-1
@@ -181,7 +181,7 @@ const apis: ApiHandlers = {
181
return { path }
182
},
183
184
- ls({ path, files=true, fileMask }, ctx) {
184
+ get_ls({ path, files=true, fileMask }, ctx) {
185
return new SendListReadable({
186
async doAtStart(list) {
187
if (!path && IS_WINDOWS) {
src/apiMiddleware.ts
+30
-18
@@ -5,7 +5,7 @@ import createSSE from './sse'
5
import { Readable } from 'stream'
6
import { asyncGeneratorToReadable, onOff, removeStarting } from './misc'
7
import events from './events'
8
-import { HTTP_BAD_REQUEST, HTTP_NOT_FOUND, HTTP_UNAUTHORIZED } from './const'
8
+import { HTTP_BAD_REQUEST, HTTP_FOOL, HTTP_NOT_FOUND, HTTP_UNAUTHORIZED } from './const'
9
import _ from 'lodash'
10
import { defineConfig } from './config'
11
@@ -24,14 +24,19 @@ export function apiMiddleware(apis: ApiHandlers) : Koa.Middleware {
24
return async (ctx) => {
25
if (!logApi.get())
26
ctx.state.dont_log = true
27
- const { params } = ctx
28
- console.debug('API', ctx.method, ctx.path, { ...params })
29
- const apiFun = apis.hasOwnProperty(ctx.path) && apis[ctx.path]!
30
- if (!apiFun) {
31
- ctx.body = 'invalid api'
32
- return ctx.status = HTTP_NOT_FOUND
33
- }
34
- const csrf = ctx.cookies.get('csrf')
27
+ const isPost = ctx.params
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
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')
40
// we don't rely on SameSite cookie option because it's https-only
41
let res
42
try {
@@ -42,8 +47,7 @@ export function apiMiddleware(apis: ApiHandlers) : Koa.Middleware {
47
fixUri(params, k)
48
else if (typeof (v as any)?.[0] === 'string')
49
(v as string[]).forEach((x,i) => fixUri(v,i))
45
- res = csrf && csrf !== params.csrf ? new ApiError(HTTP_UNAUTHORIZED, 'csrf')
46
- : await apiFun(params || {}, ctx)
50
+ res = await apiFun(params, ctx)
51
52
function fixUri(o: any, k: string | number) {
53
o[k] = removeStarting(ctx.state.revProxyPath, o[k])
@@ -61,15 +65,23 @@ export function apiMiddleware(apis: ApiHandlers) : Koa.Middleware {
65
resAsReadable.destroy())
66
return
67
}
64
- if (res instanceof ApiError) {
65
- ctx.body = res.message
66
- return ctx.status = res.status
68
+ if (res instanceof ApiError)
69
+ return send(res.status, res.message)
70
+ if (res instanceof Error) // generic error/exception
71
+ return send(HTTP_BAD_REQUEST, res.message || String(res))
72
+ ctx.body = res
73
+
74
+ function send(status: number, body?: string) {
75
+ ctx.body = body
76
+ ctx.status = status
77
}
68
- if (res instanceof Error) { // generic exception
69
- ctx.body = res.message || String(res)
70
- return ctx.status = HTTP_BAD_REQUEST
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
}
72
- ctx.body = res
85
}
86
}
87
src/frontEndApis.ts
+2
-2
@@ -1,7 +1,7 @@
1
// This file is part of HFS - Copyright 2021-2023, Massimo Melina <a@rejetto.com> - License https://www.gnu.org/licenses/gpl-3.0.txt
2
3
import { ApiError, ApiHandlers, SendListReadable } from './apiMiddleware'
4
-import { file_list } from './api.file_list'
4
+import { get_file_list } from './api.file_list'
5
import * as api_auth from './api.auth'
6
import { defineConfig } from './config'
7
import events from './events'
@@ -23,7 +23,7 @@ import { getUploadMeta } from './upload'
23
export const customHeader = defineConfig('custom_header', '')
24
25
export const frontEndApis: ApiHandlers = {
26
- file_list,
26
+ get_file_list,
27
...api_auth,
28
29
get_notifications({ channel }, ctx) {
src/log.ts
+1
-1
@@ -103,7 +103,7 @@ export const logMw: Koa.Middleware = async (ctx, next) => {
103
const date = a[2]+'/'+a[1]+'/'+a[3]+':'+a[4]+' '+a[5]?.slice(3)
104
const user = getCurrentUsername(ctx)
105
const length = ctx.state.length ?? ctx.length
106
- const uri = ctx.originalUrl?.replace(/&?csrf=[^&]+/, '') // temporary workaround. To avoid csrf in url we need to stop using EventSource.
106
+ const uri = ctx.originalUrl
107
events.emit(logger.name, Object.assign(_.pick(ctx, ['ip', 'method','status']), { length, user, ts: now, uri }))
108
debounce(() => // once in a while we check if the file is still good (not deleted, etc), or we'll reopen it
109
stat(logger.path).catch(() => logger.reopen())) // async = smoother but we may lose some entries
src/middlewares.ts
+1
-4
@@ -15,7 +15,6 @@ import {
15
dirTraversal,
16
filterMapGenerator,
17
isLocalHost,
18
- newObj,
18
stream2string,
19
tryJson
20
} from './misc'
@@ -261,10 +260,8 @@ async function srpCheck(username: string, password: string) {
260
return await step1.step2(clientRes2.A, clientRes2.M1).then(() => true, () => false)
261
}
262
264
-// unify get/post parameters, with JSON decoding to not be limited to strings
263
export const paramsDecoder: Koa.Middleware = async (ctx, next) => {
264
ctx.params = ctx.method === 'POST' && ctx.originalUrl.startsWith(API_URI)
267
- ? tryJson(await stream2string(ctx.req))
268
- : newObj(ctx.query, x => Array.isArray(x) ? x : tryJson(x))
265
+ && (tryJson(await stream2string(ctx.req)) || {})
266
await next()
267
}
src/misc.ts
+1
-1
@@ -213,7 +213,7 @@ export function same(a: any, b: any) {
213
}
214
215
export function tryJson(s?: string) {
216
- try { return s && JSON.parse(s) }
216
+ try { return s ? JSON.parse(s) : undefined }
217
catch {}
218
}
219
tests/test.ts
+1
-1
@@ -200,7 +200,7 @@ function reqApi(api: string, params: object, test:Tester) {
200
}
201
202
function reqList(uri:string, tester:Tester, params?: object) {
203
- return reqApi('file_list', { uri, ...params }, tester)
203
+ return reqApi('get_file_list', { uri, ...params }, tester)
204
}
205
206
function isInList(res:any, name:string) {