fix(project): improved access control for project users (backport #56675) (#57180)

Co-authored-by: Diptanil Saha <diptanil@frappe.io>

erpnext/buying/doctype/supplier_quotation_item/supplier_quotation_item.json

@@ -307,6 +307,7 @@
    "fieldname": "net_rate",
    "fieldtype": "Currency",
    "label": "Net Rate",
+   "options": "currency",
    "print_hide": 1,
    "read_only": 1
   },
@@ -613,7 +614,7 @@
  "index_web_pages_for_search": 1,
  "istable": 1,
  "links": [],
- "modified": "2025-06-17 12:05:52.441645",
+ "modified": "2026-07-15 10:33:24.855979",
  "modified_by": "Administrator",
  "module": "Buying",
  "name": "Supplier Quotation Item",

erpnext/controllers/stock_controller.py

@@ -1513,9 +1513,10 @@ class StockController(AccountsController):
 
 
 @frappe.whitelist()
-def show_accounting_ledger_preview(company, doctype, docname):
+def show_accounting_ledger_preview(company: str, doctype: str, docname: str):
 	filters = frappe._dict(company=company, include_dimensions=1)
 	doc = frappe.get_doc(doctype, docname)
+	doc.check_permission("read")
 	doc.run_method("before_gl_preview")
 
 	gl_columns, gl_data = get_accounting_ledger_preview(doc, filters)
@@ -1526,9 +1527,10 @@ def show_accounting_ledger_preview(company, doctype, docname):
 
 
 @frappe.whitelist()
-def show_stock_ledger_preview(company, doctype, docname):
+def show_stock_ledger_preview(company: str, doctype: str, docname: str):
 	filters = frappe._dict(company=company)
 	doc = frappe.get_doc(doctype, docname)
+	doc.check_permission("read")
 	doc.run_method("before_sl_preview")
 
 	sl_columns, sl_data = get_stock_ledger_preview(doc, filters)

erpnext/patches.txt

@@ -442,3 +442,4 @@ erpnext.patches.v15_0.backfill_sla_link_filters_on_custom_field
 erpnext.patches.v15_0.backfill_sla_link_filters_on_docfield
 erpnext.patches.v16_0.crm_settings_handle_allowed_users_for_frappe_crm
 erpnext.patches.v16_0.backfill_pick_list_transferred_qty
+erpnext.patches.v16_0.access_control_for_project_users

erpnext/patches/v16_0/access_control_for_project_users.py

@@ -0,0 +1,33 @@
+import frappe
+
+
+def execute():
+	Project = frappe.qb.DocType("Project")
+	ProjectUser = frappe.qb.DocType("Project User")
+
+	query = (
+		frappe.qb.from_(Project)
+		.join(ProjectUser)
+		.on(Project.name == ProjectUser.parent)
+		.select(Project.name, ProjectUser.user)
+	)
+
+	proj_users = query.run(as_dict=1)
+
+	project_mapped_users = get_project_mapped_users(proj_users)
+
+	for d in proj_users:
+		if d.user in project_mapped_users[d.name]:
+			continue
+
+		frappe.share.add_docshare("Project", d.name, user=d.user)
+
+
+def get_project_mapped_users(proj_users):
+	projects = set([d.name for d in proj_users])
+	project_mapped_users = {}
+
+	for d in projects:
+		project_mapped_users[d] = [d.user for d in frappe.share.get_users("Project", d)]
+
+	return project_mapped_users

erpnext/projects/doctype/project/project.json

@@ -83,7 +83,7 @@
    "no_copy": 1,
    "oldfieldname": "status",
    "oldfieldtype": "Select",
-   "options": "Open\nCompleted\nCancelled",
+   "options": "Open\nOn hold\nCompleted\nCancelled",
    "search_index": 1
   },
   {
@@ -205,13 +205,15 @@
    "fieldname": "users",
    "fieldtype": "Table",
    "label": "Users",
-   "options": "Project User"
+   "options": "Project User",
+   "permlevel": 1
   },
   {
    "fieldname": "copied_from",
    "fieldtype": "Data",
    "hidden": 1,
    "label": "Copied From",
+   "permlevel": 1,
    "read_only": 1
   },
   {
@@ -464,13 +466,25 @@
  "index_web_pages_for_search": 1,
  "links": [],
  "max_attachments": 4,
- "modified": "2026-05-22 16:45:50.762759",
+ "modified": "2026-07-14 14:32:11.328347",
  "modified_by": "Administrator",
  "module": "Projects",
  "name": "Project",
  "naming_rule": "By \"Naming Series\" field",
  "owner": "Administrator",
  "permissions": [
+  {
+   "delete": 1,
+   "email": 1,
+   "export": 1,
+   "permlevel": 1,
+   "print": 1,
+   "read": 1,
+   "report": 1,
+   "role": "Projects Manager",
+   "share": 1,
+   "write": 1
+  },
   {
    "create": 1,
    "delete": 1,

erpnext/projects/doctype/project/project.py

@@ -61,7 +61,7 @@ class Project(Document):
 		project_type: DF.Link | None
 		sales_order: DF.Link | None
 		second_email: DF.Time | None
-		status: DF.Literal["Open", "Completed", "Cancelled"]
+		status: DF.Literal["Open", "On hold", "Completed", "Cancelled"]
 		subject: DF.Data | None
 		to_time: DF.Time | None
 		total_billable_amount: DF.Currency
@@ -93,6 +93,7 @@ class Project(Document):
 	def validate(self):
 		if not self.is_new():
 			self.copy_from_template()  # nosemgrep
+			self.control_access_for_project_users()
 		self.send_welcome_email()
 		self.update_costing()
 		self.update_percent_complete()
@@ -207,6 +208,7 @@ class Project(Document):
 		self.copy_from_template()  # nosemgrep
 		if self.sales_order:
 			frappe.db.set_value("Sales Order", self.sales_order, "project", self.name)
+		self.control_access_for_project_users()
 
 	def on_trash(self):
 		frappe.db.set_value("Sales Order", {"project": self.name}, "project", "")
@@ -264,8 +266,8 @@ class Project(Document):
 					pct_complete += row["progress"] * frappe.utils.safe_div(row["task_weight"], weight_sum)
 				self.percent_complete = flt(flt(pct_complete), 2)
 
-		# don't update status if it is cancelled
-		if self.status == "Cancelled":
+		# don't update status if it is manually set to cancelled or on hold
+		if self.status in ("Cancelled", "On hold"):
 			return
 
 		self.status = "Completed" if self.percent_complete == 100 else "Open"
@@ -377,6 +379,34 @@ class Project(Document):
 					)
 					user.welcome_email_sent = 1
 
+	def control_access_for_project_users(self):
+		def revoke_access_for_project_users(removed_users):
+			users = set([d.user for d in frappe.share.get_users(self.doctype, self.name)])
+			for user in removed_users:
+				if user not in users:
+					continue
+
+				frappe.share.remove(self.doctype, self.name, user)
+
+		def grant_access_for_project_users(new_users):
+			for user in new_users:
+				frappe.share.add_docshare(self.doctype, self.name, user=user)
+
+		current_users = set([d.user for d in self.users])
+		old_doc = self.get_doc_before_save()
+
+		if not old_doc:
+			grant_access_for_project_users(current_users)
+			return
+
+		previous_users = set([d.user for d in old_doc.users])
+
+		new_users = current_users - previous_users
+		removed_users = previous_users - current_users
+
+		revoke_access_for_project_users(removed_users)
+		grant_access_for_project_users(new_users)
+
 
 def get_timeline_data(doctype: str, name: str) -> dict[int, int]:
 	"""Return timeline for attendance"""

erpnext/projects/doctype/project/project_list.js

@@ -4,6 +4,8 @@ frappe.listview_settings["Project"] = {
 	get_indicator: function (doc) {
 		if (doc.status == "Open" && doc.percent_complete) {
 			return [__("{0}%", [cint(doc.percent_complete)]), "orange", "percent_complete,>,0|status,=,Open"];
+		} else if (doc.status == "On hold") {
+			return [__("On hold"), "blue", "status,=,On hold"];
 		} else {
 			return [__(doc.status), frappe.utils.guess_colour(doc.status), "status,=," + doc.status];
 		}

erpnext/projects/doctype/project/test_project.py

@@ -227,6 +227,61 @@ class TestProject(FrappeTestCase):
 		project.save()
 		self.assertEqual(project.status, "Completed")
 
+	def _create_portal_user(self, email):
+		"""A user with no Project-related role, so read access can only come from
+		control_access_for_project_users() sharing the doc with them."""
+		if not frappe.db.exists("User", email):
+			frappe.get_doc(
+				{
+					"doctype": "User",
+					"email": email,
+					"first_name": "Portal",
+					"send_welcome_email": 0,
+				}
+			).insert(ignore_permissions=True)
+		return email
+
+	def test_new_project_grants_access_to_its_users(self):
+		member = self._create_portal_user(f"new_proj_member_{frappe.generate_hash(length=6)}@example.com")
+
+		project = frappe.get_doc(
+			doctype="Project",
+			project_name=f"_Test New Project Access {frappe.generate_hash(length=6)}",
+			status="Open",
+			company="_Test Company",
+		)
+		project.append("users", {"user": member, "welcome_email_sent": 1})
+		project.insert()  # must not raise
+
+		self.assertTrue(project.has_permission(user=member))
+		shared_with = [d.user for d in frappe.share.get_users("Project", project.name)]
+		self.assertIn(member, shared_with)
+
+	def test_adding_and_removing_project_user_updates_access(self):
+		stays = self._create_portal_user(f"stays_{frappe.generate_hash(length=6)}@example.com")
+		leaves = self._create_portal_user(f"leaves_{frappe.generate_hash(length=6)}@example.com")
+
+		project = frappe.get_doc(
+			doctype="Project",
+			project_name=f"_Test Project User Membership {frappe.generate_hash(length=6)}",
+			status="Open",
+			company="_Test Company",
+		)
+		project.append("users", {"user": stays, "welcome_email_sent": 1})
+		project.insert()
+		self.assertTrue(project.has_permission(user=stays))
+
+		# adding a user on update (not insert) must also grant them access
+		project.append("users", {"user": leaves, "welcome_email_sent": 1})
+		project.save()
+		self.assertTrue(project.has_permission(user=leaves))
+
+		# removing a user must revoke the share that was granted for membership
+		project.users = [d for d in project.users if d.user != leaves]
+		project.save()
+		self.assertFalse(project.has_permission(user=leaves))
+		self.assertTrue(project.has_permission(user=stays))
+
 
 def get_project(name, template):
 	project = frappe.get_doc(

erpnext/selling/report/sales_person_wise_transaction_summary/sales_person_wise_transaction_summary.py

@@ -4,7 +4,9 @@
 
 import frappe
 from frappe import _, msgprint, qb
-from frappe.query_builder import Criterion
+from frappe.desk.reportview import build_match_conditions
+from frappe.query_builder import Case, Criterion
+from pypika.terms import LiteralValue
 
 from erpnext import get_company_currency
 
@@ -155,84 +157,94 @@ def get_columns(filters):
 
 
 def get_entries(filters):
-	date_field = filters["doc_type"] == "Sales Order" and "transaction_date" or "posting_date"
-	if filters["doc_type"] == "Sales Order":
-		qty_field = "delivered_qty"
-	else:
-		qty_field = "qty"
-	conditions, values = get_conditions(filters, date_field)
+	doc_type = filters["doc_type"]
 
-	entries = frappe.db.sql(
-		"""
-		SELECT
-			dt.name, dt.customer, dt.territory, dt.{} as posting_date, dt_item.item_code,
-			st.sales_person, st.allocated_percentage, dt_item.warehouse,
-		CASE
-			WHEN dt.status = "Closed" THEN dt_item.{} * dt_item.conversion_factor
-			ELSE dt_item.stock_qty
-		END as stock_qty,
-		CASE
-			WHEN dt.status = "Closed" THEN (dt_item.base_net_rate * dt_item.{} * dt_item.conversion_factor)
-			ELSE dt_item.base_net_amount
-		END as base_net_amount,
-		CASE
-			WHEN dt.status = "Closed" THEN ((dt_item.base_net_rate * dt_item.{} * dt_item.conversion_factor) * st.allocated_percentage/100)
-			ELSE dt_item.base_net_amount * st.allocated_percentage/100
-		END as contribution_amt
-		FROM
-			`tab{}` dt, `tab{} Item` dt_item, `tabSales Team` st
-		WHERE
-			st.parent = dt.name and dt.name = dt_item.parent and st.parenttype = {}
-			and dt.docstatus = 1 {} order by st.sales_person, dt.name desc
-		""".format(
-			date_field,
-			qty_field,
-			qty_field,
-			qty_field,
-			filters["doc_type"],
-			filters["doc_type"],
-			"%s",
-			conditions,
-		),
-		tuple([filters["doc_type"], *values]),
-		as_dict=1,
+	date_field = "transaction_date" if doc_type == "Sales Order" else "posting_date"
+	qty_field = "delivered_qty" if doc_type == "Sales Order" else "qty"
+
+	dt = frappe.qb.DocType(doc_type)
+	dt_item = frappe.qb.DocType(f"{doc_type} Item")
+	st = frappe.qb.DocType("Sales Team")
+
+	calc_qty = dt_item[qty_field] * dt_item.conversion_factor
+	calc_net_amount = dt_item.base_net_rate * calc_qty
+
+	stock_qty_case = Case().when(dt.status == "Closed", calc_qty).else_(dt_item.stock_qty).as_("stock_qty")
+
+	base_net_amount_case = (
+		Case()
+		.when(dt.status == "Closed", calc_net_amount)
+		.else_(dt_item.base_net_amount)
+		.as_("base_net_amount")
 	)
 
-	return entries
-
-
-def get_conditions(filters, date_field):
-	conditions = [""]
-	values = []
+	contribution_amt_case = (
+		Case()
+		.when(dt.status == "Closed", (calc_net_amount * st.allocated_percentage / 100))
+		.else_(dt_item.base_net_amount * st.allocated_percentage / 100)
+		.as_("contribution_amt")
+	)
 
+	# Only pass valid document-field filters to get_query; report-specific keys such as
+	# doc_type / sales_person / item_group are handled separately below.
+	doc_filters = {"docstatus": 1}
 	for field in ["company", "customer", "territory"]:
 		if filters.get(field):
-			conditions.append(f"dt.{field}=%s")
-			values.append(filters[field])
+			doc_filters[field] = filters.get(field)
+
+	if filters.get("from_date") and filters.get("to_date"):
+		doc_filters[date_field] = ["between", [filters.get("from_date"), filters.get("to_date")]]
+	elif filters.get("from_date"):
+		doc_filters[date_field] = [">=", filters.get("from_date")]
+	elif filters.get("to_date"):
+		doc_filters[date_field] = ["<=", filters.get("to_date")]
+
+	query = (
+		frappe.get_query(dt, filters=doc_filters)
+		.join(dt_item)
+		.on(dt.name == dt_item.parent)
+		.join(st)
+		.on(dt.name == st.parent)
+		.select(
+			dt.name,
+			dt.customer,
+			dt.territory,
+			dt[date_field].as_("posting_date"),
+			dt_item.item_code,
+			st.sales_person,
+			st.allocated_percentage,
+			dt_item.warehouse,
+			stock_qty_case,
+			base_net_amount_case,
+			contribution_amt_case,
+		)
+		.where(st.parenttype == doc_type)
+	)
 
 	if filters.get("sales_person"):
-		lft, rgt = frappe.get_value("Sales Person", filters.get("sales_person"), ["lft", "rgt"])
-		conditions.append(
-			f"exists(select name from `tabSales Person` where lft >= {lft} and rgt <= {rgt} and name=st.sales_person)"
+		lft, rgt = frappe.db.get_value("Sales Person", filters.get("sales_person"), ["lft", "rgt"])
+		sp = frappe.qb.DocType("Sales Person")
+		query = query.where(
+			st.sales_person.isin(frappe.qb.from_(sp).select(sp.name).where((sp.lft >= lft) & (sp.rgt <= rgt)))
 		)
 
-	if filters.get("from_date"):
-		conditions.append(f"dt.{date_field}>=%s")
-		values.append(filters["from_date"])
+	# only resolve items when an item_group/brand filter is set; otherwise get_items
+	# would return every item in the system and add a huge IN() clause on each run
+	if filters.get("item_group") or filters.get("brand"):
+		items = get_items(filters)
+		if not items:
+			# the item_group/brand filter matched nothing -> no rows
+			return []
+		query = query.where(dt_item.item_code.isin([d[0] for d in items]))
 
-	if filters.get("to_date"):
-		conditions.append(f"dt.{date_field}<=%s")
-		values.append(filters["to_date"])
+	query = query.orderby(st.sales_person).orderby(dt.name, order=frappe.qb.desc)
 
-	items = get_items(filters)
-	if items:
-		conditions.append("dt_item.item_code in (%s)" % ", ".join(["%s"] * len(items)))
-		values += items
-	else:
-		# return empty result, if no items are fetched after filtering on 'item group' and 'brand'
-		conditions.append("dt_item.item_code = Null")
+	# Apply user permissions (v15: ignore_permissions is not available)
+	match_conditions = build_match_conditions(doc_type)
+	if match_conditions:
+		query = query.where(LiteralValue(match_conditions))
 
-	return " and ".join(conditions), values
+	return query.run(as_dict=True)
 
 
 def get_items(filters):
@@ -259,8 +271,5 @@ def get_items(filters):
 
 
 def get_item_details():
-	item_details = {}
-	for d in frappe.db.sql("""SELECT `name`, `item_group`, `brand` FROM `tabItem`""", as_dict=1):
-		item_details.setdefault(d.name, d)
-
-	return item_details
+	items = frappe.get_all("Item", fields=["name", "item_group", "brand"])
+	return {d.name: d for d in items}

erpnext/templates/pages/projects.py

@@ -6,21 +6,12 @@ import frappe
 
 
 def get_context(context):
-	project_user = frappe.db.get_value(
-		"Project User",
-		{"parent": frappe.form_dict.project, "user": frappe.session.user},
-		["user", "view_attachments", "hide_timesheets"],
-		as_dict=True,
-	)
-	if frappe.session.user != "Administrator" and (not project_user or frappe.session.user == "Guest"):
-		raise frappe.PermissionError
+	project_user = validate_and_get_project_user(project=frappe.form_dict.project)
 
 	context.no_cache = 1
 	context.show_sidebar = True
 	project = frappe.get_doc("Project", frappe.form_dict.project)
 
-	project.has_permission("read")
-
 	project.tasks = get_tasks(
 		project.name, start=0, item_status="open", search=frappe.form_dict.get("search")
 	)
@@ -64,6 +55,22 @@ def get_tasks(project, start=0, search=None, item_status=None):
 	return list(filter(lambda x: not x.parent_task, tasks))
 
 
+@frappe.whitelist()
+def get_task_html(project: str, start: int = 0, item_status: str | None = None):
+	validate_and_get_project_user(project=project)
+	return frappe.render_template(
+		"erpnext/templates/includes/projects/project_tasks.html",
+		{
+			"doc": {
+				"name": project,
+				"project_name": project,
+				"tasks": get_tasks(project, start, item_status=item_status),
+			}
+		},
+		is_path=True,
+	)
+
+
 def get_timesheets(project, start=0, search=None):
 	filters = {"project": project}
 	if search:
@@ -89,9 +96,28 @@ def get_timesheets(project, start=0, search=None):
 	return timesheets
 
 
+@frappe.whitelist()
+def get_timesheet_html(project: str, start: int = 0):
+	validate_and_get_project_user(project=project)
+	return frappe.render_template(
+		"erpnext/templates/includes/projects/project_timesheets.html",
+		{"doc": {"timesheets": get_timesheets(project, start)}},
+		is_path=True,
+	)
+
+
 def get_attachments(project):
 	return frappe.get_all(
 		"File",
 		filters={"attached_to_name": project, "attached_to_doctype": "Project", "is_private": 0},
 		fields=["file_name", "file_url", "file_size"],
 	)
+
+
+def validate_and_get_project_user(project: str):
+	project_doc = frappe.get_doc("Project", project)
+	project_doc.check_permission()
+
+	project_user = next((d for d in project_doc.users if d.user == frappe.session.user), None)
+
+	return project_user

erpnext/templates/pages/test_projects.py

@@ -0,0 +1,72 @@
+# Copyright (c) 2026, Frappe Technologies Pvt. Ltd. and Contributors
+# License: GNU General Public License v3. See license.txt
+
+import frappe
+from frappe.tests.utils import FrappeTestCase
+
+from erpnext.projects.doctype.project.test_project import make_project
+from erpnext.templates.pages.projects import validate_and_get_project_user
+
+
+class TestProjectsPage(FrappeTestCase):
+	"""validate_and_get_project_user() gates the /projects portal page. It must raise
+	frappe.PermissionError for a user who can't read the Project, and otherwise return
+	that user's Project User row (or None if they're permitted but not listed as one --
+	e.g. an internal Projects Manager browsing the portal)."""
+
+	def _create_user(self, email):
+		if not frappe.db.exists("User", email):
+			frappe.get_doc(
+				{
+					"doctype": "User",
+					"email": email,
+					"first_name": "Portal",
+					"send_welcome_email": 0,
+				}
+			).insert(ignore_permissions=True)
+		return email
+
+	def test_raises_permission_error_for_user_without_access(self):
+		project = make_project({"project_name": f"_Test Portal Access {frappe.generate_hash(length=6)}"})
+		outsider = self._create_user(f"outsider_{frappe.generate_hash(length=6)}@example.com")
+
+		with self.set_user(outsider):
+			self.assertRaises(frappe.PermissionError, validate_and_get_project_user, project.name)
+
+	def test_allows_user_listed_as_project_user_and_returns_their_row(self):
+		# Being a Project User shares the Project with that user (see
+		# Project.control_access_for_project_users), which is what lets them past
+		# check_permission() here.
+		member = self._create_user(f"member_{frappe.generate_hash(length=6)}@example.com")
+
+		project = frappe.get_doc(
+			doctype="Project",
+			project_name=f"_Test Portal Access {frappe.generate_hash(length=6)}",
+			status="Open",
+			company="_Test Company",
+		)
+		project.append(
+			"users", {"user": member, "view_attachments": 1, "hide_timesheets": 1, "welcome_email_sent": 1}
+		)
+		project.insert()
+
+		with self.set_user(member):
+			project_user = validate_and_get_project_user(project.name)
+
+		self.assertIsNotNone(project_user)
+		self.assertEqual(project_user.user, member)
+		self.assertEqual(project_user.view_attachments, 1)
+		self.assertEqual(project_user.hide_timesheets, 1)
+
+	def test_allows_internally_permitted_user_not_listed_as_project_user(self):
+		# The permission gate must be the real permission system (check_permission()),
+		# not "is this user in the Project's users child table" -- a Projects Manager
+		# can open any project's portal page without ever being added as its user.
+		project = make_project({"project_name": f"_Test Portal Access {frappe.generate_hash(length=6)}"})
+		manager = self._create_user(f"manager_{frappe.generate_hash(length=6)}@example.com")
+		frappe.get_doc("User", manager).add_roles("Projects Manager")
+
+		with self.set_user(manager):
+			project_user = validate_and_get_project_user(project.name)
+
+		self.assertIsNone(project_user)