diff --git a/app/controllers/ip_bans_controller.rb b/app/controllers/ip_bans_controller.rb index 16d309c33..5c5c2584a 100644 --- a/app/controllers/ip_bans_controller.rb +++ b/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 diff --git a/app/models/ip_ban.rb b/app/models/ip_ban.rb index bff38bfd9..f3382c2af 100644 --- a/app/models/ip_ban.rb +++ b/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 diff --git a/app/views/ip_bans/index.html.erb b/app/views/ip_bans/index.html.erb index 4af2846bc..323953d26 100644 --- a/app/views/ip_bans/index.html.erb +++ b/app/views/ip_bans/index.html.erb @@ -14,8 +14,8 @@ <% @ip_bans.each do |ip_ban| %> - <%= link_to_ip ip_ban.ip_addr %> - <%= ip_ban.creator.name %> + <%= link_to_ip ip_ban.subnetted_ip %> + <%= link_to_user ip_ban.creator %> <%= ip_ban.reason %> <%= link_to "Unban", ip_ban_path(ip_ban), :remote => true, :method => :delete, :data => {:confirm => "Do your really want to unban #{ip_ban.ip_addr}?"} %> diff --git a/app/views/ip_bans/new.html.erb b/app/views/ip_bans/new.html.erb index 352e73da0..eb2372177 100644 --- a/app/views/ip_bans/new.html.erb +++ b/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 %> diff --git a/test/factories/ip_ban.rb b/test/factories/ip_ban.rb index 1b925ccbe..db19c9aff 100644 --- a/test/factories/ip_ban.rb +++ b/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 diff --git a/test/unit/ip_ban_test.rb b/test/unit/ip_ban_test.rb index 7ab386a20..b05e0ccf1 100644 --- a/test/unit/ip_ban_test.rb +++ b/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