Skip to content

Commit

Permalink
feat: Disable Sharing globally (#20318) [backport] (#20500)
Browse files Browse the repository at this point in the history
* 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>

* test: fix test case to modified behaviour

We throw instead of showing warning now

* style: format

[skip ci]

---------

Co-authored-by: Ankush Menat <ankush@frappe.io>
  • Loading branch information
marination and ankush committed Mar 29, 2023
1 parent 5be6453 commit 758873c
Show file tree
Hide file tree
Showing 7 changed files with 92 additions and 10 deletions.
59 changes: 59 additions & 0 deletions frappe/core/doctype/docshare/test_docshare.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import frappe
import frappe.share
from frappe.automation.doctype.auto_repeat.test_auto_repeat import create_submittable_doctype
from frappe.tests.utils import change_settings

test_dependencies = ["User"]

Expand Down Expand Up @@ -128,3 +129,61 @@ def test_share_with_submit_perm(self):
)

frappe.share.remove(doctype, submittable_doc.name, self.user)

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

frappe.share.add("Event", self.event.name, self.user, share=1)
frappe.set_user(self.user)

self.assertRaises(
frappe.ValidationError,
add,
{"doctype": "Event", "name": self.event.name, "assign_to": ["test1@example.com"]},
)
11 changes: 9 additions & 2 deletions frappe/core/doctype/system_settings/system_settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
"is_first_startup",
"enable_onboarding",
"setup_complete",
"disable_document_sharing",
"date_and_number_format",
"date_format",
"time_format",
Expand Down Expand Up @@ -525,12 +526,18 @@
"fieldname": "apply_perm_level_on_api_calls",
"fieldtype": "Check",
"label": "Apply Perm Level on API calls (Recommended)"
},
{
"default": "0",
"fieldname": "disable_document_sharing",
"fieldtype": "Check",
"label": "Disable Document Sharing"
}
],
"icon": "fa fa-cog",
"issingle": 1,
"links": [],
"modified": "2023-03-03 11:47:24.867356",
"modified": "2023-03-14 11:30:56.465653",
"modified_by": "Administrator",
"module": "Core",
"name": "System Settings",
Expand All @@ -548,4 +555,4 @@
"sort_field": "modified",
"sort_order": "ASC",
"track_changes": 1
}
}
13 changes: 10 additions & 3 deletions frappe/desk/form/assign_to.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
follow_document(args["doctype"], args["name"], assign_to)
Expand Down
2 changes: 1 addition & 1 deletion frappe/model/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ def get_permitted_fields(

if doctype in core_doctypes_list:
return valid_columns

# DocType has only fields of type Table (Table, Table MultiSelect)
if set(valid_columns).issubset(default_fields):
return valid_columns
Expand Down
3 changes: 3 additions & 0 deletions frappe/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ def has_permission(
if user == "Administrator":
return True

if ptype == "share" and frappe.get_system_settings("disable_document_sharing"):
return False

meta = frappe.get_meta(doctype)

if doc:
Expand Down
10 changes: 8 additions & 2 deletions frappe/public/js/frappe/model/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -348,8 +348,14 @@ $.extend(frappe.model, {
return frappe.boot.user.can_email.indexOf(doctype)!==-1;
},

can_share: function(doctype, frm) {
if(frm) {
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;
}
return frappe.boot.user.can_share.indexOf(doctype)!==-1;
Expand Down
4 changes: 2 additions & 2 deletions frappe/tests/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ def test_case(self):
# change setting
for key, value in settings_dict.items():
setattr(settings, key, value)
settings.save()
settings.save(ignore_permissions=True)
# singles are cached by default, clear to avoid flake
frappe.db.value_cache[settings] = {}
yield # yield control to calling function
Expand All @@ -54,7 +54,7 @@ def test_case(self):
settings = frappe.get_doc(doctype)
for key, value in previous_settings.items():
setattr(settings, key, value)
settings.save()
settings.save(ignore_permissions=True)
frappe.db.value_cache[settings] = {}


Expand Down

0 comments on commit 758873c

Please sign in to comment.