fix(manufacturing): add permission checks to BOM whitelisted methods (#58789)

This commit is contained in:
Diptanil Saha
2026-09-07 00:49:03 +05:30
committed by GitHub
parent 7b53402eae
commit 223a97904a
4 changed files with 87 additions and 9 deletions

View File

@@ -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",

View File

@@ -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

View File

@@ -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:

View File

@@ -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()