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

Co-authored-by: Diptanil Saha <diptanil@frappe.io>
This commit is contained in:
mergify[bot]
2026-07-15 14:19:07 +05:30
committed by GitHub
parent caea21208e
commit b1adec7e9e
7 changed files with 220 additions and 13 deletions

View File

@@ -490,3 +490,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

View File

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

View File

@@ -210,13 +210,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
},
{
@@ -482,13 +484,25 @@
"index_web_pages_for_search": 1,
"links": [],
"max_attachments": 4,
"modified": "2026-07-14 14:20:50.418911",
"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,

View File

@@ -92,7 +92,8 @@ class Project(Document):
def validate(self):
if not self.is_new():
self.copy_from_template() # nosemgrep
self.copy_from_template()
self.control_access_for_project_users()
self.send_welcome_email()
self.update_costing()
self.update_percent_complete()
@@ -205,6 +206,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", "")
@@ -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"""

View File

@@ -244,6 +244,61 @@ class TestProject(ERPNextTestSuite):
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(

View File

@@ -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")
)
@@ -66,6 +57,7 @@ def get_tasks(project, start=0, search=None, item_status=None):
@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",
{
@@ -106,6 +98,7 @@ def get_timesheets(project, start=0, search=None):
@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)}},
@@ -119,3 +112,12 @@ def get_attachments(project):
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

View File

@@ -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 erpnext.projects.doctype.project.test_project import make_project
from erpnext.templates.pages.projects import validate_and_get_project_user
from erpnext.tests.utils import ERPNextTestSuite
class TestProjectsPage(ERPNextTestSuite):
"""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)