Company Admin Authorization Audit

📄 General
← Back to Documentation
# Company Admin Authorization Audit **Audit Date:** 2026-08-27 **Scope:** Non-Filament company-admin panel (`CompanyAdminController` and dedicated controllers) ## Executive Summary - **CompanyAdminController**: 94 `withoutGlobalScopes()` calls, 65 commented 403 checks - **Dedicated Controllers**: 0 `withoutGlobalScopes()` calls, 0 commented 403 checks (CLEAN) - **Critical Risk**: Single-resource lookup methods allow cross-tenant access via ID guessing - **Safe Patterns**: Dashboard/statistics queries use explicit `->where('company_id', $company->id)` after bypass ## Tenant Column Mapping | Model | Tenant Columns | Has Global Scopes | |-------|---------------|-------------------| | Venue | company_id, workspace_id | ✅ CompanyScope, WorkspaceScope | | Booking | company_id, workspace_id | ✅ CompanyScope, WorkspaceScope | | Invoice | company_id, workspace_id | ✅ CompanyScope, WorkspaceScope | | Client | company_id, workspace_id | ✅ CompanyScope, WorkspaceScope | | Stay | company_id, workspace_id | ✅ CompanyScope, WorkspaceScope | | VenueObject | company_id, workspace_id | ✅ CompanyScope, WorkspaceScope | | Facility | company_id | ❌ NO global scopes | | Product | company_id, workspace_id | ✅ CompanyScope, WorkspaceScope | | Payment | company_id, workspace_id | (assumed) | | Guest | (via stay relation) | (via stay) | | User | company_id | ✅ CompanyScope, WorkspaceScope | ## Dangerous Patterns: Single-Resource Lookups ### Booking Methods (14 instances) **Pattern:** `Booking::withoutGlobalScopes()->find($bookingId)` + commented 403 | Method | Line | Risk Level | Action | |--------|------|------------|--------| | showBooking | 1266 | CRITICAL | Read cross-tenant booking data | | editBooking | 1314 | CRITICAL | Edit cross-tenant booking | | updateBooking | 1365 | CRITICAL | Modify cross-tenant booking | | cancelBooking | 1553 | CRITICAL | Cancel cross-tenant booking | | checkInBooking | 1586 | CRITICAL | Check-in cross-tenant booking | | requestHousekeeperApproval | 1720 | HIGH | Approve cross-tenant booking | | sendHousekeeperReminder | 1778 | MEDIUM | Send notification for cross-tenant | | checkoutWithoutApproval | 1821 | CRITICAL | Checkout cross-tenant booking | | checkOutBooking | 1875 | CRITICAL | Checkout cross-tenant booking | | generateQRCode | 1987 | MEDIUM | Access cross-tenant QR code | | shareQRCode | 2020 | MEDIUM | Share cross-tenant QR code | | sendSMS | 2050 | MEDIUM | SMS cross-tenant guest | | sendEmail | 2085 | MEDIUM | Email cross-tenant guest | | sendPaymentLink | 2129 | HIGH | Send payment link for cross-tenant | ### Invoice Methods (3 instances) | Method | Line | Risk Level | Action | |--------|------|------------|--------| | showInvoice | 2339 | CRITICAL | Read cross-tenant invoice | | editInvoice | 2371 | CRITICAL | Edit cross-tenant invoice | | updateInvoice | 2447 | CRITICAL | Modify cross-tenant invoice | ### Client Methods (4 instances) | Method | Line | Risk Level | Action | |--------|------|------------|--------| | showClient | 2630 | HIGH | Read cross-tenant client data | | editClient | 2658 | HIGH | Edit cross-tenant client | | updateClient | 2684 | HIGH | Modify cross-tenant client | | deleteClient | 2724 | HIGH | Delete cross-tenant client | ### Venue Methods (4 instances) | Method | Line | Risk Level | Action | |--------|------|------------|--------| | showVenue | 211 | HIGH | Read cross-tenant venue | | editVenue | 406 | HIGH | Edit cross-tenant venue | | updateVenue | 440 | HIGH | Modify cross-tenant venue | | deleteVenue | 583 | HIGH | Delete cross-tenant venue | ### Facility Methods (4 instances) **Additional Risk:** Facility model has NO global scopes | Method | Line | Risk Level | Action | |--------|------|------------|--------| | showFacility | 4670 | HIGH | Read cross-tenant facility | | editFacility | 4696 | HIGH | Edit cross-tenant facility | | updateFacility | 4722 | HIGH | Modify cross-tenant facility | | deleteFacility | 4762 | HIGH | Delete cross-tenant facility | ### VenueObject Methods (4 instances) | Method | Line | Risk Level | Action | |--------|------|------------|--------| | showVenueObject | 5080 | HIGH | Read cross-tenant room | | editVenueObject | 5145 | HIGH | Edit cross-tenant room | | updateVenueObject | 5177 | HIGH | Modify cross-tenant room | | deleteVenueObject | 5241 | HIGH | Delete cross-tenant room | ### Product Methods (4 instances) | Method | Line | Risk Level | Action | |--------|------|------------|--------| | showProduct | 5814 | HIGH | Read cross-tenant product | | editProduct | 5839 | HIGH | Edit cross-tenant product | | updateProduct | 5869 | HIGH | Modify cross-tenant product | | deleteProduct | 5919 | HIGH | Delete cross-tenant product | ## Safe Patterns: Explicit Company Filtering ### Dashboard/Statistics (Lines 867-949) All queries use `->where('company_id', $company->id)` after `withoutGlobalScopes()` - **Safe**: Statistics, recent bookings, check-ins/outs, cleaning status, outstanding payments ### Index/List Queries (Lines 648, 1014, 1120-1143, 2198, 3162, 3793, 4002, 4603, 4983, 5111, 5297, 5520, 5736) All list queries use explicit company filtering - **Safe**: Booking index, Invoice index, Stay index, Payment index, Guest index, Facility index, VenueObject index, Service index, Package index, Product index ### Dropdown/Filter Data (Lines 1120-1143, 1333-1334) Dropdown data uses explicit company filtering - **Safe**: Venue, Client, VenueObject, Season, Staff, Location, Place dropdowns ## Authorization Approach Proposal ### Option 1: Explicit Filter Pattern (Recommended) Replace all `withoutGlobalScopes()->find($id)` with: ```php $resource = Resource::where('id', $id) ->where('company_id', $company->id) ->first(); if (!$resource) { abort(404); } ``` **Pros:** - Simple, explicit, easy to understand - No dependency on global scopes - Returns 404 (resource not found) instead of 403 (forbidden) - safer for ID guessing - Consistent with existing safe patterns **Cons:** - More verbose - Need to handle workspace filtering separately if needed ### Option 2: Model Binding with Policy Use Laravel route model binding with explicit policy checks: ```php // Route Route::get('/bookings/{booking}', ...) // Policy public function view(User $user, Booking $booking) { return $booking->company_id === $user->company_id; } ``` **Pros:** - Laravel standard approach - Centralized authorization logic - Clean controller code **Cons:** - Requires creating/updating policies for all resources - Need to ensure global scopes are not bypassed in binding - More complex setup ### Option 3: Helper Method (Hybrid) Create a reusable authorization helper: ```php protected function findResourceOr404($model, $id, $company) { return $model::where('id', $id) ->where('company_id', $company->id) ->first() ?? abort(404); } ``` **Pros:** - DRY principle - Easy to audit - Can add workspace filtering if needed **Cons:** - Additional abstraction layer - Still need to update all call sites ## Recommended Implementation Plan ### Phase 1: Create Authorization Helper 1. Create `authorizeResourceAccess()` helper in CompanyAdminController 2. Add unit tests for the helper 3. Document the pattern ### Phase 2: Fix Critical Booking Methods (14) 1. Update all booking single-resource methods to use helper 2. Add feature tests for cross-tenant blocking 3. Test existing booking workflows ### Phase 3: Fix High-Risk Methods (15) 1. Invoice (3), Client (4), Venue (4), Facility (4) 2. Add global scope to Facility model (missing) 3. Add feature tests for each resource ### Phase 4: Fix Remaining Methods (8) 1. VenueObject (4), Product (4) 2. Add feature tests 3. Full regression test ### Phase 5: Remove Safe withoutGlobalScopes() (Optional) 1. Remove `withoutGlobalScopes()` from dashboard/list queries where not needed 2. Rely on global scopes for standard filtering 3. Keep explicit filtering where queries are complex ## Remaining Risks After Implementation 1. **Workspace Isolation**: Current focus is on company_id; workspace_id filtering may need separate review 2. **Super-admin Bypass**: Need to define which actions should allow admin bypass 3. **Session Switching**: Code allows switching to booking's company (line 1285) - review if this is intended 4. **Facility Model**: Missing global scopes - needs model-level fix 5. **Bulk Operations**: bulkDeleteBookings and other bulk operations need review 6. **File Uploads**: Document/image uploads may need authorization checks 7. **Export Functions**: PDF/CSV exports may bypass company filtering ## Test Coverage Gaps Current tests only cover: - Stay methods (5 tests) - Route ordering (1 test) - B2B invoice routes (1 test) - VenueObject availability (2 tests) Missing tests for: - Booking cross-tenant access (14 methods) - Invoice cross-tenant access (3 methods) - Client cross-tenant access (4 methods) - Venue cross-tenant access (4 methods) - Facility cross-tenant access (4 methods) - VenueObject cross-tenant access (4 methods) - Product cross-tenant access (4 methods) - Dashboard statistics isolation - Bulk operations isolation