Merge pull request #66 from jaredhendrickson13/outbound_nat_protocol_fix

Outbound NAT mapping protocol fix

Author: jaredhendrickson13

pfSense-pkg-API/files/etc/inc/api/models/APIFirewallNATOutboundMappingCreate.inc

@@ -28,7 +28,7 @@ class APIFirewallNATOutboundMappingCreate extends APIModel {
         parent::__construct();
         $this->change_note = "Added outbound NAT mapping via API";
         $this->privileges = ["page-all", "page-firewall-nat-outbound-edit"];
-        $this->protocols = ["tcp", "udp", "tcp/udp", "icmp", "esp", "ah", "gre", "ipv6", "igmp", "pim", "ospf"];
+        $this->protocols = ["any", "tcp", "udp", "tcp/udp", "icmp", "esp", "ah", "gre", "ipv6", "igmp", "pim", "ospf"];
         $this->pool_options = ["round-robin", "round-robin sticky-address", "random", "random sticky-address", "source-hash", "bitmask"];
         $this->port_supported = false;
         $this->pool_source_hash_supported = false;
@@ -65,10 +65,13 @@ class APIFirewallNATOutboundMappingCreate extends APIModel {
         if (isset($this->initial_data['protocol'])) {
             # Require protocol to be a known/supported protocol
             if (in_array($this->initial_data['protocol'], $this->protocols)) {
-                $this->validated_data["protocol"] = $this->initial_data['protocol'];
-                # Set our port supported toggle to true if our protocol uses ports
-                if (in_array($this->validated_data["protocol"], ["tcp", "udp", "tcp/udp"])) {
-                    $this->port_supported = true;
+                # Only add the protocol if it is not any (XML expects no entry for any)
+                if ($this->initial_data["protocol"] !== "any") {
+                    $this->validated_data["protocol"] = $this->initial_data['protocol'];
+                    # Set our port supported toggle to true if our protocol uses ports
+                    if (in_array($this->validated_data["protocol"], ["tcp", "udp", "tcp/udp"])) {
+                        $this->port_supported = true;
+                    }
                 }
             } else {
                 $this->errors[] = APIResponse\get(4089);

pfSense-pkg-API/files/etc/inc/api/models/APIFirewallNATOutboundMappingUpdate.inc

@@ -28,7 +28,7 @@ class APIFirewallNATOutboundMappingUpdate extends APIModel {
         parent::__construct();
         $this->change_note = "Modified outbound NAT mapping via API";
         $this->privileges = ["page-all", "page-firewall-nat-outbound-edit"];
-        $this->protocols = ["tcp", "udp", "tcp/udp", "icmp", "esp", "ah", "gre", "ipv6", "igmp", "pim", "ospf"];
+        $this->protocols = ["any", "tcp", "udp", "tcp/udp", "icmp", "esp", "ah", "gre", "ipv6", "igmp", "pim", "ospf"];
         $this->pool_options = ["", "round-robin", "round-robin sticky-address", "random", "random sticky-address", "source-hash", "bitmask"];
         $this->port_supported = false;
         $this->pool_source_hash_supported = false;
@@ -76,7 +76,12 @@ class APIFirewallNATOutboundMappingUpdate extends APIModel {
         if (isset($this->initial_data['protocol'])) {
             # Require protocol to be a known/supported protocol
             if (in_array($this->initial_data['protocol'], $this->protocols)) {
-                $this->validated_data["protocol"] = $this->initial_data['protocol'];
+                # Unset the protocol value if it is any (XML expects no entry for any). Otherwise update value.
+                if ($this->initial_data["protocol"] === "any") {
+                    unset($this->validated_data["protocol"]);
+                } else {
+                    $this->validated_data["protocol"] = $this->initial_data['protocol'];
+                }
             } else {
                 $this->errors[] = APIResponse\get(4089);
             }

pfSense-pkg-API/files/etc/inc/api/models/APIInterfaceUpdate.inc

@@ -53,11 +53,13 @@ class APIInterfaceUpdate extends APIModel {
 
     private function __validate_if() {
         if (isset($this->initial_data["if"])) {
-            $this->validated_data["if"] = trim($this->initial_data["if"]);
-            // Check that our interface exists and is not in use
-            if (!array_key_exists($this->initial_data["if"], $this->if_list)) {
+            $if_info = $this->if_list[$this->initial_data["if"]];
+            # Return an error if the requested physical interface does not exist
+            if (empty($if_info)) {
                 $this->errors[] = APIResponse\get(3000);
-            } elseif (isset($this->if_list[$this->initial_data["if"]]["in_use"])) {
+            }
+            # Return an error if the physical interface is already in use by a different interface object
+            elseif (isset($if_info["in_use"]) and $if_info["in_use"] !== $this->id) {
                 $this->errors[] = APIResponse\get(3001);
             }
             $this->validated_data["if"] = $this->initial_data["if"];

tests/test_api_v1_firewall_nat_outbound_mapping.py

@@ -31,28 +31,54 @@ class APIUnitTestFirewallNATOutboundMapping(unit_test_framework.APIUnitTest):
             "descr": "Unit Test",
             "nosync": True,
             "top": True
+        },
+        {
+            "interface": "WAN",
+            "protocol": "any",
+            "src": "any",
+            "dst": "1.1.1.1",
+            "target": "192.168.1.123/24",
+            "poolopts": "round-robin",
+            "descr": "Unit Test 2",
+            "nosync": True,
+            "top": True
         }
     ]
     put_payloads = [
         {
             "id": 0,
             "interface": "WAN",
+            "protocol": "any",
+            "src": "any",
+            "dst": "1.1.1.1",
+            "target": "192.168.1.123/24",
+            "poolopts": "round-robin",
+            "descr": "Updated Unit Test",
+            "nonat": True,
+            "disabled": True,
+            "nosync": True,
+            "top": True
+        },
+        {
+            "id": 1,
+            "interface": "WAN",
             "protocol": "udp",
             "src": "any",
             "srcport": "433",
             "dst": "1.1.1.1",
             "dstport": "443",
             "target": "192.168.1.123/24",
-            "natstaticport": True,
+            "staticnatport": True,
             "poolopts": "round-robin",
             "descr": "Updated Unit Test",
-            "nonat": True,
+            "nonat": False,
             "disabled": True,
             "nosync": True,
             "top": True
         }
     ]
     delete_payloads = [
+        {"id": 0},
         {"id": 0}
     ]