Don't offer or allow viewing of an HTML version of a response attachment if the request is hidden, or requester_only. Google docs viewer won't be able to access it, and our own conversion process currently can produce image files that will then be publicly viewable directly from the webserver (see config/httpd.conf). If necessary we can revisit this code to enable admins and requesters to view the HTML version created by our own conversion without adding these files to a path that is served directly by the web server.

3 files changed, 29 insertions, 8 deletions

diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index c732a4b32..2c95114e6 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -743,6 +743,12 @@ class RequestController < ApplicationController
     end
 
     def get_attachment_as_html
+
+        # The conversion process can generate files in the cache directory that can be served up
+        # directly by the webserver according to httpd.conf, so don't allow it unless that's OK.
+        if @files_can_be_cached != true
+            raise ActiveRecord::RecordNotFound.new("Attachment HTML not found.")
+        end
         get_attachment_internal(true)
 
         # images made during conversion (e.g. images in PDF files) are put in the cache directory, so
diff --git a/app/views/request/_bubble.rhtml b/app/views/request/_bubble.rhtml
index 331c2163e..747e2aa1f 100644
--- a/app/views/request/_bubble.rhtml
+++ b/app/views/request/_bubble.rhtml
@@ -1,16 +1,16 @@
     <div class="correspondence_text">
         <%  if not attachments.nil? and attachments.size > 0 %>
-            <div class="attachments"> 
+            <div class="attachments">
                 <hr class="top">
                 <% attachments.each do |a| %>
                     <p class="attachment">
-                        <% 
+                        <%
                             attachment_url = get_attachment_url(:id => incoming_message.info_request_id,
-                                    :incoming_message_id => incoming_message.id, :part => a.url_part_number, 
-                                    :file_name => a.display_filename) 
+                                    :incoming_message_id => incoming_message.id, :part => a.url_part_number,
+                                    :file_name => a.display_filename)
                             attachment_as_html_url = get_attachment_as_html_url(:id => incoming_message.info_request_id,
-                                    :incoming_message_id => incoming_message.id, :part => a.url_part_number, 
-                                    :file_name => a.display_filename + '.html') 
+                                    :incoming_message_id => incoming_message.id, :part => a.url_part_number,
+                                    :file_name => a.display_filename + '.html')
                         %>
                         <% img_filename = "icon_" + a.content_type.sub('/', '_') + "_large.png"
                         full_filename = File.expand_path(File.join(File.dirname(__FILE__), "../../../public/images", img_filename))
@@ -23,9 +23,9 @@
                         <br>
                         <%= a.display_size %>
                         <%= link_to "Download", attachment_url %>
-                        <% if a.has_body_as_html? %>
+                        <% if a.has_body_as_html? && incoming_message.info_request.all_can_view? %>
                             <%= link_to "View as HTML", attachment_as_html_url %>
-                        <% end %> 
+                        <% end %>
                         <!-- (<%= a.content_type %>) -->
                         <%= a.extra_note %>
                     </p>
diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb
index b0223588e..43eca46cd 100644
--- a/spec/controllers/request_controller_spec.rb
+++ b/spec/controllers/request_controller_spec.rb
@@ -859,6 +859,21 @@ describe RequestController, "when changing prominence of a request" do
         response.should render_template('request/hidden')
     end
 
+    it 'should not generate an HTML version of an attachment whose prominence is hidden/requester
+        only even for the requester or an admin but should return a 404' do
+        ir = info_requests(:fancy_dog_request)
+        ir.prominence = 'hidden'
+        ir.save!
+        receive_incoming_mail('incoming-request-two-same-name.email', ir.incoming_email)
+        session[:user_id] = users(:admin_user).id
+        lambda do
+            get :get_attachment_as_html, :incoming_message_id => ir.incoming_messages[1].id,
+                                      :id => ir.id,
+                                      :part => 2,
+                                      :file_name => ['hello.txt']
+        end.should raise_error(ActiveRecord::RecordNotFound)
+    end
+
 end
 
 # XXX do this for invalid ids