Fix #4106: Allow moderators to IP ban subnets.

app/controllers/ip_bans_controller.rb

@@ -12,8 +12,7 @@ class IpBansController < ApplicationController
   end
 
   def index
-    @search = IpBan.search(search_params)
-    @ip_bans = @search.paginate(params[:page], :limit => params[:limit])
+    @ip_bans = IpBan.includes(:creator).search(search_params).paginate(params[:page], :limit => params[:limit])
     respond_with(@ip_bans)
   end
 

app/models/ip_ban.rb

@@ -1,18 +1,13 @@
 class IpBan < ApplicationRecord
-  IP_ADDR_REGEX = /\A(?:[0-9]{1,3}\.){3}[0-9]{1,3}\Z/
   belongs_to_creator
-  validates_presence_of :reason, :ip_addr
-  validates_format_of :ip_addr, :with => IP_ADDR_REGEX
-  validates_uniqueness_of :ip_addr, :if => ->(rec) {rec.ip_addr =~ IP_ADDR_REGEX}
-  after_create do |rec|
-    ModAction.log("#{CurrentUser.name} created ip ban for #{rec.ip_addr}",:ip_ban_create)
-  end
-  after_destroy do |rec|
-    ModAction.log("#{CurrentUser.name} deleted ip ban for #{rec.ip_addr}",:ip_ban_delete)
-  end
+  validate :validate_ip_addr
+  validates_presence_of :reason
+  validates_uniqueness_of :ip_addr
+  after_create  { ModAction.log("#{CurrentUser.name} created ip ban for #{ip_addr}", :ip_ban_create) }
+  after_destroy { ModAction.log("#{CurrentUser.name} deleted ip ban for #{ip_addr}", :ip_ban_delete) }
 
   def self.is_banned?(ip_addr)
-    exists?(["ip_addr = ?", ip_addr])
+    where("ip_addr >>= ?", ip_addr).exists?
   end
 
   def self.search(params)
@@ -24,4 +19,26 @@ class IpBan < ApplicationRecord
 
     q.apply_default_order(params)
   end
+
+  def validate_ip_addr
+    if ip_addr.blank?
+      errors[:ip_addr] << "is invalid"
+    elsif ip_addr.ipv4? && ip_addr.prefix < 24
+      errors[:ip_addr] << "may not have a subnet bigger than /24"
+    elsif ip_addr.ipv6? && ip_addr.prefix < 64
+      errors[:ip_addr] << "may not have a subnet bigger than /64"
+    elsif ip_addr.private? || ip_addr.loopback? || ip_addr.link_local?
+      errors[:ip_addr] << "must be a public address"
+    end
+  end
+
+  def has_subnet?
+    (ip_addr.ipv4? && ip_addr.prefix < 32) || (ip_addr.ipv6? && ip_addr.prefix < 128)
+  end
+
+  def subnetted_ip
+    str = ip_addr.to_s
+    str += "/" + ip_addr.prefix.to_s if has_subnet?
+    str
+  end
 end

app/views/ip_bans/index.html.erb

@@ -14,8 +14,8 @@
       <tbody>
         <% @ip_bans.each do |ip_ban| %>
           <tr>
-            <td><%= link_to_ip ip_ban.ip_addr %></td>
-            <td><%= ip_ban.creator.name %></td>
+            <td><%= link_to_ip ip_ban.subnetted_ip %></td>
+            <td><%= link_to_user ip_ban.creator %></td>
             <td><%= ip_ban.reason %></td>
             <td><%= link_to "Unban", ip_ban_path(ip_ban), :remote => true, :method => :delete, :data => {:confirm => "Do your really want to unban #{ip_ban.ip_addr}?"} %></td>
           </tr>

app/views/ip_bans/new.html.erb

@@ -5,8 +5,8 @@
     <%= error_messages_for "ip_ban" %>
 
     <%= simple_form_for(@ip_ban) do |f| %>
-      <%= f.input :ip_addr, :label => "IP Address", :as => :string %>
-      <%= f.input :reason, :input_html => {:size => "50x5"} %>
+      <%= f.input :ip_addr, label: "IP Address", as: :string, hint: "Add /24 to ban a subnet. Example: 1.2.3.4/24" %>
+      <%= f.input :reason %>
       <%= f.button :submit, "Submit" %>
     <% end %>
   </div>

test/factories/ip_ban.rb

@@ -1,7 +1,7 @@
 FactoryBot.define do
   factory(:ip_ban) do
-    creator :factory => :user
-    reason {FFaker::Lorem.words.join(" ")}
-    ip_addr {"127.0.0.2"}
+    creator
+    reason { FFaker::Lorem.words.join(" ") }
+    ip_addr { FFaker::Internet.ip_v4_address }
   end
 end

test/unit/ip_ban_test.rb

@@ -2,10 +2,8 @@ require 'test_helper'
 
 class IpBanTest < ActiveSupport::TestCase
   setup do
-    @user = FactoryBot.create(:user)
     CurrentUser.user = FactoryBot.create(:mod_user)
     CurrentUser.ip_addr = "127.0.0.1"
-    Danbooru.config.stubs(:member_comment_time_threshold).returns(1.week.from_now)
   end
 
   teardown do
@@ -13,24 +11,34 @@ class IpBanTest < ActiveSupport::TestCase
     CurrentUser.ip_addr = nil
   end
 
-  should "be able to count the number of comments an IP address is associated with" do
-    comment = FactoryBot.create(:comment)
-    counts = IpBan.count_by_ip_addr("comments", [comment.creator_id], "creator_id", "creator_ip_addr")
-    assert_equal([{"creator_ip_addr" => "127.0.0.1", "count" => 1}], counts)
-  end
-
-  should "be able to count any updates from a user, groupiny by IP address" do
-    CurrentUser.scoped(@user, "1.2.3.4") do
-      comment = FactoryBot.create(:comment, :body => "aaa")
-      counts = IpBan.query([comment.creator_id])
-      assert_equal([{"creator_ip_addr" => "1.2.3.4", "count" => 1}], counts["comments"])
-    end
-  end
-
   should "be able to ban a user" do
-    ip_ban = IpBan.create(ip_addr: "1.2.3.4", reason: "test")
+    ip_ban = create(:ip_ban, ip_addr: "1.2.3.4")
 
-    assert_equal("1.2.3.4", ip_ban.ip_addr.to_s)
+    assert_equal("1.2.3.4", ip_ban.subnetted_ip)
     assert(IpBan.is_banned?("1.2.3.4"))
   end
+
+  should "be able to ban a subnet" do
+    ip_ban = create(:ip_ban, ip_addr: "1.2.3.4/24")
+
+    assert_equal("1.2.3.0/24", ip_ban.subnetted_ip)
+    assert(IpBan.is_banned?("1.2.3.0"))
+    assert(IpBan.is_banned?("1.2.3.255"))
+  end
+
+  context "validation" do
+    subject { build(:ip_ban) }
+
+    should allow_value("1.2.3.4").for(:ip_addr)
+    should allow_value("1.2.3.4/24").for(:ip_addr)
+    should allow_value("ABCD::1234").for(:ip_addr)
+    should allow_value("ABCD::1234/64").for(:ip_addr)
+
+    should_not allow_value("").for(:ip_addr)
+    should_not allow_value("foo").for(:ip_addr)
+    should_not allow_value("10.0.0.1").for(:ip_addr)
+    should_not allow_value("127.0.0.1").for(:ip_addr)
+    should_not allow_value("1.2.3.4/16").for(:ip_addr)
+    should_not allow_value("ABCD::1234/32").for(:ip_addr)
+  end
 end