From e46bfb3d76017e635f892137da8876e7bb16bca9 Mon Sep 17 00:00:00 2001 From: albert Date: Thu, 18 Mar 2010 17:55:57 -0400 Subject: [PATCH] separated out ip ban logic from regular bans, users can no longer register if an ip ban is in place --- app/models/ban.rb | 6 +- app/models/ip_ban.rb | 9 +++ app/models/user.rb | 14 ++++- app/presenters/post_set_presenter.rb | 11 +++- db/development_structure.sql | 67 ++++++++++++++++++--- db/migrate/20100215225710_create_bans.rb | 2 - db/migrate/20100318213503_create_ip_bans.rb | 16 +++++ test/factories/ip_ban.rb | 5 ++ test/factories/user.rb | 1 + test/unit/ban_test.rb | 14 +---- test/unit/ip_ban_test.rb | 4 ++ test/unit/user_test.rb | 8 +++ 12 files changed, 124 insertions(+), 33 deletions(-) create mode 100644 app/models/ip_ban.rb create mode 100644 db/migrate/20100318213503_create_ip_bans.rb create mode 100644 test/factories/ip_ban.rb create mode 100644 test/unit/ip_ban_test.rb diff --git a/app/models/ban.rb b/app/models/ban.rb index 370be1495..ed598b7c1 100644 --- a/app/models/ban.rb +++ b/app/models/ban.rb @@ -5,14 +5,10 @@ class Ban < ActiveRecord::Base attr_accessible :reason, :duration, :user_id validate :user_is_inferior - def self.is_user_banned?(user) + def self.is_banned?(user) exists?(["user_id = ? AND expires_at > ?", user.id, Time.now]) end - def self.is_ip_banned?(ip_addr) - exists?(["ip_addr = ? AND expires_at > ?", ip_addr, Time.now]) - end - def user_is_inferior if user if user.is_admin? diff --git a/app/models/ip_ban.rb b/app/models/ip_ban.rb new file mode 100644 index 000000000..c16bc1804 --- /dev/null +++ b/app/models/ip_ban.rb @@ -0,0 +1,9 @@ +class IpBan < ActiveRecord::Base + belongs_to :creator, :class_name => "User" + validates_presence_of :reason + validates_uniqueness_of :ip_addr + + def self.is_banned?(ip_addr) + exists?(["ip_addr = ?", ip_addr]) + end +end diff --git a/app/models/user.rb b/app/models/user.rb index 0c2abb14c..3547ac026 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -3,7 +3,7 @@ require 'digest/sha1' class User < ActiveRecord::Base class Error < Exception ; end - attr_accessor :password, :old_password + attr_accessor :password, :old_password, :ip_addr attr_accessible :password, :old_password, :password_confirmation, :password_hash, :email, :last_logged_in_at, :last_forum_read_at, :has_mail, :receive_email_notifications, :comment_threshold, :always_resize_images, :favorite_tags, :blacklisted_tags, :name validates_length_of :name, :within => 2..20, :on => :create validates_format_of :name, :with => /\A[^\s;,]+\Z/, :on => :create, :message => "cannot have whitespace, commas, or semicolons" @@ -13,6 +13,8 @@ class User < ActiveRecord::Base validates_inclusion_of :default_image_size, :in => %w(medium large original) validates_confirmation_of :password validates_presence_of :email, :if => lambda {|rec| rec.new_record? && Danbooru.config.enable_email_verification?} + validates_presence_of :ip_addr, :on => :create + validate :validate_ip_addr_is_not_banned, :on => :create before_save :encrypt_password after_save :update_cache before_create :promote_to_admin_if_first_user @@ -21,6 +23,15 @@ class User < ActiveRecord::Base belongs_to :inviter, :class_name => "User" scope :named, lambda {|name| where(["lower(name) = ?", name])} + module BanMethods + def validate_ip_addr_is_not_banned + if IpBan.is_banned?(ip_addr) + self.errors[:base] << "IP address is banned" + return false + end + end + end + module NameMethods module ClassMethods def find_name(user_id) @@ -222,6 +233,7 @@ class User < ActiveRecord::Base end end + include BanMethods include NameMethods include PasswordMethods extend AuthenticationMethods diff --git a/app/presenters/post_set_presenter.rb b/app/presenters/post_set_presenter.rb index 33a207eb8..8a5910c2a 100644 --- a/app/presenters/post_set_presenter.rb +++ b/app/presenters/post_set_presenter.rb @@ -72,7 +72,7 @@ class PostSetPresenter < Presenter if current_page == total_pages # do nothing elsif current_page_max == total_pages - after_current_page.upto(total_pages) do|i| + after_current_page.upto(total_pages) do |i| html << numbered_pagination_item(template, i) end else @@ -80,8 +80,13 @@ class PostSetPresenter < Presenter html << numbered_pagination_item(template, i) end - html << "
  • ...
    • " - html << numbered_pagination_item(template, total_pages) + if total_pages > 5 + html << "
  • ...
    • " + + (after_current_page + 3).upto(total_pages) do |i| + html << numbered_pagination_item(template, i) + end + end end html << "" diff --git a/db/development_structure.sql b/db/development_structure.sql index a250ff0a6..abde90fb6 100644 --- a/db/development_structure.sql +++ b/db/development_structure.sql @@ -257,7 +257,6 @@ ALTER SEQUENCE artists_id_seq OWNED BY artists.id; CREATE TABLE bans ( id integer NOT NULL, user_id integer, - ip_addr inet, reason text NOT NULL, banner_id integer NOT NULL, expires_at timestamp without time zone NOT NULL, @@ -762,6 +761,39 @@ CREATE SEQUENCE forum_topics_id_seq ALTER SEQUENCE forum_topics_id_seq OWNED BY forum_topics.id; +-- +-- Name: ip_bans; Type: TABLE; Schema: public; Owner: -; Tablespace: +-- + +CREATE TABLE ip_bans ( + id integer NOT NULL, + creator_id integer NOT NULL, + ip_addr inet NOT NULL, + reason text NOT NULL, + created_at timestamp without time zone, + updated_at timestamp without time zone +); + + +-- +-- Name: ip_bans_id_seq; Type: SEQUENCE; Schema: public; Owner: - +-- + +CREATE SEQUENCE ip_bans_id_seq + START WITH 1 + INCREMENT BY 1 + NO MAXVALUE + NO MINVALUE + CACHE 1; + + +-- +-- Name: ip_bans_id_seq; Type: SEQUENCE OWNED BY; Schema: public; Owner: - +-- + +ALTER SEQUENCE ip_bans_id_seq OWNED BY ip_bans.id; + + -- -- Name: janitor_trials; Type: TABLE; Schema: public; Owner: -; Tablespace: -- @@ -1662,6 +1694,13 @@ ALTER TABLE forum_posts ALTER COLUMN id SET DEFAULT nextval('forum_posts_id_seq' ALTER TABLE forum_topics ALTER COLUMN id SET DEFAULT nextval('forum_topics_id_seq'::regclass); +-- +-- Name: id; Type: DEFAULT; Schema: public; Owner: - +-- + +ALTER TABLE ip_bans ALTER COLUMN id SET DEFAULT nextval('ip_bans_id_seq'::regclass); + + -- -- Name: id; Type: DEFAULT; Schema: public; Owner: - -- @@ -1970,6 +2009,14 @@ ALTER TABLE ONLY forum_topics ADD CONSTRAINT forum_topics_pkey PRIMARY KEY (id); +-- +-- Name: ip_bans_pkey; Type: CONSTRAINT; Schema: public; Owner: -; Tablespace: +-- + +ALTER TABLE ONLY ip_bans + ADD CONSTRAINT ip_bans_pkey PRIMARY KEY (id); + + -- -- Name: janitor_trials_pkey; Type: CONSTRAINT; Schema: public; Owner: -; Tablespace: -- @@ -2214,13 +2261,6 @@ CREATE INDEX index_artists_on_other_names_index ON artists USING gin (other_name CREATE INDEX index_bans_on_expires_at ON bans USING btree (expires_at); --- --- Name: index_bans_on_ip_addr; Type: INDEX; Schema: public; Owner: -; Tablespace: --- - -CREATE INDEX index_bans_on_ip_addr ON bans USING btree (ip_addr); - - -- -- Name: index_bans_on_user_id; Type: INDEX; Schema: public; Owner: -; Tablespace: -- @@ -2445,6 +2485,13 @@ CREATE INDEX index_forum_topics_on_creator_id ON forum_topics USING btree (creat CREATE INDEX index_forum_topics_on_text_index ON forum_topics USING gin (text_index); +-- +-- Name: index_ip_bans_on_ip_addr; Type: INDEX; Schema: public; Owner: -; Tablespace: +-- + +CREATE UNIQUE INDEX index_ip_bans_on_ip_addr ON ip_bans USING btree (ip_addr); + + -- -- Name: index_janitor_trials_on_user_id; Type: INDEX; Schema: public; Owner: -; Tablespace: -- @@ -2862,4 +2909,6 @@ INSERT INTO schema_migrations (version) VALUES ('20100224172146'); INSERT INTO schema_migrations (version) VALUES ('20100307073438'); -INSERT INTO schema_migrations (version) VALUES ('20100309211553'); \ No newline at end of file +INSERT INTO schema_migrations (version) VALUES ('20100309211553'); + +INSERT INTO schema_migrations (version) VALUES ('20100318213503'); \ No newline at end of file diff --git a/db/migrate/20100215225710_create_bans.rb b/db/migrate/20100215225710_create_bans.rb index 2031e2caa..b84d3daf1 100644 --- a/db/migrate/20100215225710_create_bans.rb +++ b/db/migrate/20100215225710_create_bans.rb @@ -2,7 +2,6 @@ class CreateBans < ActiveRecord::Migration def self.up create_table :bans do |t| t.column :user_id, :integer - t.column :ip_addr, "inet" t.column :reason, :text, :null => false t.column :banner_id, :integer, :null => false t.column :expires_at, :datetime, :null => false @@ -10,7 +9,6 @@ class CreateBans < ActiveRecord::Migration end add_index :bans, :user_id - add_index :bans, :ip_addr add_index :bans, :expires_at end diff --git a/db/migrate/20100318213503_create_ip_bans.rb b/db/migrate/20100318213503_create_ip_bans.rb new file mode 100644 index 000000000..ba5d3d423 --- /dev/null +++ b/db/migrate/20100318213503_create_ip_bans.rb @@ -0,0 +1,16 @@ +class CreateIpBans < ActiveRecord::Migration + def self.up + create_table :ip_bans do |t| + t.column :creator_id, :integer, :null => false + t.column :ip_addr, :inet, :null => false + t.column :reason, :text, :null => false + t.timestamps + end + + add_index :ip_bans, :ip_addr, :unique => true + end + + def self.down + drop_table :ip_bans + end +end diff --git a/test/factories/ip_ban.rb b/test/factories/ip_ban.rb new file mode 100644 index 000000000..4dc7109c4 --- /dev/null +++ b/test/factories/ip_ban.rb @@ -0,0 +1,5 @@ +Factory.define(:ip_ban) do |f| + f.creator {|x| x.association(:user)} + f.reason {Faker::Lorem.words} + f.ip_addr "127.0.0.1" +end diff --git a/test/factories/user.rb b/test/factories/user.rb index f78efdde2..78fb946e5 100644 --- a/test/factories/user.rb +++ b/test/factories/user.rb @@ -5,6 +5,7 @@ Factory.define(:user) do |f| f.email {Faker::Internet.email} f.default_image_size "medium" f.base_upload_limit 10 + f.ip_addr "127.0.0.1" end Factory.define(:banned_user, :parent => :user) do |f| diff --git a/test/unit/ban_test.rb b/test/unit/ban_test.rb index 13bd8fa05..1b9dadff5 100644 --- a/test/unit/ban_test.rb +++ b/test/unit/ban_test.rb @@ -153,18 +153,6 @@ class BanTest < ActiveSupport::TestCase ban = Factory.create(:ban, :user => user, :banner => admin, :duration => 1) assert(Ban.is_user_banned?(user)) end - end - - context "by ip address" do - should "not return expired bans" do - admin = Factory.create(:admin_user) - - ban = Factory.create(:ban, :ip_addr => "1.2.3.4", :banner => admin, :duration => -1) - assert(!Ban.is_ip_banned?("1.2.3.4")) - - ban = Factory.create(:ban, :ip_addr => "5.6.7.8", :banner => admin, :duration => 1) - assert(Ban.is_ip_banned?("5.6.7.8")) - end - end + end end end diff --git a/test/unit/ip_ban_test.rb b/test/unit/ip_ban_test.rb new file mode 100644 index 000000000..4c249233c --- /dev/null +++ b/test/unit/ip_ban_test.rb @@ -0,0 +1,4 @@ +require File.dirname(__FILE__) + '/../test_helper' + +class IpBanTest < ActiveSupport::TestCase +end diff --git a/test/unit/user_test.rb b/test/unit/user_test.rb index e085ef274..4e798c4f4 100644 --- a/test/unit/user_test.rb +++ b/test/unit/user_test.rb @@ -6,6 +6,14 @@ class UserTest < ActiveSupport::TestCase MEMCACHE.flush_all end + should "not validate if the originating ip address is banned" do + Factory.create(:ip_ban) + user = Factory.build(:user) + user.save + assert(user.errors.any?) + assert_equal("IP address is banned", user.errors.full_messages.join) + end + should "limit post uploads" do user = Factory.create(:user) assert(!user.can_upload?)