From 52b88e951846b231a8383ea4e30d12d6cc07874d Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Thu, 6 Nov 2025 15:17:45 +0530 Subject: [PATCH] feat: add test cases --- .../permission_type/permission_type.py | 12 ++- .../permission_type/test_permission_type.py | 87 +++++++++++++++++-- .../permitted_documents_for_user.py | 3 +- frappe/permissions.py | 21 +++-- frappe/utils/user.py | 5 +- 5 files changed, 107 insertions(+), 21 deletions(-) diff --git a/frappe/core/doctype/permission_type/permission_type.py b/frappe/core/doctype/permission_type/permission_type.py index 76f8258164..a7e5907447 100644 --- a/frappe/core/doctype/permission_type/permission_type.py +++ b/frappe/core/doctype/permission_type/permission_type.py @@ -28,10 +28,13 @@ class PermissionType(Document): module: DF.Link # end: auto-generated types - def validate(self): - from frappe.permissions import rights + def before_insert(self): + self.name = frappe.scrub(self.name) - if self.name in rights: + def validate(self): + from frappe.permissions import std_rights + + if self.name in std_rights: frappe.throw( _("Permission Type '{0}' is reserved. Please choose another name.").format(self.name) ) @@ -107,5 +110,6 @@ def get_doctype_ptype_map(): doctype_ptype_map = defaultdict(list) for pt in ptypes: for dt in pt.doc_types: - doctype_ptype_map[dt.doc_type].append(pt.name) + if pt.name not in doctype_ptype_map[dt.doc_type]: + doctype_ptype_map[dt.doc_type].append(pt.name) return dict(doctype_ptype_map) diff --git a/frappe/core/doctype/permission_type/test_permission_type.py b/frappe/core/doctype/permission_type/test_permission_type.py index 700e70605a..04cadffac6 100644 --- a/frappe/core/doctype/permission_type/test_permission_type.py +++ b/frappe/core/doctype/permission_type/test_permission_type.py @@ -1,7 +1,8 @@ # Copyright (c) 2025, Frappe Technologies and Contributors # See license.txt -# import frappe +import frappe +from frappe.permissions import update_permission_property from frappe.tests import IntegrationTestCase # On IntegrationTestCase, the doctype test records and all @@ -17,14 +18,86 @@ class IntegrationTestPermissionType(IntegrationTestCase): Use this class for testing interactions between multiple components. """ - def test_permission_type_creation_deletion(self): ... + def test_approve_ptype_on_blog_post(self): + """Test that custom permission types are applied correctly.""" + user_role = "Website Manager" + doc_type = "Web Page" + ptype_name = "approve" - def test_permission_type_creation_reserved_name(self): ... + user = self._create_test_user("test_approve_permission@example.com", user_role) - def test_role_permission_with_custom_permission_type(self): ... + ptype_doc = self._create_permission_type(ptype_name, doc_type) - def test_share_permission_with_custom_permission_type(self): ... + try: + self._verify_custom_fields_created(ptype_doc, doc_type) - def test_role_permission_mapping_with_custom_permission_type(self): ... + self._verify_user_lacks_permission(doc_type, ptype_name, user.name) - def test_export_import_of_permission_type(self): ... + update_permission_property(doctype=doc_type, role=user_role, permlevel=0, ptype=ptype_name, value=1) + + self._verify_user_has_permission(doc_type, ptype_name, user.name) + + update_permission_property(doctype=doc_type, role=user_role, permlevel=0, ptype=ptype_name, value=0) + + finally: + frappe.delete_doc("User", user.name, force=True) + frappe.delete_doc("Permission Type", ptype_doc.name, force=True) + + def test_permission_type_creation_reserved_name(self): + """Test that permission types with reserved names are rejected.""" + doc = frappe.get_doc( + { + "doctype": "Permission Type", + "name": "read", + "module": "Core", + "doc_types": [{"doc_type": "ToDo"}], + } + ) + + with self.assertRaises(frappe.exceptions.ValidationError): + doc.insert() + + def _create_test_user(self, email, role): + """Create a test user with the specified role.""" + user = frappe.new_doc("User") + user.email = email + user.first_name = email.split("@", 1)[0] + user.insert(ignore_if_duplicate=True) + user.reload() + user.add_roles(role) + return user + + def _create_permission_type(self, name, doc_type): + """Create a permission type for the specified doctype.""" + ptype_doc = frappe.get_doc( + { + "doctype": "Permission Type", + "name": name, + "module": "Core", + "doc_types": [{"doc_type": doc_type}], + } + ) + ptype_doc.insert(ignore_if_duplicate=True) + ptype_doc.reload() + return ptype_doc + + def _verify_custom_fields_created(self, ptype_doc, doc_type): + """Verify that custom fields are created for the permission type.""" + for target in ["Custom DocPerm", "DocPerm", "DocShare"]: + custom_field = frappe.get_doc("Custom Field", {"dt": target, "fieldname": ptype_doc.name}) + self.assertEqual(custom_field.dt, target) + self.assertEqual(custom_field.fieldname, ptype_doc.name) + self.assertEqual(custom_field.fieldtype, "Check") + self.assertIn(doc_type, custom_field.depends_on) + + def _verify_user_lacks_permission(self, doc_type, ptype_name, user_name): + """Verify that user does not have the specified permission type.""" + self.assertFalse( + frappe.has_permission(doc_type, ptype=ptype_name, user=user_name) + ) + + def _verify_user_has_permission(self, doc_type, ptype_name, user_name): + """Verify that user has the specified permission type.""" + self.assertTrue( + frappe.has_permission(doc_type, ptype=ptype_name, user=user_name) + ) 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 bf1694b164..ddde4ee833 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,7 +4,7 @@ import frappe import frappe.utils.user from frappe.model import data_fieldtypes -from frappe.permissions import rights +from frappe.permissions import get_rights def execute(filters=None): @@ -20,6 +20,7 @@ def execute(filters=None): data = frappe.get_list(doctype, fields=fields, as_list=True, user=user) if show_permissions: + rights = get_rights(doctype) columns = columns + [frappe.unscrub(right) + ":Check:80" for right in rights] data = list(data) for i, doc in enumerate(data): diff --git a/frappe/permissions.py b/frappe/permissions.py index 82fdf7a4c1..1eefdb8700 100644 --- a/frappe/permissions.py +++ b/frappe/permissions.py @@ -10,7 +10,7 @@ from frappe.core.doctype.permission_type.permission_type import get_doctype_ptyp from frappe.query_builder import DocType from frappe.utils import cint, cstr -rights = ( +std_rights = ( "select", "read", "write", @@ -27,6 +27,8 @@ rights = ( "share", ) +rights = std_rights + GUEST_ROLE = "Guest" ALL_USER_ROLE = "All" # This includes website users too. @@ -167,10 +169,10 @@ def has_permission( ) def false_if_not_shared(): - std_rights = ["read", "write", "share", "submit", "email", "print"] + share_rights = ["read", "write", "share", "submit", "email", "print"] custom_rights = get_doctype_ptype_map().get(doctype, []) - if ptype not in std_rights + custom_rights: + if ptype not in share_rights + custom_rights: debug and _debug_log(f"Permission type {ptype} can not be shared") return False @@ -282,7 +284,7 @@ def get_role_permissions(doctype_meta, user=None, is_owner=None, debug=False): if user == "Administrator": debug and _debug_log("all permissions granted because user is Administrator") - return allow_everything() + return allow_everything(doctype_meta.name) if not frappe.local.role_permissions.get(cache_key) or debug: perms = frappe._dict(if_owner={}) @@ -300,7 +302,7 @@ def get_role_permissions(doctype_meta, user=None, is_owner=None, debug=False): has_if_owner_enabled = any(p.get("if_owner", 0) for p in applicable_permissions) perms["has_if_owner_enabled"] = has_if_owner_enabled - for ptype in rights: + for ptype in get_rights(doctype_meta.name): pvalue = any(p.get(ptype, 0) for p in applicable_permissions) # check if any perm object allows perm type perms[ptype] = cint(pvalue) @@ -740,10 +742,15 @@ def get_doc_name(doc): return None return doc if isinstance(doc, str) else str(doc.name) +def get_rights(doctype = None): + if not doctype: + return std_rights + custom_rights = get_doctype_ptype_map().get(doctype, []) + return list(std_rights) + custom_rights -def allow_everything(): +def allow_everything(doctype = None): """Return a dict with access to everything, eg. {"read": 1, "write": 1, ...}.""" - return {ptype: 1 for ptype in rights} + return {ptype: 1 for ptype in get_rights(doctype)} def get_allowed_docs_for_doctype(user_permissions, doctype): diff --git a/frappe/utils/user.py b/frappe/utils/user.py index 7692c12e46..700ed6efda 100644 --- a/frappe/utils/user.py +++ b/frappe/utils/user.py @@ -9,7 +9,7 @@ import frappe.share from frappe import _dict from frappe.boot import get_allowed_reports from frappe.core.doctype.domain_settings.domain_settings import get_active_modules -from frappe.permissions import AUTOMATIC_ROLES, get_roles, get_valid_perms +from frappe.permissions import AUTOMATIC_ROLES, get_rights, get_roles, get_valid_perms from frappe.query_builder import DocType, Order from frappe.query_builder.functions import Concat_ws @@ -101,7 +101,8 @@ class UserPermissions: if dt not in self.perm_map: self.perm_map[dt] = {} - for k in frappe.permissions.rights: + rights = get_rights(dt) + for k in rights: if not self.perm_map[dt].get(k): self.perm_map[dt][k] = r.get(k)