Updated system to obfuscate IP addresses before persisting them

This commit is contained in:
Alejandro Celaya 2018-09-13 22:36:28 +02:00
parent c32e2053c3
commit a0c3b9412f
11 changed files with 119 additions and 76 deletions

View file

@ -5,6 +5,7 @@ namespace Shlinkio\Shlink\CLI\Command\Visit;
use Shlinkio\Shlink\Common\Exception\WrongIpException;
use Shlinkio\Shlink\Common\Service\IpLocationResolverInterface;
use Shlinkio\Shlink\Common\Util\IpAddress;
use Shlinkio\Shlink\Core\Entity\VisitLocation;
use Shlinkio\Shlink\Core\Service\VisitServiceInterface;
use Symfony\Component\Console\Command\Command;
@ -15,7 +16,6 @@ use Zend\I18n\Translator\TranslatorInterface;
class ProcessVisitsCommand extends Command
{
private const LOCALHOST = '127.0.0.1';
public const NAME = 'visit:process';
/**
@ -59,7 +59,7 @@ class ProcessVisitsCommand extends Command
foreach ($visits as $visit) {
$ipAddr = $visit->getRemoteAddr();
$io->write(\sprintf('%s <info>%s</info>', $this->translator->translate('Processing IP'), $ipAddr));
if ($ipAddr === self::LOCALHOST) {
if ($ipAddr === IpAddress::LOCALHOST) {
$io->writeln(
\sprintf(' (<comment>%s</comment>)', $this->translator->translate('Ignored localhost address'))
);

View file

@ -86,8 +86,8 @@ class GetVisitsCommandTest extends TestCase
'shortCode' => $shortCode,
]);
$output = $this->commandTester->getDisplay();
$this->assertTrue(strpos($output, 'foo') > 0);
$this->assertTrue(strpos($output, '1.2.3.4') > 0);
$this->assertTrue(strpos($output, 'bar') > 0);
$this->assertTrue(\strpos($output, 'foo') > 0);
$this->assertTrue(\strpos($output, '1.2.3.0') > 0);
$this->assertTrue(\strpos($output, 'bar') > 0);
}
}

View file

@ -67,9 +67,9 @@ class ProcessVisitsCommandTest extends TestCase
'command' => 'visit:process',
]);
$output = $this->commandTester->getDisplay();
$this->assertTrue(strpos($output, 'Processing IP 1.2.3.4') === 0);
$this->assertTrue(strpos($output, 'Processing IP 4.3.2.1') > 0);
$this->assertTrue(strpos($output, 'Processing IP 12.34.56.78') > 0);
$this->assertEquals(0, \strpos($output, 'Processing IP 1.2.3.0'));
$this->assertGreaterThan(0, \strpos($output, 'Processing IP 4.3.2.0'));
$this->assertGreaterThan(0, \strpos($output, 'Processing IP 12.34.56.0'));
}
/**
@ -87,15 +87,15 @@ class ProcessVisitsCommandTest extends TestCase
$this->visitService->getUnlocatedVisits()->willReturn($visits)
->shouldBeCalledTimes(1);
$this->visitService->saveVisit(Argument::any())->shouldBeCalledTimes(count($visits) - 2);
$this->visitService->saveVisit(Argument::any())->shouldBeCalledTimes(\count($visits) - 2);
$this->ipResolver->resolveIpLocation(Argument::any())->willReturn([])
->shouldBeCalledTimes(count($visits) - 2);
->shouldBeCalledTimes(\count($visits) - 2);
$this->commandTester->execute([
'command' => 'visit:process',
]);
$output = $this->commandTester->getDisplay();
$this->assertTrue(strpos($output, 'Ignored localhost address') > 0);
$this->assertGreaterThan(0, \strpos($output, 'Ignored localhost address'));
}
/**

View file

@ -5,8 +5,8 @@ namespace Shlinkio\Shlink\Common\Exception;
class WrongIpException extends RuntimeException
{
public static function fromIpAddress($ipAddress, \Throwable $prev = null)
public static function fromIpAddress($ipAddress, \Throwable $prev = null): self
{
return new self(sprintf('Provided IP "%s" is invalid', $ipAddress), 0, $prev);
return new self(\sprintf('Provided IP "%s" is invalid', $ipAddress), 0, $prev);
}
}

View file

@ -0,0 +1,74 @@
<?php
declare(strict_types=1);
namespace Shlinkio\Shlink\Common\Util;
use Shlinkio\Shlink\Common\Exception\WrongIpException;
final class IpAddress
{
private const IPV4_PARTS_COUNT = 4;
private const OBFUSCATED_OCTET = '0';
public const LOCALHOST = '127.0.0.1';
/**
* @var string
*/
private $firstOctet;
/**
* @var string
*/
private $secondOctet;
/**
* @var string
*/
private $thirdOctet;
/**
* @var string
*/
private $fourthOctet;
/**
* @var bool
*/
private $isLocalhost;
private function __construct()
{
}
/**
* @param string $address
* @return IpAddress
* @throws WrongIpException
*/
public static function fromString(string $address): self
{
$address = \trim($address);
$parts = \explode('.', $address);
if (\count($parts) !== self::IPV4_PARTS_COUNT) {
throw WrongIpException::fromIpAddress($address);
}
$instance = new self();
$instance->isLocalhost = $address === self::LOCALHOST;
[$instance->firstOctet, $instance->secondOctet, $instance->thirdOctet, $instance->fourthOctet] = $parts;
return $instance;
}
public function getObfuscatedCopy(): self
{
$copy = clone $this;
$copy->fourthOctet = $this->isLocalhost ? $this->fourthOctet : self::OBFUSCATED_OCTET;
return $copy;
}
public function __toString(): string
{
return \implode('.', [
$this->firstOctet,
$this->secondOctet,
$this->thirdOctet,
$this->fourthOctet,
]);
}
}

View file

@ -5,6 +5,8 @@ namespace Shlinkio\Shlink\Core\Entity;
use Doctrine\ORM\Mapping as ORM;
use Shlinkio\Shlink\Common\Entity\AbstractEntity;
use Shlinkio\Shlink\Common\Exception\WrongIpException;
use Shlinkio\Shlink\Common\Util\IpAddress;
/**
* Class Visit
@ -54,109 +56,81 @@ class Visit extends AbstractEntity implements \JsonSerializable
$this->date = new \DateTime();
}
/**
* @return string
*/
public function getReferer(): string
{
return $this->referer;
}
/**
* @param string $referer
* @return $this
*/
public function setReferer($referer): self
public function setReferer(string $referer): self
{
$this->referer = $referer;
return $this;
}
/**
* @return \DateTime
*/
public function getDate(): \DateTime
{
return $this->date;
}
/**
* @param \DateTime $date
* @return $this
*/
public function setDate($date): self
public function setDate(\DateTime $date): self
{
$this->date = $date;
return $this;
}
/**
* @return ShortUrl
*/
public function getShortUrl(): ShortUrl
{
return $this->shortUrl;
}
/**
* @param ShortUrl $shortUrl
* @return $this
*/
public function setShortUrl($shortUrl): self
public function setShortUrl(ShortUrl $shortUrl): self
{
$this->shortUrl = $shortUrl;
return $this;
}
/**
* @return string
*/
public function getRemoteAddr(): string
{
return $this->remoteAddr;
}
/**
* @param string $remoteAddr
* @return $this
*/
public function setRemoteAddr($remoteAddr): self
public function setRemoteAddr(?string $remoteAddr): self
{
$this->remoteAddr = $remoteAddr;
// TODO Generate hash for the original remote address
$this->remoteAddr = $this->obfuscateAddress($remoteAddr);
return $this;
}
/**
* @return string
*/
private function obfuscateAddress(?string $address): ?string
{
if ($address === null) {
return null;
}
try {
return (string) IpAddress::fromString($address)->getObfuscatedCopy();
} catch (WrongIpException $e) {
return null;
}
}
public function getUserAgent(): string
{
return $this->userAgent;
}
/**
* @param string $userAgent
* @return $this
*/
public function setUserAgent($userAgent): self
public function setUserAgent(string $userAgent): self
{
$this->userAgent = $userAgent;
return $this;
}
/**
* @return VisitLocation
*/
public function getVisitLocation(): VisitLocation
{
return $this->visitLocation;
}
/**
* @param VisitLocation $visitLocation
* @return $this
*/
public function setVisitLocation($visitLocation): self
public function setVisitLocation(VisitLocation $visitLocation): self
{
$this->visitLocation = $visitLocation;
return $this;

View file

@ -31,7 +31,7 @@ class VisitsTracker implements VisitsTrackerInterface
* @throws ORM\ORMInvalidArgumentException
* @throws ORM\OptimisticLockException
*/
public function track($shortCode, ServerRequestInterface $request)
public function track($shortCode, ServerRequestInterface $request): void
{
/** @var ShortUrl $shortUrl */
$shortUrl = $this->em->getRepository(ShortUrl::class)->findOneBy([
@ -52,9 +52,8 @@ class VisitsTracker implements VisitsTrackerInterface
/**
* @param ServerRequestInterface $request
* @return string|null
*/
private function findOutRemoteAddr(ServerRequestInterface $request)
private function findOutRemoteAddr(ServerRequestInterface $request): ?string
{
$forwardedFor = $request->getHeaderLine('X-Forwarded-For');
if (empty($forwardedFor)) {
@ -62,7 +61,7 @@ class VisitsTracker implements VisitsTrackerInterface
return $serverParams['REMOTE_ADDR'] ?? null;
}
$ips = explode(',', $forwardedFor);
$ips = \explode(',', $forwardedFor);
return $ips[0] ?? null;
}

View file

@ -16,7 +16,7 @@ interface VisitsTrackerInterface
* @param string $shortCode
* @param ServerRequestInterface $request
*/
public function track($shortCode, ServerRequestInterface $request);
public function track($shortCode, ServerRequestInterface $request): void;
/**
* Returns the visits on certain short code

View file

@ -52,8 +52,7 @@ class PixelActionTest extends TestCase
$this->urlShortener->shortCodeToUrl($shortCode)->willReturn(
(new ShortUrl())->setLongUrl('http://domain.com/foo/bar')
)->shouldBeCalledTimes(1);
$this->visitTracker->track(Argument::cetera())->willReturn(null)
->shouldBeCalledTimes(1);
$this->visitTracker->track(Argument::cetera())->shouldBeCalledTimes(1);
$request = ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode);
$response = $this->action->process($request, TestUtils::createReqHandlerMock()->reveal());

View file

@ -55,8 +55,7 @@ class RedirectActionTest extends TestCase
$shortUrl = (new ShortUrl())->setLongUrl($expectedUrl);
$this->urlShortener->shortCodeToUrl($shortCode)->willReturn($shortUrl)
->shouldBeCalledTimes(1);
$this->visitTracker->track(Argument::cetera())->willReturn(null)
->shouldBeCalledTimes(1);
$this->visitTracker->track(Argument::cetera())->shouldBeCalledTimes(1);
$request = ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode);
$response = $this->action->process($request, TestUtils::createReqHandlerMock()->reveal());
@ -75,8 +74,7 @@ class RedirectActionTest extends TestCase
$shortCode = 'abc123';
$this->urlShortener->shortCodeToUrl($shortCode)->willThrow(EntityDoesNotExistException::class)
->shouldBeCalledTimes(1);
$this->visitTracker->track(Argument::cetera())->willReturn(null)
->shouldNotBeCalled();
$this->visitTracker->track(Argument::cetera())->shouldNotBeCalled();
$delegate = $this->prophesize(RequestHandlerInterface::class);
/** @var MethodProphecy $process */
@ -98,8 +96,7 @@ class RedirectActionTest extends TestCase
$shortUrl = (new ShortUrl())->setLongUrl($expectedUrl);
$this->urlShortener->shortCodeToUrl($shortCode)->willReturn($shortUrl)
->shouldBeCalledTimes(1);
$this->visitTracker->track(Argument::cetera())->willReturn(null)
->shouldNotBeCalled();
$this->visitTracker->track(Argument::cetera())->shouldNotBeCalled();
$request = ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode)
->withQueryParams(['foobar' => true]);

View file

@ -61,7 +61,7 @@ class VisitsTrackerTest extends TestCase
$this->em->persist(Argument::any())->will(function ($args) use ($test) {
/** @var Visit $visit */
$visit = $args[0];
$test->assertEquals('4.3.2.1', $visit->getRemoteAddr());
$test->assertEquals('4.3.2.0', $visit->getRemoteAddr());
})->shouldBeCalledTimes(1);
$this->em->flush(Argument::type(Visit::class))->shouldBeCalledTimes(1);