Log IP addresses with screened emails and urls

nlalonde · nlalonde · commit 25e0c3eac1e0 · 2013-08-22T19:04:35.000-04:00
diff --git a/app/assets/javascripts/admin/models/staff_action_log.js b/app/assets/javascripts/admin/models/staff_action_log.js
@@ -16,7 +16,7 @@ Discourse.StaffActionLog = Discourse.Model.extend({
   formattedDetails: function() {
     var formatted = "";
     formatted += this.format('email', 'email');
-    formatted += this.format('admin.logs.staff_actions.ip_address', 'ip_address');
+    formatted += this.format('admin.logs.ip_address', 'ip_address');
     if (!this.get('useCustomModalForDetails')) {
       formatted += this.format('admin.logs.staff_actions.new_value', 'new_value');
       formatted += this.format('admin.logs.staff_actions.previous_value', 'previous_value');
diff --git a/app/assets/javascripts/admin/templates/logs/screened_emails.js.handlebars b/app/assets/javascripts/admin/templates/logs/screened_emails.js.handlebars
@@ -12,6 +12,7 @@
         <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="col heading ip_address">{{i18n admin.logs.ip_address}}</div>
         <div class="clearfix"></div>
       </div>
 
diff --git a/app/assets/javascripts/admin/templates/logs/screened_emails_list_item.js.handlebars b/app/assets/javascripts/admin/templates/logs/screened_emails_list_item.js.handlebars
@@ -3,4 +3,5 @@
 <div class="col match_count">{{match_count}}</div>
 <div class="col last_match_at">{{unboundAgeWithTooltip last_match_at}}</div>
 <div class="col created_at">{{unboundAgeWithTooltip created_at}}</div>
+<div class="col ip_address">{{ip_address}}</div>
 <div class="clearfix"></div>
diff --git a/app/assets/javascripts/admin/templates/logs/screened_urls.js.handlebars b/app/assets/javascripts/admin/templates/logs/screened_urls.js.handlebars
@@ -12,6 +12,7 @@
         <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="col heading ip_address">{{i18n admin.logs.ip_address}}</div>
         <div class="clearfix"></div>
       </div>
 
diff --git a/app/assets/javascripts/admin/templates/logs/screened_urls_list_item.js.handlebars b/app/assets/javascripts/admin/templates/logs/screened_urls_list_item.js.handlebars
@@ -3,4 +3,5 @@
 <div class="col match_count">{{match_count}}</div>
 <div class="col last_match_at">{{unboundAgeWithTooltip last_match_at}}</div>
 <div class="col created_at">{{unboundAgeWithTooltip created_at}}</div>
+<div class="col ip_address">{{ip_address}}</div>
 <div class="clearfix"></div>
diff --git a/app/assets/stylesheets/admin/admin_base.scss b/app/assets/stylesheets/admin/admin_base.scss
@@ -703,9 +703,9 @@ table {
 .screened-emails, .screened-urls {
   width: 900px;
   .email, .url {
-    width: 400px;
+    width: 300px;
   }
-  .action, .match_count, .last_match_at, .created_at {
+  .action, .match_count, .last_match_at, .created_at, .ip_address {
     width: 110px;
     text-align: center;
   }
diff --git a/app/models/screened_email.rb b/app/models/screened_email.rb
@@ -13,7 +13,7 @@ class ScreenedEmail < ActiveRecord::Base
   validates :email, presence: true, uniqueness: true
 
   def self.block(email, opts={})
-    find_by_email(email) || create(opts.slice(:action_type).merge({email: email}))
+    find_by_email(email) || create(opts.slice(:action_type, :ip_address).merge({email: email}))
   end
 
   def self.should_block?(email)
diff --git a/app/models/screened_url.rb b/app/models/screened_url.rb
@@ -21,6 +21,6 @@ def strip_http
   end
 
   def self.watch(url, domain, opts={})
-    find_by_url(url) || create(opts.slice(:action_type).merge(url: url, domain: domain))
+    find_by_url(url) || create(opts.slice(:action_type, :ip_address).merge(url: url, domain: domain))
   end
 end
diff --git a/app/serializers/screened_email_serializer.rb b/app/serializers/screened_email_serializer.rb
@@ -3,7 +3,8 @@ class ScreenedEmailSerializer < ApplicationSerializer
              :action,
              :match_count,
              :last_match_at,
-             :created_at
+             :created_at,
+             :ip_address
 
   def action
     ScreenedEmail.actions.key(object.action_type).to_s
diff --git a/app/serializers/screened_url_serializer.rb b/app/serializers/screened_url_serializer.rb
@@ -4,7 +4,8 @@ class ScreenedUrlSerializer < ApplicationSerializer
              :action,
              :match_count,
              :last_match_at,
-             :created_at
+             :created_at,
+             :ip_address
 
   def action
     ScreenedUrl.actions.key(object.action_type).to_s
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
@@ -1183,6 +1183,7 @@ en:
         created_at: "Created"
         last_match_at: "Last Matched"
         match_count: "Matches"
+        ip_address: "IP"
         screened_actions:
           block: "block"
           do_nothing: "do nothing"
@@ -1196,7 +1197,6 @@ en:
           when: "When"
           context: "Context"
           details: "Details"
-          ip_address: "IP"
           previous_value: "Previous"
           new_value: "New"
           diff: "Diff"
diff --git a/db/migrate/20130822213513_add_ip_address_to_screening_tables.rb b/db/migrate/20130822213513_add_ip_address_to_screening_tables.rb
@@ -0,0 +1,6 @@
+class AddIpAddressToScreeningTables < ActiveRecord::Migration
+  def change
+    add_column :screened_emails, :ip_address, :inet
+    add_column :screened_urls,   :ip_address, :inet
+  end
+end
diff --git a/lib/user_destroyer.rb b/lib/user_destroyer.rb
@@ -20,7 +20,7 @@ def destroy(user, opts={})
           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!)
+                ScreenedUrl.watch(link.url, link.domain, ip_address: user.ip_address).try(:record_match!)
               end
             end
           end
@@ -31,7 +31,7 @@ def destroy(user, opts={})
       user.destroy.tap do |u|
         if u
           if opts[:block_email]
-            b = ScreenedEmail.block(u.email)
+            b = ScreenedEmail.block(u.email, ip_address: u.ip_address)
             b.record_match! if b
           end
           Post.with_deleted.where(user_id: user.id).update_all("nuked_user = true")
diff --git a/spec/components/user_destroyer_spec.rb b/spec/components/user_destroyer_spec.rb
@@ -80,7 +80,7 @@
 
       it "adds email to block list if block_email is true" do
         b = Fabricate.build(:screened_email, email: @user.email)
-        ScreenedEmail.expects(:block).with(@user.email).returns(b)
+        ScreenedEmail.expects(:block).with(@user.email, has_key(:ip_address)).returns(b)
         b.expects(:record_match!).once.returns(true)
         UserDestroyer.new(@admin).destroy(@user, destroy_opts.merge({block_email: true}))
       end
@@ -178,7 +178,7 @@
         end
 
         it "adds ScreenedUrl records when :block_urls is true" do
-          ScreenedUrl.expects(:watch).at_least_once
+          ScreenedUrl.expects(:watch).with(anything, anything, has_key(:ip_address)).at_least_once
           UserDestroyer.new(@admin).destroy(@user, {delete_posts: true, block_urls: true})
         end
       end

Original file line number	Diff line number	Diff line change
`@@ -703,9 +703,9 @@ table {`
`703`	`703`	`.screened-emails, .screened-urls {`
`704`	`704`	`width: 900px;`
`705`	`705`	`.email, .url {`
`706`		`- width: 400px;`
	`706`	`+ width: 300px;`
`707`	`707`	`}`
`708`		`- .action, .match_count, .last_match_at, .created_at {`
	`708`	`+ .action, .match_count, .last_match_at, .created_at, .ip_address {`
`709`	`709`	`width: 110px;`
`710`	`710`	`text-align: center;`
`711`	`711`	`}`