mirror/php-e107
1
0
Fork 0
mirror of https://github.com/e107inc/e107.git synced 2025-08-05 06:07:32 +02:00
Code Issues Releases Wiki Activity

Issue #5487 Ban caching and ban query fixes. Unit tests added.

Browse Source
This commit is contained in:
camer0n
2025-05-04 12:08:42 -07:00
parent b7bb3b931e
commit dee6e8fb62
5 changed files with 385 additions and 119 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
e107_admin/admin_log.php
View File

@@ -247,7 +247,7 @@ class admin_log_ui extends e_admin_ui
$perPage = e107::getConfig()->get('sys_log_perpage');
$this->perPage = vartrue($perPage,10);
$this->prefs['sys_log_perpage']['writeParms'] = array(10=>10, 15=>15, 20=>20, 30=>30, 40=>40, 50=>50);
$this->prefs['sys_log_perpage']['writeParms'] = array(10=>10, 15=>15, 20=>20, 30=>30, 40=>40, 50=>50, 75=>75, 100=>100, 250=>250 );
$this->eventTypes = loadEventTypes('admin_log');

5
e107_handlers/admin_log_class.php
View File

@@ -364,7 +364,10 @@ class e_admin_log
//---------------------------------------
if(($target_logs & LOG_TO_ROLLING) && $this->_roll_log_active)
{ // Rolling log
if(getperms('0') && e_REQUEST_HTTP === '/e107_admin/admin_log.php') // Don't log while looking at the log.
{
return;
}
// Process source_call info
//---------------------------------------
if(is_numeric($source_call) && ($source_call >= 0))

293
e107_handlers/iphandler_class.php
View File

@@ -764,7 +764,6 @@ class eIPHandler
/**
* Generate DB query for domain name-related checks
*
* If an email address is passed, discards the individual's name
*
* @param string $email - an email address or domain name string
* @param string $fieldName
@@ -774,31 +773,51 @@ class eIPHandler
* @internal param string $fieldname - if non-empty, each array entry is a comparison with this field
*
*/
function makeDomainQuery($email, $fieldName = 'banlist_ip')
/**
* Generate DB query for network-level domain bans
*
* @param string $domain - a domain name string (e.g., mydomain.co.uk)
* @param string $fieldName - if non-empty, each array entry is a comparison with this field
* @return array|bool - array of network ban patterns, or false if invalid domain
*/
function makeDomainQuery($domain, $fieldName = 'banlist_ip'): array|bool
{
$tp = e107::getParser();
if (($tv = strrpos('@', $email)) !== FALSE)
$domain = trim($domain);
if(strpos($domain, '@') !== false) // this function not intended for email addresses.
{
$email = substr($email, $tv+1);
return false;
}
$tmp = strtolower($tp -> toDB(trim($email)));
if ($tmp == '') return FALSE;
if (strpos($tmp,'.') === FALSE) return FALSE;
$em = array_reverse(explode('.',$tmp));
if($domain === '' || strpos($domain, '.') === false)
{
return false;
}
$sanitized_domain = strtolower($domain);
$parts = array_reverse(explode('.', $sanitized_domain));
$line = '';
$out = array('*@'.$tmp); // First element looks for domain as email address
foreach ($em as $e)
$out = [];
foreach($parts as $part)
{
$line = '.'.$e.$line;
$out[] = '*'.$line;
$line = '.' . $part . $line;
$out[] = '*' . $line;
}
if ($fieldName)
if($fieldName)
{
foreach ($out as $k => $v)
foreach($out as $k => $v)
{
$out[$k] = '(`'.$fieldName."`='".$v."')";
$out[$k] = "(`$fieldName`='$v')";
}
}
return $out;
}
@@ -809,26 +828,45 @@ class eIPHandler
* @param string $email - email address to process
* @param string $fieldname - name of field being searched in DB
*
* @return bool|string false if invalid address. Otherwise returns a set of values to check
* @return bool|string|array false if invalid address. Otherwise returns a set of values to check
* (Moved in from user_handler.php)
*/
public function makeEmailQuery($email, $fieldname = 'banlist_ip')
{
$tp = e107::getParser();
$tmp = strtolower($tp -> toDB(trim(substr($email, strrpos($email, "@")+1)))); // Pull out the domain name
if ($tmp == '') return FALSE;
if (strpos($tmp,'.') === FALSE) return FALSE;
$em = array_reverse(explode('.',$tmp));
$line = '';
$out = array($fieldname."='*@{$tmp}'"); // First element looks for domain as email address
foreach ($em as $e)
{
$line = '.'.$e.$line;
$out[] = '`'.$fieldname."`='*{$line}'";
}
return implode(' OR ',$out);
}
$email = trim($email);
if(!filter_var($email, FILTER_VALIDATE_EMAIL))
{
return $fieldname ? false : [];
}
$sanitized_email = addslashes($email);
$domain = strtolower(substr($email, strrpos($email, '@') + 1));
if($domain === '' || strpos($domain, '.') === false)
{
return $fieldname ? false : [];
}
$out = [
"$sanitized_email", // Specific email (e.g., user@mydomain.co.uk)
"*@$domain" // Domain ban (e.g., *@mydomain.co.uk)
];
if($fieldname)
{
$out = [
"`$fieldname`='$sanitized_email'",
"`$fieldname`='*@$domain'"
];
return implode(' OR ', $out);
}
return $out;
}
/**
@@ -876,34 +914,26 @@ class eIPHandler
if ($ip !== e107::LOCALHOST_IP && ($ip !== e107::LOCALHOST_IP2) && ($ip !== $this->serverIP)) // Check host name, user email to see if banned
{
$vals = array();
$vals = [];
if (e107::getPref('enable_rdns'))
{
$vals = array_merge($vals, $this->makeDomainQuery($this->get_host_name($ip), ''));
}
if ((defined('USEREMAIL') && USEREMAIL))
if(deftrue('USEREMAIL'))
{
// @todo is there point to this? Usually avoid a complete query if we skip it
$vals = array_merge($vals, $this->makeDomainQuery(USEREMAIL, ''));
$vals = array_merge($vals, $this->makeEmailQuery(USEREMAIL, ''));
}
if (count($vals))
{
$vals = array_unique($vals); // Could get identical values from domain name check and email check
if($this->debug)
{
print_a($vals);
}
$match = "`banlist_ip`='".implode("' OR `banlist_ip`='", $vals)."'";
$this->checkBan($match);
}
}
elseif($this->debug)
{
print_a("IP is LocalHost - skipping ban-check");
}
}
}
@@ -913,7 +943,6 @@ class eIPHandler
* Check the banlist table. $query is used to determine the match.
* If $do_return, will always return with ban status - TRUE for OK, FALSE for banned.
* If return permitted, will never display a message for a banned user; otherwise will display any message then exit
* @todo consider whether can be simplified
*
* @param string $query - the 'WHERE' part of the DB query to be executed
* @param boolean $show_error - if true, adds a '403 Forbidden' header for a banned user
@@ -922,101 +951,131 @@ class eIPHandler
*/
public function checkBan($query, $show_error = true, $do_return = false)
{
$call_id = uniqid();
$session_key = 'ban_check_' . md5($query);
$session = e107::getSession('eIPHandler');
// Long-term cache (360 seconds)
if($session->has($session_key))
{
$cached = $session->get($session_key);
if(isset($cached['timestamp']) && (time() - $cached['timestamp'] <= 360))
{
return $cached['result'];
}
}
// Short-term in-memory cache (1 second) using registry
$cache_key = md5($query);
$current_time = microtime(true);
$recent_checks = e107::getRegistry('core/eIPHandler/checkBan', []);
if(isset($recent_checks[$cache_key]) && ($current_time - $recent_checks[$cache_key]['time'] <= 1))
{
// e107::getLog()->addEvent(4, __FILE__ . "|" . __FUNCTION__ . "@" . __LINE__, "DBG", "Using registry cache (Call: $call_id)", "Result: " . ($recent_checks[$cache_key]['result'] ? 'true' : 'false'), false, LOG_TO_ROLLING);
return $recent_checks[$cache_key]['result'];
}
$sql = e107::getDb();
$pref = e107::getPref();
$tp = e107::getParser();
$log = e107::getLog();
$log->addEvent(4,__FILE__."|".__FUNCTION__."@".__LINE__,"DBG","Check for Ban",$query,FALSE,LOG_TO_ROLLING);
$caller = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 2)[1];
if ($sql->select('banlist', '*', $query.' ORDER BY `banlist_bantype` DESC'))
$callerFunction = $caller['function']."() in ". basename($caller['file']);;
$log->addEvent(4, __FILE__ . "|" . __FUNCTION__ . "@" . __LINE__, "DBG", "Check for Ban ", $query."\nCall: $call_id\nCaller: $callerFunction", false, LOG_TO_ROLLING);
$full_query = "SELECT * FROM banlist WHERE $query ORDER BY `banlist_bantype` DESC";
$result = true;
$sqlResult = $sql->select('banlist', '*', $query . ' ORDER BY `banlist_bantype` DESC');
// $log->addEvent(4, __FILE__ . "|" . __FUNCTION__ . "@" . __LINE__, "DBG", "SQL Select Result (Call: $call_id)", "Query: $query, Result: " . ($sqlResult !== false ? 'Found' : 'Not Found'), false, LOG_TO_ROLLING);
if($sqlResult)
{
// Any whitelist entries will be first, because they are positive numbers - so we can answer based on the first DB record read
$row = $sql->fetch();
if($row['banlist_bantype'] >= eIPHandler::BAN_TYPE_WHITELIST)
{
$log->addEvent(4,__FILE__."|".__FUNCTION__."@".__LINE__,"DBG","Whitelist hit",$query,FALSE,LOG_TO_ROLLING);
return true; // Whitelisted entry
}
// Found banlist entry in table here
if(($row['banlist_banexpires'] > 0) && ($row['banlist_banexpires'] < time())) // Ban has expired - delete from DB
$log->addEvent(4, __FILE__ . "|" . __FUNCTION__ . "@" . __LINE__, "DBG", "Whitelist hit\nCall: $call_id", $query, false, LOG_TO_ROLLING);
}
elseif(($row['banlist_banexpires'] > 0) && ($row['banlist_banexpires'] < time()))
{
$sql->delete('banlist', $query);
$log->addEvent(4,__FILE__."|".__FUNCTION__."@".__LINE__,"DBG","Ban Expired",$row['banlist_ip'],FALSE,LOG_TO_ROLLING);
$log->addEvent(4, __FILE__ . "|" . __FUNCTION__ . "@" . __LINE__, "DBG", "Ban Expired ", $row['banlist_ip']."\nCall: $call_id", false, LOG_TO_ROLLING);
$this->regenerateFiles();
return true;
}
// User is banned hereafter - just need to sort out the details.
// May need to retrigger ban period
if (!empty($pref['ban_retrigger']) && !empty($pref['ban_durations'][$row['banlist_bantype']]))
else
{
$dur = (int) $pref['ban_durations'][$row['banlist_bantype']];
$updateQry = array(
'banlist_banexpires' => (time() + ($dur * 60 * 60)),
'WHERE' => "banlist_ip ='".$row['banlist_ip']."'"
);
$sql->update('banlist', $updateQry);
$this->regenerateFiles();
$log->addEvent(4,__FILE__."|".__FUNCTION__."@".__LINE__,"DBG","Retrigger Ban",$row['banlist_ip'],FALSE,LOG_TO_ROLLING);
}
$log->addEvent(4,__FILE__."|".__FUNCTION__."@".__LINE__,"DBG","Active Ban",$query,FALSE,LOG_TO_ROLLING);
if ($show_error)
{
header('HTTP/1.1 403 Forbidden', true);
}
// May want to display a message
if (!empty($pref['ban_messages']))
{
if($do_return) // Ban still current here
if(!empty($pref['ban_retrigger']) && !empty($pref['ban_durations'][$row['banlist_bantype']]))
{
return false;
$dur = (int) $pref['ban_durations'][$row['banlist_bantype']];
$updateQry = array(
'banlist_banexpires' => (time() + ($dur * 60 * 60)),
'WHERE' => "banlist_ip ='" . $row['banlist_ip'] . "'"
);
$sql->update('banlist', $updateQry);
$this->regenerateFiles();
$log->addEvent(4, __FILE__ . "|" . __FUNCTION__ . "@" . __LINE__, "DBG", "Retrigger Ban ", $row['banlist_ip']."\nCall: $call_id", false, LOG_TO_ROLLING);
}
echo $tp->toHTML(varset($pref['ban_messages'][$row['banlist_bantype']])); // Show message if one set
$log->addEvent(4, __FILE__ . "|" . __FUNCTION__ . "@" . __LINE__, "DBG", "Active Ban ", $query."\nCall: $call_id", false, LOG_TO_ROLLING);
if($show_error)
{
header('HTTP/1.1 403 Forbidden', true);
}
if(!empty($pref['ban_messages']))
{
if($do_return)
{
$result = false;
}
else
{
echo $tp->toHTML(varset($pref['ban_messages'][$row['banlist_bantype']]));
}
}
$log->addEvent(4, __FILE__ . "|" . __FUNCTION__ . "@" . __LINE__, 'BAN_03', 'LAN_AUDIT_LOG_003', $query, false, LOG_TO_ROLLING);
if($do_return)
{
$result = false;
}
else
{
exit();
}
}
$log->addEvent(4, __FILE__."|".__FUNCTION__."@".__LINE__, 'BAN_03', 'LAN_AUDIT_LOG_003', $query, FALSE, LOG_TO_ROLLING);
if($this->debug)
{
echo "<pre>query: ".$query;
echo "\nBanned</pre>";
}
// added missing if clause
if ($do_return)
{
return false;
}
exit();
}
if($this->debug)
{
echo "query: ".$query;
echo "<br />Not Banned ";
}
$log->addEvent(4, __FILE__ . "|" . __FUNCTION__ . "@" . __LINE__, "DBG", "No ban found ", $query."\nCall: $call_id\nCaller: $callerFunction", false, LOG_TO_ROLLING);
// Store in short-term in-memory cache (1 second) using registry
$recent_checks[$cache_key] = [
'result' => $result,
'time' => $current_time
];
$log->addEvent(4,__FILE__."|".__FUNCTION__."@".__LINE__,"DBG","No ban found",$query,FALSE,LOG_TO_ROLLING);
return true; // Email address OK
e107::setRegistry('core/eIPHandler/checkBan', $recent_checks);
// Store in long-term session cache (10 seconds)
$session->set($session_key, [
'result' => $result,
'timestamp' => time()
]);
return $result;
}
/**
* Add an entry to the banlist. $bantype = 1 for manual, 2 for flooding, 4 for multiple logins
* Returns TRUE if ban accepted.
@@ -1086,7 +1145,7 @@ class eIPHandler
$ban_message .= 'Host: '.$this->get_host_name($ban_ip);
}
// Add using an array - handles DB changes better
$sql->insert('banlist',
if(!$sql->insert('banlist',
array(
'banlist_id' => 0,
'banlist_ip' => $ban_ip ,
@@ -1096,7 +1155,11 @@ class eIPHandler
'banlist_admin' => $ban_user ,
'banlist_reason' => $ban_message ,
'banlist_notes' => $ban_notes
));
)))
{
trigger_error("Error adding ban to banlist table", E_USER_WARNING);;
// dbg("Error adding ban to banlist table");
}
$this->regenerateFiles();
return TRUE;

2
e107_languages/English/admin/lan_admin_log.php
View File

@@ -29,7 +29,7 @@ return [
'RL_LAN_028' => "Update Filters",
'RL_LAN_029' => "Event type filter",
'RL_LAN_030' => "Admin Log",
'RL_LAN_031' => "Actions to log",
'RL_LAN_031' => "User audit actions to log",
'RL_LAN_032' => "Pri",
'RL_LAN_033' => "Further Information",
'RL_LAN_044' => "Log events to display per page",

202
e107_tests/tests/unit/eIPHandlerTest.php
View File

@@ -21,6 +21,7 @@ class eIPHandlerTest extends \Codeception\Test\Unit
try
{
$this->ip = $this->make('eIPHandler');
$this->ip->regenerateFiles();
}
catch(Exception $e)
{
@@ -28,6 +29,16 @@ class eIPHandlerTest extends \Codeception\Test\Unit
}
}
protected function _after()
{
e107::setRegistry('core/eIPHandler/checkBan', null);
e107::getSession('eIPHandler')->clearData();
$this->ip->regenerateFiles();
}
/**
* Test IPHandler::ipDecode()
*/
@@ -101,7 +112,7 @@ class eIPHandlerTest extends \Codeception\Test\Unit
'-8' => 0 // unknown
);
e107::getConfig()->set('ban_durations', $banDurations)->save(false, true, false);
// e107::getConfig()->set('ban_durations', $banDurations)->save(false, true, false);
$result = $this->ip->add_ban(2, "unit test generated ban", '123.123.123.123');
$this::assertTrue($result);
@@ -237,5 +248,194 @@ class eIPHandlerTest extends \Codeception\Test\Unit
}
public function testMakeEmailQuery()
{
$email = 'cameron@mydomain.co.uk';
// Test with empty $fieldname
$result = $this->ip->makeEmailQuery($email, '');
$expected = ['cameron@mydomain.co.uk', '*@mydomain.co.uk'];
$this::assertSame($expected, $result);
// Test with default $fieldname
$result = $this->ip->makeEmailQuery($email);
$expected = "`banlist_ip`='cameron@mydomain.co.uk' OR `banlist_ip`='*@mydomain.co.uk'";
$this::assertSame($expected, $result);
// Test invalid email
$result = $this->ip->makeEmailQuery('invalid_email', '');
$expected = [];
$this::assertSame($expected, $result);
}
public function testMakeDomainQuery()
{
// Test valid domain
$domain = 'mydomain.co.uk';
$result = $this->ip->makeDomainQuery($domain, '');
$expected = ['*.uk', '*.co.uk', '*.mydomain.co.uk'];
$this::assertSame($expected, $result);
// Test email address
$result = $this->ip->makeDomainQuery('user@mydomain.co.uk', '');
$expected = false;
$this::assertSame($expected, $result);
// Test invalid domain
$result = $this->ip->makeDomainQuery('invalid#domain', '');
$expected = false;
$this::assertSame($expected, $result);
// Test with fieldName
$result = $this->ip->makeDomainQuery('mydomain.co.uk', 'banlist_ip');
$expected = [
"(`banlist_ip`='*.uk')",
"(`banlist_ip`='*.co.uk')",
"(`banlist_ip`='*.mydomain.co.uk')"
];
$this::assertSame($expected, $result);
}
// ----
/**
* @runInSeparateProcess
* @preserveGlobalState disabled
* @return void
*/
public function testCheckBanNoBan()
{
$query = "`banlist_ip`='cameron@mydomain.co.uk' OR `banlist_ip`='*@mydomain.co.uk'";
// Ensure no ban exists
e107::getDb()->delete('banlist', "`banlist_ip` IN ('cameron@mydomain.co.uk', '*@mydomain.co.uk')");
$this->ip->regenerateFiles();
// Clear session cache
e107::getSession('eIPHandler')->clearData();
// Test: no ban
$result = $this->ip->checkBan($query, true, true);
$this::assertTrue($result);
// Verify session cache is set
$cached = e107::getSession('eIPHandler')->get('ban_check_' . md5($query));
$this::assertIsArray($cached);
$this::assertTrue($cached['result']);
$this::assertArrayHasKey('timestamp', $cached);
}
/**
* @runInSeparateProcess
* @preserveGlobalState disabled
* @return void
*/
public function testCheckBanActiveBan()
{
$this->ip->add_ban(1, '', 'cameron@mydomain.co.uk');
// Test: active ban
$query = "`banlist_ip`='cameron@mydomain.co.uk' OR `banlist_ip`='*@mydomain.co.uk'";
$result = $this->ip->checkBan($query, true, true);
$this::assertFalse($result); // ie. banned.
// Verify session cache is set
$cached = e107::getSession('eIPHandler')->get('ban_check_' . md5($query));
$this::assertIsArray($cached);
$this::assertFalse($cached['result']);
$this::assertArrayHasKey('timestamp', $cached);
}
/**
* @runInSeparateProcess
* @preserveGlobalState disabled
* @return void
*/
public function testCheckBanThrottling()
{
$query = "`banlist_ip`='cameron@mydomain.co.uk' OR `banlist_ip`='*@mydomain.co.uk'";
// Ensure no ban exists
e107::getDb()->delete('banlist', "`banlist_ip` IN ('cameron@mydomain.co.uk', '*@mydomain.co.uk')");
// Test: multiple calls within 1 second
$startTime = microtime(true);
$result1 = $this->ip->checkBan($query, true, true);
$result2 = $this->ip->checkBan($query, true, true);
$endTime = microtime(true);
$this::assertTrue($result1);
$this::assertTrue($result2);
$this::assertLessThan(1, $endTime - $startTime, "Throttling test took too long, in-memory cache may not be working");
// Verify session cache is set
$cached = e107::getSession('eIPHandler')->get('ban_check_' . md5($query));
$this::assertIsArray($cached);
$this::assertTrue($cached['result']);
}
/**
* @runInSeparateProcess
* @preserveGlobalState disabled
* @return void
*/
public function testCheckBanCacheExpiration()
{
$query = "`banlist_ip`='cameron@mydomain.co.uk' OR `banlist_ip`='*@mydomain.co.uk'";
// Ensure no ban exists
e107::getDb()->delete('banlist', "`banlist_ip` IN ('cameron@mydomain.co.uk', '*@mydomain.co.uk')");
$this->ip->regenerateFiles();
// Set a cached result (no ban)
e107::getSession('eIPHandler')->set('ban_check_' . md5($query), [
'result' => true,
'timestamp' => time() - 5 // Within 10 seconds
]);
// Test: cached result within 10 seconds
$result = $this->ip->checkBan($query, true, true);
$this::assertTrue($result);
// Simulate cache expiration (11 seconds)
e107::getSession('eIPHandler')->set('ban_check_' . md5($query), [
'result' => true,
'timestamp' => time() - 11
]);
// Insert active ban
e107::getDb()->insert('banlist', [
'banlist_ip' => 'cameron@mydomain.co.uk',
'banlist_bantype' => 1,
'banlist_banexpires' => 0
]);
// Test: new ban after expiration
$result = $this->ip->checkBan($query, true, true);
$this::assertFalse($result);
// Cleanup
e107::getDb()->delete('banlist', "`banlist_ip`='cameron@mydomain.co.uk'");
}
}