-
Notifications
You must be signed in to change notification settings - Fork 4
feat: add payroll cancellation guards #926
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
8edadbe to
04b4196
Compare
src/components/Payroll/helpers.ts
Outdated
| const now = new Date() | ||
| const deadline = new Date(payroll.payrollDeadline) | ||
|
|
||
| const nowInPT = new Date(now.getTime() + getPacificTimeOffset(now) * 60 * 60 * 1000) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: i wonder if we could export this constant from dateFormatting and use that instead of hard coding these numbers here
| const MS_PER_HOUR = 1000 * 60 * 60 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great idea! I will update that.
|
This pull request contains client-side enforcement of a business-critical 4:00 PM PT payroll cancellation deadline using the device clock (new Date()), and a fragile timezone/DST calculation that relies on the client's local timezone (getPacificTimeOffset), making the deadline check bypassable by changing the device clock and unreliable for users outside Pacific Time.
Insecure client-side enforcement of business-critical deadline and fragile timezone logic. in
|
| Vulnerability | Insecure client-side enforcement of business-critical deadline and fragile timezone logic. |
|---|---|
| Description | The canCancelPayroll function enforces a business-critical 4:00 PM PT cancellation deadline using the client's local system clock (new Date()). This allows a user to bypass the restriction by simply modifying their device's clock. Furthermore, the getPacificTimeOffset function incorrectly calculates Daylight Saving Time (DST) transitions by using local Date constructors (new Date(year, 2, 1)), which rely on the client's local timezone. This results in incorrect UTC offsets for users outside of the Pacific Time zone, causing the deadline check to be unreliable and potentially allowing or blocking cancellations incorrectly based on the user's geographical location. |
embedded-react-sdk/src/components/Payroll/helpers.ts
Lines 635 to 638 in c93e75d
| const now = new Date() | |
| const deadline = new Date(payroll.payrollDeadline) | |
| const nowInPT = new Date(now.getTime() + getPacificTimeOffset(now) * MS_PER_HOUR) |
All finding details can be found in the DryRun Security Dashboard.
Overview
SDK-194
Implements business rule guards to prevent cancelling payrolls when the cancellation window has expired. Users can now only cancel processed payrolls before 4:00 PM PT on the payroll deadline date.
Problem
Previously, the cancel payroll UI was always available for processed payrolls regardless of timing, and the cutoff time was incorrectly set to 3:30 PM PT instead of 4:00 PM PT. This could allow users to attempt cancellations that would fail at the API level.
Solution
canCancelPayroll()helper function that enforces business rulesBusiness Rules Enforced
A payroll can only be cancelled when ALL of the following are true:
processed === true)payroll_deadlinedatepayrollStatusMeta.cancellableis not explicitlyfalseChanges Made
Core Logic
src/components/Payroll/helpers.tsgetPacificTimeOffset()- Handles DST transitionscanCancelPayroll()- Centralized cancellation eligibility logicUI Components
src/components/Payroll/PayrollHistory/PayrollHistoryPresentation.tsxcanCancelPayroll()helpersrc/components/Payroll/PayrollOverview/PayrollOverview.tsxcanCancelusing shared helpersrc/components/Payroll/PayrollOverview/PayrollOverviewPresentation.tsxcanCancelpropTests
src/components/Payroll/helpers.test.tssrc/components/Payroll/PayrollHistory/PayrollHistory.test.tsxsrc/test/mocks/fixtures/payroll-history-unprocessed-test-data.jsonprocessed: false→processed: trueto match requirementsTesting
Acceptance Criteria Met
Technical Notes
setUTCHours()for proper timezone handlingExample Behavior
Before 4 PM PT on deadline:
After 4 PM PT on deadline:
Unprocessed payroll: