diff options
36 files changed, 629 insertions, 363 deletions
@@ -19,12 +19,14 @@ gem 'memcache-client' gem 'net-http-local', :platforms => [:ruby_18, :ruby_19] gem 'net-purge' gem 'rack' +gem 'rack-utf8_sanitizer', :platforms => [:ruby_19, :ruby_20, :ruby_21] gem 'rake', '0.9.2.2' gem 'rails-i18n' gem 'recaptcha', '~> 0.3.1', :require => 'recaptcha/rails' # :require avoids "already initialized constant" warnings gem 'rmagick', :require => 'RMagick' gem 'ruby-msg', '~> 1.5.0', :git => 'git://github.com/mysociety/ruby-msg.git' +gem 'secure_headers' gem "statistics2", "~> 0.54" gem 'syslog_protocol' gem 'thin' diff --git a/Gemfile.lock b/Gemfile.lock index b5f5671f3..ca813eb1a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -180,6 +180,10 @@ GEM rack rack-test (0.6.2) rack (>= 1.0) + rack-utf8_sanitizer (1.2.3) + bundler (~> 1.0) + rack (~> 1.0) + railties (= 3.2.21) rails (3.2.21) actionmailer (= 3.2.21) actionpack (= 3.2.21) @@ -230,6 +234,7 @@ GEM railties (~> 3.2.0) sass (>= 3.1.10) tilt (~> 1.3) + secure_headers (1.3.4) simplecov (0.7.1) multi_json (~> 1.0) simplecov-html (~> 0.7.1) @@ -319,6 +324,7 @@ DEPENDENCIES pg quiet_assets rack + rack-utf8_sanitizer rails (= 3.2.21) rails-i18n rake (= 0.9.2.2) @@ -330,6 +336,7 @@ DEPENDENCIES ruby-debug ruby-msg (~> 1.5.0)! sass-rails (~> 3.2.3) + secure_headers spork-rails statistics2 (~> 0.54) syslog_protocol diff --git a/app/assets/stylesheets/responsive/_new_request_layout.scss b/app/assets/stylesheets/responsive/_new_request_layout.scss index a2ab23060..a8b24e1b1 100644 --- a/app/assets/stylesheets/responsive/_new_request_layout.scss +++ b/app/assets/stylesheets/responsive/_new_request_layout.scss @@ -58,6 +58,10 @@ } } +#typeahead_response .close-button { + float: right; +} + /* Advice sits on right hand side */ #request_advice { diff --git a/app/controllers/admin_request_controller.rb b/app/controllers/admin_request_controller.rb index 8f023bf12..7d2ac2f35 100644 --- a/app/controllers/admin_request_controller.rb +++ b/app/controllers/admin_request_controller.rb @@ -4,9 +4,8 @@ # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved. # Email: hello@mysociety.org; WWW: http://www.mysociety.org/ -require 'ostruct' - class AdminRequestController < AdminController + def index list render :action => 'list' diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 4d3f40d40..a06fa7098 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -17,6 +17,9 @@ class ApplicationController < ActionController::Base # assign our own handler method for non-local exceptions rescue_from Exception, :with => :render_exception + # Add some security-related headers (see config/initializers/secure_headers.rb) + ensure_security_headers + # Standard headers, footers and navigation for whole site layout "default" include FastGettext::Translation # make functions like _, n_, N_ etc available) @@ -27,6 +30,8 @@ class ApplicationController < ActionController::Base before_filter :check_in_post_redirect before_filter :session_remember_me before_filter :set_vary_header + before_filter :validate_session_timestamp + after_filter :persist_session_timestamp def set_vary_header response.headers['Vary'] = 'Cookie' @@ -118,6 +123,29 @@ class ApplicationController < ActionController::Base end end + # Set a TTL for non "remember me" sessions so that the cookie + # is not replayable forever + SESSION_TTL = 3.hours + def validate_session_timestamp + if session[:user_id] && session.key?(:ttl) && session[:ttl] < SESSION_TTL.ago + clear_session_credentials + redirect_to signin_path + end + end + + def persist_session_timestamp + session[:ttl] = Time.now if session[:user_id] && !session[:remember_me] + end + + # Logout form + def clear_session_credentials + session[:user_id] = nil + session[:user_circumstance] = nil + session[:remember_me] = false + session[:using_admin] = nil + session[:admin_name] = nil + end + def render_exception(exception) # In development or the admin interface let Rails handle the exception # with its stack trace templates diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb index 9e2c291dc..d529f8dbb 100644 --- a/app/controllers/request_controller.rb +++ b/app/controllers/request_controller.rb @@ -770,13 +770,14 @@ class RequestController < ApplicationController get_attachment_internal(false) return unless @attachment - # Prevent spam to magic request address. Note that the binary - # subsitution method used depends on the content type - @incoming_message.binary_mask_stuff!(@attachment.body, @attachment.content_type) # we don't use @attachment.content_type here, as we want same mime type when cached in cache_attachments above response.content_type = AlaveteliFileTypes.filename_to_mimetype(params[:file_name]) || 'application/octet-stream' + # Prevent spam to magic request address. Note that the binary + # subsitution method used depends on the content type + @incoming_message.apply_masks!(@attachment.body, @attachment.content_type) + render :text => @attachment.body end @@ -804,10 +805,9 @@ class RequestController < ApplicationController :body_prefix => render_to_string(:partial => "request/view_html_prefix") } ) - - @incoming_message.html_mask_stuff!(html) - response.content_type = 'text/html' + @incoming_message.apply_masks!(html, response.content_type) + render :text => html end @@ -900,10 +900,18 @@ class RequestController < ApplicationController # Type ahead search def search_typeahead - # Since acts_as_xapian doesn't support the Partial match flag, we work around it - # by making the last work a wildcard, which is quite the same - query = params[:q] - @xapian_requests = perform_search_typeahead(query, InfoRequestEvent) + # Since acts_as_xapian doesn't support the Partial match flag, we work + # around it by making the last word a wildcard, which is quite the same + @query = '' + + if params.key?(:requested_from) + @query << "requested_from:#{ params[:requested_from] } " + end + + @per_page = (params.fetch(:per_page) { 25 }).to_i + + @query << params[:q] + @xapian_requests = perform_search_typeahead(@query, InfoRequestEvent, @per_page) render :partial => "request/search_ahead" end diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb index baeaab18a..9798ff8e2 100644 --- a/app/controllers/user_controller.rb +++ b/app/controllers/user_controller.rb @@ -260,16 +260,8 @@ class UserController < ApplicationController do_post_redirect post_redirect end - # Logout form - def _do_signout - session[:user_id] = nil - session[:user_circumstance] = nil - session[:remember_me] = false - session[:using_admin] = nil - session[:admin_name] = nil - end def signout - self._do_signout + clear_session_credentials if params[:r] redirect_to params[:r] else diff --git a/app/mailers/outgoing_mailer.rb b/app/mailers/outgoing_mailer.rb index 797bf9fdd..19054b4e2 100644 --- a/app/mailers/outgoing_mailer.rb +++ b/app/mailers/outgoing_mailer.rb @@ -21,7 +21,7 @@ class OutgoingMailer < ApplicationMailer mail(:from => info_request.incoming_name_and_email, :to => info_request.recipient_name_and_email, - :subject => info_request.email_subject_request) + :subject => info_request.email_subject_request(:html => false)) end # Later message to public body regarding existing request @@ -32,7 +32,7 @@ class OutgoingMailer < ApplicationMailer mail(:from => info_request.incoming_name_and_email, :to => OutgoingMailer.name_and_email_for_followup(info_request, incoming_message_followup), - :subject => OutgoingMailer.subject_for_followup(info_request, outgoing_message)) + :subject => OutgoingMailer.subject_for_followup(info_request, outgoing_message, :html => false)) end # TODO: the condition checking valid_to_reply_to? also appears in views/request/_followup.html.erb, @@ -67,11 +67,12 @@ class OutgoingMailer < ApplicationMailer end end # Subject to use for followup - def OutgoingMailer.subject_for_followup(info_request, outgoing_message) + def OutgoingMailer.subject_for_followup(info_request, outgoing_message, options = {}) if outgoing_message.what_doing == 'internal_review' - return "Internal review of " + info_request.email_subject_request + return "Internal review of " + info_request.email_subject_request(:html => options[:html]) else - return info_request.email_subject_followup(outgoing_message.incoming_message_followup) + return info_request.email_subject_followup(:incoming_message => outgoing_message.incoming_message_followup, + :html => options[:html]) end end # Whether we have a valid email address for a followup diff --git a/app/mailers/request_mailer.rb b/app/mailers/request_mailer.rb index 768257ba8..89b76fe97 100644 --- a/app/mailers/request_mailer.rb +++ b/app/mailers/request_mailer.rb @@ -20,7 +20,7 @@ class RequestMailer < ApplicationMailer mail(:from => from_user.name_and_email, :to => info_request.incoming_name_and_email, - :subject => info_request.email_subject_followup) + :subject => info_request.email_subject_followup(:html => false)) end # Used when a response is uploaded using the API diff --git a/app/models/incoming_message.rb b/app/models/incoming_message.rb index db6722976..658ee969a 100644 --- a/app/models/incoming_message.rb +++ b/app/models/incoming_message.rb @@ -52,17 +52,6 @@ class IncomingMessage < ActiveRecord::Base has_prominence - # See binary_mask_stuff function below. It just test for inclusion - # in this hash, not the value of the right hand side. - DoNotBinaryMask = { - 'image/tiff' => 1, - 'image/gif' => 1, - 'image/jpeg' => 1, - 'image/png' => 1, - 'image/bmp' => 1, - 'application/zip' => 1, - } - # Given that there are in theory many info request events, a convenience method for # getting the response event def response_event @@ -218,111 +207,10 @@ class IncomingMessage < ActiveRecord::Base end end - # Converts email addresses we know about into textual descriptions of them - def mask_special_emails!(text) - # TODO: can later display some of these special emails as actual emails, - # if they are public anyway. For now just be precautionary and only - # put in descriptions of them in square brackets. - if self.info_request.public_body.is_followupable? - text.gsub!(self.info_request.public_body.request_email, _("[{{public_body}} request email]", :public_body => self.info_request.public_body.short_or_long_name)) - end - text.gsub!(self.info_request.incoming_email, _('[FOI #{{request}} email]', :request => self.info_request.id.to_s) ) - text.gsub!(AlaveteliConfiguration::contact_email, _("[{{site_name}} contact email]", :site_name => AlaveteliConfiguration::site_name) ) - end - - # Replaces all email addresses in (possibly binary data) with equal length alternative ones. - # Also replaces censor items - def binary_mask_stuff!(text, content_type) - # See if content type is one that we mask - things like zip files and - # images may get broken if we try to. We err on the side of masking too - # much, as many unknown types will really be text. - if DoNotBinaryMask.include?(content_type) - return - end - - # Special cases for some content types - if content_type == 'application/pdf' - uncompressed_text = nil - uncompressed_text = AlaveteliExternalCommand.run("pdftk", "-", "output", "-", "uncompress", :stdin_string => text) - # if we managed to uncompress the PDF... - if !uncompressed_text.nil? && !uncompressed_text.empty? - # then censor stuff (making a copy so can compare again in a bit) - censored_uncompressed_text = uncompressed_text.dup - self._binary_mask_stuff_internal!(censored_uncompressed_text) - # if the censor rule removed something... - if censored_uncompressed_text != uncompressed_text - # then use the altered file (recompressed) - recompressed_text = nil - if AlaveteliConfiguration::use_ghostscript_compression == true - command = ["gs", "-sDEVICE=pdfwrite", "-dCompatibilityLevel=1.4", "-dPDFSETTINGS=/screen", "-dNOPAUSE", "-dQUIET", "-dBATCH", "-sOutputFile=-", "-"] - else - command = ["pdftk", "-", "output", "-", "compress"] - end - recompressed_text = AlaveteliExternalCommand.run(*(command + [{:stdin_string=>censored_uncompressed_text}])) - if recompressed_text.nil? || recompressed_text.empty? - # buggy versions of pdftk sometimes fail on - # compression, I don't see it's a disaster in - # these cases to save an uncompressed version? - recompressed_text = censored_uncompressed_text - logger.warn "Unable to compress PDF; problem with your pdftk version?" - end - if !recompressed_text.nil? && !recompressed_text.empty? - text.replace recompressed_text - end - end - end - return - end - - self._binary_mask_stuff_internal!(text) - end - - # Used by binary_mask_stuff - replace text in place - def _binary_mask_stuff_internal!(text) - # Keep original size, so can check haven't resized it - orig_size = text.mb_chars.size - - # Replace ASCII email addresses... - text.gsub!(MySociety::Validate.email_find_regexp) do |email| - email.gsub(/[^@.]/, 'x') - end - - # And replace UCS-2 ones (for Microsoft Office documents)... - # Find emails, by finding them in parts of text that have ASCII - # equivalents to the UCS-2 - ascii_chars = text.gsub(/\0/, "") - emails = ascii_chars.scan(MySociety::Validate.email_find_regexp) - - # Convert back to UCS-2, making a mask at the same time - if String.method_defined?(:encode) - emails.map! do |email| - # We want the ASCII representation of UCS-2 - [email[0].encode('UTF-16LE').force_encoding('US-ASCII'), - email[0].gsub(/[^@.]/, 'x').encode('UTF-16LE').force_encoding('US-ASCII')] - end - else - emails.map! {|email| [ - Iconv.conv('ucs-2le', 'ascii', email[0]), - Iconv.conv('ucs-2le', 'ascii', email[0].gsub(/[^@.]/, 'x')) - ] } - end - - # Now search and replace the UCS-2 email with the UCS-2 mask - for email, mask in emails - text.gsub!(email, mask) - end - - # Replace censor items - self.info_request.apply_censor_rules_to_binary!(text) - - raise "internal error in binary_mask_stuff" if text.mb_chars.size != orig_size - return text - end - - # Removes censored stuff from from HTML conversion of downloaded binaries - def html_mask_stuff!(html) - self.mask_special_emails!(html) - self.remove_privacy_sensitive_things!(html) + def apply_masks!(text, content_type) + mask_options = { :censor_rules => info_request.applicable_censor_rules, + :masks => info_request.masks } + AlaveteliTextMasker.apply_masks!(text, content_type, mask_options) end # Lotus notes quoting yeuch! @@ -346,26 +234,6 @@ class IncomingMessage < ActiveRecord::Base end - # Remove emails, mobile phones and other details FOI officers ask us to remove. - def remove_privacy_sensitive_things!(text) - # Remove any email addresses - we don't want bounce messages to leak out - # either the requestor's email address or the request's response email - # address out onto the internet - text.gsub!(MySociety::Validate.email_find_regexp, "[email address]") - - # Mobile phone numbers - # http://www.whatdotheyknow.com/request/failed_test_purchases_off_licenc#incoming-1013 - # http://www.whatdotheyknow.com/request/selective_licensing_statistics_i#incoming-550 - # http://www.whatdotheyknow.com/request/common_purpose_training_graduate#incoming-774 - text.gsub!(/(Mobile|Mob)([\s\/]*(Fax|Tel))*\s*:?[\s\d]*\d/, "[mobile number]") - - # Remove WhatDoTheyKnow signup links - text.gsub!(/http:\/\/#{AlaveteliConfiguration::domain}\/c\/[^\s]+/, "[WDTK login link]") - - # Remove things from censor rules - self.info_request.apply_censor_rules_to_text!(text) - end - # Remove quoted sections from emails (eventually the aim would be for this # to do as good a job as GMail does) TODO: bet it needs a proper parser @@ -465,9 +333,8 @@ class IncomingMessage < ActiveRecord::Base raise "main body text more than 1 MB, need to implement clipping like for attachment text, or there is some other MIME decoding problem or similar" end - # remove emails for privacy/anti-spam reasons - self.mask_special_emails!(text) - self.remove_privacy_sensitive_things!(text) + # apply masks for this message + apply_masks!(text, 'text/html') # Remove existing quoted sections folded_quoted_text = self.remove_lotus_quoting(text, 'FOLDED_QUOTED_SECTION') @@ -735,7 +602,14 @@ class IncomingMessage < ActiveRecord::Base text = MySociety::Format.simplify_angle_bracketed_urls(text) text = CGI.escapeHTML(text) text = MySociety::Format.make_clickable(text, :contract => 1) - text.gsub!(/\[(email address|mobile number)\]/, '[<a href="/help/officers#mobiles">\1</a>]') + + # add a helpful link to email addresses and mobile numbers removed + # by apply_masks! + email_pattern = Regexp.escape(_("email address")) + mobile_pattern = Regexp.escape(_("mobile number")) + text.gsub!(/\[(#{email_pattern}|#{mobile_pattern})\]/, + '[<a href="/help/officers#mobiles">\1</a>]') + if collapse_quoted_sections text = text.gsub(/(\s*FOLDED_QUOTED_SECTION\s*)+/m, "FOLDED_QUOTED_SECTION") text.strip! @@ -773,8 +647,8 @@ class IncomingMessage < ActiveRecord::Base # Returns text version of attachment text def get_attachment_text_full text = self._get_attachment_text_internal - self.mask_special_emails!(text) - self.remove_privacy_sensitive_things!(text) + apply_masks!(text, 'text/html') + # This can be useful for memory debugging #STDOUT.puts 'xxx '+ MySociety::DebugHelpers::allocated_string_size_around_gc diff --git a/app/models/info_request.rb b/app/models/info_request.rb index d0052603a..20b7ef9af 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -292,13 +292,18 @@ public end # Subject lines for emails about the request - def email_subject_request - _('{{law_used_full}} request - {{title}}',:law_used_full=>self.law_used_full,:title=>self.title.html_safe) + def email_subject_request(opts = {}) + html = opts.fetch(:html, true) + _('{{law_used_full}} request - {{title}}', + :law_used_full => self.law_used_full, + :title => (html ? title : title.html_safe)) end - def email_subject_followup(incoming_message = nil) + def email_subject_followup(opts = {}) + incoming_message = opts.fetch(:incoming_message, nil) + html = opts.fetch(:html, true) if incoming_message.nil? || !incoming_message.valid_to_reply_to? || !incoming_message.subject - 'Re: ' + self.email_subject_request + 'Re: ' + self.email_subject_request(:html => html) else if incoming_message.subject.match(/^Re:/i) incoming_message.subject @@ -1148,6 +1153,22 @@ public return binary end + # Masks we apply to text associated with this request convert email addresses + # we know about into textual descriptions of them + def masks + masks = [{ :to_replace => incoming_email, + :replacement => _('[FOI #{{request}} email]', + :request => id.to_s) }, + { :to_replace => AlaveteliConfiguration::contact_email, + :replacement => _("[{{site_name}} contact email]", + :site_name => AlaveteliConfiguration::site_name)} ] + if public_body.is_followupable? + masks << { :to_replace => public_body.request_email, + :replacement => _("[{{public_body}} request email]", + :public_body => public_body.short_or_long_name) } + end + end + def is_owning_user?(user) !user.nil? && (user.id == user_id || user.owns_every_request?) end diff --git a/app/views/admin_general/stats.html.erb b/app/views/admin_general/stats.html.erb index 27dc25ee0..03268cc14 100644 --- a/app/views/admin_general/stats.html.erb +++ b/app/views/admin_general/stats.html.erb @@ -53,8 +53,3 @@ </div> </div> </div> -<div class="row"> - <div class="span12"> - <h2>Web analytics</h2> - </div> -</div> diff --git a/app/views/comment/new.html.erb b/app/views/comment/new.html.erb index 578732cdb..48fe055ac 100644 --- a/app/views/comment/new.html.erb +++ b/app/views/comment/new.html.erb @@ -1,4 +1,5 @@ -<% @title = "Make an annotation on '" + h(@info_request.title) + "'" %> +<% @title = _("Add an annotation to “{{request_title}}”", + :request_title => h(@info_request.title)) %> <% if @existing_comment %> <div class="errorExplanation" id="errorExplanation"><ul> @@ -11,8 +12,10 @@ <%= foi_error_messages_for :comment %> -<h1><%= _('Add an annotation')%></h1> -<h2>to “<%=request_link(@info_request)%>”</h2> +<h1> + <%= _("Add an annotation to “{{request_title}}”", + :request_title => request_link(@info_request)) %> +</h1> <p> <%= _('Annotations are so anyone, including you, can help the requester with their request. For example:')%> diff --git a/app/views/general/exception_caught.html.erb b/app/views/general/exception_caught.html.erb index 8d78e2e92..21223dc1e 100644 --- a/app/views/general/exception_caught.html.erb +++ b/app/views/general/exception_caught.html.erb @@ -12,13 +12,10 @@ <%= submit_tag _("Search") %> <% end %> </li> - </ul> + </ul> <% else %> <h1><%= _("Sorry, there was a problem processing this page") %></h1> <p><%= _('You have found a bug. Please <a href="{{contact_url}}">contact us</a> to tell us about the problem', :contact_url => help_contact_path) %></p> <% end %> - <h2><%= _('Technical details') %></h2> - <p><strong><%= h(@exception_class ? @exception_class : _("Unknown")) %></strong></p> - <p><strong><%= h(@exception_message) %></strong></p> </div> diff --git a/app/views/request/_search_ahead.html.erb b/app/views/request/_search_ahead.html.erb index 1e65a5458..4fbe06ebc 100644 --- a/app/views/request/_search_ahead.html.erb +++ b/app/views/request/_search_ahead.html.erb @@ -1,14 +1,20 @@ -<div id="request_search_ahead_results"> - <% if !@xapian_requests.nil? %> - <% if @xapian_requests.results.size > 0 %> +<% unless @xapian_requests.nil? %> + <div id="request_search_ahead_results"> + <% if @xapian_requests.results.any? %> + <span class="close-button">X</span> <h3><%= _("Possibly related requests:") %></h3> - <% end %> - <% for result in @xapian_requests.results %> - <%= render :partial => 'request/request_listing_short_via_event', :locals => { :event => result[:model], :info_request => result[:model].info_request } %> - <% end %> - <p> - <a id="body-site-search-link"><%= _("Or search in their website for this information.") %></a> - </p> - <% end %> -</div> + <% @xapian_requests.results.each do |result| %> + <%= render :partial => 'request/request_listing_short_via_event', + :locals => { :event => result[:model], + :info_request => result[:model].info_request } %> + <% end %> + + <p> + <a id="body-site-search-link"> + <%= _("Search in their website for this information →") %> + </a> + </p> + <% end %> + </div> +<% end %> diff --git a/app/views/request/details.html.erb b/app/views/request/details.html.erb index 3cb2f5afe..194820da6 100644 --- a/app/views/request/details.html.erb +++ b/app/views/request/details.html.erb @@ -1,5 +1,10 @@ -<% @title = _("Details of request '") + h(@info_request.title) + "'" %> -<h1><%= _("Details of request '") + request_link(@info_request) + "'" %></h1> +<% @title = _("Details of request “{{request_title}}”", + :request_title => h(@info_request.title)) %> + +<h1> + <%= _("Details of request “{{request_title}}”", + :request_title => request_link(@info_request)) %> +</h1> <h2><%= _('Event history') %></h2> diff --git a/app/views/request/followup_preview.html.erb b/app/views/request/followup_preview.html.erb index 55afc0245..83978a2f5 100644 --- a/app/views/request/followup_preview.html.erb +++ b/app/views/request/followup_preview.html.erb @@ -3,7 +3,7 @@ <div id="followup"> <%= form_for(@outgoing_message, :html => { :id => 'preview_form' }, :url => (@incoming_message.nil? ? show_response_no_followup_url(:id => @info_request.id) : show_response_url(:id => @info_request.id, :incoming_message_id => @incoming_message.id)) + "#followup" ) do |o| %> - + <% if @internal_review %> <h1><%= _('Now preview your message asking for an internal review') %></h1> <% else %> @@ -20,7 +20,7 @@ <div class="correspondence" id="outgoing-0"> <p class="preview_subject"> <strong><%= _('To:') %></strong> <%=h OutgoingMailer.name_for_followup(@info_request, @incoming_message) %> - <br><strong><%= _('Subject:') %></strong> <%=h OutgoingMailer.subject_for_followup(@info_request, @outgoing_message) %> + <br><strong><%= _('Subject:') %></strong> <%= OutgoingMailer.subject_for_followup(@info_request, @outgoing_message, :html => true) %> </p> <div class="correspondence_text"> diff --git a/app/views/request/new.html.erb b/app/views/request/new.html.erb index 7f1332464..51224129e 100644 --- a/app/views/request/new.html.erb +++ b/app/views/request/new.html.erb @@ -1,19 +1,33 @@ <% unless @batch %> <script type="text/javascript"> $(document).ready(function(){ - // Avoid triggering too often (on each keystroke) by using the debounce jQuery plugin: + // Avoid triggering too often (on each keystroke) by using the + // debounce jQuery plugin: // http://benalman.com/projects/jquery-throttle-debounce-plugin/ $("#typeahead_search").keypress($.debounce( 300, function() { - $("#typeahead_response").load("<%=search_ahead_url%>?q="+encodeURI(this.value), function() { - // When following links in typeahead results, open new tab/window - $("#typeahead_response a").attr("target","_blank"); - - // Update the public body site search link - $("#body-site-search-link").attr("href", "http://www.google.com/#q="+encodeURI($("#typeahead_search").val())+ - "+site:<%= @info_request.public_body.calculated_home_page %>"); + if ( $('#request_search_ahead_results').text().trim().length > 0) { + $('#typeahead_response').slideUp('fast'); + } + + $("#typeahead_response").load("<%= search_ahead_url %>?q="+encodeURI(this.value)+ + "&requested_from=<%= @info_request.public_body.url_name %>"+ + "&per_page=3", function() { + + if ( $('#request_search_ahead_results').text().trim().length > 0) { + $('#typeahead_response').hide().slideDown('fast'); + + // When following links in typeahead results, open new + // tab/window + $("#typeahead_response a").attr("target","_blank"); + + // Update the public body site search link + $("#body-site-search-link").attr("href", "http://www.google.com/#q="+encodeURI($("#typeahead_search").val())+ + "+site:<%= @info_request.public_body.calculated_home_page %>"); + + $('.close-button').click(function() { $(this).parent().hide() }); + } }); })); - }); </script> <% end %> diff --git a/app/views/request/preview.html.erb b/app/views/request/preview.html.erb index 0265d0328..ddd5ab30c 100644 --- a/app/views/request/preview.html.erb +++ b/app/views/request/preview.html.erb @@ -23,7 +23,7 @@ <% else %> <%=h(@info_request.public_body.name)%> <% end %> - <br><strong><%= _('Subject:') %></strong> <%=h @info_request.email_subject_request %> + <br><strong><%= _('Subject:') %></strong> <%= @info_request.email_subject_request %> </p> <div class="correspondence_text"> diff --git a/app/views/user/_signin.html.erb b/app/views/user/_signin.html.erb index afc55d249..864951733 100644 --- a/app/views/user/_signin.html.erb +++ b/app/views/user/_signin.html.erb @@ -14,7 +14,7 @@ <p> <label class="form_label" for="user_signin_password"><%= _('Password:')%></label> - <%= password_field 'user_signin', 'password', { :size => 15, :tabindex => 20 } %> + <%= password_field 'user_signin', 'password', { :size => 15, :tabindex => 20, :autocomplete => 'off' } %> </p> <p class="form_note"> diff --git a/app/views/user/signchangeemail.html.erb b/app/views/user/signchangeemail.html.erb index 7308179f4..a99bcb785 100644 --- a/app/views/user/signchangeemail.html.erb +++ b/app/views/user/signchangeemail.html.erb @@ -23,7 +23,7 @@ <p> <label class="form_label" for="signchangeemail_password"> <%= _('Your password:')%></label> - <%= password_field 'signchangeemail', 'password', { :size => 15 } %> + <%= password_field 'signchangeemail', 'password', { :size => 15, :autocomplete => 'off' } %> </p> <p class="form_note"> diff --git a/app/views/user/signchangepassword.html.erb b/app/views/user/signchangepassword.html.erb index 51bcb466d..60f5d2c62 100644 --- a/app/views/user/signchangepassword.html.erb +++ b/app/views/user/signchangepassword.html.erb @@ -13,12 +13,12 @@ <p> <label class="form_label" for="user_password"><%= _('New password:')%></label> - <%= password_field 'user', 'password', { :size => 15 } %> + <%= password_field 'user', 'password', { :size => 15, :autocomplete => 'off' } %> </p> <p> <label class="form_label" for="user_password_confirmation"><%= _('New password: (again)')%></label> - <%= password_field 'user', 'password_confirmation', { :size => 15 } %> + <%= password_field 'user', 'password_confirmation', { :size => 15, :autocomplete => 'off' } %> </p> <div class="form_button"> diff --git a/config/application.rb b/config/application.rb index ed4f07819..ff72df015 100644 --- a/config/application.rb +++ b/config/application.rb @@ -74,6 +74,10 @@ module Alaveteli ENV['RECAPTCHA_PUBLIC_KEY'] = ::AlaveteliConfiguration::recaptcha_public_key ENV['RECAPTCHA_PRIVATE_KEY'] = ::AlaveteliConfiguration::recaptcha_private_key + if RUBY_VERSION.to_f >= 1.9 + config.middleware.insert 0, Rack::UTF8Sanitizer + end + # Insert a bit of middleware code to prevent uneeded cookie setting. require "#{Rails.root}/lib/whatdotheyknow/strip_empty_sessions" config.middleware.insert_before ::ActionDispatch::Cookies, WhatDoTheyKnow::StripEmptySessions, :key => '_wdtk_cookie_session', :path => "/", :httponly => true diff --git a/config/initializers/alaveteli.rb b/config/initializers/alaveteli.rb index 2ca85579a..128f6bc5a 100644 --- a/config/initializers/alaveteli.rb +++ b/config/initializers/alaveteli.rb @@ -56,6 +56,7 @@ require 'public_body_csv' require 'category_and_heading_migrator' require 'public_body_categories' require 'routing_filters' +require 'alaveteli_text_masker' AlaveteliLocalization.set_locales(AlaveteliConfiguration::available_locales, AlaveteliConfiguration::default_locale) diff --git a/config/initializers/secure_headers.rb b/config/initializers/secure_headers.rb new file mode 100644 index 000000000..99730e6b2 --- /dev/null +++ b/config/initializers/secure_headers.rb @@ -0,0 +1,24 @@ +::SecureHeaders::Configuration.configure do |config| + + # https://tools.ietf.org/html/rfc6797 + if AlaveteliConfiguration::force_ssl + config.hsts = { :max_age => 20.years.to_i, :include_subdomains => true } + else + config.hsts = false + end + # https://tools.ietf.org/html/draft-ietf-websec-x-frame-options-02 + config.x_frame_options = "sameorigin" + + # http://msdn.microsoft.com/en-us/library/ie/gg622941%28v=vs.85%29.aspx + config.x_content_type_options = "nosniff" + + # http://msdn.microsoft.com/en-us/library/dd565647%28v=vs.85%29.aspx + config.x_xss_protection = { :value => 1 } + + # https://w3c.github.io/webappsec/specs/content-security-policy/ + config.csp = false + + # https://www.nwebsec.com/HttpHeaders/SecurityHeaders/XDownloadOptions + config.x_download_options = false +end + diff --git a/lib/alaveteli_text_masker.rb b/lib/alaveteli_text_masker.rb new file mode 100644 index 000000000..68ff0d318 --- /dev/null +++ b/lib/alaveteli_text_masker.rb @@ -0,0 +1,127 @@ +module AlaveteliTextMasker + extend self + DoNotBinaryMask = [ 'image/tiff', + 'image/gif', + 'image/jpeg', + 'image/png', + 'image/bmp', + 'application/zip' ] + + # Replaces all email addresses in (possibly binary) data + # Also applies custom masks and censor items + def apply_masks!(text, content_type, options = {}) + # See if content type is one that we mask - things like zip files and + # images may get broken if we try to. We err on the side of masking too + # much, as many unknown types will really be text. + + # Special cases for some content types + case content_type + when *DoNotBinaryMask + # do nothing + when 'text/html' + apply_text_masks!(text, options) + when 'application/pdf' + apply_pdf_masks!(text, options) + else + apply_binary_masks!(text, options) + end + end + + def apply_pdf_masks!(text, options = {}) + uncompressed_text = nil + uncompressed_text = AlaveteliExternalCommand.run("pdftk", "-", "output", "-", "uncompress", + :stdin_string => text) + # if we managed to uncompress the PDF... + if !uncompressed_text.blank? + # then censor stuff (making a copy so can compare again in a bit) + censored_uncompressed_text = uncompressed_text.dup + apply_binary_masks!(censored_uncompressed_text, options) + # if the censor rule removed something... + if censored_uncompressed_text != uncompressed_text + # then use the altered file (recompressed) + recompressed_text = nil + if AlaveteliConfiguration::use_ghostscript_compression == true + command = ["gs", "-sDEVICE=pdfwrite", "-dCompatibilityLevel=1.4", "-dPDFSETTINGS=/screen", "-dNOPAUSE", "-dQUIET", "-dBATCH", "-sOutputFile=-", "-"] + else + command = ["pdftk", "-", "output", "-", "compress"] + end + recompressed_text = AlaveteliExternalCommand.run(*(command + [{:stdin_string=>censored_uncompressed_text}])) + if recompressed_text.blank? + # buggy versions of pdftk sometimes fail on + # compression, I don't see it's a disaster in + # these cases to save an uncompressed version? + recompressed_text = censored_uncompressed_text + logger.warn "Unable to compress PDF; problem with your pdftk version?" + end + if !recompressed_text.blank? + text.replace recompressed_text + end + end + end + end + + private + + # Replace text in place + def apply_binary_masks!(text, options = {}) + # Keep original size, so can check haven't resized it + orig_size = text.mb_chars.size + + # Replace ASCII email addresses... + text.gsub!(MySociety::Validate.email_find_regexp) do |email| + email.gsub(/[^@.]/, 'x') + end + + # And replace UCS-2 ones (for Microsoft Office documents)... + # Find emails, by finding them in parts of text that have ASCII + # equivalents to the UCS-2 + ascii_chars = text.gsub(/\0/, "") + emails = ascii_chars.scan(MySociety::Validate.email_find_regexp) + + # Convert back to UCS-2, making a mask at the same time + if String.method_defined?(:encode) + emails.map! do |email| + # We want the ASCII representation of UCS-2 + [email[0].encode('UTF-16LE').force_encoding('US-ASCII'), + email[0].gsub(/[^@.]/, 'x').encode('UTF-16LE').force_encoding('US-ASCII')] + end + else + emails.map! {|email| [ + Iconv.conv('ucs-2le', 'ascii', email[0]), + Iconv.conv('ucs-2le', 'ascii', email[0].gsub(/[^@.]/, 'x')) + ] } + end + + # Now search and replace the UCS-2 email with the UCS-2 mask + for email, mask in emails + text.gsub!(email, mask) + end + + # Replace censor items + censor_rules = options[:censor_rules] || [] + censor_rules.each{ |censor_rule| censor_rule.apply_to_binary!(text) } + raise "internal error in apply_binary_masks!" if text.mb_chars.size != orig_size + return text + end + + # Remove any email addresses, login links and mobile phone numbers + def default_text_masks + [{ :to_replace => MySociety::Validate.email_find_regexp, + :replacement => "[#{_("email address")}]" }, + { :to_replace => /(Mobile|Mob)([\s\/]*(Fax|Tel))*\s*:?[\s\d]*\d/, + :replacement => "[#{_("mobile number")}]" }, + { :to_replace => /https?:\/\/#{AlaveteliConfiguration::domain}\/c\/[^\s]+/, + :replacement => "[#{_("{{site_name}} login link", + :site_name => AlaveteliConfiguration::site_name)}]" }] + end + + def apply_text_masks!(text, options = {}) + masks = options[:masks] || [] + masks += default_text_masks + censor_rules = options[:censor_rules] || [] + masks.each{ |mask| text.gsub!(mask[:to_replace], mask[:replacement]) } + censor_rules.each{ |censor_rule| censor_rule.apply_to_text!(text) } + text + end + +end diff --git a/lib/quiet_opener.rb b/lib/quiet_opener.rb index 16ea27b8e..c6e259b93 100644 --- a/lib/quiet_opener.rb +++ b/lib/quiet_opener.rb @@ -7,8 +7,19 @@ end def quietly_try_to_open(url) begin result = open(url).read.strip - rescue OpenURI::HTTPError, SocketError, Errno::ETIMEDOUT, Errno::ECONNREFUSED, Errno::EHOSTUNREACH, Errno::ECONNRESET - Rails.logger.warn("Unable to open third-party URL #{url}") + rescue OpenURI::HTTPError, + SocketError, + Errno::ETIMEDOUT, + Errno::ECONNREFUSED, + Errno::EHOSTUNREACH, + Errno::ECONNRESET, + Timeout::Error => exception + e = Exception.new("Unable to open third-party URL #{url}: #{exception.message}") + e.set_backtrace(exception.backtrace) + if !AlaveteliConfiguration.exception_notifications_from.blank? && !AlaveteliConfiguration.exception_notifications_to.blank? + ExceptionNotifier::Notifier.exception_notification(request.env, e).deliver + end + Rails.logger.warn(e.message) result = "" end return result diff --git a/lib/tasks/gettext.rake b/lib/tasks/gettext.rake index 3f357213f..9bdb6169b 100644 --- a/lib/tasks/gettext.rake +++ b/lib/tasks/gettext.rake @@ -11,11 +11,7 @@ namespace :gettext do desc "Update pot/po files for a theme." task :find_theme => :environment do - theme = ENV['THEME'] - unless theme - puts "Usage: Specify an Alaveteli-theme with THEME=[theme directory name]" - exit(0) - end + theme = find_theme(ENV['THEME']) load_gettext msgmerge = Rails.application.config.gettext_i18n_rails.msgmerge msgmerge ||= %w[--sort-output --no-location --no-wrap] @@ -28,6 +24,25 @@ namespace :gettext do ) end + desc 'Rewrite theme .po files into a consistent msgmerge format' + task :clean_theme do + theme = find_theme(ENV['THEME']) + load_gettext + + Dir.glob("#{ theme_locale_path(theme) }/*/app.po") do |po_file| + GetText::msgmerge(po_file, po_file, 'alaveteli', + :msgmerge => [:sort_output, :no_location, :no_wrap]) + end + end + + def find_theme(theme) + unless theme + puts "Usage: Specify an Alaveteli-theme with THEME=[theme directory name]" + exit(0) + end + theme + end + def theme_files_to_translate(theme) Dir.glob("{lib/themes/#{theme}/lib}/**/*.{rb,erb}") end diff --git a/spec/controllers/general_controller_spec.rb b/spec/controllers/general_controller_spec.rb index c0a9d57d3..4a7a0bb48 100644 --- a/spec/controllers/general_controller_spec.rb +++ b/spec/controllers/general_controller_spec.rb @@ -126,6 +126,35 @@ describe GeneralController, "when showing the frontpage" do end + describe 'when handling logged-in users' do + + before do + @user = FactoryGirl.create(:user) + session[:user_id] = @user.id + end + + it 'should set a time to live on a non "remember me" session' do + get :frontpage + response.body.should match @user.name + session[:ttl].should be_within(1).of(Time.now) + end + + it 'should not set a time to live on a "remember me" session' do + session[:remember_me] = true + get :frontpage + response.body.should match @user.name + session[:ttl].should be_nil + end + + it 'should end a logged-in session whose ttl has expired' do + session[:ttl] = Time.now - 4.hours + get :frontpage + response.should redirect_to signin_path + session[:user_id].should be_nil + end + + end + end diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb index 6c0f4573e..ba558cc93 100644 --- a/spec/controllers/request_controller_spec.rb +++ b/spec/controllers/request_controller_spec.rb @@ -2380,6 +2380,23 @@ describe RequestController, "when doing type ahead searches" do get :search_typeahead, :q => "dog -chicken" assigns[:xapian_requests].results.size.should == 1 end + + it 'can filter search results by public body' do + get :search_typeahead, :q => 'boring', :requested_from => 'dfh' + expect(assigns[:query]).to eq('requested_from:dfh boring') + end + + it 'defaults to 25 results per page' do + get :search_typeahead, :q => 'boring' + expect(assigns[:per_page]).to eq(25) + end + + it 'can limit the number of searches returned' do + get :search_typeahead, :q => 'boring', :per_page => '1' + expect(assigns[:per_page]).to eq(1) + expect(assigns[:xapian_requests].results.size).to eq(1) + end + end describe RequestController, "when showing similar requests" do @@ -2430,7 +2447,7 @@ describe RequestController, "when caching fragments" do :info_request_id => 132, :id => 44, :get_attachments_for_display => nil, - :html_mask_stuff! => nil, + :apply_masks! => nil, :user_can_view? => true, :all_can_view? => true) attachment = FactoryGirl.build(:body_text, :filename => long_name) diff --git a/spec/controllers/services_controller_spec.rb b/spec/controllers/services_controller_spec.rb index 14731f090..248c97ad4 100644 --- a/spec/controllers/services_controller_spec.rb +++ b/spec/controllers/services_controller_spec.rb @@ -60,21 +60,25 @@ describe ServicesController, "when returning a message for people in other count response.should be_success response.body.should == 'Hello! We have an <a href="/help/alaveteli?country_name=Deutschland">important message</a> for visitors outside Deutschland' end + it "should default to no message if the country_from_ip domain doesn't exist" do AlaveteliConfiguration.stub!(:gaze_url).and_return('http://12123sdf14qsd.com') get :other_country_message response.should be_success response.body.should == '' end + it "should default to no message if the country_from_ip service doesn't exist" do AlaveteliConfiguration.stub!(:gaze_url).and_return('http://www.google.com') get :other_country_message response.should be_success response.body.should == '' end - it "should default to no message if the country_from_ip service returns an error" do + + it "should default to no message and log the error with url if the country_from_ip service returns an error" do FakeWeb.register_uri(:get, %r|500.com|, :body => "Error", :status => ["500", "Error"]) AlaveteliConfiguration.stub!(:gaze_url).and_return('http://500.com') + Rails.logger.should_receive(:warn).with /500\.com.*500 Error/ get :other_country_message response.should be_success response.body.should == '' diff --git a/spec/integration/errors_spec.rb b/spec/integration/errors_spec.rb index 4fa12fb21..110626058 100644 --- a/spec/integration/errors_spec.rb +++ b/spec/integration/errors_spec.rb @@ -59,24 +59,20 @@ describe "When errors occur" do response.should render_template('general/exception_caught') response.code.should == '404' response.body.should match("Sorry, we couldn't find that page") - response.body.should match(%Q(invalid value for Integer)) end - # it 'should handle non utf-8 parameters' do - # pending 'until we sanitize non utf-8 parameters for Ruby >= 1.9' do - # get ('/%d3') - # response.should render_template('general/exception_caught') - # response.code.should == '404' - # response.body.should match("Sorry, we couldn't find that page") - # end - # end + it 'should handle non utf-8 parameters' do + get ('/%d3') + response.should render_template('general/exception_caught') + response.code.should == '404' + response.body.should match("Sorry, we couldn't find that page") + end it "should render a 500 for general errors using the general/exception_caught template" do InfoRequest.stub!(:find_by_url_title!).and_raise("An example error") get("/request/example") response.should render_template('general/exception_caught') - response.body.should match('An example error') response.code.should == "500" end @@ -111,7 +107,6 @@ describe "When errors occur" do get("/es/request/example") response.should render_template('general/exception_caught') response.body.should match('Lo sentimos, hubo un problema procesando esta página') - response.body.should match('An example error') end it "should render a 403 with text body for attempts at directory listing for attachments" do diff --git a/spec/lib/alaveteli_text_masker_spec.rb b/spec/lib/alaveteli_text_masker_spec.rb new file mode 100644 index 000000000..1a4782a83 --- /dev/null +++ b/spec/lib/alaveteli_text_masker_spec.rb @@ -0,0 +1,146 @@ +# -*- coding: utf-8 -*- +require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') + +describe AlaveteliTextMasker do + include AlaveteliTextMasker + + describe :apply_masks! do + + describe 'when applying censor rules' do + + before do + @cheese_censor_rule = FactoryGirl.build(:censor_rule, :text => 'Stilton', + :replacement => 'Jarlsberg') + @colour_censor_rule = FactoryGirl.build(:censor_rule, :text => 'blue', + :replacement => 'yellow') + @regex_censor_rule = FactoryGirl.build(:censor_rule, :text => 'm[a-z][a-z][a-z]e', + :replacement => 'cat', + :regexp => true) + @censor_rules = [@cheese_censor_rule, @colour_censor_rule, @regex_censor_rule] + end + + it "should do nothing to a JPEG" do + data = "There was a mouse called Stilton, he wished that he was blue." + apply_masks!(data, "image/jpeg", :censor_rules => @censor_rules) + data.should == "There was a mouse called Stilton, he wished that he was blue." + end + + it "should replace censor text in Word documents" do + data = "There was a mouse called Stilton, he wished that he was blue." + apply_masks!(data, "application/vnd.ms-word", :censor_rules => @censor_rules) + data.should == "There was a xxxxx called xxxxxxx, he wished that he was xxxx." + end + + it 'should handle multibyte characters correctly' do + data = 'á mouse' + @regex_censor_rule.text = 'á' + apply_masks!(data, "application/octet-stream", :censor_rules => @censor_rules).should == 'x mouse' + end + + it "should apply censor rules to HTML files" do + data = "There was a mouse called Stilton, he wished that he was blue." + apply_masks!(data, 'text/html', :censor_rules => @censor_rules) + data.should == "There was a cat called Jarlsberg, he wished that he was yellow." + end + + end + + it "should replace ASCII email addresses in Word documents" do + data = "His email was foo@bar.com" + expected = "His email was xxx@xxx.xxx" + apply_masks!(data, "application/vnd.ms-word") + data.should == expected + end + + + it "should replace UCS-2 addresses in Word documents" do + data = "His email was f\000o\000o\000@\000b\000a\000r\000.\000c\000o\000m\000, indeed" + apply_masks!(data, "application/vnd.ms-word") + data.should == "His email was x\000x\000x\000@\000x\000x\000x\000.\000x\000x\000x\000, indeed" + end + + def pdf_replacement_test(use_ghostscript_compression) + config = MySociety::Config.load_default() + previous = config['USE_GHOSTSCRIPT_COMPRESSION'] + config['USE_GHOSTSCRIPT_COMPRESSION'] = use_ghostscript_compression + orig_pdf = load_file_fixture('tfl.pdf') + pdf = orig_pdf.dup + + orig_text = MailHandler.get_attachment_text_one_file('application/pdf', pdf) + orig_text.should match(/foi@tfl.gov.uk/) + + apply_masks!(pdf, "application/pdf") + + masked_text = MailHandler.get_attachment_text_one_file('application/pdf', pdf) + masked_text.should_not match(/foi@tfl.gov.uk/) + masked_text.should match(/xxx@xxx.xxx.xx/) + config['USE_GHOSTSCRIPT_COMPRESSION'] = previous + end + + it "should replace everything in PDF files using pdftk" do + pdf_replacement_test(false) + end + + it "should replace everything in PDF files using ghostscript" do + pdf_replacement_test(true) + end + + it "should not produce zero length output if pdftk silently fails" do + orig_pdf = load_file_fixture('psni.pdf') + pdf = orig_pdf.dup + apply_masks!(pdf, "application/pdf") + pdf.should_not == "" + end + + it "should apply hard-coded privacy rules to HTML files" do + data = "http://test.host/c/cheese" + apply_masks!(data, 'text/html') + data.should == "[Alaveteli login link]" + end + + it 'should replace a simple email address' do + expected = "the address is [email address]" + apply_masks!("the address is test@example.com", 'text/html', {}).should == expected + end + + it 'should replace a mobile phone number prefixed with "Mobile"' do + expected = "the mobile is [mobile number]" + apply_masks!("the mobile is Mobile 55555 555555", 'text/html', {}).should == expected + end + + it 'should replace a mobile phone number prefixed with "Mob Tel"' do + expected = "the mobile is [mobile number]" + apply_masks!("the mobile is Mob Tel: 55555 555 555", 'text/html', {}).should == expected + end + + it 'should replace a mobile phone number prefixed with "Mob/Fax:"' do + expected = "the mobile is [mobile number]" + apply_masks!("the mobile is Mob/Fax: 55555 555555", 'text/html', {}).should == expected + end + + it "should replace an Alaveteli login link" do + expected = "the login link is [Alaveteli login link]" + apply_masks!("the login link is http://test.host/c/ekfmsdfkm", 'text/html', {}).should == expected + end + + it "should replace a https Alaveteli login link" do + expected = "the login link is [Alaveteli login link]" + apply_masks!("the login link is https://test.host/c/ekfmsdfkm", 'text/html', {}).should == expected + end + + it "should apply censor rules to text" do + censor_rule = FactoryGirl.build(:censor_rule, :text => 'mouse', :replacement => 'cat') + expected = "here is a cat" + apply_masks!("here is a mouse", 'text/html', {:censor_rules => [ censor_rule ]}).should == expected + end + + it 'should apply extra masks to text' do + mask = {:to_replace => 'mouse', :replacement => 'cat'} + expected = "here is a cat" + apply_masks!("here is a mouse", 'text/html', {:masks => [ mask ]}).should == expected + end + + end + +end + diff --git a/spec/mailers/outgoing_mailer_spec.rb b/spec/mailers/outgoing_mailer_spec.rb index a11d56dd3..3df5018fe 100644 --- a/spec/mailers/outgoing_mailer_spec.rb +++ b/spec/mailers/outgoing_mailer_spec.rb @@ -75,14 +75,14 @@ describe OutgoingMailer, "when working out follow up subjects" do ir = info_requests(:fancy_dog_request) im = ir.incoming_messages[0] - ir.email_subject_request.should == "Freedom of Information request - Why do you have & such a fancy dog?" + ir.email_subject_request(:html => false).should == "Freedom of Information request - Why do you have & such a fancy dog?" end it "should use 'Re:' and inital request subject for followups which aren't replies to particular messages" do ir = info_requests(:fancy_dog_request) om = outgoing_messages(:useless_outgoing_message) - OutgoingMailer.subject_for_followup(ir, om).should == "Re: Freedom of Information request - Why do you have & such a fancy dog?" + OutgoingMailer.subject_for_followup(ir, om, :html => false).should == "Re: Freedom of Information request - Why do you have & such a fancy dog?" end it "should prefix with Re: the subject of the message being replied to" do @@ -91,7 +91,7 @@ describe OutgoingMailer, "when working out follow up subjects" do om = outgoing_messages(:useless_outgoing_message) om.incoming_message_followup = im - OutgoingMailer.subject_for_followup(ir, om).should == "Re: Geraldine FOI Code AZXB421" + OutgoingMailer.subject_for_followup(ir, om, :html => false).should == "Re: Geraldine FOI Code AZXB421" end it "should not add Re: prefix if there already is such a prefix" do @@ -101,7 +101,7 @@ describe OutgoingMailer, "when working out follow up subjects" do om.incoming_message_followup = im im.raw_email.data = im.raw_email.data.sub("Subject: Geraldine FOI Code AZXB421", "Subject: Re: Geraldine FOI Code AZXB421") - OutgoingMailer.subject_for_followup(ir, om).should == "Re: Geraldine FOI Code AZXB421" + OutgoingMailer.subject_for_followup(ir, om, :html => false).should == "Re: Geraldine FOI Code AZXB421" end it "should not add Re: prefix if there already is a lower case re: prefix" do @@ -113,7 +113,7 @@ describe OutgoingMailer, "when working out follow up subjects" do im.raw_email.data = im.raw_email.data.sub("Subject: Geraldine FOI Code AZXB421", "Subject: re: Geraldine FOI Code AZXB421") im.parse_raw_email! true - OutgoingMailer.subject_for_followup(ir, om).should == "re: Geraldine FOI Code AZXB421" + OutgoingMailer.subject_for_followup(ir, om, :html => false).should == "re: Geraldine FOI Code AZXB421" end it "should use 'Re:' and initial request subject when replying to failed delivery notifications" do @@ -126,7 +126,7 @@ describe OutgoingMailer, "when working out follow up subjects" do im.raw_email.data = im.raw_email.data.sub("Subject: Geraldine FOI Code AZXB421", "Subject: Delivery Failed") im.parse_raw_email! true - OutgoingMailer.subject_for_followup(ir, om).should == "Re: Freedom of Information request - Why do you have & such a fancy dog?" + OutgoingMailer.subject_for_followup(ir, om, :html => false).should == "Re: Freedom of Information request - Why do you have & such a fancy dog?" end end diff --git a/spec/models/incoming_message_spec.rb b/spec/models/incoming_message_spec.rb index 3b6887f76..f6e524de3 100644 --- a/spec/models/incoming_message_spec.rb +++ b/spec/models/incoming_message_spec.rb @@ -423,127 +423,50 @@ describe IncomingMessage, " checking validity to reply to with real emails" do end -describe IncomingMessage, " when censoring data" do - - before(:each) do - @test_data = "There was a mouse called Stilton, he wished that he was blue." - - @im = incoming_messages(:useless_incoming_message) - - @censor_rule_1 = CensorRule.new() - @censor_rule_1.text = "Stilton" - @censor_rule_1.replacement = "Jarlsberg" - @censor_rule_1.last_edit_editor = "unknown" - @censor_rule_1.last_edit_comment = "none" - @im.info_request.censor_rules << @censor_rule_1 - - @censor_rule_2 = CensorRule.new() - @censor_rule_2.text = "blue" - @censor_rule_2.replacement = "yellow" - @censor_rule_2.last_edit_editor = "unknown" - @censor_rule_2.last_edit_comment = "none" - @im.info_request.censor_rules << @censor_rule_2 - - @regex_censor_rule = CensorRule.new() - @regex_censor_rule.text = 'm[a-z][a-z][a-z]e' - @regex_censor_rule.regexp = true - @regex_censor_rule.replacement = 'cat' - @regex_censor_rule.last_edit_editor = 'unknown' - @regex_censor_rule.last_edit_comment = 'none' - @im.info_request.censor_rules << @regex_censor_rule - load_raw_emails_data - end - - it "should do nothing to a JPEG" do - data = @test_data.dup - @im.binary_mask_stuff!(data, "image/jpeg") - data.should == @test_data - end - - it "should replace censor text in Word documents" do - data = @test_data.dup - @im.binary_mask_stuff!(data, "application/vnd.ms-word") - data.should == "There was a xxxxx called xxxxxxx, he wished that he was xxxx." - end - - it "should replace ASCII email addresses in Word documents" do - orig_data = "His email was foo@bar.com" - data = orig_data.dup - @im.binary_mask_stuff!(data, "application/vnd.ms-word") - data.should == "His email was xxx@xxx.xxx" - end - - it "should replace UCS-2 addresses in Word documents" do - orig_data = "His email was f\000o\000o\000@\000b\000a\000r\000.\000c\000o\000m\000, indeed" - data = orig_data.dup - @im.binary_mask_stuff!(data, "application/vnd.ms-word") - data.should == "His email was x\000x\000x\000@\000x\000x\000x\000.\000x\000x\000x\000, indeed" - end - - it 'should handle multibyte characters correctly' do - orig_data = 'á' - data = orig_data.dup - @regex_censor_rule = CensorRule.new() - @regex_censor_rule.text = 'á' - @regex_censor_rule.regexp = true - @regex_censor_rule.replacement = 'cat' - @regex_censor_rule.last_edit_editor = 'unknown' - @regex_censor_rule.last_edit_comment = 'none' - @im.info_request.censor_rules << @regex_censor_rule - lambda{ @im.binary_mask_stuff!(data, "text/plain") }.should_not raise_error - end - def pdf_replacement_test(use_ghostscript_compression) - config = MySociety::Config.load_default() - previous = config['USE_GHOSTSCRIPT_COMPRESSION'] - config['USE_GHOSTSCRIPT_COMPRESSION'] = use_ghostscript_compression - orig_pdf = load_file_fixture('tfl.pdf') - pdf = orig_pdf.dup - - orig_text = MailHandler.get_attachment_text_one_file('application/pdf', pdf) - orig_text.should match(/foi@tfl.gov.uk/) - - @im.binary_mask_stuff!(pdf, "application/pdf") - - masked_text = MailHandler.get_attachment_text_one_file('application/pdf', pdf) - masked_text.should_not match(/foi@tfl.gov.uk/) - masked_text.should match(/xxx@xxx.xxx.xx/) - config['USE_GHOSTSCRIPT_COMPRESSION'] = previous - end - - it "should replace everything in PDF files using pdftk" do - pdf_replacement_test(false) - end - - it "should replace everything in PDF files using ghostscript" do - pdf_replacement_test(true) - end - - it "should not produce zero length output if pdftk silently fails" do - orig_pdf = load_file_fixture('psni.pdf') - pdf = orig_pdf.dup - @im.binary_mask_stuff!(pdf, "application/pdf") - pdf.should_not == "" - end - - it "should apply censor rules to HTML files" do - data = @test_data.dup - @im.html_mask_stuff!(data) - data.should == "There was a cat called Jarlsberg, he wished that he was yellow." - end - - it "should apply hard-coded privacy rules to HTML files" do - data = "http://#{AlaveteliConfiguration::domain}/c/cheese" - @im.html_mask_stuff!(data) - data.should == "[WDTK login link]" - end +describe IncomingMessage, " when censoring data" do - it "should apply censor rules to From: addresses" do - @im.stub!(:mail_from).and_return("Stilton Mouse") - @im.stub!(:last_parsed).and_return(Time.now) - safe_mail_from = @im.safe_mail_from - safe_mail_from.should == "Jarlsberg Mouse" - end + before(:each) do + @test_data = "There was a mouse called Stilton, he wished that he was blue." + + @im = incoming_messages(:useless_incoming_message) + + @censor_rule_1 = CensorRule.new() + @censor_rule_1.text = "Stilton" + @censor_rule_1.replacement = "Jarlsberg" + @censor_rule_1.last_edit_editor = "unknown" + @censor_rule_1.last_edit_comment = "none" + @im.info_request.censor_rules << @censor_rule_1 + + @censor_rule_2 = CensorRule.new() + @censor_rule_2.text = "blue" + @censor_rule_2.replacement = "yellow" + @censor_rule_2.last_edit_editor = "unknown" + @censor_rule_2.last_edit_comment = "none" + @im.info_request.censor_rules << @censor_rule_2 + + @regex_censor_rule = CensorRule.new() + @regex_censor_rule.text = 'm[a-z][a-z][a-z]e' + @regex_censor_rule.regexp = true + @regex_censor_rule.replacement = 'cat' + @regex_censor_rule.last_edit_editor = 'unknown' + @regex_censor_rule.last_edit_comment = 'none' + @im.info_request.censor_rules << @regex_censor_rule + load_raw_emails_data + end + + it "should replace censor text" do + data = "There was a mouse called Stilton, he wished that he was blue." + @im.apply_masks!(data, "application/vnd.ms-word") + data.should == "There was a xxxxx called xxxxxxx, he wished that he was xxxx." + end + + it "should apply censor rules to From: addresses" do + @im.stub!(:mail_from).and_return("Stilton Mouse") + @im.stub!(:last_parsed).and_return(Time.now) + safe_mail_from = @im.safe_mail_from + safe_mail_from.should == "Jarlsberg Mouse" + end end @@ -565,15 +488,16 @@ describe IncomingMessage, " when censoring whole users" do it "should apply censor rules to HTML files" do data = @test_data.dup - @im.html_mask_stuff!(data) + @im.apply_masks!(data, 'text/html') data.should == "There was a mouse called Gorgonzola, he wished that he was blue." end it "should replace censor text to Word documents" do data = @test_data.dup - @im.binary_mask_stuff!(data, "application/vnd.ms-word") + @im.apply_masks!(data, "application/vnd.ms-word") data.should == "There was a mouse called xxxxxxx, he wished that he was blue." end + end @@ -770,3 +694,16 @@ describe IncomingMessage, "when extracting attachments" do end end + +describe IncomingMessage, 'when getting the body of a message for html display' do + + it 'should replace any masked email addresses with a link to the help page' do + incoming_message = IncomingMessage.new + body_text = 'there was an [email address] here' + incoming_message.stub!(:get_main_body_text_folded).and_return(body_text) + incoming_message.stub!(:get_main_body_text_unfolded).and_return(body_text) + expected = 'there was an [<a href="/help/officers#mobiles">email address</a>] here' + incoming_message.get_body_for_html_display.should == expected + end + +end diff --git a/spec/models/info_request_spec.rb b/spec/models/info_request_spec.rb index 9ad616ea5..70947584b 100644 --- a/spec/models/info_request_spec.rb +++ b/spec/models/info_request_spec.rb @@ -824,7 +824,7 @@ describe InfoRequest do im = mock_model(IncomingMessage, :subject => nil, :valid_to_reply_to? => true) - subject = ir.email_subject_followup im + subject = ir.email_subject_followup(:incoming_message => im, :html => false) subject.should match(/^Re: Freedom of Information request.*fancy dog/) end |