Merge pull request #20877 from barredterra/file-permissions

This commit is contained in:
Suraj Shetty 2023-06-27 09:22:09 +05:30 committed by GitHub
commit 92c24d9abb
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 89 additions and 66 deletions

View file

@ -174,7 +174,7 @@
"icon": "fa fa-file",
"idx": 1,
"links": [],
"modified": "2022-09-13 15:50:15.508251",
"modified": "2023-05-02 15:42:14.274901",
"modified_by": "Administrator",
"module": "Core",
"name": "File",
@ -196,14 +196,8 @@
{
"create": 1,
"delete": 1,
"email": 1,
"export": 1,
"if_owner": 1,
"print": 1,
"read": 1,
"report": 1,
"role": "All",
"share": 1,
"write": 1
}
],

View file

@ -16,6 +16,7 @@ import frappe
from frappe import _
from frappe.database.schema import SPECIAL_CHAR_PATTERN
from frappe.model.document import Document
from frappe.permissions import get_doctypes_with_read
from frappe.utils import call_hook_method, cint, get_files_path, get_hook_method, get_url
from frappe.utils.file_manager import is_safe_path
from frappe.utils.image import optimize_image, strip_exif_data
@ -718,40 +719,39 @@ def on_doctype_update():
def has_permission(doc, ptype=None, user=None):
has_access = False
user = user or frappe.session.user
if ptype == "create":
has_access = frappe.has_permission("File", "create", user=user)
return frappe.has_permission("File", "create", user=user)
if not doc.is_private or doc.owner in [user, "Guest"] or user == "Administrator":
has_access = True
if not doc.is_private or doc.owner == user or user == "Administrator":
return True
if doc.attached_to_doctype and doc.attached_to_name:
attached_to_doctype = doc.attached_to_doctype
attached_to_name = doc.attached_to_name
try:
ref_doc = frappe.get_doc(attached_to_doctype, attached_to_name)
ref_doc = frappe.get_doc(attached_to_doctype, attached_to_name)
if ptype in ["write", "create", "delete"]:
has_access = ref_doc.has_permission("write")
if ptype in ["write", "create", "delete"]:
return ref_doc.has_permission("write")
else:
return ref_doc.has_permission("read")
if ptype == "delete" and not has_access:
frappe.throw(
_(
"Cannot delete file as it belongs to {0} {1} for which you do not have permissions"
).format(doc.attached_to_doctype, doc.attached_to_name),
frappe.PermissionError,
)
else:
has_access = ref_doc.has_permission("read")
except frappe.DoesNotExistError:
# if parent doc is not created before file is created
# we cannot check its permission so we will use file's permission
pass
return False
return has_access
def get_permission_query_conditions(user: str = None) -> str:
user = user or frappe.session.user
if user == "Administrator":
return ""
readable_doctypes = ", ".join(repr(dt) for dt in get_doctypes_with_read())
return f"""
(`tabFile`.`is_private` = 0)
OR (`tabFile`.`attached_to_doctype` IS NULL AND `tabFile`.`owner` = {user !r})
OR (`tabFile`.`attached_to_doctype` IN ({readable_doctypes}))
"""
# Note: kept at the end to not cause circular, partial imports & maintain backwards compatibility

View file

@ -611,46 +611,42 @@ class TestAttachmentsAccess(FrappeTestCase):
def setUp(self) -> None:
frappe.db.delete("File", {"is_folder": 0})
def test_attachments_access(self):
def test_list_private_attachments(self):
frappe.set_user("test4@example.com")
self.attached_to_doctype, self.attached_to_docname = make_test_doc()
frappe.get_doc(
{
"doctype": "File",
"file_name": "test_user.txt",
"attached_to_doctype": self.attached_to_doctype,
"attached_to_name": self.attached_to_docname,
"content": "Testing User",
}
frappe.new_doc(
"File",
file_name="test_user_attachment.txt",
attached_to_doctype=self.attached_to_doctype,
attached_to_name=self.attached_to_docname,
content="Testing User",
is_private=1,
).insert()
frappe.get_doc(
{
"doctype": "File",
"file_name": "test_user_home.txt",
"content": "User Home",
}
frappe.new_doc(
"File",
file_name="test_user_standalone.txt",
content="User Home",
is_private=1,
).insert()
frappe.set_user("test@example.com")
frappe.get_doc(
{
"doctype": "File",
"file_name": "test_system_manager.txt",
"attached_to_doctype": self.attached_to_doctype,
"attached_to_name": self.attached_to_docname,
"content": "Testing System Manager",
}
frappe.new_doc(
"File",
file_name="test_sm_attachment.txt",
attached_to_doctype=self.attached_to_doctype,
attached_to_name=self.attached_to_docname,
content="Testing System Manager",
is_private=1,
).insert()
frappe.get_doc(
{
"doctype": "File",
"file_name": "test_sm_home.txt",
"content": "System Manager Home",
}
frappe.new_doc(
"File",
file_name="test_sm_standalone.txt",
content="System Manager Home",
is_private=1,
).insert()
system_manager_files = [file.file_name for file in get_files_in_folder("Home")["files"]]
@ -664,15 +660,47 @@ class TestAttachmentsAccess(FrappeTestCase):
file.file_name for file in get_files_in_folder("Home/Attachments")["files"]
]
self.assertIn("test_sm_home.txt", system_manager_files)
self.assertNotIn("test_sm_home.txt", user_files)
self.assertIn("test_user_home.txt", system_manager_files)
self.assertIn("test_user_home.txt", user_files)
self.assertIn("test_sm_standalone.txt", system_manager_files)
self.assertNotIn("test_sm_standalone.txt", user_files)
self.assertIn("test_system_manager.txt", system_manager_attachments_files)
self.assertNotIn("test_system_manager.txt", user_attachments_files)
self.assertIn("test_user.txt", system_manager_attachments_files)
self.assertIn("test_user.txt", user_attachments_files)
self.assertIn("test_user_standalone.txt", user_files)
self.assertNotIn("test_user_standalone.txt", system_manager_files)
self.assertIn("test_sm_attachment.txt", system_manager_attachments_files)
self.assertIn("test_sm_attachment.txt", user_attachments_files)
self.assertIn("test_user_attachment.txt", system_manager_attachments_files)
self.assertIn("test_user_attachment.txt", user_attachments_files)
def test_list_public_single_file(self):
"""Ensure that users are able to list public standalone files."""
frappe.set_user("test@example.com")
frappe.new_doc(
"File",
file_name="test_public_single.txt",
content="Public single File",
is_private=0,
).insert()
frappe.set_user("test4@example.com")
files = [file.file_name for file in get_files_in_folder("Home")["files"]]
self.assertIn("test_public_single.txt", files)
def test_list_public_attachment(self):
"""Ensure that users are able to list public attachments."""
frappe.set_user("test@example.com")
self.attached_to_doctype, self.attached_to_docname = make_test_doc()
frappe.new_doc(
"File",
file_name="test_public_attachment.txt",
attached_to_doctype=self.attached_to_doctype,
attached_to_name=self.attached_to_docname,
content="Public Attachment",
is_private=0,
).insert()
frappe.set_user("test4@example.com")
files = [file.file_name for file in get_files_in_folder("Home/Attachments")["files"]]
self.assertIn("test_public_attachment.txt", files)
def tearDown(self) -> None:
frappe.set_user("Administrator")

View file

@ -108,6 +108,7 @@ permission_query_conditions = {
"Communication": "frappe.core.doctype.communication.communication.get_permission_query_conditions_for_communication",
"Workflow Action": "frappe.workflow.doctype.workflow_action.workflow_action.get_permission_query_conditions",
"Prepared Report": "frappe.core.doctype.prepared_report.prepared_report.get_permission_query_condition",
"File": "frappe.core.doctype.file.file.get_permission_query_conditions",
}
has_permission = {