Skip to content

Commit

Permalink
Use Json::encode() throughout LRN-43802 (#85)
Browse files Browse the repository at this point in the history 
api-events should forward user_id LRN-44754

[TEST] Include user_id in api-events security packet LRN-44754

Assimilate user_id treatment to LP LRN-43802 LRN-44748

[DOC] Update change log

[Bug] Add the action value to the signature array only we not empty LRN-44748

[Bug] Fix the empty request param by converting to object and array LRN-44748

[Feature] Implement the request to jsonstring in contructor LRN-44748

[Feature] Iteration-3 Implement for generate() method the request MUST be any array LRN-44748

[Feature] Iteration-3 Convert the string to json for telemetry enabled LRN-44748

Use a decodedRequestPacket variable for the doecded request, fix up function signatures

Fix up function signatures

Remove wrong comment

[Feature] Remove the unwanted code LRN-44748

Re-use variable instead of calling function again

Remove double empty check

[Feature] Add test case for geneeratesignature() with empty array and empty object LRN-44748

[Feature] Add new testcase for request with empty lines LRN-44748

[Feature] Remove the un-used method LRN-44748

Co-authored-by: david-scarratt-lrn <david.scarratt@learnosity.com>
  • Loading branch information
@sreenivasa-murty-lrn @david-scarratt-lrn
sreenivasa-murty-lrn and david-scarratt-lrn authored Oct 10, 2024
1 parent 33be98e commit 518a2d3
Show file tree
Hide file tree
Showing 7 changed files with 200 additions and 77 deletions.
7 changes: 7 additions & 0 deletions ChangeLog.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,13 @@ and this project adheres to [Semantic
Versioning](http://semver.org/spec/v2.0.0.html).

## [Unreleased]
### Fixed
- Fixed an inconsistency with encoding to JSON
- Fixed incorrect replacement of SERVICE_ITEMS_API by SERVICE_EVENTS_API
in services not requiring `user_id` in the security packet
- Fixed handling of `user_id` in security packet for services not
requiring it

## [v1.0.4] - 2024-07-11
### Added
- Added composable services for signature generation.
Expand Down
123 changes: 68 additions & 55 deletions src/Request/Init.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,15 @@ class Init
*/
private $requestPacket;

private $decodedRequestPacket;

/**
* Tracking if the request was passed as a string
* Tracking if the request was passed as a string. If the request is indeed passed as string,
* there is an attempt to not alter the string in any way by decoding and encoding. This will
* only work if telemetry is disabled. Otherwise the metadata has to be set in the request
* meaning that decoding is required. It also does not work for assess if it has questions
* api settings
*
* @var bool
*/
private $requestPassedAsString = false;
Expand Down Expand Up @@ -141,19 +148,17 @@ public function __construct(

$this->preHashStringGenerator = $this->preHashStringFactory->getPreHashStringGenerator($service);

// First validate the arguments passed
list ($requestPacket, $securityPacket) = $this->validate($service, $secret, $securityPacket, $requestPacket);

if (self::$telemetryEnabled) {
$requestPacket = $this->addMeta($requestPacket);
}

// Set instance variables based off the arguments passed
$this->service = $service;
$this->securityPacket = $securityPacket;
$this->secret = $secret;
$this->requestPacket = $requestPacket;
$this->action = $action;
$this->validate();

if (self::$telemetryEnabled) {
$this->addMeta();
}

// Set any service specific options
$this->setServiceOptions();
Expand All @@ -177,11 +182,8 @@ public function __construct(
* }
* }
*
* @param array $requestPacket
*
* @return array
*/
private function addMeta(array $requestPacket): array
private function addMeta()
{
$sdkMetricsMeta = [
'version' => $this->getSDKVersion(),
Expand All @@ -191,15 +193,14 @@ private function addMeta(array $requestPacket): array
'platform_version' => php_uname('r')
];

if (isset($requestPacket['meta'])) {
$requestPacket['meta']['sdk'] = $sdkMetricsMeta;
if (isset($this->decodedRequestPacket['meta'])) {
$this->decodedRequestPacket['meta']['sdk'] = $sdkMetricsMeta;
} else {
$requestPacket['meta'] = [
$this->decodedRequestPacket['meta'] = [
'sdk' => $sdkMetricsMeta
];
}

return $requestPacket;
$this->requestPacket = Json::encode($this->decodedRequestPacket);
}

/**
Expand Down Expand Up @@ -230,7 +231,7 @@ public function generate(bool $encode = true)
// Add the security packet (with signature) to the output
$output['security'] = Json::encode($this->securityPacket);

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

if (!empty($this->action)) {
$output['action'] = $this->action;
Expand All @@ -241,8 +242,8 @@ public function generate(bool $encode = true)
case 'assess':
// Stringify the request packet if necessary
$output = $this->requestPassedAsString ?
Json::encode($this->requestPacket) :
$this->requestPacket;
$this->requestPacket :
$this->decodedRequestPacket;
break;
case 'author':
case 'authoraide':
Expand All @@ -253,8 +254,8 @@ public function generate(bool $encode = true)

// Stringify the request packet if necessary
$output['request'] = $this->requestPassedAsString ?
Json::encode($this->requestPacket) :
$this->requestPacket;
$this->requestPacket :
$this->decodedRequestPacket;
break;
case 'questions':
// Add the security packet (with signature) to the root of output
Expand All @@ -263,14 +264,14 @@ public function generate(bool $encode = true)
// Remove the `domain` key from the security packet
unset($output['domain']);

if (!empty($this->requestPacket)) {
$output = array_merge($output, $this->requestPacket);
if (!empty($this->decodedRequestPacket)) {
$output = array_merge($output, $this->decodedRequestPacket);
}
break;
case 'events':
// Add the security packet (with signature) to the output
$output['security'] = $this->securityPacket;
$output['config'] = $this->requestPacket;
$output['config'] = $this->decodedRequestPacket;
break;
default:
// no default
Expand Down Expand Up @@ -319,14 +320,14 @@ private function setServiceOptions()
// security information and a signature. Retrieve the security
// information from $this and generate a signature for the
// Questions API
if (array_key_exists('questionsApiActivity', $this->requestPacket)) {
if (array_key_exists('questionsApiActivity', $this->decodedRequestPacket)) {
// prepare signature parts
$signatureParts = [];
$signatureParts['consumer_key'] = $this->securityPacket['consumer_key'];
if (isset($this->securityPacket['domain'])) {
$signatureParts['domain'] = $this->securityPacket['domain'];
} elseif (isset($this->requestPacket['questionsApiActivity']['domain'])) {
$signatureParts['domain'] = $this->requestPacket['questionsApiActivity']['domain'];
} elseif (isset($this->decodedRequestPacket['questionsApiActivity']['domain'])) {
$signatureParts['domain'] = $this->decodedRequestPacket['questionsApiActivity']['domain'];
} else {
$signatureParts['domain'] = 'assess.learnosity.com';
}
Expand All @@ -338,7 +339,7 @@ private function setServiceOptions()
$signatureParts['secret'] = $this->secret;

// override security parameters in questionsApiActivity
$questionsApi = $this->requestPacket['questionsApiActivity'];
$questionsApi = $this->decodedRequestPacket['questionsApiActivity'];
$questionsApi['consumer_key'] = $signatureParts['consumer_key'];
unset($questionsApi['domain']);
$questionsApi['timestamp'] = $signatureParts['timestamp'];
Expand All @@ -351,7 +352,8 @@ private function setServiceOptions()
$this->securityPacket = $signatureParts;
$questionsApi['signature'] = $this->generateSignature();

$this->requestPacket['questionsApiActivity'] = $questionsApi;
$this->decodedRequestPacket['questionsApiActivity'] = $questionsApi;
$this->requestPacket = Json::encode($this->decodedRequestPacket);
}
break;
case 'questions':
Expand All @@ -362,15 +364,15 @@ private function setServiceOptions()
// The Events API requires a user_id, so we make sure it's a part
// of the security packet as we share the signature in some cases
if (
array_key_exists('user_id', $this->requestPacket)
&& !array_key_exists('user_id', $this->securityPacket)
array_key_exists('user_id', $this->decodedRequestPacket) &&
!array_key_exists('user_id', $this->securityPacket)
) {
$this->securityPacket['user_id'] = $this->requestPacket['user_id'];
$this->securityPacket['user_id'] = $this->decodedRequestPacket['user_id'];
}
break;
case 'events':
$this->signRequestData = false;
$users = $this->requestPacket['users'];
$users = $this->decodedRequestPacket['users'];
$hashedUsers = [];
if (!$this->isAssocArray($users)) {
throw new ValidationException('Passing an array of user IDs is deprecated,' .
Expand All @@ -385,7 +387,8 @@ private function setServiceOptions()
);
}
if (count($hashedUsers)) {
$this->requestPacket['users'] = $hashedUsers;
$this->decodedRequestPacket['users'] = $hashedUsers;
$this->requestPacket = Json::encode($this->decodedRequestPacket);
}
break;
default:
Expand All @@ -397,42 +400,52 @@ private function setServiceOptions()
/**
* Validate the arguments passed to the constructor
*
* @param string $service
* @param string $secret
* @param array|string $securityPacket
* @param array|string $requestPacket
* @return array
* @throws ValidationException
*/
public function validate(string $service, string $secret, $securityPacket, $requestPacket): array
public function validate()
{
if (is_string($requestPacket)) {
$requestPacket = json_decode($requestPacket, true);
$this->requestPassedAsString = true;
}

if (is_null($requestPacket)) {
$requestPacket = [];
$this->validateAndSetRequestPacket();
// In case the user gave us a JSON securityPacket, convert to an array
if (is_string($this->securityPacket)) {
$this->securityPacket = json_decode($this->securityPacket, true);
}

if (empty($securityPacket) || !is_array($securityPacket)) {
if (empty($this->securityPacket) || !is_array($this->securityPacket)) {
throw new ValidationException('The security packet must be an array or a valid JSON string');
}

// In case the user gave us a JSON securityPacket, convert to an array
if (!is_array($securityPacket) && is_string($securityPacket)) {
$securityPacket = json_decode($securityPacket, true);
if (empty($this->secret)) {
throw new ValidationException('The `secret` argument must be a valid string');
}

if (empty($secret)) {
throw new ValidationException('The `secret` argument must be a valid string');
$this->securityPacket = $this->preHashStringGenerator->validate($this->securityPacket);
}

private function validateAndSetRequestPacket()
{
$this->requestPassedAsString = $this->isRequestNonEmptyString();
if ($this->requestPassedAsString) {
$this->decodedRequestPacket = json_decode($this->requestPacket, true);
if (!is_array($this->decodedRequestPacket)) {
throw new ValidationException('The request packet must be an array or a valid JSON string');
}
return;
}

if (!empty($requestPacket) && !is_array($requestPacket)) {
if (!empty($this->requestPacket) && !is_array($this->requestPacket)) {
throw new ValidationException('The request packet must be an array or a valid JSON string');
}
if (empty($this->requestPacket)) {
$this->requestPacket = [];
}
$this->decodedRequestPacket = $this->requestPacket;
$this->requestPacket = Json::encode($this->decodedRequestPacket);
}

return $this->preHashStringGenerator->validate($securityPacket, $requestPacket);
private function isRequestNonEmptyString(): bool
{
return is_string($this->requestPacket) && !empty($this->requestPacket);
}

/**
Expand Down
27 changes: 13 additions & 14 deletions src/Services/PreHashStrings/LegacyPreHashString.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace LearnositySdk\Services\PreHashStrings;

use LearnositySdk\Exceptions\ValidationException;
use LearnositySdk\Utils\Json;

class LegacyPreHashString implements PreHashStringInterface
{
Expand Down Expand Up @@ -41,8 +42,8 @@ class LegacyPreHashString implements PreHashStringInterface
self::SERVICE_ANNOTATIONS_API,
self::SERVICE_AUTHOR_API,
self::SERVICE_AUTHOR_AIDE_API,
self::SERVICE_ITEMS_API,
self::SERVICE_DATA_API,
self::SERVICE_EVENTS_API,
self::SERVICE_REPORTS_API,
];

Expand Down Expand Up @@ -112,7 +113,7 @@ public function __construct(string $service, bool $v1Compat = false)

public function getPreHashString(
array $security,
?array $request,
$request,
?string $action = 'get',
?string $secret = null
): string {
Expand All @@ -125,7 +126,11 @@ public function getPreHashString(
if (isset($security['expires'])) {
$signatureArray[] = $security['expires'];
}
if (!in_array($this->service, static::SERVICES_NOT_REQUIRING_USER_ID)) { // || isset($security['user_id'])) {

if (!in_array($this->service, static::SERVICES_NOT_REQUIRING_USER_ID) || isset($security['user_id'])) {
if (!isset($security['user_id'])) {
throw new ValidationException('User ID is required for this service');
}
$signatureArray[] = $security['user_id'];
}

Expand All @@ -139,17 +144,11 @@ public function getPreHashString(
}

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

if (in_array($this->service, static::SERVICES_REQUIRING_SIGNED_ACTION)) {
if (empty($action)) {
$action = 'get';
}
// Add the action if necessary
if (!empty($action)) {
$signatureArray[] = $action;
}

Expand All @@ -171,7 +170,7 @@ public static function supportsService(string $service): bool
* nothing is done with the request packet, but future pre-hash schemes
* may require it.
*/
public function validate(array $security, array $request): array
public function validate(array $security): array
{
foreach (array_keys($security) as $key) {
if (!in_array($key, static::VALID_SECURITY_KEYS)) {
Expand All @@ -195,6 +194,6 @@ public function validate(array $security, array $request): array
}
/* end XXX */

return [ $request, $security ];
return $security;
}
}
5 changes: 2 additions & 3 deletions src/Services/PreHashStrings/PreHashStringInterface.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ interface PreHashStringInterface
* @param string|null $secret only for legacy pre-hash string generation when $v1Compat is true
* @return string the pre-hash string
*/
public function getPreHashString(array $security, ?array $request, ?string $action, ?string $secret): string;
public function getPreHashString(array $security, $request, ?string $action, ?string $secret): string;

/** Return a list of all supported services
* @return string[]
Expand All @@ -26,8 +26,7 @@ public static function supportsService(string $service): bool;

/** Validate a request to be signed, and return a potentially modified request
* @param array $security
* @param array $request
* @return array <$updatedRequest, $updatedSecurity>
* */
public function validate(array $security, array $request): array;
public function validate(array $security): array;
}
Loading

0 comments on commit 518a2d3

Please sign in to comment.