diff --git a/lib/Cleantalk/Common/Firewall.php b/lib/Cleantalk/Common/Firewall.php index a87307c09..9d7c9b817 100644 --- a/lib/Cleantalk/Common/Firewall.php +++ b/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->isWhitelisted($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'); @@ -218,6 +234,32 @@ private function prioritize($results) return $result; } + /** + * Returns the position of a single firewall result in self::$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; + } + /** * Check the result if it whitelisted or trusted network *