From 648b11a0eb3824e7a2f16cb007fd8165d9ee9cbe Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Mon, 21 Oct 2013 14:49:51 -0400 Subject: [PATCH] Add screening by IP address. When deleting a user as a spammer, block all signups from the same IP address. --- ...n_logs_screened_ip_addresses_controller.js | 21 +++ .../javascripts/admin/models/admin_user.js | 5 +- .../admin/models/screened_ip_address.js | 29 ++++ .../admin/routes/admin_logs_routes.js | 18 +++ .../javascripts/admin/routes/admin_routes.js | 1 + .../admin/templates/logs.js.handlebars | 1 + .../logs/screened_ip_addresses.js.handlebars | 24 ++++ ...eened_ip_addresses_list_item.js.handlebars | 10 ++ .../logs/screened_ip_addresses_list_view.js | 5 + .../stylesheets/common/admin/admin_base.scss | 4 +- .../admin/screened_ip_addresses_controller.rb | 8 ++ app/controllers/admin/users_controller.rb | 2 +- app/controllers/users_controller.rb | 1 + app/models/screened_ip_address.rb | 32 +++++ app/models/user.rb | 1 + .../screened_ip_address_serializer.rb | 12 ++ config/locales/client.en.yml | 10 +- config/locales/server.en.yml | 8 ++ config/routes.rb | 7 +- db/fixtures/screened_ip_addresses.rb | 30 +++++ ...1017205954_create_screened_ip_addresses.rb | 13 ++ lib/user_destroyer.rb | 4 + .../allowed_ip_address_validator.rb | 9 ++ spec/components/user_destroyer_spec.rb | 12 ++ .../allowed_ip_address_validator_spec.rb | 33 +++++ .../screened_ip_addresses_controller_spec.rb | 22 +++ .../screened_ip_address_fabricator.rb | 3 + spec/models/screened_ip_address_spec.rb | 126 ++++++++++++++++++ spec/models/user_spec.rb | 16 ++- 29 files changed, 455 insertions(+), 12 deletions(-) create mode 100644 app/assets/javascripts/admin/controllers/admin_logs_screened_ip_addresses_controller.js create mode 100644 app/assets/javascripts/admin/models/screened_ip_address.js create mode 100644 app/assets/javascripts/admin/templates/logs/screened_ip_addresses.js.handlebars create mode 100644 app/assets/javascripts/admin/templates/logs/screened_ip_addresses_list_item.js.handlebars create mode 100644 app/assets/javascripts/admin/views/logs/screened_ip_addresses_list_view.js create mode 100644 app/controllers/admin/screened_ip_addresses_controller.rb create mode 100644 app/models/screened_ip_address.rb create mode 100644 app/serializers/screened_ip_address_serializer.rb create mode 100644 db/fixtures/screened_ip_addresses.rb create mode 100644 db/migrate/20131017205954_create_screened_ip_addresses.rb create mode 100644 lib/validators/allowed_ip_address_validator.rb create mode 100644 spec/components/validators/allowed_ip_address_validator_spec.rb create mode 100644 spec/controllers/admin/screened_ip_addresses_controller_spec.rb create mode 100644 spec/fabricators/screened_ip_address_fabricator.rb create mode 100644 spec/models/screened_ip_address_spec.rb diff --git a/app/assets/javascripts/admin/controllers/admin_logs_screened_ip_addresses_controller.js b/app/assets/javascripts/admin/controllers/admin_logs_screened_ip_addresses_controller.js new file mode 100644 index 00000000000..b733558aae3 --- /dev/null +++ b/app/assets/javascripts/admin/controllers/admin_logs_screened_ip_addresses_controller.js @@ -0,0 +1,21 @@ +/** + This controller supports the interface for listing screened IP addresses in the admin section. + + @class AdminLogsScreenedIpAddressesController + @extends Ember.ArrayController + @namespace Discourse + @module Discourse +**/ +Discourse.AdminLogsScreenedIpAddressesController = Ember.ArrayController.extend(Discourse.Presence, { + loading: false, + content: [], + + show: function() { + var self = this; + this.set('loading', true); + Discourse.ScreenedIpAddress.findAll().then(function(result) { + self.set('content', result); + self.set('loading', false); + }); + } +}); diff --git a/app/assets/javascripts/admin/models/admin_user.js b/app/assets/javascripts/admin/models/admin_user.js index 6e43b8698f2..6ce8997431d 100644 --- a/app/assets/javascripts/admin/models/admin_user.js +++ b/app/assets/javascripts/admin/models/admin_user.js @@ -236,6 +236,7 @@ Discourse.AdminUser = Discourse.User.extend({ if (block) { formData["block_email"] = true; formData["block_urls"] = true; + formData["block_ip"] = true; } Discourse.ajax("/admin/users/" + user.get('id') + '.json', { type: 'DELETE', @@ -282,7 +283,7 @@ Discourse.AdminUser = Discourse.User.extend({ deleteAsSpammer: function(successCallback) { var user = this; - var message = I18n.t('flagging.delete_confirm', {posts: user.get('post_count'), topics: user.get('topic_count'), email: user.get('email')}); + var message = I18n.t('flagging.delete_confirm', {posts: user.get('post_count'), topics: user.get('topic_count'), email: user.get('email'), ip_address: user.get('ip_address')}); var buttons = [{ "label": I18n.t("composer.cancel"), "class": "cancel", @@ -293,7 +294,7 @@ Discourse.AdminUser = Discourse.User.extend({ "callback": function() { Discourse.ajax("/admin/users/" + user.get('id') + '.json', { type: 'DELETE', - data: {delete_posts: true, block_email: true, block_urls: true, context: window.location.pathname} + data: {delete_posts: true, block_email: true, block_urls: true, block_ip: true, context: window.location.pathname} }).then(function(data) { if (data.deleted) { bootbox.alert(I18n.t("admin.user.deleted"), function() { diff --git a/app/assets/javascripts/admin/models/screened_ip_address.js b/app/assets/javascripts/admin/models/screened_ip_address.js new file mode 100644 index 00000000000..1e81b61658e --- /dev/null +++ b/app/assets/javascripts/admin/models/screened_ip_address.js @@ -0,0 +1,29 @@ +/** + Represents an IP address that is watched for during account registration + (and possibly other times), and an action is taken. + + @class ScreenedIpAddress + @extends Discourse.Model + @namespace Discourse + @module Discourse +**/ +Discourse.ScreenedIpAddress = Discourse.Model.extend({ + // TODO: this is repeated in all 3 screened models. move it. + actionName: function() { + if (this.get('action') === 'do_nothing') { + return I18n.t("admin.logs.screened_ips.allow"); + } else { + return I18n.t("admin.logs.screened_actions." + this.get('action')); + } + }.property('action') +}); + +Discourse.ScreenedIpAddress.reopenClass({ + findAll: function(filter) { + return Discourse.ajax("/admin/logs/screened_ip_addresses.json").then(function(screened_ips) { + return screened_ips.map(function(b) { + return Discourse.ScreenedIpAddress.create(b); + }); + }); + } +}); diff --git a/app/assets/javascripts/admin/routes/admin_logs_routes.js b/app/assets/javascripts/admin/routes/admin_logs_routes.js index b989b068645..0ac57941d05 100644 --- a/app/assets/javascripts/admin/routes/admin_logs_routes.js +++ b/app/assets/javascripts/admin/routes/admin_logs_routes.js @@ -71,6 +71,24 @@ Discourse.AdminLogsScreenedEmailsRoute = Discourse.Route.extend({ } }); +/** + The route that lists screened IP addresses. + + @class AdminLogsScreenedIpAddresses + @extends Discourse.Route + @namespace Discourse + @module Discourse +**/ +Discourse.AdminLogsScreenedIpAddressesRoute = Discourse.Route.extend({ + renderTemplate: function() { + this.render('admin/templates/logs/screened_ip_addresses', {into: 'adminLogs'}); + }, + + setupController: function() { + return this.controllerFor('adminLogsScreenedIpAddresses').show(); + } +}); + /** The route that lists screened URLs. diff --git a/app/assets/javascripts/admin/routes/admin_routes.js b/app/assets/javascripts/admin/routes/admin_routes.js index 8551a85235e..d9ca9b5b442 100644 --- a/app/assets/javascripts/admin/routes/admin_routes.js +++ b/app/assets/javascripts/admin/routes/admin_routes.js @@ -33,6 +33,7 @@ Discourse.Route.buildRoutes(function() { this.resource('adminLogs', { path: '/logs' }, function() { this.route('staffActionLogs', { path: '/staff_action_logs' }); this.route('screenedEmails', { path: '/screened_emails' }); + this.route('screenedIpAddresses', { path: '/screened_ip_addresses' }); this.route('screenedUrls', { path: '/screened_urls' }); }); diff --git a/app/assets/javascripts/admin/templates/logs.js.handlebars b/app/assets/javascripts/admin/templates/logs.js.handlebars index 5e6af379d0b..13bcf4ee853 100644 --- a/app/assets/javascripts/admin/templates/logs.js.handlebars +++ b/app/assets/javascripts/admin/templates/logs.js.handlebars @@ -3,6 +3,7 @@ diff --git a/app/assets/javascripts/admin/templates/logs/screened_ip_addresses.js.handlebars b/app/assets/javascripts/admin/templates/logs/screened_ip_addresses.js.handlebars new file mode 100644 index 00000000000..ed6bc5521f4 --- /dev/null +++ b/app/assets/javascripts/admin/templates/logs/screened_ip_addresses.js.handlebars @@ -0,0 +1,24 @@ +

{{i18n admin.logs.screened_ips.description}}

+ +{{#if loading}} +
{{i18n loading}}
+{{else}} + {{#if model.length}} + +
+
+
{{i18n admin.logs.ip_address}}
+
{{i18n admin.logs.action}}
+
{{i18n admin.logs.match_count}}
+
{{i18n admin.logs.last_match_at}}
+
{{i18n admin.logs.created_at}}
+
+
+ + {{view Discourse.ScreenedIpAddressesListView contentBinding="controller"}} +
+ + {{else}} + {{i18n search.no_results}} + {{/if}} +{{/if}} \ No newline at end of file diff --git a/app/assets/javascripts/admin/templates/logs/screened_ip_addresses_list_item.js.handlebars b/app/assets/javascripts/admin/templates/logs/screened_ip_addresses_list_item.js.handlebars new file mode 100644 index 00000000000..22c06f7e9d9 --- /dev/null +++ b/app/assets/javascripts/admin/templates/logs/screened_ip_addresses_list_item.js.handlebars @@ -0,0 +1,10 @@ +
{{ip_address}}
+
{{actionName}}
+
{{match_count}}
+
+ {{#if last_match_at}} + {{unboundAgeWithTooltip last_match_at}} + {{/if}} +
+
{{unboundAgeWithTooltip created_at}}
+
\ No newline at end of file diff --git a/app/assets/javascripts/admin/views/logs/screened_ip_addresses_list_view.js b/app/assets/javascripts/admin/views/logs/screened_ip_addresses_list_view.js new file mode 100644 index 00000000000..a6faf21e1b0 --- /dev/null +++ b/app/assets/javascripts/admin/views/logs/screened_ip_addresses_list_view.js @@ -0,0 +1,5 @@ +Discourse.ScreenedIpAddressesListView = Ember.ListView.extend({ + height: 700, + rowHeight: 32, + itemViewClass: Ember.ListItemView.extend({templateName: "admin/templates/logs/screened_ip_addresses_list_item"}) +}); diff --git a/app/assets/stylesheets/common/admin/admin_base.scss b/app/assets/stylesheets/common/admin/admin_base.scss index 27db57bfb2c..f6958d48f62 100644 --- a/app/assets/stylesheets/common/admin/admin_base.scss +++ b/app/assets/stylesheets/common/admin/admin_base.scss @@ -700,7 +700,7 @@ table { // Logs -.screened-emails, .screened-urls { +.screened-emails, .screened-urls, .screened-ip-addresses { width: 900px; .email, .url { width: 300px; @@ -778,7 +778,7 @@ table { position: absolute; } -.staff-actions, .screened-emails, .screened-urls { +.staff-actions, .screened-emails, .screened-urls, .screened-ip-addresses { margin-left: 5px; border-bottom: dotted 1px #ddd; diff --git a/app/controllers/admin/screened_ip_addresses_controller.rb b/app/controllers/admin/screened_ip_addresses_controller.rb new file mode 100644 index 00000000000..6c1da6d1dbb --- /dev/null +++ b/app/controllers/admin/screened_ip_addresses_controller.rb @@ -0,0 +1,8 @@ +class Admin::ScreenedIpAddressesController < Admin::AdminController + + def index + screened_emails = ScreenedIpAddress.limit(200).order('last_match_at desc').to_a + render_serialized(screened_emails, ScreenedIpAddressSerializer) + end + +end diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index 3a3c0a70a95..736a5d28102 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -127,7 +127,7 @@ class Admin::UsersController < Admin::AdminController user = User.where(id: params[:id]).first guardian.ensure_can_delete_user!(user) begin - if UserDestroyer.new(current_user).destroy(user, params.slice(:delete_posts, :block_email, :block_urls, :context)) + if UserDestroyer.new(current_user).destroy(user, params.slice(:delete_posts, :block_email, :block_urls, :block_ip, :context)) render json: {deleted: true} else render json: {deleted: false, user: AdminDetailedUserSerializer.new(user, root: false).as_json} diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 2eca5e70334..a1978ed6674 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -135,6 +135,7 @@ class UsersController < ApplicationController return fake_success_response if suspicious? params user = User.new_from_params(params) + user.ip_address = request.ip auth = authenticate_user(user, params) register_nickname(user) diff --git a/app/models/screened_ip_address.rb b/app/models/screened_ip_address.rb new file mode 100644 index 00000000000..30d3cafe3c4 --- /dev/null +++ b/app/models/screened_ip_address.rb @@ -0,0 +1,32 @@ +require_dependency 'screening_model' + +# A ScreenedIpAddress record represents an IP address or subnet that is being watched, +# and possibly blocked from creating accounts. +class ScreenedIpAddress < ActiveRecord::Base + + include ScreeningModel + + default_action :block + + validates :ip_address, presence: true, uniqueness: true + + def self.watch(ip_address, opts={}) + match_for_ip_address(ip_address) || create(opts.slice(:action_type).merge(ip_address: ip_address)) + end + + def self.match_for_ip_address(ip_address) + # The <<= operator on inet columns means "is contained within or equal to". + # + # Read more about PostgreSQL's inet data type here: + # + # http://www.postgresql.org/docs/9.1/static/datatype-net-types.html + # http://www.postgresql.org/docs/9.1/static/functions-net.html + where('? <<= ip_address', ip_address).first + end + + def self.should_block?(ip_address) + b = match_for_ip_address(ip_address) + b.record_match! if b + !!b and b.action_type == actions[:block] + end +end diff --git a/app/models/user.rb b/app/models/user.rb index aeab21cb254..a8367c10cde 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -51,6 +51,7 @@ class User < ActiveRecord::Base validates :email, presence: true, uniqueness: true validates :email, email: true, if: :email_changed? validate :password_validator + validates :ip_address, allowed_ip_address: {on: :create, message: :signup_not_allowed} before_save :cook before_save :update_username_lower diff --git a/app/serializers/screened_ip_address_serializer.rb b/app/serializers/screened_ip_address_serializer.rb new file mode 100644 index 00000000000..43dd91000d1 --- /dev/null +++ b/app/serializers/screened_ip_address_serializer.rb @@ -0,0 +1,12 @@ +class ScreenedIpAddressSerializer < ApplicationSerializer + attributes :ip_address, + :action, + :match_count, + :last_match_at, + :created_at + + def action + ScreenedIpAddress.actions.key(object.action_type).to_s + end + +end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index e48234933fe..ac44464e76c 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -983,7 +983,7 @@ en: take_action: "Take Action" notify_action: 'Notify' delete_spammer: "Delete Spammer" - delete_confirm: "You are about to delete %{posts} posts and %{topics} topics from this user, remove their account, and add their email address %{email} to a permanent block list. Are you sure this user is really a spammer?" + delete_confirm: "You are about to delete %{posts} posts and %{topics} topics from this user, remove their account, block signups from their IP address %{ip_address}, and add their email address %{email} to a permanent block list. Are you sure this user is really a spammer?" yes_delete_spammer: "Yes, Delete Spammer" cant: "Sorry, you can't flag this post at this time." custom_placeholder_notify_user: "Why does this post require you to speak to this user directly and privately? Be specific, be constructive, and always be kind." @@ -1257,6 +1257,10 @@ en: title: "Screened URLs" description: "The URLs listed here were used in posts by users who have been identified as spammers." url: "URL" + screened_ips: + title: "Screened IPs" + description: "IP addresses that are being watched." + allow: "allow" impersonate: title: "Impersonate User" @@ -1352,8 +1356,8 @@ en: one: "Users can't be deleted if they registered more than %{count} day ago, or if they have posts. Delete all posts before trying to delete a user." other: "Users can't be deleted if they registered more than %{count} days ago, or if they have posts. Delete all posts before trying to delete a user." delete_confirm: "Are you SURE you want to delete this user? This action is permanent!" - delete_and_block: "Yes, and block signups from this email address" - delete_dont_block: "Yes, but allow signups from this email address" + delete_and_block: "Yes, and block signups from this email and IP address" + delete_dont_block: "Yes, but allow signups from this email and IP address" deleted: "The user was deleted." delete_failed: "There was an error deleting that user. Make sure all posts are deleted before trying to delete the user." send_activation_email: "Send Activation Email" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 27d06d17c40..7a8655e3692 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -135,6 +135,8 @@ en: name: "Category Name" post: raw: "Body" + user: + ip_address: "" errors: messages: is_invalid: "is invalid; try to be a little more descriptive" @@ -144,6 +146,10 @@ en: attributes: archetype: cant_send_pm: "Sorry, you cannot send a private message to that user." + user: + attributes: + ip_address: + signup_not_allowed: "Signup is not allowed from this account." user_profile: no_info_me: "
the About Me field of your profile is currently blank, would you like to fill it out?
" @@ -788,6 +794,8 @@ en: email: not_allowed: "is not allowed from that email provider. Please use another email address." blocked: "is not allowed." + ip_address: + blocked: "is blocked." invite_mailer: subject_template: "[%{site_name}] %{invitee_name} invited you to join a discussion on %{site_name}" diff --git a/config/routes.rb b/config/routes.rb index 0d5d73415f4..57c0aff4edc 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -65,9 +65,10 @@ Discourse::Application.routes.draw do end scope '/logs' do - resources :staff_action_logs, only: [:index] - resources :screened_emails, only: [:index] - resources :screened_urls, only: [:index] + resources :staff_action_logs, only: [:index] + resources :screened_emails, only: [:index] + resources :screened_ip_addresses, only: [:index] + resources :screened_urls, only: [:index] end get 'customize' => 'site_customizations#index', constraints: AdminConstraint.new diff --git a/db/fixtures/screened_ip_addresses.rb b/db/fixtures/screened_ip_addresses.rb new file mode 100644 index 00000000000..0831e837111 --- /dev/null +++ b/db/fixtures/screened_ip_addresses.rb @@ -0,0 +1,30 @@ +ScreenedIpAddress.seed do |s| + s.id = 1 + s.ip_address = "10.0.0.0/8" + s.action_type = ScreenedIpAddress.actions[:do_nothing] +end + +ScreenedIpAddress.seed do |s| + s.id = 2 + s.ip_address = "192.168.0.0/16" + s.action_type = ScreenedIpAddress.actions[:do_nothing] +end + +ScreenedIpAddress.seed do |s| + s.id = 3 + s.ip_address = "127.0.0.0/8" + s.action_type = ScreenedIpAddress.actions[:do_nothing] +end + +ScreenedIpAddress.seed do |s| + s.id = 4 + s.ip_address = "172.16.0.0/12" + s.action_type = ScreenedIpAddress.actions[:do_nothing] +end + +# IPv6 +ScreenedIpAddress.seed do |s| + s.id = 5 + s.ip_address = "fc00::/7" + s.action_type = ScreenedIpAddress.actions[:do_nothing] +end diff --git a/db/migrate/20131017205954_create_screened_ip_addresses.rb b/db/migrate/20131017205954_create_screened_ip_addresses.rb new file mode 100644 index 00000000000..63968f05fff --- /dev/null +++ b/db/migrate/20131017205954_create_screened_ip_addresses.rb @@ -0,0 +1,13 @@ +class CreateScreenedIpAddresses < ActiveRecord::Migration + def change + create_table :screened_ip_addresses do |t| + t.column :ip_address, :inet, null: false + t.integer :action_type, null: false + t.integer :match_count, null: false, default: 0 + t.datetime :last_match_at + t.timestamps + end + add_index :screened_ip_addresses, :ip_address, unique: true + add_index :screened_ip_addresses, :last_match_at + end +end diff --git a/lib/user_destroyer.rb b/lib/user_destroyer.rb index b41417bbf9f..8f27154a9f3 100644 --- a/lib/user_destroyer.rb +++ b/lib/user_destroyer.rb @@ -37,6 +37,10 @@ class UserDestroyer b = ScreenedEmail.block(u.email, ip_address: u.ip_address) b.record_match! if b end + if opts[:block_ip] + b = ScreenedIpAddress.watch(u.ip_address) + b.record_match! if b + end Post.with_deleted.where(user_id: user.id).update_all("user_id = NULL") StaffActionLogger.new(@staff).log_user_deletion(user, opts.slice(:context)) DiscourseHub.unregister_nickname(user.username) if SiteSetting.call_discourse_hub? diff --git a/lib/validators/allowed_ip_address_validator.rb b/lib/validators/allowed_ip_address_validator.rb new file mode 100644 index 00000000000..61795e44e2f --- /dev/null +++ b/lib/validators/allowed_ip_address_validator.rb @@ -0,0 +1,9 @@ +class AllowedIpAddressValidator < ActiveModel::EachValidator + + def validate_each(record, attribute, value) + if record.ip_address and ScreenedIpAddress.should_block?(record.ip_address) + record.errors.add(attribute, options[:message] || I18n.t('user.ip_address.blocked')) + end + end + +end \ No newline at end of file diff --git a/spec/components/user_destroyer_spec.rb b/spec/components/user_destroyer_spec.rb index bde129ee0a2..bb5a48986cb 100644 --- a/spec/components/user_destroyer_spec.rb +++ b/spec/components/user_destroyer_spec.rb @@ -226,6 +226,18 @@ describe UserDestroyer do end end end + + context 'ip address screening' do + it "doesn't create screened_ip_address records by default" do + ScreenedIpAddress.expects(:watch).never + UserDestroyer.new(@admin).destroy(@user) + end + + it "creates new screened_ip_address records when block_ip is true" do + ScreenedIpAddress.expects(:watch).with(@user.ip_address).returns(stub_everything) + UserDestroyer.new(@admin).destroy(@user, {block_ip: true}) + end + end end end diff --git a/spec/components/validators/allowed_ip_address_validator_spec.rb b/spec/components/validators/allowed_ip_address_validator_spec.rb new file mode 100644 index 00000000000..99c47fba613 --- /dev/null +++ b/spec/components/validators/allowed_ip_address_validator_spec.rb @@ -0,0 +1,33 @@ +require 'spec_helper' + +describe AllowedIpAddressValidator do + + let(:record) { Fabricate.build(:user, ip_address: '99.232.23.123') } + let(:validator) { described_class.new({attributes: :ip_address}) } + subject(:validate) { validator.validate_each(record, :ip_address, record.ip_address) } + + context "ip address should be blocked" do + it 'should add an error' do + ScreenedIpAddress.stubs(:should_block?).returns(true) + validate + record.errors[:ip_address].should be_present + end + end + + context "ip address should not be blocked" do + it "shouldn't add an error" do + ScreenedIpAddress.stubs(:should_block?).returns(false) + validate + record.errors[:ip_address].should_not be_present + end + end + + context 'ip_address is nil' do + it "shouldn't add an error" do + ScreenedIpAddress.expects(:should_block?).never + record.ip_address = nil + validate + record.errors[:ip_address].should_not be_present + end + end +end \ No newline at end of file diff --git a/spec/controllers/admin/screened_ip_addresses_controller_spec.rb b/spec/controllers/admin/screened_ip_addresses_controller_spec.rb new file mode 100644 index 00000000000..a0100a19b62 --- /dev/null +++ b/spec/controllers/admin/screened_ip_addresses_controller_spec.rb @@ -0,0 +1,22 @@ +require 'spec_helper' + +describe Admin::ScreenedIpAddressesController do + it "is a subclass of AdminController" do + (Admin::ScreenedIpAddressesController < Admin::AdminController).should be_true + end + + let!(:user) { log_in(:admin) } + + context '.index' do + before do + xhr :get, :index + end + + subject { response } + it { should be_success } + + it 'returns JSON' do + ::JSON.parse(subject.body).should be_a(Array) + end + end +end diff --git a/spec/fabricators/screened_ip_address_fabricator.rb b/spec/fabricators/screened_ip_address_fabricator.rb new file mode 100644 index 00000000000..cfc57c29714 --- /dev/null +++ b/spec/fabricators/screened_ip_address_fabricator.rb @@ -0,0 +1,3 @@ +Fabricator(:screened_ip_address) do + ip_address { sequence(:ip_address) { |n| "123.#{(n*3)%255}.#{(n*2)%255}.#{n%255}" } } +end \ No newline at end of file diff --git a/spec/models/screened_ip_address_spec.rb b/spec/models/screened_ip_address_spec.rb new file mode 100644 index 00000000000..e973dd0f3fd --- /dev/null +++ b/spec/models/screened_ip_address_spec.rb @@ -0,0 +1,126 @@ +require 'spec_helper' + +describe ScreenedIpAddress do + let(:ip_address) { '99.232.23.124' } + let(:valid_params) { {ip_address: ip_address} } + + describe 'new record' do + it 'sets a default action_type' do + described_class.create(valid_params).action_type.should == described_class.actions[:block] + end + end + + describe '#watch' do + context 'ip_address is not being watched' do + it 'should create a new record' do + record = described_class.watch(ip_address) + record.should_not be_new_record + record.action_type.should == described_class.actions[:block] + end + + it 'lets action_type be overridden' do + record = described_class.watch(ip_address, action_type: described_class.actions[:do_nothing]) + record.should_not be_new_record + record.action_type.should == described_class.actions[:do_nothing] + end + + it "a record with subnet mask exists, but doesn't match" do + existing = Fabricate(:screened_ip_address, ip_address: '99.232.23.124/24') + expect { described_class.watch('99.232.55.124') }.to change { described_class.count } + end + + it "a record with exact matching exists, but doesn't match" do + existing = Fabricate(:screened_ip_address, ip_address: '99.232.23.124') + expect { described_class.watch('99.232.23.123') }.to change { described_class.count } + end + end + + context 'ip_address is already being watched' do + shared_examples 'exact match of ip address' do + it 'should not create a new record' do + expect { described_class.watch(ip_address_arg) }.to_not change { described_class.count } + end + + it 'returns the existing record' do + described_class.watch(ip_address_arg).should == existing + end + end + + context 'using exact match' do + let!(:existing) { Fabricate(:screened_ip_address) } + let(:ip_address_arg) { existing.ip_address } + include_examples 'exact match of ip address' + end + + context 'using subnet mask 255.255.255.0' do + let!(:existing) { Fabricate(:screened_ip_address, ip_address: '99.232.23.124/24') } + + context 'at exact address' do + let(:ip_address_arg) { '99.232.23.124' } + include_examples 'exact match of ip address' + end + + context 'at address in same subnet' do + let(:ip_address_arg) { '99.232.23.135' } + include_examples 'exact match of ip address' + end + end + end + + it "doesn't block 10.0.0.0/8" do + described_class.watch('10.0.0.0').action_type.should == described_class.actions[:do_nothing] + described_class.watch('10.0.0.1').action_type.should == described_class.actions[:do_nothing] + described_class.watch('10.10.125.111').action_type.should == described_class.actions[:do_nothing] + end + + it "doesn't block 192.168.0.0/16" do + described_class.watch('192.168.0.5').action_type.should == described_class.actions[:do_nothing] + described_class.watch('192.168.10.1').action_type.should == described_class.actions[:do_nothing] + end + + it "doesn't block 127.0.0.0/8" do + described_class.watch('127.0.0.1').action_type.should == described_class.actions[:do_nothing] + end + + it "doesn't block fc00::/7 addresses (IPv6)" do + described_class.watch('fd12:db8::ff00:42:8329').action_type.should == described_class.actions[:do_nothing] + end + + + end + + describe '#should_block?' do + it 'returns false when record does not exist' do + described_class.should_block?(ip_address).should eq(false) + end + + it 'returns false when no record matches' do + Fabricate(:screened_ip_address, ip_address: '111.234.23.11', action_type: described_class.actions[:block]) + described_class.should_block?('222.12.12.12').should eq(false) + end + + context 'IPv4' do + it 'returns false when when record matches and action is :do_nothing' do + Fabricate(:screened_ip_address, ip_address: '111.234.23.11', action_type: described_class.actions[:do_nothing]) + described_class.should_block?('111.234.23.11').should eq(false) + end + + it 'returns true when when record matches and action is :block' do + Fabricate(:screened_ip_address, ip_address: '111.234.23.11', action_type: described_class.actions[:block]) + described_class.should_block?('111.234.23.11').should eq(true) + end + end + + context 'IPv6' do + it 'returns false when when record matches and action is :do_nothing' do + Fabricate(:screened_ip_address, ip_address: '2001:db8::ff00:42:8329', action_type: described_class.actions[:do_nothing]) + described_class.should_block?('2001:db8::ff00:42:8329').should eq(false) + end + + it 'returns true when when record matches and action is :block' do + Fabricate(:screened_ip_address, ip_address: '2001:db8::ff00:42:8329', action_type: described_class.actions[:block]) + described_class.should_block?('2001:db8::ff00:42:8329').should eq(true) + end + end + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 253d51b4948..f543f6ad9f9 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -226,7 +226,6 @@ describe User do end end - context 'after_save' do before do subject.save @@ -238,6 +237,21 @@ describe User do end end + describe 'ip address validation' do + it 'validates ip_address for new users' do + u = Fabricate.build(:user) + AllowedIpAddressValidator.any_instance.expects(:validate_each).with(u, :ip_address, u.ip_address) + u.valid? + end + + it 'does not validate ip_address when updating an existing user' do + u = Fabricate(:user) + u.ip_address = '87.123.23.11' + AllowedIpAddressValidator.any_instance.expects(:validate_each).never + u.valid? + end + end + describe "trust levels" do # NOTE be sure to use build to avoid db calls