* Fix IDOR vulnerability in password change (GHSA-mcc2-8rp2-q6ch)
The previous authorization check using can_modify_employee() was too
permissive - it allowed non-admin users to change other non-admin users'
passwords. For password changes, users should only be able to change
their own password. Only admins should be able to change any user's
password.
This fix replaces the can_modify_employee() check with a stricter
authorization that only allows:
- Users to change their own password
- Admins to change any user's password
Affected endpoints:
- GET /home/changePassword/{employee_id}
- POST /home/save/{employee_id}
Added tests to verify non-admin users cannot access or change other
non-admin users' passwords.
* Address PR review feedback
- Replace header/exit redirect with proper 403 response in getChangePassword
- Refactor createNonAdminEmployee helper to accept overrides array
- Simplify tests by reusing the helper
- Update tests to expect 403 response instead of redirect
---------
Co-authored-by: Ollama <ollama@steganos.dev>
* fix(security): add row-level authorization to password change endpoints
- Prevents non-admin users from viewing other users' password forms
- Prevents non-admin users from changing other users' passwords
- Uses can_modify_employee() check consistent with Employees controller fix
- Addresses BOLA vulnerability in Home controller (GHSA-q58g-gg7v-f9rf)
* test(security): add BOLA authorization tests for Home controller
- Test non-admin cannot view/change admin password
- Test user can view/change own password
- Test admin can view/change any password
- Test default employee_id uses current user
- Add JUnit test result upload to CI workflow
* refactor: apply PSR-12 naming and add DEFAULT_EMPLOYEE_ID constant
- Add DEFAULT_EMPLOYEE_ID constant to Constants.php
- Rename variables to follow PSR-12 camelCase convention
- Use ternary for default employee ID assignment
* refactor: use NEW_ENTRY constant instead of adding DEFAULT_EMPLOYEE_ID
Reuse existing NEW_ENTRY constant for default employee ID parameter.
Avoids adding redundant constants to Constants.php with same value (-1).
---------
Co-authored-by: jekkos <jeroen@steganos.dev>
- Add validateCSVStockLocations() method to check CSV columns against allowed locations
- Log error when invalid stock location columns are detected
- Tests for valid, invalid, and mixed stock location columns
- Tests for location name case sensitivity
- Tests for CSV parsing and detecting location columns
- Add error message language string for invalid locations
Co-authored-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
- Non-admin employees can no longer view/modify admin accounts
- Non-admin employees can no longer delete admin accounts
- Non-admin employees can only grant permissions they themselves have
- Added is_admin() and can_modify_employee() methods to Employee model
- Prevents privilege escalation via permission grants
Add tests for BOLA fix and permission delegation
- EmployeeTest: Unit tests for is_admin() and can_modify_employee() methods
- EmployeesControllerTest: Test cases for authorization checks (integration tests require DB)
- ReportsControllerTest: Test validating the constructor redirect fix pattern
Fix return type error in Employees controller
Use $this->response->setJSON() instead of echo json_encode() + return
to properly satisfy the ResponseInterface return type.
Complete Content-Type application/json fix for all AJAX responses
- Add missing return statements to all ->response->setJSON() calls
- Fix Items.php method calls from JSON() to setJSON()
- Convert echo statements to proper JSON responses
- Ensure consistent Content-Type headers across all controllers
- Fix 46+ instances across 12 controller files
- Change Config.php methods to : ResponseInterface (all return setJSON only):
- postSaveRewards(), postSaveBarcode(), postSaveReceipt()
- postSaveInvoice(), postRemoveLogo()
- Update PHPDoc @return tags
- Change Receivings.php _reload() to : string (only returns view)
- Change Receivings.php methods to : string (all return _reload()):
- getIndex(), postSelectSupplier(), postChangeMode(), postAdd()
- postEditItem(), getDeleteItem(), getRemoveSupplier()
- postComplete(), postRequisitionComplete(), getReceipt(), postCancelReceiving()
- Change postSave() to : ResponseInterface (returns setJSON)
- Update all PHPDoc @return tags
Fix XSS vulnerabilities in sales templates, login, and config pages
This commit addresses 5 XSS vulnerabilities by adding proper escaping
to all user-controlled configuration values in HTML contexts.
Fixed Files:
- app/Views/sales/invoice.php: Escaped company_logo (URL context) and company (HTML)
- app/Views/sales/work_order.php: Escaped company_logo (URL context)
- app/Views/sales/receipt_email.php: Added file path validation and escaping for logo
- app/Views/login.php: Escaped all config values in title, logo src, and alt
- app/Views/configs/info_config.php: Escaped company_logo (URL context)
Security Impact:
- Prevents stored XSS attacks if configuration is compromised
- Defense-in-depth principle applied to administrative interfaces
- Follows OWASP best practices for output encoding
Testing:
- Verified no script execution with XSS payloads in config values
- Confirmed proper escaping in HTML, URL, and file contexts
- All templates render correctly with valid configuration
Severity: High (4 files), Medium-High (1 file)
CVSS Score: ~6.1
CWE: CWE-79 (Improper Neutralization of Input During Web Page Generation)
Fix critical password validation bypass and add unit tests
This commit addresses a critical security vulnerability where the password
minimum length check was performed on the HASHED password (always 60
characters for bcrypt) instead of the actual password before hashing.
Vulnerability Details:
- Original code: strlen($employee_data['password']) >= 8
- This compared the hash length (always 60) instead of raw password
- Impact: Users could set 1-character passwords like "a"
- Severity: Critical (enables brute force attacks on weak passwords)
- CVE-like issue: CWE-307 (Improper Restriction of Excessive Authentication Attempts)
Fix Applied:
- Validate password length BEFORE hashing
- Clear error message when password is too short
- Added unit tests to verify minimum length enforcement
- Regression test to prevent future vulnerability re-introduction
Test Coverage:
- testPasswordMinLength_Rejects7Characters: Verify 7 chars rejected
- testPasswordMinLength_Accepts8Characters: Verify 8 chars accepted
- testPasswordMinLength_RejectsEmptyString: Verify empty rejected
- testPasswordMinLength_RejectsWhitespaceOnly: Verify whitespace rejected
- testPasswordMinLength_AcceptsSpecialCharacters: Verify special chars OK
- testPasswordMinLength_RejectsPreviousBehavior: Regression test for bug
Files Modified:
- app/Controllers/Home.php: Fixed password validation logic
- tests/Controllers/HomeTest.php: Added comprehensive unit tests
Security Impact:
- Enforces 8-character minimum password policy
- Prevents extremely weak passwords that facilitate brute-force attacks
- Critical for credential security and user account protection
Breaking Changes:
- Users with passwords < 8 characters will need to reset their password
- This is the intended security improvement
Severity: Critical
CVSS Score: ~7.5
CWE: CWE-305 (Authentication Bypass by Primary Weakness), CWE-307
Add GitHub Actions workflow to run PHPUnit tests
Move business logic from views to controllers for better separation of concerns
- Move logo URL computation from info_config view to Config::getIndex()
- Move image base64 encoding from receipt_email view to Sales controller
- Improves separation of concerns by keeping business logic in controllers
- Simplifies view templates to only handle presentation
Fix XSS vulnerabilities in report views - escape user-controllable summary data and labels
Fix base64 encoding URL issue in delete payment - properly URL encode base64 string
Fix remaining return type declarations for Sales controller
Fixed additional methods that call _reload():
- postAdd() - returns _reload($data)
- postAddPayment() - returns _reload($data)
- postEditItem() - returns _reload($data)
- postSuspend() - returns _reload($data)
- postSetPaymentType() - returns _reload()
All methods now return ResponseInterface|string to match _reload() signature.
This resolves PHP TypeError errors.
