celtic-project / LTI-PHP

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

array_merge_recursive throws off OAuth1 Signature when GET and POST keys are duplicated #36

Closed danhammari closed 2 years ago

danhammari commented 2 years ago

SCENARIO: Signing LTI 1.2 basic launch request using OAuth1 Protocol PLATFORM appends custom values to the URL Query String as well as to the POST e.g.

In the OAuth/OAuthRequest::from_request method we use the following logic:

$parameters = OAuthUtil::parse_parameters($_SERVER['QUERY_STRING']);
$post_data = OAuthUtil::parse_parameters(file_get_contents(self::$POST_INPUT));
$parameters = array_merge_recursive($parameters, $post_data);

The array_merge_recursive function causes the duplicated parameters to stack into an array: {"custom_type":["article","article"],"custom_id":["1234","1234"]}

This causes the OAuth1 signature that is calculated to yield a different signature than that provided by the platform. Could we change the OAuth/OAuthRequest::from_request method so it uses array_replace_recursive instead of array_merge_recursive? This should prevent the duplicated keys from transforming into redundant array values.

danhammari commented 2 years ago

This is my current workaround. It allows my tool to calculate the same OAuth1 signature that Canvas sends over while also sending my custom parameters in both the URL query string and the POST:

/**
 * get the request, but remove any duplicated custom parameters
 * @param string $http_method (optional)
 * @param string $http_url (optional)
 * @param array $parameters (optional)
 * @return \ceLTIc\LTI\OAuth\OAuthRequest
 * */
public static function getRequestWithDistinctCustomParams($http_method = null, $http_url = null, $parameters = null){
    $request = \ceLTIc\LTI\OAuth\OAuthRequest::from_request($http_method, $http_url, $parameters);
    // deduplicate custom properties that have been merged from query string and post
    $keyword = 'custom';
    $request_params = $request->get_parameters();
    foreach ($request_params as $request_key => $request_value) {
        if (substr($request_key, 0, strlen($keyword)) == $keyword){
            if (!empty($request_value) && is_array($request_value)) {
                $temp_value = reset($request_value);
                $request->set_parameter($request_key, $temp_value, false);
            }
        }
    }
    return $request;
}
spvickers commented 2 years ago

I believe that the current code is correct. What system are you comparing the signature with?

danhammari commented 2 years ago

I am trying to get canvas.instructure.com to work with legacy LTI 1.2 requests. When I load the links into Canvas from a thin common cartridge I end up with custom parameters duplicated in both the URL query and the POST data. I had some initial successes after I flattened the custom parameters, but now I am getting OAuth1 signature mismatches again. I will keep testing. I had hoped the library's OAuth1 signature creation would simply work out of the box, but there is something happening with the Canvas platform that is causing my OAuth1 signatures to come out differently.

spvickers commented 2 years ago

Have you set the "oauth_compliant" property to "true" for your LTI tool in Canvas? By default this is set to "false" which means that Canvas does not generate a signature which is compliant with the OAuth spec when query parameters are included in the URL.

danhammari commented 2 years ago

Well, I've been to one world fair, a picnic, and a rodeo, and that's the stupidest thing I ever heard come over a set of earphones. - Major T.J. Kong

Thanks, spvickers! I edited my Canvas XML configuration file and added a flag for Canvas that forced the LMS to treat my LTI links in an oauth_compliant manner. After rebuilding my third party app configuration using the updated XML my LTI links started working properly. I found the obscure fix you pointed out on the Canvas XML config page:

https://canvas.instructure.com/doc/api/file.tools_xml.html

I gotta say that it is kinda ludicrous to need to set a flag through config XML in order to get Canvas to treat LTI links in an oauth_compliant manner. My links are now working though--at least they are in my testing environment. My production environment still is giving me some trouble, but that might be caused by something else. I'll keep hacking away. Thanks!

danhammari commented 2 years ago

I finally got to the bottom of the problem, and the reason the OAuth1 signatures were mismatched on my production servers is because someone on my team instructed the Nginx web servers to inject a custom request parameter into the HTTP query. In Apache it would be called a mod rewrite rule. Essentially, the web server was altering the request and the OAuth1 signature came out differently in the calculation because of it. Once I removed this behavior from the web server my OAuth1 signatures began matching as they should.