Skip to content

Commit

Permalink
Code review
Browse files Browse the repository at this point in the history
  • Loading branch information
@spvickers
spvickers committed Nov 25, 2020
1 parent 0c13966 commit 91e61a7
Show file tree
Hide file tree
Showing 12 changed files with 42 additions and 33 deletions.
2 changes: 2 additions & 0 deletions src/ApiHook/ApiContext.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

namespace ceLTIc\LTI\ApiHook;

use ceLTIc\LTI\Service;

/**
* Class to implement context services for a platform via its proprietary API
*
Expand Down
2 changes: 2 additions & 0 deletions src/ApiHook/ApiResourceLink.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

namespace ceLTIc\LTI\ApiHook;

use ceLTIc\LTI\Service;

/**
* Class to implement resource link services for a platform via its proprietary API
*
Expand Down
7 changes: 7 additions & 0 deletions src/Content/LtiLinkItem.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,13 @@ class LtiLinkItem extends Item
*/
private $available = null;

/**
* Time period for submission.
*
* @var string|null $submission
*/
private $submission = null;

/**
* Class constructor.
*
Expand Down
8 changes: 0 additions & 8 deletions src/Context.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -563,19 +563,11 @@ public function hasResultService()
public function getLineItems($resourceId = null, $tag = null, $limit = null)
{
$lineItems = false;
$this->extRequest = '';
$this->extRequestHeaders = '';
$this->extResponse = '';
$this->extResponseHeaders = '';
$this->lastServiceRequest = null;
$lineItemService = $this->getLineItemService();
if (!empty($lineItemService)) {
$lineItems = $lineItemService->getAll(null, $resourceId, $tag);
$http = $lineItemService->getHttpMessage();
$this->extResponse = $http->response;
$this->extResponseHeaders = $http->responseHeaders;
$this->extRequest = $http->request;
$this->extRequestHeaders = $http->requestHeaders;
$this->lastServiceRequest = $http;
}

Expand Down
4 changes: 4 additions & 0 deletions src/DataConnector/DataConnector_oci.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ public function savePlatform($platform)
$last = date($this->dateFormat, $platform->lastAccess);
}
if (empty($id)) {
$pk = null;
$sql = "INSERT INTO {$this->dbTableNamePrefix}" . static::PLATFORM_TABLE_NAME . ' (consumer_key, name, secret, ' .
'platform_id, client_id, deployment_id, public_key, ' .
'lti_version, signature_method, consumer_name, consumer_version, consumer_guid, ' .
Expand Down Expand Up @@ -547,6 +548,7 @@ public function saveContext($context)
$id = $context->getRecordId();
$consumer_pk = $context->getPlatform()->getRecordId();
if (empty($id)) {
$pk = null;
$sql = "INSERT INTO {$this->dbTableNamePrefix}" . static::CONTEXT_TABLE_NAME . ' (consumer_pk, title, ' .
'lti_context_id, type, settings, created, updated) ' .
'VALUES (:cid, :title, :ctx, :type, :settings, :created, :updated) returning context_pk into :pk';
Expand Down Expand Up @@ -772,6 +774,7 @@ public function saveResourceLink($resourceLink)
}
$id = $resourceLink->getRecordId();
if (empty($id)) {
$pk = null;
$sql = "INSERT INTO {$this->dbTableNamePrefix}" . static::RESOURCE_LINK_TABLE_NAME . ' (consumer_pk, context_pk, ' .
'lti_resource_link_id, settings, primary_resource_link_pk, share_approved, created, updated) ' .
'VALUES (:cid, :ctx, :rlid, :settings, :prlid, :share_approved, :created, :updated) returning resource_link_pk into :pk';
Expand Down Expand Up @@ -1294,6 +1297,7 @@ public function saveUserResult($userresult)
$time = time();
$now = date("{$this->dateFormat} {$this->timeFormat}", $time);
if (is_null($userresult->created)) {
$pk = null;
$sql = "INSERT INTO {$this->dbTableNamePrefix}" . static::USER_RESULT_TABLE_NAME . ' (resource_link_pk, ' .
'lti_user_id, lti_result_sourcedid, created, updated) ' .
'VALUES (:rlid, :u_id, :sourcedid, :created, :updated) returning user_result_pk into :pk';
Expand Down
2 changes: 1 addition & 1 deletion src/Jwt/WebTokenClient.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ public function getJweHeaders()
*/
public function hasHeader($name)
{
if ($this->jwt instanceof JWS) {
if ($this->jwt instanceof Signature\JWS) {
$ok = $this->jwt->getSignature(0)->hasProtectedHeaderParameter($name);
} else {
$ok = false;
Expand Down
1 change: 1 addition & 0 deletions src/OAuth/OAuthConsumer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ class OAuthConsumer

public $key;
public $secret;
public $callback_url;

function __construct($key, $secret, $callback_url = null)
{
Expand Down
13 changes: 12 additions & 1 deletion src/Platform.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,13 @@ class Platform
*/
public $profile = null;

/**
* The tool proxy.
*
* @var object|null $toolPrixy
*/
public $toolProxy = null;

/**
* Platform GUID (as reported by first platform connection).
*
Expand Down Expand Up @@ -360,7 +367,11 @@ public function getFamilyCode()
{
$familyCode = '';
if (!empty($this->consumerVersion)) {
list($familyCode, $version) = explode('-', $this->consumerVersion, 2);
$familyCode = $this->consumerVersion;
$pos = strpos('-', $familyCode);
if ($pos !== false) {
$familyCode = substr($familyCode, 0, $pos - 1);
}
}

return $familyCode;
Expand Down
4 changes: 2 additions & 2 deletions src/Service/LineItem.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,10 +183,10 @@ public function deleteLineItem($lineItem)
public static function getLineItem($platform, $endpoint)
{
$service = new self($platform, $endpoint);
$this->scope = self::$SCOPE_READONLY;
$service->scope = self::$SCOPE_READONLY;
$service->mediaType = self::$MEDIA_TYPE_LINE_ITEM;
$http = $service->send('GET');
$this->scope = self::$SCOPE;
$service->scope = self::$SCOPE;
if ($http->ok && !empty($http->responseJson)) {
$lineItem = self::toLineItem($platform, $http->responseJson);
} else {
Expand Down
1 change: 1 addition & 0 deletions src/Service/Service.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
use ceLTIc\LTI\Platform;
use ceLTIc\LTI\ToolConsumer;
use ceLTIc\LTI\Http\HttpMessage;
use ceLTIc\LTI\Util;

/**
* Class to implement a service
Expand Down
17 changes: 9 additions & 8 deletions src/System.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use ceLTIc\LTI\DataConnector\DataConnector;
use ceLTIc\LTI\Content\Item;
use ceLTIc\LTI\Http\HttpMessage;
use ceLTIc\LTI\OAuth;
use ceLTIc\LTI\Jwt\Jwt;
use ceLTIc\LTI\Jwt\ClientInterface;
Expand Down Expand Up @@ -441,9 +442,9 @@ public function verifySignature()
} elseif (($this instanceof Tool) && !empty($this->platform)) {
$key = $this->platform->getKey();
$secret = $this->platform->secret;
} elseif (($this instanceof Platform) && !empty($Tool::$defaultTool)) {
$key = $Tool::$defaultTool->getKey();
$secret = $Tool::$defaultTool->secret;
} elseif (($this instanceof Platform) && !empty(Tool::$defaultTool)) {
$key = Tool::$defaultTool->getKey();
$secret = Tool::$defaultTool->secret;
}
if ($this instanceof Tool) {
$platform = $this->platform;
Expand Down Expand Up @@ -571,7 +572,7 @@ private function parseMessage()
$this->reason = 'aud claim does not match the azp claim';
}
}
if ($this->ok) {
if ($this->ok && ($this instanceof Tool)) {
$this->platform = Platform::fromPlatformId($iss, $aud, $deploymentId, $this->dataConnector);
if (isset($this->rawParameters['id_token'])) {
$this->ok = !empty($this->rawParameters['state']);
Expand Down Expand Up @@ -605,7 +606,7 @@ private function parseMessage()
$this->ok = false;
$this->reason = $this->rawParameters['error'];
} else { // OAuth
if (isset($this->rawParameters['oauth_consumer_key'])) {
if (isset($this->rawParameters['oauth_consumer_key']) && ($this instanceof Tool)) {
$this->platform = Platform::fromConsumerKey($this->rawParameters['oauth_consumer_key'], $this->dataConnector);
}
$this->messageParameters = $this->rawParameters;
Expand Down Expand Up @@ -783,9 +784,9 @@ private function addOAuth1Signature($endpoint, $data, $method, $type, $hash, $ti
} elseif (($this instanceof Tool) && !empty($this->platform)) {
$key = $this->platform->getKey();
$secret = $this->platform->secret;
} elseif (($this instanceof Platform) && !empty($Tool::$defaultTool)) {
$key = $Tool::$defaultTool->getKey();
$secret = $Tool::$defaultTool->secret;
} elseif (($this instanceof Platform) && !empty(Tool::$defaultTool)) {
$key = Tool::$defaultTool->getKey();
$secret = Tool::$defaultTool->secret;
}
$oauthConsumer = new OAuth\OAuthConsumer($key, $secret, null);
$oauthReq = OAuth\OAuthRequest::from_consumer_and_token($oauthConsumer, null, $method, $endpoint, $params);
Expand Down
14 changes: 1 addition & 13 deletions src/Tool.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -510,8 +510,6 @@ public static function sendForm($url, $params, $target = '')

/**
* Process a valid launch request
*
* @return bool True if no error
*/
protected function onLaunch()
{
Expand All @@ -520,8 +518,6 @@ protected function onLaunch()

/**
* Process a valid configure request
*
* @return bool True if no error
*/
protected function onConfigure()
{
Expand All @@ -530,8 +526,6 @@ protected function onConfigure()

/**
* Process a valid dashboard request
*
* @return bool True if no error
*/
protected function onDashboard()
{
Expand All @@ -540,8 +534,6 @@ protected function onDashboard()

/**
* Process a valid content-item request
*
* @return bool True if no error
*/
protected function onContentItem()
{
Expand All @@ -550,8 +542,6 @@ protected function onContentItem()

/**
* Process a valid tool proxy registration request
*
* @return bool True if no error
*/
protected function onRegister()
{
Expand All @@ -560,8 +550,6 @@ protected function onRegister()

/**
* Process a response to an invalid request
*
* @return bool True if no further error processing required
*/
protected function onError()
{
Expand Down Expand Up @@ -1391,13 +1379,13 @@ private function sendAuthenticationRequest($hint)
if (!$ok) {
$this->reason = 'Platform not found or no platform authentication request URL.';
} else {
$oauthRequest = OAuth\OAuthRequest::from_request();
do {
$nonce = new PlatformNonce($this->platform, Util::getRandomString());
$ok = !$nonce->load();
} while (!$ok);
$ok = $nonce->save();
if ($ok) {
$oauthRequest = OAuth\OAuthRequest::from_request();
$redirectUri = $oauthRequest->get_normalized_http_url();
if (!empty($_SERVER['QUERY_STRING'])) {
if ($_SERVER['REQUEST_METHOD'] === 'POST') {
Expand Down

0 comments on commit 91e61a7

Please sign in to comment.