From 447f02e8d3baecd60fecd61431f21959d498f533 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 11 Jan 2024 13:54:18 +0530 Subject: [PATCH] fix!: Remove misleading "raise_exception" (#24266) frappe.permission.has_permission won't accept raise_exception anymore, it was extremely misleading argument and actual purpose of the argument was to print perm check logs. --- frappe/__init__.py | 2 +- frappe/boot.py | 2 +- frappe/model/rename_doc.py | 3 ++- frappe/permissions.py | 27 ++++++++++--------- frappe/share.py | 2 +- .../workflow_action/workflow_action.py | 2 +- 6 files changed, 21 insertions(+), 17 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index 7b2d04cef3..ccf209e24a 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -1015,7 +1015,7 @@ def has_permission( ptype, doc=doc, user=user, - raise_exception=throw, + print_logs=throw, parent_doctype=parent_doctype, debug=debug, ) diff --git a/frappe/boot.py b/frappe/boot.py index d0e6204e78..8caa264443 100644 --- a/frappe/boot.py +++ b/frappe/boot.py @@ -237,7 +237,7 @@ def get_user_pages_or_reports(parent, cache=False): has_role[p.name] = {"modified": p.modified, "title": p.title} elif parent == "Report": - if not has_permission("Report", raise_exception=False): + if not has_permission("Report", print_logs=False): return {} reports = frappe.get_list( diff --git a/frappe/model/rename_doc.py b/frappe/model/rename_doc.py index ed7e529e4a..7d5539becd 100644 --- a/frappe/model/rename_doc.py +++ b/frappe/model/rename_doc.py @@ -4,6 +4,7 @@ from types import NoneType from typing import TYPE_CHECKING import frappe +import frappe.permissions from frappe import _, bold from frappe.model.document import Document from frappe.model.dynamic_links import get_dynamic_link_map @@ -379,7 +380,7 @@ def validate_rename( frappe.throw(_("Another {0} with name {1} exists, select another name").format(doctype, new)) if not ( - ignore_permissions or frappe.permissions.has_permission(doctype, "write", raise_exception=False) + ignore_permissions or frappe.permissions.has_permission(doctype, "write", print_logs=False) ): frappe.throw(_("You need write permission to rename")) diff --git a/frappe/permissions.py b/frappe/permissions.py index 5a681a99ab..a6d2ca3da1 100644 --- a/frappe/permissions.py +++ b/frappe/permissions.py @@ -40,20 +40,20 @@ AUTOMATIC_ROLES = (GUEST_ROLE, ALL_USER_ROLE, SYSTEM_USER_ROLE, ADMIN_ROLE) def print_has_permission_check_logs(func): @functools.wraps(func) def inner(*args, **kwargs): - raise_exception = kwargs.get("raise_exception", True) + print_logs = kwargs.get("print_logs", True) self_perm_check = True if not kwargs.get("user") else kwargs.get("user") == frappe.session.user - if raise_exception: + if print_logs: frappe.flags["has_permission_check_logs"] = [] result = func(*args, **kwargs) # print only if access denied # and if user is checking his own permission - if not result and self_perm_check and raise_exception: + if not result and self_perm_check and print_logs: msgprint(("
").join(frappe.flags.get("has_permission_check_logs", []))) - if raise_exception: + if print_logs: frappe.flags.pop("has_permission_check_logs", None) return result @@ -79,9 +79,9 @@ def has_permission( ptype="read", doc=None, user=None, - raise_exception=True, *, parent_doctype=None, + print_logs=True, debug=False, ) -> bool: """Return True if user has permission `ptype` for given `doctype`. @@ -91,11 +91,8 @@ def has_permission( :param ptype: Permission Type to check :param doc: Check User Permissions for specified document. :param user: User to check permission for. Defaults to current user. - :param raise_exception: - DOES NOT raise an exception. - If not False, will display a message using frappe.msgprint + :param print_logs: If True, will display a message using frappe.msgprint which explains why the permission check failed. - :param parent_doctype: Required when checking permission for a child DocType (unless doc is specified) """ @@ -120,7 +117,13 @@ def has_permission( if frappe.is_table(doctype): return has_child_permission( - doctype, ptype, doc, user, raise_exception, parent_doctype, debug=debug + doctype, + ptype, + doc, + user, + parent_doctype, + debug=debug, + print_logs=print_logs, ) meta = frappe.get_meta(doctype) @@ -757,10 +760,10 @@ def has_child_permission( ptype="read", child_doc=None, user=None, - raise_exception=True, parent_doctype=None, *, debug=False, + print_logs=True, ) -> bool: debug and _debug_log("This doctype is a child table, permissions will be checked on parent.") if isinstance(child_doc, str): @@ -832,7 +835,7 @@ def has_child_permission( ptype=ptype, doc=child_doc and getattr(child_doc, "parent_doc", child_doc.parent), user=user, - raise_exception=raise_exception, + print_logs=print_logs, debug=debug, ) diff --git a/frappe/share.py b/frappe/share.py index 55d235789b..07e7e6c592 100644 --- a/frappe/share.py +++ b/frappe/share.py @@ -136,7 +136,7 @@ def get_users(doctype: str, name: str) -> list: def _get_users(doc: "Document") -> list: from frappe.permissions import has_permission - if not has_permission(doc.doctype, "read", doc, raise_exception=False): + if not has_permission(doc.doctype, "read", doc, print_logs=False): return [] return frappe.get_all( diff --git a/frappe/workflow/doctype/workflow_action/workflow_action.py b/frappe/workflow/doctype/workflow_action/workflow_action.py index 9cf61126b4..27b868e020 100644 --- a/frappe/workflow/doctype/workflow_action/workflow_action.py +++ b/frappe/workflow/doctype/workflow_action/workflow_action.py @@ -470,7 +470,7 @@ def filter_allowed_users(users, doc, transition): user for user in users if has_approval_access(user, doc, transition) - and has_permission(doctype=doc, user=user, raise_exception=False) + and has_permission(doctype=doc, user=user, print_logs=False) ]