From 652829ab90d54078d9da6788a3e7b777ef5f600f Mon Sep 17 00:00:00 2001 From: Joel Wedemire Date: Wed, 8 Apr 2026 18:31:51 -0700 Subject: [PATCH] fix: bootstrap blocker + 4 security bugs - Bootstrap (critical): settings/create/index no longer 403 on fresh install. Site admins (admin/super_admin) can access settings when 0 groups exist. First group creation seeds default priorities (Low/Medium/High/Urgent). Index shows friendly first-run splash. Create shows warning + settings link. - Internal notes leak (high): submitters can no longer receive is_internal messages via ticket show, index detail panel, or any Inertia prop. filterMessagesForRole() strips internal notes for non-agents. - Arbitrary assignee (med/high): update() now validates assigned_to against actual agent-access users for the ticket's group server-side. - Cross-group priority/project forgery (medium): store() and update() now verify priority_id and project_id belong to the ticket's own group (or are global for priorities). - Foreign message_id on attachment upload (medium): message_id is now validated to belong to the current ticket, not just any message row. --- resources/js/Pages/Ticketing/Create.vue | 10 ++- resources/js/Pages/Ticketing/Index.vue | 14 +++ resources/js/Pages/Ticketing/Settings.vue | 11 +++ .../TicketAttachmentController.php | 16 +++- src/Http/Controllers/TicketController.php | 90 +++++++++++++++++-- .../TicketingSettingsController.php | 89 ++++++++++++++++-- 6 files changed, 213 insertions(+), 17 deletions(-) diff --git a/resources/js/Pages/Ticketing/Create.vue b/resources/js/Pages/Ticketing/Create.vue index 11332a8..a9dcbdb 100644 --- a/resources/js/Pages/Ticketing/Create.vue +++ b/resources/js/Pages/Ticketing/Create.vue @@ -5,7 +5,14 @@

Submit a Ticket

-
+ +
+

📦 Ticketing isn’t set up yet

+

An admin needs to create at least one group before tickets can be submitted.

+ Go to Settings +
+ +
@@ -90,6 +97,7 @@ import { Link, useForm } from '@inertiajs/vue3' const props = defineProps({ groups: Array, priorities: Array, + isBootstrap: Boolean, }) const form = useForm({ diff --git a/resources/js/Pages/Ticketing/Index.vue b/resources/js/Pages/Ticketing/Index.vue index 410c355..1ac8123 100644 --- a/resources/js/Pages/Ticketing/Index.vue +++ b/resources/js/Pages/Ticketing/Index.vue @@ -1,5 +1,17 @@
@@ -425,6 +438,7 @@ const props = defineProps({ ticketDetail: Object, detailAgents: Array, viewCounts: Object, + isBootstrap: Boolean, }) const search = ref('') diff --git a/resources/js/Pages/Ticketing/Settings.vue b/resources/js/Pages/Ticketing/Settings.vue index 3571f1a..87df2a9 100644 --- a/resources/js/Pages/Ticketing/Settings.vue +++ b/resources/js/Pages/Ticketing/Settings.vue @@ -5,6 +5,15 @@

Ticketing Settings

+ +
+

🚀 First-Run Setup

+

+ No groups exist yet. Create your first group below to get started. + Default priorities (Low, Medium, High, Urgent) will be seeded automatically. +

+
+
{{ $page.props.flash.success }} @@ -239,6 +248,8 @@ const props = defineProps({ agents: Array, priorities: Array, myGroupIds: Array, + isBootstrap: Boolean, + isSiteAdmin: Boolean, }) const activeTab = ref('groups') diff --git a/src/Http/Controllers/TicketAttachmentController.php b/src/Http/Controllers/TicketAttachmentController.php index bcc002a..2a6fc82 100644 --- a/src/Http/Controllers/TicketAttachmentController.php +++ b/src/Http/Controllers/TicketAttachmentController.php @@ -31,7 +31,21 @@ class TicketAttachmentController extends Controller $request->validate([ 'file' => 'required|file|max:' . (self::MAX_SIZE_MB * 1024), - 'message_id' => 'nullable|exists:ticket_messages,id', + // Bug #5: message_id must belong to THIS ticket, not any arbitrary message + 'message_id' => [ + 'nullable', + 'integer', + function ($attribute, $value, $fail) use ($ticket) { + if ($value !== null) { + $exists = \Dashboard\Ticketing\Models\TicketMessage::where('id', $value) + ->where('ticket_id', $ticket->id) + ->exists(); + if (!$exists) { + $fail('The message does not belong to this ticket.'); + } + } + }, + ], ]); $file = $request->file('file'); diff --git a/src/Http/Controllers/TicketController.php b/src/Http/Controllers/TicketController.php index 6f585dd..83754e8 100644 --- a/src/Http/Controllers/TicketController.php +++ b/src/Http/Controllers/TicketController.php @@ -40,12 +40,40 @@ class TicketController extends Controller ->exists(); } + /** + * Filter messages for submitters — strip internal notes. + */ + private function filterMessagesForRole($messages, bool $isAgent) + { + if ($isAgent) { + return $messages; + } + return $messages->filter(fn($m) => !$m->is_internal)->values(); + } + public function index(Request $request): Response { $user = Auth::user(); $agentGroupIds = $this->agentGroupIds(); $isAgent = count($agentGroupIds) > 0; + // If no groups exist at all, render a first-run / bootstrap state + $totalGroups = TicketingGroup::count(); + if ($totalGroups === 0) { + return Inertia::render('Ticketing/Index', [ + 'tickets' => null, + 'groups' => [], + 'priorities' => [], + 'projects' => [], + 'isAgent' => $isAgent, + 'filters' => [], + 'ticketDetail' => null, + 'detailAgents' => [], + 'viewCounts' => ['all' => 0, 'mine' => 0, 'unassigned' => 0, 'pending' => 0, 'resolved' => 0], + 'isBootstrap' => true, + ]); + } + $baseQuery = Ticket::query(); if ($isAgent) { @@ -169,9 +197,14 @@ class TicketController extends Controller $dt->submitter = $detailUsers[$dt->submitter_id] ?? null; $dt->assignee = $dt->assigned_to ? ($detailUsers[$dt->assigned_to] ?? null) : null; - $dt->messages->each(function ($msg) use ($detailUsers) { + + // Bug #2: Filter internal notes for non-agents + $visibleMessages = $this->filterMessagesForRole($dt->messages, $isAgent); + $visibleMessages->each(function ($msg) use ($detailUsers) { $msg->author = $msg->user_id ? ($detailUsers[$msg->user_id] ?? null) : null; }); + $dt->setRelation('messages', $visibleMessages); + $ticketDetail = $dt; if ($isAgent) { @@ -191,6 +224,7 @@ class TicketController extends Controller 'ticketDetail' => $ticketDetail, 'detailAgents' => $detailAgents, 'viewCounts' => $viewCounts, + 'isBootstrap' => false, ]); } @@ -201,11 +235,22 @@ class TicketController extends Controller $isAgent = count($agentGroupIds) > 0; $groups = TicketingGroup::when($isAgent, fn($q) => $q->whereIn('id', $agentGroupIds))->get(); + + // No groups at all? Show first-run state. + if ($groups->isEmpty() && TicketingGroup::count() === 0) { + return Inertia::render('Ticketing/Create', [ + 'groups' => [], + 'priorities' => [], + 'isBootstrap' => true, + ]); + } + $priorities = PriorityLevel::orderBy('sort_order')->get(); return Inertia::render('Ticketing/Create', [ 'groups' => $groups, 'priorities' => $priorities, + 'isBootstrap' => false, ]); } @@ -219,6 +264,14 @@ class TicketController extends Controller 'due_date' => 'nullable|date', ]); + // Bug #4: Ensure priority belongs to the chosen group (or is global) + if (!empty($validated['priority_id'])) { + $priority = PriorityLevel::find($validated['priority_id']); + if ($priority && $priority->group_id !== null && $priority->group_id != $validated['group_id']) { + abort(422, 'Priority does not belong to the selected group.'); + } + } + $group = TicketingGroup::findOrFail($validated['group_id']); $number = $group->nextTicketNumber(); @@ -244,13 +297,18 @@ class TicketController extends Controller $ticket->load(['group', 'priority', 'project', 'messages', 'attachments']); - $userIds = $ticket->messages->pluck('user_id')->filter()->unique(); + // Bug #2: Filter internal notes for submitters + $visibleMessages = $this->filterMessagesForRole($ticket->messages, $isAgent); + + $userIds = $visibleMessages->pluck('user_id')->filter()->unique(); $users = DB::table('users')->whereIn('id', $userIds)->get(['id', 'name', 'email'])->keyBy('id'); - $ticket->messages->each(function ($msg) use ($users) { + $visibleMessages->each(function ($msg) use ($users) { $msg->author = $msg->user_id ? ($users[$msg->user_id] ?? null) : null; }); + $ticket->setRelation('messages', $visibleMessages); + $agents = []; if ($isAgent) { $agentIds = TicketingAgentAccess::where('group_id', $ticket->group_id)->pluck('user_id'); @@ -296,6 +354,11 @@ class TicketController extends Controller abort(403); } + // Bug #3: Valid assignees are users with agent access to this group + $validAssigneeIds = TicketingAgentAccess::where('group_id', $ticket->group_id) + ->pluck('user_id') + ->toArray(); + $rules = [ 'title' => 'sometimes|required|string|max:255', 'description' => 'sometimes|required|string', @@ -303,13 +366,28 @@ class TicketController extends Controller 'priority_id' => 'nullable|exists:ticketing_priority_levels,id', 'due_date' => 'nullable|date', 'project_id' => 'nullable|exists:ticketing_projects,id', + 'assigned_to' => 'nullable|in:' . implode(',', $validAssigneeIds ?: [0]), ]; - if ($this->isAgent($ticket->group_id)) { - $rules['assigned_to'] = 'nullable|exists:users,id'; + $validated = $request->validate($rules); + + // Bug #4: priority_id must belong to this group or be global + if (!empty($validated['priority_id'])) { + $priority = PriorityLevel::find($validated['priority_id']); + if ($priority && $priority->group_id !== null && $priority->group_id !== $ticket->group_id) { + abort(422, 'Priority does not belong to this ticket\'s group.'); + } } - $ticket->update($request->validate($rules)); + // Bug #4: project_id must belong to this group + if (!empty($validated['project_id'])) { + $project = TicketingProject::find($validated['project_id']); + if ($project && $project->group_id !== $ticket->group_id) { + abort(422, 'Project does not belong to this ticket\'s group.'); + } + } + + $ticket->update($validated); return back()->with('success', 'Ticket updated.'); } diff --git a/src/Http/Controllers/TicketingSettingsController.php b/src/Http/Controllers/TicketingSettingsController.php index 010411e..a2ffe57 100644 --- a/src/Http/Controllers/TicketingSettingsController.php +++ b/src/Http/Controllers/TicketingSettingsController.php @@ -13,11 +13,38 @@ use Inertia\Response; class TicketingSettingsController extends Controller { + /** + * True if there are zero groups in the system (first-run bootstrap state). + */ + private function isBootstrapState(): bool + { + return TicketingGroup::count() === 0; + } + + /** + * True if the current user is a site-level admin or super_admin. + */ + private function isSiteAdmin(): bool + { + $role = Auth::user()?->role; + return in_array($role, ['admin', 'super_admin']); + } + + /** + * Require either: + * - existing agent access, OR + * - bootstrap state + site admin (so first admin can seed the system) + */ private function requireAgentAccess(): void { $hasAccess = TicketingAgentAccess::where('user_id', Auth::id())->exists(); + if (!$hasAccess) { - abort(403); + // Allow site admins through during bootstrap (no groups yet) + if ($this->isBootstrapState() && $this->isSiteAdmin()) { + return; + } + abort(403, 'You need agent access to manage ticketing settings.'); } } @@ -37,29 +64,49 @@ class TicketingSettingsController extends Controller $this->requireAgentAccess(); $userId = Auth::id(); + $isBootstrap = $this->isBootstrapState(); $myGroupIds = TicketingAgentAccess::where('user_id', $userId)->pluck('group_id'); - $groups = TicketingGroup::whereIn('id', $myGroupIds)->get(); + $groups = $isBootstrap + ? collect() + : TicketingGroup::whereIn('id', $myGroupIds)->get(); - $agents = TicketingAgentAccess::whereIn('group_id', $myGroupIds)->get(); - $agentUserIds = $agents->pluck('user_id')->unique(); - $agentUsers = \DB::table('users')->whereIn('id', $agentUserIds)->get(['id', 'name', 'email'])->keyBy('id'); - $agents->each(fn($a) => $a->user = $agentUsers[$a->user_id] ?? null); + $agents = $isBootstrap + ? collect() + : TicketingAgentAccess::whereIn('group_id', $myGroupIds)->get(); - $priorities = PriorityLevel::where(fn($q) => $q->whereNull('group_id')->orWhereIn('group_id', $myGroupIds)) - ->orderBy('sort_order')->get(); + if ($agents->isNotEmpty()) { + $agentUserIds = $agents->pluck('user_id')->unique(); + $agentUsers = \DB::table('users')->whereIn('id', $agentUserIds)->get(['id', 'name', 'email'])->keyBy('id'); + $agents->each(fn($a) => $a->user = $agentUsers[$a->user_id] ?? null); + } + + $priorities = $isBootstrap + ? collect() + : PriorityLevel::where(fn($q) => $q->whereNull('group_id')->orWhereIn('group_id', $myGroupIds)) + ->orderBy('sort_order')->get(); return Inertia::render('Ticketing/Settings', [ 'groups' => $groups, 'agents' => $agents, 'priorities' => $priorities, 'myGroupIds' => $myGroupIds, + 'isBootstrap' => $isBootstrap, + 'isSiteAdmin' => $this->isSiteAdmin(), ]); } public function storeGroup(Request $request) { - $this->requireAgentAccess(); + // During bootstrap, any site admin can create the first group. + // After bootstrap, require existing agent access. + if ($this->isBootstrapState()) { + if (!$this->isSiteAdmin()) { + abort(403, 'Only site admins can create the first group.'); + } + } else { + $this->requireAgentAccess(); + } $validated = $request->validate([ 'name' => 'required|string|max:100', @@ -77,6 +124,25 @@ class TicketingSettingsController extends Controller 'role' => 'manager', ]); + // Seed default priorities for this group if none exist globally + if (PriorityLevel::count() === 0) { + $defaults = [ + ['name' => 'Low', 'color' => '#6b7280', 'sort_order' => 1], + ['name' => 'Medium', 'color' => '#3b82f6', 'sort_order' => 2], + ['name' => 'High', 'color' => '#f59e0b', 'sort_order' => 3], + ['name' => 'Urgent', 'color' => '#ef4444', 'sort_order' => 4], + ]; + foreach ($defaults as $d) { + PriorityLevel::create([ + 'group_id' => $group->id, + 'name' => $d['name'], + 'color' => $d['color'], + 'description' => null, + 'sort_order' => $d['sort_order'], + ]); + } + } + return back()->with('success', 'Group created.'); } @@ -137,6 +203,11 @@ class TicketingSettingsController extends Controller 'group_id' => 'nullable|exists:ticketing_groups,id', ]); + // If group_id given, caller must be manager of that group + if (!empty($validated['group_id'])) { + $this->requireManagerAccess($validated['group_id']); + } + PriorityLevel::create($validated); return back()->with('success', 'Priority level created.');