From 60171799e78fbf9ed5d9df6a2d6f44ea3a91afa5 Mon Sep 17 00:00:00 2001 From: ignacionelson Date: Sun, 20 Sep 2026 10:44:25 -0300 Subject: [PATCH] Keep "No folder" inside the client's own folder, not the library root MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported by binghuo. With per-client folders switched on, a client editing one of their own files could choose "No folder" and the file left their home for the root of the library — beside the staff folders, where the administrator's own things are. The feature exists precisely to stop that mess, and the editor was the one door still open to it. Uploading resolves an absent folder to the client's home, and so does creating a folder without naming a parent. The portal's file editor did not, so the rule held on two paths out of three. Now it holds on all three: update() resolves a null folder to the home where the installation gives them one, and the editor stops offering "No folder" at all in that case — there is no such place for this client — and preselects their home for a file that has none. An installation with the setting off is unchanged: no folder still means no folder, because there every client's file sits at the root. --- .../Http/Controllers/MyFilesController.php | 13 +++++ resources/js/pages/portal/edit-file.tsx | 12 +++- tests/Feature/Files/ClientHomeFoldersTest.php | 58 +++++++++++++++++++ 3 files changed, 81 insertions(+), 2 deletions(-) diff --git a/app/Modules/Files/Http/Controllers/MyFilesController.php b/app/Modules/Files/Http/Controllers/MyFilesController.php index 78c87b37..2c30f42d 100644 --- a/app/Modules/Files/Http/Controllers/MyFilesController.php +++ b/app/Modules/Files/Http/Controllers/MyFilesController.php @@ -446,6 +446,10 @@ class MyFilesController extends Controller 'categories' => $file->categories->pluck('id')->all(), ], 'can_delete' => Gate::forUser($client)->allows('delete', $file), + // Their own root, where the installation gives them one. The + // form offers no "No folder" beside it: there is no such place + // for this client, and update() resolves it here anyway. + 'home_folder_id' => $this->homeFolders->for($client)?->id, 'can_publish' => $client->can('upload_public'), // The public links on this file, and where to make and revoke // one — the same shape the staff screen uses. A file marked @@ -544,6 +548,15 @@ class MyFilesController extends Controller $folderId = isset($validated['folder_id']) ? (int) $validated['folder_id'] : null; + // "No folder" means the top of what this client sees, which on an + // installation that gives them a home folder is inside it — not the + // root of the library, beside the staff folders. Uploading and + // creating a folder already resolve it this way; the editor did + // not, so a client could move their own file out of their home and + // into the administrator's root by choosing "No folder" (reported + // by binghuo). + $folderId ??= $this->homeFolders->for($client)?->id; + // The client rule, not the staff one: somewhere they could have // uploaded it in the first place. Same check the upload path makes, // so moving a file cannot reach a folder that uploading it could diff --git a/resources/js/pages/portal/edit-file.tsx b/resources/js/pages/portal/edit-file.tsx index 95e9209a..068d1795 100644 --- a/resources/js/pages/portal/edit-file.tsx +++ b/resources/js/pages/portal/edit-file.tsx @@ -51,6 +51,8 @@ interface PortalEditFileProps { }; can_delete: boolean; can_publish: boolean; + /** This client's own root, where the installation gives them one. Null otherwise. */ + home_folder_id: number | null; /** The public links on this file, newest first, and where to make or revoke one. */ share_links: PortalShareLink[]; share_link_store_url: string; @@ -80,6 +82,7 @@ export default function PortalEditFile({ file, can_delete, can_publish, + home_folder_id, share_links, share_link_store_url, can_set_expiration, @@ -95,7 +98,9 @@ export default function PortalEditFile({ const form = useForm({ name: file.name, description: file.description ?? '', - folder_id: file.folder_id === null ? 'root' : String(file.folder_id), + // A file with no folder belongs in this client's own root where + // there is one — which is also what the server does with it. + folder_id: file.folder_id === null ? (home_folder_id === null ? 'root' : String(home_folder_id)) : String(file.folder_id), public: file.public, commentable: file.commentable, expires_at: file.expires_at ?? '', @@ -170,7 +175,10 @@ export default function PortalEditFile({ - {t('No folder')} + {/* Not offered where this client has a home + folder: "no folder" would mean the root + of the library, which is not theirs. */} + {home_folder_id === null && {t('No folder')}} {folders.map((folder) => ( diff --git a/tests/Feature/Files/ClientHomeFoldersTest.php b/tests/Feature/Files/ClientHomeFoldersTest.php index 3a761bdc..c80d657b 100644 --- a/tests/Feature/Files/ClientHomeFoldersTest.php +++ b/tests/Feature/Files/ClientHomeFoldersTest.php @@ -247,3 +247,61 @@ test('turning the setting off leaves an existing home working', function () { // strand them somewhere no listing looks. expect(app(ClientHomeFolders::class)->for($client->refresh())->id)->toBe($home->id); }); + +test('a client editing their own file cannot send it to the library root', function () { + // Reported by binghuo: choosing "No folder" in the portal's file editor + // moved the file out of the client's home and into the administrator's + // root, beside the staff folders — the exact mess the home folder + // exists to end. Uploading and creating a folder already resolved an + // absent folder to the home; the editor did not. + enableHomeFolders(); + $client = clientWhoCanUpload(); + RolePermission::query()->create(['role_id' => $client->role_id, 'permission' => Permission::EditFiles->value]); + $home = app(ClientHomeFolders::class)->for($client); + + $file = File::factory()->create(['uploaded_by' => $client->id, 'folder_id' => $home->id, 'name' => 'Theirs']); + + $this->actingAs($client)->patch("/my-files/{$file->id}", [ + 'name' => 'Theirs', + 'folder_id' => null, + ])->assertSessionHasNoErrors(); + + expect($file->refresh()->folder_id)->toBe($home->id); +}); + +test('with no home folder, no folder still means no folder', function () { + // The installation that never turned this on keeps what it had: a + // client's file sits at the root of the library because that is where + // every client's file sits there. + app(Settings::class)->set(Setting::ClientsHomeFolders, false); + $client = clientWhoCanUpload(); + RolePermission::query()->create(['role_id' => $client->role_id, 'permission' => Permission::EditFiles->value]); + $folder = app(FolderService::class)->create('Somewhere', null); + shareFolderWithClient($folder, $client); + + $file = File::factory()->create(['uploaded_by' => $client->id, 'folder_id' => $folder->id]); + + $this->actingAs($client)->patch("/my-files/{$file->id}", [ + 'name' => 'Theirs', + 'folder_id' => null, + ])->assertSessionHasNoErrors(); + + expect($file->refresh()->folder_id)->toBeNull(); +}); + +test('the editor offers no "no folder" where the client has a home', function () { + enableHomeFolders(); + $client = clientWhoCanUpload(); + RolePermission::query()->create(['role_id' => $client->role_id, 'permission' => Permission::EditFiles->value]); + $home = app(ClientHomeFolders::class)->for($client); + + $file = File::factory()->create(['uploaded_by' => $client->id, 'folder_id' => null]); + + $this->actingAs($client)->get("/my-files/{$file->id}/edit")->assertInertia( + fn (AssertableInertia $page) => $page + ->component('portal/edit-file') + // The page hides the option and preselects this, so the form + // cannot post the root even by accident. + ->where('home_folder_id', $home->id), + ); +});