Discource-C
/
discourse
mirror of https://github.com/discourse/discourse.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Add screening by IP address. When deleting a user as a spammer, block all signups from the same IP address.

This commit is contained in:
Neil Lalonde 2013-10-21 14:49:51 -04:00
parent e527cbf884
commit 648b11a0eb
29 changed files with 455 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
app/assets/javascripts/admin/controllers/admin_logs_screened_ip_addresses_controller.js Normal file
View File

@ -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);
});
}
});

5
app/assets/javascripts/admin/models/admin_user.js
View File

@ -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() {

29
app/assets/javascripts/admin/models/screened_ip_address.js Normal file
View File

@ -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);
});
});
}
});

18
app/assets/javascripts/admin/routes/admin_logs_routes.js
View File

@ -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.

1
app/assets/javascripts/admin/routes/admin_routes.js
View File

@ -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' });
});

1
app/assets/javascripts/admin/templates/logs.js.handlebars
View File

@ -3,6 +3,7 @@
<ul class="nav nav-pills">
<li>{{#link-to 'adminLogs.staffActionLogs'}}{{i18n admin.logs.staff_actions.title}}{{/link-to}}</li>
<li>{{#link-to 'adminLogs.screenedEmails'}}{{i18n admin.logs.screened_emails.title}}{{/link-to}}</li>
<li>{{#link-to 'adminLogs.screenedIpAddresses'}}{{i18n admin.logs.screened_ips.title}}{{/link-to}}</li>
<li>{{#link-to 'adminLogs.screenedUrls'}}{{i18n admin.logs.screened_urls.title}}{{/link-to}}</li>
</ul>
</div>

24
app/assets/javascripts/admin/templates/logs/screened_ip_addresses.js.handlebars Normal file
View File

@ -0,0 +1,24 @@
<p>{{i18n admin.logs.screened_ips.description}}</p>
{{#if loading}}
<div class='admin-loading'>{{i18n loading}}</div>
{{else}}
{{#if model.length}}
<div class='table screened-ip-addresses'>
<div class="heading-container">
<div class="col heading first ip_address">{{i18n admin.logs.ip_address}}</div>
<div class="col heading action">{{i18n admin.logs.action}}</div>
<div class="col heading match_count">{{i18n admin.logs.match_count}}</div>
<div class="col heading last_match_at">{{i18n admin.logs.last_match_at}}</div>
<div class="col heading created_at">{{i18n admin.logs.created_at}}</div>
<div class="clearfix"></div>
</div>
{{view Discourse.ScreenedIpAddressesListView contentBinding="controller"}}
</div>
{{else}}
{{i18n search.no_results}}
{{/if}}
{{/if}}

10
app/assets/javascripts/admin/templates/logs/screened_ip_addresses_list_item.js.handlebars Normal file
View File

@ -0,0 +1,10 @@
<div class="col first ip_address">{{ip_address}}</div>
<div class="col action">{{actionName}}</div>
<div class="col match_count">{{match_count}}</div>
<div class="col last_match_at">
{{#if last_match_at}}
{{unboundAgeWithTooltip last_match_at}}
{{/if}}
</div>
<div class="col created_at">{{unboundAgeWithTooltip created_at}}</div>
<div class="clearfix"></div>

5
app/assets/javascripts/admin/views/logs/screened_ip_addresses_list_view.js Normal file
View File

@ -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"})
});

4
app/assets/stylesheets/common/admin/admin_base.scss
View File

@ -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;

8
app/controllers/admin/screened_ip_addresses_controller.rb Normal file
View File

@ -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

2
app/controllers/admin/users_controller.rb
View File

@ -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}

1
app/controllers/users_controller.rb
View File

@ -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)

32
app/models/screened_ip_address.rb Normal file
View File

@ -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

1
app/models/user.rb
View File

@ -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

12
app/serializers/screened_ip_address_serializer.rb Normal file
View File

@ -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

10
config/locales/client.en.yml
View File

@ -983,7 +983,7 @@ en:
take_action: "Take Action"
notify_action: 'Notify'
delete_spammer: "Delete Spammer"
delete_confirm: "You are about to delete <b>%{posts}</b> posts and <b>%{topics}</b> topics from this user, remove their account, and add their email address <b>%{email}</b> to a permanent block list. Are you sure this user is really a spammer?"
delete_confirm: "You are about to delete <b>%{posts}</b> posts and <b>%{topics}</b> topics from this user, remove their account, block signups from their IP address <b>%{ip_address}</b>, and add their email address <b>%{email}</b> 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: "<b>Yes</b>, and <b>block</b> signups from this email address"
delete_dont_block: "<b>Yes</b>, but <b>allow</b> signups from this email address"
delete_and_block: "<b>Yes</b>, and <b>block</b> signups from this email and IP address"
delete_dont_block: "<b>Yes</b>, but <b>allow</b> 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"

8
config/locales/server.en.yml
View File

@ -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: "<div class='missing-profile'>the About Me field of your profile is currently blank, <a href='/users/%{username_lower}/preferences/about-me'>would you like to fill it out?</a></div>"
@ -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}"

7
config/routes.rb
View File

@ -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

30
db/fixtures/screened_ip_addresses.rb Normal file
View File

@ -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

13
db/migrate/20131017205954_create_screened_ip_addresses.rb Normal file
View File

@ -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

4
lib/user_destroyer.rb
View File

@ -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?

9
lib/validators/allowed_ip_address_validator.rb Normal file
View File

@ -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

12
spec/components/user_destroyer_spec.rb
View File

@ -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

33
spec/components/validators/allowed_ip_address_validator_spec.rb Normal file
View File

@ -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

22
spec/controllers/admin/screened_ip_addresses_controller_spec.rb Normal file
View File

@ -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

3
spec/fabricators/screened_ip_address_fabricator.rb Normal file
View File

@ -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

126
spec/models/screened_ip_address_spec.rb Normal file
View File

@ -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

16
spec/models/user_spec.rb
View File

@ -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