Support for editing LTI 1.3 collaborations

The collaboration edit link already passes in a content_item_id to the
`retrieve` endpoint. In ExtgernalToolsController#retrieve, this is used
in the LTI 1.1 path to make a content_item_id in the data for the
content item success URL, but it is not used in the LTI 1.3 path. This
commit:

1. passes thru the content_item_id into the `data` param (a JWT) for the
  deep linking return URL
2. In the deep-linking response page, includes content_item_id as
  "service_id" (following LTI 1.1 nomenclature) in the postMessage, and
3. in the collaborations code, correctly extracts the content_item_id
  from the the postMessage sent by the deep_linking_response page.

closes INTEROP-7986
flag=none

Test plan:
- Install the LTI 1.3 test tool
- From the course collaborations page, create a new collaboration using
  the tool
- Update the collaboration to have an updateUrl, e.g.:
    c=ExternalToolCollaboration.where(workflow_state: :active).last
    c.data['updateUrl'] = "http://web.lti-13-test-tool.docker/launch"
    c.save!
- Refresh the collaborations page and click the "edit" button to launch
  the tool
- In the tool, enter a new value in the title field, and send the link
  back to Canvas. The collaboration should be modified with the new
  title -- no new collaboration should be created

Change-Id: Iac13b5a2a5b3034a26cfb8541d34749c89704b91
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/317193
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Xander Moffatt <xmoffatt@instructure.com>
QA-Review: Xander Moffatt <xmoffatt@instructure.com>
Product-Review: Alexis Nast <alexis.nast@instructure.com>

app/controllers/external_tools_controller.rb

@@ -574,6 +574,7 @@ class ExternalToolsController < ApplicationController
       opts[:content_item_id] = content_item_id if content_item_id
       content_item_selection_request(tool, selection_type, opts)
     else
+      opts[:content_item_id] = content_item_id if content_item_id
       basic_lti_launch_request(tool, selection_type, opts)
     end
   rescue Lti::Errors::UnauthorizedError => e
@@ -634,6 +635,10 @@ class ExternalToolsController < ApplicationController
     end
   end
 
+  # This handles non-content item 1.1 launches, and 1.3 launches including deep linking requests.
+  # LtiAdvantageAdapter#generate_lti_params (called via
+  # adapter.generate_post_payload) determines whether or not an LTI 1.3 launch
+  # is a deep linking request
   def basic_lti_launch_request(tool, selection_type = nil, opts = {})
     lti_launch = tool.settings["post_only"] ? Lti::Launch.new(post_only: true) : Lti::Launch.new
     default_opts = {
@@ -655,13 +660,22 @@ class ExternalToolsController < ApplicationController
     # resource_link_id in regular QN launches is assignment.lti_resource_link_id
     opts[:link_code] = @tool.opaque_identifier_for(assignment.external_tool_tag) if assignment.present? && assignment.quiz_lti?
 
-    expander = variable_expander(assignment: assignment,
-                                 tool: tool,
-                                 launch: lti_launch,
-                                 post_message_token: opts[:launch_token],
-                                 secure_params: params[:secure_params],
-                                 placement: opts[:resource_type],
-                                 launch_url: opts[:launch_url])
+    # This is only for 1.3: editing collaborations for 1.1 goes thru content_item_selection_request()
+    if selection_type == "collaboration"
+      collaboration = opts[:content_item_id].presence&.then { ExternalToolCollaboration.find _1 }
+      collaboration = nil unless collaboration&.update_url == params[:url]
+    end
+
+    expander = variable_expander(
+      assignment: assignment,
+      tool: tool,
+      launch: lti_launch,
+      post_message_token: opts[:launch_token],
+      secure_params: params[:secure_params],
+      placement: opts[:resource_type],
+      launch_url: opts[:launch_url],
+      collaboration: collaboration
+    )
 
     adapter = if tool.use_1_3?
                 a = Lti::LtiAdvantageAdapter.new(

app/controllers/lti/ims/deep_linking_controller.rb

@@ -135,6 +135,7 @@ module Lti
                  deep_link_response: {
                    placement: return_url_parameters[:placement],
                    content_items: items,
+                   service_id: return_url_parameters[:content_item_id],
                    msg: messaging_value("msg"),
                    log: messaging_value("log"),
                    errormsg: messaging_value("errormsg"),

lib/lti/messages/deep_linking_request.rb

@@ -129,7 +129,8 @@ module Lti::Messages
     def add_deep_linking_request_claims!
       lti_assignment_id = Lti::Security.decoded_lti_assignment_id(@expander.controller&.params&.[]("secure_params"))
       assignment = Assignment.find_by(lti_context_id: lti_assignment_id) if lti_assignment_id
-      @message.deep_linking_settings.deep_link_return_url = return_url(assignment&.id)
+      content_item_id = @expander.collaboration&.id
+      @message.deep_linking_settings.deep_link_return_url = return_url(assignment&.id, content_item_id)
       @message.deep_linking_settings.accept_types = DEEP_LINKING_DETAILS.dig(placement, :accept_types)
       @message.deep_linking_settings.accept_presentation_document_targets = DEEP_LINKING_DETAILS.dig(placement, :document_targets)
       @message.deep_linking_settings.accept_media_types = DEEP_LINKING_DETAILS.dig(placement, :media_types).join(",")
@@ -141,20 +142,21 @@ module Lti::Messages
       @opts[:resource_type]
     end
 
-    def return_url(assignment_id = nil)
+    def return_url(assignment_id = nil, content_item_id = nil)
       @expander.controller.polymorphic_url(
         [@context, :deep_linking_response],
-        deep_link_params(assignment_id)
+        deep_link_params(assignment_id, content_item_id)
       )
     end
 
-    def deep_link_params(assignment_id)
+    def deep_link_params(assignment_id, content_item_id)
       {
         data: Lti::DeepLinkingData.jwt_from({
           modal: MODAL_PLACEMENTS.include?(placement),
           placement: placement,
           context_module_id: @opts[:context_module_id],
           assignment_id: assignment_id,
+          content_item_id: content_item_id,
           parent_frame_context: @opts[:parent_frame_context]
         }.compact),
       }

spec/controllers/external_tools_controller_spec.rb

@@ -1031,6 +1031,62 @@ describe ExternalToolsController do
         end
       end
 
+      context "tool is used to edit an existing collaboration" do
+        let(:collab) do
+          ExternalToolCollaboration.create!(
+            title: "my collab",
+            user: @teacher,
+            url: "http://www.example.com/launch",
+            context: @course,
+            data: {
+              "updateUrl" => "http://www.example.com/launch?abc=def"
+            }
+          )
+        end
+
+        let(:launch_params) do
+          JSON.parse(fetch_and_delete_launch(@course, decoded_jwt["verifier"]))
+        end
+
+        # it "it passes collaboration into the expander" do
+        #   get :retrieve, params: { course_id: @course.id, url: collab.update_url, launch_type: "collaboration", content_item_id: collab.id }
+        #   expect(launch_hash["https://purl.imsglobal.org/spec/lti/claim/custom"]["assignment_id"]).to eq(lti_assignment_id)
+        # end
+
+        let(:jwt) do
+          deep_link_return_url = launch_params["https://purl.imsglobal.org/spec/lti-dl/claim/deep_linking_settings"]["deep_link_return_url"]
+          return_jwt = deep_link_return_url.match(/data=([^&]*)/)[1]
+          JSON::JWT.decode(return_jwt, :skip_verification)
+        end
+
+        before do
+          lti_1_3_tool.collaboration = {
+            "enabled" => true,
+            "message_type" => "LtiDeepLinkingRequest",
+            "placement" => "collaboration",
+            "text" => "tool",
+          }
+          lti_1_3_tool.save!
+        end
+
+        it "adds content_item_id to the data JWT" do
+          get :retrieve, params: { course_id: @course.id, url: collab.update_url, launch_type: "collaboration", content_item_id: collab.id }
+          expect(jwt[:content_item_id]).to eq(collab.id)
+        end
+
+        context "when the update_url in the collaboration given by content_item_id does not match up with the given launch URL" do
+          it "does not add content_item_id to the data JWT" do
+            get :retrieve, params: {
+              course_id: @course.id,
+              url: collab.update_url + "&mismatch",
+              launch_type: "collaboration",
+              content_item_id: collab.id
+            }
+            expect(jwt[:content_item_id]).to be_nil
+          end
+        end
+      end
+
       context "tool is used for student_context_card" do
         # If and when we add the functionality, we can also test here that the
         # student_id appears in the launch.

spec/controllers/lti/ims/deep_linking_controller_spec.rb

@@ -29,7 +29,7 @@ module Lti
         subject { post :deep_linking_response, params: params }
 
         let(:placement) { "editor_button" }
-        let(:return_url_params) { { placement: placement } }
+        let(:return_url_params) { { placement: placement, content_item_id: 123 } }
         let(:data_token) { Lti::DeepLinkingData.jwt_from(return_url_params) }
         let(:params) { { JWT: deep_linking_jwt, account_id: account.id, data: data_token } }
         let(:course) { course_model(account: account) }
@@ -58,6 +58,7 @@ module Lti
                                                         deep_link_response: {
                                                           placement: placement,
                                                           content_items: content_items,
+                                                          service_id: 123,
                                                           msg: msg,
                                                           log: log,
                                                           errormsg: errormsg,

spec/lib/lti/messages/deep_linking_request_spec.rb

@@ -91,6 +91,19 @@ describe Lti::Messages::DeepLinkingRequest do
         let(:auto_create) { true }
         let(:accept_multiple) { false }
       end
+
+      context "when editing an existing collaboration (expander.collaboration != nil)" do
+        let(:collaboration) do
+          ExternalToolCollaboration.create! context: course, title: "foo", url: "http://notneededhere.example.com"
+        end
+
+        it "includes the content_item_id in the deep linking return URL's data JWT" do
+          expect(Lti::DeepLinkingData).to receive(:jwt_from) do |claims|
+            expect(claims[:content_item_id]).to eq(collaboration.id)
+          end
+          subject
+        end
+      end
     end
 
     context 'when resource type is "link_selection"' do

spec/lib/lti/messages/lti_advantage_shared_examples.rb

@@ -49,10 +49,12 @@ RSpec.shared_context "lti_advantage_shared_examples" do
       {
         current_user: user,
         tool: tool,
-        assignment: assignment
+        assignment: assignment,
+        collaboration: collaboration
       }
     )
   end
+  let(:collaboration) { nil }
   let(:assignment) do
     assignment_model(
       course: course,

ui/features/lti_collaborations/react/__tests__/router.test.js

@@ -73,18 +73,19 @@ describe('router', () => {
       })
 
       it('sends externalContentReady action for valid message', async () => {
-        const item = {service_id: 1, hello: 'world'}
+        const item = {hello: 'world'}
         window.postMessage(
           {
             subject: 'LtiDeepLinkingResponse',
             content_items: [item],
+            service_id: 123,
           },
           ENV.DEEP_LINKING_POST_MESSAGE_ORIGIN
         )
         await sleep(100)
 
         expect(actions.externalContentReady).toHaveBeenCalledWith({
-          service_id: item.service_id,
+          service_id: 123,
           contentItems: [item],
         })
         expect(actions.externalContentRetrievalFailed).not.toHaveBeenCalled()

ui/features/lti_collaborations/react/router.js

@@ -40,7 +40,7 @@ const attachListeners = () => {
       const item = processSingleContentItem(event)
       store.dispatch(
         actions.externalContentReady({
-          service_id: item.service_id,
+          service_id: event.data?.service_id,
           contentItems: [item],
         })
       )

ui/shared/deep-linking/__tests__/collaborations.test.js

@@ -71,6 +71,23 @@ describe('handleDeepLinking', () => {
     )
   })
 
+  describe('when there is a service_id in the postMessage', () => {
+    const overrides = {data: {subject: 'LtiDeepLinkingResponse', content_items, service_id: 123}}
+    it('updates the collaboration', async () => {
+      jest.spyOn(document, 'querySelector')
+      await handleDeepLinking(event(overrides))
+
+      expect($.ajaxJSON).toHaveBeenCalledWith(
+        undefined,
+        'PUT',
+        {contentItems: JSON.stringify(content_items)},
+        expect.anything(),
+        expect.anything()
+      )
+      expect(document.querySelector).toHaveBeenCalledWith('.collaboration_123 a.title')
+    })
+  })
+
   describe('when there is a unhandled error parsing the content item', () => {
     const overrides = {
       data: {subject: 'LtiDeepLinkingResponse', content_items: 1},

ui/shared/deep-linking/collaborations.js

@@ -52,10 +52,14 @@ export function onExternalContentReady(e, data) {
 export const handleDeepLinking = async event => {
   try {
     const item = processSingleContentItem(event)
-    onExternalContentReady(event, {
-      service_id: item.service_id,
-      contentItems: [item],
-    })
+    if (typeof item !== 'object') {
+      $.flashError(I18n.t('Error retrieving content from tool (bad content item)'))
+    } else {
+      onExternalContentReady(event, {
+        service_id: event.data?.service_id,
+        contentItems: [item],
+      })
+    }
   } catch (e) {
     $.flashError(I18n.t('Error retrieving content from tool'))
   }