Feature #139
open[HR][P1] Access and privacy (API)
0%
Description
Owner checks the deployed HR authorization mode, then flip to Enforce. Hospital-scope attendance, roster, leave, balance and holiday endpoints. Close the attendance PUT/check-in/check-out/status side door. Server-side module flag on payroll/salary/appraisal/ESS-payslip/document APIs (live today despite the portal flag being off); drop payroll and appraisal keys from Department Head; relationship checks on appraisal reviews; restrict grantable roles in the employee wizard. Masked PII DTOs (Aadhaar/PAN/bank) with audited reveal; remove the bulk by-hospital PII endpoint. ESS landing and gating plumbing.
Size: L
Needs owner decision #1 (see #137).
Source: clinq-docs/modules/hr/reviews/2026-09-28-ui-audit/ (branch docs/hr-ui-audit-2026-09-28)
Findings covered:
- A-01
- B-01
- B-39
- C-05
- C-21
- C-39
- B-18
- B-19
- D-01
- D-10
- D-18
- D-37
- D-14
- D-15
- D-31
- A-04
- A-05
- A-06
- A-19
- A-20
- A-21
- A-30
SV Updated by Sravan V S about 2 months ago
- Assignee set to Sravan V S
S- Updated by SK - 13 days ago
- Status changed from New to Resolved
- Assignee changed from SK - to Roshni Harish.
- Resolved By set to SK -
Fixed and merged to dev on nuvita-hr. Worked from the ticket's terse description, verified each of the 9 named areas against current code first (source audit doc confirmed permanently lost, see #137). The Audit-to-Enforce authorization-mode flip itself was deliberately NOT touched, per your confirmed answer on #137 - everything below makes the underlying logic correct regardless of which mode is currently active, so it's safe to ship ahead of that separate decision.
- Hospital-scope attendance/roster/leave/balance/holiday - confirmed real cross-hospital leaks: several endpoints fell back to "every hospital in the tenant" when no hospital id was supplied, and several more trusted a caller-supplied employee/record id with no hospital check at all. Fixed via a new shared hospital-access guard, wired into every affected controller. Holidays' own hospital-scoped listing was already fine; its bare-by-id lookup has the same low-severity gap as the others but was left open (non-PII, flagged rather than spending more on it).
- Attendance PUT/check-in/check-out/status side door - confirmed real: these 4 actions wrote attendance directly, bypassing the proper manual-punch approval workflow (self-approval block, qualified-approver check, audit trail) entirely - and check-in/check-out have zero real callers today. Raised their permission requirement to the same elevated key the real workflow already requires; confirmed both roles that currently hold it still work.
- Server-side module/capability gates on Payroll/Salary/Appraisal/ESS-payslip/Documents - confirmed: none of these had a server-side check at all, so disabling a module's flag only hid the UI, the API kept answering. Added the proper capability gate to each, matching how a licensed tenant's flags already map to these exact routes.
- Drop payroll/appraisal keys from Department Head - payroll was already fixed in a prior change (confirmed, no action needed). Appraisal keys were deliberately kept, with a documented reason: they're how a department head reviews their own team today, and removing them would break real functionality - the actual gap (any holder could review ANY employee, not just their own team) is fixed properly in item 5 instead. Flagging this as a considered deviation from the ticket's literal wording, not an oversight.
- Relationship checks on appraisal reviews - confirmed real: no check existed that a self-review was for yourself, or that a manager-review was for someone you actually manage. Fixed - now requires the real reporting relationship, with an explicit override for HR Manager/Hospital Administrator filing on someone's behalf.
- Restrict grantable roles in employee wizard - confirmed the realistic risk (an HR Manager, the only role besides Hospital Administrator that can reach this wizard, minting a new Hospital Administrator account) and closed that specific case. Did not build a general role-hierarchy check - there's no rank/seniority concept anywhere on the platform to build it on, and inventing one is a cross-module (admin-owned) decision outside what's safe to guess at here. Flagging as a real open gap for the broader case.
- Masked PII DTOs with audited reveal - confirmed Aadhaar/PAN/bank details were returned in full plaintext to anyone with ordinary employee-read access. Now masked by default (last 4 digits visible), with a separate, explicitly audited endpoint for the real value when genuinely needed.
- Bulk by-hospital PII endpoint - confirmed real: one call returned every employee's full PII for an entire hospital, with no legitimate caller anywhere in the platform (checked every other module). Removed.
- ESS landing/gating plumbing - checked and already fine on both sides; no gap beyond the capability-gating already covered in item 3. The actual "who ESS is meant for" question stays open per #137.
Tests: HR backend unit 1319/1319 passed; HR backend integration 250/250 passed; HR frontend 105/105 files, 809/809 tests passed, type-check clean. No new migrations - no schema changes.
RH Updated by Roshni Harish. 10 days ago
- Assignee changed from Roshni Harish. to Sravan V S