Merge pull request #24828 from marination/txt-attachment-privacy

fix: Use doctype setting to set auto-extracted file as private

frappe/core/doctype/communication/communication.json

@@ -401,7 +401,8 @@
  "icon": "fa fa-comment",
  "idx": 1,
  "links": [],
- "modified": "2023-11-27 20:38:27.467076",
+ "make_attachments_public": 1,
+ "modified": "2024-02-09 12:10:01.200845",
  "modified_by": "Administrator",
  "module": "Core",
  "name": "Communication",

frappe/core/doctype/file/test_file.py

@@ -18,8 +18,8 @@
 	unzip_file,
 )
 from frappe.core.doctype.file.exceptions import FileTypeNotAllowed
-from frappe.core.doctype.file.utils import delete_file, get_extension
-from frappe.core.doctype.user.test_user import test_user
+from frappe.core.doctype.file.utils import get_extension
+from frappe.desk.form.utils import add_comment
 from frappe.exceptions import ValidationError
 from frappe.tests.utils import FrappeTestCase, change_settings
 from frappe.utils import get_files_path, set_request
@@ -691,27 +691,65 @@ def tearDown(self) -> None:
 
 class TestFileUtils(FrappeTestCase):
 	def test_extract_images_from_doc(self):
+		is_private = not frappe.get_meta("ToDo").make_attachments_public
+
 		# with filename in data URI
 		todo = frappe.get_doc(
-			{
-				"doctype": "ToDo",
-				"description": 'Test <img src="data:image/png;filename=pix.png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=">',
-			}
+			doctype="ToDo",
+			description='Test <img src="data:image/png;filename=pix.png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=">',
 		).insert()
-		self.assertTrue(frappe.db.exists("File", {"attached_to_name": todo.name}))
-		self.assertIn('<img src="/files/pix.png">', todo.description)
-		self.assertListEqual(get_attached_images("ToDo", [todo.name])[todo.name], ["/files/pix.png"])
+		self.assertTrue(frappe.db.exists("File", {"attached_to_name": todo.name, "is_private": is_private}))
+		self.assertRegex(todo.description, r"<img src=\"(.*)/files/pix\.png(.*)\">")
+		self.assertListEqual(get_attached_images("ToDo", [todo.name])[todo.name], ["/private/files/pix.png"])
 
 		# without filename in data URI
 		todo = frappe.get_doc(
-			{
-				"doctype": "ToDo",
-				"description": 'Test <img src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=">',
-			}
+			doctype="ToDo",
+			description='Test <img src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=">',
 		).insert()
 		filename = frappe.db.exists("File", {"attached_to_name": todo.name})
 		self.assertIn(f'<img src="{frappe.get_doc("File", filename).file_url}', todo.description)
 
+	def test_extract_images_from_comment(self):
+		"""
+		Ensure that images are extracted from comments and become private attachments.
+		"""
+		is_private = not frappe.get_meta("ToDo").make_attachments_public
+		test_doc = frappe.get_doc(doctype="ToDo", description="comment test").insert()
+		comment = add_comment(
+			"ToDo",
+			test_doc.name,
+			'<div class="ql-editor read-mode"><img src="data:image/png;filename=pix.png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII="></div>',
+			frappe.session.user,
+			frappe.session.user,
+		)
+
+		self.assertTrue(
+			frappe.db.exists("File", {"attached_to_name": test_doc.name, "is_private": is_private})
+		)
+		self.assertRegex(comment.content, r"<img src=\"(.*)/files/pix\.png(.*)\">")
+
+	def test_extract_images_from_communication(self):
+		"""
+		Ensure that images are extracted from communication and become public attachments.
+		"""
+		is_private = not frappe.get_meta("Communication").make_attachments_public
+		communication = frappe.get_doc(
+			doctype="Communication",
+			communication_type="Communication",
+			communication_medium="Email",
+			content='<div class="ql-editor read-mode"><img src="data:image/png;filename=pix.png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII="></div>',
+			recipients="to <to@test.com>",
+			cc=None,
+			bcc=None,
+			sender="sender@test.com",
+		).insert(ignore_permissions=True)
+
+		self.assertTrue(
+			frappe.db.exists("File", {"attached_to_name": communication.name, "is_private": is_private})
+		)
+		self.assertRegex(communication.content, r"<img src=\"(.*)/files/pix\.png(.*)\">")
+
 	def test_create_new_folder(self):
 		folder = create_new_folder("test_folder", "Home")
 		self.assertTrue(folder.is_folder)

frappe/core/doctype/file/utils.py

@@ -218,7 +218,7 @@ def get_file_name(fname: str, optional_suffix: str | None = None) -> str:
 
 def extract_images_from_doc(doc: "Document", fieldname: str):
 	content = doc.get(fieldname)
-	content = extract_images_from_html(doc, content)
+	content = extract_images_from_html(doc, content, is_private=(not doc.meta.make_attachments_public))
 	if frappe.flags.has_dataurl:
 		doc.set(fieldname, content)