mirror of
https://github.com/frappe/erpnext.git
synced 2026-08-28 14:18:24 +00:00
fix(project): improved access control for project users (#56675)
* fix: permission check for `get_task_html` and `get_timesheet_html` * fix(project): enabled project access control for users without `Projects User` Role * fix(portal): validate user permissions for project portal * fix: patch to add docshare for the project users * fix(patch): selecting correct column on the query * fix(project): grant access to all the current users for new project * fix(portal): fixed condition to display timesheets on project * test(portal): add access control tests for project user * fix(project): using `frappe.has_permission` instead of `self.has_permission` to validate user permissions * fix(project): granting docshare access for every ProjectUser Roles for an User can be removed any time or an User Permission can be added which might restrict the access to the Project. * fix(patch): create docshare documents for non-cancelled projects and users who have no docshare documents * test(project): removed `test_control_access_does_not_touch_users_with_real_permission`
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -90,6 +90,7 @@ class Project(Document):
|
||||
def validate(self):
|
||||
if not self.is_new():
|
||||
self.copy_from_template()
|
||||
self.control_access_for_project_users()
|
||||
self.send_welcome_email()
|
||||
self.update_costing()
|
||||
self.update_percent_complete()
|
||||
@@ -239,6 +240,7 @@ class Project(Document):
|
||||
def after_insert(self):
|
||||
self.copy_from_template("after_insert")
|
||||
self.link_with_sales_order()
|
||||
self.control_access_for_project_users()
|
||||
|
||||
def link_with_sales_order(self) -> None:
|
||||
"""Back-link the source Sales Order to this project.
|
||||
@@ -434,6 +436,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"""
|
||||
|
||||
@@ -436,6 +436,61 @@ class TestProject(ERPNextTestSuite):
|
||||
self.assertEqual(project.total_consumed_material_cost, sum(row.amount for row in issue.items))
|
||||
self.assertGreater(project.total_consumed_material_cost, 0)
|
||||
|
||||
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(
|
||||
|
||||
Reference in New Issue
Block a user