From cea50a860efae5576ca80164a84b56f18662a0ab Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 22 Mar 2020 10:01:34 +0100 Subject: [PATCH 1/6] Improved docker image generation --- .dockerignore | 7 ++----- Dockerfile | 5 +++-- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/.dockerignore b/.dockerignore index 4559d3e2..46b96529 100644 --- a/.dockerignore +++ b/.dockerignore @@ -15,11 +15,8 @@ vendor docs indocker docker-* -php* -.php* +phpstan.neon infection.json **/test* build* -.git* -.scrutinizer.yml -.travis.yml +**/.* diff --git a/Dockerfile b/Dockerfile index 1eb2715b..64cd7ebe 100644 --- a/Dockerfile +++ b/Dockerfile @@ -40,9 +40,10 @@ RUN wget https://download.microsoft.com/download/e/4/e/e4e67866-dffd-428c-aac7-8 FROM base as builder COPY . . COPY --from=composer:1.10.1 /usr/bin/composer ./composer.phar -RUN php composer.phar install --no-dev --optimize-autoloader --prefer-dist --no-progress --no-interaction && \ +RUN apk add --no-cache git && \ + php composer.phar install --no-dev --optimize-autoloader --prefer-dist --no-progress --no-interaction && \ php composer.phar clear-cache && \ - rm composer.* && \ + rm -r docker composer.* && \ sed -i "s/%SHLINK_VERSION%/${SHLINK_VERSION}/g" config/autoload/app_options.global.php From 3fef4b4a2826eb2084eaa9f7ada007645bc0c337 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 22 Mar 2020 10:48:27 +0100 Subject: [PATCH 2/6] Ensured non-obfuscated IP address is passed to event listener which geolocates it --- .../Core/src/EventDispatcher/LocateShortUrlVisit.php | 11 ++++++----- module/Core/src/EventDispatcher/ShortUrlVisited.php | 11 +++++++++-- module/Core/src/Service/VisitsTracker.php | 2 +- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/module/Core/src/EventDispatcher/LocateShortUrlVisit.php b/module/Core/src/EventDispatcher/LocateShortUrlVisit.php index a0f8d033..6abbe02b 100644 --- a/module/Core/src/EventDispatcher/LocateShortUrlVisit.php +++ b/module/Core/src/EventDispatcher/LocateShortUrlVisit.php @@ -53,7 +53,7 @@ class LocateShortUrlVisit } if ($this->downloadOrUpdateGeoLiteDb($visitId)) { - $this->locateVisit($visitId, $visit); + $this->locateVisit($visitId, $shortUrlVisited->originalIpAddress(), $visit); } $this->eventDispatcher->dispatch(new VisitLocated($visitId)); @@ -80,12 +80,13 @@ class LocateShortUrlVisit return true; } - private function locateVisit(string $visitId, Visit $visit): void + private function locateVisit(string $visitId, ?string $originalIpAddress, Visit $visit): void { + $isLocatable = $originalIpAddress !== null || $visit->isLocatable(); + $addr = $originalIpAddress ?? $visit->getRemoteAddr(); + try { - $location = $visit->isLocatable() - ? $this->ipLocationResolver->resolveIpLocation($visit->getRemoteAddr()) - : Location::emptyInstance(); + $location = $isLocatable ? $this->ipLocationResolver->resolveIpLocation($addr) : Location::emptyInstance(); $visit->locate(new VisitLocation($location)); $this->em->flush(); diff --git a/module/Core/src/EventDispatcher/ShortUrlVisited.php b/module/Core/src/EventDispatcher/ShortUrlVisited.php index 1f0b5b5c..c33f805a 100644 --- a/module/Core/src/EventDispatcher/ShortUrlVisited.php +++ b/module/Core/src/EventDispatcher/ShortUrlVisited.php @@ -9,10 +9,12 @@ use JsonSerializable; final class ShortUrlVisited implements JsonSerializable { private string $visitId; + private ?string $originalIpAddress; - public function __construct(string $visitId) + public function __construct(string $visitId, ?string $originalIpAddress = null) { $this->visitId = $visitId; + $this->originalIpAddress = $originalIpAddress; } public function visitId(): string @@ -20,8 +22,13 @@ final class ShortUrlVisited implements JsonSerializable return $this->visitId; } + public function originalIpAddress(): ?string + { + return $this->originalIpAddress; + } + public function jsonSerialize(): array { - return ['visitId' => $this->visitId]; + return ['visitId' => $this->visitId, 'originalIpAddress' => $this->originalIpAddress]; } } diff --git a/module/Core/src/Service/VisitsTracker.php b/module/Core/src/Service/VisitsTracker.php index 54abe319..f477681a 100644 --- a/module/Core/src/Service/VisitsTracker.php +++ b/module/Core/src/Service/VisitsTracker.php @@ -39,7 +39,7 @@ class VisitsTracker implements VisitsTrackerInterface $this->em->persist($visit); $this->em->flush(); - $this->eventDispatcher->dispatch(new ShortUrlVisited($visit->getId())); + $this->eventDispatcher->dispatch(new ShortUrlVisited($visit->getId(), $visitor->getRemoteAddress())); } /** From fdd8efc12d4a6ff8aae3bfb39faa5aeb0ae7f6ec Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 22 Mar 2020 11:12:30 +0100 Subject: [PATCH 3/6] Added test covering case in which the original address is provided when locating visits --- .../EventDispatcher/LocateShortUrlVisitTest.php | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/module/Core/test/EventDispatcher/LocateShortUrlVisitTest.php b/module/Core/test/EventDispatcher/LocateShortUrlVisitTest.php index 5f40bb7b..c8f6f388 100644 --- a/module/Core/test/EventDispatcher/LocateShortUrlVisitTest.php +++ b/module/Core/test/EventDispatcher/LocateShortUrlVisitTest.php @@ -130,13 +130,16 @@ class LocateShortUrlVisitTest extends TestCase yield 'localhost' => [new Visit($shortUrl, new Visitor('', '', IpAddress::LOCALHOST))]; } - /** @test */ - public function locatableVisitsResolveToLocation(): void + /** + * @test + * @dataProvider provideIpAddresses + */ + public function locatableVisitsResolveToLocation(string $anonymizedIpAddress, ?string $originalIpAddress): void { - $ipAddr = '1.2.3.0'; + $ipAddr = $originalIpAddress ?? $anonymizedIpAddress; $visit = new Visit(new ShortUrl(''), new Visitor('', '', $ipAddr)); $location = new Location('', '', '', '', 0.0, 0.0, ''); - $event = new ShortUrlVisited('123'); + $event = new ShortUrlVisited('123', $originalIpAddress); $findVisit = $this->em->find(Visit::class, '123')->willReturn($visit); $flush = $this->em->flush()->will(function (): void { @@ -155,6 +158,12 @@ class LocateShortUrlVisitTest extends TestCase $dispatch->shouldHaveBeenCalledOnce(); } + public function provideIpAddresses(): iterable + { + yield 'no original IP address' => ['1.2.3.0', null]; + yield 'original IP address' => ['1.2.3.0', '1.2.3.4']; + } + /** @test */ public function errorWhenUpdatingGeoLiteWithExistingCopyLogsWarning(): void { From 3a14483568e115e4dfa449f9bc097e99483640df Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 22 Mar 2020 11:13:33 +0100 Subject: [PATCH 4/6] Updated changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e5915375..8304e678 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this * [#626](https://github.com/shlinkio/shlink/issues/626) Added support for Microsoft SQL Server. * [#556](https://github.com/shlinkio/shlink/issues/556) Short code lengths can now be customized, both globally and on a per-short URL basis. * [#541](https://github.com/shlinkio/shlink/issues/541) Added a request ID that is returned on `X-Request-Id` header, can be provided from outside and is set in log entries. +* [#642](https://github.com/shlinkio/shlink/issues/642) IP geolocation is now performed over the non-anonymized IP address when using swoole. #### Changed From 8fb54e815ea2c019494fe0208678ed6261bad061 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 22 Mar 2020 11:27:03 +0100 Subject: [PATCH 5/6] Ensured scrutinizer build ignores platform requirements when installing dependencies --- .scrutinizer.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.scrutinizer.yml b/.scrutinizer.yml index 3fa0e966..ed831706 100644 --- a/.scrutinizer.yml +++ b/.scrutinizer.yml @@ -6,6 +6,9 @@ checks: code_rating: true duplication: true build: + dependencies: + override: + - composer install --no-interaction --no-scripts --ignore-platform-reqs nodes: analysis: tests: From e10b2884c08f186dc33551c6f2aa22d142c62d88 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 22 Mar 2020 11:33:00 +0100 Subject: [PATCH 6/6] Added more exclussions to dockerignore --- .dockerignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.dockerignore b/.dockerignore index 46b96529..9a48c84c 100644 --- a/.dockerignore +++ b/.dockerignore @@ -16,6 +16,7 @@ docs indocker docker-* phpstan.neon +php*xml* infection.json **/test* build*