Merge pull request #21543 from anandbaburajan/unattached_guest_files

frappe/core/doctype/file/file.py

@@ -724,7 +724,7 @@ def has_permission(doc, ptype=None, user=None):
 	if ptype == "create":
 		return frappe.has_permission("File", "create", user=user)
 
-	if not doc.is_private or doc.owner == user or user == "Administrator":
+	if not doc.is_private or (user != "Guest" and doc.owner == user) or user == "Administrator":
 		return True
 
 	if doc.attached_to_doctype and doc.attached_to_name:

frappe/core/doctype/file/test_file.py

@@ -29,11 +29,11 @@ test_content1 = "Hello"
 test_content2 = "Hello World"
 
 
-def make_test_doc():
+def make_test_doc(ignore_permissions=False):
 	d = frappe.new_doc("ToDo")
 	d.description = "Test"
 	d.assigned_by = frappe.session.user
-	d.save()
+	d.save(ignore_permissions)
 	return d.doctype, d.name
 
 
@@ -785,3 +785,81 @@ class TestFileOptimization(FrappeTestCase):
 			file_content = f.read()
 
 		self.assertEqual(get_extension("", None, file_content), "jpg")
+
+
+class TestGuestFileAndAttachments(FrappeTestCase):
+	def setUp(self) -> None:
+		frappe.db.delete("File", {"is_folder": 0})
+		frappe.get_doc(
+			doctype="DocType",
+			name="Test For Attachment",
+			module="Custom",
+			custom=1,
+			fields=[
+				{"label": "Title", "fieldname": "title", "fieldtype": "Data"},
+				{"label": "Attachment", "fieldname": "attachment", "fieldtype": "Attach"},
+			],
+		).insert(ignore_if_duplicate=True)
+
+	def tearDown(self) -> None:
+		frappe.set_user("Administrator")
+		frappe.db.rollback()
+		frappe.delete_doc("DocType", "Test For Attachment")
+
+	def test_attach_unattached_guest_file(self):
+		"""Ensure that unattached files are attached on doc update."""
+		f = frappe.new_doc(
+			"File",
+			file_name="test_private_guest_attachment.txt",
+			content="Guest Home",
+			is_private=1,
+		).insert(ignore_permissions=True)
+
+		d = frappe.new_doc("Test For Attachment")
+		d.title = "Test for attachment on update"
+		d.attachment = f.file_url
+		d.assigned_by = frappe.session.user
+		d.save()
+
+		self.assertTrue(
+			frappe.db.exists(
+				"File",
+				{
+					"file_name": "test_private_guest_attachment.txt",
+					"file_url": f.file_url,
+					"attached_to_doctype": "Test For Attachment",
+					"attached_to_name": d.name,
+					"attached_to_field": "attachment",
+				},
+			)
+		)
+
+	def test_list_private_guest_single_file(self):
+		"""Ensure that guests are not able to read private standalone guest files."""
+		frappe.set_user("Guest")
+
+		file = frappe.new_doc(
+			"File",
+			file_name="test_private_guest_single_txt",
+			content="Private single File",
+			is_private=1,
+		).insert(ignore_permissions=True)
+
+		self.assertFalse(file.is_downloadable())
+
+	def test_list_private_guest_attachment(self):
+		"""Ensure that guests are not able to read private guest attachments."""
+		frappe.set_user("Guest")
+
+		self.attached_to_doctype, self.attached_to_docname = make_test_doc(ignore_permissions=True)
+
+		file = frappe.new_doc(
+			"File",
+			file_name="test_private_guest_attachment.txt",
+			attached_to_doctype=self.attached_to_doctype,
+			attached_to_name=self.attached_to_docname,
+			content="Private Attachment",
+			is_private=1,
+		).insert(ignore_permissions=True)
+
+		self.assertFalse(file.is_downloadable())

frappe/core/doctype/file/utils.py

@@ -294,10 +294,11 @@ def update_existing_file_docs(doc: "File") -> None:
 	).run()
 
 
-def attach_files_to_document(doc: "File", event) -> None:
+def attach_files_to_document(doc: "Document", event) -> None:
 	"""Runs on on_update hook of all documents.
-	Goes through every Attach and Attach Image field and attaches
-	the file url to the document if it is not already attached.
+	Goes through every file linked with the Attach and Attach Image field and attaches
+	the file to the document if not already attached. If no file is found, a new file
+	is created.
 	"""
 
 	attach_fields = doc.meta.get("fields", {"fieldtype": ["in", ["Attach", "Attach Image"]]})
@@ -320,6 +321,28 @@ def attach_files_to_document(doc: "File", event) -> None:
 		):
 			return
 
+		unattached_file = frappe.db.exists(
+			"File",
+			{
+				"file_url": value,
+				"attached_to_name": None,
+				"attached_to_doctype": None,
+				"attached_to_field": None,
+			},
+		)
+
+		if unattached_file:
+			frappe.db.set_value(
+				"File",
+				unattached_file,
+				field={
+					"attached_to_name": doc.name,
+					"attached_to_doctype": doc.doctype,
+					"attached_to_field": df.fieldname,
+				},
+			)
+			return
+
 		file: "File" = frappe.get_doc(
 			doctype="File",
 			file_url=value,