aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/controllers/admin_request_controller.rb11
-rw-r--r--app/controllers/request_controller.rb4
-rw-r--r--app/models/info_request.rb67
-rw-r--r--app/views/admin_request/edit.rhtml24
-rw-r--r--app/views/request/_followup.rhtml2
-rw-r--r--db/migrate/078_expand_stop_new_responses.rb18
-rw-r--r--db/schema.rb25
-rw-r--r--spec/controllers/admin_request_controller_spec.rb2
-rw-r--r--spec/fixtures/incoming-request-plain.email2
-rw-r--r--spec/fixtures/incoming-request-two-same-name.email2
-rw-r--r--spec/models/request_mailer_spec.rb81
-rw-r--r--spec/spec_helper.rb3
12 files changed, 202 insertions, 39 deletions
diff --git a/app/controllers/admin_request_controller.rb b/app/controllers/admin_request_controller.rb
index dfd44f32b..ca00da9ab 100644
--- a/app/controllers/admin_request_controller.rb
+++ b/app/controllers/admin_request_controller.rb
@@ -4,7 +4,7 @@
# Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved.
# Email: francis@mysociety.org; WWW: http://www.mysociety.org/
#
-# $Id: admin_request_controller.rb,v 1.34 2009-04-08 16:13:11 louise Exp $
+# $Id: admin_request_controller.rb,v 1.35 2009-06-15 14:42:11 francis Exp $
class AdminRequestController < AdminController
def index
@@ -45,7 +45,8 @@ class AdminRequestController < AdminController
old_prominence = @info_request.prominence
old_described_state = @info_request.described_state
old_awaiting_description = @info_request.awaiting_description
- old_stop_new_responses = @info_request.stop_new_responses
+ old_allow_new_responses_from = @info_request.allow_new_responses_from
+ old_handle_rejected_responses = @info_request.handle_rejected_responses
@info_request.title = params[:info_request][:title]
@info_request.prominence = params[:info_request][:prominence]
@@ -53,7 +54,8 @@ class AdminRequestController < AdminController
@info_request.set_described_state(params[:info_request][:described_state])
end
@info_request.awaiting_description = params[:info_request][:awaiting_description] == "true" ? true : false
- @info_request.stop_new_responses = params[:info_request][:stop_new_responses] == "true" ? true : false
+ @info_request.allow_new_responses_from = params[:info_request][:allow_new_responses_from]
+ @info_request.handle_rejected_responses = params[:info_request][:handle_rejected_responses]
if @info_request.valid?
@info_request.save!
@@ -63,7 +65,8 @@ class AdminRequestController < AdminController
:old_prominence => old_prominence, :prominence => @info_request.prominence,
:old_described_state => old_described_state, :described_state => @info_request.described_state,
:old_awaiting_description => old_awaiting_description, :awaiting_description => @info_request.awaiting_description,
- :old_stop_new_responses => old_stop_new_responses, :stop_new_responses => @info_request.stop_new_responses
+ :old_allow_new_responses_from => old_allow_new_responses_from, :allow_new_responses_from => @info_request.allow_new_responses_from,
+ :old_handle_rejected_responses => old_handle_rejected_responses, :handle_rejected_responses => @info_request.handle_rejected_responses
})
flash[:notice] = 'Request successfully updated.'
redirect_to request_admin_url(@info_request)
diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index 8316eebf3..97181ddf7 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -4,7 +4,7 @@
# Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved.
# Email: francis@mysociety.org; WWW: http://www.mysociety.org/
#
-# $Id: request_controller.rb,v 1.160 2009-06-12 13:53:45 francis Exp $
+# $Id: request_controller.rb,v 1.161 2009-06-15 14:42:11 francis Exp $
class RequestController < ApplicationController
@@ -451,7 +451,7 @@ class RequestController < ApplicationController
end
if !params[:submitted_followup].nil? && !params[:reedit]
- if @info_request.stop_new_responses
+ if @info_request.allow_new_responses_from == 'nobody'
flash[:error] = 'Your follow up has not been sent because this request has been stopped to prevent spam. Please <a href="/help/contact">contact us</a> if you really want to send a follow up message.'
else
# See if values were valid or not
diff --git a/app/models/info_request.rb b/app/models/info_request.rb
index c247f9286..3f6b29a3b 100644
--- a/app/models/info_request.rb
+++ b/app/models/info_request.rb
@@ -23,7 +23,7 @@
# Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved.
# Email: francis@mysociety.org; WWW: http://www.mysociety.org/
#
-# $Id: info_request.rb,v 1.189 2009-04-23 14:20:47 francis Exp $
+# $Id: info_request.rb,v 1.190 2009-06-15 14:42:11 francis Exp $
require 'digest/sha1'
require File.join(File.dirname(__FILE__),'../../vendor/plugins/acts_as_xapian/lib/acts_as_xapian')
@@ -73,6 +73,19 @@ class InfoRequest < ActiveRecord::Base
'foi', # Freedom of Information Act
'eir', # Environmental Information Regulations
]
+
+ # who can send new responses
+ validates_inclusion_of :allow_new_responses_from, :in => [
+ 'anybody', # anyone who knows the request email address
+ 'authority_only', # only people from authority domains
+ 'nobody'
+ ]
+ # what to do with rejected new responses
+ validates_inclusion_of :handle_rejected_responses, :in => [
+ 'bounce', # return them to sender
+ 'holding_pen', # put them in the holding pen
+ 'blackhole' # just dump them
+ ]
OLD_AGE_IN_DAYS = 14.days
@@ -265,10 +278,47 @@ public
# A new incoming email to this request
def receive(email, raw_email_data, override_stop_new_responses = false)
- # See if new responses are prevented for spam reasons
- if self.stop_new_responses && !override_stop_new_responses
- RequestMailer.deliver_stopped_responses(self, email)
- return
+ if !override_stop_new_responses
+ allow = nil
+
+ # See if new responses are prevented for spam reasons
+ if self.allow_new_responses_from == 'nobody'
+ allow = false
+ elsif self.allow_new_responses_from == 'anybody'
+ allow = true
+ elsif self.allow_new_responses_from == 'authority_only'
+ if email.from_addrs.nil? || email.from_addrs.size == 0
+ allow = false
+ else
+ sender_email = email.from_addrs[0].to_s
+ sender_domain = PublicBody.extract_domain_from_email(sender_email)
+ allow = false
+ # Allow any domain that has already sent reply
+ for row in self.who_can_followup_to
+ request_domain = PublicBody.extract_domain_from_email(row[1])
+ if request_domain == sender_domain
+ allow = true
+ end
+ end
+ end
+ else
+ raise "Unknown allow_new_responses_from '" + self.allow_new_responses_from + "'"
+ end
+
+ if !allow
+ if self.handle_rejected_responses == 'bounce'
+ RequestMailer.deliver_stopped_responses(self, email)
+ elsif self.handle_rejected_responses == 'holding_pen'
+ InfoRequest.holding_pen_request.receive(email, raw_email_data)
+ elsif self.handle_rejected_responses == 'blackhole'
+ # do nothing - just lose the message (Note: a copy will be
+ # in the backup mailbox if the server is configured to send
+ # new incoming messages there as well as this script)
+ else
+ raise "Unknown handle_rejected_responses '" + self.handle_rejected_responses + "'"
+ end
+ return
+ end
end
# Otherwise log the message
@@ -763,6 +813,13 @@ public
!user.nil? && (user.id == user_id || user.owns_every_request?)
end
+ # XXX to be called from a cron job later
+ def self.stop_new_responses_on_old_requests
+ # 6 months since last change to request, only allow new incoming messages from authority domains
+ InfoRequest.update_all "allow_new_responses_from = 'authority_only' where updated_at < (now() - interval '6 months') and allow_new_responses_from = 'anybody'"
+ # 1 year since last change requests, don't allow any new incoming messages
+ PostRedirect.update_all "allow_new_responses_from = 'nobody' where updated_at < (now() - interval '1 year') and allow_new_responses_from in ('anybody', 'authority_only')"
+ end
end
diff --git a/app/views/admin_request/edit.rhtml b/app/views/admin_request/edit.rhtml
index 7374042f1..8756ee0fb 100644
--- a/app/views/admin_request/edit.rhtml
+++ b/app/views/admin_request/edit.rhtml
@@ -4,17 +4,24 @@
<% form_tag '../update/' + @info_request.id.to_s do %>
- <p><label for="info_request_title">Title</label> (warning: editing this will break URLs right now)<br/>
+ <p><label for="info_request_title"><strong>Title</strong></label> (warning: editing this will break URLs right now)<br/>
<%= text_field 'info_request', 'title', :size => 50 %></p>
- <p><label for="info_request_prominence">Prominence</label> (whether is shown in lists of requests / search or not)<br/>
+ <p><label for="info_request_prominence"><strong>Prominence</strong></label>
<%= select( 'info_request', "prominence", { "normal" => "normal", "backpage" => "backpage"}) %>
+ (whether request is shown in lists / search or not)
+ </p>
- <p><label for="info_request_stop_new_responses">Stop new responses</label> (by email; use this on requests getting spam, but also work out how the email leaked and plug it)<br/>
- <%= select('info_request', "stop_new_responses", [["Yes - stop new responses",true],["No - allow new responses",false]]) %>
+ <p>
+ <label for="info_request_allow_new_responses_from"><strong>Allow new responses</strong> from</label>
+ <%= select( 'info_request', "allow_new_responses_from", [ "anybody", "authority_only", "nobody" ] ) %>;
+ <label for="info_request_handle_rejected_responses"><strong>Handle rejected responses</strong> with</label>
+ <%= select( 'info_request', "handle_rejected_responses", [ "bounce", "holding_pen", "blackhole" ] ) %>
+ <br>
+ ('authority_only' means email From: domain of authority request email or any domain that has previously sent a response; 'nobody' also stops requester making followups; take care when using 'blackhole' which just drops mail)
</p>
- <p><label for="info_request_described_state">Described state</label> (don't forget to change 'awaiting description' below too if necessary)<br/>
+ <p><label for="info_request_described_state"><strong>Described state</strong></label>
<%= select( 'info_request', "described_state",
[
'waiting_response',
@@ -28,11 +35,10 @@
'error_message',
'requires_admin',
'user_withdrawn'
- ]) %>
- </p>
-
- <p><label for="info_request_awaiting_description">Awaiting description</label><br/>
+ ]) %>;
+ <label for="info_request_awaiting_description"><strong>Awaiting description</strong></label>
<%= select('info_request', "awaiting_description", [["Yes - needs state updating",true],["No - state is up to date",false]]) %>
+ <br/>(don't forget to change 'awaiting description' when you set described state)<br/>
</p>
<p><%= submit_tag 'Save changes', :accesskey => 's' %>
diff --git a/app/views/request/_followup.rhtml b/app/views/request/_followup.rhtml
index a738e4e80..5c07b1b67 100644
--- a/app/views/request/_followup.rhtml
+++ b/app/views/request/_followup.rhtml
@@ -14,7 +14,7 @@
</h2>
<% end %>
- <% if @info_request.stop_new_responses %>
+ <% if @info_request.allow_new_responses_from == 'nobody' %>
<p>Follow ups and new responses to this request have been stopped to prevent spam. Please
<a href="/help/contact">contact us</a> if you are <%= user_link(@info_request.user) %>
and need to send a follow up.</p>
diff --git a/db/migrate/078_expand_stop_new_responses.rb b/db/migrate/078_expand_stop_new_responses.rb
new file mode 100644
index 000000000..ae0d4db91
--- /dev/null
+++ b/db/migrate/078_expand_stop_new_responses.rb
@@ -0,0 +1,18 @@
+class ExpandStopNewResponses < ActiveRecord::Migration
+ def self.up
+ add_column :info_requests, :allow_new_responses_from, :string
+ InfoRequest.update_all "allow_new_responses_from = 'anybody'"
+ InfoRequest.update_all "allow_new_responses_from = 'nobody' where stop_new_responses"
+ change_column :info_requests, :allow_new_responses_from, :string, :null => false, :default => 'anybody'
+ remove_column :info_requests, :stop_new_responses
+
+ add_column :info_requests, :handle_rejected_responses, :string
+ InfoRequest.update_all "handle_rejected_responses = 'bounce'"
+ change_column :info_requests, :handle_rejected_responses, :string, :null => false, :default => 'bounce'
+ end
+
+ def self.down
+ raise "No code for reversing this"
+ end
+end
+
diff --git a/db/schema.rb b/db/schema.rb
index 2da739ae1..7055727eb 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -9,7 +9,7 @@
#
# It's strongly recommended to check this file into your version control system.
-ActiveRecord::Schema.define(:version => 77) do
+ActiveRecord::Schema.define(:version => 78) do
create_table "acts_as_xapian_jobs", :force => true do |t|
t.string "model", :null => false
@@ -94,17 +94,18 @@ ActiveRecord::Schema.define(:version => 77) do
add_index "info_request_events", ["info_request_id"], :name => "index_info_request_events_on_info_request_id"
create_table "info_requests", :force => true do |t|
- t.text "title", :null => false
- t.integer "user_id", :null => false
- t.integer "public_body_id", :null => false
- t.datetime "created_at", :null => false
- t.datetime "updated_at", :null => false
- t.string "described_state", :null => false
- t.boolean "awaiting_description", :default => false, :null => false
- t.string "prominence", :default => "normal", :null => false
- t.text "url_title", :null => false
- t.boolean "stop_new_responses", :default => false, :null => false
- t.string "law_used", :default => "foi", :null => false
+ t.text "title", :null => false
+ t.integer "user_id", :null => false
+ t.integer "public_body_id", :null => false
+ t.datetime "created_at", :null => false
+ t.datetime "updated_at", :null => false
+ t.string "described_state", :null => false
+ t.boolean "awaiting_description", :default => false, :null => false
+ t.string "prominence", :default => "normal", :null => false
+ t.text "url_title", :null => false
+ t.string "law_used", :default => "foi", :null => false
+ t.string "allow_new_responses_from", :default => "anybody", :null => false
+ t.string "handle_rejected_responses", :default => "bounce", :null => false
end
add_index "info_requests", ["created_at"], :name => "index_info_requests_on_created_at"
diff --git a/spec/controllers/admin_request_controller_spec.rb b/spec/controllers/admin_request_controller_spec.rb
index ff0f74aed..cb30541b6 100644
--- a/spec/controllers/admin_request_controller_spec.rb
+++ b/spec/controllers/admin_request_controller_spec.rb
@@ -18,7 +18,7 @@ describe AdminRequestController, "when administering requests" do
it "saves edits to a request" do
info_requests(:fancy_dog_request).title.should == "Why do you have & such a fancy dog?"
- post :update, { :id => info_requests(:fancy_dog_request), :info_request => { :title => "Renamed", :prominence => "normal", :described_state => "waiting_response", :awaiting_description => false } }
+ post :update, { :id => info_requests(:fancy_dog_request), :info_request => { :title => "Renamed", :prominence => "normal", :described_state => "waiting_response", :awaiting_description => false, :allow_new_responses_from => 'anybody', :handle_rejected_responses => 'bounce' } }
response.flash[:notice].should include('successful')
ir = InfoRequest.find(info_requests(:fancy_dog_request).id)
ir.title.should == "Renamed"
diff --git a/spec/fixtures/incoming-request-plain.email b/spec/fixtures/incoming-request-plain.email
index 4ee258d2a..8aca7707b 100644
--- a/spec/fixtures/incoming-request-plain.email
+++ b/spec/fixtures/incoming-request-plain.email
@@ -1,4 +1,4 @@
-From: geraldinequango@localhost
+From: EMAIL_FROM
To: FOI Person <EMAIL_TO>
Bcc:
Subject: Re: Freedom of Information Request - Why aren't you leaving the house?
diff --git a/spec/fixtures/incoming-request-two-same-name.email b/spec/fixtures/incoming-request-two-same-name.email
index f0ab9f29c..f1024d607 100644
--- a/spec/fixtures/incoming-request-two-same-name.email
+++ b/spec/fixtures/incoming-request-two-same-name.email
@@ -1,4 +1,4 @@
-From: Francis Irving <francis@flourish.org>
+From: EMAIL_FROM
To: FOI Person <EMAIL_TO>
Subject: Same attachment twice
Content-Type: multipart/mixed; boundary="Q68bSM7Ycu6FN28Q"
diff --git a/spec/models/request_mailer_spec.rb b/spec/models/request_mailer_spec.rb
index 6f765d982..2cc132b50 100644
--- a/spec/models/request_mailer_spec.rb
+++ b/spec/models/request_mailer_spec.rb
@@ -14,6 +14,7 @@ describe RequestMailer, " when receiving incoming mail" do
deliveries = ActionMailer::Base.deliveries
deliveries.size.should == 1
mail = deliveries[0]
+ mail.to.should == [ 'bob@localhost' ] # to the user who sent fancy_dog_request
deliveries.clear
end
@@ -32,10 +33,11 @@ describe RequestMailer, " when receiving incoming mail" do
deliveries.clear
end
- it "should return incoming mail to sender when a request is stopped for spam" do
+ it "should return incoming mail to sender when a request is stopped fully for spam" do
# mark request as anti-spam
ir = info_requests(:fancy_dog_request)
- ir.stop_new_responses = true
+ ir.allow_new_responses_from = 'nobody'
+ ir.handle_rejected_responses = 'bounce'
ir.save!
# test what happens if something arrives
@@ -51,6 +53,81 @@ describe RequestMailer, " when receiving incoming mail" do
deliveries.clear
end
+ it "should return incoming mail to sender if not authority when a request is stopped for non-authority spam" do
+ # mark request as anti-spam
+ ir = info_requests(:fancy_dog_request)
+ ir.allow_new_responses_from = 'authority_only'
+ ir.handle_rejected_responses = 'bounce'
+ ir.save!
+
+ # Test what happens if something arrives from authority domain (@localhost)
+ ir.incoming_messages.size.should == 1 # in the fixture
+ receive_incoming_mail('incoming-request-plain.email', ir.incoming_email)
+ ir.incoming_messages.size.should == 2 # one more arrives
+
+ # ... should get "responses arrived" message for original requester
+ deliveries = ActionMailer::Base.deliveries
+ deliveries.size.should == 1
+ mail = deliveries[0]
+ mail.to.should == [ 'bob@localhost' ] # to the user who sent fancy_dog_request
+ deliveries.clear
+
+ # Test what happens if something arrives from another domain
+ ir.incoming_messages.size.should == 2 # in fixture and above
+ receive_incoming_mail('incoming-request-plain.email', ir.incoming_email, "dummy-address@dummy.localhost")
+ ir.incoming_messages.size.should == 2 # nothing should arrive
+
+ # ... should be a bounce message back to sender
+ deliveries = ActionMailer::Base.deliveries
+ deliveries.size.should == 1
+ mail = deliveries[0]
+ mail.to.should == [ 'dummy-address@dummy.localhost' ]
+ deliveries.clear
+ end
+
+ it "should send all new responses to holding pen if a request is marked to do so" do
+ # mark request as anti-spam
+ ir = info_requests(:fancy_dog_request)
+ ir.allow_new_responses_from = 'nobody'
+ ir.handle_rejected_responses = 'holding_pen'
+ ir.save!
+
+ # test what happens if something arrives
+ ir = info_requests(:fancy_dog_request)
+ ir.incoming_messages.size.should == 1
+ InfoRequest.holding_pen_request.incoming_messages.size.should == 0
+ receive_incoming_mail('incoming-request-plain.email', ir.incoming_email)
+ ir.incoming_messages.size.should == 1
+ InfoRequest.holding_pen_request.incoming_messages.size.should == 1 # arrives in holding pen
+
+ # should be a message to admin regarding holding pen
+ deliveries = ActionMailer::Base.deliveries
+ deliveries.size.should == 1
+ mail = deliveries[0]
+ mail.to.should == [ MySociety::Config.get("CONTACT_EMAIL", 'contact@localhost') ]
+ deliveries.clear
+ end
+
+ it "should dump messages to a request if marked to do so" do
+ # mark request as anti-spam
+ ir = info_requests(:fancy_dog_request)
+ ir.allow_new_responses_from = 'nobody'
+ ir.handle_rejected_responses = 'blackhole'
+ ir.save!
+
+ # test what happens if something arrives - should be nothing
+ ir = info_requests(:fancy_dog_request)
+ ir.incoming_messages.size.should == 1
+ InfoRequest.holding_pen_request.incoming_messages.size.should == 0
+ receive_incoming_mail('incoming-request-plain.email', ir.incoming_email)
+ ir.incoming_messages.size.should == 1
+ InfoRequest.holding_pen_request.incoming_messages.size.should == 0
+
+ # should be no messages to anyone
+ deliveries = ActionMailer::Base.deliveries
+ deliveries.size.should == 0
+ end
+
it "should not mutilate long URLs when trying to word wrap them" do
long_url = 'http://www.this.is.quite.a.long.url.flourish.org/there.is.no.way.it.is.short.whatsoever'
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index c56e15f4c..6852909a6 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -23,10 +23,11 @@ Spec::Runner.configure do |config|
end
# XXX No idea what namespace/class/module to put this in
-def receive_incoming_mail(email_name, email_to)
+def receive_incoming_mail(email_name, email_to, email_from = 'geraldinequango@localhost')
email_name = File.join(Spec::Runner.configuration.fixture_path, email_name)
content = File.read(email_name)
content.gsub!('EMAIL_TO', email_to)
+ content.gsub!('EMAIL_FROM', email_from)
RequestMailer.receive(content)
end
generated by cgit v1.2.3 (git 2.25.1) at 2025-07-28 08:22:20 +0000