Merge pull request #16257 from surajshetty3416/permission-fixes

Fix user permission checks

erpnext/accounts/party.py

@@ -5,7 +5,7 @@ from __future__ import unicode_literals
 
 import frappe, erpnext
 from frappe import _, msgprint, scrub
-from frappe.defaults import get_user_permissions
+from frappe.core.doctype.user_permission.user_permission import get_permitted_documents
 from frappe.model.utils import get_fetch_values
 from frappe.utils import (add_days, getdate, formatdate, date_diff,
 	add_years, get_timestamp, nowdate, flt, add_months, get_last_day)
@@ -151,10 +151,7 @@ def get_default_price_list(party):
 
 def set_price_list(out, party, party_type, given_price_list):
 	# price list
-	price_list = filter(None, get_user_permissions()
+	price_list = get_permitted_documents('Price List')
-		.get("Price List", {})
-		.get("docs", []))
-	price_list = list(price_list)
 
 	if price_list:
 		price_list = price_list[0]

erpnext/hr/doctype/leave_application/leave_application.js

@@ -83,7 +83,7 @@ frappe.ui.form.on("Leave Application", {
 		if (!frm.doc.employee && frappe.defaults.get_user_permissions()) {
 			const perm = frappe.defaults.get_user_permissions();
 			if (perm && perm['Employee']) {
-				frm.set_value('employee', perm['Employee']["docs"][0])
+				frm.set_value('employee', perm['Employee'].map(perm_doc => perm_doc.doc)[0]);
 			}
 		}
 	},

erpnext/patches/v11_0/skip_user_permission_check_for_department.py

@@ -1,28 +1,60 @@
 import frappe
+from frappe.desk.form.linked_with import get_linked_doctypes
 
 # Skips user permission check for doctypes where department link field was recently added
 # https://github.com/frappe/erpnext/pull/14121
 
 def execute():
-    user_permissions = frappe.get_all("User Permission",
+	doctypes_to_skip = []
-        filters=[['allow', '=', 'Department']],
+	for doctype in ['Appraisal', 'Leave Allocation', 'Expense Claim', 'Instructor', 'Salary Slip',
-        fields=['name', 'skip_for_doctype'])
+					'Attendance', 'Training Feedback', 'Training Result Employee',
+					'Leave Application', 'Employee Advance', 'Activity Cost', 'Training Event Employee',
+					'Timesheet', 'Sales Person', 'Payroll Employee Detail']:
+		if frappe.db.exists('Custom Field', { 'dt': doctype, 'fieldname': 'department'}): continue
+		doctypes_to_skip.append(doctype)
 
-    doctypes_to_skip = []
+	frappe.reload_doctype('User Permission')
 
-    for doctype in ['Appraisal', 'Leave Allocation', 'Expense Claim', 'Instructor', 'Salary Slip',
+	user_permissions = frappe.get_all("User Permission",
-                    'Attendance', 'Training Feedback', 'Training Result Employee',
+		filters=[['allow', '=', 'Department'], ['applicable_for', 'in', [None] + doctypes_to_skip]],
-                    'Leave Application', 'Employee Advance', 'Activity Cost', 'Training Event Employee',
+		fields=['name', 'applicable_for'])
-                    'Timesheet', 'Sales Person', 'Payroll Employee Detail']:
-        if frappe.db.exists('Custom Field', { 'dt': doctype, 'fieldname': 'department'}): continue
-        doctypes_to_skip.append(doctype)
 
-    for perm in user_permissions:
+	user_permissions_to_delete = []
-        skip_for_doctype = perm.get('skip_for_doctype')
+	new_user_permissions_list = []
 
-        skip_for_doctype = skip_for_doctype.split('\n') + doctypes_to_skip
+	for user_permission in user_permissions:
-        skip_for_doctype = set(skip_for_doctype) # to remove duplicates
+		if user_permission.applicable_for:
-        skip_for_doctype = '\n'.join(skip_for_doctype) # convert back to string
+			# simply delete user permission record since it needs to be skipped.
+			user_permissions_to_delete.append(user_permission.name)
+		else:
+			# if applicable_for is `None` it means that user permission is applicable for every doctype
+			# to avoid this we need to create other user permission records and only skip the listed doctypes in this patch
+			linked_doctypes = get_linked_doctypes(user_permission.allow, True).keys()
+			applicable_for_doctypes = list(set(linked_doctypes) - set(doctypes_to_skip))
 
-        frappe.set_value('User Permission', perm.name, 'skip_for_doctype', skip_for_doctype)
+			user_permissions_to_delete.append(user_permission.name)
 
+			for doctype in applicable_for_doctypes:
+				if doctype:
+					# Maintain sequence (name, user, allow, for_value, applicable_for, apply_to_all_doctypes)
+					new_user_permissions_list.append((
+						frappe.generate_hash("", 10),
+						user_permission.user,
+						user_permission.allow,
+						user_permission.for_value,
+						doctype,
+						0
+					))
+
+	if new_user_permissions_list:
+		frappe.db.sql('''
+			INSERT INTO `tabUser Permission`
+			(`name`, `user`, `allow`, `for_value`, `applicable_for`, `apply_to_all_doctypes`)
+			VALUES {}'''.format(', '.join(['%s'] * len(new_user_permissions_list))), # nosec
+			tuple(new_user_permissions_list)
+		)
+
+	if user_permissions_to_delete:
+		frappe.db.sql('DELETE FROM `tabUser Permission` WHERE `name` IN ({})'.format( # nosec
+			','.join(['%s'] * len(user_permissions_to_delete))
+		), tuple(user_permissions_to_delete))

erpnext/public/js/utils.js

@@ -237,7 +237,7 @@ $.extend(erpnext.utils, {
 		let unscrub_option = frappe.model.unscrub(option);
 		let user_permission = frappe.defaults.get_user_permissions();
 		if(user_permission && user_permission[unscrub_option]) {
-			return user_permission[unscrub_option]["docs"];
+			return user_permission[unscrub_option].map(perm => perm.doc);
 		} else {
 			return $.map(locals[`:${unscrub_option}`], function(c) { return c.name; }).sort();
 		}

erpnext/stock/report/warehouse_wise_item_balance_age_and_value/warehouse_wise_item_balance_age_and_value.py

@@ -94,11 +94,10 @@ def validate_filters(filters):
 		filters["company"] = frappe.defaults.get_user_default("Company")
 
 def get_warehouse_list(filters):
-	from frappe.defaults import get_user_permissions
+	from frappe.core.doctype.user_permission.user_permission import get_permitted_documents
+
 	condition = ''
-	user_permitted_warehouse = filter(None, get_user_permissions()
+	user_permitted_warehouse = get_permitted_documents('Warehouse')
-		.get("Warehouse", {})
-		.get("docs", []))
 	value = ()
 	if user_permitted_warehouse:
 		condition = "and name in %s"