]> git.mxchange.org Git - friendica.git/commitdiff
Allow setting arbitrary keys in the cookie array
authorHypolite Petovan <hypolite@mrpetovan.com>
Sun, 17 Jan 2021 22:30:18 +0000 (17:30 -0500)
committerHypolite Petovan <hypolite@mrpetovan.com>
Sat, 23 Jan 2021 10:42:58 +0000 (05:42 -0500)
src/Model/User/Cookie.php
src/Security/Authentication.php
tests/Util/StaticCookie.php
tests/src/Model/User/CookieTest.php

index cda814e694cf2f682ac78fecb882295c7477e2c2..4b39ab7ebb2b30aadf79495cd506a7bc19acfd87 100644 (file)
@@ -41,126 +41,118 @@ class Cookie
        const HTTPONLY = true;
 
        /** @var string The remote address of this node */
-       private $remoteAddr = '0.0.0.0';
+       private $remoteAddr;
        /** @var bool True, if the connection is ssl enabled */
-       private $sslEnabled = false;
+       private $sslEnabled;
        /** @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;
+       private $lifetime;
+       /** @var array The Friendica cookie data array */
+       private $data;
 
-       public function __construct(IConfig $config, App\BaseURL $baseURL, array $server = [], array $cookie = [])
+       /**
+        * @param IConfig     $config
+        * @param App\BaseURL $baseURL
+        * @param array       $SERVER The $_SERVER array
+        * @param array       $COOKIE The $_COOKIE array
+        */
+       public function __construct(IConfig $config, App\BaseURL $baseURL, array $SERVER = [], array $COOKIE = [])
        {
-               if (!empty($server['REMOTE_ADDR'])) {
-                       $this->remoteAddr = $server['REMOTE_ADDR'];
-               }
-
                $this->sslEnabled     = $baseURL->getSSLPolicy() === 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;
+
+               $this->remoteAddr = ($SERVER['REMOTE_ADDR'] ?? null) ?: '0.0.0.0';
+
+               $this->data = json_decode($COOKIE[self::NAME] ?? '[]', true) ?: [];
        }
 
        /**
-        * 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
+        * Returns the value for a key of the Friendica cookie
         *
+        * @param string $key
+        * @param mixed  $default
+        * @return mixed|null The value for the provided cookie key
         */
-       public function check(string $hash, string $password, string $privateKey)
+       public function get(string $key, $default = null)
        {
-               return hash_equals(
-                       $this->getHash($password, $privateKey),
-                       $hash
-               );
+               return $this->data[$key] ?? $default;
        }
 
        /**
-        * 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
+        * Set a single cookie key value.
+        * Overwrites an existing value with the same key.
         *
+        * @param $key
+        * @param $value
         * @return bool
         */
-       public function set(int $uid, string $password, string $privateKey, int $seconds = null)
+       public function set($key, $value): bool
        {
-               if (!isset($seconds)) {
-                       $seconds = $this->lifetime + time();
-               } elseif (isset($seconds) && $seconds != 0) {
-                       $seconds = $seconds + time();
-               }
+               return $this->setMultiple([$key => $value]);
+       }
 
-               $value = json_encode([
-                       'uid'  => $uid,
-                       'hash' => $this->getHash($password, $privateKey),
-                       'ip'   => $this->remoteAddr,
-               ]);
+       /**
+        * Sets multiple cookie key values.
+        * Overwrites existing values with the same key.
+        *
+        * @param array $values
+        * @return bool
+        */
+       public function setMultiple(array $values): bool
+       {
+               $this->data = $values + $this->data;
 
-               return $this->setCookie(self::NAME, $value, $seconds, $this->sslEnabled);
+               return $this->send();
        }
 
        /**
-        * Returns the data of the Friendicas user cookie
+        * Remove a cookie key
         *
-        * @return mixed|null The JSON data, null if not set
+        * @param string $key
         */
-       public function getData()
+       public function unset(string $key)
        {
-               // 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;
-                       }
-               }
+               if (isset($this->data[$key])) {
+                       unset($this->data[$key]);
 
-               return null;
+                       $this->send();
+               }
        }
 
        /**
-        * Clears the Friendica cookie of this user after leaving the page
+        * Clears the Friendica cookie
         */
-       public function clear()
+       public function clear(): bool
        {
+               $this->data = [];
                // make sure cookie is deleted on browser close, as a security measure
-               return $this->setCookie(self::NAME, '', -3600, $this->sslEnabled);
+               return $this->setCookie( '', -3600, $this->sslEnabled);
        }
 
        /**
-        * 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
+        * Send the cookie, should be called every time $this->data is changed or to refresh the cookie.
         *
-        * @return string Hashed data
+        * @return bool
         */
-       private function getHash(string $password, string $privateKey)
+       public function send(): bool
        {
-               return hash_hmac(
-                       'sha256',
-                       hash_hmac('sha256', $password, $privateKey),
-                       $this->sitePrivateKey
+               return $this->setCookie(
+                       json_encode(['ip' => $this->remoteAddr] + $this->data),
+                       $this->lifetime + time(),
+                       $this->sslEnabled
                );
        }
 
        /**
-        * Send a cookie - protected, internal function for test-mocking possibility
+        * setcookie() wrapper: 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 bool   $secure [optional]
@@ -168,9 +160,43 @@ class Cookie
         * @return bool If output exists prior to calling this function,
         *
         */
-       protected function setCookie(string $name, string $value = null, int $expire = null,
-                                    bool $secure = null)
+       protected function setCookie(string $value = null, int $expire = null,
+                                    bool $secure = null): bool
+       {
+               return setcookie(self::NAME, $value, $expire, self::PATH, self::DOMAIN, $secure, self::HTTPONLY);
+       }
+
+       /**
+        * Calculate a hash of a user's private data for storage in the cookie.
+        * Hashed twice, with the user's own private key first, then the node's private key second.
+        *
+        * @param string $privateData User private data
+        * @param string $privateKey  User private key
+        *
+        * @return string Hashed data
+        */
+       public function hashPrivateData(string $privateData, string $privateKey): string
+       {
+               return hash_hmac(
+                       'sha256',
+                       hash_hmac('sha256', $privateData, $privateKey),
+                       $this->sitePrivateKey
+               );
+       }
+
+       /**
+        * @param string $hash        Hash from a cookie key value
+        * @param string $privateData User private data
+        * @param string $privateKey  User private key
+        *
+        * @return boolean
+        *
+        */
+       public function comparePrivateDataHash(string $hash, string $privateData, string $privateKey): bool
        {
-               return setcookie($name, $value, $expire, self::PATH, self::DOMAIN, $secure, self::HTTPONLY);
+               return hash_equals(
+                       $this->hashPrivateData($privateData, $privateKey),
+                       $hash
+               );
        }
 }
index 5c6624a33f1f3972f28b3c3597815e95841c9d16..089035bb7f1fa803da61e0db30dc8a5f7dbabe07 100644 (file)
@@ -33,6 +33,7 @@ use Friendica\Database\DBA;
 use Friendica\DI;
 use Friendica\Model\User;
 use Friendica\Network\HTTPException;
+use Friendica\Repository\TwoFactor\TrustedBrowser;
 use Friendica\Util\DateTimeFormat;
 use Friendica\Util\Network;
 use Friendica\Util\Strings;
@@ -100,16 +101,13 @@ class Authentication
         */
        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($data->uid)) {
-
+               if ($this->cookie->get('uid')) {
                        $user = $this->dba->selectFirst(
                                'user',
                                [],
                                [
-                                       'uid'             => $data->uid,
+                                       'uid'             => $this->cookie->get('uid'),
                                        'blocked'         => false,
                                        'account_expired' => false,
                                        'account_removed' => false,
@@ -117,24 +115,25 @@ class Authentication
                                ]
                        );
                        if ($this->dba->isResult($user)) {
-                               if (!$this->cookie->check($data->hash,
+                               if (!$this->cookie->comparePrivateDataHash($this->cookie->get('hash'),
                                        $user['password'] ?? '',
-                                       $user['prvkey'] ?? '')) {
-                                       $this->logger->notice("Hash doesn't fit.", ['user' => $data->uid]);
+                                       $user['prvkey'] ?? '')
+                               ) {
+                                       $this->logger->notice("Hash doesn't fit.", ['user' => $this->cookie->get('uid')]);
                                        $this->session->clear();
                                        $this->cookie->clear();
                                        $this->baseUrl->redirect();
                                }
 
                                // Renew the cookie
-                               $this->cookie->set($user['uid'], $user['password'], $user['prvkey']);
+                               $this->cookie->send();
 
                                // Do the authentification if not done by now
                                if (!$this->session->get('authenticated')) {
                                        $this->setForUser($a, $user);
 
                                        if ($this->config->get('system', 'paranoia')) {
-                                               $this->session->set('addr', $data->ip);
+                                               $this->session->set('addr', $this->cookie->get('ip'));
                                        }
                                }
                        }
@@ -377,12 +376,15 @@ class Authentication
                         */
                        if ($this->session->get('remember')) {
                                $this->logger->info('Injecting cookie for remembered user ' . $user_record['nickname']);
-                               $this->cookie->set($user_record['uid'], $user_record['password'], $user_record['prvkey']);
+                               $this->cookie->setMultiple([
+                                       'uid'  => $user_record['uid'],
+                                       'hash' => $this->cookie->hashPrivateData($user_record['password'], $user_record['prvkey']),
+                               ]);
                                $this->session->remove('remember');
                        }
                }
 
-               $this->twoFactorCheck($user_record['uid'], $a);
+               $this->redirectForTwoFactorAuthentication($user_record['uid'], $a);
 
                if ($interactive) {
                        if ($user_record['login_date'] <= DBA::NULL_DATETIME) {
@@ -404,19 +406,23 @@ class Authentication
        }
 
        /**
+        * Decides whether to redirect the user to two-factor authentication.
+        * All return calls in this method skip two-factor authentication
+        *
         * @param int $uid The User Identified
         * @param App $a   The Friendica Application context
         *
         * @throws HTTPException\ForbiddenException In case the two factor authentication is forbidden (e.g. for AJAX calls)
+        * @throws HTTPException\InternalServerErrorException
         */
-       private function twoFactorCheck(int $uid, App $a)
+       private function redirectForTwoFactorAuthentication(int $uid, App $a)
        {
                // Check user setting, if 2FA disabled return
                if (!$this->pConfig->get($uid, '2fa', 'verified')) {
                        return;
                }
 
-               // Check current path, if 2fa authentication module return
+               // Check current path, if public or 2fa module return
                if ($a->argc > 0 && in_array($a->argv[0], ['2fa', 'view', 'help', 'api', 'proxy', 'logout'])) {
                        return;
                }
index 6cfbdc3ab7df3d8757cbc9558b47072ffec19dd1..4bee43185fb775a5ef416d5df631a20e8f78053d 100644 (file)
@@ -35,22 +35,24 @@ class StaticCookie extends Cookie
 
        /**
         * Send a cookie - protected, internal function for test-mocking possibility
-        * @see Cookie::setCookie()
         *
-        * @link  https://php.net/manual/en/function.setcookie.php
-        *
-        * @param string $name
         * @param string $value  [optional]
         * @param int    $expire [optional]
         * @param bool   $secure [optional]
+        * @return bool
         *
         * @noinspection PhpMissingParentCallCommonInspection
         *
+        * @link         https://php.net/manual/en/function.setcookie.php
+        *
+        * @see          Cookie::setCookie()
         */
-       protected function setCookie(string $name, string $value = null, int $expire = null, bool $secure = null)
+       protected function setCookie(string $value = null, int $expire = null, bool $secure = null): bool
        {
-               self::$_COOKIE[$name] = $value;
+               self::$_COOKIE[self::NAME] = $value;
                self::$_EXPIRE = $expire;
+
+               return true;
        }
 
        public static function clearStatic()
index e6e29048d9ba6534d0834eaad056163f9518bd16..a69577e6ea7f7983a8d2ddc7857de85a09f678bc 100644 (file)
@@ -128,30 +128,20 @@ class CookieTest extends MockedTest
                $cookie = new Cookie($this->config, $this->baseUrl, [], $cookieData);
                self::assertInstanceOf(Cookie::class, $cookie);
 
-               $assertData = $cookie->getData();
-
-               if (!$hasValues) {
-                       self::assertEmpty($assertData);
+               if (isset($uid)) {
+                       self::assertEquals($uid, $cookie->get('uid'));
+               } else {
+                       self::assertNull($cookie->get('uid'));
+               }
+               if (isset($hash)) {
+                       self::assertEquals($hash, $cookie->get('hash'));
                } else {
-                       self::assertNotEmpty($assertData);
-                       if (isset($uid)) {
-                               self::assertObjectHasAttribute('uid', $assertData);
-                               self::assertEquals($uid, $assertData->uid);
-                       } else {
-                               self::assertObjectNotHasAttribute('uid', $assertData);
-                       }
-                       if (isset($hash)) {
-                               self::assertObjectHasAttribute('hash', $assertData);
-                               self::assertEquals($hash, $assertData->hash);
-                       } else {
-                               self::assertObjectNotHasAttribute('hash', $assertData);
-                       }
-                       if (isset($ip)) {
-                               self::assertObjectHasAttribute('ip', $assertData);
-                               self::assertEquals($ip, $assertData->ip);
-                       } else {
-                               self::assertObjectNotHasAttribute('ip', $assertData);
-                       }
+                       self::assertNull($cookie->get('hash'));
+               }
+               if (isset($ip)) {
+                       self::assertEquals($ip, $cookie->get('ip'));
+               } else {
+                       self::assertNull($cookie->get('ip'));
                }
        }
 
@@ -196,7 +186,7 @@ class CookieTest extends MockedTest
                $cookie = new Cookie($this->config, $this->baseUrl);
                self::assertInstanceOf(Cookie::class, $cookie);
 
-               self::assertEquals($assertTrue, $cookie->check($assertHash, $password, $userPrivateKey));
+               self::assertEquals($assertTrue, $cookie->comparePrivateDataHash($assertHash, $password, $userPrivateKey));
        }
 
        public function dataSet()
@@ -210,7 +200,6 @@ class CookieTest extends MockedTest
                                'assertHash'  => 'b657a15cfe7ed1f7289c9aa51af14a9a26c966f4ddd74e495fba103d8e872a39',
                                'remoteIp'    => '0.0.0.0',
                                'serverArray' => [],
-                               'lifetime'    => null,
                        ],
                        'withServerArray' => [
                                'serverKey'   => 23,
@@ -220,32 +209,11 @@ class CookieTest extends MockedTest
                                '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)
+       public function assertCookie($uid, $hash, $remoteIp)
        {
                self::assertArrayHasKey(Cookie::NAME, StaticCookie::$_COOKIE);
 
@@ -258,11 +226,7 @@ class CookieTest extends MockedTest
                self::assertObjectHasAttribute('ip', $data);
                self::assertEquals($remoteIp, $data->ip);
 
-               if (isset($lifetime) && $lifetime !== 0) {
-                       self::assertLessThanOrEqual(time() + $lifetime, StaticCookie::$_EXPIRE);
-               } else {
-                       self::assertLessThanOrEqual(time() + Cookie::DEFAULT_EXPIRE * 24 * 60 * 60, StaticCookie::$_EXPIRE);
-               }
+               self::assertLessThanOrEqual(time() + Cookie::DEFAULT_EXPIRE * 24 * 60 * 60, StaticCookie::$_EXPIRE);
        }
 
        /**
@@ -270,7 +234,7 @@ class CookieTest extends MockedTest
         *
         * @dataProvider dataSet
         */
-       public function testSet($serverKey, $uid, $password, $privateKey, $assertHash, $remoteIp, $serverArray, $lifetime)
+       public function testSet($serverKey, $uid, $password, $privateKey, $assertHash, $remoteIp, $serverArray)
        {
                $this->baseUrl->shouldReceive('getSSLPolicy')->andReturn(true)->once();
                $this->config->shouldReceive('get')->with('system', 'site_prvkey')->andReturn($serverKey)->once();
@@ -279,17 +243,20 @@ class CookieTest extends MockedTest
                $cookie = new StaticCookie($this->config, $this->baseUrl, $serverArray);
                self::assertInstanceOf(Cookie::class, $cookie);
 
-               $cookie->set($uid, $password, $privateKey, $lifetime);
+               $cookie->setMultiple([
+                       'uid' => $uid,
+                       'hash' => $assertHash,
+               ]);
 
-               self::assertCookie($uid, $assertHash, $remoteIp, $lifetime);
+               self::assertCookie($uid, $assertHash, $remoteIp);
        }
 
        /**
-        * Test two different set() of the cookie class (first set is invalid)
+        * Test the set() method of the cookie class
         *
         * @dataProvider dataSet
         */
-       public function testDoubleSet($serverKey, $uid, $password, $privateKey, $assertHash, $remoteIp, $serverArray, $lifetime)
+       public function testDoubleSet($serverKey, $uid, $password, $privateKey, $assertHash, $remoteIp, $serverArray)
        {
                $this->baseUrl->shouldReceive('getSSLPolicy')->andReturn(true)->once();
                $this->config->shouldReceive('get')->with('system', 'site_prvkey')->andReturn($serverKey)->once();
@@ -298,12 +265,10 @@ class CookieTest extends MockedTest
                $cookie = new StaticCookie($this->config, $this->baseUrl, $serverArray);
                self::assertInstanceOf(Cookie::class, $cookie);
 
-               // Invalid set, should get overwritten
-               $cookie->set(-1, 'invalid', 'nothing', -234);
-
-               $cookie->set($uid, $password, $privateKey, $lifetime);
+               $cookie->set('uid', $uid);
+               $cookie->set('hash', $assertHash);
 
-               self::assertCookie($uid, $assertHash, $remoteIp, $lifetime);
+               self::assertCookie($uid, $assertHash, $remoteIp);
        }
 
        /**