Proxy iframe content through media_objects_controller
This changes the returned URL used in the content iframes to proxy through canvas and not return the underlying file URL. This allows us to better wrap permissions around file access. fixes LF-310 flag=authenticated_iframe_content test plan: - With the flag off - Have a course with some RCE video content - Verify the iframe is using a direct notirious file link - Verify that going directly to that link renders the file directly - Turn the flag on - Refresh the page with RCE content - Verify the iframe is using a canvas url - Verify the content is still visible and working as intended - Verify that going directly to the canvas link will download the file and that the underlying file URL is not exposed Change-Id: I76010e32a68fdcfcaebe925f690fe373abe22157 Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/321280 QA-Review: Luis Oliveira <luis.oliveira@instructure.com> Reviewed-by: Luis Oliveira <luis.oliveira@instructure.com> Product-Review: Eric Saupe <eric.saupe@instructure.com> Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
This commit is contained in:
Eric Saupe
parent
4f0daaae90
commit
7c55d04c94
|
|
@ -241,7 +241,20 @@ class MediaObjectsController < ApplicationController
|
|||
@media_object&.viewed!
|
||||
config = CanvasKaltura::ClientV3.config
|
||||
if config
|
||||
if Account.site_admin.feature_enabled?(:authenticated_iframe_content)
|
||||
media_sources = CanvasKaltura::ClientV3.new.media_sources(@attachment.media_entry_id)
|
||||
# the bitrate query gives us a unique-enough key to use when querying for specific versions of the file
|
||||
media_source = media_sources.find { |ms| ms[:bitrate].to_i == params[:bitrate].to_i } if params[:bitrate]
|
||||
media_source ||= media_sources.first # if the bitrate is invalid or doesn't find anything
|
||||
begin
|
||||
temp_file = media_source_temp_file(media_source[:url])
|
||||
rescue CanvasHttp::InvalidResponseCodeError => e
|
||||
return(render(plain: e.message, status: e.code))
|
||||
end
|
||||
send_file(temp_file, filename: @attachment.filename, type: @attachment.content_type, stream: true)
|
||||
else
|
||||
redirect_to CanvasKaltura::ClientV3.new.assetSwfUrl(params[:id])
|
||||
end
|
||||
else
|
||||
render plain: t(:media_objects_not_configured, "Media Objects not configured")
|
||||
end
|
||||
|
|
@ -299,4 +312,21 @@ class MediaObjectsController < ApplicationController
|
|||
|
||||
@media_object.viewed!
|
||||
end
|
||||
|
||||
# Fetches the media source file from the given url and returns a tempfile
|
||||
# This logic is similar to the logic in Attachment#clone_url_as_attachment to handle large files except
|
||||
# that it doesn't store it in the database
|
||||
def media_source_temp_file(url)
|
||||
_, uri = CanvasHttp.validate_url(url, check_host: true)
|
||||
tmpfile = CanvasHttp.tempfile_for_uri(uri)
|
||||
CanvasHttp.get(url) do |http_response|
|
||||
if http_response.code.to_i == 200
|
||||
http_response.read_body(tmpfile)
|
||||
tmpfile.rewind
|
||||
else
|
||||
raise CanvasHttp::InvalidResponseCodeError.new(http_response.code.to_i, "error fetching #{url}")
|
||||
end
|
||||
end
|
||||
tmpfile
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -30,6 +30,10 @@ module FilesHelper
|
|||
elsif params[:media_object_id].present?
|
||||
@media_id = params[:media_object_id]
|
||||
@media_object = MediaObject.by_media_id(@media_id).take
|
||||
elsif Account.site_admin.feature_enabled?(:authenticated_iframe_content) && params[:id].present?
|
||||
@media_id = params[:id]
|
||||
@media_object = MediaObject.find_by(id: params[:id])
|
||||
@attachment = @media_object&.attachment
|
||||
end
|
||||
end
|
||||
|
||||
|
|
|
|||
|
|
@ -59,6 +59,9 @@ module Api::V1::MediaObject
|
|||
|
||||
def media_sources_json(media_object)
|
||||
media_object.media_sources&.map do |mo|
|
||||
if Account.site_admin.feature_enabled?(:authenticated_iframe_content)
|
||||
mo[:url] = media_object_redirect_url(media_object.id, bitrate: mo[:bitrate])
|
||||
end
|
||||
mo[:src] = mo[:url]
|
||||
mo[:label] = "#{(mo[:bitrate].to_i / 1024).floor} kbps"
|
||||
mo
|
||||
|
|
|
|||
|
|
@ -1108,4 +1108,96 @@ describe MediaObjectsController do
|
|||
)
|
||||
end
|
||||
end
|
||||
|
||||
describe "#media_sources_json" do
|
||||
before do
|
||||
@media_object = @course.media_objects.create! media_id: "0_deadbeef", user_entered_title: "blah.flv"
|
||||
allow_any_instance_of(MediaObject).to receive(:media_sources).and_return(
|
||||
[{ url: "whatever man", bitrate: 12_345 }]
|
||||
)
|
||||
end
|
||||
|
||||
it "returns the media object url as the source" do
|
||||
expect(controller.media_sources_json(@media_object)).to eq(
|
||||
[
|
||||
{
|
||||
bitrate: 12_345,
|
||||
label: "12 kbps",
|
||||
src: "whatever man",
|
||||
url: "whatever man"
|
||||
}
|
||||
]
|
||||
)
|
||||
end
|
||||
|
||||
context "with authenticated_iframe_content feature flag enabled" do
|
||||
before do
|
||||
Account.site_admin.enable_feature!(:authenticated_iframe_content)
|
||||
end
|
||||
|
||||
it "returns the redirect url as the source" do
|
||||
expect(controller.media_sources_json(@media_object)).to eq(
|
||||
[
|
||||
{
|
||||
bitrate: 12_345,
|
||||
label: "12 kbps",
|
||||
src: "http://test.host/media_objects/#{@media_object.id}/redirect?bitrate=12345",
|
||||
url: "http://test.host/media_objects/#{@media_object.id}/redirect?bitrate=12345"
|
||||
}
|
||||
]
|
||||
)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe "GET '/media_objects/:id/redirect'" do
|
||||
before do
|
||||
allow(CanvasKaltura::ClientV3).to receive(:config).and_return({})
|
||||
allow_any_instance_of(CanvasKaltura::ClientV3).to receive(:assetSwfUrl).and_return(
|
||||
"http://test.host/media_redirect"
|
||||
)
|
||||
@media_object = @course.media_objects.create! media_id: "0_deadbeef", user_entered_title: "blah.flv"
|
||||
user_session(@teacher)
|
||||
end
|
||||
|
||||
context "with authenticated_iframe_content feature flag enabled" do
|
||||
before do
|
||||
Account.site_admin.enable_feature!(:authenticated_iframe_content)
|
||||
allow_any_instance_of(CanvasKaltura::ClientV3).to receive(:media_sources).and_return(
|
||||
[
|
||||
{ bitrate: 1, url: "http://test.host/media_redirect" },
|
||||
{ bitrate: 2, url: "http://test.host/media_redirect_2" }
|
||||
]
|
||||
)
|
||||
@attachment = @media_object.attachment
|
||||
end
|
||||
|
||||
it "returns the file" do
|
||||
temp_file = Tempfile.new("foo")
|
||||
expect(controller).to receive(:media_source_temp_file).with("http://test.host/media_redirect").and_return(temp_file)
|
||||
expect(controller).to receive(:send_file).with(temp_file, filename: @attachment.filename, type: @attachment.content_type, stream: true).and_call_original
|
||||
get :media_object_redirect, params: { id: @media_object.id }
|
||||
end
|
||||
|
||||
it "returns the file by bitrate" do
|
||||
temp_file = Tempfile.new("foo")
|
||||
expect(controller).to receive(:media_source_temp_file).with("http://test.host/media_redirect_2").and_return(temp_file)
|
||||
expect(controller).to receive(:send_file).with(temp_file, filename: @attachment.filename, type: @attachment.content_type, stream: true).and_call_original
|
||||
get :media_object_redirect, params: { id: @media_object.id, bitrate: 2 }
|
||||
end
|
||||
|
||||
it "returns the first file if the bitrate is invalid" do
|
||||
temp_file = Tempfile.new("foo")
|
||||
expect(controller).to receive(:media_source_temp_file).with("http://test.host/media_redirect").and_return(temp_file)
|
||||
expect(controller).to receive(:send_file).with(temp_file, filename: @attachment.filename, type: @attachment.content_type, stream: true).and_call_original
|
||||
get :media_object_redirect, params: { id: @media_object.id, bitrate: "not real" }
|
||||
end
|
||||
|
||||
it "renders an error if there was a problem fetching the file" do
|
||||
allow(controller).to receive(:media_source_temp_file).and_raise(CanvasHttp::InvalidResponseCodeError.new(400, "error fetching url"))
|
||||
get :media_object_redirect, params: { id: @media_object.id }
|
||||
assert_status(400)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
|||
Loading…
Reference in New Issue