Skip to content

Fix IMS/LTI Deep Linking for multiple resources and cross-site returns - #8947

Open
moigaston2 wants to merge 1 commit into
chamilo:1.11.xfrom
moigaston2:fix-ims-lti-deep-linking-1.11
Open

Fix IMS/LTI Deep Linking for multiple resources and cross-site returns#8947
moigaston2 wants to merge 1 commit into
chamilo:1.11.xfrom
moigaston2:fix-ims-lti-deep-linking-1.11

Conversation

@moigaston2

@moigaston2 moigaston2 commented Aug 25, 2026

Copy link
Copy Markdown

Summary
This PR fixes several issues in the IMS/LTI 1.9.0 plugin affecting LTI 1.1 Deep Linking, particularly when an LTI provider exposes several resources through the same launch URL.
The issues were identified and tested with Chamilo 1.11.40 and Edupool / Schulmediathek Hamburg, using LTI 1.1 with Deep Linking.
Before these changes, the plugin could effectively handle only one resource from a provider using a common launch URL: adding another video replaced the previous one. Additional issues in the Deep Linking workflow could also cause fatal errors, lost sessions, access-denied responses, and incorrect return behaviour.
After these changes, multiple resources from the same LTI provider can be added independently to the same course.

Problems identified
1. New LTI resources with the same launch URL overwrite previous resources
saveItemAsLtiLink() searches for an existing child tool using:
• launch URL
• parent LTI tool
• course
If the provider uses the same launch URL for several resources, Chamilo therefore treats each newly selected resource as the existing one and overwrites its title, description and custom parameters.
This affects LTI providers such as Edupool, where different videos can share the same launch endpoint.
2. Fatal error when enabling or editing Deep Linking
When editing a parent LTI tool, edit.php can try to update the corresponding course tool without first checking whether a CTool actually exists.
This can result in:
Argument 1 passed to ImsLtiPlugin::updateCourseTool()
must be an instance of Chamilo\CourseBundle\Entity\CTool, null given
The code now checks whether the course tool exists. If it does not, it creates it instead of calling updateCourseTool() with null.
3. Incomplete LTI 1.1 Deep Linking request parameters
The LTI 1.1 Deep Linking request did not fully describe the expected behaviour for multiple returned resources.
The request is now explicitly configured with:
accept_multiple = true
accept_unsigned = false
auto_create = false
The return URL also preserves the Chamilo course context required to process the returned resource correctly.
4. Session loss during the cross-site Deep Linking return
The Deep Linking response is sent back to Chamilo through a cross-site POST.
Depending on browser cookie handling, Chamilo may receive this POST without the active Chamilo session. The request is then rejected or the user is redirected to the Chamilo login page.
The return handling has therefore been made more robust by preserving the original Deep Linking POST and replaying it from the Chamilo context before processing the returned content.
5. OAuth validation during the internal repost
The cross-site session recovery mechanism uses an internal marker to distinguish a replayed request from the original POST.
This internal parameter must not participate in OAuth signature validation because it was not part of the request originally signed by the LTI provider.
The internal repost marker is therefore removed before OAuth verification so that the signature is calculated only from the original LTI parameters.
6. Incorrect behaviour after successful resource selection
After saving a returned resource, the previous implementation redirected back to:
ims_lti/start.php?id=...
This launches the LTI provider again and can make the provider page appear to reload immediately after a resource has been selected.
The return process now sends the user back to the Chamilo course and correctly handles the case where the LTI provider was opened in a separate window.
7. LTI 1.3 Deep Linking return handling
The corresponding LTI 1.3 return code has also received defensive improvements, including safer handling of unsupported or missing content-item objects.
These LTI 1.3 changes were syntax-checked but were not functionally tested with the Edupool integration, which uses LTI 1.1.

Changes
The following files are modified:
plugin/ims_lti/ImsLtiPlugin.php
plugin/ims_lti/edit.php
plugin/ims_lti/form.php
plugin/ims_lti/item_return.php
plugin/ims_lti/item_return2.php

ImsLtiPlugin.php
A Deep Linking selection now creates a new LTI resource instance instead of reusing an existing instance solely because the launch URL is identical.
This allows multiple resources from the same provider to coexist:
Same LTI provider
├── Resource A
├── Resource B
├── Resource C
└── ...
even when all resources use the same launch URL.

edit.php
Checks whether the corresponding course tool exists before calling updateCourseTool().
If it does not exist, the course tool is created instead.

form.php
Improves the LTI 1.1 Deep Linking request by adding multiple-resource support and preserving the required Chamilo return context.

item_return.php
Improves LTI 1.1 Deep Linking return handling, cross-site session recovery, OAuth verification, and the final return to the Chamilo course.

item_return2.php
Applies corresponding defensive improvements to the LTI 1.3 Deep Linking return path.

Reproduction before the fix
1. Configure an LTI 1.1 provider with Deep Linking.
2. Add the provider to a course.
3. Select a first video or resource.
4. The resource appears as an LTI course tool.
5. Open the provider again.
6. Select a second video or resource using the same launch URL.

Actual behaviour
The second resource replaces the first one.
Depending on the Deep Linking workflow, additional problems may also occur:
• fatal error when enabling or editing Deep Linking;
• expired-session/login screen during the return;
• access denied after selecting a resource;
• provider page being launched again instead of returning to the course.

Expected behaviour
Every newly selected Deep Linking resource should create an independent LTI resource instance, even when several resources share the same launch URL.

Tested behaviour after the fix
Tested with:
Chamilo: 1.11.40
IMS/LTI plugin: 1.9.0
LTI version: 1.1
Deep Linking: enabled
Provider: Edupool / Schulmediathek Hamburg
Verified:
• the LTI provider opens successfully;
• resources can be selected through Deep Linking;
• the return to Chamilo works;
• multiple videos can be added successively;
• previously added videos are no longer overwritten;
• each video receives its own LTI course-tool instance;
• no fatal error occurs when Deep Linking is enabled;
• OAuth validation succeeds;
• the user is returned to the Chamilo course after resource selection.
The server was configured according to the plugin README regarding cross-site cookies (SameSite=None; Secure).

Relation to Moodle
Moodle's LTI implementation was used as a functional reference for understanding the expected Deep Linking workflow, particularly:
• treating separate Deep Linking selections as separate activity instances;
• supporting multiple returned resources;
• preserving context during the Deep Linking return;
• handling cross-site POST/session behaviour.
The implementation in this PR was written for and adapted to Chamilo's own plugin architecture.

Note regarding Chamilo 2
The same underlying multiple-resource issue should also be reviewed in Chamilo 2.x.
Chamilo 2.0.3 uses a reorganized IMS/LTI plugin, but its resource-saving logic also searches existing LTI tools by launchUrl and course context. Providers using a common launch URL for several distinct resources may therefore be affected by the same underlying issue.
This PR targets Chamilo 1.11.40 / IMS-LTI 1.9.0 only. The changes should not be applied directly to Chamilo 2.x because the plugin architecture has changed. The corresponding logic should be reviewed and, where applicable, adapted separately for Chamilo 2.

@AngelFQC

Copy link
Copy Markdown
Member

Thanks for the detailed write-up — the Deep Linking issues are real. Two blockers before this can move forward.

1. Wrong file paths. The files were added at the repository root instead of modifying plugin/ims_lti/:

5 changed files, 1396 additions, 0 deletions   (all ADDED at repo root)

The content itself is fine — placed under plugin/ims_lti/ it would be a ~+194/−33 diff — but as new root-level files nothing here takes effect. The repo root is the docroot in 1.11.x, so they would also be publicly reachable (https://host/item_return.php, …) with a require_once that resolves outside the project, giving a fatal error with path disclosure. Please rebase with the files under plugin/ims_lti/ as modifications.

2. This plugin is LTI-certified, so changes must hold to LTI 1.3 / Deep Linking 2.0.

Some of your changes are exactly right and you undersell them: dropping the launch-URL deduplication is the spec-conformant behaviour — Deep Linking models each selection as an independent resource link, two links may share a url and differ only by custom, and it keeps resource_link.id unique. auth.php already does this for 1.3 via LtiResourceLink::save(). Likewise accept_multiple and the narrowed accept_media_types align 1.1 with the accept_types => ['ltiResourceLink'] already declared at auth.php:177.

These, however, need fixing:

  • auto_create = 'false' — per spec, false means the platform will not persist without giving the user a chance to cancel. item_return.php persists immediately, and auth.php:184 declares true for 1.3. It should be true.
  • frame is not a valid accept_presentation_document_targets value in DL 2.0 (iframe, window, embed only), and ImsLtiTool::setDocumenTarget() would silently degrade it to iframe.
  • The item_return2.php null check swallows the condition instead of reporting it: an unaccepted content_item type is now skipped while the ToolAdded success flash is still shown. It previously failed loudly. Since you are already in this file, this is the place to also validate message_type === 'LtiDeepLinkingResponse', version === '1.3.0', deployment_id, nonce and azp, handle aud being an array, and surface the msg / errormsg claims.
  • The repost mechanism works against the spec's design: deep_link_return_url is meant to be validated cryptographically (signed JWT + data round-trip), not via a session cookie. Carrying the course context inside the data claim — which the tool must return unchanged — removes the need for both the repost and the ?cidReq= that alters the OAuth base string in 1.1. Note data is currently unsigned (tool:<id>), so it would need a platform HMAC if it carries more.

Also, on the repost as written: is_scalar($value) drops array values (custom[]), and browsers normalise line breaks to CRLF in form values, so a content_items JSON with real newlines comes back with different bytes and oauth_signature fails intermittently per provider.

Finally: please bump the plugin version and update README.md, and split this up — the clear fixes can land quickly while the repost approach needs redesign.

@AngelFQC AngelFQC left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants