From 61ec02671299e46c34c38c57390220c0a43c648f Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Sat, 30 Jul 2022 22:24:41 +0530 Subject: [PATCH 1/4] refactor: improve `frappe.only_for` --- frappe/__init__.py | 21 +++++++++++-------- .../permitted_documents_for_user.py | 13 +++--------- frappe/permissions.py | 5 +++++ 3 files changed, 20 insertions(+), 19 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index e1fa902eba..d017a2cc6b 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -818,21 +818,24 @@ def write_only(): return innfn -def only_for(roles: list[str] | str, message=False): - """Raise `frappe.PermissionError` if the user does not have any of the given **Roles**. +def only_for(roles: list[str] | tuple[str] | str, message=False): + """ + Raises `frappe.PermissionError` if the user does not have any of the permitted roles. - :param roles: List of roles to check.""" - if local.flags.in_test: + :param roles: Permitted role(s) + """ + + if local.flags.in_test or local.session.user == "Administrator": return - if not isinstance(roles, (tuple, list)): + if isinstance(roles, str): roles = (roles,) - roles = set(roles) - myroles = set(get_roles()) - if not roles.intersection(myroles): + + if not set(roles).intersection(get_roles()): if message: msgprint( - _("This action is only allowed for {}").format(bold(", ".join(roles))), _("Not Permitted") + _("This action is only allowed for {}").format(bold(", ".join(roles))), + _("Not Permitted"), ) raise PermissionError diff --git a/frappe/core/report/permitted_documents_for_user/permitted_documents_for_user.py b/frappe/core/report/permitted_documents_for_user/permitted_documents_for_user.py index 362cc6b105..a7eff77ed0 100644 --- a/frappe/core/report/permitted_documents_for_user/permitted_documents_for_user.py +++ b/frappe/core/report/permitted_documents_for_user/permitted_documents_for_user.py @@ -4,19 +4,18 @@ import frappe import frappe.utils.user from frappe.model import data_fieldtypes -from frappe.permissions import check_admin_or_system_manager, rights +from frappe.permissions import rights def execute(filters=None): + frappe.only_for("System Manager") + user, doctype, show_permissions = ( filters.get("user"), filters.get("doctype"), filters.get("show_permissions"), ) - if not validate(user, doctype): - return [], [] - columns, fields = get_columns_and_fields(doctype) data = frappe.get_list(doctype, fields=fields, as_list=True, user=user) @@ -30,12 +29,6 @@ def execute(filters=None): return columns, data -def validate(user, doctype): - # check if current user is System Manager - check_admin_or_system_manager() - return user and doctype - - def get_columns_and_fields(doctype): columns = [f"Name:Link/{doctype}:200"] fields = ["`name`"] diff --git a/frappe/permissions.py b/frappe/permissions.py index acbdf76989..98786ce789 100644 --- a/frappe/permissions.py +++ b/frappe/permissions.py @@ -28,6 +28,11 @@ rights = ( def check_admin_or_system_manager(user=None): + """ + DEPRECATED: This function will be removed in version 15. + Use `frappe.only_for` instead. + """ + if not user: user = frappe.session.user From 53118367b2c38b4c8182191d30e0ec0b98c67ebf Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Tue, 2 Aug 2022 19:08:02 +0530 Subject: [PATCH 2/4] fix: use warn util --- frappe/permissions.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/frappe/permissions.py b/frappe/permissions.py index 98786ce789..50d7366626 100644 --- a/frappe/permissions.py +++ b/frappe/permissions.py @@ -28,10 +28,13 @@ rights = ( def check_admin_or_system_manager(user=None): - """ - DEPRECATED: This function will be removed in version 15. - Use `frappe.only_for` instead. - """ + from frappe.utils.commands import warn + + warn( + "The function check_admin_or_system_manager will be deprecated in version 15." + 'Please use frappe.only_for("System Manager") instead.', + category=PendingDeprecationWarning, + ) if not user: user = frappe.session.user From 74c26ac34d32ca4e35c2f8b62523512ddab00bec Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Tue, 2 Aug 2022 19:16:53 +0530 Subject: [PATCH 3/4] fix: use `throw` --- frappe/__init__.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index d017a2cc6b..c2a24e6ad0 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -832,12 +832,14 @@ def only_for(roles: list[str] | tuple[str] | str, message=False): roles = (roles,) if not set(roles).intersection(get_roles()): - if message: - msgprint( - _("This action is only allowed for {}").format(bold(", ".join(roles))), - _("Not Permitted"), - ) - raise PermissionError + if not message: + raise PermissionError + + throw( + _("This action is only allowed for {}").format(bold(", ".join(roles))), + PermissionError, + _("Not Permitted"), + ) def get_domain_data(module): From eb1c9fff689cf5235db6b3fda5eaed367512788e Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Tue, 2 Aug 2022 19:27:16 +0530 Subject: [PATCH 4/4] fix: translate each role --- frappe/__init__.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index c2a24e6ad0..16e5f5f53c 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -836,7 +836,9 @@ def only_for(roles: list[str] | tuple[str] | str, message=False): raise PermissionError throw( - _("This action is only allowed for {}").format(bold(", ".join(roles))), + _("This action is only allowed for {}").format( + ", ".join(bold(_(role)) for role in roles), + ), PermissionError, _("Not Permitted"), )