LTI dynamic registration CSP fix
Use iframe in iframe approach to add DR url to CSP whitelist during the registration process. fixes INTEROP-8645 flag=lti_dynamic_registration test plan: - Enable the Content Security Policy javascript_csp feature flag. - Reload the Account Settings page. A new Security tab appears. Click it and enable the CSP header. - Have a tool set up that supports LTI dynamic registration, like YALTT or the 1.3 vercel tool. - Go to Account → Developer Keys → + Dynamic Registration, and enter the tool’s DR Url (without this fix, the tool install page cannot load into the iframe) - Install and click on the 'Enable & Close' Change-Id: I3e64c4f89e8ffb18dd2a514e8d1e975ccb2663fe Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/350827 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: Paul Gray <paul.gray@instructure.com>
This commit is contained in:
parent
b2f9fe2831
commit
4e86ee4781
|
@ -21,7 +21,7 @@
|
||||||
class DeveloperKeysController < ApplicationController
|
class DeveloperKeysController < ApplicationController
|
||||||
before_action :set_key, only: [:update, :destroy]
|
before_action :set_key, only: [:update, :destroy]
|
||||||
before_action :require_manage_developer_keys
|
before_action :require_manage_developer_keys
|
||||||
before_action :require_root_account, only: [:index, :create]
|
before_action :require_root_account, only: %i[index create]
|
||||||
|
|
||||||
include Api::V1::DeveloperKey
|
include Api::V1::DeveloperKey
|
||||||
|
|
||||||
|
|
|
@ -24,7 +24,7 @@
|
||||||
# send messages to the embedding (trusted) tool (usually set in js_env to
|
# send messages to the embedding (trusted) tool (usually set in js_env to
|
||||||
# the value of parent_frame_origin)
|
# the value of parent_frame_origin)
|
||||||
# 2. Add the embedding tool's host to the Content-Security-Policy (CSP)
|
# 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
|
# header's frame-ancestor directive to allow the page to be loaded in an
|
||||||
# iframe embedded in the embedding tool. For this, call
|
# iframe embedded in the embedding tool. For this, call
|
||||||
# set_extra_csp_frame_ancestor!
|
# set_extra_csp_frame_ancestor!
|
||||||
module Lti::Concerns
|
module Lti::Concerns
|
||||||
|
|
|
@ -174,6 +174,29 @@ module Lti
|
||||||
redirect_to account_developer_key_view_url(registration.root_account_id, registration.developer_key_id)
|
redirect_to account_developer_key_view_url(registration.root_account_id, registration.developer_key_id)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def dr_iframe
|
||||||
|
@dr_url = params.require(:url)
|
||||||
|
token = CGI.parse(URI.parse(@dr_url).query)["registration_token"].first
|
||||||
|
jwt = Canvas::Security.decode_jwt(token)
|
||||||
|
|
||||||
|
if jwt["root_account_global_id"] != @context.global_id
|
||||||
|
render status: :unauthorized,
|
||||||
|
json: {
|
||||||
|
errorMessage: "Invalid root_account_id in registration_token"
|
||||||
|
}
|
||||||
|
return
|
||||||
|
end
|
||||||
|
if jwt["user_id"] != @current_user.id
|
||||||
|
render status: :unauthorized,
|
||||||
|
json: {
|
||||||
|
errorMessage: "registration_token was created for a different user"
|
||||||
|
}
|
||||||
|
return
|
||||||
|
end
|
||||||
|
request.env["dynamic_reg_url_csp"] = @dr_url
|
||||||
|
render("lti/ims/dynamic_registration/dr_iframe", layout: false, formats: :html)
|
||||||
|
end
|
||||||
|
|
||||||
private
|
private
|
||||||
|
|
||||||
def render_registration(registration, developer_key)
|
def render_registration(registration, developer_key)
|
||||||
|
|
|
@ -129,6 +129,7 @@ module Csp::AccountHelper
|
||||||
domains += Setting.get("csp.global_whitelist", "").split(",").map(&:strip)
|
domains += Setting.get("csp.global_whitelist", "").split(",").map(&:strip)
|
||||||
domains += cached_tool_domains if include_tools
|
domains += cached_tool_domains if include_tools
|
||||||
domains += csp_files_domains(request) if include_files
|
domains += csp_files_domains(request) if include_files
|
||||||
|
domains += csp_dynamic_registration_domain(request)
|
||||||
domains.compact.uniq.sort
|
domains.compact.uniq.sort
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -201,4 +202,13 @@ module Csp::AccountHelper
|
||||||
def csp_logging_config
|
def csp_logging_config
|
||||||
@config ||= Rails.application.credentials.csp_logging || {}
|
@config ||= Rails.application.credentials.csp_logging || {}
|
||||||
end
|
end
|
||||||
|
|
||||||
|
private
|
||||||
|
|
||||||
|
def csp_dynamic_registration_domain(request)
|
||||||
|
return [] unless request.respond_to?(:env)
|
||||||
|
return [] unless request.env&.[]("dynamic_reg_url_csp")
|
||||||
|
|
||||||
|
[URI.parse(request.env["dynamic_reg_url_csp"]).host]
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -0,0 +1,43 @@
|
||||||
|
<%
|
||||||
|
# Copyright (C) 2024 - present Instructure, Inc.
|
||||||
|
#
|
||||||
|
# This file is part of Canvas.
|
||||||
|
#
|
||||||
|
# Canvas is free software: you can redistribute it and/or modify it under
|
||||||
|
# the terms of the GNU Affero General Public License as published by the Free
|
||||||
|
# Software Foundation, version 3 of the License.
|
||||||
|
#
|
||||||
|
# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
|
||||||
|
# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
|
||||||
|
# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
|
||||||
|
# details.
|
||||||
|
#
|
||||||
|
# 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/>.
|
||||||
|
%>
|
||||||
|
<style>
|
||||||
|
body {
|
||||||
|
margin: 0;
|
||||||
|
padding: 0;
|
||||||
|
}
|
||||||
|
</style>
|
||||||
|
<script>
|
||||||
|
function originOfUrl(urlStr) {
|
||||||
|
const url = new URL(urlStr)
|
||||||
|
return url.origin
|
||||||
|
}
|
||||||
|
|
||||||
|
const onMessage = function(message) {
|
||||||
|
if (
|
||||||
|
message.data.subject === 'org.imsglobal.lti.close' &&
|
||||||
|
message.origin === originOfUrl("<%= @dr_url %>")
|
||||||
|
) {
|
||||||
|
window.removeEventListener('message', onMessage)
|
||||||
|
// Forward close message to parent window, which is the DR wizzard
|
||||||
|
window.parent.postMessage(message.data, '*');
|
||||||
|
}
|
||||||
|
};
|
||||||
|
window.addEventListener('message', onMessage);
|
||||||
|
</script>
|
||||||
|
|
||||||
|
<iframe src="<%= @dr_url %>" style="width: 100%; height: 600px; border: 0; display: block"></iframe>
|
|
@ -2862,6 +2862,7 @@ CanvasRails::Application.routes.draw do
|
||||||
get "accounts/:account_id/registration_token", action: :registration_token
|
get "accounts/:account_id/registration_token", action: :registration_token
|
||||||
get "accounts/:account_id/registrations/uuid/:registration_uuid", action: :registration_by_uuid
|
get "accounts/:account_id/registrations/uuid/:registration_uuid", action: :registration_by_uuid
|
||||||
put "accounts/:account_id/registrations/:registration_id/overlay", action: :update_registration_overlay
|
put "accounts/:account_id/registrations/:registration_id/overlay", action: :update_registration_overlay
|
||||||
|
get "accounts/:account_id/dr_iframe", action: :dr_iframe
|
||||||
get "registrations/:registration_id/view", action: :registration_view, as: :lti_registration_config
|
get "registrations/:registration_id/view", action: :registration_view, as: :lti_registration_config
|
||||||
post "registrations", action: :create, as: :create_lti_registration
|
post "registrations", action: :create, as: :create_lti_registration
|
||||||
end
|
end
|
||||||
|
|
|
@ -23,7 +23,7 @@ describe Lti::IMS::DynamicRegistrationController do
|
||||||
let(:controller_routes) do
|
let(:controller_routes) do
|
||||||
dynamic_registration_routes = []
|
dynamic_registration_routes = []
|
||||||
CanvasRails::Application.routes.routes.each do |route|
|
CanvasRails::Application.routes.routes.each do |route|
|
||||||
dynamic_registration_routes << route if route.defaults[:controller] == "lti/ims/dynamic_registration"
|
dynamic_registration_routes << route if route.defaults[:controller] == "lti/ims/dynamic_registration" && route.defaults[:action] != "dr_iframe"
|
||||||
end
|
end
|
||||||
|
|
||||||
dynamic_registration_routes
|
dynamic_registration_routes
|
||||||
|
@ -314,4 +314,61 @@ describe Lti::IMS::DynamicRegistrationController do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe "#dr_iframe" do
|
||||||
|
before do
|
||||||
|
account_admin_user(account: Account.default)
|
||||||
|
Account.default.root_account.enable_feature! :javascript_csp
|
||||||
|
Account.default.root_account.enable_csp!
|
||||||
|
user_session(@admin)
|
||||||
|
end
|
||||||
|
|
||||||
|
it "must include the url parameter" do
|
||||||
|
get :dr_iframe, params: { account_id: Account.default.id }
|
||||||
|
expect(response).to be_bad_request
|
||||||
|
end
|
||||||
|
|
||||||
|
it "returns unauthorized if jwt is expired" do
|
||||||
|
expired_jwt = Canvas::Security.create_jwt({
|
||||||
|
user_id: @admin.id,
|
||||||
|
root_account_global_id: Account.default.id
|
||||||
|
},
|
||||||
|
5.minutes.ago)
|
||||||
|
get :dr_iframe, params: { account_id: Account.default.id, url: "http://testexample.com?registration_token=#{expired_jwt}" }
|
||||||
|
expect(response).to be_unauthorized
|
||||||
|
end
|
||||||
|
|
||||||
|
it "returns unauthorized if jwt is issued for other account" do
|
||||||
|
expired_jwt = Canvas::Security.create_jwt({
|
||||||
|
user_id: @admin.id,
|
||||||
|
root_account_global_id: 123
|
||||||
|
},
|
||||||
|
5.minutes.from_now)
|
||||||
|
get :dr_iframe, params: { account_id: Account.default.id, url: "http://testexample.com?registration_token=#{expired_jwt}" }
|
||||||
|
expect(response).to be_unauthorized
|
||||||
|
expect(response.headers["Content-Security-Policy"]).not_to include("testexample.com")
|
||||||
|
end
|
||||||
|
|
||||||
|
it "returns unauthorized if jwt is issued for other user" do
|
||||||
|
expired_jwt = Canvas::Security.create_jwt({
|
||||||
|
user_id: 123,
|
||||||
|
root_account_global_id: Account.default.id
|
||||||
|
},
|
||||||
|
5.minutes.from_now)
|
||||||
|
get :dr_iframe, params: { account_id: Account.default.id, url: "http://testexample.com?registration_token=#{expired_jwt}" }
|
||||||
|
expect(response).to be_unauthorized
|
||||||
|
expect(response.headers["Content-Security-Policy"]).not_to include("testexample.com")
|
||||||
|
end
|
||||||
|
|
||||||
|
it "adds url to CSP whitelist if registration_token is valid" do
|
||||||
|
valid_jwt = Canvas::Security.create_jwt({
|
||||||
|
user_id: @admin.id,
|
||||||
|
root_account_global_id: Account.default.global_id
|
||||||
|
},
|
||||||
|
5.minutes.from_now)
|
||||||
|
get :dr_iframe, params: { account_id: Account.default.id, url: "http://testexample.com?registration_token=#{valid_jwt}" }
|
||||||
|
expect(response).to be_successful
|
||||||
|
expect(response.headers["Content-Security-Policy"]).to include("testexample.com")
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -64,7 +64,7 @@ export const DynamicRegistrationModal = (props: DynamicRegistrationModalProps) =
|
||||||
/>
|
/>
|
||||||
<Heading>{I18n.t('Register App')}</Heading>
|
<Heading>{I18n.t('Register App')}</Heading>
|
||||||
</Modal.Header>
|
</Modal.Header>
|
||||||
<DynamicRegistrationModalBody />
|
<DynamicRegistrationModalBody contextId={props.contextId} />
|
||||||
<Modal.Footer>
|
<Modal.Footer>
|
||||||
<DynamicRegistrationModalFooter {...props} />
|
<DynamicRegistrationModalFooter {...props} />
|
||||||
</Modal.Footer>
|
</Modal.Footer>
|
||||||
|
@ -87,12 +87,14 @@ const addParams = (url: string, params: Record<string, string>) => {
|
||||||
Object.entries(params).forEach(([key, value]) => {
|
Object.entries(params).forEach(([key, value]) => {
|
||||||
u.searchParams.set(key, value)
|
u.searchParams.set(key, value)
|
||||||
})
|
})
|
||||||
return u.toString()
|
return encodeURIComponent(u.toString())
|
||||||
}
|
}
|
||||||
|
|
||||||
type DynamicRegistrationModalBodyProps = {}
|
type DynamicRegistrationModalBodyProps = {
|
||||||
|
contextId: string
|
||||||
|
}
|
||||||
|
|
||||||
const DynamicRegistrationModalBody = (_props: DynamicRegistrationModalBodyProps) => {
|
const DynamicRegistrationModalBody = ({contextId}: DynamicRegistrationModalBodyProps) => {
|
||||||
const state = useDynamicRegistrationState(s => s.state)
|
const state = useDynamicRegistrationState(s => s.state)
|
||||||
const setUrl = useDynamicRegistrationState(s => s.setUrl)
|
const setUrl = useDynamicRegistrationState(s => s.setUrl)
|
||||||
switch (state.tag) {
|
switch (state.tag) {
|
||||||
|
@ -115,15 +117,20 @@ const DynamicRegistrationModalBody = (_props: DynamicRegistrationModalBodyProps)
|
||||||
)
|
)
|
||||||
case 'registering':
|
case 'registering':
|
||||||
return (
|
return (
|
||||||
|
<Modal.Body padding="none">
|
||||||
<iframe
|
<iframe
|
||||||
src={addParams(state.dynamicRegistrationUrl, {
|
src={
|
||||||
|
`/api/lti/accounts/${contextId}/dr_iframe?url=` +
|
||||||
|
addParams(state.dynamicRegistrationUrl, {
|
||||||
openid_configuration: state.registrationToken.oidc_configuration_url,
|
openid_configuration: state.registrationToken.oidc_configuration_url,
|
||||||
registration_token: state.registrationToken.token,
|
registration_token: state.registrationToken.token,
|
||||||
})}
|
})
|
||||||
|
}
|
||||||
style={{width: '100%', height: '600px', border: '0', display: 'block'}}
|
style={{width: '100%', height: '600px', border: '0', display: 'block'}}
|
||||||
title={I18n.t('Register App')}
|
title={I18n.t('Register App')}
|
||||||
data-testid="dynamic-reg-modal-iframe"
|
data-testid="dynamic-reg-modal-iframe"
|
||||||
/>
|
/>
|
||||||
|
</Modal.Body>
|
||||||
)
|
)
|
||||||
case 'loading_registration':
|
case 'loading_registration':
|
||||||
return (
|
return (
|
||||||
|
|
|
@ -267,7 +267,8 @@ export const useDynamicRegistrationState = create<
|
||||||
const onMessage = (message: MessageEvent) => {
|
const onMessage = (message: MessageEvent) => {
|
||||||
if (
|
if (
|
||||||
message.data.subject === 'org.imsglobal.lti.close' &&
|
message.data.subject === 'org.imsglobal.lti.close' &&
|
||||||
message.origin === originOfUrl(state.dynamicRegistrationUrl)
|
// Message is coming from an iframe in an iframe to handle CSP restrictions. (dr_iframe / lti tool iframe)
|
||||||
|
message.origin === window.location.origin
|
||||||
) {
|
) {
|
||||||
window.removeEventListener('message', onMessage)
|
window.removeEventListener('message', onMessage)
|
||||||
loadingRegistration(registrationToken, state.dynamicRegistrationUrl)
|
loadingRegistration(registrationToken, state.dynamicRegistrationUrl)
|
||||||
|
|
|
@ -74,7 +74,7 @@ describe('DynamicRegistrationModal', () => {
|
||||||
expect(iframe).toBeInTheDocument()
|
expect(iframe).toBeInTheDocument()
|
||||||
expect(iframe).toHaveAttribute(
|
expect(iframe).toHaveAttribute(
|
||||||
'src',
|
'src',
|
||||||
'http://localhost/?foo=bar&openid_configuration=http%3A%2F%2Fcanvas.instructure.com®istration_token=abc'
|
'/api/lti/accounts/1/dr_iframe?url=http%3A%2F%2Flocalhost%2F%3Ffoo%3Dbar%26openid_configuration%3Dhttp%253A%252F%252Fcanvas.instructure.com%26registration_token%3Dabc'
|
||||||
)
|
)
|
||||||
})
|
})
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue