diff options
| -rw-r--r-- | app/controllers/admin_censor_rule_controller.rb | 8 | ||||
| -rw-r--r-- | app/controllers/admin_controller.rb | 32 | ||||
| -rw-r--r-- | app/controllers/admin_request_controller.rb | 11 | ||||
| -rw-r--r-- | app/controllers/request_controller.rb | 26 | ||||
| -rw-r--r-- | app/models/info_request.rb | 10 | ||||
| -rw-r--r-- | app/models/user.rb | 8 | ||||
| -rw-r--r-- | app/views/request/hidden.rhtml | 2 | ||||
| -rw-r--r-- | public/.cvsignore | 1 | ||||
| -rw-r--r-- | spec/controllers/request_controller_spec.rb | 25 |
9 files changed, 72 insertions, 51 deletions
diff --git a/app/controllers/admin_censor_rule_controller.rb b/app/controllers/admin_censor_rule_controller.rb index b51c1e01e..28130b6ee 100644 --- a/app/controllers/admin_censor_rule_controller.rb +++ b/app/controllers/admin_censor_rule_controller.rb @@ -4,7 +4,7 @@ # Copyright (c) 2008 UK Citizens Online Democracy. All rights reserved. # Email: francis@mysociety.org; WWW: http://www.mysociety.org/ # -# $Id: admin_censor_rule_controller.rb,v 1.6 2009-06-23 13:52:25 francis Exp $ +# $Id: admin_censor_rule_controller.rb,v 1.7 2009-06-30 14:28:25 francis Exp $ class AdminCensorRuleController < AdminController def new @@ -14,7 +14,6 @@ class AdminCensorRuleController < AdminController def create params[:censor_rule][:last_edit_editor] = admin_http_auth_user() @censor_rule = CensorRule.new(params[:censor_rule]) - expire_for_request(@censor_rule.info_request) if @censor_rule.save expire_for_request(@censor_rule.info_request) flash[:notice] = 'CensorRule was successfully created.' @@ -31,8 +30,6 @@ class AdminCensorRuleController < AdminController def update params[:censor_rule][:last_edit_editor] = admin_http_auth_user() @censor_rule = CensorRule.find(params[:id]) - # expire before and after change, in case filename changes - expire_for_request(@censor_rule.info_request) if @censor_rule.update_attributes(params[:censor_rule]) expire_for_request(@censor_rule.info_request) flash[:notice] = 'CensorRule was successfully updated.' @@ -46,13 +43,10 @@ class AdminCensorRuleController < AdminController censor_rule = CensorRule.find(params[:censor_rule_id]) info_request = censor_rule.info_request - # expire before and after change, in case filename changes - expire_for_request(info_request) censor_rule.destroy expire_for_request(info_request) flash[:notice] = "CensorRule was successfully destroyed." - redirect_to admin_url('request/show/' + info_request.id.to_s) end diff --git a/app/controllers/admin_controller.rb b/app/controllers/admin_controller.rb index 5810b4b04..9ca891bf2 100644 --- a/app/controllers/admin_controller.rb +++ b/app/controllers/admin_controller.rb @@ -4,7 +4,7 @@ # Copyright (c) 2009 UK Citizens Online Democracy. All rights reserved. # Email: francis@mysociety.org; WWW: http://www.mysociety.org/ # -# $Id: admin_controller.rb,v 1.25 2009-06-23 13:52:25 francis Exp $ +# $Id: admin_controller.rb,v 1.26 2009-06-30 14:28:25 francis Exp $ class AdminController < ApplicationController @@ -18,22 +18,20 @@ class AdminController < ApplicationController # Expire cached attachment files for a request def expire_for_request(info_request) - # So is using latest censor rules - info_request.reload - - # clear out cached entries - for incoming_message in info_request.incoming_messages - for attachment in incoming_message.get_attachments_for_display - expire_page :controller => 'request', :action => "get_attachment", :id => info_request.id, - :incoming_message_id => incoming_message.id, - :part => attachment.url_part_number, :file_name => attachment.display_filename - expire_page :controller => 'request', :action => "get_attachment_as_html", :id => info_request.id, - :incoming_message_id => incoming_message.id, - :part => attachment.url_part_number, :file_name => attachment.display_filename - end - end + # Clear out cached entries - use low level expire_fragment, even though + # we are clearing results from caches_action, for several reasons: + # * We can't use expire_action here, as doesn't seem to be + # compatible with the :only_path we used in the caches_action + # call. + # * expire_fragment lets us use a regular expression which is + # simpler than having to get all the parameters right for the + # path, and calling for HTML version vs. raw attachment version. + # * Regular expression means we cope properly with filenames + # changed by censor rules, which change the URL. + # * It's also possible to load a file with any name by changing + # the URL, the regular expression makes sure the cache is + # cleared even if someone did that. + expire_fragment /views\/request\/#{info_request.id}.*/ end - - end diff --git a/app/controllers/admin_request_controller.rb b/app/controllers/admin_request_controller.rb index fe7b6e8a6..df22f1341 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.36 2009-06-23 13:52:25 francis Exp $ +# $Id: admin_request_controller.rb,v 1.37 2009-06-30 14:28:25 francis Exp $ class AdminRequestController < AdminController def index @@ -48,12 +48,6 @@ class AdminRequestController < AdminController old_allow_new_responses_from = @info_request.allow_new_responses_from old_handle_rejected_responses = @info_request.handle_rejected_responses - expire = false - if @info_request.prominence != params[:info_request][:prominence] - # in case it has become hidden, clear cache after saving - expire = true - end - @info_request.title = params[:info_request][:title] @info_request.prominence = params[:info_request][:prominence] if @info_request.described_state != params[:info_request][:described_state] @@ -65,9 +59,6 @@ class AdminRequestController < AdminController if @info_request.valid? @info_request.save! - if expire - expire_for_request(@info_request) - end @info_request.log_event("edit", { :editor => admin_http_auth_user(), :old_title => old_title, :title => @info_request.title, diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb index 7579ee63e..435c73724 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.163 2009-06-23 13:52:25 francis Exp $ +# $Id: request_controller.rb,v 1.164 2009-06-30 14:28:25 francis Exp $ class RequestController < ApplicationController @@ -497,7 +497,20 @@ class RequestController < ApplicationController end # Download an attachment - caches_page :get_attachment + + before_filter :authenticate_attachment, :only => [ :get_attachment, :get_attachment_as_html ] + def authenticate_attachment + # Test for hidden + incoming_message = IncomingMessage.find(params[:incoming_message_id]) + if !incoming_message.info_request.user_can_view?(authenticated_user) + render :template => 'request/hidden' + end + end + + # use :cache_path here so domain doesn't appear in filename, as /admin + # interface runs on separate domain (so can use HTTPS without its own + # certificate) in mySociety server configuration. + caches_action :get_attachment, :cache_path => { :only_path => true } def get_attachment if !get_attachment_internal return @@ -513,7 +526,7 @@ class RequestController < ApplicationController render :text => @attachment.body end - caches_page :get_attachment_as_html + caches_action :get_attachment_as_html, :cache_path => { :only_path => true } def get_attachment_as_html if !get_attachment_internal return @@ -545,11 +558,8 @@ class RequestController < ApplicationController @part_number = params[:part].to_i @filename = params[:file_name] - # Test for hidden - if !@info_request.user_can_view?(authenticated_user) - render :template => 'request/hidden' - return false - end + # check permissions + raise "internal error, pre-auth filter should have caught this" if !@info_request.user_can_view?(authenticated_user) @attachment = IncomingMessage.get_attachment_by_url_part_number(@incoming_message.get_attachments_for_display, @part_number) diff --git a/app/models/info_request.rb b/app/models/info_request.rb index 9d14e70d2..aa9b49c83 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -24,7 +24,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.196 2009-06-27 02:25:31 francis Exp $ +# $Id: info_request.rb,v 1.197 2009-06-30 14:28:26 francis Exp $ require 'digest/sha1' require File.join(File.dirname(__FILE__),'../../vendor/plugins/acts_as_xapian/lib/acts_as_xapian') @@ -819,10 +819,10 @@ public end def user_can_view?(user) - return self.prominence != 'hidden' - # || self.is_owning_user?(user) # XXX this doesn't work, as have to - # mess with caching of HTML versions - need to change from using - # caches_pages in the request controller first. + if self.prominence == 'hidden' + return User.view_hidden_requests?(user) + end + return true end # This is called from cron regularly. diff --git a/app/models/user.rb b/app/models/user.rb index 13b2c58d0..03a076c24 100644 --- a/app/models/user.rb +++ b/app/models/user.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: user.rb,v 1.95 2009-06-26 14:28:38 francis Exp $ +# $Id: user.rb,v 1.96 2009-06-30 14:28:26 francis Exp $ require 'digest/sha1' @@ -219,7 +219,11 @@ class User < ActiveRecord::Base def self.owns_every_request?(user) !user.nil? && user.owns_every_request? end - + + def self.view_hidden_requests?(user) + !user.nil? && user.admin_level == 'super' + end + # Does the user get "(admin)" links on each page on the main site? def admin_page_links? self.admin_level == 'super' diff --git a/app/views/request/hidden.rhtml b/app/views/request/hidden.rhtml index e32e7fa12..781bc9184 100644 --- a/app/views/request/hidden.rhtml +++ b/app/views/request/hidden.rhtml @@ -7,7 +7,7 @@ </p> <p>The request you have tried to view has been removed. There are -lots of reasons why we might have done this, sorry we can't +various reasons why we might have done this, sorry we can't be more specific here. Please <a href="/help/contact">contact us</a> if you have any questions. </p> diff --git a/public/.cvsignore b/public/.cvsignore index f1e08352c..2a51cd65f 100644 --- a/public/.cvsignore +++ b/public/.cvsignore @@ -1,4 +1,3 @@ down.html foi-live-creation.png foi-user-use.png -request diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb index 57d092afb..13117506c 100644 --- a/spec/controllers/request_controller_spec.rb +++ b/spec/controllers/request_controller_spec.rb @@ -49,6 +49,15 @@ describe RequestController, "when showing one request" do get :show, :url_title => info_requests(:naughty_chicken_request).id response.should redirect_to(:action => 'show', :url_title => info_requests(:naughty_chicken_request).url_title) end + + it "should not show hidden requests" do + ir = info_requests(:fancy_dog_request) + ir.prominence = 'hidden' + ir.save! + + get :show, :url_title => 'why_do_you_have_such_a_fancy_dog' + response.should render_template('hidden') + end describe 'when handling an update_status parameter' do @@ -108,6 +117,22 @@ describe RequestController, "when showing one request" do response.content_type.should == "text/plain" response.should have_text(/First hello/) end + + it "should not download attachments if hidden" do + ir = info_requests(:fancy_dog_request) + ir.prominence = 'hidden' + ir.save! + receive_incoming_mail('incoming-request-two-same-name.email', ir.incoming_email) + + get :get_attachment, :incoming_message_id => ir.incoming_messages[1].id, :id => ir.id, :part => 2 + response.content_type.should == "text/html" + response.should_not have_text(/Second hello/) + response.should render_template('request/hidden') + get :get_attachment, :incoming_message_id => ir.incoming_messages[1].id, :id => ir.id, :part => 3 + response.content_type.should == "text/html" + response.should_not have_text(/First hello/) + response.should render_template('request/hidden') + end end end |