fix: ensure consistent error in response

frappe/__init__.py

@@ -481,7 +481,7 @@ Action: TypeAlias = ServerAction | ClientAction
 def msgprint(
 	msg: str,
 	title: str | None = None,
-	raise_exception: bool | type[Exception] = False,
+	raise_exception: bool | type[Exception] | Exception = False,
 	as_table: bool = False,
 	as_list: bool = False,
 	indicator: Literal["blue", "green", "orange", "red", "yellow"] | None = None,
@@ -515,6 +515,9 @@ def msgprint(
 		if raise_exception:
 			if inspect.isclass(raise_exception) and issubclass(raise_exception, Exception):
 				exc = raise_exception(msg)
+			elif isinstance(raise_exception, Exception):
+				exc = raise_exception
+				exc.args = (msg,)
 			else:
 				exc = ValidationError(msg)
 			if out.__frappe_exc_id:
@@ -590,7 +593,7 @@ def clear_last_message():
 
 def throw(
 	msg: str,
-	exc: type[Exception] = ValidationError,
+	exc: type[Exception] | Exception = ValidationError,
 	title: str | None = None,
 	is_minimizable: bool = False,
 	wide: bool = False,
@@ -987,6 +990,7 @@ def has_permission(
 	*,
 	parent_doctype=None,
 	debug=False,
+	ignore_share_permissions=False,
 ):
 	"""
 	Return True if the user has permission `ptype` for given `doctype` or `doc`.
@@ -1012,6 +1016,7 @@ def has_permission(
 		print_logs=throw,
 		parent_doctype=parent_doctype,
 		debug=debug,
+		ignore_share_permissions=ignore_share_permissions,
 	)
 
 	if throw and not out:
@@ -1275,7 +1280,7 @@ def get_last_doc(
 	if d:
 		return get_doc(doctype, d[0], for_update=for_update)
 	else:
-		raise DoesNotExistError
+		raise DoesNotExistError(doctype=doctype)
 
 
 def get_single(doctype):

frappe/app.py

@@ -22,6 +22,7 @@ import frappe.utils.response
 from frappe import _
 from frappe.auth import SAFE_HTTP_METHODS, UNSAFE_HTTP_METHODS, HTTPRequest, check_request_ip, validate_auth
 from frappe.middlewares import StaticDataMiddleware
+from frappe.permissions import handle_does_not_exist_error
 from frappe.utils import CallbackManager, cint, get_site_name
 from frappe.utils.data import escape_html
 from frappe.utils.error import log_error, log_error_snapshot
@@ -324,6 +325,8 @@ def make_form_dict(request: Request):
 
 
 def handle_exception(e):
+	e = handle_does_not_exist_error(e)
+
 	response = None
 	http_status_code = getattr(e, "http_status_code", 500)
 	accept_header = frappe.get_request_header("Accept") or ""

frappe/client.py

@@ -484,13 +484,17 @@ def delete_doc(doctype, name):
 	if frappe.is_table(doctype):
 		values = frappe.db.get_value(doctype, name, ["parenttype", "parent", "parentfield"])
 		if not values:
-			raise frappe.DoesNotExistError
+			raise frappe.DoesNotExistError(doctype=doctype)
+
 		parenttype, parent, parentfield = values
 		parent = frappe.get_doc(parenttype, parent)
+		if not parent.has_permission("write"):
+			raise frappe.DoesNotExistError(doctype=doctype)
+
 		for row in parent.get(parentfield):
 			if row.name == name:
 				parent.remove(row)
-				parent.save()
+				parent.save(ignore_permissions=True)
 				break
 	else:
 		frappe.delete_doc(doctype, name, ignore_missing=False)

frappe/core/doctype/communication/email.py

@@ -9,6 +9,7 @@ import frappe
 import frappe.email.smtp
 from frappe import _
 from frappe.email.email_body import get_message_id
+from frappe.permissions import check_doctype_permission
 from frappe.utils import (
 	cint,
 	get_datetime,
@@ -78,8 +79,9 @@ def make(
 			category=DeprecationWarning,
 		)
 
-	if doctype and name and not frappe.has_permission(doctype=doctype, ptype="email", doc=name):
-		raise frappe.PermissionError(f"You are not allowed to send emails related to: {doctype} {name}")
+	if doctype and name:
+		doc = frappe.get_doc(doctype, name)
+		doc.check_permission("email")
 
 	return _make(
 		doctype=doctype,

frappe/desk/form/load.py

@@ -12,7 +12,7 @@ import frappe.utils
 from frappe import _, _dict
 from frappe.desk.form.document_follow import is_document_followed
 from frappe.model.utils.user_settings import get_user_settings
-from frappe.permissions import get_doc_permissions, has_permission
+from frappe.permissions import check_doctype_permission, get_doc_permissions, has_permission
 from frappe.utils.data import cstr
 
 if typing.TYPE_CHECKING:
@@ -33,6 +33,7 @@ def getdoc(doctype, name):
 	try:
 		doc = frappe.get_doc(doctype, name)
 	except frappe.DoesNotExistError:
+		check_doctype_permission(doctype)
 		frappe.clear_last_message()
 		return []
 

frappe/exceptions.py

@@ -44,6 +44,10 @@ class PermissionError(Exception):
 class DoesNotExistError(ValidationError):
 	http_status_code = 404
 
+	def __init__(self, *args, doctype=None):
+		super().__init__(*args)
+		self.doctype = doctype
+
 
 class PageDoesNotExistError(ValidationError):
 	http_status_code = 404
@@ -302,12 +306,6 @@ class LinkExpired(ValidationError):
 	message = "The link has expired"
 
 
-class InvalidKeyError(ValidationError):
-	http_status_code = 401
-	title = "Invalid Key"
-	message = "The document key is invalid"
-
-
 class CommandFailedError(Exception):
 	def __init__(self, message: str, out: str, err: str):
 		super().__init__(message)

frappe/handler.py

@@ -13,6 +13,7 @@ import frappe.utils
 from frappe import _, is_whitelisted, ping
 from frappe.core.doctype.server_script.server_script_utils import get_server_script_map
 from frappe.monitor import add_data_to_monitor
+from frappe.permissions import check_doctype_permission
 from frappe.utils import cint
 from frappe.utils.csvutils import build_csv_response
 from frappe.utils.deprecations import deprecated
@@ -204,18 +205,22 @@ def upload_file():
 
 
 def check_write_permission(doctype: str | None = None, name: str | None = None):
-	check_doctype = doctype and not name
-	if doctype and name:
-		try:
-			doc = frappe.get_doc(doctype, name)
-			doc.check_permission("write")
-		except frappe.DoesNotExistError:
-			# doc has not been inserted yet, name is set to "new-some-doctype"
-			# If doc inserts fine then only this attachment will be linked see file/utils.py:relink_mismatched_files
-			return
+	if not doctype:
+		return
 
-	if check_doctype:
+	if not name:
 		frappe.has_permission(doctype, "write", throw=True)
+		return
+
+	try:
+		doc = frappe.get_doc(doctype, name)
+	except frappe.DoesNotExistError:
+		# doc has not been inserted yet, name is set to "new-some-doctype"
+		# If doc inserts fine then only this attachment will be linked see file/utils.py:relink_mismatched_files
+		check_doctype_permission(doctype, "write")
+		return
+
+	doc.check_permission("write")
 
 
 @frappe.whitelist(allow_guest=True)

frappe/model/delete_doc.py

@@ -55,7 +55,7 @@ def delete_doc(
 		# already deleted..?
 		if not frappe.db.exists(doctype, name):
 			if not ignore_missing:
-				raise frappe.DoesNotExistError
+				raise frappe.DoesNotExistError(doctype=doctype)
 			else:
 				return False
 

frappe/model/document.py

@@ -254,7 +254,8 @@ class Document(BaseDocument, DocRef):
 
 			if not d:
 				frappe.throw(
-					_("{0} {1} not found").format(_(self.doctype), self.name), frappe.DoesNotExistError
+					_("{0} {1} not found").format(_(self.doctype), self.name),
+					frappe.DoesNotExistError(doctype=self.doctype),
 				)
 
 			super().__init__(d)
@@ -320,7 +321,7 @@ class Document(BaseDocument, DocRef):
 	def check_permission(self, permtype="read", permlevel=None):
 		"""Raise `frappe.PermissionError` if not permitted"""
 		if not self.has_permission(permtype):
-			self.raise_no_permission_to(permtype)
+			self._handle_permission_failure(permtype)
 
 	def has_permission(self, permtype="read", *, debug=False, user=None) -> bool:
 		"""
@@ -336,6 +337,12 @@ class Document(BaseDocument, DocRef):
 
 		return frappe.permissions.has_permission(self.doctype, permtype, self, debug=debug, user=user)
 
+	def _handle_permission_failure(self, perm_type):
+		from frappe.permissions import check_doctype_permission
+
+		check_doctype_permission(self.doctype, perm_type)
+		self.raise_no_permission_to(perm_type)
+
 	def raise_no_permission_to(self, perm_type):
 		"""Raise `frappe.PermissionError`."""
 		frappe.flags.error_message = _(

frappe/permissions.py

@@ -83,6 +83,7 @@ def has_permission(
 	parent_doctype=None,
 	print_logs=True,
 	debug=False,
+	ignore_share_permissions=False,
 ) -> bool:
 	"""Return True if user has permission `ptype` for given `doctype`.
 	If `doc` is passed, also check user, share and owner permissions.
@@ -190,7 +191,7 @@ def has_permission(
 
 		return False
 
-	if not perm:
+	if not perm and not ignore_share_permissions:
 		debug and _debug_log("Checking if document/doctype is explicitly shared with user")
 		perm = false_if_not_shared()
 
@@ -833,3 +834,30 @@ def has_child_permission(
 
 def is_system_user(user: str | None = None) -> bool:
 	return frappe.get_cached_value("User", user or frappe.session.user, "user_type") == "System User"
+
+
+def check_doctype_permission(doctype: str, ptype: str = "read") -> None:
+	"""
+	Designed specfically to override DoesNotExistError in some scenarios.
+	Ignores share permissions.
+	"""
+
+	_message_log = frappe.local.message_log
+	frappe.local.message_log = []
+	try:
+		frappe.has_permission(doctype, ptype, throw=True, ignore_share_permissions=True)
+	except frappe.PermissionError:
+		frappe.flags.disable_traceback = True
+		raise
+
+	frappe.local.message_log = _message_log
+
+
+def handle_does_not_exist_error(e: Exception) -> None:
+	if not isinstance(e, frappe.DoesNotExistError) or not (doctype := getattr(e, "doctype", None)):
+		return e
+
+	try:
+		check_doctype_permission(doctype)
+	except frappe.PermissionError as _e:
+		return _e

frappe/realtime.py

@@ -115,9 +115,8 @@ def emit_via_redis(event, message, room):
 
 @frappe.whitelist(allow_guest=True)
 def has_permission(doctype: str, name: str) -> bool:
-	if not frappe.has_permission(doctype=doctype, doc=name, ptype="read"):
-		raise frappe.PermissionError
-
+	doc = frappe.get_doc(doctype, name)
+	doc.check_permission("read")
 	return True
 
 

frappe/tests/test_document.py

@@ -562,7 +562,7 @@ class TestDocumentWebView(IntegrationTestCase):
 
 		# without key
 		url_without_key = f"/ToDo/{todo.name}"
-		self.assertEqual(self.get(url_without_key).status, "404 NOT FOUND")
+		self.assertEqual(self.get(url_without_key).status, "403 FORBIDDEN")
 
 		# Logged-in user can access the page without key
 		self.assertEqual(self.get(url_without_key, "Administrator").status, "200 OK")

frappe/utils/response.py

@@ -34,11 +34,7 @@ def report_error(status_code):
 	"""Build error. Show traceback in developer mode"""
 	from frappe.api import ApiVersion, get_api_version
 
-	allow_traceback = (
-		(frappe.get_system_settings("allow_error_traceback") if frappe.db else False)
-		and not frappe.local.flags.disable_traceback
-		and (status_code != 404 or frappe.conf.logging)
-	)
+	allow_traceback = is_traceback_allowed() and (status_code != 404 or frappe.conf.logging)
 
 	traceback = frappe.utils.get_traceback()
 	exc_type, exc_value, _ = sys.exc_info()
@@ -62,6 +58,14 @@ def report_error(status_code):
 	return response
 
 
+def is_traceback_allowed():
+	return (
+		frappe.db
+		and frappe.get_system_settings("allow_error_traceback")
+		and not frappe.local.flags.disable_traceback
+	)
+
+
 def _link_error_with_message_log(error_log, exception, message_logs):
 	for message in list(message_logs):
 		if message.get("__frappe_exc_id") == getattr(exception, "__frappe_exc_id", None):
@@ -175,9 +179,8 @@ def _make_logs_v1():
 	from frappe.utils.error import guess_exception_source
 
 	response = frappe.local.response
-	allow_traceback = frappe.get_system_settings("allow_error_traceback") if frappe.db else False
 
-	if frappe.error_log and allow_traceback:
+	if frappe.error_log and is_traceback_allowed():
 		if source := guess_exception_source(frappe.local.error_log and frappe.local.error_log[0]["exc"]):
 			response["_exc_source"] = source
 		response["exc"] = json.dumps([frappe.utils.cstr(d["exc"]) for d in frappe.local.error_log])

frappe/website/doctype/web_form/web_form.py

@@ -10,6 +10,7 @@ from frappe.core.api.file import get_max_file_size
 from frappe.core.doctype.file.utils import remove_file_by_url
 from frappe.desk.form.meta import get_code_files_via_hooks
 from frappe.modules.utils import export_module_json, get_doc_module
+from frappe.permissions import check_doctype_permission
 from frappe.rate_limiter import rate_limit
 from frappe.utils import dict_with_keys, strip_html
 from frappe.utils.caching import redis_cache
@@ -164,9 +165,11 @@ def get_context(context):
 				)
 
 			if not frappe.db.exists(self.doc_type, frappe.form_dict.name):
+				check_doctype_permission(self.doc_type)
 				raise frappe.PageDoesNotExistError()
 
 			if not self.has_web_form_permission(self.doc_type, frappe.form_dict.name):
+				check_doctype_permission(self.doc_type)
 				frappe.throw(
 					_("You don't have the permissions to access this document"), frappe.PermissionError
 				)
@@ -610,7 +613,7 @@ def accept(web_form, data):
 
 
 @frappe.whitelist()
-def delete(web_form_name, docname):
+def delete(web_form_name: str, docname: str):
 	web_form = frappe.get_doc("Web Form", web_form_name)
 
 	owner = frappe.db.get_value(web_form.doc_type, docname, "owner")
@@ -621,7 +624,7 @@ def delete(web_form_name, docname):
 
 
 @frappe.whitelist()
-def delete_multiple(web_form_name, docnames):
+def delete_multiple(web_form_name: str, docnames: list[str]):
 	web_form = frappe.get_doc("Web Form", web_form_name)
 
 	docnames = json.loads(docnames)

frappe/website/page_renderers/print_page.py

@@ -10,11 +10,10 @@ class PrintPage(TemplatePage):
 
 	def can_render(self):
 		parts = self.path.split("/", 1)
-		if len(parts) == 2:
-			if frappe.db.exists("DocType", parts[0], True) and frappe.db.exists(parts[0], parts[1], True):
-				return True
+		if len(parts) != 2 or not frappe.db.exists("DocType", parts[0], True):
+			return False
 
-		return False
+		return True
 
 	def render(self):
 		parts = self.path.split("/", 1)

frappe/website/serve.py

@@ -1,6 +1,7 @@
 from werkzeug.wrappers import Response
 
 import frappe
+from frappe.permissions import handle_does_not_exist_error
 from frappe.website.page_renderers.error_page import ErrorPage
 from frappe.website.page_renderers.not_found_page import NotFoundPage
 from frappe.website.page_renderers.not_permitted_page import NotPermittedPage
@@ -10,24 +11,27 @@ from frappe.website.path_resolver import PathResolver
 
 def get_response(path=None, http_status_code=200) -> Response:
 	"""Resolves path and renders page"""
-	response = None
 	path = path or frappe.local.request.path
 	endpoint = path
 
 	try:
 		path_resolver = PathResolver(path, http_status_code)
 		endpoint, renderer_instance = path_resolver.resolve()
-		response = renderer_instance.render()
-	except frappe.Redirect as e:
-		return RedirectPage(endpoint or path, e.http_status_code).render()
-	except frappe.PermissionError as e:
-		response = NotPermittedPage(endpoint, http_status_code, exception=e).render()
-	except frappe.PageDoesNotExistError:
-		response = NotFoundPage(endpoint, http_status_code).render()
-	except Exception as e:
-		response = ErrorPage(exception=e).render()
+		return renderer_instance.render()
 
-	return response
+	except Exception as e:
+		e = handle_does_not_exist_error(e)
+
+		if isinstance(e, frappe.Redirect):
+			return RedirectPage(endpoint or path, e.http_status_code).render()
+
+		if isinstance(e, frappe.PermissionError):
+			return NotPermittedPage(endpoint, http_status_code, exception=e).render()
+
+		if isinstance(e, frappe.PageDoesNotExistError):
+			return NotFoundPage(endpoint, http_status_code).render()
+
+		return ErrorPage(exception=e).render()
 
 
 def get_response_content(path=None, http_status_code=200) -> str:

frappe/www/error.py

@@ -2,6 +2,7 @@
 # License: MIT. See LICENSE
 import frappe
 from frappe import _
+from frappe.utils.response import is_traceback_allowed
 
 no_cache = 1
 
@@ -10,15 +11,13 @@ def get_context(context):
 	if frappe.flags.in_migrate:
 		return
 
-	allow_traceback = frappe.get_system_settings("allow_error_traceback") if frappe.db else False
-	if frappe.local.flags.disable_traceback and not frappe.local.dev_server:
-		allow_traceback = False
-
 	if not context.title:
 		context.title = _("Server Error")
 	if not context.message:
 		context.message = _("There was an error building this page")
 
+	allow_traceback = is_traceback_allowed() and not frappe.local.dev_server
+
 	return {
 		"error": frappe.get_traceback().replace("<", "&lt;").replace(">", "&gt;") if allow_traceback else ""
 	}

frappe/www/printview.py

@@ -72,10 +72,6 @@ def get_context(context) -> PrintContext:
 
 	print_format = get_print_format_doc(None, meta=meta)
 
-	make_access_log(
-		doctype=frappe.form_dict.doctype, document=frappe.form_dict.name, file_type="PDF", method="Print"
-	)
-
 	body = get_rendered_template(
 		doc,
 		print_format=print_format,
@@ -85,11 +81,14 @@ def get_context(context) -> PrintContext:
 		letterhead=letterhead,
 		settings=settings,
 	)
-	print_style = get_print_style(frappe.form_dict.style, print_format)
+
+	make_access_log(
+		doctype=frappe.form_dict.doctype, document=frappe.form_dict.name, file_type="PDF", method="Print"
+	)
 
 	return {
 		"body": body,
-		"print_style": print_style,
+		"print_style": get_print_style(frappe.form_dict.style, print_format),
 		"comment": frappe.session.user,
 		"title": frappe.utils.strip_html(cstr(doc.get_title() or doc.name)),
 		"lang": frappe.local.lang,
@@ -127,6 +126,9 @@ def get_rendered_template(
 	trigger_print: bool = False,
 	settings: dict | None = None,
 ) -> str:
+	if not frappe.flags.ignore_print_permissions:
+		validate_print_permission(doc)
+
 	print_settings = frappe.get_single("Print Settings").as_dict()
 	print_settings.update(settings or {})
 
@@ -139,9 +141,6 @@ def get_rendered_template(
 	doc.flags.in_print = True
 	doc.flags.print_settings = print_settings
 
-	if not frappe.flags.ignore_print_permissions:
-		validate_print_permission(doc)
-
 	if doc.meta.is_submittable:
 		if doc.docstatus.is_draft() and not cint(print_settings.allow_print_for_draft):
 			frappe.throw(_("Not allowed to print draft documents"), frappe.PermissionError)
@@ -382,11 +381,10 @@ def validate_print_permission(doc: "Document") -> None:
 	if frappe.has_website_permission(doc):
 		return
 
-	if (key := frappe.form_dict.key) and isinstance(key, str):
-		validate_key(key, doc)
+	if (key := frappe.form_dict.key) and isinstance(key, str) and validate_key(key, doc) is not False:
 		return
 
-	frappe.throw(_("{0} {1} not found").format(_(doc.doctype), doc.name), frappe.DoesNotExistError)
+	doc._handle_permission_failure("print")
 
 
 def validate_key(key: str, doc: "Document") -> None:
@@ -405,7 +403,7 @@ def validate_key(key: str, doc: "Document") -> None:
 	if frappe.get_system_settings("allow_older_web_view_links") and key == doc.get_signature():
 		return
 
-	raise frappe.exceptions.InvalidKeyError
+	return False
 
 
 def get_letter_head(doc: "Document", no_letterhead: bool, letterhead: str | None = None) -> dict: