Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions hrms/hr/doctype/leave_application/leave_application.py
Original file line number Diff line number Diff line change
Expand Up @@ -993,7 +993,7 @@ def get_number_of_leave_days(

@frappe.whitelist()
def get_leave_details(employee: str, date: str | datetime.date, for_salary_slip: bool = False) -> dict:
frappe.has_permission("Employee", "read", employee, throw=True)
validate_leave_access(employee)

allocation_records = get_leave_allocation_records(employee, date)
leave_allocation = {}
Expand Down Expand Up @@ -1054,8 +1054,7 @@ def get_leave_balance_on(
if True, returns a dict eg: {'leave_balance': 10, 'leave_balance_for_consumption': 1}
else, returns leave_balance (in this case 10)
"""
if frappe.request:
frappe.has_permission("Employee", "read", employee, throw=True)
validate_leave_access(employee)

if not to_date:
to_date = nowdate()
Expand Down Expand Up @@ -1559,3 +1558,13 @@ def get_leave_approver_and_mandatory(employee: str) -> dict:
"is_mandatory": 1 if mandatory else 0,
"leave_approver": get_leave_approver(employee),
}


def validate_leave_access(employee):
employee_user = frappe.db.get_value("Employee", employee, "user_id")
leave_approver = get_leave_approver(employee)

if frappe.session.user not in (employee_user, leave_approver) and (
not frappe.has_permission("Employee", "read", employee)
):
frappe.throw(_("Not permitted"), frappe.PermissionError)
77 changes: 76 additions & 1 deletion hrms/hr/doctype/leave_application/test_leave_application.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
# License: GNU General Public License v3. See license.txt

import frappe
from frappe.permissions import clear_user_permissions_for_doctype
from frappe.permissions import add_user_permission, clear_user_permissions_for_doctype
from frappe.utils import (
add_days,
add_months,
Expand Down Expand Up @@ -1423,6 +1423,81 @@ def test_status_on_discard(self):
application.reload()
self.assertEqual(application.status, "Cancelled")

def test_leave_access_control_flow(self):
leave_approver = "test_approver_access@example.com"
make_employee(leave_approver, "_Test Company")

employee_user = "test_leave_access@example.com"
employee = make_employee(employee_user, "_Test Company", leave_approver=leave_approver)
make_allocation_record(employee, from_date="2026-04-1", to_date="2027-03-31")

random_user = "unauth_user@example.com"
make_employee(random_user, "_Test Company")

frappe.set_user(employee_user)
leave_application = make_leave_application(
employee,
from_date="2026-04-11",
to_date="2026-04-11",
leave_type="_Test Leave Type",
status="Draft",
leave_approver=leave_approver,
submit=False,
)

# Unauthorized user should not access leave data
frappe.set_user(random_user)
doc = frappe.get_doc("Leave Application", leave_application.name)
self.assertRaises(frappe.PermissionError, doc.check_permission, "read")
self.assertRaises(
frappe.PermissionError,
get_leave_details,
employee,
leave_application.from_date,
)

frappe.set_user(leave_approver)
leave_application.status = "Approved"
leave_application.submit()
self.assertEqual(leave_application.docstatus, 1)

def test_leave_approver_with_restricted_employee_access(self):
permitted_employee = make_employee(
"test_employee_with_permission@example.com",
"_Test Company",
)
leave_approver = "approver_restricted@example.com"
make_employee(leave_approver, "_Test Company")
add_user_permission("Employee", permitted_employee, leave_approver)

target_employee_user = "employee_without_user_perm_for_leave_approver@example.com"
target_employee = make_employee(target_employee_user, "_Test Company", leave_approver=leave_approver)
make_allocation_record(target_employee, from_date="2026-04-01", to_date="2027-03-31")

frappe.set_user(target_employee_user)
leave_application = make_leave_application(
target_employee,
from_date="2026-04-20",
to_date="2026-04-20",
leave_type="_Test Leave Type",
status="Draft",
leave_approver=leave_approver,
submit=False,
)

# Approver not having access to target employee master but can approve leave
frappe.set_user(leave_approver)
doc_employee = frappe.get_doc("Employee", target_employee)
self.assertRaises(
frappe.PermissionError,
doc_employee.check_permission,
"read",
)

leave_application.status = "Approved"
leave_application.submit()
self.assertEqual(leave_application.docstatus, 1)


def create_carry_forwarded_allocation(employee, leave_type, date=None):
date = date or nowdate()
Expand Down
4 changes: 2 additions & 2 deletions hrms/hr/doctype/shift_assignment/shift_assignment.py
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ def get_events(start: str | date, end: str | date, filters: list | None = None):


def mark_expired_shift_assignments_as_inactive():
today = getdate()
yesterday = add_days(getdate(), -1)
shift_assignment = frappe.qb.DocType("Shift Assignment")

expired_assignments = (
Expand All @@ -210,7 +210,7 @@ def mark_expired_shift_assignments_as_inactive():
(shift_assignment.docstatus == 1)
& (shift_assignment.status == "Active")
& (shift_assignment.end_date.isnotnull())
& (shift_assignment.end_date < today)
& (shift_assignment.end_date < yesterday)
)
).run(pluck=True)

Expand Down
3 changes: 2 additions & 1 deletion hrms/hr/doctype/shift_assignment/test_shift_assignment.py
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,7 @@ def test_auto_attendance_calculates_ot_for_default_shift(self):

def test_mark_expired_shift_assignments_as_inactive(self):
today = getdate()
yesterday = add_days(today, -1)
shift_type = setup_shift_type(shift_type="Expired Shift", start_time="08:00:00", end_time="16:00:00")

expired_employee = make_employee("test_expired_shift_assignment@example.com", company="_Test Company")
Expand All @@ -310,7 +311,7 @@ def test_mark_expired_shift_assignments_as_inactive(self):
shift_type.name,
expired_employee,
add_days(today, -7),
add_days(today, -1),
add_days(yesterday, -1),
)
active_assignment = make_shift_assignment(
shift_type.name,
Expand Down
Loading
Loading