From 074f9f082856bb61c6d89da70fbf2344d3a0d812 Mon Sep 17 00:00:00 2001 From: rohitwaghchaure Date: Sat, 5 Sep 2026 13:54:20 +0530 Subject: [PATCH] fix: check write permission in whitelisted document methods (backport #58689) (#58701) * fix: check write permission in whitelisted document methods * test: permission coverage for production plan status roll-ups * fix: add type hints to whitelisted arguments and submit MR in test --- erpnext/crm/doctype/lead/lead.py | 4 +- .../production_plan/production_plan.py | 4 +- .../production_plan/test_production_plan.py | 56 +++++++++++++++++++ .../doctype/work_order/work_order.py | 2 + .../import_supplier_invoice.py | 2 + erpnext/stock/doctype/batch/batch.py | 2 + .../material_request/material_request.py | 1 + .../repost_item_valuation.py | 2 + .../stock_closing_entry.py | 3 + .../stock_reposting_settings.py | 2 + 10 files changed, 76 insertions(+), 2 deletions(-) diff --git a/erpnext/crm/doctype/lead/lead.py b/erpnext/crm/doctype/lead/lead.py index bd89193f1b6..bd0fa414903 100644 --- a/erpnext/crm/doctype/lead/lead.py +++ b/erpnext/crm/doctype/lead/lead.py @@ -237,7 +237,9 @@ class Lead(SellingController, CRMNote): return frappe.db.get_value("Quotation", {"party_name": self.name, "docstatus": 1, "status": "Lost"}) @frappe.whitelist() - def create_prospect_and_contact(self, data): + def create_prospect_and_contact(self, data: dict): + self.check_permission("write") + data = frappe._dict(data) if data.create_contact: self.create_contact() diff --git a/erpnext/manufacturing/doctype/production_plan/production_plan.py b/erpnext/manufacturing/doctype/production_plan/production_plan.py index 9b992722fc7..0301fe2ba06 100644 --- a/erpnext/manufacturing/doctype/production_plan/production_plan.py +++ b/erpnext/manufacturing/doctype/production_plan/production_plan.py @@ -687,7 +687,9 @@ class ProductionPlan(Document): frappe.delete_doc("Work Order", d.name) @frappe.whitelist() - def set_status(self, close=None, update_bin=False): + def set_status(self, close: bool | None = None, update_bin: bool = False): + self.check_permission("write") + self.status = {0: "Draft", 1: "Submitted", 2: "Cancelled"}.get(self.docstatus) if close: diff --git a/erpnext/manufacturing/doctype/production_plan/test_production_plan.py b/erpnext/manufacturing/doctype/production_plan/test_production_plan.py index 7ba22675724..1f95609499a 100644 --- a/erpnext/manufacturing/doctype/production_plan/test_production_plan.py +++ b/erpnext/manufacturing/doctype/production_plan/test_production_plan.py @@ -3167,6 +3167,46 @@ class TestProductionPlan(ERPNextTestSuite): "The phantom BOM was not re-exploded for the second po_item.", ) + def test_set_status_requires_write_permission(self): + pln = create_production_plan(item_code="Test Production Item 1") + + with self.set_user(create_user_without_production_plan_access()): + doc = frappe.get_doc("Production Plan", pln.name) + self.assertRaises(frappe.PermissionError, doc.set_status) + + def test_work_order_status_rollup_without_production_plan_permission(self): + pln = create_production_plan(item_code="Test Production Item 1") + pln.make_work_order() + + wo_name = frappe.db.get_value("Work Order", {"production_plan": pln.name}, "name") + frappe.db.set_value("Production Plan Item", pln.po_items[0].name, "ordered_qty", 99) + + with self.set_user(create_user_without_production_plan_access()): + frappe.get_doc("Work Order", wo_name).update_ordered_qty() + + pln.reload() + self.assertEqual(pln.po_items[0].ordered_qty, 0.0) + self.assertEqual(pln.status, "Submitted") + + def test_material_request_status_rollup_without_production_plan_permission(self): + pln = create_production_plan(item_code="Test Production Item 1") + pln.make_material_request() + + plan_item = pln.mr_items[0].name + mr_name = frappe.db.get_value( + "Material Request Item", {"material_request_plan_item": plan_item}, "parent" + ) + frappe.get_doc("Material Request", mr_name).submit() + frappe.db.set_value("Material Request Plan Item", plan_item, "requested_qty", 0) + + with self.set_user(create_user_without_production_plan_access()): + frappe.get_doc("Material Request", mr_name).update_requested_qty_in_production_plan() + + pln.reload() + requested_qty = frappe.db.get_value("Material Request Plan Item", plan_item, "requested_qty") + self.assertGreater(requested_qty, 0) + self.assertEqual(pln.status, "Material Requested") + def create_production_plan(**args): """ @@ -3299,3 +3339,19 @@ def make_bom(**args): frappe.set_value("Item", args.item, "default_bom", bom.name) return bom + + +def create_user_without_production_plan_access(): + user = "test_production_plan_no_access@example.com" + if not frappe.db.exists("User", user): + frappe.get_doc( + { + "doctype": "User", + "email": user, + "first_name": "Production Plan No Access", + "send_welcome_email": 0, + "roles": [{"doctype": "Has Role", "role": "Stock User"}], + } + ).insert(ignore_permissions=True) + + return user diff --git a/erpnext/manufacturing/doctype/work_order/work_order.py b/erpnext/manufacturing/doctype/work_order/work_order.py index 7e5c41b06aa..68a71e69569 100644 --- a/erpnext/manufacturing/doctype/work_order/work_order.py +++ b/erpnext/manufacturing/doctype/work_order/work_order.py @@ -924,6 +924,7 @@ class WorkOrder(Document): def update_production_plan_status(self): production_plan = frappe.get_doc("Production Plan", self.production_plan) + production_plan.flags.ignore_permissions = True produced_qty = 0 if self.production_plan_item: total_qty = frappe.get_all( @@ -1351,6 +1352,7 @@ class WorkOrder(Document): ) doc = frappe.get_doc("Production Plan", self.production_plan) + doc.flags.ignore_permissions = True doc.set_status() doc.db_set("status", doc.status) diff --git a/erpnext/regional/doctype/import_supplier_invoice/import_supplier_invoice.py b/erpnext/regional/doctype/import_supplier_invoice/import_supplier_invoice.py index 436bfafdaf8..61dde7aa34a 100644 --- a/erpnext/regional/doctype/import_supplier_invoice/import_supplier_invoice.py +++ b/erpnext/regional/doctype/import_supplier_invoice/import_supplier_invoice.py @@ -157,6 +157,8 @@ class ImportSupplierInvoice(Document): @frappe.whitelist() def process_file_data(self): + self.check_permission("write") + self.db_set("status", "Processing File Data", notify=True, commit=True) frappe.enqueue_doc(self.doctype, self.name, "import_xml_data", queue="long", timeout=3600) diff --git a/erpnext/stock/doctype/batch/batch.py b/erpnext/stock/doctype/batch/batch.py index dc73e9ffddf..8ddfcf973fe 100644 --- a/erpnext/stock/doctype/batch/batch.py +++ b/erpnext/stock/doctype/batch/batch.py @@ -157,6 +157,8 @@ class Batch(Document): @frappe.whitelist() def recalculate_batch_qty(self): + self.check_permission("write") + batches = get_batch_qty( batch_no=self.name, item_code=self.item, diff --git a/erpnext/stock/doctype/material_request/material_request.py b/erpnext/stock/doctype/material_request/material_request.py index b9e141878e9..466646775a9 100644 --- a/erpnext/stock/doctype/material_request/material_request.py +++ b/erpnext/stock/doctype/material_request/material_request.py @@ -483,6 +483,7 @@ class MaterialRequest(BuyingController): for production_plan in production_plans: doc = frappe.get_doc("Production Plan", production_plan) + doc.flags.ignore_permissions = True doc.set_status() doc.db_set("status", doc.status) diff --git a/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py b/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py index d0e1cac2b23..60a1ed17920 100644 --- a/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py +++ b/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py @@ -277,6 +277,8 @@ class RepostItemValuation(Document): @frappe.whitelist() def restart_reposting(self): + self.check_permission("write") + self.set_status("Queued", write=False) self.current_index = 0 self.distinct_item_and_warehouse = None diff --git a/erpnext/stock/doctype/stock_closing_entry/stock_closing_entry.py b/erpnext/stock/doctype/stock_closing_entry/stock_closing_entry.py index eb72f1e54cd..72a4e4db283 100644 --- a/erpnext/stock/doctype/stock_closing_entry/stock_closing_entry.py +++ b/erpnext/stock/doctype/stock_closing_entry/stock_closing_entry.py @@ -153,6 +153,8 @@ class StockClosingEntry(Document): @frappe.whitelist(methods=["POST"]) def enqueue_job(self): + self.check_permission("write") + self.db_set("status", "In Progress") enqueue(prepare_closing_stock_balance, name=self.name, queue="long", timeout=1500) frappe.msgprint( @@ -163,6 +165,7 @@ class StockClosingEntry(Document): @frappe.whitelist(methods=["POST"]) def regenerate_closing_balance(self): + self.check_permission("write") self.validate_closed_period_lock() self.remove_stock_closing() self.enqueue_job() diff --git a/erpnext/stock/doctype/stock_reposting_settings/stock_reposting_settings.py b/erpnext/stock/doctype/stock_reposting_settings/stock_reposting_settings.py index f703a694dad..b9d6c9c7a3d 100644 --- a/erpnext/stock/doctype/stock_reposting_settings/stock_reposting_settings.py +++ b/erpnext/stock/doctype/stock_reposting_settings/stock_reposting_settings.py @@ -73,6 +73,8 @@ class StockRepostingSettings(Document): def convert_to_item_wh_reposting(self): """Convert Transaction reposting to Item Warehouse based reposting if Item Based Reposting has enabled.""" + self.check_permission("write") + reposting_data = get_reposting_entries() vouchers = [d.voucher_no for d in reposting_data]