From 7da00fbc8cc2a8115b4f457924463f7a85e8650e Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 8 May 2020 12:58:49 +0200 Subject: [PATCH 1/7] Updated Visit entity so that the address can be optionally obfuscated --- module/Core/src/Entity/Visit.php | 8 +++---- .../Repository/VisitRepositoryTest.php | 8 ++++++- module/Core/test/Entity/VisitTest.php | 24 ++++++++++++++++++- 3 files changed, 34 insertions(+), 6 deletions(-) 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']; + } } From eac468514ba3e4a6d6909b328170514af630879a Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 8 May 2020 13:10:58 +0200 Subject: [PATCH 2/7] Allow to determine if remote addresses should be obfuscated at configuration level --- config/autoload/url-shortener.global.php | 1 + module/Core/config/dependencies.config.php | 6 +++++- module/Core/src/Service/VisitsTracker.php | 11 ++++++++--- module/Core/test/Service/VisitsTrackerTest.php | 2 +- 4 files changed, 15 insertions(+), 5 deletions(-) diff --git a/config/autoload/url-shortener.global.php b/config/autoload/url-shortener.global.php index 165e0258..8439cc8a 100644 --- a/config/autoload/url-shortener.global.php +++ b/config/autoload/url-shortener.global.php @@ -12,6 +12,7 @@ return [ 'hostname' => '', ], 'validate_url' => false, + 'obfuscate_remote_addr' => true, 'visits_webhooks' => [], 'default_short_codes_length' => DEFAULT_SHORT_CODES_LENGTH, ], diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 5db524b8..90a4ffaf 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -54,7 +54,11 @@ return [ Options\UrlShortenerOptions::class => ['config.url_shortener'], Service\UrlShortener::class => [Util\UrlValidator::class, 'em', Resolver\PersistenceDomainResolver::class], - Service\VisitsTracker::class => ['em', EventDispatcherInterface::class], + Service\VisitsTracker::class => [ + 'em', + EventDispatcherInterface::class, + 'config.url_shortener.obfuscate_remote_addr', + ], Service\ShortUrlService::class => ['em', Service\ShortUrl\ShortUrlResolver::class, Util\UrlValidator::class], Visit\VisitLocator::class => ['em'], Visit\VisitsStatsHelper::class => ['em'], diff --git a/module/Core/src/Service/VisitsTracker.php b/module/Core/src/Service/VisitsTracker.php index f477681a..a60513e4 100644 --- a/module/Core/src/Service/VisitsTracker.php +++ b/module/Core/src/Service/VisitsTracker.php @@ -22,11 +22,16 @@ class VisitsTracker implements VisitsTrackerInterface { private ORM\EntityManagerInterface $em; private EventDispatcherInterface $eventDispatcher; + private bool $obfuscateRemoteAddr; - public function __construct(ORM\EntityManagerInterface $em, EventDispatcherInterface $eventDispatcher) - { + public function __construct( + ORM\EntityManagerInterface $em, + EventDispatcherInterface $eventDispatcher, + bool $obfuscateRemoteAddr + ) { $this->em = $em; $this->eventDispatcher = $eventDispatcher; + $this->obfuscateRemoteAddr = $obfuscateRemoteAddr; } /** @@ -34,7 +39,7 @@ class VisitsTracker implements VisitsTrackerInterface */ public function track(ShortUrl $shortUrl, Visitor $visitor): void { - $visit = new Visit($shortUrl, $visitor); + $visit = new Visit($shortUrl, $visitor, $this->obfuscateRemoteAddr); $this->em->persist($visit); $this->em->flush(); diff --git a/module/Core/test/Service/VisitsTrackerTest.php b/module/Core/test/Service/VisitsTrackerTest.php index 9028d2c7..6ae5acf6 100644 --- a/module/Core/test/Service/VisitsTrackerTest.php +++ b/module/Core/test/Service/VisitsTrackerTest.php @@ -37,7 +37,7 @@ class VisitsTrackerTest extends TestCase $this->em = $this->prophesize(EntityManager::class); $this->eventDispatcher = $this->prophesize(EventDispatcherInterface::class); - $this->visitsTracker = new VisitsTracker($this->em->reveal(), $this->eventDispatcher->reveal()); + $this->visitsTracker = new VisitsTracker($this->em->reveal(), $this->eventDispatcher->reveal(), true); } /** @test */ From ba13d99a71a78b9de839f0f0abed3cc1b19a686f Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 8 May 2020 13:19:40 +0200 Subject: [PATCH 3/7] Allowed remote addr obfuscation to be configured on docker image by using the OBFUSCATE_REMOTE_ADDR env var --- docker/README.md | 8 +++++--- docker/config/shlink_in_docker.local.php | 1 + 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/docker/README.md b/docker/README.md index aa9ae16b..a2283710 100644 --- a/docker/README.md +++ b/docker/README.md @@ -168,12 +168,12 @@ This is the complete list of supported env vars: * `TASK_WORKER_NUM`: The amount of concurrent background tasks this shlink instance will be able to execute. Defaults to 16. * `VISITS_WEBHOOKS`: A comma-separated list of URLs that will receive a `POST` request when a short URL receives a visit. * `DEFAULT_SHORT_CODES_LENGTH`: The length you want generated short codes to have. It defaults to 5 and has to be at least 4, so any value smaller than that will fall back to 4. +* `GEOLITE_LICENSE_KEY`: The license key used to download new GeoLite2 database files. This is not mandatory, as a default license key is provided, but it is **strongly recommended** that you provide your own. Go to [https://shlink.io/documentation/geolite-license-key](https://shlink.io/documentation/geolite-license-key) to know how to generate it. * `REDIS_SERVERS`: A comma-separated list of redis servers where Shlink locks are stored (locks are used to prevent some operations to be run more than once in parallel). * `MERCURE_PUBLIC_HUB_URL`: The public URL of a mercure hub server to which Shlink will sent updates. This URL will also be served to consumers that want to subscribe to those updates. * `MERCURE_INTERNAL_HUB_URL`: An internal URL for a mercure hub. Will be used only when publishing updates to mercure, and does not need to be public. If this is not provided but `MERCURE_PUBLIC_HUB_URL` was, the former one will be used to publish updates. * `MERCURE_JWT_SECRET`: The secret key that was provided to the mercure hub server, in order to be able to generate valid JWTs for publishing/subscribing to that server. - -* `GEOLITE_LICENSE_KEY`: The license key used to download new GeoLite2 database files. This is not mandatory, as a default license key is provided, but it is **strongly recommended** that you provide your own. Go to [https://shlink.io/documentation/geolite-license-key](https://shlink.io/documentation/geolite-license-key) to know how to generate it. +* `OBFUSCATE_REMOTE_ADDR`: Tells if IP addresses from visitors should be obfuscated before storing them in the database. Default value is `true`. **Careful!** Setting this to `false` will make your Shlink instance no longer be in compliance with the GDPR and other similar laws. An example using all env vars could look like this: @@ -205,6 +205,7 @@ docker run \ -e "MERCURE_PUBLIC_HUB_URL=https://example.com" \ -e "MERCURE_INTERNAL_HUB_URL=http://my-mercure-hub.prod.svc.cluster.local" \ -e MERCURE_JWT_SECRET=super_secret_key \ + -e OBFUSCATE_REMOTE_ADDR=false \ shlinkio/shlink:stable ``` @@ -249,7 +250,8 @@ The whole configuration should have this format, but it can be split into multip "geolite_license_key": "kjh23ljkbndskj345", "mercure_public_hub_url": "https://example.com", "mercure_internal_hub_url": "http://my-mercure-hub.prod.svc.cluster.local", - "mercure_jwt_secret": "super_secret_key" + "mercure_jwt_secret": "super_secret_key", + "obfuscate_remote_addr": false } ``` diff --git a/docker/config/shlink_in_docker.local.php b/docker/config/shlink_in_docker.local.php index 5662aaee..d85d0e79 100644 --- a/docker/config/shlink_in_docker.local.php +++ b/docker/config/shlink_in_docker.local.php @@ -117,6 +117,7 @@ return [ 'hostname' => env('SHORT_DOMAIN_HOST', ''), ], 'validate_url' => (bool) env('VALIDATE_URLS', false), + 'obfuscate_remote_addr' => (bool) env('OBFUSCATE_REMOTE_ADDR', true), 'visits_webhooks' => $helper->getVisitsWebhooks(), 'default_short_codes_length' => $helper->getDefaultShortCodesLength(), ], From bfdd6e0c508590b5743be0da36b4395f9b989283 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 8 May 2020 13:21:49 +0200 Subject: [PATCH 4/7] Ensured SimplifiedConfigParser properly handles obfuscate_remote_addr option --- module/Core/src/Config/SimplifiedConfigParser.php | 1 + module/Core/test/Config/SimplifiedConfigParserTest.php | 2 ++ 2 files changed, 3 insertions(+) diff --git a/module/Core/src/Config/SimplifiedConfigParser.php b/module/Core/src/Config/SimplifiedConfigParser.php index fe4a4192..d3c22118 100644 --- a/module/Core/src/Config/SimplifiedConfigParser.php +++ b/module/Core/src/Config/SimplifiedConfigParser.php @@ -37,6 +37,7 @@ class SimplifiedConfigParser 'mercure_public_hub_url' => ['mercure', 'public_hub_url'], 'mercure_internal_hub_url' => ['mercure', 'internal_hub_url'], 'mercure_jwt_secret' => ['mercure', 'jwt_secret'], + 'obfuscate_remote_addr' => ['url_shortener', 'obfuscate_remote_addr'], ]; private const SIMPLIFIED_CONFIG_SIDE_EFFECTS = [ 'delete_short_url_threshold' => [ diff --git a/module/Core/test/Config/SimplifiedConfigParserTest.php b/module/Core/test/Config/SimplifiedConfigParserTest.php index 6bc6f1f7..94eb4116 100644 --- a/module/Core/test/Config/SimplifiedConfigParserTest.php +++ b/module/Core/test/Config/SimplifiedConfigParserTest.php @@ -64,6 +64,7 @@ class SimplifiedConfigParserTest extends TestCase 'mercure_public_hub_url' => 'public_url', 'mercure_internal_hub_url' => 'internal_url', 'mercure_jwt_secret' => 'super_secret_value', + 'obfuscate_remote_addr' => false, ]; $expected = [ 'app_options' => [ @@ -92,6 +93,7 @@ class SimplifiedConfigParserTest extends TestCase 'https://third-party.io/foo', ], 'default_short_codes_length' => 8, + 'obfuscate_remote_addr' => false, ], 'delete_short_urls' => [ From 8f06e4b20fb32085804da8c1c943823d2fb27528 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 8 May 2020 15:43:09 +0200 Subject: [PATCH 5/7] Replaced references to obfuscate by anonymize --- config/autoload/url-shortener.global.php | 2 +- docker/README.md | 6 +++--- docker/config/shlink_in_docker.local.php | 2 +- module/Core/config/dependencies.config.php | 2 +- .../src/Config/SimplifiedConfigParser.php | 2 +- module/Core/src/Entity/Visit.php | 10 +++++----- module/Core/src/Service/VisitsTracker.php | 8 ++++---- .../Config/SimplifiedConfigParserTest.php | 4 ++-- module/Core/test/Entity/VisitTest.php | 16 +++++++-------- .../Core/test/Service/VisitsTrackerTest.php | 20 ------------------- 10 files changed, 26 insertions(+), 46 deletions(-) diff --git a/config/autoload/url-shortener.global.php b/config/autoload/url-shortener.global.php index 8439cc8a..5ad66bea 100644 --- a/config/autoload/url-shortener.global.php +++ b/config/autoload/url-shortener.global.php @@ -12,7 +12,7 @@ return [ 'hostname' => '', ], 'validate_url' => false, - 'obfuscate_remote_addr' => true, + 'anonymize_remote_addr' => true, 'visits_webhooks' => [], 'default_short_codes_length' => DEFAULT_SHORT_CODES_LENGTH, ], diff --git a/docker/README.md b/docker/README.md index a2283710..e17e570e 100644 --- a/docker/README.md +++ b/docker/README.md @@ -173,7 +173,7 @@ This is the complete list of supported env vars: * `MERCURE_PUBLIC_HUB_URL`: The public URL of a mercure hub server to which Shlink will sent updates. This URL will also be served to consumers that want to subscribe to those updates. * `MERCURE_INTERNAL_HUB_URL`: An internal URL for a mercure hub. Will be used only when publishing updates to mercure, and does not need to be public. If this is not provided but `MERCURE_PUBLIC_HUB_URL` was, the former one will be used to publish updates. * `MERCURE_JWT_SECRET`: The secret key that was provided to the mercure hub server, in order to be able to generate valid JWTs for publishing/subscribing to that server. -* `OBFUSCATE_REMOTE_ADDR`: Tells if IP addresses from visitors should be obfuscated before storing them in the database. Default value is `true`. **Careful!** Setting this to `false` will make your Shlink instance no longer be in compliance with the GDPR and other similar laws. +* `ANONYMIZE_REMOTE_ADDR`: Tells if IP addresses from visitors should be obfuscated before storing them in the database. Default value is `true`. **Careful!** Setting this to `false` will make your Shlink instance no longer be in compliance with the GDPR and other similar data protection regulations. An example using all env vars could look like this: @@ -205,7 +205,7 @@ docker run \ -e "MERCURE_PUBLIC_HUB_URL=https://example.com" \ -e "MERCURE_INTERNAL_HUB_URL=http://my-mercure-hub.prod.svc.cluster.local" \ -e MERCURE_JWT_SECRET=super_secret_key \ - -e OBFUSCATE_REMOTE_ADDR=false \ + -e ANONYMIZE_REMOTE_ADDR=false \ shlinkio/shlink:stable ``` @@ -251,7 +251,7 @@ The whole configuration should have this format, but it can be split into multip "mercure_public_hub_url": "https://example.com", "mercure_internal_hub_url": "http://my-mercure-hub.prod.svc.cluster.local", "mercure_jwt_secret": "super_secret_key", - "obfuscate_remote_addr": false + "anonymize_remote_addr": false } ``` diff --git a/docker/config/shlink_in_docker.local.php b/docker/config/shlink_in_docker.local.php index d85d0e79..b870ccd7 100644 --- a/docker/config/shlink_in_docker.local.php +++ b/docker/config/shlink_in_docker.local.php @@ -117,7 +117,7 @@ return [ 'hostname' => env('SHORT_DOMAIN_HOST', ''), ], 'validate_url' => (bool) env('VALIDATE_URLS', false), - 'obfuscate_remote_addr' => (bool) env('OBFUSCATE_REMOTE_ADDR', true), + 'anonymize_remote_addr' => (bool) env('ANONYMIZE_REMOTE_ADDR', true), 'visits_webhooks' => $helper->getVisitsWebhooks(), 'default_short_codes_length' => $helper->getDefaultShortCodesLength(), ], diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 90a4ffaf..debf021f 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -57,7 +57,7 @@ return [ Service\VisitsTracker::class => [ 'em', EventDispatcherInterface::class, - 'config.url_shortener.obfuscate_remote_addr', + 'config.url_shortener.anonymize_remote_addr', ], Service\ShortUrlService::class => ['em', Service\ShortUrl\ShortUrlResolver::class, Util\UrlValidator::class], Visit\VisitLocator::class => ['em'], diff --git a/module/Core/src/Config/SimplifiedConfigParser.php b/module/Core/src/Config/SimplifiedConfigParser.php index d3c22118..81f05d14 100644 --- a/module/Core/src/Config/SimplifiedConfigParser.php +++ b/module/Core/src/Config/SimplifiedConfigParser.php @@ -37,7 +37,7 @@ class SimplifiedConfigParser 'mercure_public_hub_url' => ['mercure', 'public_hub_url'], 'mercure_internal_hub_url' => ['mercure', 'internal_hub_url'], 'mercure_jwt_secret' => ['mercure', 'jwt_secret'], - 'obfuscate_remote_addr' => ['url_shortener', 'obfuscate_remote_addr'], + 'anonymize_remote_addr' => ['url_shortener', 'anonymize_remote_addr'], ]; private const SIMPLIFIED_CONFIG_SIDE_EFFECTS = [ 'delete_short_url_threshold' => [ diff --git a/module/Core/src/Entity/Visit.php b/module/Core/src/Entity/Visit.php index fb5e3bfb..6a4f44e5 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, bool $obfuscate = true, ?Chronos $date = null) + public function __construct(ShortUrl $shortUrl, Visitor $visitor, bool $anonymize = true, ?Chronos $date = null) { $this->shortUrl = $shortUrl; $this->date = $date ?? Chronos::now(); $this->userAgent = $visitor->getUserAgent(); $this->referer = $visitor->getReferer(); - $this->remoteAddr = $this->processAddress($obfuscate, $visitor->getRemoteAddress()); + $this->remoteAddr = $this->processAddress($anonymize, $visitor->getRemoteAddress()); } - private function processAddress(bool $obfuscate, ?string $address): ?string + private function processAddress(bool $anonymize, ?string $address): ?string { - // Localhost addresses do not need to be obfuscated - if (! $obfuscate || $address === null || $address === IpAddress::LOCALHOST) { + // Localhost addresses do not need to be anonymized + if (! $anonymize || $address === null || $address === IpAddress::LOCALHOST) { return $address; } diff --git a/module/Core/src/Service/VisitsTracker.php b/module/Core/src/Service/VisitsTracker.php index a60513e4..39f74cae 100644 --- a/module/Core/src/Service/VisitsTracker.php +++ b/module/Core/src/Service/VisitsTracker.php @@ -22,16 +22,16 @@ class VisitsTracker implements VisitsTrackerInterface { private ORM\EntityManagerInterface $em; private EventDispatcherInterface $eventDispatcher; - private bool $obfuscateRemoteAddr; + private bool $anonymizeRemoteAddr; public function __construct( ORM\EntityManagerInterface $em, EventDispatcherInterface $eventDispatcher, - bool $obfuscateRemoteAddr + bool $anonymizeRemoteAddr ) { $this->em = $em; $this->eventDispatcher = $eventDispatcher; - $this->obfuscateRemoteAddr = $obfuscateRemoteAddr; + $this->anonymizeRemoteAddr = $anonymizeRemoteAddr; } /** @@ -39,7 +39,7 @@ class VisitsTracker implements VisitsTrackerInterface */ public function track(ShortUrl $shortUrl, Visitor $visitor): void { - $visit = new Visit($shortUrl, $visitor, $this->obfuscateRemoteAddr); + $visit = new Visit($shortUrl, $visitor, $this->anonymizeRemoteAddr); $this->em->persist($visit); $this->em->flush(); diff --git a/module/Core/test/Config/SimplifiedConfigParserTest.php b/module/Core/test/Config/SimplifiedConfigParserTest.php index 94eb4116..3700b042 100644 --- a/module/Core/test/Config/SimplifiedConfigParserTest.php +++ b/module/Core/test/Config/SimplifiedConfigParserTest.php @@ -64,7 +64,7 @@ class SimplifiedConfigParserTest extends TestCase 'mercure_public_hub_url' => 'public_url', 'mercure_internal_hub_url' => 'internal_url', 'mercure_jwt_secret' => 'super_secret_value', - 'obfuscate_remote_addr' => false, + 'anonymize_remote_addr' => false, ]; $expected = [ 'app_options' => [ @@ -93,7 +93,7 @@ class SimplifiedConfigParserTest extends TestCase 'https://third-party.io/foo', ], 'default_short_codes_length' => 8, - 'obfuscate_remote_addr' => false, + 'anonymize_remote_addr' => false, ], 'delete_short_urls' => [ diff --git a/module/Core/test/Entity/VisitTest.php b/module/Core/test/Entity/VisitTest.php index a82e1939..73e41f12 100644 --- a/module/Core/test/Entity/VisitTest.php +++ b/module/Core/test/Entity/VisitTest.php @@ -39,20 +39,20 @@ class VisitTest extends TestCase * @test * @dataProvider provideAddresses */ - public function addressIsObfuscatedWhenRequested(bool $obfuscate, ?string $address, ?string $expectedAddress): void + public function addressIsAnonymizedWhenRequested(bool $anonymize, ?string $address, ?string $expectedAddress): void { - $visit = new Visit(new ShortUrl(''), new Visitor('Chrome', 'some site', $address), $obfuscate); + $visit = new Visit(new ShortUrl(''), new Visitor('Chrome', 'some site', $address), $anonymize); $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']; + yield 'anonymized null address' => [true, null, null]; + yield 'non-anonymized null address' => [false, null, null]; + yield 'anonymized localhost' => [true, IpAddress::LOCALHOST, IpAddress::LOCALHOST]; + yield 'non-anonymized localhost' => [false, IpAddress::LOCALHOST, IpAddress::LOCALHOST]; + yield 'anonymized regular address' => [true, '1.2.3.4', '1.2.3.0']; + yield 'non-anonymized regular address' => [false, '1.2.3.4', '1.2.3.4']; } } diff --git a/module/Core/test/Service/VisitsTrackerTest.php b/module/Core/test/Service/VisitsTrackerTest.php index 6ae5acf6..0722352f 100644 --- a/module/Core/test/Service/VisitsTrackerTest.php +++ b/module/Core/test/Service/VisitsTrackerTest.php @@ -6,7 +6,6 @@ namespace ShlinkioTest\Shlink\Core\Service; use Doctrine\ORM\EntityManager; use Laminas\Stdlib\ArrayUtils; -use PHPUnit\Framework\Assert; use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; @@ -53,25 +52,6 @@ class VisitsTrackerTest extends TestCase $this->eventDispatcher->dispatch(Argument::type(ShortUrlVisited::class))->shouldHaveBeenCalled(); } - /** @test */ - public function trackedIpAddressGetsObfuscated(): void - { - $shortCode = '123ABC'; - - $this->em->persist(Argument::any())->will(function ($args) { - /** @var Visit $visit */ - $visit = $args[0]; - Assert::assertEquals('4.3.2.0', $visit->getRemoteAddr()); - $visit->setId('1'); - return $visit; - })->shouldBeCalledOnce(); - $this->em->flush()->shouldBeCalledOnce(); - - $this->visitsTracker->track(new ShortUrl($shortCode), new Visitor('', '', '4.3.2.1')); - - $this->eventDispatcher->dispatch(Argument::type(ShortUrlVisited::class))->shouldHaveBeenCalled(); - } - /** @test */ public function infoReturnsVisitsForCertainShortCode(): void { From f4bf3551f6a13cec6a47a4aeaa77e5762209684f Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 8 May 2020 15:50:16 +0200 Subject: [PATCH 6/7] Updated shlink-installer to a version supporting IP anonymization param --- composer.json | 2 +- config/autoload/installer.global.php | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 5ba58a8e..380cd237 100644 --- a/composer.json +++ b/composer.json @@ -51,7 +51,7 @@ "shlinkio/shlink-common": "dev-master#e659cf9d9b5b3b131419e2f55f2e595f562baafc as 3.1.0", "shlinkio/shlink-config": "^1.0", "shlinkio/shlink-event-dispatcher": "^1.4", - "shlinkio/shlink-installer": "dev-master#dae6644587d0c1c59ca773722531551b9f436786 as 5.0.0", + "shlinkio/shlink-installer": "dev-master#50be18de1e505d2609d96c6cc86571b1b1ca7b57 as 5.0.0", "shlinkio/shlink-ip-geolocation": "^1.4", "symfony/console": "^5.0", "symfony/filesystem": "^5.0", diff --git a/config/autoload/installer.global.php b/config/autoload/installer.global.php index d46aa8d7..db1914db 100644 --- a/config/autoload/installer.global.php +++ b/config/autoload/installer.global.php @@ -36,6 +36,7 @@ return [ Option\Mercure\MercureInternalUrlConfigOption::class, Option\Mercure\MercureJwtSecretConfigOption::class, Option\UrlShortener\GeoLiteLicenseKeyConfigOption::class, + Option\UrlShortener\IpAnonymizationConfigOption::class, ], 'installation_commands' => [ From e8ab664561c53e8feef7ee95aa650f791fff9c43 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 8 May 2020 15:52:35 +0200 Subject: [PATCH 7/7] Updated changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 49fc0a29..c4b6fd82 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this Now, if the `withStats=true` query param is provided, the response payload will include a new `stats` property which is a list with the amount of short URLs and visits for every tag. +* [#640](https://github.com/shlinkio/shlink/issues/640) Allowed to optionally disable visitors' IP address anonymization. This will make Shlink no longer be GDPR-compliant, but it's OK if you only plan to share your URLs in countries without this regulation. + #### Changed * [#692](https://github.com/shlinkio/shlink/issues/692) Drastically improved performance when loading visits. Specially noticeable when loading big result sets.