ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

amend CSP header to allow LTIs embedded in LTIs 

This lets the retrieve and resource_selection endpoints embed inside
an iframe inside of New Quizzes.

closes INTEROP-7752
closes INTEROP-7742

Test plan:
- if you have New Quizzes set up, and they are embedding tools from
  within their tool, you might be able to test with that. Otherwise, you
  can test with LTI 1.3 test tool as mentioned below.
- modify the LTI 1.3 test tool dev key to have internal_service=true
- in the LTI 1.3 test tool's app/views/launch/launch.html.erb add an
  iframe pointing to resource_selection like this:
    <iframe
      src="http://web.canvas-lms.docker/courses/66/external_tools/181/resource_selection?parent_frame_context=155&editor=1"
      style="width: 600px; height: 400px" ></iframe>
  The parent_frame_context parameter should be your LTI 1.3 test tool
  ID. The first tool ID can be any LTI 1.1 tool (LTI 1.3 currently needs
  an additional change -- to the LTI 1.3 'authorize' endpoint -- to
  work)
- Launch the LTI 1.3 tool. Your LTI 1.1 tool should successfully launch
- Change the iframe you added above to a retrieve URL, e.g.:
    http://web.canvas-lms.docker/courses/66/external_tools/retrieve?url=http%3A%2F%2Fmylti1tool.example.com%2F&parent_frame_context=155"
- Where the URL is a URL for an LTI 1.1 tool available in the course,
  and 155 is your LTI 1.3 test tool ID

Change-Id: Ie27c030bbd95b85af60e391c14bf6c52aad087be
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/306023
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Paul Gray <paul.gray@instructure.com>
QA-Review: Paul Gray <paul.gray@instructure.com>
Product-Review: Evan Battaglia <ebattaglia@instructure.com>
This commit is contained in:
Evan Battaglia 2022-11-23 17:01:19 -07:00
parent e06724f8d1
commit f58237d341
7 changed files with 153 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
app/controllers/application_controller.rb
View File

@ -799,20 +799,19 @@ class ApplicationController < ActionController::Base
require_user
end
def csp_frame_ancestors
# Allow iframing on all vanity domains as well as the canonical one
unless @domain_root_account.nil?
HostUrl.context_hosts(@domain_root_account, request.host)
end
end
def set_response_headers
# we can't block frames on the files domain, since files domain requests
# are typically embedded in an iframe in canvas, but the hostname is
# different
if !files_domain? && Setting.get("block_html_frames", "true") == "true" && !@embeddable
#
# Allow iframing on all vanity domains as well as the canonical one
#
equivalent_domains = []
unless @domain_root_account.nil?
equivalent_domains = HostUrl.context_hosts(@domain_root_account, request.host)
end
append_to_header("Content-Security-Policy", "frame-ancestors 'self' #{equivalent_domains.join(" ")};")
append_to_header("Content-Security-Policy", "frame-ancestors 'self' #{csp_frame_ancestors&.join(" ")};")
end
headers["Strict-Transport-Security"] = "max-age=31536000" if request.ssl?
RequestContext::Generator.store_request_meta(request, @context, @sentry_trace)

7
app/controllers/external_content_controller.rb
View File

@ -88,11 +88,8 @@ class ExternalContentController < ApplicationController
error_message: param_if_set(:lti_errormsg),
error_log: param_if_set(:lti_errorlog)
})
tool_origin = parent_frame_origin(params[:parent_frame_context])
if tool_origin
js_env({
DEEP_LINKING_POST_MESSAGE_ORIGIN: tool_origin
}, true)
if parent_frame_origin
js_env({ DEEP_LINKING_POST_MESSAGE_ORIGIN: parent_frame_origin }, true)
end
end

2
app/controllers/external_tools_controller.rb
View File

@ -33,11 +33,13 @@ class ExternalToolsController < ApplicationController
before_action :require_user, only: [:generate_sessionless_launch]
before_action :get_context, only: %i[retrieve show resource_selection]
before_action :parse_context_codes, only: [:all_visible_nav_tools]
before_action :set_extra_csp_frame_ancestor!, only: %i[retrieve resource_selection]
skip_before_action :verify_authenticity_token, only: :resource_selection
include Api::V1::ExternalTools
include Lti::RedisMessageClient
include Lti::Concerns::SessionlessLaunches
include Lti::Concerns::ParentFrame
include K5Mode
WHITELISTED_QUERY_PARAMS = [

56
app/controllers/lti/concerns/parent_frame.rb
View File

@ -17,21 +17,44 @@
# You should have received a copy of the GNU Affero General Public License along
# with this program. If not, see <http://www.gnu.org/licenses/>.
# Methods for controller actions which take a parent_frame_context param to
# facilitate embedding inside trusted tools (e.g. New Quizzes). Depending on
# the action then may need to one or both of these things:
# 1. Set the origin to be used with postMessage to allow the embedded tool to
# send messages to the embedding (trusted) tool (usually set in js_env to
# the value of parent_frame_origin)
# 2. Add the embedding tool's host to the Content-Security-Policy (CSP)
# header's frame-ancestor directive to allow the page to me loaded in an
# iframe embedded in the embedding tool. For this, call
# set_extra_csp_frame_ancestor!
module Lti::Concerns
module ParentFrame
extend ActiveSupport::Concern
# Takes an id of a tool and returns the origin of that tool if it exists
# otherwise returns nil
def parent_frame_origin(tool_id)
tool = tool_id ? ContextExternalTool.find_by(id: tool_id) : nil
return nil unless tool&.active? && tool&.developer_key&.internal_service
private
if tool.url
override_parent_frame_origin(tool.url)
elsif tool.domain
"https://#{tool.domain}"
end
# Can be overridden by controller if the parent_frame_context (tool ID)
# comes from someplace else (as it does for DeepLinkingController)
def parent_frame_context
params[:parent_frame_context]
end
# Finds the tool with id parent_frame_context and returns the origin
# (host/scheme/port based off of URL/domain) of that tool if it exists.
# otherwise returns nil. Memoized.
def parent_frame_origin
return @parent_frame_origin if defined?(@parent_frame_origin)
tool = parent_frame_context.presence && ContextExternalTool.find_by(id: parent_frame_context)
@parent_frame_origin =
if !tool&.active? || !tool&.developer_key&.internal_service
nil
elsif tool.url
override_parent_frame_origin(tool.url)
elsif tool.domain
"https://#{tool.domain}"
end
end
def override_parent_frame_origin(url)
@ -39,5 +62,18 @@ module Lti::Concerns
origin = URI("#{uri.scheme}://#{uri.host}:#{uri.port}")
origin.to_s
end
# Overrides the method in ApplicationController, and is used there
def csp_frame_ancestors
[*super, @extra_csp_frame_ancestor].compact
end
def set_extra_csp_frame_ancestor!
# require http/https URI (e.g., no 'data' uris') & don't allow potential
# specially characters that could mess up header
if parent_frame_origin&.match(/^https?:[^ *;]+$/)
@extra_csp_frame_ancestor = parent_frame_origin
end
end
end
end

14
app/controllers/lti/ims/deep_linking_controller.rb
View File

@ -134,6 +134,11 @@ module Lti
raise e
end
# Overrides method in Lti::Concerns::ParentFrame
def parent_frame_context
return_url_parameters[:parent_frame_context]
end
private
def for_placement?(placement)
@ -151,14 +156,11 @@ module Lti
errorlog: messaging_value("errorlog"),
ltiEndpoint: polymorphic_url([:retrieve, @context, :external_tools]),
reloadpage: reload_page,
moduleCreated: module_created
moduleCreated: module_created,
}.compact
})
tool_origin = parent_frame_origin(return_url_parameters[:parent_frame_context])
if tool_origin
js_env({
DEEP_LINKING_POST_MESSAGE_ORIGIN: tool_origin
}, true)
if parent_frame_origin
js_env({ DEEP_LINKING_POST_MESSAGE_ORIGIN: parent_frame_origin }, true)
end
render layout: "bare"

81
spec/controllers/external_tools_controller_spec.rb
View File

@ -697,6 +697,68 @@ describe ExternalToolsController do
end
end
shared_examples_for "an endpoint which uses parent_frame_context to set the CSP header" do
before do
user_session(@student)
end
let(:pfc_tool) do
@course.context_external_tools.create(
name: "instructure_tool", consumer_key: "foo", shared_secret: "bar",
url: "http://inst-tool.example.com/abc", lti_version: "1.1",
developer_key: DeveloperKey.new(
root_account: @course.root_account,
internal_service: true
)
)
end
let(:csp_header) { response.headers["Content-Security-Policy"] }
it "adds the tool URL to the header if the parent_frame_context tool is trusted" do
subject
expect(response).to be_successful
expect(csp_header).to match %r{frame-ancestors [^;]*http://inst-tool.example.com(;| |$)}
# Make sure it also has 'self', which is added in application_controller.rb:
expect(csp_header).to match(/frame-ancestors [^;]*'self'/)
end
it "doesn't add the URL to the header if the parent_frame_context tool is not trusted" do
pfc_tool.developer_key.update! internal_service: false
subject
expect(response).to be_successful
expect(csp_header).to_not include("inst-tool.example.com")
end
it "doesn't add the URL to the header if the parent_frame_context tool is not active" do
pfc_tool.update! workflow_state: :deleted
subject
expect(response).to be_successful
expect(csp_header).to_not include("inst-tool.example.com")
end
it "doesn't add the URL to the header if the parent_frame_context tool's URL has unsafe characters" do
pfc_tool.update_attribute :url, "http://inst-tool.example.com;default-src"
subject
expect(response).to be_successful
expect(csp_header).to_not include("inst-tool.example.com")
end
it "doesn't add the parent_frame_context tool's URL to the header if it is a data URL" do
pfc_tool.update! url: "data:123"
subject
expect(response).to be_successful
expect(csp_header).to_not include("data:abc")
end
it "doesn't add the parent_frame_context tool's URL to the header if the user is not authenticated" do
user_session(user_model)
subject
expect(response.status).to eq(401)
expect(csp_header.to_s).to_not include("inst-tool.example.com")
end
end
describe "GET 'retrieve'" do
let :account do
Account.default
@ -1019,6 +1081,15 @@ describe ExternalToolsController do
expect(assigns[:lti_launch].params["ext_lti_assignment_id"]).to eq lti_assignment_id
end
it_behaves_like "an endpoint which uses parent_frame_context to set the CSP header" do
subject do
get :retrieve, params: {
url: tool.url, course_id: @course.id,
parent_frame_context: pfc_tool.id
}
end
end
context "for Quizzes Next launch" do
let(:assignment) do
a = assignment_model(course: @course)
@ -1331,6 +1402,16 @@ describe ExternalToolsController do
expect(assigns[:lti_launch].params["custom_canvas_enrollment_state"]).to eq "inactive"
end
it_behaves_like "an endpoint which uses parent_frame_context to set the CSP header" do
subject do
tool = new_valid_tool(@course)
get "resource_selection", params: {
course_id: @course.id, external_tool_id: tool.id,
parent_frame_context: pfc_tool.id
}
end
end
context "with RCE parameters" do
subject do
user_session(@student)

14
spec/controllers/lti/ims/deep_linking_controller_spec.rb
View File

@ -72,14 +72,14 @@ module Lti
subject
end
# TODO: update all these with hash_including()
context "when returning from a non-internal service" do
let(:return_url_params) { { placement: placement, parent_frame_context: context_external_tool.id } }
it "does not set the DEEP_LINKING_POST_MESSAGE_ORIGIN value in jsenv" do
expect(controller).to receive(:js_env).with(anything)
expect(controller).not_to receive(:js_env).with({ DEEP_LINKING_POST_MESSAGE_ORIGIN: "http://tool.url" }, true)
it "does not change the DEEP_LINKING_POST_MESSAGE_ORIGIN value in jsenv" do
subject
# base_url is the default
expect(assigns(:js_env)[:DEEP_LINKING_POST_MESSAGE_ORIGIN]).to eq(@controller.request.base_url)
end
end
@ -90,11 +90,9 @@ module Lti
let(:return_url_params) { { placement: placement, parent_frame_context: context_external_tool.id } }
it "sets the DEEP_LINKING_POST_MESSAGE_ORIGIN value in jsenv" do
expect(controller).to receive(:js_env).with(anything)
expect(controller).to receive(:js_env).with({ DEEP_LINKING_POST_MESSAGE_ORIGIN: "http://tool.url" }, true)
it "sets the DEEP_LINKING_POST_MESSAGE_ORIGIN value in js_env" do
subject
expect(assigns(:js_env)[:DEEP_LINKING_POST_MESSAGE_ORIGIN]).to eq("http://tool.url")
end
end