From d41baf6b086a808175d77417b207c2dc950a46a5 Mon Sep 17 00:00:00 2001 From: Pavel Denisjuk Date: Tue, 11 Aug 2026 15:18:55 +0200 Subject: [PATCH] fix(app-file-manager): gate accessControl behind canUsePrivateFiles WCP check FileDetailsPresenter unconditionally sent accessControl in the UpdateFile mutation payload. When Private Files is not licensed, the API removes accessControl from FmFileUpdateInput, causing every file save to fail with a GraphQL validation error. Inject WcpService and gate accessControl in buildForm(), loadFile(), and saveFile() behind canUsePrivateFiles(). Add tests for the unlicensed path. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../FileDetails/FileDetailsPresenter.test.ts | 76 ++++++++++- .../FileDetails/FileDetailsPresenter.ts | 120 ++++++++++++------ 2 files changed, 151 insertions(+), 45 deletions(-) diff --git a/packages/app-file-manager/src/presentation/FileDetails/FileDetailsPresenter.test.ts b/packages/app-file-manager/src/presentation/FileDetails/FileDetailsPresenter.test.ts index d5a6f480304..79d31f86bbf 100644 --- a/packages/app-file-manager/src/presentation/FileDetails/FileDetailsPresenter.test.ts +++ b/packages/app-file-manager/src/presentation/FileDetails/FileDetailsPresenter.test.ts @@ -11,6 +11,7 @@ import { FileManagerPermissions } from "../../features/permissions/abstractions. import { GetSettingsRepository } from "../../features/settings/abstractions.js"; import { FileDetailsPresenter as Abstraction, type IFileDetailsPresenter } from "./abstractions.js"; import { FileDetailsPresenter } from "./FileDetailsPresenter.js"; +import { WcpService } from "@webiny/app-admin/features/wcp/abstractions.js"; import type { FmFile } from "../../features/shared/types.js"; // --------------------------------------------------------------------------- @@ -33,7 +34,7 @@ function createMockFormModel(overrides?: Partial): IFormModel { const data: Record = {}; return { - field: vi.fn() as any, + field: vi.fn().mockReturnValue({ setDisabled: vi.fn() }) as any, fields: vi.fn() as any, layout: vi.fn() as any, getData: vi.fn(() => ({ ...data })), @@ -110,6 +111,17 @@ function createMockSettingsRepository(): GetSettingsRepository.Interface { }; } +function createMockWcpService(canUsePrivateFiles = true): WcpService.Interface { + return { + getProject: vi.fn().mockReturnValue({ + canUsePrivateFiles: vi.fn().mockReturnValue(canUsePrivateFiles) + }), + isLoaded: vi.fn().mockReturnValue(true), + canUseFeature: vi.fn().mockReturnValue(false), + loadProject: vi.fn().mockResolvedValue(undefined) + } as unknown as WcpService.Interface; +} + function createTestFile(overrides?: Partial): FmFile { return { id: "file-1", @@ -141,16 +153,18 @@ interface Mocks { formModelFactory: FormModelFactory.Interface; permissions: FileManagerPermissions.Interface; settingsRepository: GetSettingsRepository.Interface; + wcpService: WcpService.Interface; } -function createMocks(): Mocks { +function createMocks(canUsePrivateFiles = true): Mocks { return { getFileUseCase: createMockGetFileUseCase(), updateFileUseCase: createMockUpdateFileUseCase(), deleteFileUseCase: createMockDeleteFileUseCase(), formModelFactory: createMockFormModelFactory(), permissions: createMockPermissions(), - settingsRepository: createMockSettingsRepository() + settingsRepository: createMockSettingsRepository(), + wcpService: createMockWcpService(canUsePrivateFiles) }; } @@ -163,6 +177,7 @@ function createContainer(mocks: Mocks) { container.registerInstance(FormModelFactory, mocks.formModelFactory); container.registerInstance(FileManagerPermissions, mocks.permissions); container.registerInstance(GetSettingsRepository, mocks.settingsRepository); + container.registerInstance(WcpService, mocks.wcpService); // Register the real FileDetailsPresenter implementation. container.register(FileDetailsPresenter).inSingletonScope(); @@ -344,4 +359,59 @@ describe("FileDetailsPresenter", () => { expect(loadingDuringExecution).toBe("Loading file..."); expect(presenter.vm.loading).toBeNull(); }); + + // ------------------------------------------------------------------- + // When Private Files is NOT licensed. + // ------------------------------------------------------------------- + + describe("when Private Files is NOT licensed", () => { + let unlicensedMocks: Mocks; + let unlicensedPresenter: IFileDetailsPresenter; + + beforeEach(() => { + unlicensedMocks = createMocks(false); + const container = createContainer(unlicensedMocks); + unlicensedPresenter = container.resolve(Abstraction); + }); + + it("should NOT include accessControl in the save payload", async () => { + await unlicensedPresenter.loadFile("file-1"); + + const mockForm = (unlicensedMocks.formModelFactory.create as ReturnType) + .mock.results[1].value as IFormModel; + (mockForm.submit as ReturnType).mockResolvedValue({ + name: "updated.jpg", + description: "", + tags: ["updated"] + }); + + await unlicensedPresenter.saveFile(); + + const callArgs = (unlicensedMocks.updateFileUseCase.execute as ReturnType) + .mock.calls[0][0]; + expect(callArgs.data).not.toHaveProperty("accessControl"); + }); + + it("should NOT set accessControl in form data during loadFile", async () => { + await unlicensedPresenter.loadFile("file-1"); + + const mockForm = (unlicensedMocks.formModelFactory.create as ReturnType) + .mock.results[1].value as IFormModel; + const setDataCall = (mockForm.setData as ReturnType).mock.calls[0][0]; + expect(setDataCall).not.toHaveProperty("accessControl"); + }); + + it("should NOT disable accessControl field when user lacks edit permission", async () => { + (unlicensedMocks.permissions.canEdit as ReturnType).mockReturnValue( + false + ); + + await unlicensedPresenter.loadFile("file-1"); + + const mockForm = (unlicensedMocks.formModelFactory.create as ReturnType) + .mock.results[1].value as IFormModel; + expect(mockForm.field).toHaveBeenCalledTimes(3); + expect(mockForm.field).not.toHaveBeenCalledWith("accessControl"); + }); + }); }); diff --git a/packages/app-file-manager/src/presentation/FileDetails/FileDetailsPresenter.ts b/packages/app-file-manager/src/presentation/FileDetails/FileDetailsPresenter.ts index 832f327530a..9298ab3fa2a 100644 --- a/packages/app-file-manager/src/presentation/FileDetails/FileDetailsPresenter.ts +++ b/packages/app-file-manager/src/presentation/FileDetails/FileDetailsPresenter.ts @@ -1,4 +1,5 @@ import { makeAutoObservable, runInAction, computed } from "mobx"; +import { WcpService } from "@webiny/app-admin/features/wcp/abstractions.js"; import { FileDetailsPresenter as Abstraction, type IFileDetailsPresenter, @@ -6,12 +7,12 @@ import { } from "./abstractions.js"; import { GetFileUseCase } from "../../features/getFile/abstractions.js"; import { UpdateFileUseCase } from "../../features/updateFile/abstractions.js"; -import { DeleteFileUseCase } from "../../features/deleteFile/abstractions.js"; import { FormModelFactory } from "@webiny/app-admin/features/formModel/abstractions.js"; import type { IFormModel } from "@webiny/app-admin/features/formModel/abstractions.js"; import { FileManagerPermissions } from "../../features/permissions/abstractions.js"; import { GetSettingsRepository } from "../../features/settings/abstractions.js"; import type { FmFile } from "../../features/shared/types.js"; +import type { UpdateFileData } from "../../features/updateFile/abstractions.js"; class FileDetailsPresenterImpl implements IFileDetailsPresenter { private file: FmFile | null = null; @@ -21,10 +22,10 @@ class FileDetailsPresenterImpl implements IFileDetailsPresenter { constructor( private getFileUseCase: GetFileUseCase.Interface, private updateFileUseCase: UpdateFileUseCase.Interface, - private deleteFileUseCase: DeleteFileUseCase.Interface, private formModelFactory: FormModelFactory.Interface, private permissions: FileManagerPermissions.Interface, - private settingsRepository: GetSettingsRepository.Interface + private settingsRepository: GetSettingsRepository.Interface, + private wcp: WcpService.Interface ) { // Build an empty form initially. this.form = this.buildForm(); @@ -56,18 +57,26 @@ class FileDetailsPresenterImpl implements IFileDetailsPresenter { if (result.success) { this.file = result.file; this.form = this.buildForm(); - this.form.setData({ + + const formData: Record = { name: result.file.name ?? "", description: result.file.description ?? "", - tags: result.file.tags ?? [], - accessControl: result.file.accessControl?.type ?? "public" - }); + tags: result.file.tags ?? [] + }; + + if (this.canUsePrivateFiles()) { + formData.accessControl = result.file.accessControl?.type ?? "public"; + } + + this.form.setData(formData); if (!this.permissions.canEdit("file", result.file)) { this.form.field("name").setDisabled(true); this.form.field("description").setDisabled(true); this.form.field("tags").setDisabled(true); - this.form.field("accessControl").setDisabled(true); + if (this.canUsePrivateFiles()) { + this.form.field("accessControl").setDisabled(true); + } } } }); @@ -93,16 +102,21 @@ class FileDetailsPresenterImpl implements IFileDetailsPresenter { }); try { + const updateData: UpdateFileData = { + name: data.name as string, + description: data.description as string, + tags: data.tags as string[] + }; + + if (this.canUsePrivateFiles()) { + updateData.accessControl = { + type: data.accessControl as "public" | "private-authenticated" + }; + } + await this.updateFileUseCase.execute({ id: this.file.id, - data: { - name: data.name as string, - description: data.description as string, - tags: data.tags as string[], - accessControl: { - type: data.accessControl as "public" | "private-authenticated" - } - } + data: updateData }); return true; } finally { @@ -114,33 +128,55 @@ class FileDetailsPresenterImpl implements IFileDetailsPresenter { private buildForm(): IFormModel { return this.formModelFactory.create({ - fields: fields => ({ - name: fields.text().label("Name").required().placeholder("Enter name"), - description: fields - .text() - .label("Description") - .renderer("textarea") - .placeholder("Enter description"), - tags: fields.text().label("Tags").list().renderer("tags").placeholder("Add tags"), - accessControl: fields - .text() - .label("Access Control") - .help("Control who can access this file.") - .options([ - { label: "Public", value: "public" }, - { label: "Private (Authenticated)", value: "private-authenticated" } - ]) - .defaultValue("public") - }), - layout: layout => [ - layout.row("name"), - layout.row("description"), - layout.row("tags"), - layout.row("accessControl") - ] + fields: fields => { + const fieldDefs: Record = { + name: fields.text().label("Name").required().placeholder("Enter name"), + description: fields + .text() + .label("Description") + .renderer("textarea") + .placeholder("Enter description"), + tags: fields + .text() + .label("Tags") + .list() + .renderer("tags") + .placeholder("Add tags") + }; + + if (this.canUsePrivateFiles()) { + fieldDefs.accessControl = fields + .text() + .label("Access Control") + .help("Control who can access this file.") + .options([ + { label: "Public", value: "public" }, + { + label: "Private (Authenticated)", + value: "private-authenticated" + } + ]) + .defaultValue("public"); + } + + return fieldDefs; + }, + layout: layout => { + const rows = [layout.row("name"), layout.row("description"), layout.row("tags")]; + + if (this.canUsePrivateFiles()) { + rows.push(layout.row("accessControl")); + } + + return rows; + } }); } + private canUsePrivateFiles(): boolean { + return this.wcp.getProject().canUsePrivateFiles(); + } + private buildPreviewUrl(): string | null { if (!this.file?.key) { return null; @@ -155,9 +191,9 @@ export const FileDetailsPresenter = Abstraction.createImplementation({ dependencies: [ GetFileUseCase, UpdateFileUseCase, - DeleteFileUseCase, FormModelFactory, FileManagerPermissions, - GetSettingsRepository + GetSettingsRepository, + WcpService ] });