diff --git a/src/App/Authentication.php b/src/App/Authentication.php index 99231cb77..bf62cf8a4 100644 --- a/src/App/Authentication.php +++ b/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', diff --git a/src/Model/User/Cookie.php b/src/Model/User/Cookie.php index b6e20dfed..79882d641 100644 --- a/src/Model/User/Cookie.php +++ b/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, - '/', '', $this->sslEnabled, true); + return $this->setCookie(self::NAME, $value, $seconds, $this->sslEnabled); } /** @@ -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, - '/', '', $this->sslEnabled, true); + return $this->setCookie(self::NAME, '', -3600, $this->sslEnabled); } /** @@ -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 $httponly = null) + bool $secure = null) { - return setcookie($name, $value, $expire, $path, $domain, $secure, $httponly); + return setcookie($name, $value, $expire, self::PATH, self::DOMAIN, $secure, self::HTTPONLY); } } diff --git a/tests/Util/StaticCookie.php b/tests/Util/StaticCookie.php new file mode 100644 index 000000000..01a8c49be --- /dev/null +++ b/tests/Util/StaticCookie.php @@ -0,0 +1,28 @@ +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); } }