Add ScreenedUrl. Rename BlockedEmail to ScreenedEmail.

This commit is contained in:
Neil Lalonde 2013-08-14 11:05:53 -04:00
parent 8fa9c51bf4
commit 86647f0a54
23 changed files with 251 additions and 90 deletions

View file

@ -235,6 +235,7 @@ Discourse.AdminUser = Discourse.User.extend({
var formData = { context: window.location.pathname }; var formData = { context: window.location.pathname };
if (block) { if (block) {
formData["block_email"] = true; formData["block_email"] = true;
formData["block_urls"] = true;
} }
Discourse.ajax("/admin/users/" + user.get('id') + '.json', { Discourse.ajax("/admin/users/" + user.get('id') + '.json', {
type: 'DELETE', type: 'DELETE',
@ -292,7 +293,7 @@ Discourse.AdminUser = Discourse.User.extend({
"callback": function() { "callback": function() {
Discourse.ajax("/admin/users/" + user.get('id') + '.json', { Discourse.ajax("/admin/users/" + user.get('id') + '.json', {
type: 'DELETE', type: 'DELETE',
data: {delete_posts: true, block_email: true, context: window.location.pathname} data: {delete_posts: true, block_email: true, block_urls: true, context: window.location.pathname}
}).then(function(data) { }).then(function(data) {
if (data.deleted) { if (data.deleted) {
bootbox.alert(I18n.t("admin.user.deleted"), function() { bootbox.alert(I18n.t("admin.user.deleted"), function() {

View file

@ -1,8 +0,0 @@
class Admin::BlockedEmailsController < Admin::AdminController
def index
blocked_emails = BlockedEmail.limit(200).order('last_match_at desc').to_a
render_serialized(blocked_emails, BlockedEmailSerializer)
end
end

View file

@ -0,0 +1,8 @@
class Admin::ScreenedEmailsController < Admin::AdminController
def index
screened_emails = ScreenedEmail.limit(200).order('last_match_at desc').to_a
render_serialized(screened_emails, ScreenedEmailSerializer)
end
end

View file

@ -118,7 +118,7 @@ class Admin::UsersController < Admin::AdminController
user = User.where(id: params[:id]).first user = User.where(id: params[:id]).first
guardian.ensure_can_delete_user!(user) guardian.ensure_can_delete_user!(user)
begin begin
if UserDestroyer.new(current_user).destroy(user, params.slice(:delete_posts, :block_email, :context)) if UserDestroyer.new(current_user).destroy(user, params.slice(:delete_posts, :block_email, :block_urls, :context))
render json: {deleted: true} render json: {deleted: true}
else else
render json: {deleted: false, user: AdminDetailedUserSerializer.new(user, root: false).as_json} render json: {deleted: false, user: AdminDetailedUserSerializer.new(user, root: false).as_json}

View file

@ -1,54 +0,0 @@
# A BlockedEmail record represents an email address that is being watched,
# typically when creating a new User account. If the email of the signup form
# (or some other form) matches a BlockedEmail record, an action can be
# performed based on the action_type.
class BlockedEmail < ActiveRecord::Base
before_validation :set_defaults
validates :email, presence: true, uniqueness: true
def self.actions
@actions ||= Enum.new(:block, :do_nothing)
end
def self.block(email, opts={})
find_by_email(email) || create(opts.slice(:action_type).merge({email: email}))
end
def self.should_block?(email)
blocked_email = BlockedEmail.where(email: email).first
blocked_email.record_match! if blocked_email
blocked_email && blocked_email.action_type == actions[:block]
end
def set_defaults
self.action_type ||= BlockedEmail.actions[:block]
end
def record_match!
self.match_count += 1
self.last_match_at = Time.zone.now
save
end
end
# == Schema Information
#
# Table name: blocked_emails
#
# id :integer not null, primary key
# email :string(255) not null
# action_type :integer not null
# match_count :integer default(0), not null
# last_match_at :datetime
# created_at :datetime not null
# updated_at :datetime not null
#
# Indexes
#
# index_blocked_emails_on_email (email) UNIQUE
# index_blocked_emails_on_last_match_at (last_match_at)
#

View file

@ -0,0 +1,25 @@
require_dependency 'screening_model'
# A ScreenedEmail record represents an email address that is being watched,
# typically when creating a new User account. If the email of the signup form
# (or some other form) matches a ScreenedEmail record, an action can be
# performed based on the action_type.
class ScreenedEmail < ActiveRecord::Base
include ScreeningModel
default_action :block
validates :email, presence: true, uniqueness: true
def self.block(email, opts={})
find_by_email(email) || create(opts.slice(:action_type).merge({email: email}))
end
def self.should_block?(email)
screened_email = ScreenedEmail.where(email: email).first
screened_email.record_match! if screened_email
screened_email && screened_email.action_type == actions[:block]
end
end

View file

@ -0,0 +1,26 @@
require_dependency 'screening_model'
# A ScreenedUrl record represents a URL that is being watched.
# If the URL is found in a post, some action can be performed.
# For now, nothing is done. We're just collecting the data and will decide
# what to do with it later.
class ScreenedUrl < ActiveRecord::Base
include ScreeningModel
default_action :do_nothing
before_validation :strip_http
validates :url, presence: true, uniqueness: true
validates :domain, presence: true
def strip_http
self.url.gsub!(/http(s?):\/\//i, '')
end
def self.watch(url, domain, opts={})
find_by_url(url) || create(opts.slice(:action_type).merge(url: url, domain: domain))
end
end

View file

@ -1,4 +1,4 @@
class BlockedEmailSerializer < ApplicationSerializer class ScreenedEmailSerializer < ApplicationSerializer
attributes :email, attributes :email,
:action, :action,
:match_count, :match_count,
@ -6,6 +6,6 @@ class BlockedEmailSerializer < ApplicationSerializer
:created_at :created_at
def action def action
BlockedEmail.actions.key(object.action_type).to_s ScreenedEmail.actions.key(object.action_type).to_s
end end
end end

View file

@ -63,7 +63,7 @@ Discourse::Application.routes.draw do
end end
scope '/logs' do scope '/logs' do
resources :blocked_emails, only: [:index, :create, :update, :destroy] resources :screened_emails, only: [:index, :create, :update, :destroy]
resources :staff_action_logs, only: [:index, :create, :update, :destroy] resources :staff_action_logs, only: [:index, :create, :update, :destroy]
end end

View file

@ -0,0 +1,14 @@
class CreateScreenedUrls < ActiveRecord::Migration
def change
create_table :screened_urls do |t|
t.string :url, null: false
t.string :domain, 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_urls, :url, unique: true
add_index :screened_urls, :last_match_at
end
end

View file

@ -0,0 +1,5 @@
class RenameBlockedEmailsToScreenedEmails < ActiveRecord::Migration
def change
rename_table :blocked_emails, :screened_emails
end
end

View file

@ -31,6 +31,10 @@ module Oneboxer
1.day 1.day
end end
def self.oneboxer_exists_for_url?(url)
Whitelist.entry_for_url(url) || matchers.any? { |matcher| url =~ matcher.regexp }
end
# Return a oneboxer for a given URL # Return a oneboxer for a given URL
def self.onebox_for_url(url) def self.onebox_for_url(url)
matchers.each do |matcher| matchers.each do |matcher|

31
lib/screening_model.rb Normal file
View file

@ -0,0 +1,31 @@
module ScreeningModel
extend ActiveSupport::Concern
module ClassMethods
def actions
@actions ||= Enum.new(:block, :do_nothing)
end
def default_action(action_key)
@default_action = action_key
end
def df_action
@default_action || :do_nothing
end
end
included do
before_validation :set_default_action
end
def set_default_action
self.action_type ||= self.class.actions[self.class.df_action]
end
def record_match!
self.match_count += 1
self.last_match_at = Time.zone.now
save
end
end

View file

@ -17,6 +17,13 @@ class UserDestroyer
User.transaction do User.transaction do
if opts[:delete_posts] if opts[:delete_posts]
user.posts.each do |post| user.posts.each do |post|
if opts[:block_urls]
post.topic_links.each do |link|
unless link.internal or Oneboxer.oneboxer_exists_for_url?(link.url)
ScreenedUrl.watch(link.url, link.domain).try(:record_match!)
end
end
end
PostDestroyer.new(@staff, post).destroy PostDestroyer.new(@staff, post).destroy
end end
raise PostsExistError if user.reload.post_count != 0 raise PostsExistError if user.reload.post_count != 0
@ -24,7 +31,7 @@ class UserDestroyer
user.destroy.tap do |u| user.destroy.tap do |u|
if u if u
if opts[:block_email] if opts[:block_email]
b = BlockedEmail.block(u.email) b = ScreenedEmail.block(u.email)
b.record_match! if b b.record_match! if b
end end
Post.with_deleted.where(user_id: user.id).update_all("nuked_user = true") Post.with_deleted.where(user_id: user.id).update_all("nuked_user = true")

View file

@ -10,7 +10,7 @@ class EmailValidator < ActiveModel::EachValidator
record.errors.add(attribute, I18n.t(:'user.email.not_allowed')) record.errors.add(attribute, I18n.t(:'user.email.not_allowed'))
end end
end end
if record.errors[attribute].blank? and BlockedEmail.should_block?(value) if record.errors[attribute].blank? and ScreenedEmail.should_block?(value)
record.errors.add(attribute, I18n.t(:'user.email.blocked')) record.errors.add(attribute, I18n.t(:'user.email.blocked'))
end end
end end

View file

@ -74,13 +74,13 @@ describe UserDestroyer do
shared_examples "email block list" do shared_examples "email block list" do
it "doesn't add email to block list by default" do it "doesn't add email to block list by default" do
BlockedEmail.expects(:block).never ScreenedEmail.expects(:block).never
destroy destroy
end end
it "adds email to block list if block_email is true" do it "adds email to block list if block_email is true" do
b = Fabricate.build(:blocked_email, email: @user.email) b = Fabricate.build(:screened_email, email: @user.email)
BlockedEmail.expects(:block).with(@user.email).returns(b) ScreenedEmail.expects(:block).with(@user.email).returns(b)
b.expects(:record_match!).once.returns(true) b.expects(:record_match!).once.returns(true)
UserDestroyer.new(@admin).destroy(@user, destroy_opts.merge({block_email: true})) UserDestroyer.new(@admin).destroy(@user, destroy_opts.merge({block_email: true}))
end end
@ -164,6 +164,50 @@ describe UserDestroyer do
end end
end end
end end
context 'user has posts with links' do
context 'external links' do
before do
@post = Fabricate(:post_with_external_links, user: @user)
TopicLink.extract_from(@post)
end
it "doesn't add ScreenedUrl records by default" do
ScreenedUrl.expects(:watch).never
UserDestroyer.new(@admin).destroy(@user, {delete_posts: true})
end
it "adds ScreenedUrl records when :block_urls is true" do
ScreenedUrl.expects(:watch).at_least_once
UserDestroyer.new(@admin).destroy(@user, {delete_posts: true, block_urls: true})
end
end
context 'internal links' do
before do
@post = Fabricate(:post_with_external_links, user: @user)
TopicLink.extract_from(@post)
TopicLink.any_instance.stubs(:internal).returns(true)
end
it "doesn't add ScreenedUrl records" do
ScreenedUrl.expects(:watch).never
UserDestroyer.new(@admin).destroy(@user, {delete_posts: true, block_urls: true})
end
end
context 'with oneboxed links' do
before do
@post = Fabricate(:post_with_youtube, user: @user)
TopicLink.extract_from(@post)
end
it "doesn't add ScreenedUrl records" do
ScreenedUrl.expects(:watch).never
UserDestroyer.new(@admin).destroy(@user, {delete_posts: true, block_urls: true})
end
end
end
end end
end end

View file

@ -8,13 +8,13 @@ describe EmailValidator do
context "blocked email" do context "blocked email" do
it "doesn't add an error when email doesn't match a blocked email" do it "doesn't add an error when email doesn't match a blocked email" do
BlockedEmail.stubs(:should_block?).with(record.email).returns(false) ScreenedEmail.stubs(:should_block?).with(record.email).returns(false)
validate validate
record.errors[:email].should_not be_present record.errors[:email].should_not be_present
end end
it "adds an error when email matches a blocked email" do it "adds an error when email matches a blocked email" do
BlockedEmail.stubs(:should_block?).with(record.email).returns(true) ScreenedEmail.stubs(:should_block?).with(record.email).returns(true)
validate validate
record.errors[:email].should be_present record.errors[:email].should be_present
end end

View file

@ -1,8 +1,8 @@
require 'spec_helper' require 'spec_helper'
describe Admin::BlockedEmailsController do describe Admin::ScreenedEmailsController do
it "is a subclass of AdminController" do it "is a subclass of AdminController" do
(Admin::BlockedEmailsController < Admin::AdminController).should be_true (Admin::ScreenedEmailsController < Admin::AdminController).should be_true
end end
let!(:user) { log_in(:admin) } let!(:user) { log_in(:admin) }

View file

@ -1,4 +0,0 @@
Fabricator(:blocked_email) do
email { sequence(:email) { |n| "bad#{n}@spammers.org" } }
action_type BlockedEmail.actions[:block]
end

View file

@ -0,0 +1,4 @@
Fabricator(:screened_email) do
email { sequence(:email) { |n| "bad#{n}@spammers.org" } }
action_type ScreenedEmail.actions[:block]
end

View file

@ -0,0 +1,5 @@
Fabricator(:screened_url) do
url { sequence(:url) { |n| "spammers#{n}.org/buy/stuff" } }
domain { sequence(:domain) { |n| "spammers#{n}.org" } }
action_type ScreenedEmail.actions[:do_nothing]
end

View file

@ -1,6 +1,6 @@
require 'spec_helper' require 'spec_helper'
describe BlockedEmail do describe ScreenedEmail do
let(:email) { 'block@spamfromhome.org' } let(:email) { 'block@spamfromhome.org' }
@ -34,7 +34,7 @@ describe BlockedEmail do
end end
context 'email is already being blocked' do context 'email is already being blocked' do
let!(:existing) { Fabricate(:blocked_email, email: email) } let!(:existing) { Fabricate(:screened_email, email: email) }
it "doesn't create a new record" do it "doesn't create a new record" do
expect { described_class.block(email) }.to_not change { described_class.count } expect { described_class.block(email) }.to_not change { described_class.count }
@ -53,25 +53,25 @@ describe BlockedEmail do
subject.should be_false subject.should be_false
end end
shared_examples "when a BlockedEmail record matches" do shared_examples "when a ScreenedEmail record matches" do
it "updates statistics" do it "updates statistics" do
Timecop.freeze(Time.zone.now) do Timecop.freeze(Time.zone.now) do
expect { subject }.to change { blocked_email.reload.match_count }.by(1) expect { subject }.to change { screened_email.reload.match_count }.by(1)
blocked_email.last_match_at.should be_within_one_second_of(Time.zone.now) screened_email.last_match_at.should be_within_one_second_of(Time.zone.now)
end end
end end
end end
context "action_type is :block" do context "action_type is :block" do
let!(:blocked_email) { Fabricate(:blocked_email, email: email, action_type: BlockedEmail.actions[:block]) } let!(:screened_email) { Fabricate(:screened_email, email: email, action_type: described_class.actions[:block]) }
it { should be_true } it { should be_true }
include_examples "when a BlockedEmail record matches" include_examples "when a ScreenedEmail record matches"
end end
context "action_type is :do_nothing" do context "action_type is :do_nothing" do
let!(:blocked_email) { Fabricate(:blocked_email, email: email, action_type: BlockedEmail.actions[:do_nothing]) } let!(:screened_email) { Fabricate(:screened_email, email: email, action_type: described_class.actions[:do_nothing]) }
it { should be_false } it { should be_false }
include_examples "when a BlockedEmail record matches" include_examples "when a ScreenedEmail record matches"
end end
end end

View file

@ -0,0 +1,53 @@
require 'spec_helper'
describe ScreenedUrl do
let(:url) { 'http://shopppping.com/bad/drugz' }
let(:domain) { 'shopppping.com' }
let(:valid_params) { {url: url, domain: domain} }
describe "new record" do
it "sets a default action_type" do
described_class.create(valid_params).action_type.should == described_class.actions[:do_nothing]
end
it "last_match_at is null" do
described_class.create(valid_params).last_match_at.should be_nil
end
['http://', 'HTTP://', 'https://', 'HTTPS://'].each do |prefix|
it "strips #{prefix}" do
described_class.create(valid_params.merge(url: url.gsub('http://', prefix))).url.should == url.gsub('http://', '')
end
end
end
describe '#watch' do
context 'url is not being blocked' do
it 'creates a new record with default action of :do_nothing' do
record = described_class.watch(url, domain)
record.should_not be_new_record
record.action_type.should == described_class.actions[:do_nothing]
end
it 'lets action_type be overriden' do
record = described_class.watch(url, domain, action_type: described_class.actions[:block])
record.should_not be_new_record
record.action_type.should == described_class.actions[:block]
end
end
context 'url is already being blocked' do
let!(:existing) { Fabricate(:screened_url, url: url, domain: domain) }
it "doesn't create a new record" do
expect { described_class.watch(url, domain) }.to_not change { described_class.count }
end
it "returns the existing record" do
described_class.watch(url, domain).should == existing
end
end
end
end