diff --git a/app/Modules/Files/Http/Controllers/FoldersController.php b/app/Modules/Files/Http/Controllers/FoldersController.php index 43f2fde5..f6a4e10e 100644 --- a/app/Modules/Files/Http/Controllers/FoldersController.php +++ b/app/Modules/Files/Http/Controllers/FoldersController.php @@ -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 + */ + 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) { diff --git a/tests/Feature/Files/StaffFolderBoundaryTest.php b/tests/Feature/Files/StaffFolderBoundaryTest.php new file mode 100644 index 00000000..b53f3f23 --- /dev/null +++ b/tests/Feature/Files/StaffFolderBoundaryTest.php @@ -0,0 +1,96 @@ +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); +});