Trim the staff breadcrumb to the library's reach, and ask before nesting into a public folder

Two edges of the staff folder screens, found while the folder API was
built to answer the same questions.

A client-scoped staff member can hold one of their clients' folders that
sits inside somebody else's tree. The breadcrumb above it named every
folder on the way up, including ones their library does not show them.
It now starts at the first folder they can reach, as the client portal's
already does (BreadcrumbBuilder::visible). Unscoped staff see the whole
trail as before.

Creating a folder did not ask Folder::uploadableBy for its parent, though
every other write of a parent_id does: a folder inside a public one is
public. Files were already refused there by the upload check, so what
this closes is an empty folder's name appearing on a public page without
upload_public. Staff holding upload_public, or creating inside a private
folder, are unaffected.
This commit is contained in:
ignacionelson
2026-10-02 23:45:51 -03:00
parent 185c46fff1
commit 48a1c9f227
2 changed files with 128 additions and 2 deletions
@@ -252,7 +252,7 @@ class FoldersController extends Controller
return Inertia::render('files/index', [
'folder' => $current === null ? null : ['id' => $current->id, 'name' => $current->name],
'breadcrumb' => $flat ? [] : $this->breadcrumbs->for($current),
'breadcrumb' => $flat ? [] : $this->breadcrumb($user, $current),
'folders' => $folderRows->map(fn (Folder $folder): array => $this->folderRow($user, $folder))->all(),
'files' => $fileRows->map(fn (File $file): array => $this->fileRow($user, $file, $commentCounts, $pendingCounts, $versions))->all(),
'pagination' => Pagination::meta($sliced['paginator']),
@@ -426,7 +426,7 @@ class FoldersController extends Controller
'public_url' => $folder->public
? $this->publicUrl->for($folder)
: null,
'breadcrumb' => $this->breadcrumbs->for($folder),
'breadcrumb' => $this->breadcrumb($user, $folder),
'can_update' => Gate::forUser($user)->allows('update', $folder),
'can_manage_public' => $user->can('upload_public'),
...$this->shareTargets->forSubject($folder, $user),
@@ -450,6 +450,11 @@ class FoldersController extends Controller
$parent = $this->resolveParent($user, $validated['parent_id'] ?? null);
// A folder inside a public one is public, so creating it there is
// placing content into a public folder: the question every other
// write of a parent_id already asks (Folder::uploadableBy).
abort_unless(Folder::uploadableBy($user, $parent), 403);
$folder = $this->folders->create($validated['name'], $parent);
// Only a user who can manage public state may set it on create —
@@ -631,6 +636,31 @@ class FoldersController extends Controller
->count();
}
/**
* The trail to $folder, trimmed for a client-scoped staff member to
* start at the first folder their library shows them: one of their
* clients' folders can sit inside somebody else's tree, and the names
* above it are not theirs to read. The client portal trims the same way.
*
* @return list<array{id: int, name: string}>
*/
private function breadcrumb(User $user, ?Folder $folder): array
{
if ($folder === null || ! $user->isClientScoped()) {
return $this->breadcrumbs->for($folder);
}
$visibleIds = array_values(array_map(
'intval',
$this->scope->folders($user)
->whereIn('folders.id', [...$folder->ancestorIds(), $folder->id])
->pluck('folders.id')
->all(),
));
return $this->breadcrumbs->visible($folder, $visibleIds);
}
private function resolveParent(?User $user, ?int $parentId): ?Folder
{
if ($user === null || $parentId === null) {
@@ -0,0 +1,96 @@
<?php
declare(strict_types=1);
use App\Models\User;
use App\Modules\Files\Models\Folder;
use App\Modules\Identity\Permissions\SystemRole;
use Inertia\Testing\AssertableInertia;
/**
* Two edges of the staff folder screens that the folder API made visible,
* because the API had to answer the same questions and answered them more
* strictly.
*/
beforeEach(function () {
$this->admin = User::factory()->create();
});
/*
* A client-scoped staff member can hold a client's folder that sits inside
* somebody else's tree. The trail above it named every folder on the way,
* including the ones their library does not show them. The client portal
* already trims the same trail (BreadcrumbBuilder::visible).
*/
test('a client-scoped staff member is not told the names of folders above their reach', function () {
$client = User::factory()->client()->create();
$manager = User::factory()->role(SystemRole::ClientManager)->create();
$manager->assignedClients()->sync([$client->id]);
$secret = makeFolder('Board minutes');
$acme = makeFolder('Acme', $secret);
$year = makeFolder('2026', $acme);
$this->actingAs($this->admin)->post("/folders/{$acme->id}/assignments", ['type' => 'client', 'id' => $client->id]);
$this->actingAs($manager)->get("/files?folder={$year->id}")
->assertOk()
->assertInertia(fn (AssertableInertia $page) => $page->where('breadcrumb', [
['id' => $acme->id, 'name' => 'Acme'],
['id' => $year->id, 'name' => '2026'],
]));
$this->actingAs($manager)->get("/folders/{$acme->id}")
->assertOk()
->assertInertia(fn (AssertableInertia $page) => $page->where('breadcrumb', [
['id' => $acme->id, 'name' => 'Acme'],
]));
});
test('an unscoped staff member still sees the whole trail', function () {
$secret = makeFolder('Board minutes');
$acme = makeFolder('Acme', $secret);
$this->actingAs($this->admin)->get("/files?folder={$acme->id}")
->assertOk()
->assertInertia(fn (AssertableInertia $page) => $page->where('breadcrumb', [
['id' => $secret->id, 'name' => 'Board minutes'],
['id' => $acme->id, 'name' => 'Acme'],
]));
});
/*
* A folder inside a public folder is public, so creating one there is
* placing something into a public folder — the question
* Folder::uploadableBy answers for every other write of a parent_id.
* Creation was the one write that did not ask it.
*/
test('a folder cannot be created inside a public folder without permission to publish', function () {
$public = makeFolder('Press kit');
$public->update(['public' => true]);
$staff = staffWithPermissions(['create_own_folders', 'upload', 'edit_others_files']);
$this->actingAs($staff)->post('/folders', ['name' => 'Drafts', 'parent_id' => $public->id])->assertForbidden();
expect(Folder::query()->where('name', 'Drafts')->exists())->toBeFalse();
});
test('with upload_public, creating inside a public folder still works', function () {
$public = makeFolder('Press kit');
$public->update(['public' => true]);
$staff = staffWithPermissions(['create_own_folders', 'upload', 'edit_others_files', 'upload_public']);
$this->actingAs($staff)->post('/folders', ['name' => 'Drafts', 'parent_id' => $public->id])->assertRedirect();
expect(Folder::query()->where('name', 'Drafts')->value('parent_id'))->toBe($public->id);
});
test('creating inside a private folder needs nothing extra', function () {
$private = makeFolder('Internal');
$staff = staffWithPermissions(['create_own_folders', 'upload', 'edit_others_files']);
$this->actingAs($staff)->post('/folders', ['name' => 'Drafts', 'parent_id' => $private->id])->assertRedirect();
expect(Folder::query()->where('name', 'Drafts')->value('parent_id'))->toBe($private->id);
});