From a22cbe8ae5fb3e1842bea7c31c9dc08276cec3ac Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Sat, 22 Apr 2023 14:58:28 +0530 Subject: [PATCH] fix: Setup permission_map & use get_permitted_fields --- frappe/model/__init__.py | 6 ++++++ frappe/model/db_query.py | 40 +++++++++++++++++++++++++--------------- 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/frappe/model/__init__.py b/frappe/model/__init__.py index d7baa0815d..ff6ad36c42 100644 --- a/frappe/model/__init__.py +++ b/frappe/model/__init__.py @@ -207,9 +207,15 @@ def get_permitted_fields( if set(valid_columns).issubset(default_fields): return valid_columns + if permission_type is None: + permission_type = "select" if frappe.only_has_select_perm(doctype, user=user) else "read" + if permitted_fields := meta.get_permitted_fieldnames( parenttype=parenttype, user=user, permission_type=permission_type ): + if permission_type == "select": + return permitted_fields + meta_fields = meta.default_fields.copy() optional_meta_fields = [x for x in optional_fields if x in valid_columns] diff --git a/frappe/model/db_query.py b/frappe/model/db_query.py index d785825a77..50c6a053c3 100644 --- a/frappe/model/db_query.py +++ b/frappe/model/db_query.py @@ -68,6 +68,7 @@ class DatabaseQuery: self.ignore_ifnull = False self.flags = frappe._dict() self.reference_doctype = None + self.permission_map = {} @property def doctype_meta(self): @@ -115,15 +116,8 @@ class DatabaseQuery: parent_doctype=None, ) -> list: - if ( - not ignore_permissions - and not frappe.has_permission(self.doctype, "select", user=user, parent_doctype=parent_doctype) - and not frappe.has_permission(self.doctype, "read", user=user, parent_doctype=parent_doctype) - ): - frappe.flags.error_message = _("Insufficient Permission for {0}").format( - frappe.bold(self.doctype) - ) - raise frappe.PermissionError(self.doctype) + if not ignore_permissions: + self.check_read_permission(self.doctype, parent_doctype=parent_doctype) # filters and fields swappable # its hard to remember what comes first @@ -495,14 +489,26 @@ class DatabaseQuery: frappe._dict(doctype=doctype, fieldname=fieldname, table_name=f"`tab{doctype}`") ) - def check_read_permission(self, doctype): - if not self.flags.ignore_permissions and not frappe.has_permission( + def check_read_permission(self, doctype, parent_doctype=None): + if self.flags.ignore_permissions: + return + + if doctype not in self.permission_map: + self._set_permission_map(doctype, parent_doctype) + + return self.permission_map[doctype] + + def _set_permission_map(self, doctype, parent_doctype=None): + ptype = "select" if frappe.only_has_select_perm(doctype) else "read" + val = frappe.has_permission( doctype, - ptype="select" if frappe.only_has_select_perm(doctype) else "read", - parent_doctype=self.doctype, - ): + ptype=ptype, + parent_doctype=parent_doctype, + ) + if not val: frappe.flags.error_message = _("Insufficient Permission for {0}").format(frappe.bold(doctype)) raise frappe.PermissionError(doctype) + self.permission_map[doctype] = ptype def set_field_tables(self): """If there are more than one table, the fieldname must not be ambiguous. @@ -608,7 +614,11 @@ class DatabaseQuery: return asterisk_fields = [] - permitted_fields = get_permitted_fields(doctype=self.doctype, parenttype=self.parent_doctype) + permitted_fields = get_permitted_fields( + doctype=self.doctype, + parenttype=self.parent_doctype, + permission_type=self.permission_map.get(self.doctype), + ) for i, field in enumerate(self.fields): if "distinct" in field.lower():