Merge pull request #195 from acelaya/feature/gdpr

Fix GDPR compliance
This commit is contained in:
Alejandro Celaya 2018-09-14 19:30:14 +02:00 committed by GitHub
commit 5b9784cd9e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
18 changed files with 272 additions and 102 deletions

View file

@ -14,6 +14,7 @@
"require": {
"php": "^7.1",
"ext-json": "*",
"ext-pdo": "*",
"acelaya/ze-content-based-error-handler": "^2.2",
"cocur/slugify": "^3.0",
"doctrine/cache": "^1.6",

View file

@ -0,0 +1,74 @@
<?php
declare(strict_types=1);
namespace ShlinkMigrations;
use Doctrine\DBAL\DBALException;
use Doctrine\DBAL\Schema\Schema;
use Doctrine\Migrations\AbstractMigration;
use Shlinkio\Shlink\Common\Exception\WrongIpException;
use Shlinkio\Shlink\Common\Util\IpAddress;
/**
* Auto-generated Migration: Please modify to your needs!
*/
final class Version20180913205455 extends AbstractMigration
{
/**
* @param Schema $schema
*/
public function up(Schema $schema): void
{
// Nothing to create
}
/**
* @param Schema $schema
* @throws DBALException
*/
public function postUp(Schema $schema): void
{
$qb = $this->connection->createQueryBuilder();
$qb->select('id', 'remote_addr')
->from('visits');
$st = $this->connection->executeQuery($qb->getSQL());
$qb = $this->connection->createQueryBuilder();
$qb->update('visits', 'v')
->set('v.remote_addr', ':obfuscatedAddr')
->where('v.id=:id');
while ($row = $st->fetch(\PDO::FETCH_ASSOC)) {
$addr = $row['remote_addr'] ?? null;
if ($addr === null) {
continue;
}
$qb->setParameters([
'id' => $row['id'],
'obfuscatedAddr' => $this->determineAddress((string) $addr),
])->execute();
}
}
private function determineAddress(string $addr): ?string
{
if ($addr === IpAddress::LOCALHOST) {
return $addr;
}
try {
return (string) IpAddress::fromString($addr)->getObfuscatedCopy();
} catch (WrongIpException $e) {
return null;
}
}
/**
* @param Schema $schema
*/
public function down(Schema $schema): void
{
// Nothing to rollback
}
}

View file

@ -2,17 +2,25 @@
"type": "object",
"properties": {
"referer": {
"type": "string"
"type": "string",
"description": "The origin from which the visit was performed"
},
"date": {
"type": "string",
"format": "date-time"
},
"remoteAddr": {
"type": "string"
"format": "date-time",
"description": "The date in which the visit was performed"
},
"userAgent": {
"type": "string"
"type": "string",
"description": "The user agent from which the visit was performed"
},
"visitLocation": {
"$ref": "./VisitLocation.json"
},
"remoteAddr": {
"type": "string",
"description": "This value is deprecated and will always be null",
"deprecated": true
}
}
}

View file

@ -0,0 +1,26 @@
{
"type": "object",
"properties": {
"cityName": {
"type": "string"
},
"countryCode": {
"type": "string"
},
"countryName": {
"type": "string"
},
"latitude": {
"type": "string"
},
"longitude": {
"type": "string"
},
"regionName": {
"type": "string"
},
"timezone": {
"type": "string"
}
}
}

View file

@ -70,20 +70,28 @@
{
"referer": "https://twitter.com",
"date": "2015-08-20T05:05:03+04:00",
"remoteAddr": "10.20.30.40",
"userAgent": "Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:47.0) Gecko/20100101 Firefox/47.0 Mozilla/5.0 (Macintosh; Intel Mac OS X x.y; rv:42.0) Gecko/20100101 Firefox/42.0"
"userAgent": "Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:47.0) Gecko/20100101 Firefox/47.0 Mozilla/5.0 (Macintosh; Intel Mac OS X x.y; rv:42.0) Gecko/20100101 Firefox/42.0",
"visitLocation": null
},
{
"referer": "https://t.co",
"date": "2015-08-20T05:05:03+04:00",
"remoteAddr": "11.22.33.44",
"userAgent": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.103 Safari/537.36"
"userAgent": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.103 Safari/537.36",
"visitLocation": {
"cityName": "Cupertino",
"countryCode": "US",
"countryName": "United States",
"latitude": "37.3042",
"longitude": "-122.0946",
"regionName": "California",
"timezone": "America/Los_Angeles"
}
},
{
"referer": null,
"date": "2015-08-20T05:05:03+04:00",
"remoteAddr": "110.220.5.6",
"userAgent": "some_web_crawler/1.4"
"userAgent": "some_web_crawler/1.4",
"visitLocation": null
}
]
}

View file

@ -235,7 +235,11 @@ class InstallCommand extends Command
$this->phpBinary = $this->phpFinder->find(false) ?: 'php';
}
$this->io->writeln('Running "' . sprintf('%s %s', $this->phpBinary, $command) . '"');
$this->io->write(
' <options=bold>[Running "' . sprintf('%s %s', $this->phpBinary, $command) . '"]</> ',
false,
OutputInterface::VERBOSITY_VERBOSE
);
$process = $this->processHelper->run($output, sprintf('%s %s', $this->phpBinary, $command));
if ($process->isSuccessful()) {
$this->io->writeln(' <info>Success!</info>');

View file

@ -85,16 +85,19 @@ class GetVisitsCommand extends Command
$rows = [];
foreach ($visits as $row) {
$rowData = $row->jsonSerialize();
// Unset location info
unset($rowData['visitLocation']);
// Unset location info and remote addr
unset($rowData['visitLocation'], $rowData['remoteAddr']);
$rowData['country'] = $row->getVisitLocation()->getCountryName();
$rows[] = \array_values($rowData);
}
$io->table([
$this->translator->translate('Referer'),
$this->translator->translate('Date'),
$this->translator->translate('Remote Address'),
$this->translator->translate('User agent'),
$this->translator->translate('Country'),
], $rows);
}

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

@ -9,6 +9,7 @@ use Prophecy\Prophecy\ObjectProphecy;
use Shlinkio\Shlink\CLI\Command\Shortcode\GetVisitsCommand;
use Shlinkio\Shlink\Common\Util\DateRange;
use Shlinkio\Shlink\Core\Entity\Visit;
use Shlinkio\Shlink\Core\Entity\VisitLocation;
use Shlinkio\Shlink\Core\Service\VisitsTrackerInterface;
use Symfony\Component\Console\Application;
use Symfony\Component\Console\Tester\CommandTester;
@ -77,7 +78,7 @@ class GetVisitsCommandTest extends TestCase
$shortCode = 'abc123';
$this->visitsTracker->info($shortCode, Argument::any())->willReturn([
(new Visit())->setReferer('foo')
->setRemoteAddr('1.2.3.4')
->setVisitLocation((new VisitLocation())->setCountryName('Spain'))
->setUserAgent('bar'),
])->shouldBeCalledTimes(1);
@ -86,8 +87,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->assertGreaterThan(0, \strpos($output, 'foo'));
$this->assertGreaterThan(0, \strpos($output, 'Spain'));
$this->assertGreaterThan(0, \strpos($output, 'bar'));
}
}

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;
private function __construct(string $firstOctet, string $secondOctet, string $thirdOctet, string $fourthOctet)
{
$this->firstOctet = $firstOctet;
$this->secondOctet = $secondOctet;
$this->thirdOctet = $thirdOctet;
$this->fourthOctet = $fourthOctet;
}
/**
* @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);
}
return new self(...$parts);
}
public function getObfuscatedCopy(): self
{
return new self(
$this->firstOctet,
$this->secondOctet,
$this->thirdOctet,
self::OBFUSCATED_OCTET
);
}
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
@ -20,143 +22,115 @@ class Visit extends AbstractEntity implements \JsonSerializable
* @var string
* @ORM\Column(type="string", length=256, nullable=true)
*/
protected $referer;
private $referer;
/**
* @var \DateTime
* @ORM\Column(type="datetime", nullable=false)
*/
protected $date;
private $date;
/**
* @var string
* @var string|null
* @ORM\Column(type="string", length=256, name="remote_addr", nullable=true)
*/
protected $remoteAddr;
private $remoteAddr;
/**
* @var string
* @ORM\Column(type="string", length=256, name="user_agent", nullable=true)
*/
protected $userAgent;
private $userAgent;
/**
* @var ShortUrl
* @ORM\ManyToOne(targetEntity=ShortUrl::class)
* @ORM\JoinColumn(name="short_url_id", referencedColumnName="id")
*/
protected $shortUrl;
private $shortUrl;
/**
* @var VisitLocation
* @ORM\ManyToOne(targetEntity=VisitLocation::class, cascade={"persist"})
* @ORM\JoinColumn(name="visit_location_id", referencedColumnName="id", nullable=true)
*/
protected $visitLocation;
private $visitLocation;
public function __construct()
{
$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
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;
$this->remoteAddr = $this->obfuscateAddress($remoteAddr);
return $this;
}
/**
* @return string
*/
private function obfuscateAddress(?string $address): ?string
{
// Localhost addresses do not need to be obfuscated
if ($address === null || $address === IpAddress::LOCALHOST) {
return $address;
}
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;
@ -174,9 +148,11 @@ class Visit extends AbstractEntity implements \JsonSerializable
return [
'referer' => $this->referer,
'date' => isset($this->date) ? $this->date->format(\DateTime::ATOM) : null,
'remoteAddr' => $this->remoteAddr,
'userAgent' => $this->userAgent,
'visitLocation' => $this->visitLocation,
// Deprecated
'remoteAddr' => null,
];
}
}

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);