Skip to content

Commit 803aa4b

Browse files
westonkdwestonkd
committed
Re-add copy GET to POST on LTI launch
fixes PLAT-1010 test plan: - make sure any GET params get copied to POST on LTI launch Change-Id: I432dfd795999f0869e22fc22d6bf77906b8d6b5b Reviewed-on: https://gerrit.instructure.com/53545 Tested-by: Jenkins Reviewed-by: Nathan Mills <nathanm@instructure.com> QA-Review: August Thornton <august@instructure.com> Product-Review: Weston Dransfield <wdransfield@instructure.com>
1 parent 9db5197 commit 803aa4b

3 files changed

Lines changed: 56 additions & 54 deletions

File tree

‎gems/lti_outbound/lib/lti_outbound/tool_launch.rb‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -171,12 +171,19 @@ def self.generate_params(params, url, key, secret)
171171
end
172172

173173
consumer = OAuth::Consumer.new(key, secret, {
174-
:site => "#{uri.scheme}://#{host}",
175-
:signature_method => 'HMAC-SHA1'
176-
})
174+
:site => "#{uri.scheme}://#{host}",
175+
:signature_method => 'HMAC-SHA1'
176+
})
177177

178178
path = uri.path
179179
path = '/' if path.empty?
180+
if uri.query && uri.query != ''
181+
CGI.parse(uri.query).each do |query_key, query_values|
182+
unless params[query_key]
183+
params[query_key] = query_values.first
184+
end
185+
end
186+
end
180187
options = {
181188
:scheme => 'body',
182189
:timestamp => @timestamp,

‎gems/lti_outbound/spec/lti_outbound/tool_launch_spec.rb‎

Lines changed: 39 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,7 @@
230230
expect(hash['tool_consumer_instance_guid']).to eq 'root_account_lti_guid' #was hash['tool_consumer_instance_guid']).to eq sub_account.root_account.lti_guid
231231
end
232232

233-
it 'does not include URI query parameters' do
233+
it 'includes URI query parameters' do
234234
hash = LtiOutbound::ToolLaunch.new(:url => 'http://www.yahoo.com?paramater_a=value_a&parameter_b=value_b',
235235
:tool => tool,
236236
:user => user,
@@ -239,8 +239,8 @@
239239
:link_code => '123456',
240240
:return_url => 'http://www.google.com',
241241
:variable_expander => variable_expander).generate
242-
expect(hash.key?('paramater_a')).to be false
243-
expect(hash.key?('parameter_b')).to be false
242+
expect(hash['paramater_a']).to eq 'value_a'
243+
expect(hash['parameter_b']).to eq 'value_b'
244244
end
245245

246246
it 'does not allow overwriting other parameters from the URI query string' do
@@ -259,13 +259,13 @@
259259
it 'includes custom fields' do
260260
tool.privacy_level = LtiOutbound::LTITool::PRIVACY_LEVEL_ANONYMOUS
261261
tool.settings = {:custom_fields => {
262-
'custom_bob' => 'bob',
263-
'custom_fred' => 'fred',
264-
'john' => 'john',
265-
'@$TAA$#$#' => 123}}
262+
'custom_bob' => 'bob',
263+
'custom_fred' => 'fred',
264+
'john' => 'john',
265+
'@$TAA$#$#' => 123}}
266266
hash = tool_launch.generate
267267
expect(hash.keys.select { |k| k.match(/^custom_/) }.sort).to eq(
268-
['custom___taa____', 'custom_bob', 'custom_canvas_enrollment_state', 'custom_fred', 'custom_john'])
268+
['custom___taa____', 'custom_bob', 'custom_canvas_enrollment_state', 'custom_fred', 'custom_john'])
269269
expect(hash['custom_bob']).to eql('bob')
270270
expect(hash['custom_fred']).to eql('fred')
271271
expect(hash['custom_john']).to eql('john')
@@ -390,20 +390,20 @@ def explicit_signature_settings(timestamp, nonce)
390390
explicit_signature_settings('1251600739', 'c8350c0e47782d16d2fa48b2090c1d8f')
391391

392392
hash = LtiOutbound::ToolLaunch.send(:generate_params, {
393-
:resource_link_id => '120988f929-274612',
394-
:user_id => '292832126',
395-
:roles => 'Instructor',
396-
:lis_person_name_full => 'Jane Q. Public',
397-
:lis_person_contact_email_primary => 'user@school.edu',
398-
:lis_person_sourced_id => 'school.edu:user',
399-
:context_id => '456434513',
400-
:context_title => 'Design of Personal Environments',
401-
:context_label => 'SI182',
402-
:lti_version => 'LTI-1p0',
403-
:lti_message_type => 'basic-lti-launch-request',
404-
:tool_consumer_instance_guid => 'lmsng.school.edu',
405-
:tool_consumer_instance_description => 'University of School (LMSng)',
406-
:lti_submit => 'Launch Endpoint with LTI Data'
393+
:resource_link_id => '120988f929-274612',
394+
:user_id => '292832126',
395+
:roles => 'Instructor',
396+
:lis_person_name_full => 'Jane Q. Public',
397+
:lis_person_contact_email_primary => 'user@school.edu',
398+
:lis_person_sourced_id => 'school.edu:user',
399+
:context_id => '456434513',
400+
:context_title => 'Design of Personal Environments',
401+
:context_label => 'SI182',
402+
:lti_version => 'LTI-1p0',
403+
:lti_message_type => 'basic-lti-launch-request',
404+
:tool_consumer_instance_guid => 'lmsng.school.edu',
405+
:tool_consumer_instance_description => 'University of School (LMSng)',
406+
:lti_submit => 'Launch Endpoint with LTI Data'
407407
}, 'http://dr-chuck.com/ims/php-simple/tool.php', '12345', 'secret')
408408

409409
expect(hash['oauth_signature']).to eql('l1ZTsn1HjGXzqeaTQMPbjrqvjLU=')
@@ -412,23 +412,23 @@ def explicit_signature_settings(timestamp, nonce)
412412
it 'generate a correct signature with URL query parameters' do
413413
explicit_signature_settings('1251600739', 'c8350c0e47782d16d2fa48b2090c1d8f')
414414
hash = LtiOutbound::ToolLaunch.send(:generate_params, {
415-
:resource_link_id => '120988f929-274612',
416-
:user_id => '292832126',
417-
:roles => 'Instructor',
418-
:lis_person_name_full => 'Jane Q. Public',
419-
:lis_person_contact_email_primary => 'user@school.edu',
420-
:lis_person_sourced_id => 'school.edu:user',
421-
:context_id => '456434513',
422-
:context_title => 'Design of Personal Environments',
423-
:context_label => 'SI182',
424-
:lti_version => 'LTI-1p0',
425-
:lti_message_type => 'basic-lti-launch-request',
426-
:tool_consumer_instance_guid => 'lmsng.school.edu',
427-
:tool_consumer_instance_description => 'University of School (LMSng)',
428-
:lti_submit => 'Launch Endpoint with LTI Data'
415+
:resource_link_id => '120988f929-274612',
416+
:user_id => '292832126',
417+
:roles => 'Instructor',
418+
:lis_person_name_full => 'Jane Q. Public',
419+
:lis_person_contact_email_primary => 'user@school.edu',
420+
:lis_person_sourced_id => 'school.edu:user',
421+
:context_id => '456434513',
422+
:context_title => 'Design of Personal Environments',
423+
:context_label => 'SI182',
424+
:lti_version => 'LTI-1p0',
425+
:lti_message_type => 'basic-lti-launch-request',
426+
:tool_consumer_instance_guid => 'lmsng.school.edu',
427+
:tool_consumer_instance_description => 'University of School (LMSng)',
428+
:lti_submit => 'Launch Endpoint with LTI Data'
429429
}, 'http://dr-chuck.com/ims/php-simple/tool.php?a=1&b=2&c=3%20%26a', '12345', 'secret')
430-
expect(hash['oauth_signature']).to eql('l1ZTsn1HjGXzqeaTQMPbjrqvjLU=')
431-
expect(hash.key?('c')).to be false
430+
expect(hash['oauth_signature']).to eql('k/+aMdax1Jm5kuGF6DG/ptN5VfY=')
431+
expect(hash['c']).to eq '3 &a'
432432
end
433433

434434
it 'generate a correct signature with a non-standard port' do
@@ -463,4 +463,4 @@ def explicit_signature_settings(timestamp, nonce)
463463
expect(hash['oauth_signature']).to eql('X8Aq2HXSHnr6u/6z/G9zI5aDoR0=')
464464
end
465465
end
466-
end
466+
end

‎spec/models/lti/lti_integration_spec.rb‎

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,8 @@
9595
adapter = Lti::LtiOutboundAdapter.new(canvas_tool, canvas_user, canvas_course)
9696

9797
variable_expander = Lti::VariableExpander.new(root_account, canvas_course, controller, {
98-
current_user: canvas_user,
99-
current_pseudonym: pseudonym})
98+
current_user: canvas_user,
99+
current_pseudonym: pseudonym})
100100

101101
adapter.prepare_tool_launch(return_url, variable_expander)
102102
post_payload = adapter.generate_post_payload
@@ -134,13 +134,8 @@
134134
expect(post_payload['oauth_callback']).to eq 'about:blank'
135135
expect(post_payload['resource_link_id']).to eq canvas_tool.opaque_identifier_for(canvas_course)
136136
expect(post_payload['resource_link_title']).to eq canvas_tool.name
137-
138-
expect(post_payload['roles'].split(',')).to match_array ['urn:lti:role:ims/lis/TeachingAssistant', 'Instructor']
139-
expect(post_payload['ext_roles'].split(',')).to match_array ['urn:lti:instrole:ims/lis/Administrator',
140-
'urn:lti:instrole:ims/lis/Instructor',
141-
'urn:lti:role:ims/lis/Instructor',
142-
'urn:lti:role:ims/lis/TeachingAssistant',
143-
'urn:lti:sysrole:ims/lis/User']
137+
expect(post_payload['roles']).to eq 'urn:lti:role:ims/lis/TeachingAssistant,Instructor'
138+
expect(post_payload['ext_roles']).to eq "urn:lti:instrole:ims/lis/Administrator,urn:lti:instrole:ims/lis/Instructor,urn:lti:role:ims/lis/Instructor,urn:lti:role:ims/lis/TeachingAssistant,urn:lti:sysrole:ims/lis/User"
144139
expect(post_payload['tool_consumer_info_product_family_code']).to eq 'canvas'
145140
expect(post_payload['tool_consumer_info_version']).to eq 'cloud'
146141
expect(post_payload['tool_consumer_instance_contact_email']).to eq HostUrl.outgoing_email_address
@@ -263,14 +258,14 @@
263258
expect(hash['tool_consumer_instance_guid']).to eq sub_account.root_account.lti_guid
264259
end
265260

266-
it "should not include URI query parameters" do
261+
it "should include URI query parameters" do
267262
adapter = Lti::LtiOutboundAdapter.new(@tool, @user, @course)
268263
variable_expander = Lti::VariableExpander.new(root_account, canvas_course, controller)
269264
adapter.prepare_tool_launch('http://www.google.com', variable_expander, launch_url: 'http://www.yahoo.com?a=1&b=2', link_code: '123456')
270265
hash = adapter.generate_post_payload
271266

272-
expect(hash.key('a')).to be_falsey
273-
expect(hash.key('b')).to be_falsey
267+
expect(hash['a']).to eq '1'
268+
expect(hash['b']).to eq '2'
274269
end
275270

276271
it "should not allow overwriting other parameters from the URI query string" do

0 commit comments

Comments
 (0)