From 223a97904acbd25e02213269945b48a6aea5b68c Mon Sep 17 00:00:00 2001 From: Diptanil Saha Date: Mon, 7 Sep 2026 00:49:03 +0530 Subject: [PATCH] fix(manufacturing): add permission checks to BOM whitelisted methods (#58789) --- erpnext/manufacturing/doctype/bom/bom.json | 18 +++--- erpnext/manufacturing/doctype/bom/bom.py | 12 +++- erpnext/manufacturing/doctype/bom/mapper.py | 3 + erpnext/manufacturing/doctype/bom/test_bom.py | 63 ++++++++++++++++++- 4 files changed, 87 insertions(+), 9 deletions(-) diff --git a/erpnext/manufacturing/doctype/bom/bom.json b/erpnext/manufacturing/doctype/bom/bom.json index f2c4fd183cf..49f13f52919 100644 --- a/erpnext/manufacturing/doctype/bom/bom.json +++ b/erpnext/manufacturing/doctype/bom/bom.json @@ -771,7 +771,7 @@ "image_field": "image", "is_submittable": 1, "links": [], - "modified": "2026-08-23 15:20:11.032436", + "modified": "2026-09-06 20:45:00.000000", "modified_by": "Administrator", "module": "Manufacturing", "name": "BOM", @@ -808,12 +808,12 @@ "select": 1 }, { - "role": "Purchase Manager", - "select": 1 + "read": 1, + "role": "Purchase Manager" }, { - "role": "Purchase User", - "select": 1 + "read": 1, + "role": "Purchase User" }, { "role": "Sales Manager", @@ -824,8 +824,12 @@ "select": 1 }, { - "role": "Stock Manager", - "select": 1 + "read": 1, + "role": "Stock Manager" + }, + { + "read": 1, + "role": "Stock User" } ], "row_format": "Dynamic", diff --git a/erpnext/manufacturing/doctype/bom/bom.py b/erpnext/manufacturing/doctype/bom/bom.py index 560158fb955..25843fd7ae4 100644 --- a/erpnext/manufacturing/doctype/bom/bom.py +++ b/erpnext/manufacturing/doctype/bom/bom.py @@ -1343,7 +1343,11 @@ def get_bom_items_as_dict( fetch_secondary_items=0, include_non_stock_items=False, fetch_qty_in_stock_uom=True, + ignore_permissions=True, ): + if not ignore_permissions: + frappe.has_permission("BOM", "read", doc=bom, throw=True) + item_dict = {} opts = frappe._dict( qty=qty, @@ -1351,6 +1355,7 @@ def get_bom_items_as_dict( fetch_secondary_items=fetch_secondary_items, include_non_stock_items=include_non_stock_items, fetch_qty_in_stock_uom=fetch_qty_in_stock_uom, + ignore_permissions=ignore_permissions, ) items = _query_bom_items(bom, company, opts) @@ -1606,6 +1611,7 @@ def _merge_phantom_bom_items(item_dict, item, company, opts): fetch_secondary_items=opts.fetch_secondary_items, include_non_stock_items=opts.include_non_stock_items, fetch_qty_in_stock_uom=opts.fetch_qty_in_stock_uom, + ignore_permissions=opts.ignore_permissions, ) for k, v in data.items(): @@ -1645,7 +1651,11 @@ def _set_default_accounts_for_items(item_dict, company): @frappe.whitelist() def get_bom_items(bom: str, company: str, qty: float = 1, fetch_exploded: int = 1): - items = get_bom_items_as_dict(bom, company, qty, fetch_exploded, include_non_stock_items=True).values() + frappe.has_permission("BOM", "read", doc=bom, throw=True) + + items = get_bom_items_as_dict( + bom, company, qty, fetch_exploded, include_non_stock_items=True, ignore_permissions=False + ).values() items = list(items) items.sort(key=lambda item: item.item_code) return items diff --git a/erpnext/manufacturing/doctype/bom/mapper.py b/erpnext/manufacturing/doctype/bom/mapper.py index f237172fb56..e99e994f355 100644 --- a/erpnext/manufacturing/doctype/bom/mapper.py +++ b/erpnext/manufacturing/doctype/bom/mapper.py @@ -85,6 +85,9 @@ def get_bom_diff(bom1: str, bom2: str): doc1 = frappe.get_doc("BOM", bom1) doc2 = frappe.get_doc("BOM", bom2) + doc1.check_permission() + doc2.check_permission() + out = get_diff(doc1, doc2) out.row_changed, out.added, out.removed = [], [], [] for df in doc1.meta.fields: diff --git a/erpnext/manufacturing/doctype/bom/test_bom.py b/erpnext/manufacturing/doctype/bom/test_bom.py index 8f3c8b39168..afdd89fc0ac 100644 --- a/erpnext/manufacturing/doctype/bom/test_bom.py +++ b/erpnext/manufacturing/doctype/bom/test_bom.py @@ -55,7 +55,47 @@ class TestBOM(ERPNextTestSuite): def test_get_items_list(self): from erpnext.manufacturing.doctype.bom.bom import get_bom_items - self.assertEqual(len(get_bom_items(bom=get_default_bom(), company="_Test Company")), 3) + bom = get_default_bom() + self.assertEqual(len(get_bom_items(bom=bom, company="_Test Company")), 3) + + # the roles that own the Stock Entry / Material Request workflows fetch components here + with self.set_user(make_user_with_roles("_test_bom_stock_user@example.com", "Stock User")): + self.assertEqual(len(get_bom_items(bom=bom, company="_Test Company")), 3) + + with self.set_user(make_user_with_roles("_test_bom_no_access@example.com")): + self.assertRaises(frappe.PermissionError, get_bom_items, bom, "_Test Company") + + @timeout + def test_get_items_as_dict_only_checks_permission_when_asked(self): + from erpnext.manufacturing.doctype.bom.bom import get_bom_items_as_dict + + bom = get_default_bom() + with self.set_user(make_user_with_roles("_test_bom_no_access@example.com")): + # internal callers keep the privileged default, and nested BOMs inherit it + self.assertTrue(get_bom_items_as_dict(bom=bom, company="_Test Company", fetch_exploded=0)) + self.assertRaises( + frappe.PermissionError, + partial( + get_bom_items_as_dict, + bom=bom, + company="_Test Company", + fetch_exploded=0, + ignore_permissions=False, + ), + ) + + @timeout + def test_get_bom_diff_checks_both_boms(self): + from erpnext.manufacturing.doctype.bom.mapper import get_bom_diff + + bom1 = get_default_bom() + bom2 = get_default_bom("_Test FG Item") + + with self.set_user(make_user_with_roles("_test_bom_stock_user@example.com", "Stock User")): + self.assertTrue(get_bom_diff(bom1, bom2)) + + with self.set_user(make_user_with_roles("_test_bom_no_access@example.com")): + self.assertRaises(frappe.PermissionError, get_bom_diff, bom1, bom2) @timeout def test_get_items_keeps_bom_no_phantom_pair_coherent(self): @@ -1406,6 +1446,27 @@ def get_default_bom(item_code="_Test FG Item 2"): return frappe.db.get_value("BOM", {"item": item_code, "is_active": 1, "is_default": 1}) +def make_user_with_roles(email, *roles): + """A user holding exactly `roles`, so permission boundaries are pinned to the roles alone.""" + if not frappe.db.exists("User", email): + frappe.get_doc( + { + "doctype": "User", + "email": email, + "first_name": email.split("@")[0], + "send_welcome_email": 0, + } + ).insert(ignore_permissions=True) + + user = frappe.get_doc("User", email) + if existing := [row.role for row in user.roles]: + user.remove_roles(*existing) + if roles: + user.add_roles(*roles) + + return email + + def level_order_traversal(node): traversal = [] q = deque()