Skip to content

Support editing via LTI Deep Linking in Content Author#2578

Merged
emmachughes merged 4 commits intomasterfrom
ca-deep-link-editor-support
Nov 2, 2023
Merged

Support editing via LTI Deep Linking in Content Author#2578
emmachughes merged 4 commits intomasterfrom
ca-deep-link-editor-support

Conversation

@emmachughes
Copy link
Contributor

This adds editing support via LTI Deep Linking. It works by checking if content is launched via a Deep Linking request, and if so, redirects to the editor.

Contained in a separate commit is some refactoring of LTI handling in CA. The big changes are:

  • App\Http\Requests\LTIRequest::fromRequest() is removed. This didn't verify that the LTI request was signed, yet was used "everywhere". App\Lti\Lti::getRequest() is the intended replacement.
  • App\H5pLti is renamed to App\Lti\Lti. This had nothing to do with H5P anyway.
  • App\Http\Requests\LTIRequest is renamed to App\Lti\LtiRequest.
  • App\H5pLti::getValidatedLtiRequest() is now App\Lti\Lti::getRequest(), and takes Laravel's request object as its param.
  • Some general cleanup around code that uses the request object.

The justification for these changes:

I'm making another risky change to Content Author. In the past it's been possible to retrieve an LTI request object without actually verifying that it's signed correctly, but now you won't be able to do this without signature verification having been performed once.

The parameters sent via LTI can determine who you are, what your role in the system is, extra CSS and JS that's added to the page, and possibly other dangerous actions, so we need to reliably know that the parameters are verified to come from a trusted source. "This was probably verified further up in the chain" isn't a good approach to that.

@codecov
Copy link

codecov bot commented Oct 31, 2023

Codecov Report

Merging #2578 (42bc5f3) into master (95ed61d) will decrease coverage by 0.12%.
The diff coverage is 48.61%.

@@             Coverage Diff              @@
##             master    #2578      +/-   ##
============================================
- Coverage     62.78%   62.67%   -0.12%     
- Complexity     2737     2738       +1     
============================================
  Files           279      280       +1     
  Lines         12146    12145       -1     
============================================
- Hits           7626     7612      -14     
- Misses         4520     4533      +13     
Components Coverage Δ
contentauthor 62.67% <48.61%> (-0.12%) ⬇️

@emmachughes emmachughes requested a review from chrieinv October 31, 2023 11:37
@emmachughes emmachughes merged commit 6a2f9d0 into master Nov 2, 2023
@emmachughes emmachughes deleted the ca-deep-link-editor-support branch November 2, 2023 11:29
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