From b47b6405985d46d34e6ed7e9688a979c9bbc732c Mon Sep 17 00:00:00 2001
From: Guo Xiang Tan <gxtan1990@gmail.com>
Date: Thu, 17 Sep 2020 10:55:29 +0800
Subject: [PATCH] FEATURE: Hidden `SiteSetting.keep_old_ip_address_count` to
 track IP history.

---
 app/models/user.rb                            | 22 ++++++++
 app/models/user_ip_address_history.rb         |  8 +++
 config/site_settings.yml                      |  3 ++
 ...085541_create_user_ip_address_histories.rb | 18 +++++++
 spec/models/user_spec.rb                      | 50 +++++++++++++++++++
 5 files changed, 101 insertions(+)
 create mode 100644 app/models/user_ip_address_history.rb
 create mode 100644 db/migrate/20200916085541_create_user_ip_address_histories.rb

diff --git a/app/models/user.rb b/app/models/user.rb
index 865b6676fa..432c3b7686 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -724,6 +724,28 @@ class User < ActiveRecord::Base
   def update_ip_address!(new_ip_address)
     unless ip_address == new_ip_address || new_ip_address.blank?
       update_column(:ip_address, new_ip_address)
+
+      if SiteSetting.keep_old_ip_address_count > 0
+        DB.exec(<<~SQL, user_id: self.id, ip_address: new_ip_address, current_timestamp: Time.zone.now)
+        INSERT INTO user_ip_address_histories (user_id, ip_address, created_at, updated_at)
+        VALUES (:user_id, :ip_address, :current_timestamp, :current_timestamp)
+        ON CONFLICT (user_id, ip_address)
+        DO
+          UPDATE SET updated_at = :current_timestamp
+        SQL
+
+        DB.exec(<<~SQL, user_id: self.id, offset: SiteSetting.keep_old_ip_address_count)
+        DELETE FROM user_ip_address_histories
+        WHERE id IN (
+          SELECT
+            id
+          FROM user_ip_address_histories
+          WHERE user_id = :user_id
+          ORDER BY updated_at DESC
+          OFFSET :offset
+        )
+        SQL
+      end
     end
   end
 
diff --git a/app/models/user_ip_address_history.rb b/app/models/user_ip_address_history.rb
new file mode 100644
index 0000000000..0e195d26c0
--- /dev/null
+++ b/app/models/user_ip_address_history.rb
@@ -0,0 +1,8 @@
+# frozen_string_literal: true
+
+class UserIpAddressHistory < ActiveRecord::Base
+  belongs_to :user
+
+  validates :user_id, presence: true
+  validates :ip_address, presence: true, uniqueness: { scope: :user_id }
+end
diff --git a/config/site_settings.yml b/config/site_settings.yml
index fa32b4f989..433f94a9ac 100644
--- a/config/site_settings.yml
+++ b/config/site_settings.yml
@@ -1483,6 +1483,9 @@ security:
   cors_origins:
     default: ""
     type: list
+  keep_old_ip_address_count:
+    default: 0
+    hidden: true
   use_admin_ip_allowlist:
     default: false
     client: true
diff --git a/db/migrate/20200916085541_create_user_ip_address_histories.rb b/db/migrate/20200916085541_create_user_ip_address_histories.rb
new file mode 100644
index 0000000000..d9d50b8aed
--- /dev/null
+++ b/db/migrate/20200916085541_create_user_ip_address_histories.rb
@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+class CreateUserIpAddressHistories < ActiveRecord::Migration[6.0]
+  def up
+    create_table :user_ip_address_histories do |t|
+      t.integer :user_id, null: false
+      t.inet :ip_address, null: false
+
+      t.timestamps
+    end
+
+    add_index :user_ip_address_histories, [:user_id, :ip_address], unique: true
+  end
+
+  def down
+    drop_table :user_ip_address_histories
+  end
+end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 2d770a05dd..36640cd1e8 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -2431,4 +2431,54 @@ describe User do
       expect(user.encoded_username(lower: true)).to eq("l%C3%B6we")
     end
   end
+
+  describe '#update_ip_address!' do
+    it 'updates ip_address correctly' do
+      expect do
+        user.update_ip_address!('127.0.0.1')
+      end.to change { user.reload.ip_address.to_s }.to('127.0.0.1')
+
+      expect do
+        user.update_ip_address!('127.0.0.1')
+      end.to_not change { user.reload.ip_address }
+    end
+
+    describe 'keeping old ip address' do
+      before do
+        SiteSetting.keep_old_ip_address_count = 2
+      end
+
+      it 'tracks old user record correctly' do
+        expect do
+          user.update_ip_address!('127.0.0.1')
+        end.to change { UserIpAddressHistory.where(user_id: user.id).count }.by(1)
+
+        freeze_time 10.minutes.from_now
+
+        expect do
+          user.update_ip_address!('0.0.0.0')
+        end.to change { UserIpAddressHistory.where(user_id: user.id).count }.by(1)
+
+        freeze_time 11.minutes.from_now
+
+        expect do
+          user.update_ip_address!('127.0.0.1')
+        end.to_not change { UserIpAddressHistory.where(user_id: user.id).count }
+
+        expect(UserIpAddressHistory.find_by(
+          user_id: user.id, ip_address: '127.0.0.1'
+        ).updated_at).to eq_time(Time.zone.now)
+
+        freeze_time 12.minutes.from_now
+
+        expect do
+          user.update_ip_address!('0.0.0.1')
+        end.to change { UserIpAddressHistory.where(user_id: user.id).count }.by(0)
+
+        expect(
+          UserIpAddressHistory.where(user_id: user.id).pluck(:ip_address).map(&:to_s)
+        ).to eq(['127.0.0.1', '0.0.0.1'])
+      end
+    end
+  end
 end