diff --git a/src/Http/Controllers/TicketingSettingsController.php b/src/Http/Controllers/TicketingSettingsController.php index cd5720f..87dbd87 100644 --- a/src/Http/Controllers/TicketingSettingsController.php +++ b/src/Http/Controllers/TicketingSettingsController.php @@ -54,6 +54,15 @@ class TicketingSettingsController extends Controller } } + /** + * Require manager rights over one group. + * + * Deliberately NOT widened to site admins: GroupRetirementTest pins that a + * super admin with no agent access is refused here, and #4710 is a + * visibility bug, not a licence to hand every `ticketing.settings` holder + * manager rights over every group. Seeing a group you do not manage is the + * fix; acting on it is a separate decision for a human to make. + */ private function requireManagerAccess(int $groupId): void { $isManager = TicketingAgentAccess::where('user_id', Auth::id()) @@ -71,14 +80,38 @@ class TicketingSettingsController extends Controller $userId = Auth::id(); $isBootstrap = $this->isBootstrapState(); + $isSiteAdmin = $this->isSiteAdmin(); $myGroupIds = TicketingAgentAccess::where('user_id', $userId)->pluck('group_id'); + // The groups this user may actually ACT on — requireManagerAccess() in + // prop form. A site admin now sees groups they do not manage (#4710), + // so the page has to say which rows its buttons will work on instead + // of offering an Edit that 403s. + $myManagedGroupIds = TicketingAgentAccess::where('user_id', $userId) + ->where('role', 'manager') + ->pluck('group_id'); + + // What this page is allowed to SHOW — separate from what it lets you + // DO, which stays requireManagerAccess() per group. A site admin sees + // every group: scoping the list to agent-access rows left an admin + // staring at "No groups yet. Create your first group above." with + // twelve groups in the database, inviting a duplicate (#4710). + // `null` means "no restriction". + // + // Every one of these routes sits behind `permission:ticketing.settings` + // and isSiteAdmin() is `is_super_admin || can('ticketing.settings')`, + // so today this is true for anyone who reaches the page at all. The + // scoped branch is kept because it is the correct behaviour if that + // route gate is ever loosened — it is not load-bearing right now. + $visibleGroupIds = $isSiteAdmin ? null : $myGroupIds; + // Archived groups stay in this list — this is the one place they must // remain visible, so a manager can see what happened and restore it. // Actives first, then archived, each alphabetical. $groups = $isBootstrap ? collect() - : TicketingGroup::whereIn('id', $myGroupIds) + : TicketingGroup::when($visibleGroupIds !== null, + fn($q) => $q->whereIn('id', $visibleGroupIds)) // tickets_count decides archive-vs-delete, so the UI can warn // the operator which one their click is about to do. ->withCount('tickets') @@ -88,7 +121,10 @@ class TicketingSettingsController extends Controller $agents = $isBootstrap ? collect() - : TicketingAgentAccess::whereIn('group_id', $myGroupIds)->with('group')->get(); + : TicketingAgentAccess::when($visibleGroupIds !== null, + fn($q) => $q->whereIn('group_id', $visibleGroupIds)) + ->with('group') + ->get(); if ($agents->isNotEmpty()) { $agentUserIds = $agents->pluck('user_id')->unique(); @@ -102,13 +138,16 @@ class TicketingSettingsController extends Controller $projects = $isBootstrap ? collect() - : TicketingProject::whereIn('group_id', $myGroupIds) + : TicketingProject::when($visibleGroupIds !== null, + fn($q) => $q->whereIn('group_id', $visibleGroupIds)) ->orderBy('name') ->get(); $emailConnections = $isBootstrap ? collect() - : EmailConnection::whereIn('group_id', $myGroupIds)->get(); + : EmailConnection::when($visibleGroupIds !== null, + fn($q) => $q->whereIn('group_id', $visibleGroupIds)) + ->get(); return Inertia::render('Ticketing/Settings', [ 'activeTab' => $tab, @@ -119,8 +158,9 @@ class TicketingSettingsController extends Controller 'ticketStatuses' => TicketStatus::orderBy('sort_order')->get(), 'emailConnections' => $emailConnections, 'myGroupIds' => $myGroupIds, + 'myManagedGroupIds' => $myManagedGroupIds, 'isBootstrap' => $isBootstrap, - 'isSiteAdmin' => $this->isSiteAdmin(), + 'isSiteAdmin' => $isSiteAdmin, 'autoCloseDays' => (int) Setting::get('ticketing.auto_close_days', 0), // #938 — the deploy-time repair, and how many rows it would touch. The // count is what turns an abstract toggle into a decision somebody can