Merge pull request #15 from jaredhendrickson13/v100_bug_fixes

v1.0.0 Bug Fixes

Author: jaredhendrickson13

README.md

@@ -21,22 +21,29 @@ webConfigurator are required to make calls to the API endpoints
 # Installation
 To install pfSense API, simply run the following command from the pfSense shell:<br>
 ```
-pkg add https://github.com/jaredhendrickson13/pfsense-api/releases/download/v1.0.0/pfSense-2-4-pkg-API-1.0_0.txz && /etc/rc.restart_webgui
+pkg add https://github.com/jaredhendrickson13/pfsense-api/releases/latest/download/pfSense-2.4-pkg-API.txz && /etc/rc.restart_webgui
 ```
 
-To uninstall, run the following command:<br>
+To uninstall pfSense API, run the following command:<br>
 ```
 pkg delete pfSense-pkg-API
 ```
 
+To update pfSense API to latest version, run the following command:
+```
+pkg delete pfSense-pkg-API && pkg add https://github.com/jaredhendrickson13/pfsense-api/releases/latest/download/pfSense-2.4-pkg-API.txz && /etc/rc.restart_webgui
+```
+
 ### Notes: 
+- pfSense API is supported on the pfSense 2.5 developer snapshots. To install the 2.5 package, simply change the `2.4` in the install URL to `2.5`.
 - In order for pfSense to apply some required web server changes, it is required to restart the webConfigurator after installing the package
 - If you do not have shell access to pfSense, you can still install via the webConfigurator by navigating to 
 'Diagnostics > Command Prompt' and enter the commands there
+- When updating pfSense, **_you must reinstall pfSense API afterwards_**. Unfortunately, pfSense removes all existing packages and only reinstalls packages found within pfSense's package repositories. Since pfSense API is not an official package in pfSense's repositories, it does not get reinstalled automatically.
 
 
 # UI Settings & Documentation
-After installation, you will be able to access the API user interface pages within the pfSense webConfigurator. These will be found under System > API. The settings tab will allow you change various API settings such as enabled API interfaces, authentication modes, and more. Additionally, the documentation tab will give you access to an embedded documentation tool that makes it easy to view the full API documentation with context to your pfSense instance.
+After installation, you will be able to access the API user interface pages within the pfSense webConfigurator. These will be found under System > API. The settings tab will allow you change various API settings such as enabled API interfaces, authentication modes, and more. Additionally, the documentation tab will give you access to an embedded documentation tool that makes it easy to view the full API documentation in context to your pfSense instance.
 
 ### Notes: 
 - Users must hold the `page-all` or `page-system-api` privileges to access the API page within the webConfigurator

docs/CONTRIBUTING.md

@@ -100,6 +100,10 @@ writing a model that tests user's local database credentials and do not want the
 auth mode you would specify `$this->set_auth_mode = "local";` to always force local authentication. Defaults to the 
 API's configured auth mode in the /api/ webConfigurator page.
 
+- `$this->set_read_mode` : Allows the read-only API setting to be bypassed for this model. If you set this value to 
+`true` the model will be allowed to use POST, PUT or DELETE methods even when the API is in read-only mode. There is 
+rarely a use case for this. Do not override this property unless absolutely needed. 
+
 - `$this->change_note` : Sets the description of the action that occurred via API. This value will be shown in the 
 change logs found at Diagnostics > Backup & Restore > Config History. This defaults to "Made unknown change via API". 
 This is only necessary if your API model writes changes to the configuration.

docs/documentation.json

@@ -20,6 +20,7 @@ class APIAuth {
     private $api_config;
     private $request;
     public $auth_mode;
+    public $read_mode;
     public $req_privs;
     public $username;
     public $privs;
@@ -28,9 +29,10 @@ class APIAuth {
     public $is_authorized;
 
     # Create our method constructor
-    public function __construct($req_privs, $enforce_auth_mode=null){
+    public function __construct($req_privs, $enforce_auth_mode=null, $read_mode=null){
         $this->api_config = APITools\get_api_config()[1];
         $this->auth_mode = (is_null($enforce_auth_mode)) ? $this->api_config["authmode"] : $enforce_auth_mode;
+        $this->read_mode = (is_null($read_mode)) ? array_key_exists("readonly", $this->api_config) : $read_mode;
         $this->request = APITools\get_request_data();
         $this->req_privs = $req_privs;
         $this->privs = [];
@@ -113,17 +115,16 @@ class APIAuth {
     # AUTHORIZATION #
     # Checks if the current user has the necessary privileges to access this resource
     public function authorize() {
-        // Local variables
+        # Local variables
         $authorized = false;
         $client_config =& getUserEntry($this->username);;
         $this->privs = get_user_privileges($client_config);
-        $read_only = array_key_exists("readonly", $this->api_config);
 
         # If no require privileges were given, assume call is always authorized
         if (!empty($this->req_privs)) {
-            // Ensure our API is not read only OR if it is read only, only authorize the request if it is a GET request
-            if ($read_only === false or ($read_only === true and $_SERVER['REQUEST_METHOD'] === "GET")) {
-                // Loop through each of our req privs and ensure the client has them, also check if access is read only
+            # If API is in readonly mode, only allow GET requests
+            if (!$this->read_mode or ($this->read_mode and $_SERVER['REQUEST_METHOD'] === "GET")) {
+                # Loop through each of our req privs and ensure the client has them, also check if access is read only
                 foreach ($this->req_privs as &$priv) {
                     if (in_array($priv, $this->privs)) {
                         $authorized = true;

pfSense-pkg-API/files/etc/inc/api/framework/APIAuth.inc

@@ -30,17 +30,15 @@ class APIModel {
     public $change_note;
     public $requires_auth;
     public $set_auth_mode;
+    public $bypass_read_mode;
 
     public function __construct() {
         global $config;
         error_reporting(E_ERROR);
         $this->config =& $config;
         $this->privileges = ["page-all"];
-        $this->client = null;
         $this->requires_auth = true;
-        $this->set_auth_mode = null;
         $this->change_note = "Made unknown change via API";
-        $this->id = null;
         $this->validate_id = true;
         $this->initial_data = APITools\get_request_data();
         $this->validated_data = [];
@@ -95,7 +93,7 @@ class APIModel {
     }
 
     private function check_authentication() {
-        $this->client = new APIAuth($this->privileges, $this->set_auth_mode);
+        $this->client = new APIAuth($this->privileges, $this->set_auth_mode, $this->bypass_read_mode);
         if ($this->requires_auth === true) {
             if (!$this->client->is_authenticated) {
                 $this->errors[] = APIResponse\get(3);

pfSense-pkg-API/files/etc/inc/api/framework/APIModel.inc

@@ -21,6 +21,7 @@ class APIAccessTokenCreate extends APIModel {
     public function __construct() {
         parent::__construct();
         $this->set_auth_mode = "local";
+        $this->bypass_read_mode = false;
     }
 
     # Validate our API configurations auth mode (must be JWT)

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

@@ -34,6 +34,9 @@ class APIFirewallRuleCreate extends APIModel {
     }
 
     public function validate_payload() {
+        # Include static rule ID
+        $this->validated_data["id"] = "";
+
         # Check for our required 'type' payload value
         if (isset($this->initial_data["type"])) {
             $type_options = array("pass", "block", "reject");
@@ -179,8 +182,8 @@ class APIFirewallRuleCreate extends APIModel {
 
 
         # Check for our optional 'disabled' payload value
-        if ($this->initial_data['disabled'] !== true) {
-            $this->validated_data["id"] = "";
+        if ($this->initial_data['disabled'] === true) {
+            $this->validated_data["disabled"] = "";
         }
 
         # Check for our optional 'descr' payload value

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

@@ -51,7 +51,7 @@ class APIFirewallRuleUpdate extends APIModel {
             $this->errors[] = APIResponse\get(4031);
         }
 
-        # Check for our required 'type' payload value
+        # Check for our optional 'type' payload value
         if (isset($this->initial_data["type"])) {
             $type_options = array("pass", "block", "reject");
             # Ensure our type is a valid option
@@ -62,7 +62,7 @@ class APIFirewallRuleUpdate extends APIModel {
             }
         }
 
-        # Check for our required 'interface' payload value
+        # Check for our optional 'interface' payload value
         if (isset($this->initial_data['interface'])) {
             $this->initial_data['interface'] = APITools\get_pfsense_if_id($this->initial_data['interface']);
             # Check that we found the request pfSense interface ID
@@ -73,7 +73,7 @@ class APIFirewallRuleUpdate extends APIModel {
             }
         }
 
-        # Check for our required 'ipprotocol' payload value
+        # Check for our optional 'ipprotocol' payload value
         if (isset($this->initial_data['ipprotocol'])) {
             $ipprotocol_options = array("inet", "inet6", "inet46");
             # Check that our ipprotocol value is a support option
@@ -84,13 +84,16 @@ class APIFirewallRuleUpdate extends APIModel {
             }
         }
 
-        # Check for our required 'protocol' payload value
+        # Check for our optional 'protocol' payload value
         if (isset($this->initial_data['protocol'])) {
-            $port_required = (!in_array($this->validated_data["protocol"], ["tcp", "udp", "tcp/udp"])) ? true : false;
+            $missing_port = (!in_array($this->validated_data["protocol"], ["tcp", "udp", "tcp/udp"])) ? true : false;
+            $requires_port = (in_array($this->initial_data["protocol"], ["tcp", "udp", "tcp/udp"])) ? true : false;
+            $this->unset_ports();
             $protocol_options = [
                 "any", "tcp", "udp", "tcp/udp", "icmp", "esp", "ah",
                 "gre", "ipv6", "igmp", "pim", "ospf", "carp", "pfsync"
             ];
+
             # Check that our protocol value is a support option
             if (in_array($this->initial_data["protocol"], $protocol_options)) {
                 # Don't add a specific protocol if any
@@ -102,9 +105,10 @@ class APIFirewallRuleUpdate extends APIModel {
             } else {
                 $this->errors[] = APIResponse\get(4042);
             }
-            $protocol = $this->initial_data['protocol'];
         } else {
-            $this->errors[] = APIResponse\get(4036);
+            # If a new protocol was not selected don't validate ports
+            $requires_port = false;
+            $missing_port = false;
         }
 
         # Check for our optional 'icmpsubtype' payload value when our protocol is set to ICMP
@@ -130,7 +134,7 @@ class APIFirewallRuleUpdate extends APIModel {
             }
         }
 
-        # Check for our required 'src' payload value
+        # Check for our optional 'src' payload value
         if (isset($this->initial_data['src'])) {
             // Check if our source and destination values are valid
             $dir_check = APITools\is_valid_rule_addr($this->initial_data['src'], "source");
@@ -141,7 +145,7 @@ class APIFirewallRuleUpdate extends APIModel {
             }
         }
 
-        # Check for our required 'src' payload value
+        # Check for our optional 'src' payload value
         if (isset($this->initial_data['dst'])) {
             // Check if our source and destination values are valid
             $dir_check = APITools\is_valid_rule_addr($this->initial_data['dst'], "destination");
@@ -152,16 +156,16 @@ class APIFirewallRuleUpdate extends APIModel {
             }
         }
 
-        # Check for our required 'srcport' and 'dstport' payload values if protocol is TCP, UDP or TCP/UDP
-        if ($port_required && in_array($this->initial_data["protocol"], ["tcp", "udp", "tcp/udp"])) {
+        # Check for our optional 'srcport' and 'dstport' payload values if protocol is TCP, UDP or TCP/UDP
+        if ($requires_port) {
             if (isset($this->initial_data['srcport'])) {
                 $val = str_replace("-", ":", $this->initial_data['srcport']);
                 if (!is_port_or_range($val) and $val !== "any") {
                     $this->errors[] = APIResponse\get(4048);
                 } elseif ($val !== "any") {
                     $this->validated_data["source"]["port"] = str_replace(":", "-", $val);;
                 }
-            } else {
+            } elseif ($missing_port) {
                 $this->errors[] = APIResponse\get(4047);
             }
 
@@ -172,7 +176,7 @@ class APIFirewallRuleUpdate extends APIModel {
                 } elseif ($val !== "any") {
                     $this->validated_data["destination"]["port"] = str_replace(":", "-", $val);;
                 }
-            } else {
+            } elseif ($missing_port) {
                 $this->errors[] = APIResponse\get(4047);
             }
         }
@@ -187,10 +191,11 @@ class APIFirewallRuleUpdate extends APIModel {
             }
         }
 
-
         # Check for our optional 'disabled' payload value
-        if ($this->initial_data['disabled'] !== true) {
-            $this->validated_data["id"] = "";
+        if ($this->initial_data['disabled'] === true) {
+            $this->validated_data["disabled"] = "";
+        } elseif ($this->initial_data['disabled'] === false) {
+            unset($this->validated_data["disabled"]);
         }
 
         # Check for our optional 'descr' payload value
@@ -214,4 +219,13 @@ class APIFirewallRuleUpdate extends APIModel {
             "username" => $this->client->username."@".$this->client->ip_address." (API)"
         ];
     }
+
+    # Removes port specifications when updating to a protocol that is not port based
+    private function unset_ports() {
+        # If a new protocol was chosen that doesn't require a port, remove existing ports from the rule
+        if ((!in_array($this->initial_data["protocol"], ["tcp", "udp", "tcp/udp"]))) {
+            unset($this->validated_data["source"]["port"]);
+            unset($this->validated_data["destination"]["port"]);
+        }
+    }
 }