feat: Disable Sharing globally (#20318)
* feat: Disable Sharing globally - Checkbox in System Settings - If disabled, avoid share UI render - Share APIs return None (non-obstructing) if share APIs are invoked * feat: Settings checkbox must toggle share permission globally - Treat feature like a perm toggler. Essentially noone is allowed to explicity share anything - Implicit sharing via `ignore_share_permissions` is allowed. Devs can decide where sharing should happen under the hood - UI is made read only and not hidden. Users must see who doc is already shared with - Make sure perm APIs used by share feature return false if sharing is disabled - Rename checkbox to `Disable Document Sharing` * test: (server side) Impact of disabling sharing on APIs - Also, fix missed system setting rename in `assign_to` * fix: Inform assigner if assignee lacks perms and sharing is disabled - misc: readable conditions * fix: throw instead of msgprint * fix: Typo and appropriate message for `throw` --------- Co-authored-by: Ankush Menat <ankush@frappe.io>
This commit is contained in:
parent
63338a3806
commit
90f8f945b4
5 changed files with 92 additions and 6 deletions
|
|
@ -4,7 +4,7 @@
|
|||
import frappe
|
||||
import frappe.share
|
||||
from frappe.automation.doctype.auto_repeat.test_auto_repeat import create_submittable_doctype
|
||||
from frappe.tests.utils import FrappeTestCase
|
||||
from frappe.tests.utils import FrappeTestCase, change_settings
|
||||
|
||||
test_dependencies = ["User"]
|
||||
|
||||
|
|
@ -139,3 +139,66 @@ class TestDocShare(FrappeTestCase):
|
|||
|
||||
test_doc.reload()
|
||||
self.assertTrue(test_doc.has_permission("read"))
|
||||
|
||||
@change_settings("System Settings", {"disable_document_sharing": 1})
|
||||
def test_share_disabled_add(self):
|
||||
"Test if user loses share access on disabling share globally."
|
||||
frappe.share.add("Event", self.event.name, self.user, share=1) # Share as admin
|
||||
frappe.set_user(self.user)
|
||||
|
||||
# User does not have share access although given to them
|
||||
self.assertFalse(self.event.has_permission("share"))
|
||||
self.assertRaises(
|
||||
frappe.PermissionError, frappe.share.add, "Event", self.event.name, "test1@example.com"
|
||||
)
|
||||
|
||||
@change_settings("System Settings", {"disable_document_sharing": 1})
|
||||
def test_share_disabled_add_with_ignore_permissions(self):
|
||||
frappe.share.add("Event", self.event.name, self.user, share=1)
|
||||
frappe.set_user(self.user)
|
||||
|
||||
# User does not have share access although given to them
|
||||
self.assertFalse(self.event.has_permission("share"))
|
||||
|
||||
# Test if behaviour is consistent for developer overrides
|
||||
frappe.share.add_docshare(
|
||||
"Event", self.event.name, "test1@example.com", flags={"ignore_share_permission": True}
|
||||
)
|
||||
|
||||
@change_settings("System Settings", {"disable_document_sharing": 1})
|
||||
def test_share_disabled_set_permission(self):
|
||||
frappe.share.add("Event", self.event.name, self.user, share=1)
|
||||
frappe.set_user(self.user)
|
||||
|
||||
# User does not have share access although given to them
|
||||
self.assertFalse(self.event.has_permission("share"))
|
||||
self.assertRaises(
|
||||
frappe.PermissionError,
|
||||
frappe.share.set_permission,
|
||||
"Event",
|
||||
self.event.name,
|
||||
"test1@example.com",
|
||||
"read",
|
||||
)
|
||||
|
||||
@change_settings("System Settings", {"disable_document_sharing": 1})
|
||||
def test_share_disabled_assign_to(self):
|
||||
"""
|
||||
Assigning a document to a user without access must not share the document,
|
||||
if sharing disabled.
|
||||
"""
|
||||
from frappe.desk.form.assign_to import add, get
|
||||
|
||||
frappe.share.add("Event", self.event.name, self.user, share=1)
|
||||
frappe.set_user(self.user)
|
||||
|
||||
# Assign to 'test1@example.com'
|
||||
add({"doctype": "Event", "name": self.event.name, "assign_to": ["test1@example.com"]})
|
||||
|
||||
# Check if assigned to 'test1@example.com'
|
||||
assignments = get(dict(doctype="Event", name=self.event.name))
|
||||
self.assertEqual(len(assignments), 1)
|
||||
|
||||
# Check if not shared with 'test1@example.com'
|
||||
shared_users = [x.user for x in frappe.share.get_users("Event", self.event.name)]
|
||||
self.assertNotIn("test1@example.com", shared_users)
|
||||
|
|
|
|||
|
|
@ -13,6 +13,7 @@
|
|||
"time_zone",
|
||||
"enable_onboarding",
|
||||
"setup_complete",
|
||||
"disable_document_sharing",
|
||||
"date_and_number_format",
|
||||
"date_format",
|
||||
"time_format",
|
||||
|
|
@ -528,12 +529,18 @@
|
|||
"fieldtype": "Select",
|
||||
"label": "Rounding Method",
|
||||
"options": "Banker's Rounding (legacy)\nBanker's Rounding\nCommercial Rounding"
|
||||
},
|
||||
{
|
||||
"default": "0",
|
||||
"fieldname": "disable_document_sharing",
|
||||
"fieldtype": "Check",
|
||||
"label": "Disable Document Sharing"
|
||||
}
|
||||
],
|
||||
"icon": "fa fa-cog",
|
||||
"issingle": 1,
|
||||
"links": [],
|
||||
"modified": "2023-03-10 12:23:45.248125",
|
||||
"modified": "2023-03-14 11:30:56.465653",
|
||||
"modified_by": "Administrator",
|
||||
"module": "Core",
|
||||
"name": "System Settings",
|
||||
|
|
@ -552,4 +559,4 @@
|
|||
"sort_order": "ASC",
|
||||
"states": [],
|
||||
"track_changes": 1
|
||||
}
|
||||
}
|
||||
|
|
@ -93,10 +93,17 @@ def add(args=None):
|
|||
|
||||
doc = frappe.get_doc(args["doctype"], args["name"])
|
||||
|
||||
# if assignee does not have permissions, share
|
||||
# if assignee does not have permissions, share or inform
|
||||
if not frappe.has_permission(doc=doc, user=assign_to):
|
||||
frappe.share.add(doc.doctype, doc.name, assign_to)
|
||||
shared_with_users.append(assign_to)
|
||||
if frappe.get_system_settings("disable_document_sharing"):
|
||||
msg = _("User {0} is not permitted to access this document.").format(frappe.bold(assign_to))
|
||||
msg += "<br>" + _(
|
||||
"As document sharing is disabled, please give them the required permissions before assigning."
|
||||
)
|
||||
frappe.throw(msg, title=_("Missing Permission"))
|
||||
else:
|
||||
frappe.share.add(doc.doctype, doc.name, assign_to)
|
||||
shared_with_users.append(assign_to)
|
||||
|
||||
# make this document followed by assigned user
|
||||
if frappe.get_cached_value("User", assign_to, "follow_assigned_documents"):
|
||||
|
|
|
|||
|
|
@ -77,6 +77,9 @@ def has_permission(
|
|||
if user == "Administrator":
|
||||
return True
|
||||
|
||||
if ptype == "share" and frappe.get_system_settings("disable_document_sharing"):
|
||||
return False
|
||||
|
||||
if not doc and hasattr(doctype, "doctype"):
|
||||
# first argument can be doc or doctype
|
||||
doc = doctype
|
||||
|
|
|
|||
|
|
@ -447,6 +447,12 @@ $.extend(frappe.model, {
|
|||
},
|
||||
|
||||
can_share: function (doctype, frm) {
|
||||
let disable_sharing = cint(frappe.sys_defaults.disable_document_sharing);
|
||||
|
||||
if (disable_sharing && frappe.session.user !== "Administrator") {
|
||||
return false;
|
||||
}
|
||||
|
||||
if (frm) {
|
||||
return frm.perm[0].share === 1;
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue