Use StringUtils::equals on CSRF token and add unit tests

2 files changed, 99 insertions, 5 deletions

diff --git a/lib/private/appframework/http/request.php b/lib/private/appframework/http/request.php
index baf2f0c4745..6f108fedc6d 100644
--- a/lib/private/appframework/http/request.php
+++ b/lib/private/appframework/http/request.php
@@ -33,6 +33,7 @@ use OC\Security\TrustedDomainHelper;
 use OCP\IConfig;
 use OCP\IRequest;
 use OCP\Security\ISecureRandom;
+use OCP\Security\StringUtils;
 
 /**
  * Class for accessing variables in the request.
@@ -416,12 +417,10 @@ class Request implements \ArrayAccess, \Countable, IRequest {
 		}
 
 		// Check if the token is valid
-		if($token !== $this->items['requesttoken']) {
-			// Not valid
-			return false;
-		} else {
-			// Valid token
+		if(StringUtils::equals($token, $this->items['requesttoken'])) {
 			return true;
+		} else {
+			return false;
 		}
 	}
 
diff --git a/tests/lib/appframework/http/RequestTest.php b/tests/lib/appframework/http/RequestTest.php
index 6e86f3d7041..10a9e486c97 100644
--- a/tests/lib/appframework/http/RequestTest.php
+++ b/tests/lib/appframework/http/RequestTest.php
@@ -1156,4 +1156,99 @@ class RequestTest extends \Test\TestCase {
 		$this->assertSame($expectedUri, $request->getRequestUri());
 	}
 
+	public function testPassesCSRFCheckWithGet() {
+		/** @var Request $request */
+		$request = $this->getMockBuilder('\OC\AppFramework\Http\Request')
+			->setMethods(['getScriptName'])
+			->setConstructorArgs([
+				[
+					'get' => [
+						'requesttoken' => 'MyStoredRequestToken',
+					],
+					'requesttoken' => 'MyStoredRequestToken',
+				],
+				$this->secureRandom,
+				$this->config,
+				$this->stream
+			])
+			->getMock();
+
+		$this->assertTrue($request->passesCSRFCheck());
+	}
+
+	public function testPassesCSRFCheckWithPost() {
+		/** @var Request $request */
+		$request = $this->getMockBuilder('\OC\AppFramework\Http\Request')
+			->setMethods(['getScriptName'])
+			->setConstructorArgs([
+				[
+					'post' => [
+						'requesttoken' => 'MyStoredRequestToken',
+					],
+					'requesttoken' => 'MyStoredRequestToken',
+				],
+				$this->secureRandom,
+				$this->config,
+				$this->stream
+			])
+			->getMock();
+
+		$this->assertTrue($request->passesCSRFCheck());
+	}
+
+	public function testPassesCSRFCheckWithHeader() {
+		/** @var Request $request */
+		$request = $this->getMockBuilder('\OC\AppFramework\Http\Request')
+			->setMethods(['getScriptName'])
+			->setConstructorArgs([
+				[
+					'server' => [
+						'HTTP_REQUESTTOKEN' => 'MyStoredRequestToken',
+					],
+					'requesttoken' => 'MyStoredRequestToken',
+				],
+				$this->secureRandom,
+				$this->config,
+				$this->stream
+			])
+			->getMock();
+
+		$this->assertTrue($request->passesCSRFCheck());
+	}
+
+	public function testPassesCSRFCheckWithInvalidToken() {
+		/** @var Request $request */
+		$request = $this->getMockBuilder('\OC\AppFramework\Http\Request')
+			->setMethods(['getScriptName'])
+			->setConstructorArgs([
+				[
+					'server' => [
+						'HTTP_REQUESTTOKEN' => 'MyInvalidSentToken',
+					],
+					'requesttoken' => 'MyStoredRequestToken',
+				],
+				$this->secureRandom,
+				$this->config,
+				$this->stream
+			])
+			->getMock();
+
+		$this->assertFalse($request->passesCSRFCheck());
+	}
+
+	public function testPassesCSRFCheckWithoutTokenFail() {
+		/** @var Request $request */
+		$request = $this->getMockBuilder('\OC\AppFramework\Http\Request')
+			->setMethods(['getScriptName'])
+			->setConstructorArgs([
+				[],
+				$this->secureRandom,
+				$this->config,
+				$this->stream
+			])
+			->getMock();
+
+		$this->assertFalse($request->passesCSRFCheck());
+	}
+
 }