From 3d0bca27811fddd6a96eee45a5b875ef1ccd7373 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Fri, 14 Sep 2018 19:04:40 +0200
Subject: [PATCH] Finally dropped the hashing of the address

---
 data/migrations/Version20180913205455.php | 28 ++++-------------------
 docs/swagger/definitions/Visit.json       | 10 ++++----
 module/Core/src/Entity/Visit.php          | 24 ++++---------------
 3 files changed, 13 insertions(+), 49 deletions(-)

diff --git a/data/migrations/Version20180913205455.php b/data/migrations/Version20180913205455.php
index 78c07855..0a156293 100644
--- a/data/migrations/Version20180913205455.php
+++ b/data/migrations/Version20180913205455.php
@@ -5,8 +5,6 @@ namespace ShlinkMigrations;
 
 use Doctrine\DBAL\DBALException;
 use Doctrine\DBAL\Schema\Schema;
-use Doctrine\DBAL\Schema\SchemaException;
-use Doctrine\DBAL\Types\Type;
 use Doctrine\Migrations\AbstractMigration;
 use Shlinkio\Shlink\Common\Exception\WrongIpException;
 use Shlinkio\Shlink\Common\Util\IpAddress;
@@ -18,35 +16,25 @@ final class Version20180913205455 extends AbstractMigration
 {
     /**
      * @param Schema $schema
-     * @throws SchemaException
      */
     public function up(Schema $schema): void
     {
-        $visits = $schema->getTable('visits');
-        if ($visits->hasColumn('remote_addr_hash')) {
-            return;
-        }
-
-        $visits->addColumn('remote_addr_hash', Type::STRING, [
-            'notnull' => false,
-            'length' => 128,
-        ]);
+        // Nothing to create
     }
 
     /**
      * @param Schema $schema
      * @throws DBALException
      */
-    public function postUp(Schema $schema)
+    public function postUp(Schema $schema): void
     {
         $qb = $this->connection->createQueryBuilder();
-        $qb->select('id', 'remote_addr', 'visit_location_id')
+        $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_hash', ':hash')
            ->set('v.remote_addr', ':obfuscatedAddr')
            ->where('v.id=:id');
 
@@ -58,7 +46,6 @@ final class Version20180913205455 extends AbstractMigration
 
             $qb->setParameters([
                 'id' => $row['id'],
-                'hash' => \hash('sha256', $addr),
                 'obfuscatedAddr' => $this->determineAddress((string) $addr, $row),
             ])->execute();
         }
@@ -66,11 +53,6 @@ final class Version20180913205455 extends AbstractMigration
 
     private function determineAddress(string $addr, array $row): ?string
     {
-        // When the visit has already been located, drop the IP address
-        if (isset($row['visit_location_id'])) {
-            return null;
-        }
-
         if ($addr === IpAddress::LOCALHOST) {
             return $addr;
         }
@@ -84,11 +66,9 @@ final class Version20180913205455 extends AbstractMigration
 
     /**
      * @param Schema $schema
-     * @throws SchemaException
      */
     public function down(Schema $schema): void
     {
-        $visits = $schema->getTable('visits');
-        $visits->dropColumn('remote_addr_hash');
+        // Nothing to rollback
     }
 }
diff --git a/docs/swagger/definitions/Visit.json b/docs/swagger/definitions/Visit.json
index ab80ad8a..131bc84e 100644
--- a/docs/swagger/definitions/Visit.json
+++ b/docs/swagger/definitions/Visit.json
@@ -10,17 +10,17 @@
             "format": "date-time",
             "description": "The date in which the visit was performed"
         },
-        "remoteAddr": {
-            "type": "string",
-            "description": "This value is deprecated and will always be null",
-            "deprecated": true
-        },
         "userAgent": {
             "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
         }
     }
 }
diff --git a/module/Core/src/Entity/Visit.php b/module/Core/src/Entity/Visit.php
index 303344a7..9390005c 100644
--- a/module/Core/src/Entity/Visit.php
+++ b/module/Core/src/Entity/Visit.php
@@ -33,11 +33,6 @@ class Visit extends AbstractEntity implements \JsonSerializable
      * @ORM\Column(type="string", length=256, name="remote_addr", nullable=true)
      */
     private $remoteAddr;
-    /**
-     * @var string|null
-     * @ORM\Column(type="string", length=256, name="remote_addr_hash", nullable=true)
-     */
-    private $remoteAddrHash;
     /**
      * @var string
      * @ORM\Column(type="string", length=256, name="user_agent", nullable=true)
@@ -94,7 +89,7 @@ class Visit extends AbstractEntity implements \JsonSerializable
         return $this;
     }
 
-    public function getRemoteAddr(): string
+    public function getRemoteAddr(): ?string
     {
         return $this->remoteAddr;
     }
@@ -102,8 +97,6 @@ class Visit extends AbstractEntity implements \JsonSerializable
     public function setRemoteAddr(?string $remoteAddr): self
     {
         $this->remoteAddr = $this->obfuscateAddress($remoteAddr);
-        $this->remoteAddrHash = $this->hashAddress($remoteAddr);
-
         return $this;
     }
 
@@ -121,17 +114,6 @@ class Visit extends AbstractEntity implements \JsonSerializable
         }
     }
 
-    private function hashAddress(?string $address): ?string
-    {
-        return $address ? \hash('sha256', $address) : null;
-    }
-
-    public function resetObfuscatedAddr(): self
-    {
-        $this->remoteAddr = null;
-        return $this;
-    }
-
     public function getUserAgent(): string
     {
         return $this->userAgent;
@@ -166,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,
         ];
     }
 }