Drop dmail filters.

Few people used dmail filters (~900 users in 5 years) and even fewer
used them correctly. Most people used them to try to block dmail spam,
but usually they either blocked too much (by adding common words that
are present in nearly all dmails, causing all mails to them to be
filtered) or too little (blocking specific email addresses or urls,
which usually are never seen again after the spammer is banned).
Nowadays the spam detection system does a better job of filtering spam.
This commit is contained in:
evazion
2020-01-19 12:55:58 -06:00
parent c2688e3aff
commit cae9a5d7e3
15 changed files with 11 additions and 317 deletions

View File

@@ -1,33 +0,0 @@
module Maintenance
module User
class DmailFiltersController < ApplicationController
before_action :ensure_ownership
respond_to :html, :json, :xml
def edit
@dmail_filter = CurrentUser.dmail_filter || DmailFilter.new
end
def update
@dmail_filter = CurrentUser.dmail_filter || DmailFilter.new
@dmail_filter.update(dmail_filter_params)
flash[:notice] = "Filter updated"
respond_with(@dmail)
end
private
def ensure_ownership
@dmail = Dmail.find(params[:dmail_id])
if @dmail.owner_id != CurrentUser.user.id
raise ::User::PrivilegeError.new
end
end
def dmail_filter_params
params.require(:dmail_filter).permit(:words)
end
end
end
end

View File

@@ -118,7 +118,6 @@ class UsersController < ApplicationController
enable_recommended_posts opt_out_tracking enable_recommended_posts opt_out_tracking
] ]
permitted_params += [dmail_filter_attributes: %i[id words]]
permitted_params << :name if context == :create permitted_params << :name if context == :create
permitted_params << :level if CurrentUser.is_admin? permitted_params << :level if CurrentUser.is_admin?

View File

@@ -14,7 +14,6 @@ class Dmail < ApplicationRecord
belongs_to :from, :class_name => "User" belongs_to :from, :class_name => "User"
after_initialize :initialize_attributes, if: :new_record? after_initialize :initialize_attributes, if: :new_record?
before_create :auto_read_if_filtered
after_create :update_recipient after_create :update_recipient
after_commit :send_email, on: :create after_commit :send_email, on: :create
@@ -185,16 +184,6 @@ class Dmail < ApplicationRecord
owner == to owner == to
end end
def filtered?
CurrentUser.dmail_filter.try(:filtered?, self)
end
def auto_read_if_filtered
if is_recipient? && to.dmail_filter.try(:filtered?, self)
self.is_read = true
end
end
def update_recipient def update_recipient
if is_recipient? && !is_deleted? && !is_read? if is_recipient? && !is_deleted? && !is_read?
to.update(has_mail: true, unread_dmail_count: to.dmails.unread.count) to.update(has_mail: true, unread_dmail_count: to.dmails.unread.count)

View File

@@ -1,27 +0,0 @@
class DmailFilter < ApplicationRecord
extend Memoist
belongs_to :user
before_validation :initialize_user
def initialize_user
unless user_id
self.user_id = CurrentUser.user.id
end
end
def filtered?(dmail)
dmail.from.level < User::Levels::MODERATOR && has_filter? && (dmail.body.match?(regexp) || dmail.title.match?(regexp) || dmail.from.name.match?(regexp))
end
def has_filter?
!words.strip.empty?
end
def regexp
union = words.split(/[[:space:]]+/).map { |word| Regexp.escape(word) }.join("|")
/\b#{union}\b/i
end
memoize :regexp
end

View File

@@ -107,7 +107,6 @@ class User < ApplicationRecord
has_one :recent_ban, -> {order("bans.id desc")}, :class_name => "Ban" has_one :recent_ban, -> {order("bans.id desc")}, :class_name => "Ban"
has_one :api_key has_one :api_key
has_one :dmail_filter
has_one :super_voter has_one :super_voter
has_one :token_bucket has_one :token_bucket
has_many :notes, foreign_key: :creator_id has_many :notes, foreign_key: :creator_id
@@ -122,7 +121,6 @@ class User < ApplicationRecord
has_many :tag_aliases, foreign_key: :creator_id has_many :tag_aliases, foreign_key: :creator_id
has_many :tag_implications, foreign_key: :creator_id has_many :tag_implications, foreign_key: :creator_id
belongs_to :inviter, class_name: "User", optional: true belongs_to :inviter, class_name: "User", optional: true
accepts_nested_attributes_for :dmail_filter
enum theme: { light: 0, dark: 100 }, _suffix: true enum theme: { light: 0, dark: 100 }, _suffix: true

View File

@@ -15,21 +15,13 @@
<%= compact_time(dmail.created_at) %> <%= compact_time(dmail.created_at) %>
<% end %> <% end %>
<% t.column "From" do |dmail| %> <% t.column "From" do |dmail| %>
<% if dmail.filtered? %> <%= link_to_user dmail.from %>
<%= link_to "[filtered]", user_path(dmail.from) %>
<% else %>
<%= link_to_user dmail.from %>
<% end %>
<% end %> <% end %>
<% t.column "To" do |dmail| %> <% t.column "To" do |dmail| %>
<%= link_to_user dmail.to %> <%= link_to_user dmail.to %>
<% end %> <% end %>
<% t.column "Subject", td: { class: "col-expand" } do |dmail| %> <% t.column "Subject", td: { class: "col-expand" } do |dmail| %>
<% if dmail.filtered? %> <%= link_to dmail.title, dmail_path(dmail) %>
<%= link_to "[filtered]", dmail_path(dmail) %>
<% else %>
<%= link_to dmail.title, dmail_path(dmail) %>
<% end %>
<% end %> <% end %>
<% t.column do |dmail| %> <% t.column do |dmail| %>
<%= link_to "delete", dmail_path(dmail), :method => :delete, :data => {:confirm => "Are you sure you want to delete this Dmail?"} %> <%= link_to "delete", dmail_path(dmail), :method => :delete, :data => {:confirm => "Are you sure you want to delete this Dmail?"} %>

View File

@@ -24,7 +24,6 @@
<p> <p>
<%= link_to "Respond", new_dmail_path(:respond_to_id => @dmail) %> <%= link_to "Respond", new_dmail_path(:respond_to_id => @dmail) %>
| <%= link_to "Forward", new_dmail_path(:respond_to_id => @dmail, :forward => true) %> | <%= link_to "Forward", new_dmail_path(:respond_to_id => @dmail, :forward => true) %>
| <%= link_to "Filter messages like these", edit_maintenance_user_dmail_filter_path(:dmail_id => @dmail.id) %>
| <%= link_to "Permalink", dmail_path(@dmail, :key => @dmail.key), :title => "Use this URL to privately share with a moderator" %> | <%= link_to "Permalink", dmail_path(@dmail, :key => @dmail.key), :title => "Use this URL to privately share with a moderator" %>
<% if CurrentUser.is_gold? %> <% if CurrentUser.is_gold? %>
<span id="spam-links"> <span id="spam-links">

View File

@@ -1,34 +0,0 @@
<div id="c-maintenance-user-dmail-filters">
<div id="a-edit">
<h1>Edit Message Filters</h1>
<div>
<h2><%= @dmail.title %></h2>
<ul style="margin-bottom: 1em;">
<li><strong>Sender</strong>: <%= link_to_user @dmail.from %></li>
<li><strong>Recipient</strong>: <%= link_to_user @dmail.to, :raw => true %></li>
<li><strong>Date</strong>: <%= compact_time(@dmail.created_at) %></li>
</ul>
<h3>Body</h3>
<div class="prose">
<%= format_text(@dmail.body) %>
</div>
</div>
<%= edit_form_for @dmail_filter, :url => maintenance_user_dmail_filter_path(:dmail_id => @dmail.id) do |f| %>
<div class="input text optional field_with_hint">
<label class="text" for="dmail_filter_words">Banned Words</label>
<%= text_area_tag "dmail_filter[words]", @dmail_filter.words, :id => "dmail_filter_words", :class => "text", :style => "height: 10em;" %>
<p class="hint">A list of banned words or users (space delimited). Any message you receive with a banned word will automatically be deleted. Make sure user names have no spaces in them (replace with underscores).</p>
</div>
<%= f.button :submit, "Submit" %>
<% end %>
</div>
</div>
<% content_for(:page_title) do %>
Edit Message Filters - <%= Danbooru.config.app_name %>
<% end %>

View File

@@ -65,13 +65,6 @@
<%= f.input :disable_responsive_mode, :as => :select, :collection => [["No", "false"], ["Yes", "true"]], :include_blank => false, :hint => "Disable alternative layout for mobile and tablet" %> <%= f.input :disable_responsive_mode, :as => :select, :collection => [["No", "false"], ["Yes", "true"]], :include_blank => false, :hint => "Disable alternative layout for mobile and tablet" %>
<%= f.input :opt_out_tracking, :as => :select, :collection => [["No", "false"], ["Yes", "true"]], :include_blank => false, :hint => "Opt out of tracking" %> <%= f.input :opt_out_tracking, :as => :select, :collection => [["No", "false"], ["Yes", "true"]], :include_blank => false, :hint => "Opt out of tracking" %>
<div class="input text optional field_with_hint">
<label class="text optional" for="user_dmail_filter_attributes_words">Dmail filter</label>
<%= hidden_field_tag "user[dmail_filter_attributes][id]", @user.dmail_filter.try(:id) %>
<%= text_field_tag "user[dmail_filter_attributes][words]", @user.dmail_filter.try(:words), :id => "user_dmail_filter_attributes_words", :class => "text optional", :size => 40 %>
<span class="hint">A list of banned words (space delimited). Any dmail you receive with a banned word will automatically be deleted.</span>
</div>
<%= f.input :favorite_tags, :label => "Frequent tags", :hint => "A list of tags that you use often. They will appear when using the list of Related Tags.", :input_html => { :rows => 5, :data => { :autocomplete => "tag-query" } } %> <%= f.input :favorite_tags, :label => "Frequent tags", :hint => "A list of tags that you use often. They will appear when using the list of Related Tags.", :input_html => { :rows => 5, :data => { :autocomplete => "tag-query" } } %>
<%= f.input :custom_style, :label => "Custom <a href='https://en.wikipedia.org/wiki/Cascading_Style_Sheets'>CSS</a> style".html_safe, :hint => "CSS rules to apply to the whole site.", :input_html => {:size => "40x5"} %> <%= f.input :custom_style, :label => "Custom <a href='https://en.wikipedia.org/wiki/Cascading_Style_Sheets'>CSS</a> style".html_safe, :hint => "CSS rules to apply to the whole site.", :input_html => {:size => "40x5"} %>
</fieldset> </fieldset>

View File

@@ -59,7 +59,6 @@ Rails.application.routes.draw do
resource :password_reset, :only => [:new, :create, :edit, :update] resource :password_reset, :only => [:new, :create, :edit, :update]
resource :deletion, :only => [:show, :destroy] resource :deletion, :only => [:show, :destroy]
resource :email_change, :only => [:new, :create] resource :email_change, :only => [:new, :create]
resource :dmail_filter, :only => [:edit, :update]
resource :api_key, :only => [:show, :view, :update, :destroy] do resource :api_key, :only => [:show, :view, :update, :destroy] do
post :view post :view
end end

View File

@@ -0,0 +1,7 @@
require_relative "20141120045943_create_dmail_filters"
class DropDmailFilters < ActiveRecord::Migration[6.0]
def change
revert CreateDmailFilters
end
end

View File

@@ -807,38 +807,6 @@ CREATE SEQUENCE public.delayed_jobs_id_seq
ALTER SEQUENCE public.delayed_jobs_id_seq OWNED BY public.delayed_jobs.id; ALTER SEQUENCE public.delayed_jobs_id_seq OWNED BY public.delayed_jobs.id;
--
-- Name: dmail_filters; Type: TABLE; Schema: public; Owner: -
--
CREATE TABLE public.dmail_filters (
id integer NOT NULL,
user_id integer NOT NULL,
words text NOT NULL,
created_at timestamp without time zone NOT NULL,
updated_at timestamp without time zone NOT NULL
);
--
-- Name: dmail_filters_id_seq; Type: SEQUENCE; Schema: public; Owner: -
--
CREATE SEQUENCE public.dmail_filters_id_seq
START WITH 1
INCREMENT BY 1
NO MINVALUE
NO MAXVALUE
CACHE 1;
--
-- Name: dmail_filters_id_seq; Type: SEQUENCE OWNED BY; Schema: public; Owner: -
--
ALTER SEQUENCE public.dmail_filters_id_seq OWNED BY public.dmail_filters.id;
-- --
-- Name: dmails; Type: TABLE; Schema: public; Owner: - -- Name: dmails; Type: TABLE; Schema: public; Owner: -
-- --
@@ -3330,13 +3298,6 @@ ALTER TABLE ONLY public.comments ALTER COLUMN id SET DEFAULT nextval('public.com
ALTER TABLE ONLY public.delayed_jobs ALTER COLUMN id SET DEFAULT nextval('public.delayed_jobs_id_seq'::regclass); ALTER TABLE ONLY public.delayed_jobs ALTER COLUMN id SET DEFAULT nextval('public.delayed_jobs_id_seq'::regclass);
--
-- Name: dmail_filters id; Type: DEFAULT; Schema: public; Owner: -
--
ALTER TABLE ONLY public.dmail_filters ALTER COLUMN id SET DEFAULT nextval('public.dmail_filters_id_seq'::regclass);
-- --
-- Name: dmails id; Type: DEFAULT; Schema: public; Owner: - -- Name: dmails id; Type: DEFAULT; Schema: public; Owner: -
-- --
@@ -4385,14 +4346,6 @@ ALTER TABLE ONLY public.delayed_jobs
ADD CONSTRAINT delayed_jobs_pkey PRIMARY KEY (id); ADD CONSTRAINT delayed_jobs_pkey PRIMARY KEY (id);
--
-- Name: dmail_filters dmail_filters_pkey; Type: CONSTRAINT; Schema: public; Owner: -
--
ALTER TABLE ONLY public.dmail_filters
ADD CONSTRAINT dmail_filters_pkey PRIMARY KEY (id);
-- --
-- Name: dmails dmails_pkey; Type: CONSTRAINT; Schema: public; Owner: - -- Name: dmails dmails_pkey; Type: CONSTRAINT; Schema: public; Owner: -
-- --
@@ -4932,13 +4885,6 @@ CREATE INDEX index_delayed_jobs_on_locked_by ON public.delayed_jobs USING btree
CREATE INDEX index_delayed_jobs_on_run_at ON public.delayed_jobs USING btree (run_at); CREATE INDEX index_delayed_jobs_on_run_at ON public.delayed_jobs USING btree (run_at);
--
-- Name: index_dmail_filters_on_user_id; Type: INDEX; Schema: public; Owner: -
--
CREATE UNIQUE INDEX index_dmail_filters_on_user_id ON public.dmail_filters USING btree (user_id);
-- --
-- Name: index_dmails_on_created_at; Type: INDEX; Schema: public; Owner: - -- Name: index_dmails_on_created_at; Type: INDEX; Schema: public; Owner: -
-- --
@@ -7479,6 +7425,7 @@ INSERT INTO "schema_migrations" (version) VALUES
('20200114204550'), ('20200114204550'),
('20200115010442'), ('20200115010442'),
('20200117220602'), ('20200117220602'),
('20200118015014'); ('20200118015014'),
('20200119184442');

View File

@@ -1,35 +0,0 @@
require "test_helper"
module Maintenance
module User
class DmailFiltersControllerTest < ActionDispatch::IntegrationTest
context "The dmail filters controller" do
setup do
@user1 = create(:user)
@user2 = create(:user)
end
context "update action" do
setup do
as(@user1) do
@dmail = create(:dmail, owner: @user1)
end
end
should "not allow a user to create a filter belonging to another user" do
params = {
:dmail_id => @dmail.id,
:dmail_filter => {
:words => "owned",
:user_id => @user2.id
}
}
put_auth maintenance_user_dmail_filter_path, @user1, params: params
assert_not_equal("owned", @user2.reload.dmail_filter.try(&:words))
end
end
end
end
end
end

View File

@@ -1,57 +0,0 @@
require 'test_helper'
class DmailFilterTest < ActiveSupport::TestCase
def setup
super
@receiver = FactoryBot.create(:user)
@sender = FactoryBot.create(:user)
end
def create_dmail(body, title)
CurrentUser.scoped(@sender, "127.0.0.1") do
Dmail.create_split(:to_id => @receiver.id, :body => body, :title => title)
end
end
context "a dmail filter for a word" do
setup do
@dmail_filter = @receiver.create_dmail_filter(:words => "banned")
end
should "filter on that word in the body" do
create_dmail("banned", "okay")
assert_equal(true, @receiver.dmails.last.is_read?)
end
should "filter on that word in the title" do
create_dmail("okay", "banned")
assert_equal(true, @receiver.dmails.last.is_read?)
end
should "be case insensitive" do
create_dmail("Banned.", "okay")
assert_equal(true, @receiver.dmails.last.is_read?)
end
end
context "a dmail filter for a user name" do
setup do
@dmail_filter = @receiver.create_dmail_filter(:words => @sender.name)
end
should "filter on the sender" do
create_dmail("okay", "okay")
assert_equal(true, @receiver.dmails.last.is_read?)
end
end
context "a dmail filter containing multiple words" do
should "filter dmails containing any of the words" do
@receiver.create_dmail_filter(words: "foo bar spam")
create_dmail("this is a test (not *SPAM*)", "hello world")
assert_equal(true, @receiver.dmails.last.is_read?)
end
end
end

View File

@@ -43,49 +43,6 @@ class DmailTest < ActiveSupport::TestCase
end end
end end
context "filter" do
setup do
@recipient = FactoryBot.create(:user)
@recipient.create_dmail_filter(:words => "banned")
@dmail = FactoryBot.build(:dmail, :title => "xxx", :owner => @recipient, :body => "banned word here", :to => @recipient, :from => @user)
end
should "detect banned words" do
assert(@recipient.dmail_filter.filtered?(@dmail))
end
should "autoread if it has a banned word" do
@dmail.save
assert_equal(true, @dmail.is_read?)
end
should "not update the recipient's has_mail if filtered" do
@dmail.save
@recipient.reload
assert_equal(false, @recipient.has_mail?)
end
should "be ignored when sender is a moderator" do
CurrentUser.scoped(FactoryBot.create(:moderator_user), "127.0.0.1") do
@dmail = FactoryBot.create(:dmail, :owner => @recipient, :body => "banned word here", :to => @recipient)
end
assert_equal(false, @recipient.dmail_filter.filtered?(@dmail))
assert_equal(false, @dmail.is_read?)
assert_equal(true, @recipient.has_mail?)
end
context "that is empty" do
setup do
@recipient.dmail_filter.update(words: " ")
end
should "not filter everything" do
assert(!@recipient.dmail_filter.filtered?(@dmail))
end
end
end
context "from a banned user" do context "from a banned user" do
setup do setup do
@user.update_attribute(:is_banned, true) @user.update_attribute(:is_banned, true)