Skip to content

Commit

Permalink
Merge pull request #195 from acelaya/feature/gdpr
Browse files Browse the repository at this point in the history
Fix GDPR compliance
  • Loading branch information
acelaya authored Sep 14, 2018
2 parents c32e205 + 9d9b61c commit 5b9784c
Show file tree
Hide file tree
Showing 18 changed files with 272 additions and 102 deletions.
1 change: 1 addition & 0 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
74 changes: 74 additions & 0 deletions data/migrations/Version20180913205455.php
Original file line number Diff line number Diff line change
@@ -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
}
}
20 changes: 14 additions & 6 deletions docs/swagger/definitions/Visit.json
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
}
26 changes: 26 additions & 0 deletions docs/swagger/definitions/VisitLocation.json
Original file line number Diff line number Diff line change
@@ -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"
}
}
}
20 changes: 14 additions & 6 deletions docs/swagger/paths/v1_short-codes_{shortCode}_visits.json
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
]
}
Expand Down
6 changes: 5 additions & 1 deletion module/CLI/src/Command/Install/InstallCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,11 @@ private function runPhpCommand($command, $errorMessage, OutputInterface $output)
$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>');
Expand Down
9 changes: 6 additions & 3 deletions module/CLI/src/Command/Shortcode/GetVisitsCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -85,16 +85,19 @@ public function execute(InputInterface $input, OutputInterface $output)
$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);
}

Expand Down
4 changes: 2 additions & 2 deletions module/CLI/src/Command/Visit/ProcessVisitsCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

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;
Expand All @@ -15,7 +16,6 @@

class ProcessVisitsCommand extends Command
{
private const LOCALHOST = '127.0.0.1';
public const NAME = 'visit:process';

/**
Expand Down Expand Up @@ -59,7 +59,7 @@ public function execute(InputInterface $input, OutputInterface $output)
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'))
);
Expand Down
9 changes: 5 additions & 4 deletions module/CLI/test/Command/Shortcode/GetVisitsCommandTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
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;
Expand Down Expand Up @@ -77,7 +78,7 @@ public function outputIsProperlyGenerated()
$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);

Expand All @@ -86,8 +87,8 @@ public function outputIsProperlyGenerated()
'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'));
}
}
12 changes: 6 additions & 6 deletions module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,9 @@ public function allReturnedVisitsIpsAreProcessed()
'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'));
}

/**
Expand All @@ -87,15 +87,15 @@ public function localhostAddressIsIgnored()
$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'));
}

/**
Expand Down
4 changes: 2 additions & 2 deletions module/Common/src/Exception/WrongIpException.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@

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);
}
}
74 changes: 74 additions & 0 deletions module/Common/src/Util/IpAddress.php
Original file line number Diff line number Diff line change
@@ -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,
]);
}
}
Loading

0 comments on commit 5b9784c

Please sign in to comment.