Skip to content

Commit d450655

Browse files
spvickersspvickers
committed
Add warning messages and third-party cookie check
1 parent 7b4d5dc commit d450655

File tree

6 files changed

+447
-196
lines changed

6 files changed

+447
-196
lines changed

‎src/ApiHook/canvas/CanvasApiTool.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ class CanvasApiTool extends \ceLTIc\LTI\ApiHook\ApiTool
1515
public function getUserId()
1616
{
1717
$userId = '';
18-
$messageParameters = $this->tool->getMessageParameters();
18+
$messageParameters = $this->tool->getMessageParameters(true, true, false);
1919
if (isset($messageParameters['custom_canvas_user_id'])) {
2020
$userId = trim($messageParameters['custom_canvas_user_id']);
2121
}

‎src/Platform.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -430,7 +430,7 @@ public function hasAccessTokenService()
430430
public function getMessageParameters()
431431
{
432432
if ($this->ok && is_null($this->messageParameters)) {
433-
$this->parseMessage();
433+
$this->parseMessage(true, true, false);
434434
}
435435

436436
return $this->messageParameters;
@@ -637,7 +637,7 @@ protected function onError()
637637
*/
638638
private function authenticate()
639639
{
640-
$this->checkMessage();
640+
$this->ok = $this->checkMessage();
641641
if ($this->ok) {
642642
$this->ok = $this->verifySignature();
643643
}

‎src/System.php

Lines changed: 79 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -748,11 +748,11 @@ public function addSignature($endpoint, $data, $method = 'POST', $type = null, $
748748
*/
749749
public function checkMessage()
750750
{
751-
$this->ok = $_SERVER['REQUEST_METHOD'] === 'POST';
752-
if (!$this->ok) {
751+
$ok = $_SERVER['REQUEST_METHOD'] === 'POST';
752+
if (!$ok) {
753753
$this->reason = 'LTI messages must use HTTP POST';
754754
} elseif (!empty($this->jwt) && !empty($this->jwt->hasJwt())) {
755-
$this->ok = false;
755+
$ok = false;
756756
if (is_null($this->messageParameters['oauth_consumer_key']) || (strlen($this->messageParameters['oauth_consumer_key']) <= 0)) {
757757
$this->reason = 'Missing iss claim';
758758
} elseif (empty($this->jwt->getClaim('iat', ''))) {
@@ -764,7 +764,7 @@ public function checkMessage()
764764
} elseif (empty($this->jwt->getClaim('nonce', ''))) {
765765
$this->reason = 'Missing nonce claim';
766766
} else {
767-
$this->ok = true;
767+
$ok = true;
768768
}
769769
}
770770
// Set signature method from request
@@ -775,21 +775,21 @@ public function checkMessage()
775775
}
776776
}
777777
// Check all required launch parameters
778-
if ($this->ok) {
779-
$this->ok = isset($this->messageParameters['lti_message_type']);
780-
if (!$this->ok) {
778+
if ($ok) {
779+
$ok = isset($this->messageParameters['lti_message_type']);
780+
if (!$ok) {
781781
$this->reason = 'Missing lti_message_type parameter.';
782782
}
783783
}
784-
if ($this->ok) {
785-
$this->ok = isset($this->messageParameters['lti_version']) && in_array($this->messageParameters['lti_version'],
784+
if ($ok) {
785+
$ok = isset($this->messageParameters['lti_version']) && in_array($this->messageParameters['lti_version'],
786786
Util::$LTI_VERSIONS);
787-
if (!$this->ok) {
787+
if (!$ok) {
788788
$this->reason = 'Invalid or missing lti_version parameter.';
789789
}
790790
}
791791

792-
return $this->ok;
792+
return $ok;
793793
}
794794

795795
/**
@@ -839,6 +839,9 @@ public function verifySignature()
839839
$method = new OAuth\OAuthSignatureMethod_HMAC_SHA1();
840840
$server->add_signature_method($method);
841841
$request = OAuth\OAuthRequest::from_request();
842+
if (isset($request->get_parameters()['_new_window']) && !isset($this->messageParameters['_new_window'])) {
843+
$request->unset_parameter('_new_window');
844+
}
842845
$server->verify_request($request);
843846
$ok = true;
844847
} catch (\Exception $e) {
@@ -884,9 +887,13 @@ public function verifySignature()
884887
###
885888

886889
/**
887-
* Parse the message
890+
* Parse the message.
891+
*
892+
* @param bool $strictMode True if full compliance with the LTI specification is required
893+
* @param bool $disableCookieCheck True if no cookie check should be made
894+
* @param bool $generateWarnings True if warning messages should be generated
888895
*/
889-
private function parseMessage()
896+
private function parseMessage($strictMode, $disableCookieCheck, $generateWarnings)
890897
{
891898
if (is_null($this->messageParameters)) {
892899
$this->getRawParameters();
@@ -943,16 +950,32 @@ private function parseMessage()
943950
if (isset($this->rawParameters['id_token'])) {
944951
$this->ok = !empty($this->rawParameters['state']);
945952
if ($this->ok) {
946-
$nonce = new PlatformNonce($this->platform, $this->rawParameters['state']);
953+
$state = $this->rawParameters['state'];
954+
if (!$disableCookieCheck) {
955+
$parts = explode('.', $state);
956+
if (empty($_COOKIE) && !isset($_POST['_new_window'])) { // Reopen in a new window
957+
Util::setTestCookie();
958+
$_POST['_new_window'] = '';
959+
echo Util::sendForm($_SERVER['REQUEST_URI'], $_POST, '_blank');
960+
exit;
961+
} elseif (!empty(session_id()) && (count($parts) > 1) && (session_id() !== $parts[1])) { // Reset to original session
962+
session_abort();
963+
session_id($parts[1]);
964+
session_start();
965+
$this->onResetSessionId();
966+
}
967+
Util::setTestCookie(true);
968+
}
969+
$nonce = new PlatformNonce($this->platform, $state);
947970
$this->ok = $nonce->load();
948971
if (!$this->ok) {
949972
$platform = Platform::fromPlatformId($iss, $aud, null, $this->dataConnector);
950-
$nonce = new PlatformNonce($platform, $this->rawParameters['state']);
973+
$nonce = new PlatformNonce($platform, $state);
951974
$this->ok = $nonce->load();
952975
}
953976
if (!$this->ok) {
954977
$platform = Platform::fromPlatformId($iss, null, null, $this->dataConnector);
955-
$nonce = new PlatformNonce($platform, $this->rawParameters['state']);
978+
$nonce = new PlatformNonce($platform, $state);
956979
$this->ok = $nonce->load();
957980
}
958981
if ($this->ok) {
@@ -965,7 +988,7 @@ private function parseMessage()
965988
$this->messageParameters = array();
966989
$this->messageParameters['oauth_consumer_key'] = $aud;
967990
$this->messageParameters['oauth_signature_method'] = $this->jwt->getHeader('alg');
968-
$this->parseClaims();
991+
$this->parseClaims($strictMode, $generateWarnings);
969992
} else {
970993
$this->reason = 'state parameter is invalid or missing';
971994
}
@@ -985,8 +1008,34 @@ private function parseMessage()
9851008
$this->reason .= ": {$this->rawParameters['error_description']}";
9861009
}
9871010
} else { // OAuth
988-
if (isset($this->rawParameters['oauth_consumer_key']) && ($this instanceof Tool)) {
989-
$this->platform = Platform::fromConsumerKey($this->rawParameters['oauth_consumer_key'], $this->dataConnector);
1011+
if ($this instanceof Tool) {
1012+
if (isset($this->rawParameters['oauth_consumer_key'])) {
1013+
$this->platform = Platform::fromConsumerKey($this->rawParameters['oauth_consumer_key'], $this->dataConnector);
1014+
}
1015+
if (isset($this->rawParameters['tool_state'])) { // Relaunch?
1016+
$state = $this->rawParameters['tool_state'];
1017+
if (!$disableCookieCheck) {
1018+
$parts = explode('.', $state);
1019+
if (empty($_COOKIE) && !isset($_POST['_new_window'])) { // Reopen in a new window
1020+
Util::setTestCookie();
1021+
$_POST['_new_window'] = '';
1022+
echo Util::sendForm($_SERVER['REQUEST_URI'], $_POST, '_blank');
1023+
exit;
1024+
} elseif (!empty(session_id()) && (count($parts) > 1) && (session_id() !== $parts[1])) { // Reset to original session
1025+
session_abort();
1026+
session_id($parts[1]);
1027+
session_start();
1028+
$this->onResetSessionId();
1029+
}
1030+
unset($this->rawParameters['_new_window']);
1031+
Util::setTestCookie(true);
1032+
}
1033+
$nonce = new PlatformNonce($this->platform, $state);
1034+
$this->ok = $nonce->load();
1035+
if (!$this->ok) {
1036+
$this->reason = "Invalid tool_state parameter: '{$state}'";
1037+
}
1038+
}
9901039
}
9911040
$this->messageParameters = $this->rawParameters;
9921041
}
@@ -995,8 +1044,11 @@ private function parseMessage()
9951044

9961045
/**
9971046
* Parse the claims
1047+
*
1048+
* @param bool $strictMode True if full compliance with the LTI specification is required
1049+
* @param bool $generateWarnings True if warning messages should be generated
9981050
*/
999-
private function parseClaims()
1051+
private function parseClaims($strictMode, $generateWarnings)
10001052
{
10011053
$payload = Util::cloneObject($this->jwt->getPayload());
10021054
$errors = array();
@@ -1041,6 +1093,13 @@ private function parseClaims()
10411093
$value = $value ? 'true' : 'false';
10421094
} elseif (isset($mapping['isInteger']) && $mapping['isInteger']) {
10431095
$value = strval($value);
1096+
} elseif (!is_string($value)) {
1097+
if ($generateWarnings) {
1098+
$this->warnings[] = "Value of claim '{$claim}' is not a string: '{$value}'";
1099+
}
1100+
if (!$strictMode) {
1101+
$value = strval($value);
1102+
}
10441103
}
10451104
}
10461105
if (!is_null($value) && is_string($value)) {

0 commit comments

Comments
 (0)