Udp.Code.Firewall sorting

lib/Cleantalk/Common/Firewall.php

@@ -33,6 +33,20 @@ class Firewall
     public $debug;
     public $debug_data = '';
 
+    /**
+     * Statuses from the lowest priority to the highest. The position in this array IS the priority,
+     * self::calculatePriority() does nothing but look the status up here.
+     *
+     * A result of the personal lists of the site owner carries the is_personal flag, which is not a
+     * part of the status itself - the very same DENY_SFW comes both from the personal and from the
+     * common list. Such results are listed here with the PERSONAL__ prefix, so that both variants
+     * can take their own place in the order. A personal status absent from the list falls back to
+     * the position of its common variant.
+     *
+     * Note the position of PASS_SFW__BY_WHITELIST: it is the global (non-personal) white list of the
+     * cloud - good bots and the other common exclusions. It has to stay UNDER the DENY_* statuses,
+     * so a black listed User-Agent outweighs a good bot IP.
+     */
     private $statuses_priority = array(
         // Lowest
         'PASS_SFW',
@@ -41,12 +55,14 @@ class Firewall
         'PASS_ANTIFLOOD',
         'PASS_ANTICRAWLER_UA',
         'PASS_ANTICRAWLER',
+        'PASS_SFW__BY_WHITELIST',
         'DENY_ANTIFLOOD_UA',
         'DENY_ANTIFLOOD',
         'DENY_ANTICRAWLER_UA',
         'DENY_ANTICRAWLER',
         'DENY_SFW',
-        'PASS_SFW__BY_WHITELIST',
+        'PERSONAL__DENY_SFW',
+        'PERSONAL__PASS_SFW__BY_WHITELIST',
         // Highest
     );
 
@@ -109,7 +125,11 @@ public function run()
 
         $results = array();
 
-        // Checking
+        // Checking.
+        // Every module has to be run before any decision is made. An early exit here would hide the
+        // results of the modules below - e.g. a UA black list hit of the AntiCrawler would never be
+        // taken into account if the SFW had found the IP in the white list or in a trusted network.
+        // The whole picture is collected first, the decision is made by self::prioritize().
         foreach ($this->fw_modules as $module) {
             if (isset($module->isExcluded) && $module->isExcluded) {
                 continue;
@@ -119,13 +139,10 @@ public function run()
             if ( ! empty($module_results)) {
                 $results[$module->module_name] = $module_results;
             }
-
-            if ($this->isWhitelisted($results)) {
-                // Break protection logic if it whitelisted or trusted network.
-                break;
-            }
         }
 
+        $this->setWhitelistedCookie($results);
+
         // Write Logs
         foreach ($this->fw_modules as $module) {
             if (array_key_exists($module->module_name, $results)) {
@@ -194,8 +211,7 @@ private function prioritize($results)
             foreach ($this->fw_modules as $module) {
                 if (array_key_exists($module->module_name, $results)) {
                     foreach ($results[$module->module_name] as $fw_result) {
-                        $priority = array_search($fw_result['status'], $this->statuses_priority) +
-                                    (isset($fw_result['is_personal']) && $fw_result['is_personal'] ? count($this->statuses_priority) : 0);
+                        $priority = $this->calculatePriority($fw_result);
                         if ($priority >= $current_fw_result_priority) {
                             $current_fw_result_priority = $priority;
                             $result['status']           = TT::getArrayValueAsString($fw_result, 'status');
@@ -219,13 +235,39 @@ private function prioritize($results)
     }
 
     /**
-     * Check the result if it whitelisted or trusted network
+     * Returns the position of a single firewall result in $this->statuses_priority.
+     *
+     * A result of a personal list is looked up by the PERSONAL__ prefixed status first, so it takes
+     * its own place in the order. If there is no such entry, the common variant is used.
+     *
+     * @param array $fw_result
+     *
+     * @return int
+     */
+    private function calculatePriority($fw_result)
+    {
+        $status = TT::getArrayValueAsString($fw_result, 'status');
+
+        if (isset($fw_result['is_personal']) && $fw_result['is_personal']) {
+            $personal_priority = array_search('PERSONAL__' . $status, $this->statuses_priority);
+            if ($personal_priority !== false) {
+                return $personal_priority;
+            }
+        }
+
+        $priority = array_search($status, $this->statuses_priority);
+
+        return $priority === false ? 0 : $priority;
+    }
+
+    /**
+     * Set the white list cookie if any of the results is whitelisted or belongs to a trusted network.
      *
      * @param array $results
      *
-     * @return bool
+     * @return void
      */
-    private function isWhitelisted($results)
+    private function setWhitelistedCookie($results)
     {
         global $apbct;
 
@@ -242,12 +284,10 @@ private function isWhitelisted($results)
                             Cookie::set('ct_sfw_ip_wl', $cookie_val, time() + 86400 * 30, '/', '', null, true, 'Lax');
                         }
 
-                        return true;
+                        return;
                     }
                 }
             }
         }
-
-        return false;
     }
 }