Merge pull request #32 from ucfcdl/issue/31-lti-oauth-age-checking

Crazily, oauth timestamp checking is grossly incorrect, fixes #31
ucfopen · Nov 8, 2016 · d6b2556 · d6b2556
2 parents 6096be7 + dee6d64
commit d6b2556
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 28 deletions.
diff --git a/internal/classes/lti/API.class.php b/internal/classes/lti/API.class.php
@@ -51,39 +51,27 @@ public static function handleLtiLaunch()
 
 		// ================ RENDER OR REDIRECT DEPENDING ON LTI LAUNCH ROLE ================
 
-		if($ltiData->isTestUser())
-		{
-			$instanceData = static::getInstanceDataOrRenderError($instID);
-			static::initAssessmentSession($instID, $ltiData);
-			profile('lti', "'lti-launch-testuser', '$instID', '".time()."'");
-			// test user shows a special page
-			\lti\Views::renderTestUserConfirmPage($instanceData);
-			exit();
-		}
-
 		// does the lti role indicate this is an instructor?
 		// this overrides their state in the local obojobo database!
-		if($ltiData->isInstructor())
+		if($ltiData->isInstructor() || $ltiData->isTestUser())
 		{
 			$instanceData = static::getInstanceDataOrRenderError($instID);
-			$loID = $instanceData->loID;
 
 			// We want to store in some additional permissions info in
 			// the session so this gives the instructor a way to be
 			// able to view the instance in Obojobo
-			if(!empty($ltiData->username))
+			if($ltiData->isInstructor() && !empty($ltiData->username))
 			{
-				$AM = \rocketD\auth\AuthManager::getInstance();
-				$user = $AM->fetchUserByUserName($ltiData->username);
-
-				$PM = \obo\perms\PermManager::getInstance();
-				$PM->setSessionPermsForUserToItem($user->userID, \cfg_core_Perm::TYPE_INSTANCE, $instID, array(20));
+				$user = \rocketD\auth\AuthManager::getInstance()->fetchUserByUserName($ltiData->username);
+				if($user instanceof \rocketD\auth\User)
+				{
+					\obo\perms\PermManager::getInstance()->setSessionPermsForUserToItem($user->userID, \cfg_core_Perm::TYPE_INSTANCE, $instID, array(20));
+				}
 			}
 
-			// redirect to preview
-			$previewURL = \AppCfg::URL_WEB . 'preview/' . $loID;
-			profile('lti',"'lti-launch-redirect-instructor', '$previewURL', '".time()."'");
-			header('Location: ' . $previewURL);
+			profile('lti',"'lti-launch-".($ltiData->isTestUser() ? 'testuser' : 'instructor')."', '$previewURL', '".time()."'");
+			// static::initAssessmentSession($instID, $ltiData);
+			\lti\Views::renderTestUserConfirmPage($instanceData);
 			exit();
 		}
 

diff --git a/internal/classes/lti/OAuth.class.php b/internal/classes/lti/OAuth.class.php
@@ -5,11 +5,6 @@ class Exception extends \Exception {}
 
 class OAuth
 {
-	public static $ltiData;
-	public static $key;
-	public static $secret;
-	public static $timeout;
-
 
 	public static function validateLtiPassback($key, $secret, $timeout)
 	{
@@ -32,7 +27,8 @@ public static function validateLtiPassback($key, $secret, $timeout)
 	public static function validateLtiMessage($ltiData, $key, $secret, $timeout)
 	{
 		if(empty($_REQUEST['oauth_nonce'])) throw new Exception("Authorization fingerprint is missing.");
-		if($_REQUEST['oauth_timestamp'] >= (time() - \lti\OAuth::$timeout)) throw new Exception("Authorization signature is too old.");
+		// IS THE OAUTH TIMESTAMP LESS THEN THE OLDEST VALID TIMESTAMP (NOW - MAX AGE DELTA)
+		if(((int) $_REQUEST['oauth_timestamp']) < (time() - $timeout)) throw new Exception("Authorization signature is too old.");
 		if($_REQUEST['oauth_consumer_key'] !== $key) throw new Exception("Authorization signature failure.");
 
 		// OK, so OAUTH IS FUN