fix(docshare): validate that the user has the permission they are trying to share
Signed-off-by: Akhil Narang <me@akhilnarang.dev>
This commit is contained in:
parent
5df262350f
commit
7854c75ffa
2 changed files with 73 additions and 17 deletions
|
|
@ -224,3 +224,26 @@ class TestDocShare(IntegrationTestCase):
|
|||
add,
|
||||
{"doctype": "Event", "name": self.event.name, "assign_to": ["test1@example.com"]},
|
||||
)
|
||||
|
||||
def test_cannot_share_without_permission(self):
|
||||
"""Test that users cannot share permissions they don't have."""
|
||||
# Users don't have write permission on Communication
|
||||
doc = frappe.new_doc("Communication", subject="Hello World").save()
|
||||
|
||||
try:
|
||||
frappe.set_user(self.user)
|
||||
|
||||
# Attempting to share with write permission should fail
|
||||
self.assertRaises(
|
||||
frappe.PermissionError,
|
||||
frappe.share.add,
|
||||
"Communication",
|
||||
doc.name,
|
||||
"test1@example.com",
|
||||
write=1,
|
||||
)
|
||||
|
||||
# Can share read
|
||||
frappe.share.add("Communication", doc.name, "test1@example.com")
|
||||
finally:
|
||||
doc.delete()
|
||||
|
|
|
|||
|
|
@ -42,20 +42,6 @@ def add_docshare(
|
|||
if not user:
|
||||
user = frappe.session.user
|
||||
|
||||
if not (flags or {}).get("ignore_share_permission"):
|
||||
check_share_permission(doctype, name)
|
||||
|
||||
share_name = get_share_name(doctype, name, user, everyone)
|
||||
|
||||
if share_name:
|
||||
doc = frappe.get_doc("DocShare", share_name)
|
||||
else:
|
||||
doc = frappe.new_doc("DocShare")
|
||||
doc.update({"user": user, "share_doctype": doctype, "share_name": name, "everyone": cint(everyone)})
|
||||
|
||||
if flags:
|
||||
doc.flags.update(flags)
|
||||
|
||||
share_perms = {
|
||||
# always add read, since you are adding!
|
||||
"read": 1,
|
||||
|
|
@ -69,6 +55,18 @@ def add_docshare(
|
|||
if ptype in kwargs:
|
||||
share_perms[ptype] = cint(kwargs.get(ptype))
|
||||
|
||||
if not (flags or {}).get("ignore_share_permission"):
|
||||
check_share_permission(doctype, name, share_perms, custom_perms)
|
||||
|
||||
if share_name := get_share_name(doctype, name, user, everyone):
|
||||
doc = frappe.get_doc("DocShare", share_name)
|
||||
else:
|
||||
doc = frappe.new_doc("DocShare")
|
||||
doc.update({"user": user, "share_doctype": doctype, "share_name": name, "everyone": cint(everyone)})
|
||||
|
||||
if flags:
|
||||
doc.flags.update(flags)
|
||||
|
||||
doc.update(share_perms)
|
||||
doc.save(ignore_permissions=True)
|
||||
notify_assignment(user, doctype, name, everyone, notify=notify)
|
||||
|
|
@ -95,7 +93,8 @@ def set_permission(doctype, name, user, permission_to, value=1, everyone=0):
|
|||
def set_docshare_permission(doctype, name, user, permission_to, value=1, everyone=0, flags=None):
|
||||
"""Set share permission."""
|
||||
if not (flags or {}).get("ignore_share_permission"):
|
||||
check_share_permission(doctype, name)
|
||||
permissions = {permission_to: value} if cint(value) else None
|
||||
check_share_permission(doctype, name, permissions)
|
||||
|
||||
share_name = get_share_name(doctype, name, user, everyone)
|
||||
value = int(value)
|
||||
|
|
@ -209,13 +208,47 @@ def get_share_name(doctype, name, user, everyone):
|
|||
return share_name
|
||||
|
||||
|
||||
def check_share_permission(doctype, name):
|
||||
"""Check if the user can share with other users"""
|
||||
def check_share_permission(doctype, name, permissions=None, custom_perms=None):
|
||||
"""Check if the user can share with other users and has the permissions they are trying to grant.
|
||||
|
||||
:param doctype: DocType being shared
|
||||
:param name: Document name being shared
|
||||
:param permissions: Permissions that the user wants to share
|
||||
:param custom_perms: List of custom permission types for the doctype
|
||||
"""
|
||||
if not frappe.has_permission(doctype, ptype="share", doc=name):
|
||||
frappe.throw(
|
||||
_("No permission to {0} {1} {2}").format("share", _(doctype), name), frappe.PermissionError
|
||||
)
|
||||
|
||||
# If permissions specified, validate that the user has those permissions
|
||||
if not permissions:
|
||||
return
|
||||
|
||||
# Validate user has the permissions they're trying to grant
|
||||
restricted_permissions = ["read", "write", "submit"]
|
||||
|
||||
# Append custom permissions
|
||||
if custom_perms is None:
|
||||
custom_perms = get_doctype_ptype_map().get(doctype, [])
|
||||
restricted_permissions.extend(custom_perms)
|
||||
|
||||
from frappe.permissions import get_role_permissions
|
||||
|
||||
doc = frappe.get_doc(doctype, name)
|
||||
is_owner = (doc.get("owner") or "").lower() == frappe.session.user.lower()
|
||||
|
||||
user_perms = get_role_permissions(doc.meta, user=frappe.session.user, is_owner=is_owner)
|
||||
|
||||
for ptype in restricted_permissions:
|
||||
if cint(permissions.get(ptype)) and not user_perms.get(ptype):
|
||||
frappe.throw(
|
||||
_("You cannot share `{0}` on {1} `{2}` as you do not have `{0}` permission on `{1}`").format(
|
||||
_(ptype), _(doctype), name
|
||||
),
|
||||
frappe.PermissionError,
|
||||
)
|
||||
|
||||
|
||||
def notify_assignment(shared_by, doctype, doc_name, everyone, notify=0):
|
||||
if not (shared_by and doctype and doc_name) or everyone or not notify:
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue