celtic-project / LTI-PHP

PHP class library for building LTI integrations
GNU Lesser General Public License v3.0
48 stars 38 forks source link

Can we mask the request URL when validating the OAuth signature? (lti 1.2) #35

Closed danhammari closed 2 years ago

danhammari commented 2 years ago

Under the OAuth 1.0 specification we take the request and hash it against the shared secret to get an OAuth signature. The OAuthRequest class uses its from_request static method to reconstruct the OAuth request from server variables, but there are times when we may wish to override the calculated request URL with a custom URL. For example, a server redirect may reroute a request and cause the calculated request URL to differ from the request URL that was used to encode the original OAuth signature. The OAuthRequest::from_request static method provides parameter options that we may use to override the calculated request values. However, the current System class does not leverage these parameters in its verifySignature method.

Could we add the $http_method, $http_url, and $parameters options to the System verifySignature method so they can be passed into the OAuthRequest::from_request static method? This will allow us to extend the Tool class and provide custom URL values to the verifySignature method call. This should help with test environments that implement custom ports as well as instances where redirects have changed the destination of the LTI request.

spvickers commented 2 years ago

My normal expectation when the URL used by the tool consumer is not the same as the URL seen by the tool provider, is that environment variables such as HTTP_X_FORWARDED_HOST are populated to indicate the change(s). I have adapted the code originally written by Google to accommodate this, though they only cover changes to the protocol and host. Why would you expect the HTTP method and parameters to change whilst a message is processed?

Are you calling the verifySignature method directly in your code? If not, did you have any views on how the values of the additional parameters you propose would be made available to the existing call to this method? It might be easiest to add an OAuth client interface as exists for the JWT-based security model, so that you can override the existing OAuth code with your own. But since the OAuth security model has been deprecated by 1EdTech, I was not expecting to be making changes to this part of the code.

danhammari commented 2 years ago

You are likely correct that editing methods that are considered deprecated is a waste of resources. These are the issues I am dealing with:

Basically, I need to keep legacy LTI 1.2 links alive as well as forward them to a new service provider subdomain and path. My docker test environment throws in an additional wrench as it requires me to use custom ports while testing, but that is less important.

For now, I think I can extend the library for my own purposes and make it so the calculated OAuth 1.0 signature matches the OAuth 1.0 signature that the LMS generated from its historical links. My plan is to first check the signature using the default settings (should validate all newly issued LTI links), and if that signature check fails, I will generate an alternative signature based on the historic LTI launch URL and check against that. This should allow me to accommodate both the legacy redirected LTI 1.2 requests as well as any new LTI 1.2 requests.