disallow caching of instfs redirects

closes RECNVS-358
refs RECNVS-73

instfs JWTs cannot be shared across users, so we cannot cache them
across users. while most browsers will only service one user and caching
independent of user would not be detrimental, we cannot guarantee that.
so we can't let the browser cache the instfs redirect.

test-plan:
- enable inst-fs
- login as a teacher of a course:
- upload an image to a course's files area
- create or edit a course page and insert the uploaded image (click on
  the image from the RCE sidebar) into the page
- save and view the page; the image should display
- log out and then login, using the same browser, as a student of the
  same course
- view the same course page
- the image should display without issue

Change-Id: I9831ce61d8224112c049e2ac7b72d580a5fe8cde
Reviewed-on: https://gerrit.instructure.com/143226
Reviewed-by: Andrew Huff <[email protected]>
Tested-by: Jenkins
QA-Review: Andrew Huff <[email protected]>
Product-Review: Jacob Fugal <[email protected]>

app/helpers/attachment_helper.rb

@@ -90,7 +90,15 @@ def safer_domain_available?
   end
 
   def set_cache_header(attachment, inline)
-    unless attachment.content_type.match(/\Atext/) || attachment.extension == '.html' || attachment.extension == '.htm'
+    # TODO [RECNVS-73]
+    # instfs JWTs cannot be shared across users, so we cannot cache them across
+    # users. while most browsers will only service one user and caching
+    # independent of user would not be detrimental, we cannot guarantee that.
+    # so we can't let the browser cache the instfs redirect. we should still
+    # investigate opportunities to reuse JWTs when the same user requests the
+    # same file within a reasonable window of time, so that the URL redirected
+    # too can still take advantage of browser caching.
+    unless attachment.instfs_hosted? || attachment.content_type.match(/\Atext/) || attachment.extension == '.html' || attachment.extension == '.htm'
       cancel_cache_buster
       #set cache to expire whenever the s3 url does (or one day if local or inline proxy), max-age take seconds, and Expires takes a date
       ttl = attachment.stored_locally? || (inline && attachment.can_be_proxied?) ? attachment.url_ttl : 1.day

spec/helpers/attachment_helper_spec.rb

@@ -44,4 +44,13 @@
     expect(attrs).to match /#{@current_user.id}/
     expect(attrs).to match /#{@att.id}/
   end
+
+  describe "set_cache_header" do
+    it "should not allow caching of instfs redirects" do
+      allow(@att).to receive(:instfs_hosted?).and_return(true)
+      expect(self).to receive(:cancel_cache_buster).never
+      set_cache_header(@att, false)
+      expect(response.headers).not_to have_key('Cache-Control')
+    end
+  end
 end