Skip to content

Commit

Permalink
refactor: split up large methods into smaller ones (#3202)
Browse files Browse the repository at this point in the history
  • Loading branch information
@thorsten
thorsten committed Nov 1, 2024
1 parent 2a15b8f commit a902e7d
Show file tree
Hide file tree
Showing 3 changed files with 192 additions and 67 deletions.
7 changes: 3 additions & 4 deletions phpmyfaq/src/phpMyFAQ/Network.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,12 @@ public function __construct(private Configuration $configuration)
/**
* Performs a check if an IPv4 or IPv6 address is banned.
*
* @param string $ip IPv4 or IPv6 address
*
* @param string $ipAddress IPv4 or IPv6 address
* @return bool false, if not banned
*/
public function isBanned(string $ip): bool
public function isBanned(string $ipAddress): bool
{
$bannedIps = explode(' ', (string) $this->configuration->get('security.bannedIPs'));
return IpUtils::checkIp($ip, $bannedIps);
return IpUtils::checkIp($ipAddress, $bannedIps);
}
}
163 changes: 100 additions & 63 deletions phpmyfaq/src/phpMyFAQ/User/Tracking.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use phpMyFAQ\Filter;
use phpMyFAQ\Network;
use phpMyFAQ\Strings;
use Symfony\Component\HttpFoundation\HeaderBag;
use Symfony\Component\HttpFoundation\IpUtils;
use Symfony\Component\HttpFoundation\Request;

Expand Down Expand Up @@ -40,22 +41,14 @@ public static function getInstance(
/**
* @throws Exception
*/
public function log(string $action, int|string|null $data = null): void
public function log(string $action, int|string|null $data = null): bool
{
if (!$this->configuration->get('main.enableUserTracking')) {
return;
return false;
}

$bots = 0;
$banned = false;
$this->currentSessionId = Filter::filterVar(
$this->request->query->get(UserSession::KEY_NAME_SESSION_ID),
FILTER_VALIDATE_INT
);
$cookieId = Filter::filterVar(
$this->request->query->get(UserSession::COOKIE_NAME_SESSION_ID),
FILTER_VALIDATE_INT
);
$this->initializeSessionId();
$cookieId = $this->getCookieId();

if (!is_null($cookieId)) {
$this->userSession->setCurrentSessionId($cookieId);
Expand All @@ -65,78 +58,117 @@ public function log(string $action, int|string|null $data = null): void
$this->userSession->setCurrentSessionId(0);
}

$bots = $this->countBots();
$remoteAddress = $this->getRemoteAddress();
$banned = $this->isBanned($remoteAddress);

if (0 === $bots && false === $banned) {
$this->handleSession($cookieId, $remoteAddress, $action, $data);
}

return true;
}

private function initializeSessionId(): void
{
$sessionId = $this->request->query->get(UserSession::KEY_NAME_SESSION_ID);
if ($sessionId) {
$this->currentSessionId = $sessionId;
}
}

private function getCookieId(): ?int
{
return Filter::filterVar(
$this->request->query->get(UserSession::COOKIE_NAME_SESSION_ID),
FILTER_VALIDATE_INT
);
}

private function countBots(): int
{
$bots = 0;
foreach ($this->getBotIgnoreList() as $bot) {
if (Strings::strstr($this->request->headers->get('user-agent'), $bot)) {
if (Strings::strstr($this->getRequestHeaders()->get('user-agent') ?? 1, $bot)) {
++$bots;
}
}
return $bots;
}

// if we're running behind a reverse proxy like nginx/varnish, fix the client IP
public function getRemoteAddress(): string
{
$remoteAddress = $this->request->getClientIp();
$localAddresses = ['127.0.0.1', '::1'];

if (in_array($remoteAddress, $localAddresses) && $this->request->headers->has('X-Forwarded-For')) {
$remoteAddress = $this->request->headers->get('X-Forwarded-For');
if (in_array($remoteAddress, $localAddresses) && $this->getRequestHeaders()->has('X-Forwarded-For')) {
$remoteAddress = $this->getRequestHeaders()->get('X-Forwarded-For');
}

// clean up as well
$remoteAddress = preg_replace('([^0-9a-z:.]+)i', '', (string) $remoteAddress);

// Anonymize IP address
$remoteAddress = IpUtils::anonymize($remoteAddress);
return preg_replace('([^0-9a-z:.]+)i', '', (string)$remoteAddress);
}

private function isBanned(string $remoteAddress): bool
{
$network = new Network($this->configuration);
if ($network->isBanned($remoteAddress)) {
$banned = true;
}
return $network->isBanned(IpUtils::anonymize($remoteAddress));
}

if (0 === $bots && false === $banned) {
if ($this->currentSessionId === null) {
$this->currentSessionId = $this->configuration->getDb()->nextId(
Database::getTablePrefix() . 'faqsessions',
'sid'
);
// Check: force the session cookie to contains the current $sid
if (!is_null($cookieId) && (!$cookieId != $this->userSession->getCurrentSessionId())) {
$this->userSession->setCookie(
UserSession::COOKIE_NAME_SESSION_ID,
$this->userSession->getCurrentSessionId()
);
}

$query = sprintf(
"INSERT INTO %sfaqsessions (sid, user_id, ip, time) VALUES (%d, %d, '%s', %d)",
Database::getTablePrefix(),
$this->userSession->getCurrentSessionId(),
CurrentUser::getCurrentUser($this->configuration)->getUserId(),
$remoteAddress,
$this->request->server->get('REQUEST_TIME')
/**
* @throws Exception
*/
private function handleSession(?int $cookieId, string $remoteAddress, string $action, int|string|null $data): void
{
if ($this->currentSessionId === null) {
$this->currentSessionId = $this->configuration->getDb()->nextId(
Database::getTablePrefix() . 'faqsessions',
'sid'
);

if (!is_null($cookieId) && (!$cookieId != $this->userSession->getCurrentSessionId())) {
$this->userSession->setCookie(
UserSession::COOKIE_NAME_SESSION_ID,
$this->userSession->getCurrentSessionId()
);

$this->configuration->getDb()->query($query);
}

$data = $this->userSession->getCurrentSessionId() . ';' .
str_replace(';', ',', $action) . ';' .
$data . ';' .
$remoteAddress . ';' .
str_replace(';', ',', $this->request->server->get('QUERY_STRING') ?? '') . ';' .
str_replace(';', ',', $this->request->server->get('HTTP_REFERER') ?? '') . ';' .
str_replace(';', ',', urldecode((string) $this->request->server->get('HTTP_USER_AGENT'))) . ';' .
$this->request->server->get('REQUEST_TIME') . ";\n";
$query = sprintf(
"INSERT INTO %sfaqsessions (sid, user_id, ip, time) VALUES (%d, %d, '%s', %d)",
Database::getTablePrefix(),
$this->userSession->getCurrentSessionId(),
CurrentUser::getCurrentUser($this->configuration)->getUserId(),
$remoteAddress,
$this->request->server->get('REQUEST_TIME')
);

$file = PMF_ROOT_DIR . '/content/core/data/tracking' . date('dmY');
$this->configuration->getDb()->query($query);
}

if (!is_file($file)) {
touch($file);
}
$this->writeTrackingData($action, $data, $remoteAddress);
}

if (!is_writable($file)) {
$this->configuration->getLogger()->error('Cannot write to ' . $file);
}
private function writeTrackingData(string $action, int|string|null $data, string $remoteAddress): void
{
$data = $this->userSession->getCurrentSessionId() . ';' .
str_replace(';', ',', $action) . ';' .
$data . ';' .
$remoteAddress . ';' .
str_replace(';', ',', $this->request->server->get('QUERY_STRING') ?? '') . ';' .
str_replace(';', ',', $this->request->server->get('HTTP_REFERER') ?? '') . ';' .
str_replace(';', ',', urldecode((string) $this->request->server->get('HTTP_USER_AGENT'))) . ';' .
$this->request->server->get('REQUEST_TIME') . ";\n";

$file = PMF_ROOT_DIR . '/content/core/data/tracking' . date('dmY');

if (!is_file($file)) {
touch($file);
}

file_put_contents($file, $data, FILE_APPEND | LOCK_EX);
if (!is_writable($file)) {
$this->configuration->getLogger()->error('Cannot write to ' . $file);
}

file_put_contents($file, $data, FILE_APPEND | LOCK_EX);
}

/**
Expand All @@ -147,4 +179,9 @@ private function getBotIgnoreList(): array
{
return explode(',', (string) $this->configuration->get('main.botIgnoreList'));
}

private function getRequestHeaders(): HeaderBag
{
return $this->request->headers;
}
}
89 changes: 89 additions & 0 deletions tests/phpMyFAQ/User/TrackingTest.php
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
<?php

namespace phpMyFAQ\User;

use phpMyFAQ\Configuration;
use phpMyFAQ\Database\DatabaseDriver;
use PHPUnit\Framework\MockObject\Exception;
use PHPUnit\Framework\TestCase;
use ReflectionClass;
use Symfony\Component\HttpFoundation\HeaderBag;
use Symfony\Component\HttpFoundation\InputBag;
use Symfony\Component\HttpFoundation\Request;

class TrackingTest extends TestCase
{
private Configuration $configurationMock;
private Request $requestMock;
private UserSession $userSessionMock;
private DatabaseDriver $databaseMock;
private Tracking $tracking;

/**
* @throws Exception
*/
protected function setUp(): void
{
$this->configurationMock = $this->createMock(Configuration::class);
$this->requestMock = $this->createMock(Request::class);
$this->requestMock->headers = new HeaderBag([
'X-Forwarded-For' => '192.168.1.1'
]);
$this->requestMock->method('getClientIp')->willReturn('127.0.0.1');
$this->userSessionMock = $this->createMock(UserSession::class);
$this->databaseMock = $this->createMock(DatabaseDriver::class);

$this->configurationMock->method('getDb')->willReturn($this->databaseMock);
$this->tracking = Tracking::getInstance($this->configurationMock, $this->requestMock, $this->userSessionMock);
}

public function testInitializeSessionId(): void
{
$this->requestMock->query = new InputBag([
UserSession::KEY_NAME_SESSION_ID => 123,
UserSession::COOKIE_NAME_SESSION_ID => 456
]);

$reflection = new ReflectionClass($this->tracking);
$method = $reflection->getMethod('initializeSessionId');
$method->setAccessible(true);
$method->invoke($this->tracking);

$property = $reflection->getProperty('currentSessionId');
$property->setAccessible(true);
$this->assertEquals(123, $property->getValue($this->tracking));
}

public function testGetCookieId(): void
{
$this->requestMock->query = new InputBag([
UserSession::KEY_NAME_SESSION_ID => 123,
UserSession::COOKIE_NAME_SESSION_ID => 456
]);

$reflection = new ReflectionClass($this->tracking);
$method = $reflection->getMethod('getCookieId');
$method->setAccessible(true);
$result = $method->invoke($this->tracking);

$this->assertEquals(456, $result);
}

/**
* @throws \ReflectionException
*/
public function testCountBots(): void
{
$this->configurationMock->method('get')->with('main.botIgnoreList')->willReturn('bot1,bot2');
$this->requestMock->headers = new HeaderBag([
'user-agent' => 'bot1'
]);

$reflection = new ReflectionClass($this->tracking);
$method = $reflection->getMethod('countBots');
$method->setAccessible(true);
$result = $method->invoke($this->tracking);

$this->assertEquals(1, $result);
}
}

0 comments on commit a902e7d

Please sign in to comment.