Add more Cookie tests (create new StaticCookie class for mocking setcookie())

src/App/Authentication.php

@@ -75,7 +75,7 @@ class Authentication
 		$data = $this->cookie->getData();
 
 		// When the "Friendica" cookie is set, take the value to authenticate and renew the cookie.
-		if (isset($data) && isset($data->uid)) {
+		if (isset($data->uid)) {
 
 			$user = $this->dba->selectFirst(
 				'user',

src/Model/User/Cookie.php

@@ -14,6 +14,12 @@ class Cookie
 	const DEFAULT_EXPIRE = 7;
 	/** @var string The name of the Friendica cookie */
 	const NAME = 'Friendica';
+	/** @var string The path of the Friendica cookie */
+	const PATH = '/';
+	/** @var string The domain name of the Friendica cookie */
+	const DOMAIN = '';
+	/** @var bool True, if the cookie should only be accessable through HTTP */
+	const HTTPONLY = true;
 
 	/** @var string The remote address of this node */
 	private $remoteAddr = '0.0.0.0';
@@ -72,7 +78,7 @@ class Cookie
 	public function set(int $uid, string $password, string $privateKey, int $seconds = null)
 	{
 		if (!isset($seconds)) {
-			$seconds = $this->lifetime;
+			$seconds = $this->lifetime + time();
 		} elseif (isset($seconds) && $seconds != 0) {
 			$seconds = $seconds + time();
 		}
@@ -83,8 +89,7 @@ class Cookie
 			'ip'   => $this->remoteAddr,
 		]);
 
-		return $this->setCookie(self::NAME, $value, $seconds,
+		return $this->setCookie(self::NAME, $value, $seconds, $this->sslEnabled);
-			'/', '', $this->sslEnabled, true);
 	}
 
 	/**
@@ -111,8 +116,7 @@ class Cookie
 	public function clear()
 	{
 		// make sure cookie is deleted on browser close, as a security measure
-		return $this->setCookie(self::NAME, '', -3600,
+		return $this->setCookie(self::NAME, '', -3600, $this->sslEnabled);
-			'/', '', $this->sslEnabled, true);
 	}
 
 	/**
@@ -140,18 +144,14 @@ class Cookie
 	 * @param string $name
 	 * @param string $value    [optional]
 	 * @param int    $expire   [optional]
-	 * @param string $path     [optional]
-	 * @param string $domain   [optional]
 	 * @param bool   $secure   [optional]
-	 * @param bool   $httponly [optional]
 	 *
 	 * @return bool If output exists prior to calling this function,
 	 *
 	 */
 	protected function setCookie(string $name, string $value = null, int $expire = null,
-	                             string $path = null, string $domain = null,
+	                             bool $secure = null)
-	                             bool $secure = null, bool $httponly = null)
 	{
-		return setcookie($name, $value, $expire, $path, $domain, $secure, $httponly);
+		return setcookie($name, $value, $expire, self::PATH, self::DOMAIN, $secure, self::HTTPONLY);
 	}
 }

tests/Util/StaticCookie.php

@@ -0,0 +1,28 @@
+<?php
+
+namespace Friendica\Test\Util;
+
+use Friendica\Model\User\Cookie;
+
+/**
+ * Overrides the Cookie class so all cookie information will be saved to a static public variable
+ */
+class StaticCookie extends Cookie
+{
+	/** @var array static Cookie array mock */
+	public static $_COOKIE = [];
+	/** @var int The last expire time set */
+	public static $_EXPIRE;
+
+	protected function setCookie(string $name, string $value = null, int $expire = null, bool $secure = null)
+	{
+		self::$_COOKIE[$name] = $value;
+		self::$_EXPIRE = $expire;
+	}
+
+	public static function clearStatic()
+	{
+		self::$_EXPIRE = null;
+		self::$_COOKIE = [];
+	}
+}

tests/src/Model/User/CookieTest.php

@@ -5,6 +5,7 @@ namespace Friendica\Testsrc\Model\User;
 use Friendica\Core\Config\Configuration;
 use Friendica\Model\User\Cookie;
 use Friendica\Test\DatabaseTest;
+use Friendica\Test\Util\StaticCookie;
 use Mockery\MockInterface;
 
 class CookieTest extends DatabaseTest
@@ -14,11 +15,21 @@ class CookieTest extends DatabaseTest
 
 	protected function setUp()
 	{
-		parent::setUp();;
+		StaticCookie::clearStatic();
+
+		parent::setUp();
 
 		$this->config = \Mockery::mock(Configuration::class);
 	}
 
+	protected function tearDown()
+	{
+		StaticCookie::clearStatic();
+	}
+
+	/**
+	 * Test if we can create a basic cookie instance
+	 */
 	public function testInstance()
 	{
 		$this->config->shouldReceive('get')->with('system', 'ssl_policy')->andReturn(1)->once();
@@ -79,6 +90,8 @@ class CookieTest extends DatabaseTest
 	}
 
 	/**
+	 * Test the get() method of the cookie class
+	 *
 	 * @dataProvider dataGet
 	 */
 	public function testGet(array $cookieData, bool $hasValues, $uid, $hash, $ip)
@@ -134,7 +147,7 @@ class CookieTest extends DatabaseTest
 				'assertHash'       => '',
 				'assertTrue'       => false,
 			],
-			'invalid' => [
+			'invalid'   => [
 				'serverPrivateKey' => 'serverkey',
 				'userPrivateKey'   => 'bla',
 				'password'         => 'nope',
@@ -145,6 +158,8 @@ class CookieTest extends DatabaseTest
 	}
 
 	/**
+	 * Test the check() method of the cookie class
+	 *
 	 * @dataProvider dataCheck
 	 */
 	public function testCheck(string $serverPrivateKey, string $userPrivateKey, string $password, string $assertHash, bool $assertTrue)
@@ -159,13 +174,135 @@ class CookieTest extends DatabaseTest
 		$this->assertEquals($assertTrue, $cookie->check($assertHash, $password, $userPrivateKey));
 	}
 
-	public function testSet()
+	public function dataSet()
 	{
-		$this->markTestIncomplete('Needs mocking of setcookie() first.');
+		return [
+			'default'         => [
+				'serverKey'   => 23,
+				'uid'         => 0,
+				'password'    => '234',
+				'privateKey'  => '124',
+				'assertHash'  => 'b657a15cfe7ed1f7289c9aa51af14a9a26c966f4ddd74e495fba103d8e872a39',
+				'remoteIp'    => '0.0.0.0',
+				'serverArray' => [],
+				'lifetime'    => null,
+			],
+			'withServerArray' => [
+				'serverKey'   => 23,
+				'uid'         => 0,
+				'password'    => '234',
+				'privateKey'  => '124',
+				'assertHash'  => 'b657a15cfe7ed1f7289c9aa51af14a9a26c966f4ddd74e495fba103d8e872a39',
+				'remoteIp'    => '1.2.3.4',
+				'serverArray' => ['REMOTE_ADDR' => '1.2.3.4',],
+				'lifetime'    => null,
+			],
+			'withLifetime0'   => [
+				'serverKey'   => 23,
+				'uid'         => 0,
+				'password'    => '234',
+				'privateKey'  => '124',
+				'assertHash'  => 'b657a15cfe7ed1f7289c9aa51af14a9a26c966f4ddd74e495fba103d8e872a39',
+				'remoteIp'    => '1.2.3.4',
+				'serverArray' => ['REMOTE_ADDR' => '1.2.3.4',],
+				'lifetime'    => 0,
+			],
+			'withLifetime'     => [
+				'serverKey'   => 23,
+				'uid'         => 0,
+				'password'    => '234',
+				'privateKey'  => '124',
+				'assertHash'  => 'b657a15cfe7ed1f7289c9aa51af14a9a26c966f4ddd74e495fba103d8e872a39',
+				'remoteIp'    => '1.2.3.4',
+				'serverArray' => ['REMOTE_ADDR' => '1.2.3.4',],
+				'lifetime'    => 2 * 24 * 60 * 60,
+			],
+		];
 	}
 
+	public function assertCookie($uid, $hash, $remoteIp, $lifetime)
+	{
+		$this->assertArrayHasKey(Cookie::NAME, StaticCookie::$_COOKIE);
+
+		$data = json_decode(StaticCookie::$_COOKIE[Cookie::NAME]);
+
+		$this->assertObjectHasAttribute('uid', $data);
+		$this->assertEquals($uid, $data->uid);
+		$this->assertObjectHasAttribute('hash', $data);
+		$this->assertEquals($hash, $data->hash);
+		$this->assertObjectHasAttribute('ip', $data);
+		$this->assertEquals($remoteIp, $data->ip);
+
+		if (isset($lifetime) && $lifetime !== 0) {
+			$this->assertLessThanOrEqual(time() + $lifetime, StaticCookie::$_EXPIRE);
+		} else {
+			$this->assertLessThanOrEqual(time() + Cookie::DEFAULT_EXPIRE * 24 * 60 * 60, StaticCookie::$_EXPIRE);
+		}
+	}
+
+	/**
+	 * Test the set() method of the cookie class
+	 *
+	 * @dataProvider dataSet
+	 */
+	public function testSet($serverKey, $uid, $password, $privateKey, $assertHash, $remoteIp, $serverArray, $lifetime)
+	{
+		$this->config->shouldReceive('get')->with('system', 'ssl_policy')->andReturn(1)->once();
+		$this->config->shouldReceive('get')->with('system', 'site_prvkey')->andReturn($serverKey)->once();
+		$this->config->shouldReceive('get')->with('system', 'auth_cookie_lifetime', Cookie::DEFAULT_EXPIRE)->andReturn(Cookie::DEFAULT_EXPIRE)->once();
+
+		$cookie = new StaticCookie($this->config, $serverArray);
+		$this->assertInstanceOf(Cookie::class, $cookie);
+
+		$cookie->set($uid, $password, $privateKey, $lifetime);
+
+		$this->assertCookie($uid, $assertHash, $remoteIp, $lifetime);
+	}
+
+	/**
+	 * Test two different set() of the cookie class (first set is invalid)
+	 *
+	 * @dataProvider dataSet
+	 */
+	public function testDoubleSet($serverKey, $uid, $password, $privateKey, $assertHash, $remoteIp, $serverArray, $lifetime)
+	{
+		$this->config->shouldReceive('get')->with('system', 'ssl_policy')->andReturn(1)->once();
+		$this->config->shouldReceive('get')->with('system', 'site_prvkey')->andReturn($serverKey)->once();
+		$this->config->shouldReceive('get')->with('system', 'auth_cookie_lifetime', Cookie::DEFAULT_EXPIRE)->andReturn(Cookie::DEFAULT_EXPIRE)->once();
+
+		$cookie = new StaticCookie($this->config, $serverArray);
+		$this->assertInstanceOf(Cookie::class, $cookie);
+
+		// Invalid set, should get overwritten
+		$cookie->set(-1, 'invalid', 'nothing', -234);
+
+		$cookie->set($uid, $password, $privateKey, $lifetime);
+
+		$this->assertCookie($uid, $assertHash, $remoteIp, $lifetime);
+	}
+
+	/**
+	 * Test the clear() method of the cookie class
+	 */
 	public function testClear()
 	{
-		$this->markTestIncomplete('Needs mocking of setcookie() first.');
+		StaticCookie::$_COOKIE = [
+			Cookie::NAME => 'test'
+		];
+
+		$this->config->shouldReceive('get')->with('system', 'ssl_policy')->andReturn(1)->once();
+		$this->config->shouldReceive('get')->with('system', 'site_prvkey')->andReturn(24)->once();
+		$this->config->shouldReceive('get')->with('system', 'auth_cookie_lifetime', Cookie::DEFAULT_EXPIRE)->andReturn(Cookie::DEFAULT_EXPIRE)->once();
+
+		$cookie = new StaticCookie($this->config, []);
+		$this->assertInstanceOf(Cookie::class, $cookie);
+
+		$this->assertEquals('test', StaticCookie::$_COOKIE[Cookie::NAME]);
+		$this->assertEquals(null, StaticCookie::$_EXPIRE);
+
+		$cookie->clear();
+
+		$this->assertEmpty(StaticCookie::$_COOKIE[Cookie::NAME]);
+		$this->assertEquals(-3600, StaticCookie::$_EXPIRE);
 	}
 }