Learnosity / learnosity-sdk-php

Learnosity SDK for PHP
Apache License 2.0
10 stars 10 forks source link

Discrepancy between request packet data and its signature #82

Open armatronic opened 3 weeks ago

armatronic commented 3 weeks ago

Hi,

I've discovered what I believe is a significant bug in 1.0.4, which will prevent any payload which contains a URL from being accepted by Learnosity's API.

The bug is caused by a difference between how the payload is encoded when it is returned by the SDK, and how it's encoded when it's used to generate a message signature.

Example code using the SDK

When loading a list of items for a student to attempt a quiz:

return (new \LearnositySdk\Request\Init(
    'items',
    [
        'domain'       => $this->serverName,
        'consumer_key' => $this->consumerKey
    ],
    $this->consumerSecret,
    $requestPacket // provided as an array
))->generate();

SDK 1.0.3 and earlier

\LearnositySdk\Request\Init::generate encodes $requestPacket using \LearnositySdk\Utils\Json::encode:

$output['request'] = $this->requestPassedAsString ?
    Json::encode($this->requestPacket) :
    $this->requestPacket;

It also uses Json::encode to encode the request packet when signing the request in \LearnositySdk\Request\Init::generateSignature:

// Add the requestPacket if necessary
if ($this->signRequestData && !empty($this->requestPacket)) {
    $signatureArray[] = Json::encode($this->requestPacket);
}

(Note that Json::encode uses json_encode with the JSON_UNESCAPED_UNICODE | JSON_UNESCAPED_SLASHES flags.)

SDK current behavior

$requestPacket is still encoded using Json::encode, but is now signed using json_encode without any flags; in \LearnositySdk\Services\PreHashStrings\LegacyPreHashString::getPreHashString:

if (in_array($this->service, static::SERVICES_REQUIRING_SIGNED_REQUEST)) {
    $requestJson = $request;
    if (is_array($request)) {
        $requestJson = json_encode($request);
    }
    $signatureArray[] = $requestJson;
}

This results in a mismatch between the request packet data used to sign the request and the request itself if the request packet contains a URL (e.g. a configuration.onsave_redirect_url value):

That of course means that Learnosity's API rejects the request this SDK generates. For example:

// initOptions is passed from `generate` above
const itemsApp = LearnosityItems.init(initOptions);

IMO this is a significant issue, so please advise.

vikram-pai commented 3 weeks ago

Thanks. We are aware of this issue and working on a fix for it this sprint. SDK releases are not tied to our normal releases, so we hope to get this fixed asap.