feat: allow restricting public file uploads to System Managers (#34879)

Co-authored-by: barredterra <14891507+barredterra@users.noreply.github.com>
This commit is contained in:
Manzoor Sofi 2025-12-11 05:08:47 +05:30 committed by GitHub
parent 624c0df987
commit dbad0ce1fe
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 381 additions and 13 deletions

View file

@ -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();
});
});
});

View file

@ -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) {

View file

@ -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)

View file

@ -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)

View file

@ -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 <i>Is Private</i> 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",

View file

@ -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

View file

@ -20,17 +20,24 @@
</div>
<div class="flex config-area">
<label v-if="allow_toggle_optimize" class="frappe-checkbox"
<label
v-if="allow_toggle_optimize"
class="frappe-checkbox"
id="uploader-optimize-checkbox"
><input
type="checkbox"
:checked="optimize"
@change="emit('toggle_optimize')"
/>{{ __("Optimize") }}</label
>
<label v-if="allow_toggle_private" class="frappe-checkbox"
<label
v-if="show_private_checkbox"
class="frappe-checkbox"
id="uploader-private-checkbox"
><input
type="checkbox"
:checked="file.private"
:disabled="!allow_toggle_private"
@change="emit('toggle_private')"
/>{{ __("Private") }}</label
>
@ -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 (

View file

@ -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(),
};
});

View file

@ -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();

View file

@ -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"]);
},
});