fix: admin/shared: permissions changes were not reflected on descendants even after saving (until reload) #1218

Massimo Melina committed Apr 21, 2026 at 10:54 UTC cae524b20d7e5b76392cfc540b29c2d6c6c1d6df
3 files changed +92 -18
admin/src/FileForm.ts
+3 -18
@@ -14,7 +14,7 @@ import {
14 } from './misc'
15 import { isModifiedConfig } from './AccountForm'
16 import { Btn, Flex, IconBtn, LinkBtn, propsForModifiedValues, useBreakpoint, wikiLink } from './mui'
17 -import { deleteVfs, id2vfsNode, reindexVfs, VfsNodeAdmin } from './VfsPage'
17 +import { deleteVfs, getInheritedPerms, id2vfsNode, reindexVfs, VfsNodeAdmin } from './VfsPage'
18 import _ from 'lodash'
19 import FileField from './FileField'
20 import { alertDialog, toast, useDialogBarColors } from './dialog'
@@ -226,11 +226,11 @@ export default function FileForm({ file, addToBar, statusApi, accounts, saved, i
226 const dontShow = [perm, ...onlyTruthy(_.map(show, (v,k) => !v && k))]
227 const others = _.difference(Object.keys(defaultPerms), dontShow)
228 // a freshly created node can be selected before `inherited` is filled by a server roundtrip
229 - let inherit = file.inherited?.[perm] ?? getParentInheritedPerm(perm) ?? defaultPerms[perm]
229 + let inherit = file.inherited?.[perm] ?? getInheritedPerms(file)?.[perm] ?? defaultPerms[perm]
230 while (typeof inherit === 'string' && _.get(show, inherit) === false) // is 'inherit' referring to another permission that is not displayed?
231 inherit = _.get(values, inherit)
232 // non-permission who values (like WHO_ANY_ACCOUNT) are not valid keys for inherited lookup
233 - ?? (inherit !== WHO_ANY_ACCOUNT ? getParentInheritedPerm(inherit) : undefined)
233 + ?? (inherit !== WHO_ANY_ACCOUNT ? getInheritedPerms(file)?.[inherit] : undefined)
234 ?? _.get(defaultPerms, inherit)! // then show its value instead
235 return {
236 comp: WhoField,
@@ -245,21 +245,6 @@ export default function FileForm({ file, addToBar, statusApi, accounts, saved, i
245 }
246 }
247
248 - function getParentInheritedPerm(perm: keyof VfsPerms): Who | undefined {
249 - if (file[perm] !== undefined)
250 - return
251 - let cursor = file.parent
252 - while (cursor) {
253 - let inheritedPerm = cursor[perm]
254 - if (!isWhoObject(inheritedPerm))
255 - return inheritedPerm
256 - inheritedPerm = inheritedPerm.children
257 - if (inheritedPerm !== undefined)
258 - return inheritedPerm
259 - cursor = cursor.parent
260 - }
261 - }
262 -
248 }
249
250 function perm2word(perm: string) {
admin/src/VfsPage.ts
+29
@@ -11,6 +11,7 @@ import { markVfsModified, prepareVfsUndo, state, useSnapState } from './state'
11 import VfsTree, { vfsNodeIcon } from './VfsTree'
12 import {
13 CFG, matches, newDialog, normalizeHost, onlyTruthy, pathEncode, prefix, VfsNodeAdminSend, HIDE_IN_TESTS, wait,
14 + isWhoObject, PERM_KEYS, VfsPerms, Who,
15 } from './misc'
16 import { Flex, useBreakpoint } from './mui'
17 import { reactJoin } from '@hfs/shared'
@@ -198,6 +199,7 @@ export function reindexVfs({
199 function recur(node: VfsNodeAdmin, pre: string, parent: VfsNodeAdmin | undefined) {
200 const oldId = node.id
201 node.parent = parent
202 + node.inherited = getInheritedPerms(node) // refresh cached inheritance while reindexing, because local edits do not get a server roundtrip
203 const newId = node.isRoot ? '/' : prefix(pre, pathEncode(node.name), node.type === 'folder' ? '/' : '')
204 if (oldId && oldId !== newId)
205 id2vfsNode.delete(oldId)
@@ -211,6 +213,33 @@ export function reindexVfs({
213 }
214 }
215
216 +export function getInheritedPerms(child: VfsNodeAdmin | undefined) {
217 + const parent = child?.parent
218 + if (!parent) return
219 + const ret: VfsPerms = {}
220 + for (const k of PERM_KEYS) {
221 + const inheritedPerm = getInheritedPerm(parent, k)
222 + // null is the form's local representation of an unset permission
223 + if (inheritedPerm !== undefined && child[k] == null)
224 + ret[k] = inheritedPerm
225 + }
226 + return _.isEmpty(ret) ? undefined : ret
227 +
228 + function getInheritedPerm(cursor: VfsNodeAdmin | undefined, perm: keyof VfsPerms): Who | undefined {
229 + while (cursor) {
230 + let inheritedPerm = cursor[perm]
231 + if (inheritedPerm != null) {
232 + if (!isWhoObject(inheritedPerm))
233 + return inheritedPerm
234 + inheritedPerm = inheritedPerm.children
235 + if (inheritedPerm !== undefined)
236 + return inheritedPerm
237 + }
238 + cursor = cursor.parent
239 + }
240 + }
241 +}
242 +
243 export function reloadVfs(pleaseSelect?: string[]) {
244 selectOnReload = pleaseSelect
245 state.vfs = undefined
e2e/admin-vfs.spec.ts
+60
@@ -246,3 +246,63 @@ test('apply keeps unset permissions nullish in-memory', async ({ page }) => {
246 unsetPerms: ['can_see', 'can_read', 'can_list', 'can_upload', 'can_delete', 'can_archive'],
247 })
248 })
249 +
250 +test('apply refreshes inherited permissions for descendants in-memory', async ({ page }) => {
251 + await page.goto(URL + '~/admin/')
252 + await page.getByRole('textbox', { name: 'Username' }).fill(username)
253 + await page.getByRole('textbox', { name: 'Password' }).fill(password)
254 + await page.getByRole('textbox', { name: 'Password' }).press('Enter')
255 + await clickAdminMenu(page, /Shared files/)
256 + await page.getByText('zipNoList', { exact: true }).waitFor({ timeout: 10_000 })
257 +
258 + await expect.poll(() => page.evaluate(() => {
259 + const state = (window as any).state
260 + if (!state?.vfs) return ''
261 + state.selectedFiles = [state.vfs]
262 + return state.selectedFiles[0]?.id || ''
263 + })).toBe('/')
264 + await page.getByRole('combobox', { name: 'Who can download' }).click()
265 + await page.getByRole('option', { name: 'No one' }).click()
266 + await page.locator('button:has-text("Apply")').click()
267 +
268 + await expect.poll(() => page.evaluate(() => {
269 + function findById(node: any, id: string): any {
270 + if (!node) return
271 + if (node.id === id) return node
272 + for (const child of node.children || []) {
273 + const found = findById(child, id)
274 + if (found) return found
275 + }
276 + }
277 + const node = findById((window as any).state?.vfs, '/f1/')
278 + return {
279 + rootCanRead: (window as any).state?.vfs?.can_read,
280 + inheritedCanRead: node?.inherited?.can_read,
281 + }
282 + })).toEqual({
283 + rootCanRead: false,
284 + inheritedCanRead: false,
285 + })
286 +
287 + await selectVfsNode(page, 'f1', '/f1/')
288 + await page.locator('button:has-text("Apply")').click()
289 +
290 + await expect.poll(() => page.evaluate(() => {
291 + function findById(node: any, id: string): any {
292 + if (!node) return
293 + if (node.id === id) return node
294 + for (const child of node.children || []) {
295 + const found = findById(child, id)
296 + if (found) return found
297 + }
298 + }
299 + const node = findById((window as any).state?.vfs, '/f1/f2/')
300 + return {
301 + middleCanRead: findById((window as any).state?.vfs, '/f1/')?.can_read,
302 + inheritedCanRead: node?.inherited?.can_read,
303 + }
304 + })).toEqual({
305 + middleCanRead: null,
306 + inheritedCanRead: false,
307 + })
308 +})