Used middleware from library to actually find visitor IP addresses

module/Common/src/Middleware/IpAddressMiddlewareFactory.php

@@ -11,6 +11,8 @@ use Zend\ServiceManager\Factory\FactoryInterface;
 
 class IpAddressMiddlewareFactory implements FactoryInterface
 {
+    public const REMOTE_ADDRESS = 'remote_address';
+
     /**
      * Create an object
      *
@@ -22,6 +24,6 @@ class IpAddressMiddlewareFactory implements FactoryInterface
      */
     public function __invoke(ContainerInterface $container, $requestedName, array $options = null): IpAddress
     {
-        return new IpAddress(true, []);
+        return new IpAddress(true, [], self::REMOTE_ADDRESS);
     }
 }

module/Common/src/Util/IpAddress.php

@@ -4,6 +4,10 @@ declare(strict_types=1);
 namespace Shlinkio\Shlink\Common\Util;
 
 use Shlinkio\Shlink\Common\Exception\WrongIpException;
+use function count;
+use function explode;
+use function implode;
+use function trim;
 
 final class IpAddress
 {
@@ -43,9 +47,9 @@ final class IpAddress
      */
     public static function fromString(string $address): self
     {
-        $address = \trim($address);
-        $parts = \explode('.', $address);
-        if (\count($parts) !== self::IPV4_PARTS_COUNT) {
+        $address = trim($address);
+        $parts = explode('.', $address);
+        if (count($parts) !== self::IPV4_PARTS_COUNT) {
             throw WrongIpException::fromIpAddress($address);
         }
 
@@ -64,7 +68,7 @@ final class IpAddress
 
     public function __toString(): string
     {
-        return \implode('.', [
+        return implode('.', [
             $this->firstOctet,
             $this->secondOctet,
             $this->thirdOctet,

module/Common/test/Middleware/IpAddressMiddlewareFactoryTest.php

@@ -29,8 +29,11 @@ class IpAddressMiddlewareFactoryTest extends TestCase
         $checkProxyHeaders->setAccessible(true);
         $trustedProxies = $ref->getProperty('trustedProxies');
         $trustedProxies->setAccessible(true);
+        $attributeName = $ref->getProperty('attributeName');
+        $attributeName->setAccessible(true);
 
         $this->assertTrue($checkProxyHeaders->getValue($instance));
         $this->assertEquals([], $trustedProxies->getValue($instance));
+        $this->assertEquals(IpAddressMiddlewareFactory::REMOTE_ADDRESS, $attributeName->getValue($instance));
     }
 }

module/Core/config/routes.config.php

@@ -1,6 +1,7 @@
 <?php
 declare(strict_types=1);
 
+use RKA\Middleware\IpAddress;
 use Shlinkio\Shlink\Core\Action;
 use Shlinkio\Shlink\Core\Middleware;
 
@@ -10,13 +11,19 @@ return [
         [
             'name' => 'long-url-redirect',
             'path' => '/{shortCode}',
-            'middleware' => Action\RedirectAction::class,
+            'middleware' => [
+                IpAddress::class,
+                Action\RedirectAction::class,
+            ],
             'allowed_methods' => ['GET'],
         ],
         [
             'name' => 'pixel-tracking',
             'path' => '/{shortCode}/track',
-            'middleware' => Action\PixelAction::class,
+            'middleware' => [
+                IpAddress::class,
+                Action\PixelAction::class,
+            ],
             'allowed_methods' => ['GET'],
         ],
         [

module/Core/src/Action/AbstractTrackingAction.php

@@ -9,9 +9,11 @@ use Psr\Http\Server\MiddlewareInterface;
 use Psr\Http\Server\RequestHandlerInterface;
 use Psr\Log\LoggerInterface;
 use Psr\Log\NullLogger;
+use Shlinkio\Shlink\Common\Middleware\IpAddressMiddlewareFactory;
 use Shlinkio\Shlink\Core\Action\Util\ErrorResponseBuilderTrait;
 use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException;
 use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException;
+use Shlinkio\Shlink\Core\Model\Visitor;
 use Shlinkio\Shlink\Core\Options\AppOptions;
 use Shlinkio\Shlink\Core\Service\UrlShortenerInterface;
 use Shlinkio\Shlink\Core\Service\VisitsTrackerInterface;
@@ -69,7 +71,11 @@ abstract class AbstractTrackingAction implements MiddlewareInterface
 
             // Track visit to this short code
             if ($disableTrackParam === null || ! \array_key_exists($disableTrackParam, $query)) {
-                $this->visitTracker->track($shortCode, $request);
+                $this->visitTracker->track($shortCode, new Visitor(
+                    $request->getHeaderLine('User-Agent'),
+                    $request->getHeaderLine('Referer'),
+                    $request->getAttribute(IpAddressMiddlewareFactory::REMOTE_ADDRESS)
+                ));
             }
 
             return $this->createResp($url->getLongUrl());

module/Core/src/Model/Visitor.php

@@ -0,0 +1,47 @@
+<?php
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Core\Model;
+
+final class Visitor
+{
+    /**
+     * @var string
+     */
+    private $userAgent;
+    /**
+     * @var string
+     */
+    private $referer;
+    /**
+     * @var string|null
+     */
+    private $remoteAddress;
+
+    public function __construct(string $userAgent, string $referer, ?string $remoteAddress)
+    {
+        $this->userAgent = $userAgent;
+        $this->referer = $referer;
+        $this->remoteAddress = $remoteAddress;
+    }
+
+    public static function emptyInstance(): self
+    {
+        return new self('', '', null);
+    }
+
+    public function getUserAgent(): string
+    {
+        return $this->userAgent;
+    }
+
+    public function getReferer(): string
+    {
+        return $this->referer;
+    }
+
+    public function getRemoteAddress(): ?string
+    {
+        return $this->remoteAddress;
+    }
+}

module/Core/src/Service/VisitsTracker.php

@@ -4,11 +4,11 @@ declare(strict_types=1);
 namespace Shlinkio\Shlink\Core\Service;
 
 use Doctrine\ORM;
-use Psr\Http\Message\ServerRequestInterface;
 use Shlinkio\Shlink\Common\Util\DateRange;
 use Shlinkio\Shlink\Core\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\Entity\Visit;
 use Shlinkio\Shlink\Core\Exception\InvalidArgumentException;
+use Shlinkio\Shlink\Core\Model\Visitor;
 use Shlinkio\Shlink\Core\Repository\VisitRepository;
 
 class VisitsTracker implements VisitsTrackerInterface
@@ -24,14 +24,9 @@ class VisitsTracker implements VisitsTrackerInterface
     }
 
     /**
-     * Tracks a new visit to provided short code, using an array of data to look up information
-     *
-     * @param string $shortCode
-     * @param ServerRequestInterface $request
-     * @throws ORM\ORMInvalidArgumentException
-     * @throws ORM\OptimisticLockException
+     * Tracks a new visit to provided short code from provided visitor
      */
-    public function track($shortCode, ServerRequestInterface $request): void
+    public function track(string $shortCode, Visitor $visitor): void
     {
         /** @var ShortUrl $shortUrl */
         $shortUrl = $this->em->getRepository(ShortUrl::class)->findOneBy([
@@ -40,9 +35,9 @@ class VisitsTracker implements VisitsTrackerInterface
 
         $visit = new Visit();
         $visit->setShortUrl($shortUrl)
-              ->setUserAgent($request->getHeaderLine('User-Agent'))
-              ->setReferer($request->getHeaderLine('Referer'))
-              ->setRemoteAddr($this->findOutRemoteAddr($request));
+              ->setUserAgent($visitor->getUserAgent())
+              ->setReferer($visitor->getReferer())
+              ->setRemoteAddr($visitor->getRemoteAddress());
 
         /** @var ORM\EntityManager $em */
         $em = $this->em;
@@ -50,21 +45,6 @@ class VisitsTracker implements VisitsTrackerInterface
         $em->flush($visit);
     }
 
-    /**
-     * @param ServerRequestInterface $request
-     */
-    private function findOutRemoteAddr(ServerRequestInterface $request): ?string
-    {
-        $forwardedFor = $request->getHeaderLine('X-Forwarded-For');
-        if (empty($forwardedFor)) {
-            $serverParams = $request->getServerParams();
-            return $serverParams['REMOTE_ADDR'] ?? null;
-        }
-
-        $ips = \explode(',', $forwardedFor);
-        return $ips[0] ?? null;
-    }
-
     /**
      * Returns the visits on certain short code
      *

module/Core/src/Service/VisitsTrackerInterface.php

@@ -3,20 +3,17 @@ declare(strict_types=1);
 
 namespace Shlinkio\Shlink\Core\Service;
 
-use Psr\Http\Message\ServerRequestInterface;
 use Shlinkio\Shlink\Common\Util\DateRange;
 use Shlinkio\Shlink\Core\Entity\Visit;
 use Shlinkio\Shlink\Core\Exception\InvalidArgumentException;
+use Shlinkio\Shlink\Core\Model\Visitor;
 
 interface VisitsTrackerInterface
 {
     /**
-     * Tracks a new visit to provided short code, using an array of data to look up information
-     *
-     * @param string $shortCode
-     * @param ServerRequestInterface $request
+     * Tracks a new visit to provided short code from provided visitor
      */
-    public function track($shortCode, ServerRequestInterface $request): void;
+    public function track(string $shortCode, Visitor $visitor): void;
 
     /**
      * Returns the visits on certain short code

module/Core/test/Service/VisitsTrackerTest.php

@@ -10,6 +10,7 @@ use Prophecy\Argument;
 use Prophecy\Prophecy\ObjectProphecy;
 use Shlinkio\Shlink\Core\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\Entity\Visit;
+use Shlinkio\Shlink\Core\Model\Visitor;
 use Shlinkio\Shlink\Core\Repository\VisitRepository;
 use Shlinkio\Shlink\Core\Service\VisitsTracker;
 use Zend\Diactoros\ServerRequestFactory;
@@ -44,13 +45,13 @@ class VisitsTrackerTest extends TestCase
         $this->em->persist(Argument::any())->shouldBeCalledTimes(1);
         $this->em->flush(Argument::type(Visit::class))->shouldBeCalledTimes(1);
 
-        $this->visitsTracker->track($shortCode, ServerRequestFactory::fromGlobals());
+        $this->visitsTracker->track($shortCode, Visitor::emptyInstance());
     }
 
     /**
      * @test
      */
-    public function trackUsesForwardedForHeaderIfPresent()
+    public function trackedIpAddressGetsObfuscated()
     {
         $shortCode = '123ABC';
         $test = $this;
@@ -65,9 +66,7 @@ class VisitsTrackerTest extends TestCase
         })->shouldBeCalledTimes(1);
         $this->em->flush(Argument::type(Visit::class))->shouldBeCalledTimes(1);
 
-        $this->visitsTracker->track($shortCode, ServerRequestFactory::fromGlobals(
-            ['REMOTE_ADDR' => '1.2.3.4']
-        )->withHeader('X-Forwarded-For', '4.3.2.1,99.99.99.99'));
+        $this->visitsTracker->track($shortCode, new Visitor('', '', '4.3.2.1'));
     }
 
     /**