fix: ignore duplicate perm check on assign hooks (frappe#22832)

* fix: Ignore permissions while assigning if flag set

* fix: Avoid double permission checks on assignment rule

When it's triggered via doc events either:
- Permission check is done or
- Permission checks are not applicable

frappe/__init__.py

@@ -1713,6 +1713,7 @@ def get_newargs(fn: Callable, kwargs: dict[str, Any]) -> dict[str, Any]:
 		if (a in fnargs) or varkw_exist:
 			newargs[a] = kwargs.get(a)
 
+	# WARNING: This behaviour is now  part of business logic in places, never remove.
 	newargs.pop("ignore_permissions", None)
 	newargs.pop("flags", None)
 

frappe/automation/doctype/assignment_rule/assignment_rule.py

@@ -78,7 +78,7 @@ def apply_assign(self, doc):
 
 	def do_assignment(self, doc):
 		# clear existing assignment, to reassign
-		assign_to.clear(doc.get("doctype"), doc.get("name"))
+		assign_to.clear(doc.get("doctype"), doc.get("name"), ignore_permissions=True)
 
 		user = self.get_user(doc)
 
@@ -92,7 +92,8 @@ def do_assignment(self, doc):
 					assignment_rule=self.name,
 					notify=True,
 					date=doc.get(self.due_date_based_on) if self.due_date_based_on else None,
-				)
+				),
+				ignore_permissions=True,
 			)
 
 			# set for reference in round robin
@@ -104,12 +105,14 @@ def do_assignment(self, doc):
 	def clear_assignment(self, doc):
 		"""Clear assignments"""
 		if self.safe_eval("unassign_condition", doc):
-			return assign_to.clear(doc.get("doctype"), doc.get("name"))
+			return assign_to.clear(doc.get("doctype"), doc.get("name"), ignore_permissions=True)
 
 	def close_assignments(self, doc):
 		"""Close assignments"""
 		if self.safe_eval("close_condition", doc):
-			return assign_to.close_all_assignments(doc.get("doctype"), doc.get("name"))
+			return assign_to.close_all_assignments(
+				doc.get("doctype"), doc.get("name"), ignore_permissions=True
+			)
 
 	def get_user(self, doc):
 		"""

frappe/automation/doctype/assignment_rule/test_assignment_rule.py

@@ -113,6 +113,20 @@ def test_load_balancing(self):
 				len(frappe.get_all("ToDo", dict(allocated_to=user, reference_type=TEST_DOCTYPE))), 10
 			)
 
+	def test_assingment_on_guest_submissions(self):
+		"""Sometimes documents are inserted as guest, check if assignment rules run on them. Use case: Web Forms"""
+		with self.set_user("Guest"):
+			doc = _make_test_record(ignore_permissions=True, public=1)
+
+		# check assignment to *anyone*
+		self.assertTrue(
+			frappe.db.get_value(
+				"ToDo",
+				{"reference_type": TEST_DOCTYPE, "reference_name": doc.name, "status": "Open"},
+				"allocated_to",
+			),
+		)
+
 	def test_based_on_field(self):
 		self.assignment_rule.rule = "Based on Field"
 		self.assignment_rule.field = "owner"
@@ -373,13 +387,17 @@ def get_assignment_rule(days, assign=None):
 	return assignment_rule
 
 
-def _make_test_record(**kwargs):
+def _make_test_record(
+	*,
+	ignore_permissions=False,
+	**kwargs,
+):
 	doc = frappe.new_doc(TEST_DOCTYPE)
 
 	if kwargs:
 		doc.update(kwargs)
 
-	return doc.insert()
+	return doc.insert(ignore_permissions=ignore_permissions)
 
 
 def create_test_doctype(doctype: str):

frappe/desk/form/assign_to.py

@@ -39,7 +39,7 @@ def get(args=None):
 
 
 @frappe.whitelist()
-def add(args=None):
+def add(args=None, *, ignore_permissions=False):
 	"""add in someone's to do list
 	args = {
 	        "assign_to": [],
@@ -63,8 +63,8 @@ def add(args=None):
 			"status": "Open",
 			"allocated_to": assign_to,
 		}
-		parent_doc = frappe.get_doc(args["doctype"], args["name"])
-		parent_doc.check_permission()
+		if not ignore_permissions:
+			frappe.get_doc(args["doctype"], args["name"]).check_permission()
 
 		if frappe.get_all("ToDo", filters=filters):
 			users_with_duplicate_todo.append(assign_to)
@@ -146,7 +146,7 @@ def add_multiple(args=None):
 		add(args)
 
 
-def close_all_assignments(doctype, name):
+def close_all_assignments(doctype, name, ignore_permissions=False):
 	assignments = frappe.get_all(
 		"ToDo",
 		fields=["allocated_to", "name"],
@@ -156,29 +156,42 @@ def close_all_assignments(doctype, name):
 		return False
 
 	for assign_to in assignments:
-		set_status(doctype, name, todo=assign_to.name, assign_to=assign_to.allocated_to, status="Closed")
+		set_status(
+			doctype,
+			name,
+			todo=assign_to.name,
+			assign_to=assign_to.allocated_to,
+			status="Closed",
+			ignore_permissions=ignore_permissions,
+		)
 
 	return True
 
 
 @frappe.whitelist()
-def remove(doctype, name, assign_to):
-	return set_status(doctype, name, "", assign_to, status="Cancelled")
+def remove(doctype, name, assign_to, ignore_permissions=False):
+	return set_status(
+		doctype, name, "", assign_to, status="Cancelled", ignore_permissions=ignore_permissions
+	)
 
 
 @frappe.whitelist()
-def close(doctype: str, name: str, assign_to: str):
+def close(doctype: str, name: str, assign_to: str, ignore_permissions=False):
 	if assign_to != frappe.session.user:
 		frappe.throw(_("Only the assignee can complete this to-do."))
 
-	return set_status(doctype, name, "", assign_to, status="Closed")
+	return set_status(
+		doctype, name, "", assign_to, status="Closed", ignore_permissions=ignore_permissions
+	)
 
 
-def set_status(doctype, name, todo=None, assign_to=None, status="Cancelled"):
+def set_status(
+	doctype, name, todo=None, assign_to=None, status="Cancelled", ignore_permissions=False
+):
 	"""remove from todo"""
 
-	doc = frappe.get_doc(doctype, name)
-	doc.check_permission()
+	if not ignore_permissions:
+		frappe.get_doc(doctype, name).check_permission()
 	try:
 		if not todo:
 			todo = frappe.db.get_value(
@@ -206,7 +219,7 @@ def set_status(doctype, name, todo=None, assign_to=None, status="Cancelled"):
 	return get({"doctype": doctype, "name": name})
 
 
-def clear(doctype, name):
+def clear(doctype, name, ignore_permissions=False):
 	"""
 	Clears assignments, return False if not assigned.
 	"""
@@ -220,7 +233,12 @@ def clear(doctype, name):
 
 	for assign_to in assignments:
 		set_status(
-			doctype, name, todo=assign_to.name, assign_to=assign_to.allocated_to, status="Cancelled"
+			doctype,
+			name,
+			todo=assign_to.name,
+			assign_to=assign_to.allocated_to,
+			status="Cancelled",
+			ignore_permissions=ignore_permissions,
 		)
 
 	return True

frappe/tests/utils.py

@@ -140,22 +140,26 @@ def enable_safe_exec(cls) -> None:
 
 	@contextmanager
 	def set_user(self, user: str):
-		old_user = frappe.session.user
-		frappe.set_user(user)
-		yield
-		frappe.set_user(old_user)
+		try:
+			old_user = frappe.session.user
+			frappe.set_user(user)
+			yield
+		finally:
+			frappe.set_user(old_user)
 
 	@contextmanager
 	def switch_site(self, site: str):
 		"""Switch connection to different site.
 		Note: Drops current site connection completely."""
 
-		old_site = frappe.local.site
-		frappe.init(site, force=True)
-		frappe.connect()
-		yield
-		frappe.init(old_site, force=True)
-		frappe.connect()
+		try:
+			old_site = frappe.local.site
+			frappe.init(site, force=True)
+			frappe.connect()
+			yield
+		finally:
+			frappe.init(old_site, force=True)
+			frappe.connect()
 
 	@contextmanager
 	def freeze_time(self, time_to_freeze, *args, **kwargs):