fix: update firewall rule handling to ensure proper API request formatting and improve error logging

extensions/Servers/Proxmox/Livewire/Firewall.php

@@ -346,11 +346,12 @@ class Firewall extends Component
             $headers['Pragma'] = 'no-cache';
         }
         
+        // Proxmox API requires form-encoded data for POST/PUT requests
         $request = Http::withHeaders($headers)->withoutVerifying();
 
         $response = match($method) {
-            'post' => $request->post($fullUrl, $data),
+            'post' => $request->asForm()->post($fullUrl, $data),
-            'put' => $request->put($fullUrl, $data),
+            'put' => $request->asForm()->put($fullUrl, $data),
             'delete' => $request->delete($fullUrl),
             default => $request->get($fullUrl, $data),
         };
@@ -596,51 +597,61 @@ class Firewall extends Component
         return $this->macroDefinitions[$macro] ?? null;
     }
     
+    // ==================== FIREWALL RULES ====================
+    
+    /**
+     * Load all firewall rules from Proxmox API
+     */
     protected function loadFirewallRules(): void
     {
         try {
-            $response = $this->firewallRequest(
+            $url = '/nodes/' . $this->server->node->name . '/qemu/' . $this->server->vm_id . '/firewall/rules';
-                '/nodes/' . $this->server->node->name . '/qemu/' . $this->server->vm_id . '/firewall/rules'
+            $response = $this->firewallRequest($url);
-            );
             
             if ($response->successful()) {
-                $this->rules = $response->json()['data'] ?? [];
+                $data = $response->json()['data'] ?? [];
+                // Sort rules by position
+                usort($data, fn($a, $b) => ($a['pos'] ?? 0) <=> ($b['pos'] ?? 0));
+                $this->rules = $data;
+                
+                Log::debug('Loaded firewall rules', ['count' => count($this->rules)]);
+            } else {
+                Log::error('Failed to load firewall rules', [
+                    'status' => $response->status(),
+                    'body' => $response->body()
+                ]);
             }
         } catch (\Exception $e) {
-            Log::error('Failed to load firewall rules: ' . $e->getMessage());
+            Log::error('Exception loading firewall rules: ' . $e->getMessage());
+            $this->rules = [];
         }
     }
 
+    /**
+     * Open the form to create a new rule
+     */
     public function startCreateRule(): void
     {
         $this->resetRuleForm();
-        
-        // Set default destination to first IP
-        if (!empty($this->availableIps)) {
-            $this->ruleDest = $this->availableIps[0]['ip'];
-        }
-        
-        // Set default interface to first adapter
-        if (!empty($this->networkAdapters)) {
-            $this->ruleIface = $this->networkAdapters[0]['name'];
-        }
-        
         $this->editingRuleIndex = null;
         $this->showRuleForm = true;
     }
 
+    /**
+     * Open the form to edit an existing rule
+     */
     public function startEditRule(int $index): void
     {
         if (!isset($this->rules[$index])) {
-            $this->error = 'Rule not found';
+            $this->error = 'Rule not found at index ' . $index;
             $this->showErrorModal = true;
             return;
         }
 
         $rule = $this->rules[$index];
         $this->editingRuleIndex = $index;
-        $this->showRuleForm = true;
+        
-
+        // Populate form with existing rule data
         $this->ruleAction = $rule['action'] ?? 'ACCEPT';
         $this->ruleType = $rule['type'] ?? 'in';
         $this->ruleSource = $rule['source'] ?? '';
@@ -649,30 +660,36 @@ class Firewall extends Component
         $this->ruleDport = $rule['dport'] ?? '';
         $this->ruleSport = $rule['sport'] ?? '';
         $this->ruleComment = $rule['comment'] ?? '';
-        $this->ruleEnable = !isset($rule['enable']) || $rule['enable'] != 0;
+        $this->ruleEnable = !isset($rule['enable']) || (int)$rule['enable'] !== 0;
         $this->ruleMacro = $rule['macro'] ?? '';
         $this->ruleIface = $rule['iface'] ?? '';
         $this->ruleLog = $rule['log'] ?? 'nolog';
+        
+        $this->showRuleForm = true;
     }
 
+    /**
+     * Create or update a firewall rule via Proxmox API
+     */
     public function saveRule(): void
     {
         $this->loading = true;
         
         try {
-            // Build the payload according to Proxmox API requirements
+            // Build the API endpoint
-            $payload = [
+            $baseUrl = '/nodes/' . $this->server->node->name . '/qemu/' . $this->server->vm_id . '/firewall/rules';
-                'action' => strtoupper($this->ruleAction), // Ensure uppercase (ACCEPT, REJECT, DROP)
-                'type' => $this->ruleType, // 'in' or 'out'
-                'enable' => $this->ruleEnable ? 1 : 0,
-            ];
-
-            // Add optional fields only if they have values
-            // Proxmox API is strict about empty values
-            if (!empty($this->ruleIface)) {
-                $payload['iface'] = trim($this->ruleIface);
-            }
             
+            // Build payload - Proxmox requires specific field names
+            $payload = [];
+            
+            // Required fields
+            $payload['action'] = strtoupper(trim($this->ruleAction));
+            $payload['type'] = strtolower(trim($this->ruleType));
+            
+            // Enable/disable (Proxmox uses 1/0)
+            $payload['enable'] = $this->ruleEnable ? '1' : '0';
+            
+            // Optional fields - only add if not empty
             if (!empty($this->ruleSource)) {
                 $payload['source'] = trim($this->ruleSource);
             }
@@ -681,7 +698,7 @@ class Firewall extends Component
                 $payload['dest'] = trim($this->ruleDest);
             }
             
-            // Protocol must be lowercase for Proxmox API
+            // Protocol must be lowercase
             if (!empty($this->ruleProto)) {
                 $payload['proto'] = strtolower(trim($this->ruleProto));
             }
@@ -698,113 +715,141 @@ class Firewall extends Component
                 $payload['comment'] = trim($this->ruleComment);
             }
             
+            if (!empty($this->ruleIface)) {
+                $payload['iface'] = trim($this->ruleIface);
+            }
+            
             if (!empty($this->ruleLog) && $this->ruleLog !== 'nolog') {
                 $payload['log'] = $this->ruleLog;
             }
-
+            
-            // Determine if we're editing an existing rule or creating a new one
+            // If macro is set, use it instead of manual proto/port
-            if ($this->editingRuleIndex !== null && isset($this->rules[$this->editingRuleIndex])) {
+            if (!empty($this->ruleMacro)) {
-                // EDIT EXISTING RULE
+                $payload['macro'] = trim($this->ruleMacro);
-                $pos = $this->rules[$this->editingRuleIndex]['pos'];
+                // Remove proto/dport/sport when using macro
-                $url = '/nodes/' . $this->server->node->name . '/qemu/' . $this->server->vm_id . '/firewall/rules/' . $pos;
+                unset($payload['proto'], $payload['dport'], $payload['sport']);
-                
-                Log::info('Updating firewall rule', [
-                    'url' => $url,
-                    'payload' => $payload,
-                    'pos' => $pos
-                ]);
-                
-                $response = $this->firewallRequest($url, 'put', $payload);
-            } else {
-                // CREATE NEW RULE
-                $url = '/nodes/' . $this->server->node->name . '/qemu/' . $this->server->vm_id . '/firewall/rules';
-                
-                Log::info('Creating firewall rule', [
-                    'url' => $url,
-                    'payload' => $payload
-                ]);
-                
-                $response = $this->firewallRequest($url, 'post', $payload);
             }
 
-            // Log the response for debugging
+            Log::info('Saving firewall rule', [
-            Log::info('Firewall rule API response', [
+                'editing' => $this->editingRuleIndex !== null,
+                'payload' => $payload
+            ]);
+
+            // Determine if creating or updating
+            if ($this->editingRuleIndex !== null && isset($this->rules[$this->editingRuleIndex])) {
+                // UPDATE existing rule
+                $pos = $this->rules[$this->editingRuleIndex]['pos'];
+                $url = $baseUrl . '/' . $pos;
+                
+                Log::info('Updating rule at position ' . $pos, ['url' => $url]);
+                $response = $this->firewallRequest($url, 'put', $payload);
+            } else {
+                // CREATE new rule
+                Log::info('Creating new rule', ['url' => $baseUrl]);
+                $response = $this->firewallRequest($baseUrl, 'post', $payload);
+            }
+
+            Log::info('API Response', [
                 'status' => $response->status(),
                 'successful' => $response->successful(),
                 'body' => $response->body()
             ]);
 
             if ($response->successful()) {
-                $this->dispatch('notify', 'Rule ' . ($this->editingRuleIndex !== null ? 'updated' : 'created') . ' successfully', 'success');
+                $action = $this->editingRuleIndex !== null ? 'updated' : 'created';
+                $this->dispatch('notify', "Firewall rule {$action} successfully", 'success');
                 
-                // Reset the form
+                // Reset form and close modal
                 $this->resetRuleForm();
                 $this->editingRuleIndex = null;
                 $this->showRuleForm = false;
                 
-                // Reload rules from Proxmox to get the updated list
+                // Reload rules from Proxmox
                 $this->loadFirewallRules();
             } else {
-                // Extract error message from Proxmox API response
+                // Parse error from response
                 $responseData = $response->json();
-                $message = $responseData['errors'][0]['message'] ?? $responseData['message'] ?? $response->body();
+                $errorMsg = 'Unknown error';
+                
+                if (isset($responseData['errors'])) {
+                    $errorMsg = is_array($responseData['errors']) 
+                        ? json_encode($responseData['errors']) 
+                        : $responseData['errors'];
+                } elseif (isset($responseData['message'])) {
+                    $errorMsg = $responseData['message'];
+                } elseif (isset($responseData['data'])) {
+                    $errorMsg = is_string($responseData['data']) ? $responseData['data'] : json_encode($responseData['data']);
+                } else {
+                    $errorMsg = $response->body();
+                }
                 
                 Log::error('Failed to save firewall rule', [
                     'status' => $response->status(),
-                    'response' => $responseData,
+                    'error' => $errorMsg,
                     'payload' => $payload
                 ]);
                 
-                $this->error = 'Failed to save rule: ' . $message;
+                $this->error = 'Failed to save rule: ' . $errorMsg;
                 $this->showErrorModal = true;
             }
         } catch (\Exception $e) {
-            Log::error('Exception saving firewall rule', [
+            Log::error('Exception in saveRule', [
                 'message' => $e->getMessage(),
                 'trace' => $e->getTraceAsString()
             ]);
             
-            $this->error = 'Failed to save rule: ' . $e->getMessage();
+            $this->error = 'Error: ' . $e->getMessage();
             $this->showErrorModal = true;
         } finally {
             $this->loading = false;
         }
     }
 
+    /**
+     * Toggle a rule's enabled/disabled state
+     */
     public function toggleRule(int $index): void
     {
-        try {
+        if (!isset($this->rules[$index])) {
-            if (!isset($this->rules[$index])) {
+            $this->error = 'Rule not found';
-                return;
+            $this->showErrorModal = true;
-            }
+            return;
+        }
 
+        try {
             $rule = $this->rules[$index];
             $pos = $rule['pos'];
-            $currentEnable = isset($rule['enable']) ? (int)$rule['enable'] : 1;
+            $currentlyEnabled = isset($rule['enable']) ? (int)$rule['enable'] : 1;
-            $newEnable = $currentEnable == 0 ? 1 : 0;
+            $newState = $currentlyEnabled === 0 ? '1' : '0';
 
-            $response = $this->firewallRequest(
+            $url = '/nodes/' . $this->server->node->name . '/qemu/' . $this->server->vm_id . '/firewall/rules/' . $pos;
-                '/nodes/' . $this->server->node->name . '/qemu/' . $this->server->vm_id . '/firewall/rules/' . $pos,
+            
-                'put',
+            $response = $this->firewallRequest($url, 'put', ['enable' => $newState]);
-                ['enable' => $newEnable]
-            );
 
             if ($response->successful()) {
-                $this->dispatch('notify', 'Rule ' . ($newEnable == 1 ? 'enabled' : 'disabled'), 'success');
+                $stateText = $newState === '1' ? 'enabled' : 'disabled';
+                $this->dispatch('notify', "Rule {$stateText}", 'success');
                 $this->loadFirewallRules();
             } else {
+                Log::error('Failed to toggle rule', ['response' => $response->body()]);
                 $this->error = 'Failed to toggle rule';
                 $this->showErrorModal = true;
             }
         } catch (\Exception $e) {
-            $this->error = 'Failed to toggle rule: ' . $e->getMessage();
+            Log::error('Exception toggling rule: ' . $e->getMessage());
+            $this->error = 'Error: ' . $e->getMessage();
             $this->showErrorModal = true;
         }
     }
 
+    /**
+     * Show confirmation modal for deleting a rule
+     */
     public function confirmDeleteRule(int $index): void
     {
         if (!isset($this->rules[$index])) {
+            $this->error = 'Rule not found';
+            $this->showErrorModal = true;
             return;
         }
 
@@ -812,32 +857,40 @@ class Firewall extends Component
         $this->showDeleteRuleModal = true;
     }
 
+    /**
+     * Delete a firewall rule
+     */
     public function deleteRule(): void
     {
         if ($this->deletingRuleIndex === null || !isset($this->rules[$this->deletingRuleIndex])) {
+            $this->showDeleteRuleModal = false;
             return;
         }
 
-        $rule = $this->rules[$this->deletingRuleIndex];
-        $pos = $rule['pos'];
         $this->loading = true;
         $this->showDeleteRuleModal = false;
 
         try {
-            $response = $this->firewallRequest(
+            $rule = $this->rules[$this->deletingRuleIndex];
-                '/nodes/' . $this->server->node->name . '/qemu/' . $this->server->vm_id . '/firewall/rules/' . $pos,
+            $pos = $rule['pos'];
-                'delete'
+            
-            );
+            $url = '/nodes/' . $this->server->node->name . '/qemu/' . $this->server->vm_id . '/firewall/rules/' . $pos;
+            
+            Log::info('Deleting firewall rule', ['url' => $url, 'pos' => $pos]);
+            
+            $response = $this->firewallRequest($url, 'delete');
 
             if ($response->successful()) {
                 $this->dispatch('notify', 'Rule deleted successfully', 'success');
                 $this->loadFirewallRules();
             } else {
-                $this->error = 'Failed to delete rule';
+                Log::error('Failed to delete rule', ['response' => $response->body()]);
+                $this->error = 'Failed to delete rule: ' . $response->body();
                 $this->showErrorModal = true;
             }
         } catch (\Exception $e) {
-            $this->error = 'Failed to delete rule: ' . $e->getMessage();
+            Log::error('Exception deleting rule: ' . $e->getMessage());
+            $this->error = 'Error: ' . $e->getMessage();
             $this->showErrorModal = true;
         } finally {
             $this->loading = false;
@@ -845,41 +898,50 @@ class Firewall extends Component
         }
     }
 
+    /**
+     * Cancel delete confirmation
+     */
     public function cancelDeleteRule(): void
     {
         $this->deletingRuleIndex = null;
         $this->showDeleteRuleModal = false;
     }
 
+    /**
+     * Reorder rules by moving one rule to another position
+     */
     public function reorderRules(int $fromPos, int $toPos): void
     {
         try {
-            $response = $this->firewallRequest(
+            $url = '/nodes/' . $this->server->node->name . '/qemu/' . $this->server->vm_id . '/firewall/rules/' . $fromPos;
-                '/nodes/' . $this->server->node->name . '/qemu/' . $this->server->vm_id . '/firewall/rules/' . $fromPos,
+            
-                'put',
+            $response = $this->firewallRequest($url, 'put', ['moveto' => (string)$toPos]);
-                ['moveto' => $toPos]
-            );
 
             if ($response->successful()) {
-                $this->dispatch('notify', 'Rules reordered successfully', 'success');
+                $this->dispatch('notify', 'Rules reordered', 'success');
                 $this->loadFirewallRules();
             } else {
+                Log::error('Failed to reorder rules', ['response' => $response->body()]);
                 $this->error = 'Failed to reorder rules';
                 $this->showErrorModal = true;
             }
         } catch (\Exception $e) {
-            $this->error = 'Failed to reorder rules: ' . $e->getMessage();
+            Log::error('Exception reordering rules: ' . $e->getMessage());
+            $this->error = 'Error: ' . $e->getMessage();
             $this->showErrorModal = true;
         }
     }
 
+    /**
+     * Reset the rule form to default values
+     */
     protected function resetRuleForm(): void
     {
         $this->ruleAction = 'ACCEPT';
         $this->ruleType = 'in';
         $this->ruleSource = '';
         $this->ruleDest = '';
-        $this->ruleProto = '';
+        $this->ruleProto = 'tcp';
         $this->ruleDport = '';
         $this->ruleSport = '';
         $this->ruleComment = '';
@@ -889,6 +951,9 @@ class Firewall extends Component
         $this->ruleLog = 'nolog';
     }
 
+    /**
+     * Cancel and close the rule form
+     */
     public function cancelRuleForm(): void
     {
         $this->resetRuleForm();