From 568426668fb60bc6c9ac044dd50930825274db11 Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Fri, 11 Dec 2020 13:55:31 +0530 Subject: [PATCH 1/3] feat: add alert flag for permission validation In case default permissions are not set, the alert flag will indicate if an alert has to be shown in the UI or not --- frappe/core/doctype/doctype/doctype.py | 9 +++++---- .../page/permission_manager/permission_manager.py | 14 +++++++++++++- frappe/permissions.py | 4 ++-- 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index cce5968f9c..fced5d1fa1 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -1000,10 +1000,10 @@ def validate_fields(meta): check_sort_field(meta) check_image_field(meta) -def validate_permissions_for_doctype(doctype, for_remove=False): +def validate_permissions_for_doctype(doctype, for_remove=False, alert=True): """Validates if permissions are set correctly.""" doctype = frappe.get_doc("DocType", doctype) - validate_permissions(doctype, for_remove) + validate_permissions(doctype, for_remove, alert=alert) # save permissions for perm in doctype.get("permissions"): @@ -1026,9 +1026,10 @@ def clear_permissions_cache(doctype): """, doctype): frappe.clear_cache(user=user) -def validate_permissions(doctype, for_remove=False): +def validate_permissions(doctype, for_remove=False, alert=True): permissions = doctype.get("permissions") - if not permissions: + # Some DocTypes may not have permissions by default, don't show alert for them + if not permissions and alert: frappe.msgprint(_('No Permissions Specified'), alert=True, indicator='orange') issingle = issubmittable = isimportable = False if doctype: diff --git a/frappe/core/page/permission_manager/permission_manager.py b/frappe/core/page/permission_manager/permission_manager.py index 637b526d5c..5b4ccb6ce0 100644 --- a/frappe/core/page/permission_manager/permission_manager.py +++ b/frappe/core/page/permission_manager/permission_manager.py @@ -77,8 +77,20 @@ def add(parent, role, permlevel): @frappe.whitelist() def update(doctype, role, permlevel, ptype, value=None): + """Update role permission params + + Args: + doctype (str): Name of the DocType to update params for + role (str): Role to be updated for, eg "Website Manager". + permlevel (int): perm level the provided rule applies to + ptype (str): permission type, example "read", "delete", etc. + value (None, optional): value for ptype, None indicates False + + Returns: + str: Refresh flag is permission is updated successfully + """ frappe.only_for("System Manager") - out = update_permission_property(doctype, role, permlevel, ptype, value) + out = update_permission_property(doctype, role, permlevel, ptype, value, alert=False) return 'refresh' if out else None @frappe.whitelist() diff --git a/frappe/permissions.py b/frappe/permissions.py index 0d766aec8d..e9724b7418 100644 --- a/frappe/permissions.py +++ b/frappe/permissions.py @@ -446,7 +446,7 @@ def can_export(doctype, raise_exception=False): raise frappe.PermissionError(_("You are not allowed to export {} doctype").format(doctype)) return has_access -def update_permission_property(doctype, role, permlevel, ptype, value=None, validate=True): +def update_permission_property(doctype, role, permlevel, ptype, value=None, validate=True, alert=True): '''Update a property in Custom Perm''' from frappe.core.doctype.doctype.doctype import validate_permissions_for_doctype out = setup_custom_perms(doctype) @@ -458,7 +458,7 @@ def update_permission_property(doctype, role, permlevel, ptype, value=None, vali update `tabCustom DocPerm` set `{0}`=%s where name=%s""".format(ptype), (value, name)) if validate: - validate_permissions_for_doctype(doctype) + validate_permissions_for_doctype(doctype, alert=alert) return out From 424c0c50f8055bc18feed762c8c3e7279e4f74ac Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Wed, 23 Dec 2020 13:50:18 +0530 Subject: [PATCH 2/3] fix: set alert flag to false by default --- frappe/core/doctype/doctype/doctype.py | 4 ++-- frappe/core/page/permission_manager/permission_manager.py | 4 ++-- frappe/permissions.py | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index fced5d1fa1..71fca9b597 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -1000,7 +1000,7 @@ def validate_fields(meta): check_sort_field(meta) check_image_field(meta) -def validate_permissions_for_doctype(doctype, for_remove=False, alert=True): +def validate_permissions_for_doctype(doctype, for_remove=False, alert=False): """Validates if permissions are set correctly.""" doctype = frappe.get_doc("DocType", doctype) validate_permissions(doctype, for_remove, alert=alert) @@ -1026,7 +1026,7 @@ def clear_permissions_cache(doctype): """, doctype): frappe.clear_cache(user=user) -def validate_permissions(doctype, for_remove=False, alert=True): +def validate_permissions(doctype, for_remove=False, alert=False): permissions = doctype.get("permissions") # Some DocTypes may not have permissions by default, don't show alert for them if not permissions and alert: diff --git a/frappe/core/page/permission_manager/permission_manager.py b/frappe/core/page/permission_manager/permission_manager.py index 5b4ccb6ce0..be8921e2ff 100644 --- a/frappe/core/page/permission_manager/permission_manager.py +++ b/frappe/core/page/permission_manager/permission_manager.py @@ -90,7 +90,7 @@ def update(doctype, role, permlevel, ptype, value=None): str: Refresh flag is permission is updated successfully """ frappe.only_for("System Manager") - out = update_permission_property(doctype, role, permlevel, ptype, value, alert=False) + out = update_permission_property(doctype, role, permlevel, ptype, value) return 'refresh' if out else None @frappe.whitelist() @@ -104,7 +104,7 @@ def remove(doctype, role, permlevel): if not frappe.get_all('Custom DocPerm', dict(parent=doctype)): frappe.throw(_('There must be atleast one permission rule.'), title=_('Cannot Remove')) - validate_permissions_for_doctype(doctype, for_remove=True) + validate_permissions_for_doctype(doctype, for_remove=True, alert=True) @frappe.whitelist() def reset(doctype): diff --git a/frappe/permissions.py b/frappe/permissions.py index e9724b7418..15bb2c8887 100644 --- a/frappe/permissions.py +++ b/frappe/permissions.py @@ -446,7 +446,7 @@ def can_export(doctype, raise_exception=False): raise frappe.PermissionError(_("You are not allowed to export {} doctype").format(doctype)) return has_access -def update_permission_property(doctype, role, permlevel, ptype, value=None, validate=True, alert=True): +def update_permission_property(doctype, role, permlevel, ptype, value=None, validate=True): '''Update a property in Custom Perm''' from frappe.core.doctype.doctype.doctype import validate_permissions_for_doctype out = setup_custom_perms(doctype) From 3b6ae2de6c0f1fab9ef858b0736ed43b47e87adb Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Wed, 23 Dec 2020 13:58:11 +0530 Subject: [PATCH 3/3] fix: reference error --- frappe/permissions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/permissions.py b/frappe/permissions.py index 15bb2c8887..0d766aec8d 100644 --- a/frappe/permissions.py +++ b/frappe/permissions.py @@ -458,7 +458,7 @@ def update_permission_property(doctype, role, permlevel, ptype, value=None, vali update `tabCustom DocPerm` set `{0}`=%s where name=%s""".format(ptype), (value, name)) if validate: - validate_permissions_for_doctype(doctype, alert=alert) + validate_permissions_for_doctype(doctype) return out