Skip to content

Commit fdfdc9f

Browse files
committed
default to launch url for content-items missing url
refs PLAT-1571 test plan: return a content item LTI Link that is missing a URL it should use the URL for the content item launch Change-Id: I8bf991202b89b2e64a190046c664cca52d5bccb0 Reviewed-on: https://gerrit.instructure.com/82449 Tested-by: Jenkins Reviewed-by: Andrew Butterfield <abutterfield@instructure.com> QA-Review: August Thornton <august@instructure.com> Product-Review: Nathan Mills <nathanm@instructure.com>
1 parent 5e083a1 commit fdfdc9f

4 files changed

Lines changed: 47 additions & 9 deletions

File tree

app/controllers/external_content_controller.rb

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,6 @@ def success
5656
if params[:id]
5757
message_auth = Lti::MessageAuthenticator.new(request.original_url, request.GET.merge(request.POST))
5858
render_unauthorized_action and return unless message_auth.valid?
59-
json_data = (params[:data] && Canvas::Security.decode_jwt(params[:data])) || {}
6059
render_unauthorized_action and return unless json_data[:content_item_id] == params[:id]
6160
render_unauthorized_action and return unless json_data[:oauth_consumer_key] == params[:oauth_consumer_key]
6261
end
@@ -112,7 +111,8 @@ def content_items_for_canvas
112111
content_item_selection.map do |item|
113112
item.placement_advice ||= default_placement_advice
114113
if item.type == IMS::LTI::Models::ContentItems::LtiLinkItem::TYPE
115-
url_gen_params = {url: item.url}
114+
launch_url = item.url || json_data[:default_launch_url]
115+
url_gen_params = {url: launch_url}
116116
url_gen_params[:display] = 'borderless' if item.placement_advice.presentation_document_target == 'iframe'
117117
item.canvas_url = named_context_url(@context, :retrieve_context_external_tools_path, url_gen_params)
118118
end
@@ -164,4 +164,8 @@ def default_placement_advice
164164
)
165165
end
166166

167+
def json_data
168+
@json_data ||= ((params[:data] && Canvas::Security.decode_jwt(params[:data])) || {}).with_indifferent_access
169+
end
170+
167171
end

app/controllers/external_tools_controller.rb

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -488,18 +488,21 @@ def content_item_selection_request(tool, placement, opts = {})
488488
accept_unsigned= true
489489
auto_create= false
490490
return_url_opts = {service: 'external_tool_dialog'}
491+
launch_url = opts[:launch_url] || tool.extension_setting(placement, :url)
492+
data_hash = {default_launch_url: launch_url}
491493
if opts[:content_item_id]
492-
extra_params[:data] = Canvas::Security.create_jwt(
493-
{
494-
content_item_id: opts[:content_item_id],
495-
oauth_consumer_key: tool.consumer_key
496-
}
497-
)
494+
data_hash.merge!(
495+
{
496+
content_item_id: opts[:content_item_id],
497+
oauth_consumer_key: tool.consumer_key
498+
}
499+
)
498500
return_url_opts[:id] = opts[:content_item_id]
499501
return_url = polymorphic_url([@context, :external_content_update], return_url_opts)
500502
else
501503
return_url = polymorphic_url([@context, :external_content_success], return_url_opts)
502504
end
505+
extra_params[:data] = Canvas::Security.create_jwt(data_hash)
503506
# choose accepted return types based on placement
504507
# todo, make return types configurable at installation?
505508
case placement
@@ -549,7 +552,7 @@ def content_item_selection_request(tool, placement, opts = {})
549552
}).merge(extra_params).merge(variable_expander(tool:tool).expand_variables!(tool.set_custom_fields(placement)))
550553

551554
lti_launch = @tool.settings['post_only'] ? Lti::Launch.new(post_only: true) : Lti::Launch.new
552-
lti_launch.resource_url = opts[:launch_url]|| tool.extension_setting(placement, :url)
555+
lti_launch.resource_url = launch_url
553556
lti_launch.params = Lti::Security.signed_post_params(
554557
params,
555558
lti_launch.resource_url,

spec/controllers/external_content_controller_spec.rb

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,25 @@
166166
expect(data.first.placement_advice.display_height).to eq(600)
167167
expect(data.first.placement_advice.display_width).to eq(800)
168168
end
169+
170+
it "uses the default url if one isn't provided" do
171+
c = course
172+
json = JSON.parse(File.read(File.join(Rails.root, 'spec', 'fixtures', 'lti', 'content_items_2.json')))
173+
json['@graph'][0].delete('url')
174+
launch_url = 'http://example.com/launch'
175+
post(:success, service: 'external_tool_dialog', course_id: c.id, lti_message_type: 'ContentItemSelection',
176+
lti_version: 'LTI-1p0',
177+
data: Canvas::Security.create_jwt({default_launch_url: launch_url}),
178+
content_items: json.to_json,
179+
lti_msg: '',
180+
lti_log: '',
181+
lti_errormsg: '',
182+
lti_errorlog: '')
183+
184+
data = controller.js_env[:retrieved_data]
185+
expect(data.first.canvas_url).to include "http%3A%2F%2Fexample.com%2Flaunch"
186+
end
187+
169188
end
170189

171190
end

spec/controllers/external_tools_controller_spec.rb

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -705,6 +705,18 @@ def new_valid_tool(course)
705705
expect(json_data[:oauth_consumer_key]).to eq tool.consumer_key
706706
end
707707

708+
it 'adds to the data element the default launch url' do
709+
u = user(active_all: true)
710+
account.account_users.create!(user:u)
711+
user_session u
712+
tool.collaboration = { message_type: 'ContentItemSelectionRequest' }
713+
tool.save!
714+
get :retrieve, {url: tool.url, course_id: @course.id, placement: 'collaboration', content_item_id:3 }
715+
data = assigns[:lti_launch].params['data']
716+
json_data = Canvas::Security.decode_jwt(data)
717+
expect(json_data[:default_launch_url]).to eq tool.url
718+
end
719+
708720
end
709721

710722
describe "GET 'resource_selection'" do

0 commit comments

Comments
 (0)