convert params to "unsafe" hash before filtering

fixes PLAT-2770 test plan: • Go to any account/course settings page • Click on the Apps tab • Click on any LTI that populates within the App Center (I use 3DGameLab) • Click '+ Add App' • Click the 'Add App' button in the window that appears • Verify app is added successfully Change-Id: I060ed314423c841ff5d8ef12c46f2377f31b58b1 Reviewed-on: https://gerrit.instructure.com/122918 Tested-by: Jenkins Reviewed-by: Andrew Butterfield <[email protected]> QA-Review: Weston Dransfield <[email protected]> Product-Review: August Thornton <[email protected]>
javierjeronimo · Aug 16, 2017 · f2bc368 · f2bc368
1 parent 2dba660
commit f2bc368
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 14 deletions.
diff --git a/app/controllers/external_tools_controller.rb b/app/controllers/external_tools_controller.rb
@@ -588,8 +588,12 @@ def basic_lti_launch_request(tool, selection_type = nil, opts = {})
     opts = default_opts.merge(opts)
 
     assignment = @context.assignments.active.find(params[:assignment_id]) if params[:assignment_id]
+    adapter = Lti::LtiOutboundAdapter.new(tool, @current_user, @context).prepare_tool_launch(
+      @return_url,
+      variable_expander(assignment: assignment, tool: tool, launch: lti_launch),
+      opts
+    )
 
-    adapter = Lti::LtiOutboundAdapter.new(tool, @current_user, @context).prepare_tool_launch(@return_url, variable_expander(assignment: assignment, tool: tool), opts)
     lti_launch.params = if selection_type == 'homework_submission' && assignment
                           adapter.generate_post_payload_for_homework_submission(assignment)
                         else
@@ -963,8 +967,9 @@ def create_tool_with_verification
         :config_settings
       ]
 
-      external_tool_params = params.permit(*required_params).to_unsafe_h
-      external_tool_params[:config_url] = app_api.get_app_config_url(params[:app_center_id], params[:config_settings])
+      # we're ok with an "unsafe" hash because we're filtering via required_params
+      external_tool_params = params.to_unsafe_h.select{|k, _| required_params.include?(k.to_sym)}
+      external_tool_params[:config_url] = app_api.get_app_config_url(external_tool_params[:app_center_id], external_tool_params[:config_settings])
       external_tool_params[:config_type] = 'by_url'
 
       @tool = @context.context_external_tools.new

diff --git a/config/initializers/strong_parameters.rb b/config/initializers/strong_parameters.rb
@@ -109,7 +109,7 @@ def skip_error_report?; true; end
 if CANVAS_RAILS5_0
   module RaiseOnDeprecateHashMethods
     def raise_deprecation_error(method)
-      raise "The method '#{method})' is going away for `params` in Rails 5.1 because ActionController::Parameters will no longer inherit from Hash - Use #to_unsafe_h if needed"
+      raise "The method '#{method}' is going away for `params` in Rails 5.1 because ActionController::Parameters will no longer inherit from Hash - Use #to_unsafe_h if needed"
     end
 
     def method_missing(method_sym, *args, &block)

diff --git a/spec/integration/external_tools_controller_spec.rb b/spec/integration/external_tools_controller_spec.rb
@@ -134,9 +134,10 @@
           shared_secret: 'N/A',
           config_url: 'https://www.edu-apps.org/lti_public_resources/config.xml?id=youtube&name=YouTube&channel_name=jangbricks',
           config_type: 'by_url',
-          name:'YouTube',
+          name: 'YouTube',
           app_center_id: 'pr_youtube',
-          course_navigation: {enabled: true}
+          config_settings: { name: 'YouTube', channel_name: 'foo-bar' },
+          course_navigation: { enabled: true }
         }
       end
 
@@ -170,12 +171,8 @@
         }
       end
 
-      let(:app_api) { double() }
-
-      before do
-        allow(AppCenter::AppApi).to receive(:new).and_return(app_api)
-        allow(app_api).to receive(:fetch_app_center_response).and_return(app_center_response)
-        allow(app_api).to receive(:get_app_config_url).and_return(app_center_response['config_xml_url'])
+      before(:each) do
+        allow_any_instance_of(AppCenter::AppApi).to receive(:fetch_app_center_response).and_return(app_center_response)
 
         configxml = File.read(File.join(Rails.root, 'spec', 'fixtures', 'lti', 'config.youtube.xml'))
         stub_request(:get, app_center_response['config_xml_url']).to_return(body: configxml)
@@ -195,7 +192,7 @@
       end
 
       it 'gives error if app_center_id is not provided' do
-        allow(app_api).to receive(:get_app_config_url).and_return('')
+        allow_any_instance_of(AppCenter::AppApi).to receive(:get_app_config_url).and_return('')
         user_session(@teacher)
 
         post(
@@ -205,7 +202,6 @@
         )
 
         expect(response).not_to be_success
-        allow(app_api).to receive(:get_app_config_url).and_return(app_center_response['config_xml_url'])
       end
 
       it 'ignores non-required params' do