Authorization Implementation Plan

📄 General
← Back to Documentation
# Authorization Implementation Plan **Status:** Ready for implementation **Last Updated:** 2026-08-27 **Baseline:** AUTHORIZATION_AUDIT.md ## Selected Approach: Explicit Filter Pattern with Helper **Decision:** Use `CompanyAuthorizationHelper` with explicit `where('company_id', $company->id)` filtering. **Rationale:** - Returns 404 for cross-tenant access (safer than 403 for ID guessing) - No dependency on global scopes being bypassed - Consistent with existing safe patterns in the codebase - Easy to audit and verify - Can be extended for workspace filtering if needed ## Implementation Phases ### Phase 0: Prerequisites ✅ COMPLETED - [x] Audit all `withoutGlobalScopes()` usage - [x] Map tenant columns for all resources - [x] Create `CompanyAuthorizationHelper` - [x] Add unit tests for helper - [x] Document current insecure behavior in tests ### Phase 1: Fix Critical Booking Methods (14 methods) **Priority:** CRITICAL - Booking lifecycle operations **Estimated Time:** 2-3 hours **Methods to fix:** 1. `showBooking($bookingId)` - Line 1266 2. `editBooking($bookingId)` - Line 1314 3. `updateBooking($request, $bookingId)` - Line 1365 4. `cancelBooking($bookingId)` - Line 1553 5. `checkInBooking($request, $bookingId)` - Line 1586 6. `requestHousekeeperApproval($request, $bookingId)` - Line 1720 7. `sendHousekeeperReminder($request, $bookingId)` - Line 1778 8. `checkoutWithoutApproval($request, $bookingId)` - Line 1821 9. `checkOutBooking($request, $bookingId)` - Line 1875 10. `generateQRCode($bookingId)` - Line 1987 11. `shareQRCode($bookingId)` - Line 2020 12. `sendSMS($request, $bookingId)` - Line 2050 13. `sendEmail($request, $bookingId)` - Line 2085 14. `sendPaymentLink($request, $bookingId)` - Line 2129 **Pattern to apply:** ```php // OLD (INSECURE): $booking = Booking::withoutGlobalScopes()->find($bookingId); if (!$booking) { abort(404); } // if (!$user->isAdmin() && $booking->company_id != $user->company_id) { // abort(403); // } // NEW (SECURE): $booking = CompanyAuthorizationHelper::findResourceOr404( Booking::class, $bookingId, $company ); ``` **Tests to add:** - Cross-tenant booking show returns 404 - Cross-tenant booking edit returns 404 - Cross-tenant booking update returns 404 - Cross-tenant booking cancel returns 404 - Cross-tenant booking check-in returns 404 - Cross-tenant booking checkout returns 404 ### Phase 2: Fix High-Risk Resource Methods (15 methods) **Priority:** HIGH - Single-resource CRUD for invoices, clients, venues, facilities **Estimated Time:** 2 hours **Invoice Methods (3):** 1. `showInvoice($invoiceId)` - Line 2339 2. `editInvoice($invoiceId)` - Line 2371 3. `updateInvoice($request, $invoiceId)` - Line 2447 **Client Methods (4):** 1. `showClient($clientId)` - Line 2630 2. `editClient($clientId)` - Line 2658 3. `updateClient($request, $clientId)` - Line 2684 4. `deleteClient($clientId)` - Line 2724 **Venue Methods (4):** 1. `showVenue($venueId)` - Line 211 2. `editVenue($venueId)` - Line 406 3. `updateVenue($request, $venueId)` - Line 440 4. `deleteVenue($venueId)` - Line 583 **Facility Methods (4) + MODEL FIX:** 1. Add global scopes to `Facility` model (CRITICAL - model has no scopes) 2. `showFacility($facilityId)` - Line 4670 3. `editFacility($facilityId)` - Line 4696 4. `updateFacility($request, $facilityId)` - Line 4722 5. `deleteFacility($facilityId)` - Line 4762 **Model Fix Required:** ```php // app/Models/Facility.php - Add to booted(): protected static function booted() { static::addGlobalScope(new \App\Models\Scopes\CompanyScope()); // Workspace scope if applicable } ``` **Tests to add:** - Cross-tenant invoice access returns 404 - Cross-tenant client access returns 404 - Cross-tenant venue access returns 404 - Cross-tenant facility access returns 404 ### Phase 3: Fix Remaining Resource Methods (8 methods) **Priority:** MEDIUM - Venue objects and products **Estimated Time:** 1 hour **VenueObject Methods (4):** 1. `showVenueObject($venueObjectId)` - Line 5080 2. `editVenueObject($venueObjectId)` - Line 5145 3. `updateVenueObject($request, $venueObjectId)` - Line 5177 4. `deleteVenueObject($venueObjectId)` - Line 5241 **Product Methods (4):** 1. `showProduct($productId)` - Line 5814 2. `editProduct($productId)` - Line 5839 3. `updateProduct($request, $productId)` - Line 5869 4. `deleteProduct($productId)` - Line 5919 **Tests to add:** - Cross-tenant venue object access returns 404 - Cross-tenant product access returns 404 ### Phase 4: Review and Clean Up (Optional) **Priority:** LOW - Remove unnecessary `withoutGlobalScopes()` from safe queries **Estimated Time:** 1-2 hours **Review dashboard/list queries (lines 867-949, 1120-1143, etc.):** - Remove `withoutGlobalScopes()` where explicit filtering makes it redundant - Keep `withoutGlobalScopes()` only where needed for complex queries - Update tests to verify dashboard statistics are tenant-isolated ### Phase 5: Edge Cases and Bulk Operations **Priority:** MEDIUM - Review bulk operations and special cases **Estimated Time:** 1 hour **Items to review:** 1. `bulkDeleteBookings` - Line 4286 2. Session switching logic (line 1285) - verify if intentional 3. Export functions (PDF/CSV) - verify company filtering 4. File uploads - verify authorization checks 5. Any remaining commented 403 checks ## Testing Strategy ### Unit Tests - ✅ `CompanyAuthorizationHelperTest` - All helper methods tested ### Feature Tests - ✅ Stay methods (5 tests) - Already completed - ✅ Route ordering tests - Already completed - ✅ VenueObject availability tests - Already completed - ⏳ Booking cross-tenant tests (6 tests) - Phase 1 - ⏳ Invoice cross-tenant tests (3 tests) - Phase 2 - ⏳ Client cross-tenant tests (4 tests) - Phase 2 - ⏳ Venue cross-tenant tests (4 tests) - Phase 2 - ⏳ Facility cross-tenant tests (4 tests) - Phase 2 - ⏳ VenueObject cross-tenant tests (4 tests) - Phase 3 - ⏳ Product cross-tenant tests (4 tests) - Phase 3 ### Regression Tests - Run full test suite after each phase - Test booking lifecycle end-to-end - Test invoice generation and payment - Test check-in/check-out workflows ## Rollback Plan If issues arise after implementation: 1. Each phase can be reverted independently 2. Helper can be disabled by commenting out usage 3. Git revert per-phase commits 4. Feature flag can be added to toggle helper usage ## Success Criteria - [x] Authorization audit completed - [x] Helper created and tested - [ ] All 37 single-resource methods use helper - [ ] Facility model has global scopes - [] All cross-tenant access returns 404 - [] All existing tests pass - [] New security tests pass - [] No commented 403 checks remain - [] Documentation updated ## Remaining Risks After Implementation 1. **Workspace Isolation**: Current focus is `company_id`; `workspace_id` may need separate review 2. **Super-admin Bypass**: Define which actions should allow admin bypass 3. **Session Switching**: Code allows switching to booking's company (line 1285) - review if intended 4. **Bulk Operations**: Need to verify bulk delete operations are tenant-isolated 5. **File Uploads**: Document/image uploads may need authorization checks 6. **Export Functions**: PDF/CSV exports may bypass company filtering 7. **API Endpoints**: This audit covers web panel only; API endpoints need separate review ## Commands to Run Tests ```bash # Run unit tests for helper php artisan test --filter=CompanyAuthorizationHelperTest # Run security tests php artisan test --filter=CompanyAdminSecurityTest # Run specific test groups php artisan test --filter=test_show_stay_tenant_isolation php artisan test --filter=test_current_insecure_cross_tenant_booking_access # Run full test suite (requires PHP 8.3+) php artisan test ``` ## Notes - Local PHP version is 8.2.12; tests require PHP 8.3+ to run - Dedicated company-admin controllers (CheckOutFolioReviewController, PreCheckInController, RoomAssignmentController, StayModificationController) are CLEAN - no changes needed - All main models except Facility already have CompanyScope and WorkspaceScope - The helper returns 404 instead of 403 for security (ID guessing protection)