Move Cookie to own class (with tests)

Move Authentication to App namespace

include/api.php

@@ -12,7 +12,7 @@ use Friendica\Content\ContactSelector;
 use Friendica\Content\Feature;
 use Friendica\Content\Text\BBCode;
 use Friendica\Content\Text\HTML;
-use Friendica\Core\Authentication;
+use Friendica\App\Authentication;
 use Friendica\Core\Config;
 use Friendica\Core\Hook;
 use Friendica\Core\L10n;

index.php

@@ -23,5 +23,5 @@ $a->runFrontend(
 	$dice->create(\Friendica\App\Module::class),
 	$dice->create(\Friendica\App\Router::class),
 	$dice->create(\Friendica\Core\Config\PConfiguration::class),
-	$dice->create(\Friendica\Core\Authentication::class)
+	$dice->create(\Friendica\App\Authentication::class)
 );

mod/dfrn_poll.php

@@ -6,7 +6,7 @@
 
 use Friendica\App;
 use Friendica\BaseObject;
-use Friendica\Core\Authentication;
+use Friendica\App\Authentication;
 use Friendica\Core\Config;
 use Friendica\Core\L10n;
 use Friendica\Core\Logger;
@@ -24,7 +24,7 @@ function dfrn_poll_init(App $a)
 {
 	/** @var Authentication $authentication */
 	$authentication = BaseObject::getClass(Authentication::class);
-	$authentication->withSession($a, $_COOKIE);
+	$authentication->withSession($a);
 
 	$dfrn_id         =  $_GET['dfrn_id']         ?? '';
 	$type            = ($_GET['type']            ?? '') ?: 'data';

mod/openid.php

@@ -5,7 +5,7 @@
 
 use Friendica\App;
 use Friendica\BaseObject;
-use Friendica\Core\Authentication;
+use Friendica\App\Authentication;
 use Friendica\Core\Config;
 use Friendica\Core\L10n;
 use Friendica\Core\Logger;

src/App.php

@@ -8,7 +8,7 @@ use Exception;
 use Friendica\App\Arguments;
 use Friendica\App\BaseURL;
 use Friendica\App\Page;
-use Friendica\Core\Authentication;
+use Friendica\App\Authentication;
 use Friendica\Core\Config\Cache\ConfigCache;
 use Friendica\Core\Config\Configuration;
 use Friendica\Core\Config\PConfiguration;
@@ -720,7 +720,7 @@ class App
 				Model\Profile::openWebAuthInit($token);
 			}
 
-			$auth->withSession($this, $_COOKIE);
+			$auth->withSession($this);
 
 			if (empty($_SESSION['authenticated'])) {
 				header('X-Account-Management-Status: none');

src/App/Authentication.php

@@ -4,11 +4,15 @@
  * @file /src/Core/Authentication.php
  */
 
-namespace Friendica\Core;
+namespace Friendica\App;
 
 use Exception;
 use Friendica\App;
 use Friendica\Core\Config\Configuration;
+use Friendica\Core\Hook;
+use Friendica\Core\PConfig;
+use Friendica\Core\Session;
+use Friendica\Core\System;
 use Friendica\Database\Database;
 use Friendica\Database\DBA;
 use Friendica\Model\User;
@@ -35,6 +39,8 @@ class Authentication
 	private $dba;
 	/** @var LoggerInterface */
 	private $logger;
+	/** @var User\Cookie */
+	private $cookie;
 
 	/**
 	 * Authentication constructor.
@@ -44,31 +50,32 @@ class Authentication
 	 * @param L10n            $l10n
 	 * @param Database        $dba
 	 * @param LoggerInterface $logger
+	 * @param User\Cookie     $cookie
 	 */
-	public function __construct(Configuration $config, App\BaseURL $baseUrl, L10n $l10n, Database $dba, LoggerInterface $logger)
+	public function __construct(Configuration $config, App\BaseURL $baseUrl, L10n $l10n, Database $dba, LoggerInterface $logger, User\Cookie $cookie)
 	{
 		$this->config  = $config;
 		$this->baseUrl = $baseUrl;
 		$this->l10n    = $l10n;
 		$this->dba     = $dba;
 		$this->logger  = $logger;
+		$this->cookie = $cookie;
 	}
 
 	/**
 	 * @brief Tries to auth the user from the cookie or session
 	 *
 	 * @param App   $a      The Friendica Application context
-	 * @param array $cookie The $_COOKIE array
 	 *
 	 * @throws HttpException\InternalServerErrorException In case of Friendica internal exceptions
 	 * @throws Exception In case of general exceptions (like SQL Grammar)
 	 */
-	public function withSession(App $a, array $cookie)
+	public function withSession(App $a)
 	{
+		$data = $this->cookie->getData();
+
 		// When the "Friendica" cookie is set, take the value to authenticate and renew the cookie.
-		if (isset($cookie["Friendica"])) {
+		if (isset($data) && isset($data->uid)) {
-			$data = json_decode($cookie["Friendica"]);
-			if (isset($data->uid)) {
 
 			$user = $this->dba->selectFirst(
 				'user',
@@ -82,17 +89,16 @@ class Authentication
 				]
 			);
 			if (DBA::isResult($user)) {
-					if (!Session::checkCookie($data->hash, $user)) {
+				if (!$this->cookie->check($data->hash,
+					$user['password'] ?? '',
+					$user['prvKey'] ?? '')) {
 					$this->logger->notice("Hash doesn't fit.", ['user' => $data->uid]);
 					Session::delete();
 					$this->baseUrl->redirect();
 				}
 
 				// Renew the cookie
-					// Expires after 7 days by default,
+				$this->cookie->set($user['uid'], $user['password'], $user['prvKey']);
-					// can be set via system.auth_cookie_lifetime
-					$authcookiedays = $this->config->get('system', 'auth_cookie_lifetime', 7);
-					Session::setCookie($authcookiedays * 24 * 60 * 60, $user);
 
 				// Do the authentification if not done by now
 				if (!Session::get('authenticated')) {
@@ -104,7 +110,6 @@ class Authentication
 				}
 			}
 		}
-		}
 
 		if (Session::get('authenticated')) {
 			if (Session::get('visitor_id') && !Session::get('uid')) {
@@ -241,7 +246,7 @@ class Authentication
 		}
 
 		if (!$remember) {
-			Session::setCookie(0); // 0 means delete on browser exit
+			$this->cookie->clear();
 		}
 
 		// if we haven't failed up this point, log them in.
@@ -343,7 +348,7 @@ class Authentication
 			 */;
 			if (Session::get('remember')) {
 				$a->getLogger()->info('Injecting cookie for remembered user ' . $user_record['nickname']);
-				Session::setCookie(604800, $user_record);
+				$this->cookie->set($user_record['uid'], $user_record['password'], $user_record['prvKey']);
 				Session::remove('remember');
 			}
 		}

src/Core/Session.php

@@ -6,10 +6,12 @@
 namespace Friendica\Core;
 
 use Friendica\App;
+use Friendica\BaseObject;
 use Friendica\Core\Session\CacheSessionHandler;
 use Friendica\Core\Session\DatabaseSessionHandler;
 use Friendica\Database\DBA;
 use Friendica\Model\Contact;
+use Friendica\Model\User;
 use Friendica\Util\Strings;
 
 /**
@@ -171,73 +173,15 @@ class Session
 		return $_SESSION['authenticated'];
 	}
 
-	/**
-	 * @brief Calculate the hash that is needed for the "Friendica" cookie
-	 *
-	 * @param array $user Record from "user" table
-	 *
-	 * @return string Hashed data
-	 * @throws \Friendica\Network\HTTPException\InternalServerErrorException
-	 */
-	private static function getCookieHashForUser($user)
-	{
-		return hash_hmac(
-			"sha256",
-			hash_hmac("sha256", $user["password"], $user["prvkey"]),
-			Config::get("system", "site_prvkey")
-		);
-	}
-
-	/**
-	 * @brief Set the "Friendica" cookie
-	 *
-	 * @param int   $time
-	 * @param array $user Record from "user" table
-	 * @throws \Friendica\Network\HTTPException\InternalServerErrorException
-	 */
-	public static function setCookie($time, $user = [])
-	{
-		if ($time != 0) {
-			$time = $time + time();
-		}
-
-		if ($user) {
-			$value = json_encode([
-				"uid" => $user["uid"],
-				"hash" => self::getCookieHashForUser($user),
-				"ip" => ($_SERVER['REMOTE_ADDR'] ?? '') ?: '0.0.0.0'
-			]);
-		} else {
-			$value = "";
-		}
-
-		setcookie("Friendica", $value, $time, "/", "", (Config::get('system', 'ssl_policy') == App\BaseURL::SSL_POLICY_FULL), true);
-	}
-
-	/**
-	 * @brief Checks if the "Friendica" cookie is set
-	 *
-	 * @param string $hash
-	 * @param array  $user Record from "user" table
-	 *
-	 * @return boolean True, if the cookie is set
-	 *
-	 * @throws \Friendica\Network\HTTPException\InternalServerErrorException
-	 */
-	public static function checkCookie(string $hash, array $user)
-	{
-		return hash_equals(
-			self::getCookieHashForUser($user),
-			$hash
-		);
-	}
-
 	/**
 	 * @brief Kills the "Friendica" cookie and all session data
 	 */
 	public static function delete()
 	{
-		self::setCookie(-3600); // make sure cookie is deleted on browser close, as a security measure
+		/** @var User\Cookie $cookie */
+		$cookie = BaseObject::getClass(User\Cookie::class);
+		$cookie->clear();
+		$_SESSION = [];
 		session_unset();
 		session_destroy();
 	}

src/Model/User.php

@@ -9,12 +9,14 @@ namespace Friendica\Model;
 
 use DivineOmega\PasswordExposed;
 use Exception;
+use Friendica\App;
 use Friendica\Core\Config;
 use Friendica\Core\Hook;
 use Friendica\Core\L10n;
 use Friendica\Core\Logger;
 use Friendica\Core\PConfig;
 use Friendica\Core\Protocol;
+use Friendica\Core\Session;
 use Friendica\Core\System;
 use Friendica\Core\Worker;
 use Friendica\Database\DBA;

src/Model/User/Cookie.php

@@ -0,0 +1,159 @@
+<?php
+
+namespace Friendica\Model\User;
+
+use Friendica\App;
+use Friendica\Core\Config\Configuration;
+
+/**
+ * Interacting with the Friendica Cookie of a user
+ */
+class Cookie
+{
+	/** @var int Default expire duration in days */
+	const DEFAULT_EXPIRE = 7;
+	/** @var string The name of the Friendica cookie */
+	const NAME = 'Friendica';
+
+	/** @var string The remote address of this node */
+	private $remoteAddr = '0.0.0.0';
+	/** @var bool True, if the connection is ssl enabled */
+	private $sslEnabled = false;
+	/** @var string The private key of this Friendica node */
+	private $sitePrivateKey;
+	/** @var int The default cookie lifetime */
+	private $lifetime = self::DEFAULT_EXPIRE * 24 * 60 * 60;
+	/** @var array The $_COOKIE array */
+	private $cookie;
+
+	public function __construct(Configuration $config, array $server = [], array $cookie = [])
+	{
+		if (!empty($server['REMOTE_ADDR'])) {
+			$this->remoteAddr = $server['REMOTE_ADDR'];
+		}
+
+		$this->sslEnabled     = $config->get('system', 'ssl_policy') === App\BaseURL::SSL_POLICY_FULL;
+		$this->sitePrivateKey = $config->get('system', 'site_prvkey');
+
+		$authCookieDays = $config->get('system', 'auth_cookie_lifetime',
+			self::DEFAULT_EXPIRE);
+		$this->lifetime = $authCookieDays * 24 * 60 * 60;
+		$this->cookie   = $cookie;
+	}
+
+	/**
+	 * Checks if the Friendica cookie is set for a user
+	 *
+	 * @param string $hash       The cookie hash
+	 * @param string $password   The user password
+	 * @param string $privateKey The private Key of the user
+	 *
+	 * @return boolean True, if the cookie is set
+	 *
+	 */
+	public function check(string $hash, string $password, string $privateKey)
+	{
+		return hash_equals(
+			$this->getHash($password, $privateKey),
+			$hash
+		);
+	}
+
+	/**
+	 * Set the Friendica cookie for a user
+	 *
+	 * @param int    $uid        The user id
+	 * @param string $password   The user password
+	 * @param string $privateKey The user private key
+	 * @param int|null    $seconds    optional the seconds
+	 *
+	 * @return bool
+	 */
+	public function set(int $uid, string $password, string $privateKey, int $seconds = null)
+	{
+		if (!isset($seconds)) {
+			$seconds = $this->lifetime;
+		} elseif (isset($seconds) && $seconds != 0) {
+			$seconds = $seconds + time();
+		}
+
+		$value = json_encode([
+			'uid'  => $uid,
+			'hash' => $this->getHash($password, $privateKey),
+			'ip'   => $this->remoteAddr,
+		]);
+
+		return $this->setCookie(self::NAME, $value, $seconds,
+			'/', '', $this->sslEnabled, true);
+	}
+
+	/**
+	 * Returns the data of the Friendicas user cookie
+	 *
+	 * @return mixed|null The JSON data, null if not set
+	 */
+	public function getData()
+	{
+		// When the "Friendica" cookie is set, take the value to authenticate and renew the cookie.
+		if (isset($this->cookie[self::NAME])) {
+			$data = json_decode($this->cookie[self::NAME]);
+			if (!empty($data)) {
+				return $data;
+			}
+		}
+
+		return null;
+	}
+
+	/**
+	 * Clears the Friendica cookie of this user after leaving the page
+	 */
+	public function clear()
+	{
+		// make sure cookie is deleted on browser close, as a security measure
+		return $this->setCookie(self::NAME, '', -3600,
+			'/', '', $this->sslEnabled, true);
+	}
+
+	/**
+	 * Calculate the hash that is needed for the Friendica cookie
+	 *
+	 * @param string $password   The user password
+	 * @param string $privateKey The private key of the user
+	 *
+	 * @return string Hashed data
+	 */
+	private function getHash(string $password, string $privateKey)
+	{
+		return hash_hmac(
+			'sha256',
+			hash_hmac('sha256', $password, $privateKey),
+			$this->sitePrivateKey
+		);
+	}
+
+	/**
+	 * Send a cookie - protected, internal function for test-mocking possibility
+	 *
+	 * @link  https://php.net/manual/en/function.setcookie.php
+	 *
+	 * @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] <p>
+	 *
+	 * @return bool If output exists prior to calling this function,
+	 *
+	 * @since 4.0
+	 * @since 5.0
+	 */
+	protected function setCookie(string $name, string $value = null, int $expire = null,
+	                             string $path = null, string $domain = null,
+	                             bool $secure = null, bool $httponly = null)
+	{
+		return setcookie($name, $value, $expire, $path, $domain, $secure, $httponly);
+	}
+}

src/Module/Delegation.php

@@ -3,7 +3,7 @@
 namespace Friendica\Module;
 
 use Friendica\BaseModule;
-use Friendica\Core\Authentication;
+use Friendica\App\Authentication;
 use Friendica\Core\Hook;
 use Friendica\Core\L10n;
 use Friendica\Core\Renderer;

src/Module/Login.php

@@ -7,7 +7,7 @@
 namespace Friendica\Module;
 
 use Friendica\BaseModule;
-use Friendica\Core\Authentication;
+use Friendica\App\Authentication;
 use Friendica\Core\Config;
 use Friendica\Core\Hook;
 use Friendica\Core\L10n;

src/Module/Logout.php

@@ -6,7 +6,7 @@
 namespace Friendica\Module;
 
 use Friendica\BaseModule;
-use Friendica\Core\Authentication;
+use Friendica\App\Authentication;
 use Friendica\Core\Cache;
 use Friendica\Core\Hook;
 use Friendica\Core\L10n;

src/Module/TwoFactor/Recovery.php

@@ -3,7 +3,7 @@
 namespace Friendica\Module\TwoFactor;
 
 use Friendica\BaseModule;
-use Friendica\Core\Authentication;
+use Friendica\App\Authentication;
 use Friendica\Core\L10n;
 use Friendica\Core\Renderer;
 use Friendica\Core\Session;

src/Module/TwoFactor/Verify.php

@@ -3,7 +3,7 @@
 namespace Friendica\Module\TwoFactor;
 
 use Friendica\BaseModule;
-use Friendica\Core\Authentication;
+use Friendica\App\Authentication;
 use Friendica\Core\L10n;
 use Friendica\Core\PConfig;
 use Friendica\Core\Renderer;

src/Network/FKOAuth1.php

@@ -5,7 +5,7 @@
 namespace Friendica\Network;
 
 use Friendica\BaseObject;
-use Friendica\Core\Authentication;
+use Friendica\App\Authentication;
 use Friendica\Core\Logger;
 use Friendica\Core\Session;
 use Friendica\Database\DBA;

tests/src/Model/User/CookieTest.php

@@ -0,0 +1,171 @@
+<?php
+
+namespace Friendica\Testsrc\Model\User;
+
+use Friendica\Core\Config\Configuration;
+use Friendica\Model\User\Cookie;
+use Friendica\Test\DatabaseTest;
+use Mockery\MockInterface;
+
+class CookieTest extends DatabaseTest
+{
+	/** @var MockInterface|Configuration */
+	private $config;
+
+	protected function setUp()
+	{
+		parent::setUp();;
+
+		$this->config = \Mockery::mock(Configuration::class);
+	}
+
+	public function testInstance()
+	{
+		$this->config->shouldReceive('get')->with('system', 'ssl_policy')->andReturn(1)->once();
+		$this->config->shouldReceive('get')->with('system', 'site_prvkey')->andReturn('1235')->once();
+		$this->config->shouldReceive('get')->with('system', 'auth_cookie_lifetime', Cookie::DEFAULT_EXPIRE)->andReturn('7')->once();
+
+		$cookie = new Cookie($this->config, []);
+		$this->assertInstanceOf(Cookie::class, $cookie);
+	}
+
+	public function dataGet()
+	{
+		return [
+			'default'    => [
+				'cookieData' => [
+					Cookie::NAME => json_encode([
+						'uid'  => -1,
+						'hash' => 12345,
+						'ip'   => '127.0.0.1',
+					])
+				],
+				'hasValues'  => true,
+				'uid'        => -1,
+				'hash'       => 12345,
+				'ip'         => '127.0.0.1',
+			],
+			'missing'    => [
+				'cookieData' => [
+
+				],
+				'hasValues'  => false,
+				'uid'        => null,
+				'hash'       => null,
+				'ip'         => null,
+			],
+			'invalid'    => [
+				'cookieData' => [
+					Cookie::NAME => 'test',
+				],
+				'hasValues'  => false,
+				'uid'        => null,
+				'hash'       => null,
+				'ip'         => null,
+			],
+			'incomplete' => [
+				'cookieData' => [
+					Cookie::NAME => json_encode([
+						'uid'  => -1,
+						'hash' => 12345,
+					])
+				],
+				'hasValues'  => true,
+				'uid'        => -1,
+				'hash'       => 12345,
+				'ip'         => null,
+			],
+		];
+	}
+
+	/**
+	 * @dataProvider dataGet
+	 */
+	public function testGet(array $cookieData, bool $hasValues, $uid, $hash, $ip)
+	{
+		$this->config->shouldReceive('get')->with('system', 'ssl_policy')->andReturn(1)->once();
+		$this->config->shouldReceive('get')->with('system', 'site_prvkey')->andReturn('1235')->once();
+		$this->config->shouldReceive('get')->with('system', 'auth_cookie_lifetime', Cookie::DEFAULT_EXPIRE)->andReturn('7')->once();
+
+		$cookie = new Cookie($this->config, [], $cookieData);
+		$this->assertInstanceOf(Cookie::class, $cookie);
+
+		$assertData = $cookie->getData();
+
+		if (!$hasValues) {
+			$this->assertEmpty($assertData);
+		} else {
+			$this->assertNotEmpty($assertData);
+			if (isset($uid)) {
+				$this->assertObjectHasAttribute('uid', $assertData);
+				$this->assertEquals($uid, $assertData->uid);
+			} else {
+				$this->assertObjectNotHasAttribute('uid', $assertData);
+			}
+			if (isset($hash)) {
+				$this->assertObjectHasAttribute('hash', $assertData);
+				$this->assertEquals($hash, $assertData->hash);
+			} else {
+				$this->assertObjectNotHasAttribute('hash', $assertData);
+			}
+			if (isset($ip)) {
+				$this->assertObjectHasAttribute('ip', $assertData);
+				$this->assertEquals($ip, $assertData->ip);
+			} else {
+				$this->assertObjectNotHasAttribute('ip', $assertData);
+			}
+		}
+	}
+
+	public function dataCheck()
+	{
+		return [
+			'default'   => [
+				'serverPrivateKey' => 'serverkey',
+				'userPrivateKey'   => 'userkey',
+				'password'         => 'test',
+				'assertHash'       => 'e9b4eb16275a2907b5659d22905b248221d0517dde4a9d5c320b8fe051b1267b',
+				'assertTrue'       => true,
+			],
+			'emptyUser' => [
+				'serverPrivateKey' => 'serverkey',
+				'userPrivateKey'   => '',
+				'password'         => '',
+				'assertHash'       => '',
+				'assertTrue'       => false,
+			],
+			'invalid' => [
+				'serverPrivateKey' => 'serverkey',
+				'userPrivateKey'   => 'bla',
+				'password'         => 'nope',
+				'assertHash'       => 'real wrong!',
+				'assertTrue'       => false,
+			]
+		];
+	}
+
+	/**
+	 * @dataProvider dataCheck
+	 */
+	public function testCheck(string $serverPrivateKey, string $userPrivateKey, string $password, string $assertHash, bool $assertTrue)
+	{
+		$this->config->shouldReceive('get')->with('system', 'ssl_policy')->andReturn(1)->once();
+		$this->config->shouldReceive('get')->with('system', 'site_prvkey')->andReturn($serverPrivateKey)->once();
+		$this->config->shouldReceive('get')->with('system', 'auth_cookie_lifetime', Cookie::DEFAULT_EXPIRE)->andReturn('7')->once();
+
+		$cookie = new Cookie($this->config, []);
+		$this->assertInstanceOf(Cookie::class, $cookie);
+
+		$this->assertEquals($assertTrue, $cookie->check($assertHash, $password, $userPrivateKey));
+	}
+
+	public function testSet()
+	{
+		$this->markTestIncomplete('Needs mocking of setcookie() first.');
+	}
+
+	public function testClear()
+	{
+		$this->markTestIncomplete('Needs mocking of setcookie() first.');
+	}
+}