always redirect to S3 for file downloads

fixes CNVS-35006

test plan:
 * make sure you have a files domain setup
 * download/view files in various places in Canvas, especially
   submissions of raw images in speedgrader

Change-Id: Iffe32e5b29182cf229755fd087b453edd064357f
Reviewed-on: https://gerrit.instructure.com/110817
Reviewed-by: Tyler Pickett <tpickett@instructure.com>
Tested-by: Jenkins
QA-Review: Jeremy Putnam <jeremyp@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>

app/controllers/files_controller.rb

@@ -626,12 +626,12 @@ class FilesController < ApplicationController
         redirect_to(named_context_url(@context, :context_file_url, attachment.id))
       end
     else
-      send_stored_file(attachment, false, true)
+      send_stored_file(attachment, false)
     end
   end
   protected :send_attachment
 
-  def send_stored_file(attachment, inline=true, redirect_to_s3=false)
+  def send_stored_file(attachment, inline=true)
     user = @current_user
     user ||= api_find(User, params[:user_id]) if params[:user_id].present?
     attachment.context_module_action(user, :read) if user && !params[:preview]
@@ -639,8 +639,7 @@ class FilesController < ApplicationController
     render_or_redirect_to_stored_file(
       attachment: attachment,
       verifier: params[:verifier],
-      inline: inline,
-      redirect_to_s3: redirect_to_s3
+      inline: inline
     )
   end
   protected :send_stored_file

app/controllers/lti/submissions_api_controller.rb

@@ -173,7 +173,7 @@ module Lti
       attachment = Attachment.find(params[:attachment_id])
       render_unauthorized and return unless attachment_for_submission?(attachment)
       render_or_redirect_to_stored_file(
-        attachment: attachment, redirect_to_s3: true)
+        attachment: attachment)
     end
 
 

app/helpers/attachment_helper.rb

@@ -57,18 +57,15 @@ module AttachmentHelper
     }
   end
 
-  def render_or_redirect_to_stored_file(attachment:, verifier: nil, inline: false, redirect_to_s3: false)
+  def render_or_redirect_to_stored_file(attachment:, verifier: nil, inline: false)
     set_cache_header(attachment)
     if safer_domain_available?
       redirect_to safe_domain_file_url(attachment, @safer_domain_host, verifier, !inline)
     elsif Attachment.local_storage?
       @headers = false if @files_domain
       send_file(attachment.full_filename, :type => attachment.content_type_with_encoding, :disposition => (inline ? 'inline' : 'attachment'), :filename => attachment.display_name)
-    elsif redirect_to_s3
-      redirect_to(inline ? attachment.inline_url : attachment.download_url)
     else
-      send_file_headers!( :length=> attachment.s3object.content_length, :filename=>attachment.filename, :disposition => 'inline', :type => attachment.content_type_with_encoding)
-      render :status => 200, :text => attachment.s3object.get.body.read
+      redirect_to(inline ? attachment.inline_url : attachment.download_url)
     end
   end
 

spec/controllers/files_controller_spec.rb

@@ -294,7 +294,7 @@ describe FilesController do
     it "should force download when download_frd is set" do
       user_session(@teacher)
       # this call should happen inside of FilesController#send_attachment
-      FilesController.any_instance.expects(:send_stored_file).with(@file, false, true)
+      FilesController.any_instance.expects(:send_stored_file).with(@file, false)
       get 'show', :course_id => @course.id, :id => @file.id, :download => 1, :verifier => @file.uuid, :download_frd => 1
     end
 

spec/integration/files_spec.rb

@@ -48,8 +48,8 @@ describe FilesController do
       remove_user_session
 
       get location
-      expect(response).to be_success
-      expect(response.content_type).to eq 'image/png'
+      # could be success or redirect, depending on S3 config
+      expect([200, 302]).to be_include(response.status)
       # ensure that the user wasn't logged in by the normal means
       expect(controller.instance_variable_get(:@current_user)).to be_nil
     end
@@ -57,8 +57,8 @@ describe FilesController do
     it "without safefiles" do
       HostUrl.stubs(:file_host_with_shard).returns(['test.host', Shard.default])
       get "http://test.host/files/#{@submission.attachment.id}/download", :inline => '1', :verifier => @submission.attachment.uuid
-      expect(response).to be_success
-      expect(response.content_type).to eq 'image/png'
+      # could be success or redirect, depending on S3 config
+      expect([200, 302]).to be_include(response.status)
       expect(response['Pragma']).to be_nil
       expect(response['Cache-Control']).not_to match(/no-cache/)
     end
@@ -164,8 +164,8 @@ describe FilesController do
     remove_user_session
 
     get location
-    expect(response).to be_success
-    expect(response.content_type).to eq 'image/png'
+    # could be success or redirect, depending on S3 config
+    expect([200, 302]).to be_include(response.status)
     # ensure that the user wasn't logged in by the normal means
     expect(controller.instance_variable_get(:@current_user)).to be_nil
   end
@@ -258,7 +258,8 @@ describe FilesController do
     # and verify the module progress was recorded
     remove_user_session
     get location
-    expect(response).to be_success
+    # could be success or redirect, depending on S3 config
+    expect([200, 302]).to be_include(response.status)
     expect(@module.evaluate_for(@user).state).to eql(:completed)
   end
 

spec/selenium/grades/speedgrader/speedgrader_teacher_submission_spec.rb

@@ -240,9 +240,11 @@ describe "speed grader submissions" do
         # validates the image\attachment is inside the iframe as expected
         image_element = f('img')
 
-        expect(image_element.attribute('src')).to include('download')
-        expect(image_element.attribute('style')).to include('max-height: 100')
-        expect(image_element.attribute('style')).to include('max-width: 100')
+        if Attachment.local_storage?
+          expect(image_element.attribute('src')).to include('download')
+        else
+          expect(image_element.attribute('src')).to include('amazonaws')
+        end
       end
     end