diff --git a/module/Core/src/Entity/Visit.php b/module/Core/src/Entity/Visit.php index e8cbb119..fb5e3bfb 100644 --- a/module/Core/src/Entity/Visit.php +++ b/module/Core/src/Entity/Visit.php @@ -21,19 +21,19 @@ class Visit extends AbstractEntity implements JsonSerializable private ShortUrl $shortUrl; private ?VisitLocation $visitLocation = null; - public function __construct(ShortUrl $shortUrl, Visitor $visitor, ?Chronos $date = null) + public function __construct(ShortUrl $shortUrl, Visitor $visitor, bool $obfuscate = true, ?Chronos $date = null) { $this->shortUrl = $shortUrl; $this->date = $date ?? Chronos::now(); $this->userAgent = $visitor->getUserAgent(); $this->referer = $visitor->getReferer(); - $this->remoteAddr = $this->obfuscateAddress($visitor->getRemoteAddress()); + $this->remoteAddr = $this->processAddress($obfuscate, $visitor->getRemoteAddress()); } - private function obfuscateAddress(?string $address): ?string + private function processAddress(bool $obfuscate, ?string $address): ?string { // Localhost addresses do not need to be obfuscated - if ($address === null || $address === IpAddress::LOCALHOST) { + if (! $obfuscate || $address === null || $address === IpAddress::LOCALHOST) { return $address; } diff --git a/module/Core/test-db/Repository/VisitRepositoryTest.php b/module/Core/test-db/Repository/VisitRepositoryTest.php index 034b15f9..13fc8581 100644 --- a/module/Core/test-db/Repository/VisitRepositoryTest.php +++ b/module/Core/test-db/Repository/VisitRepositoryTest.php @@ -139,13 +139,19 @@ class VisitRepositoryTest extends DatabaseTestCase $this->getEntityManager()->persist($shortUrlWithDomain); for ($i = 0; $i < 6; $i++) { - $visit = new Visit($shortUrl, Visitor::emptyInstance(), Chronos::parse(sprintf('2016-01-0%s', $i + 1))); + $visit = new Visit( + $shortUrl, + Visitor::emptyInstance(), + true, + Chronos::parse(sprintf('2016-01-0%s', $i + 1)), + ); $this->getEntityManager()->persist($visit); } for ($i = 0; $i < 3; $i++) { $visit = new Visit( $shortUrlWithDomain, Visitor::emptyInstance(), + true, Chronos::parse(sprintf('2016-01-0%s', $i + 1)), ); $this->getEntityManager()->persist($visit); diff --git a/module/Core/test/Entity/VisitTest.php b/module/Core/test/Entity/VisitTest.php index 9af71a09..a82e1939 100644 --- a/module/Core/test/Entity/VisitTest.php +++ b/module/Core/test/Entity/VisitTest.php @@ -6,6 +6,7 @@ namespace ShlinkioTest\Shlink\Core\Entity; use Cake\Chronos\Chronos; use PHPUnit\Framework\TestCase; +use Shlinkio\Shlink\Common\Util\IpAddress; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Model\Visitor; @@ -18,7 +19,7 @@ class VisitTest extends TestCase */ public function isProperlyJsonSerialized(?Chronos $date): void { - $visit = new Visit(new ShortUrl(''), new Visitor('Chrome', 'some site', '1.2.3.4'), $date); + $visit = new Visit(new ShortUrl(''), new Visitor('Chrome', 'some site', '1.2.3.4'), true, $date); $this->assertEquals([ 'referer' => 'some site', @@ -33,4 +34,25 @@ class VisitTest extends TestCase yield 'null date' => [null]; yield 'not null date' => [Chronos::now()->subDays(10)]; } + + /** + * @test + * @dataProvider provideAddresses + */ + public function addressIsObfuscatedWhenRequested(bool $obfuscate, ?string $address, ?string $expectedAddress): void + { + $visit = new Visit(new ShortUrl(''), new Visitor('Chrome', 'some site', $address), $obfuscate); + + $this->assertEquals($expectedAddress, $visit->getRemoteAddr()); + } + + public function provideAddresses(): iterable + { + yield 'obfuscated null address' => [true, null, null]; + yield 'non-obfuscated null address' => [false, null, null]; + yield 'obfuscated localhost' => [true, IpAddress::LOCALHOST, IpAddress::LOCALHOST]; + yield 'non-obfuscated localhost' => [false, IpAddress::LOCALHOST, IpAddress::LOCALHOST]; + yield 'obfuscated regular address' => [true, '1.2.3.4', '1.2.3.0']; + yield 'non-obfuscated regular address' => [false, '1.2.3.4', '1.2.3.4']; + } }