From dbad0ce1febca4fa11fcd0112438aec70a1ec6ba Mon Sep 17 00:00:00 2001 From: Manzoor Sofi <97725740+manzoorsofi@users.noreply.github.com> Date: Thu, 11 Dec 2025 05:08:47 +0530 Subject: [PATCH] feat: allow restricting public file uploads to System Managers (#34879) Co-authored-by: barredterra <14891507+barredterra@users.noreply.github.com> --- cypress/integration/file_uploader.js | 212 +++++++++++++++++- frappe/core/doctype/file/file.js | 4 + frappe/core/doctype/file/file.py | 10 + frappe/core/doctype/file/test_file.py | 94 ++++++++ .../system_settings/system_settings.json | 10 +- .../system_settings/system_settings.py | 1 + .../js/frappe/file_uploader/FilePreview.vue | 25 ++- .../js/frappe/file_uploader/FileUploader.vue | 2 +- .../file_uploader/file_uploader.bundle.js | 22 +- frappe/public/js/frappe/utils/utils.js | 14 ++ 10 files changed, 381 insertions(+), 13 deletions(-) diff --git a/cypress/integration/file_uploader.js b/cypress/integration/file_uploader.js index 5e56d0e596..4549c2464e 100644 --- a/cypress/integration/file_uploader.js +++ b/cypress/integration/file_uploader.js @@ -1,6 +1,6 @@ context("FileUploader", () => { before(() => { - cy.login(); + cy.login("Administrator", Cypress.env("adminPassword") || "admin"); }); beforeEach(() => { @@ -52,4 +52,214 @@ context("FileUploader", () => { .should("have.property", "file_name", "example.json"); cy.get(".modal:visible").should("not.exist"); }); + + describe("Public file upload restriction", () => { + const test_user = "test_file_uploader@example.com"; + const test_password = "test_password"; + + before(() => { + // Create test user without System Manager role + cy.call("frappe.tests.ui_test_helpers.create_test_user", { + username: test_user, + }); + cy.remove_role(test_user, "System Manager"); + // Set password for test user + cy.set_value("User", test_user, { + new_password: test_password, + }); + }); + + after(() => { + // Clean up test user + cy.login("Administrator", Cypress.env("adminPassword") || "admin"); + cy.visit("/desk"); + cy.wait(1000); + cy.remove_doc("User", test_user, true); // true = ignore_missing + }); + + it("should show checkbox and toggle when setting is disabled for System Manager", () => { + cy.login("Administrator", Cypress.env("adminPassword") || "admin"); + cy.visit("/desk"); + cy.wait(1000); + + // Disable the setting + cy.set_value("System Settings", "System Settings", { + only_allow_system_managers_to_upload_public_files: 0, + }); + // Update sysdefaults in window to avoid reload + cy.window() + .its("frappe") + .then((frappe) => { + frappe.boot.sysdefaults.only_allow_system_managers_to_upload_public_files = 0; + }); + cy.visit("/desk"); + cy.wait(2000); + + open_upload_dialog(); + + // Add a file to the dialog + cy.get_open_dialog() + .find(".file-upload-area") + .selectFile("cypress/fixtures/example.json", { + action: "drag-drop", + }); + + // Wait for file preview to render + cy.get_open_dialog().find(".file-preview").should("exist"); + + // Checkbox should be visible and enabled + cy.get_open_dialog().find("#uploader-private-checkbox").should("be.visible"); + cy.get_open_dialog() + .find("#uploader-private-checkbox input") + .should("not.be.disabled"); + + // Toggle button should be visible (secondary action button) + cy.get_open_dialog() + .find(".modal-footer .btn-secondary") + .should("be.visible") + .and("contain", "Set all"); + + cy.hide_dialog(); + }); + + it("should show checkbox and toggle when setting is disabled for non-System Manager", () => { + cy.login("Administrator", Cypress.env("adminPassword") || "admin"); + cy.visit("/desk"); + cy.wait(1000); + + // Disable the setting + cy.set_value("System Settings", "System Settings", { + only_allow_system_managers_to_upload_public_files: 0, + }); + cy.visit("/desk"); + cy.wait(1000); + + // Login as non-System Manager + cy.login(test_user, test_password); + cy.visit("/desk"); + cy.wait(2000); + // Update sysdefaults in window + cy.window() + .its("frappe") + .then((frappe) => { + frappe.boot.sysdefaults.only_allow_system_managers_to_upload_public_files = 0; + }); + + open_upload_dialog(); + + // Add a file to the dialog + cy.get_open_dialog() + .find(".file-upload-area") + .selectFile("cypress/fixtures/example.json", { + action: "drag-drop", + }); + + // Wait for file preview to render + cy.get_open_dialog().find(".file-preview").should("exist"); + + // Checkbox should be visible and enabled + cy.get_open_dialog().find("#uploader-private-checkbox").should("be.visible"); + cy.get_open_dialog() + .find("#uploader-private-checkbox input") + .should("not.be.disabled"); + + // Toggle button should be visible (secondary action button) + cy.get_open_dialog() + .find(".modal-footer .btn-secondary") + .should("be.visible") + .and("contain", "Set all"); + + cy.hide_dialog(); + }); + + it("should show checkbox and toggle when setting is enabled for System Manager", () => { + cy.login("Administrator", Cypress.env("adminPassword") || "admin"); + cy.visit("/desk"); + cy.wait(1000); + + // Enable the setting + cy.set_value("System Settings", "System Settings", { + only_allow_system_managers_to_upload_public_files: 1, + }); + // Update sysdefaults in window to avoid reload + cy.window() + .its("frappe") + .then((frappe) => { + frappe.boot.sysdefaults.only_allow_system_managers_to_upload_public_files = 1; + }); + cy.visit("/desk"); + cy.wait(2000); + + open_upload_dialog(); + + // Add a file to the dialog + cy.get_open_dialog() + .find(".file-upload-area") + .selectFile("cypress/fixtures/example.json", { + action: "drag-drop", + }); + + // Wait for file preview to render + cy.get_open_dialog().find(".file-preview").should("exist"); + + // Checkbox should be visible and enabled + cy.get_open_dialog().find("#uploader-private-checkbox").should("be.visible"); + cy.get_open_dialog() + .find("#uploader-private-checkbox input") + .should("not.be.disabled"); + + // Toggle button should be visible (secondary action button) + cy.get_open_dialog() + .find(".modal-footer .btn-secondary") + .should("be.visible") + .and("contain", "Set all"); + + cy.hide_dialog(); + }); + + it("should show disabled checkbox and hide toggle when setting is enabled for non-System Manager", () => { + cy.login("Administrator", Cypress.env("adminPassword") || "admin"); + cy.visit("/desk"); + cy.wait(1000); + + // Enable the setting + cy.set_value("System Settings", "System Settings", { + only_allow_system_managers_to_upload_public_files: 1, + }); + cy.visit("/desk"); + cy.wait(1000); + + // Login as non-System Manager + cy.login(test_user, test_password); + cy.visit("/desk"); + cy.wait(2000); + // Update sysdefaults in window + cy.window() + .its("frappe") + .then((frappe) => { + frappe.boot.sysdefaults.only_allow_system_managers_to_upload_public_files = 1; + }); + + open_upload_dialog(); + + // Add a file to the dialog + cy.get_open_dialog() + .find(".file-upload-area") + .selectFile("cypress/fixtures/example.json", { + action: "drag-drop", + }); + + // Wait for file preview to render + cy.get_open_dialog().find(".file-preview").should("exist"); + + // Checkbox should be visible but disabled + cy.get_open_dialog().find("#uploader-private-checkbox").should("be.visible"); + cy.get_open_dialog().find("#uploader-private-checkbox input").should("be.disabled"); + + // Toggle button should not be visible (secondary action button should be hidden) + cy.get_open_dialog().find(".modal-footer .btn-secondary").should("not.be.visible"); + + cy.hide_dialog(); + }); + }); }); diff --git a/frappe/core/doctype/file/file.js b/frappe/core/doctype/file/file.js index f6e3fab1f8..c8122ee0af 100644 --- a/frappe/core/doctype/file/file.js +++ b/frappe/core/doctype/file/file.js @@ -37,6 +37,10 @@ frappe.ui.form.on("File", { if (frm.doc.file_name && frm.doc.file_name.split(".").splice(-1)[0] === "zip") { frm.add_custom_button(__("Unzip"), () => frm.trigger("unzip")); } + + if (!frappe.utils.can_upload_public_files() && frm.doc.is_private) { + frm.set_df_property("is_private", "read_only", 1); + } }, preview_file: function (frm) { diff --git a/frappe/core/doctype/file/file.py b/frappe/core/doctype/file/file.py index ebf884daa1..b0442288dc 100755 --- a/frappe/core/doctype/file/file.py +++ b/frappe/core/doctype/file/file.py @@ -132,6 +132,7 @@ class File(Document): return self.validate_attachment_references() + self.enforce_public_file_restrictions() # when dict is passed to get_doc for creation of new_doc, is_new returns None # this case is handled inside handle_is_private_changed @@ -156,6 +157,15 @@ class File(Document): if self.attached_to_field and SPECIAL_CHAR_PATTERN.search(self.attached_to_field): frappe.throw(_("The fieldname you've specified in Attached To Field is invalid")) + def enforce_public_file_restrictions(self): + if not self.is_private and frappe.get_system_settings( + "only_allow_system_managers_to_upload_public_files" + ): + try: + frappe.only_for("System Manager") + except PermissionError: + frappe.throw(_("Only System Managers can make this file public.")) + def after_rename(self, *args, **kwargs): for successor in self.get_successors(): setup_folder_path(successor, self.name) diff --git a/frappe/core/doctype/file/test_file.py b/frappe/core/doctype/file/test_file.py index 12e6b751f4..128d74dae8 100644 --- a/frappe/core/doctype/file/test_file.py +++ b/frappe/core/doctype/file/test_file.py @@ -958,3 +958,97 @@ class TestGuestFileAndAttachments(IntegrationTestCase): self.assertEqual(doc_pri.get_content(), content) doc_pri.delete() self.assertFalse(os.path.exists(doc_pri.get_full_path())) + + +class TestPublicFileRestriction(IntegrationTestCase): + """Test public file upload restriction for non-System Managers.""" + + @classmethod + def setUpClass(cls): + super().setUpClass() + # Create a test user without System Manager role + if not frappe.db.exists("User", "test_restricted@example.com"): + user = frappe.get_doc( + { + "doctype": "User", + "email": "test_restricted@example.com", + "first_name": "Test Restricted", + "roles": [{"role": "Website Manager"}], + } + ) + user.insert(ignore_permissions=True) + + def tearDown(self): + frappe.set_user("Administrator") + + @IntegrationTestCase.change_settings( + "System Settings", {"only_allow_system_managers_to_upload_public_files": 1} + ) + def test_non_system_manager_cannot_upload_public_file_when_setting_enabled(self): + """Non-System Manager should not be able to upload public files when setting is enabled.""" + frappe.set_user("test_restricted@example.com") + + file_doc = frappe.get_doc( + { + "doctype": "File", + "file_name": "test_public_restricted.txt", + "content": "Test content", + "is_private": 0, + } + ) + + self.assertRaises(frappe.PermissionError, file_doc.insert) + + @IntegrationTestCase.change_settings( + "System Settings", {"only_allow_system_managers_to_upload_public_files": 1} + ) + def test_non_system_manager_can_upload_private_file_when_setting_enabled(self): + """Non-System Manager should still be able to upload private files when setting is enabled.""" + frappe.set_user("test_restricted@example.com") + + file_doc = frappe.get_doc( + { + "doctype": "File", + "file_name": "test_private_allowed.txt", + "content": "Test content", + "is_private": 1, + } + ) + + file_doc.insert() + self.assertTrue(file_doc.is_private) + + @IntegrationTestCase.change_settings( + "System Settings", {"only_allow_system_managers_to_upload_public_files": 1} + ) + def test_system_manager_can_upload_public_file_when_setting_enabled(self): + """System Manager should be able to upload public files even when setting is enabled.""" + frappe.set_user("Administrator") + + file_doc = frappe.get_doc( + { + "doctype": "File", + "file_name": "test_public_admin.txt", + "content": "Test content", + "is_private": 0, + } + ) + + file_doc.insert() + self.assertFalse(file_doc.is_private) + + def test_non_system_manager_can_upload_public_file_when_setting_disabled(self): + """Non-System Manager should be able to upload public files when setting is disabled.""" + frappe.set_user("test_restricted@example.com") + + file_doc = frappe.get_doc( + { + "doctype": "File", + "file_name": "test_public_allowed.txt", + "content": "Test content", + "is_private": 0, + } + ) + + file_doc.insert() + self.assertFalse(file_doc.is_private) diff --git a/frappe/core/doctype/system_settings/system_settings.json b/frappe/core/doctype/system_settings/system_settings.json index c56becffd9..fe9c2c86fe 100644 --- a/frappe/core/doctype/system_settings/system_settings.json +++ b/frappe/core/doctype/system_settings/system_settings.json @@ -85,6 +85,7 @@ "allow_guests_to_upload_files", "force_web_capture_mode_for_uploads", "strip_exif_metadata_from_uploaded_images", + "only_allow_system_managers_to_upload_public_files", "delete_background_exported_reports_after", "column_break_uqma", "allowed_file_extensions", @@ -775,12 +776,19 @@ { "fieldname": "column_break_ewhs", "fieldtype": "Column Break" + }, + { + "default": "0", + "description": "If enabled, only System Managers can upload public files. Other users can't see the checkbox Is Private in the upload dialog.", + "fieldname": "only_allow_system_managers_to_upload_public_files", + "fieldtype": "Check", + "label": "Only allow System Managers to upload public files" } ], "icon": "fa fa-cog", "issingle": 1, "links": [], - "modified": "2025-12-10 05:27:21.774897", + "modified": "2025-12-10 20:43:50.675887", "modified_by": "Administrator", "module": "Core", "name": "System Settings", diff --git a/frappe/core/doctype/system_settings/system_settings.py b/frappe/core/doctype/system_settings/system_settings.py index 84747565cb..687fd006df 100644 --- a/frappe/core/doctype/system_settings/system_settings.py +++ b/frappe/core/doctype/system_settings/system_settings.py @@ -88,6 +88,7 @@ class SystemSettings(Document): "#.###", "#,###", ] + only_allow_system_managers_to_upload_public_files: DF.Check otp_issuer_name: DF.Data | None otp_sms_template: DF.SmallText | None password_reset_limit: DF.Int diff --git a/frappe/public/js/frappe/file_uploader/FilePreview.vue b/frappe/public/js/frappe/file_uploader/FilePreview.vue index 8054df3f3c..97dae5c9c2 100644 --- a/frappe/public/js/frappe/file_uploader/FilePreview.vue +++ b/frappe/public/js/frappe/file_uploader/FilePreview.vue @@ -20,17 +20,24 @@
- - @@ -95,6 +102,9 @@ const props = defineProps({ allow_toggle_private: { default: true, }, + show_private_checkbox: { + default: true, + }, allow_toggle_optimize: { default: true, }, @@ -127,9 +137,18 @@ let allow_toggle_optimize = computed(() => { !props.file.failed ); }); + let allow_toggle_private = computed(() => { - return props.allow_toggle_private && !uploaded.value && !props.file.failed; + if (!frappe.utils.can_upload_public_files()) { + return false; + } + return props.allow_toggle_private; }); + +let show_private_checkbox = computed(() => { + return !uploaded.value && !props.file.failed; +}); + let is_cropable = computed(() => { let croppable_types = ["image/jpeg", "image/png"]; return ( diff --git a/frappe/public/js/frappe/file_uploader/FileUploader.vue b/frappe/public/js/frappe/file_uploader/FileUploader.vue index 570edd7fce..709e081d66 100644 --- a/frappe/public/js/frappe/file_uploader/FileUploader.vue +++ b/frappe/public/js/frappe/file_uploader/FileUploader.vue @@ -441,7 +441,7 @@ function add_files(file_array) { request_succeeded: false, error_message: null, uploading: false, - private: !props.make_attachments_public, + private: !props.make_attachments_public || !frappe.utils.can_upload_public_files(), }; }); diff --git a/frappe/public/js/frappe/file_uploader/file_uploader.bundle.js b/frappe/public/js/frappe/file_uploader/file_uploader.bundle.js index 9b356e7aa5..03c1a53132 100644 --- a/frappe/public/js/frappe/file_uploader/file_uploader.bundle.js +++ b/frappe/public/js/frappe/file_uploader/file_uploader.bundle.js @@ -30,6 +30,8 @@ class FileUploader { } = {}) { frm && frm.attachments.max_reached(true); + this.can_toggle_private = frappe.utils.can_upload_public_files(); + if (!wrapper) { this.make_dialog(dialog_title); } else { @@ -89,7 +91,7 @@ class FileUploader { () => this.uploader.files, (files) => { let all_private = files.every((file) => file.private); - if (this.dialog) { + if (this.dialog && this.can_toggle_private) { this.dialog.set_secondary_action_label( all_private ? __("Set all public") : __("Set all private") ); @@ -139,18 +141,24 @@ class FileUploader { } make_dialog(title) { - this.dialog = new frappe.ui.Dialog({ + const dialog_opts = { title: title || __("Upload"), primary_action_label: __("Upload"), primary_action: () => this.upload_files(), - secondary_action_label: __("Set all private"), - secondary_action: () => { - this.uploader.toggle_all_private(); - }, on_page_show: () => { this.uploader.wrapper_ready = true; }, - }); + }; + + // Only add secondary action if user is allowed to toggle privacy + if (this.can_toggle_private) { + dialog_opts.secondary_action_label = __("Set all private"); + dialog_opts.secondary_action = () => { + this.uploader.toggle_all_private(); + }; + } + + this.dialog = new frappe.ui.Dialog(dialog_opts); this.wrapper = this.dialog.body; this.dialog.show(); diff --git a/frappe/public/js/frappe/utils/utils.js b/frappe/public/js/frappe/utils/utils.js index 2facd65c3b..49f9f77054 100644 --- a/frappe/public/js/frappe/utils/utils.js +++ b/frappe/public/js/frappe/utils/utils.js @@ -1960,4 +1960,18 @@ Object.assign(frappe.utils, { }); }); }, + + /** + * Check if current user can upload public files. + * @returns {boolean} + */ + can_upload_public_files() { + if ( + Number(frappe.boot.sysdefaults?.only_allow_system_managers_to_upload_public_files) !== + 1 + ) { + return true; + } + return frappe.user.has_role(["System Manager", "Administrator"]); + }, });