Merge pull request frappe#23904 from cogk/fix-fieldlevel-access-to-sh…

…ared-document-list

fix(meta)!: Allow level 0 fields if a doc has been shared with user

frappe/core/doctype/docshare/test_docshare.py

@@ -56,6 +56,24 @@ def test_doc_permission(self):
 		with self.assertRowsRead(1):
 			self.assertTrue(self.event.has_permission())
 
+	def test_list_permission(self):
+		frappe.set_user(self.user)
+		with self.assertRaises(frappe.PermissionError):
+			frappe.get_list("Web Page")
+
+		frappe.set_user("Administrator")
+		doc = frappe.new_doc("Web Page")
+		doc.update({"title": "test document for docshare permissions"})
+		doc.insert()
+		frappe.share.add("Web Page", doc.name, self.user)
+
+		frappe.set_user(self.user)
+		self.assertEqual(len(frappe.get_list("Web Page")), 1)
+
+		doc.delete(ignore_permissions=True)
+		with self.assertRaises(frappe.PermissionError):
+			frappe.get_list("Web Page")
+
 	def test_share_permission(self):
 		frappe.share.add("Event", self.event.name, self.user, write=1, share=1)
 

frappe/model/__init__.py

@@ -230,6 +230,9 @@ def get_permitted_fields(
 	if permission_type is None:
 		permission_type = "select" if frappe.only_has_select_perm(doctype, user=user) else "read"
 
+	meta_fields = meta.default_fields.copy()
+	optional_meta_fields = [x for x in optional_fields if x in valid_columns]
+
 	if permitted_fields := meta.get_permitted_fieldnames(
 		parenttype=parenttype,
 		user=user,
@@ -239,15 +242,12 @@ def get_permitted_fields(
 		if permission_type == "select":
 			return permitted_fields
 
-		meta_fields = meta.default_fields.copy()
-		optional_meta_fields = [x for x in optional_fields if x in valid_columns]
-
 		if meta.istable:
 			meta_fields.extend(child_table_fields)
 
 		return meta_fields + permitted_fields + optional_meta_fields
 
-	return []
+	return meta_fields + optional_meta_fields
 
 
 def is_default_field(fieldname: str) -> bool:

frappe/model/meta.py

@@ -596,6 +596,10 @@ def get_permitted_fieldnames(
 			self.get_permlevel_access(permission_type=permission_type, parenttype=parenttype, user=user)
 		)
 
+		if 0 not in permlevel_access and permission_type in ("read", "select"):
+			if frappe.share.get_shared(self.name, user, rights=[permission_type], limit=1):
+				permlevel_access.add(0)
+
 		permitted_fieldnames.extend(
 			df.fieldname
 			for df in self.get_fieldnames_with_value(

frappe/tests/test_model_utils.py

@@ -36,10 +36,10 @@ def test_get_permitted_fields(self):
 		todo_all_columns = frappe.get_meta("ToDo").get_valid_columns()
 		self.assertListEqual(todo_all_fields, todo_all_columns)
 
-		# Guest should have access to no fields in ToDo
+		# Guest should have access to no non-std fields in ToDo
 		with set_user("Guest"):
 			guest_permitted_fields = get_permitted_fields("ToDo")
-			self.assertEqual(guest_permitted_fields, [])
+			self.assertNotIn("description", guest_permitted_fields)
 
 		# everyone should have access to all fields of core doctypes
 		with set_user("Guest"):
@@ -55,15 +55,14 @@ def test_get_permitted_fields(self):
 				"Installed Application", parenttype="Installed Applications"
 			)
 			child_all_fields = frappe.get_meta("Installed Application").get_valid_columns()
-			self.assertEqual(without_parent_fields, [])
 			self.assertLess(len(without_parent_fields), len(with_parent_fields))
 			self.assertSequenceEqual(set(with_parent_fields), set(child_all_fields))
 
-		# guest has access to no fields
+		# guest has access to no non-std fields
 		with set_user("Guest"):
-			self.assertEqual(get_permitted_fields("Installed Application"), [])
-			self.assertEqual(
-				get_permitted_fields("Installed Application", parenttype="Installed Applications"), []
+			self.assertNotIn("app_name", get_permitted_fields("Installed Application"))
+			self.assertNotIn(
+				"app_name", get_permitted_fields("Installed Application", parenttype="Installed Applications")
 			)
 
 	def test_is_default_field(self):