From c644d76d28e697c87165e8c5fc12d7bfa5ff3475 Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Sun, 17 Jan 2021 17:30:18 -0500 Subject: [PATCH 1/8] Allow setting arbitrary keys in the cookie array --- src/Model/User/Cookie.php | 206 ++++++++++++++++------------ src/Security/Authentication.php | 34 +++-- tests/Util/StaticCookie.php | 14 +- tests/src/Model/User/CookieTest.php | 89 ++++-------- 4 files changed, 171 insertions(+), 172 deletions(-) diff --git a/src/Model/User/Cookie.php b/src/Model/User/Cookie.php index cda814e694..4b39ab7ebb 100644 --- a/src/Model/User/Cookie.php +++ b/src/Model/User/Cookie.php @@ -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 + * Set a single cookie key value. + * Overwrites an existing value with the same key. * - * @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 + * @param $key + * @param $value + * @return bool + */ + public function set($key, $value): bool + { + return $this->setMultiple([$key => $value]); + } + + /** + * 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->send(); + } + + /** + * Remove a cookie key + * + * @param string $key + */ + public function unset(string $key) + { + if (isset($this->data[$key])) { + unset($this->data[$key]); + + $this->send(); + } + } + + /** + * Clears the Friendica cookie + */ + public function clear(): bool + { + $this->data = []; + // make sure cookie is deleted on browser close, as a security measure + return $this->setCookie( '', -3600, $this->sslEnabled); + } + + /** + * Send the cookie, should be called every time $this->data is changed or to refresh the cookie. * * @return bool */ - public function set(int $uid, string $password, string $privateKey, int $seconds = null) + public function send(): bool { - if (!isset($seconds)) { - $seconds = $this->lifetime + time(); - } 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); - } - - /** - * 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); - } - - /** - * 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 + 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($name, $value, $expire, self::PATH, self::DOMAIN, $secure, self::HTTPONLY); + 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 hash_equals( + $this->hashPrivateData($privateData, $privateKey), + $hash + ); } } diff --git a/src/Security/Authentication.php b/src/Security/Authentication.php index 5c6624a33f..089035bb7f 100644 --- a/src/Security/Authentication.php +++ b/src/Security/Authentication.php @@ -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; } diff --git a/tests/Util/StaticCookie.php b/tests/Util/StaticCookie.php index 6cfbdc3ab7..4bee43185f 100644 --- a/tests/Util/StaticCookie.php +++ b/tests/Util/StaticCookie.php @@ -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() diff --git a/tests/src/Model/User/CookieTest.php b/tests/src/Model/User/CookieTest.php index e6e29048d9..a69577e6ea 100644 --- a/tests/src/Model/User/CookieTest.php +++ b/tests/src/Model/User/CookieTest.php @@ -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::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('uid')); + } + if (isset($hash)) { + self::assertEquals($hash, $cookie->get('hash')); + } else { + 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', $uid); + $cookie->set('hash', $assertHash); - $cookie->set($uid, $password, $privateKey, $lifetime); - - self::assertCookie($uid, $assertHash, $remoteIp, $lifetime); + self::assertCookie($uid, $assertHash, $remoteIp); } /** From 3e257d42660136efff37adc6ef8e4c68eb5cc4c5 Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Mon, 18 Jan 2021 22:53:06 -0500 Subject: [PATCH 2/8] Move all two-factor authentication classes in Security\TwoFactor --- src/Model/User.php | 2 +- src/Module/Security/TwoFactor/Recovery.php | 2 +- src/Module/Settings/TwoFactor/AppSpecific.php | 2 +- src/Module/Settings/TwoFactor/Index.php | 4 ++-- src/Module/Settings/TwoFactor/Recovery.php | 2 +- .../TwoFactor/Model}/AppSpecificPassword.php | 2 +- .../TwoFactor => Security/TwoFactor/Model}/RecoveryCode.php | 2 +- 7 files changed, 8 insertions(+), 8 deletions(-) rename src/{Model/TwoFactor => Security/TwoFactor/Model}/AppSpecificPassword.php (98%) rename src/{Model/TwoFactor => Security/TwoFactor/Model}/RecoveryCode.php (98%) diff --git a/src/Model/User.php b/src/Model/User.php index dbace74e5d..41e612ac80 100644 --- a/src/Model/User.php +++ b/src/Model/User.php @@ -34,7 +34,7 @@ use Friendica\Core\System; use Friendica\Core\Worker; use Friendica\Database\DBA; use Friendica\DI; -use Friendica\Model\TwoFactor\AppSpecificPassword; +use Friendica\Security\TwoFactor\Model\AppSpecificPassword; use Friendica\Network\HTTPException; use Friendica\Object\Image; use Friendica\Util\Crypto; diff --git a/src/Module/Security/TwoFactor/Recovery.php b/src/Module/Security/TwoFactor/Recovery.php index 7af1b6ac01..384d36e0d3 100644 --- a/src/Module/Security/TwoFactor/Recovery.php +++ b/src/Module/Security/TwoFactor/Recovery.php @@ -25,7 +25,7 @@ use Friendica\BaseModule; use Friendica\Core\Renderer; use Friendica\Core\Session; use Friendica\DI; -use Friendica\Model\TwoFactor\RecoveryCode; +use Friendica\Security\TwoFactor\Model\RecoveryCode; /** * // Page 1a: Recovery code verification diff --git a/src/Module/Settings/TwoFactor/AppSpecific.php b/src/Module/Settings/TwoFactor/AppSpecific.php index db094a8855..8c11af0295 100644 --- a/src/Module/Settings/TwoFactor/AppSpecific.php +++ b/src/Module/Settings/TwoFactor/AppSpecific.php @@ -23,7 +23,7 @@ namespace Friendica\Module\Settings\TwoFactor; use Friendica\Core\Renderer; use Friendica\DI; -use Friendica\Model\TwoFactor\AppSpecificPassword; +use Friendica\Security\TwoFactor\Model\AppSpecificPassword; use Friendica\Module\BaseSettings; use Friendica\Module\Security\Login; diff --git a/src/Module/Settings/TwoFactor/Index.php b/src/Module/Settings/TwoFactor/Index.php index 8cc04787f5..bd3840485f 100644 --- a/src/Module/Settings/TwoFactor/Index.php +++ b/src/Module/Settings/TwoFactor/Index.php @@ -24,8 +24,8 @@ namespace Friendica\Module\Settings\TwoFactor; use Friendica\Core\Renderer; use Friendica\Core\Session; use Friendica\DI; -use Friendica\Model\TwoFactor\AppSpecificPassword; -use Friendica\Model\TwoFactor\RecoveryCode; +use Friendica\Security\TwoFactor\Model\AppSpecificPassword; +use Friendica\Security\TwoFactor\Model\RecoveryCode; use Friendica\Model\User; use Friendica\Module\BaseSettings; use Friendica\Module\Security\Login; diff --git a/src/Module/Settings/TwoFactor/Recovery.php b/src/Module/Settings/TwoFactor/Recovery.php index 7b0d28534f..6ee52bd036 100644 --- a/src/Module/Settings/TwoFactor/Recovery.php +++ b/src/Module/Settings/TwoFactor/Recovery.php @@ -23,7 +23,7 @@ namespace Friendica\Module\Settings\TwoFactor; use Friendica\Core\Renderer; use Friendica\DI; -use Friendica\Model\TwoFactor\RecoveryCode; +use Friendica\Security\TwoFactor\Model\RecoveryCode; use Friendica\Module\BaseSettings; use Friendica\Module\Security\Login; diff --git a/src/Model/TwoFactor/AppSpecificPassword.php b/src/Security/TwoFactor/Model/AppSpecificPassword.php similarity index 98% rename from src/Model/TwoFactor/AppSpecificPassword.php rename to src/Security/TwoFactor/Model/AppSpecificPassword.php index 1e8d56f759..1524e25920 100644 --- a/src/Model/TwoFactor/AppSpecificPassword.php +++ b/src/Security/TwoFactor/Model/AppSpecificPassword.php @@ -19,7 +19,7 @@ * */ -namespace Friendica\Model\TwoFactor; +namespace Friendica\Security\TwoFactor\Model; use Friendica\Database\DBA; use Friendica\Model\User; diff --git a/src/Model/TwoFactor/RecoveryCode.php b/src/Security/TwoFactor/Model/RecoveryCode.php similarity index 98% rename from src/Model/TwoFactor/RecoveryCode.php rename to src/Security/TwoFactor/Model/RecoveryCode.php index 2c3b71ad41..d6a1a6e951 100644 --- a/src/Model/TwoFactor/RecoveryCode.php +++ b/src/Security/TwoFactor/Model/RecoveryCode.php @@ -19,7 +19,7 @@ * */ -namespace Friendica\Model\TwoFactor; +namespace Friendica\Security\TwoFactor\Model; use Friendica\Database\DBA; use Friendica\Util\DateTimeFormat; From 0fc5f26ff7c0301da42bfc360b1ad70359184e5c Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Sun, 17 Jan 2021 16:04:00 -0500 Subject: [PATCH 3/8] Replace BaseEntity with BaseDataTransferObject class for API representation classes --- ...seEntity.php => BaseDataTransferObject.php} | 8 ++++---- src/BaseModel.php | 18 +++++++++++------- src/Object/Api/Friendica/Notification.php | 4 ++-- src/Object/Api/Mastodon/Account.php | 6 +++--- src/Object/Api/Mastodon/Activity.php | 4 ++-- src/Object/Api/Mastodon/Application.php | 4 ++-- src/Object/Api/Mastodon/Attachment.php | 6 +++--- src/Object/Api/Mastodon/Card.php | 8 ++++---- src/Object/Api/Mastodon/Emoji.php | 4 ++-- src/Object/Api/Mastodon/Error.php | 6 +++--- src/Object/Api/Mastodon/Field.php | 4 ++-- src/Object/Api/Mastodon/Instance.php | 4 ++-- src/Object/Api/Mastodon/Mention.php | 4 ++-- src/Object/Api/Mastodon/Relationship.php | 4 ++-- src/Object/Api/Mastodon/Stats.php | 4 ++-- src/Object/Api/Mastodon/Status.php | 6 +++--- src/Object/Api/Mastodon/Tag.php | 4 ++-- src/Object/Api/Twitter/User.php | 4 ++-- 18 files changed, 53 insertions(+), 49 deletions(-) rename src/{BaseEntity.php => BaseDataTransferObject.php} (83%) diff --git a/src/BaseEntity.php b/src/BaseDataTransferObject.php similarity index 83% rename from src/BaseEntity.php rename to src/BaseDataTransferObject.php index 1ea3f8a16b..5736cdf323 100644 --- a/src/BaseEntity.php +++ b/src/BaseDataTransferObject.php @@ -22,20 +22,20 @@ namespace Friendica; /** - * The API entity classes are meant as data transfer objects. As such, their member should be protected. + * These data transfer object classes are meant for API representations. As such, their members should be protected. * Then the JsonSerializable interface ensures the protected members will be included in a JSON encode situation. * * Constructors are supposed to take as arguments the Friendica dependencies/model/collection/data it needs to * populate the class members. */ -abstract class BaseEntity implements \JsonSerializable +abstract class BaseDataTransferObject implements \JsonSerializable { /** * Returns the current entity as an json array * * @return array */ - public function jsonSerialize() + public function jsonSerialize(): array { return $this->toArray(); } @@ -45,7 +45,7 @@ abstract class BaseEntity implements \JsonSerializable * * @return array */ - public function toArray() + public function toArray(): array { return get_object_vars($this); } diff --git a/src/BaseModel.php b/src/BaseModel.php index 41320d8bd5..a2750d568d 100644 --- a/src/BaseModel.php +++ b/src/BaseModel.php @@ -31,7 +31,7 @@ use Psr\Log\LoggerInterface; * * @property int id */ -abstract class BaseModel extends BaseEntity +abstract class BaseModel extends BaseDataTransferObject { /** @var Database */ protected $dba; @@ -67,7 +67,7 @@ abstract class BaseModel extends BaseEntity $this->originalData = $data; } - public function getOriginalData() + public function getOriginalData(): array { return $this->originalData; } @@ -84,7 +84,7 @@ abstract class BaseModel extends BaseEntity * @param array $data * @return BaseModel */ - public static function createFromPrototype(BaseModel $prototype, array $data) + public static function createFromPrototype(BaseModel $prototype, array $data): BaseModel { $model = clone $prototype; $model->data = $data; @@ -100,7 +100,7 @@ abstract class BaseModel extends BaseEntity * @param $name * @return bool */ - public function __isset($name) + public function __isset($name): bool { return in_array($name, array_merge(array_keys($this->data), array_keys(get_object_vars($this)))); } @@ -126,15 +126,19 @@ abstract class BaseModel extends BaseEntity } /** + * * Magic setter. This allows to set model fields with the following syntax: + * - $model->field = $value (outside of class) + * - $this->field = $value (inside of class) + * * @param string $name - * @param mixed $value + * @param mixed $value */ - public function __set($name, $value) + public function __set(string $name, $value) { $this->data[$name] = $value; } - public function toArray() + public function toArray(): array { return $this->data; } diff --git a/src/Object/Api/Friendica/Notification.php b/src/Object/Api/Friendica/Notification.php index aee787da17..60c824a566 100644 --- a/src/Object/Api/Friendica/Notification.php +++ b/src/Object/Api/Friendica/Notification.php @@ -21,7 +21,7 @@ namespace Friendica\Object\Api\Friendica; -use Friendica\BaseEntity; +use Friendica\BaseDataTransferObject; use Friendica\Content\Text\BBCode; use Friendica\Content\Text\HTML; use Friendica\Model\Notify; @@ -33,7 +33,7 @@ use Friendica\Util\Temporal; * * @see https://github.com/friendica/friendica/blob/develop/doc/API-Entities.md#notification */ -class Notification extends BaseEntity +class Notification extends BaseDataTransferObject { /** @var integer */ protected $id; diff --git a/src/Object/Api/Mastodon/Account.php b/src/Object/Api/Mastodon/Account.php index 587c6ce6d4..e74339563f 100644 --- a/src/Object/Api/Mastodon/Account.php +++ b/src/Object/Api/Mastodon/Account.php @@ -22,7 +22,7 @@ namespace Friendica\Object\Api\Mastodon; use Friendica\App\BaseURL; -use Friendica\BaseEntity; +use Friendica\BaseDataTransferObject; use Friendica\Collection\Api\Mastodon\Fields; use Friendica\Content\Text\BBCode; use Friendica\Database\DBA; @@ -34,7 +34,7 @@ use Friendica\Util\DateTimeFormat; * * @see https://docs.joinmastodon.org/entities/account */ -class Account extends BaseEntity +class Account extends BaseDataTransferObject { /** @var string */ protected $id; @@ -138,7 +138,7 @@ class Account extends BaseEntity * * @return array */ - public function toArray() + public function toArray(): array { $account = parent::toArray(); diff --git a/src/Object/Api/Mastodon/Activity.php b/src/Object/Api/Mastodon/Activity.php index a73307eb47..73bde98a43 100644 --- a/src/Object/Api/Mastodon/Activity.php +++ b/src/Object/Api/Mastodon/Activity.php @@ -21,14 +21,14 @@ namespace Friendica\Object\Api\Mastodon; -use Friendica\BaseEntity; +use Friendica\BaseDataTransferObject; /** * Class Activity * * @see https://docs.joinmastodon.org/entities/activity */ -class Activity extends BaseEntity +class Activity extends BaseDataTransferObject { /** @var string (UNIX Timestamp) */ protected $week; diff --git a/src/Object/Api/Mastodon/Application.php b/src/Object/Api/Mastodon/Application.php index d26d270d98..8cac4aaf79 100644 --- a/src/Object/Api/Mastodon/Application.php +++ b/src/Object/Api/Mastodon/Application.php @@ -21,14 +21,14 @@ namespace Friendica\Object\Api\Mastodon; -use Friendica\BaseEntity; +use Friendica\BaseDataTransferObject; /** * Class Application * * @see https://docs.joinmastodon.org/entities/application */ -class Application extends BaseEntity +class Application extends BaseDataTransferObject { /** @var string */ protected $name; diff --git a/src/Object/Api/Mastodon/Attachment.php b/src/Object/Api/Mastodon/Attachment.php index 1651e9c40c..cc55b3715c 100644 --- a/src/Object/Api/Mastodon/Attachment.php +++ b/src/Object/Api/Mastodon/Attachment.php @@ -21,14 +21,14 @@ namespace Friendica\Object\Api\Mastodon; -use Friendica\BaseEntity; +use Friendica\BaseDataTransferObject; /** * Class Attachment * * @see https://docs.joinmastodon.org/entities/attachment */ -class Attachment extends BaseEntity +class Attachment extends BaseDataTransferObject { /** @var string */ protected $id; @@ -67,7 +67,7 @@ class Attachment extends BaseEntity * * @return array */ - public function toArray() + public function toArray(): array { $attachment = parent::toArray(); diff --git a/src/Object/Api/Mastodon/Card.php b/src/Object/Api/Mastodon/Card.php index 2f46e4779d..33e3c10599 100644 --- a/src/Object/Api/Mastodon/Card.php +++ b/src/Object/Api/Mastodon/Card.php @@ -21,14 +21,14 @@ namespace Friendica\Object\Api\Mastodon; -use Friendica\BaseEntity; +use Friendica\BaseDataTransferObject; /** * Class Card * * @see https://docs.joinmastodon.org/entities/card */ -class Card extends BaseEntity +class Card extends BaseDataTransferObject { /** @var string */ protected $url; @@ -67,10 +67,10 @@ class Card extends BaseEntity * * @return array */ - public function toArray() + public function toArray(): array { if (empty($this->url)) { - return null; + return []; } return parent::toArray(); diff --git a/src/Object/Api/Mastodon/Emoji.php b/src/Object/Api/Mastodon/Emoji.php index 1f6f121507..837b186b60 100644 --- a/src/Object/Api/Mastodon/Emoji.php +++ b/src/Object/Api/Mastodon/Emoji.php @@ -21,14 +21,14 @@ namespace Friendica\Object\Api\Mastodon; -use Friendica\BaseEntity; +use Friendica\BaseDataTransferObject; /** * Class Emoji * * @see https://docs.joinmastodon.org/entities/emoji/ */ -class Emoji extends BaseEntity +class Emoji extends BaseDataTransferObject { //Required attributes /** @var string */ diff --git a/src/Object/Api/Mastodon/Error.php b/src/Object/Api/Mastodon/Error.php index 0bfd1826c6..44573987f2 100644 --- a/src/Object/Api/Mastodon/Error.php +++ b/src/Object/Api/Mastodon/Error.php @@ -21,14 +21,14 @@ namespace Friendica\Object\Api\Mastodon; -use Friendica\BaseEntity; +use Friendica\BaseDataTransferObject; /** * Class Error * * @see https://docs.joinmastodon.org/entities/error */ -class Error extends BaseEntity +class Error extends BaseDataTransferObject { /** @var string */ protected $error; @@ -53,7 +53,7 @@ class Error extends BaseEntity * * @return array */ - public function toArray() + public function toArray(): array { $error = parent::toArray(); diff --git a/src/Object/Api/Mastodon/Field.php b/src/Object/Api/Mastodon/Field.php index 95cbc89dfa..91e8fb1911 100644 --- a/src/Object/Api/Mastodon/Field.php +++ b/src/Object/Api/Mastodon/Field.php @@ -21,14 +21,14 @@ namespace Friendica\Object\Api\Mastodon; -use Friendica\BaseEntity; +use Friendica\BaseDataTransferObject; /** * Class Field * * @see https://docs.joinmastodon.org/entities/field/ */ -class Field extends BaseEntity +class Field extends BaseDataTransferObject { /** @var string */ protected $name; diff --git a/src/Object/Api/Mastodon/Instance.php b/src/Object/Api/Mastodon/Instance.php index 6105a8bee1..d08637509d 100644 --- a/src/Object/Api/Mastodon/Instance.php +++ b/src/Object/Api/Mastodon/Instance.php @@ -21,7 +21,7 @@ namespace Friendica\Object\Api\Mastodon; -use Friendica\BaseEntity; +use Friendica\BaseDataTransferObject; use Friendica\Database\DBA; use Friendica\DI; use Friendica\Model\User; @@ -32,7 +32,7 @@ use Friendica\Module\Register; * * @see https://docs.joinmastodon.org/api/entities/#instance */ -class Instance extends BaseEntity +class Instance extends BaseDataTransferObject { /** @var string (URL) */ protected $uri; diff --git a/src/Object/Api/Mastodon/Mention.php b/src/Object/Api/Mastodon/Mention.php index 22e623e604..6de6f00487 100644 --- a/src/Object/Api/Mastodon/Mention.php +++ b/src/Object/Api/Mastodon/Mention.php @@ -22,14 +22,14 @@ namespace Friendica\Object\Api\Mastodon; use Friendica\App\BaseURL; -use Friendica\BaseEntity; +use Friendica\BaseDataTransferObject; /** * Class Mention * * @see https://docs.joinmastodon.org/entities/mention */ -class Mention extends BaseEntity +class Mention extends BaseDataTransferObject { /** @var string */ protected $id; diff --git a/src/Object/Api/Mastodon/Relationship.php b/src/Object/Api/Mastodon/Relationship.php index bb3aa55416..06f973788c 100644 --- a/src/Object/Api/Mastodon/Relationship.php +++ b/src/Object/Api/Mastodon/Relationship.php @@ -21,7 +21,7 @@ namespace Friendica\Object\Api\Mastodon; -use Friendica\BaseEntity; +use Friendica\BaseDataTransferObject; use Friendica\Model\Contact; use Friendica\Util\Network; @@ -30,7 +30,7 @@ use Friendica\Util\Network; * * @see https://docs.joinmastodon.org/api/entities/#relationship */ -class Relationship extends BaseEntity +class Relationship extends BaseDataTransferObject { /** @var int */ protected $id; diff --git a/src/Object/Api/Mastodon/Stats.php b/src/Object/Api/Mastodon/Stats.php index 6ead526729..398b7252d9 100644 --- a/src/Object/Api/Mastodon/Stats.php +++ b/src/Object/Api/Mastodon/Stats.php @@ -21,7 +21,7 @@ namespace Friendica\Object\Api\Mastodon; -use Friendica\BaseEntity; +use Friendica\BaseDataTransferObject; use Friendica\Core\Protocol; use Friendica\Database\DBA; use Friendica\DI; @@ -31,7 +31,7 @@ use Friendica\DI; * * @see https://docs.joinmastodon.org/api/entities/#stats */ -class Stats extends BaseEntity +class Stats extends BaseDataTransferObject { /** @var int */ protected $user_count = 0; diff --git a/src/Object/Api/Mastodon/Status.php b/src/Object/Api/Mastodon/Status.php index d14eb4efa3..c4b7e397dc 100644 --- a/src/Object/Api/Mastodon/Status.php +++ b/src/Object/Api/Mastodon/Status.php @@ -21,7 +21,7 @@ namespace Friendica\Object\Api\Mastodon; -use Friendica\BaseEntity; +use Friendica\BaseDataTransferObject; use Friendica\Content\Text\BBCode; use Friendica\Object\Api\Mastodon\Status\Counts; use Friendica\Object\Api\Mastodon\Status\UserAttributes; @@ -32,7 +32,7 @@ use Friendica\Util\DateTimeFormat; * * @see https://docs.joinmastodon.org/entities/status */ -class Status extends BaseEntity +class Status extends BaseDataTransferObject { /** @var string */ protected $id; @@ -143,7 +143,7 @@ class Status extends BaseEntity * * @return array */ - public function toArray() + public function toArray(): array { $status = parent::toArray(); diff --git a/src/Object/Api/Mastodon/Tag.php b/src/Object/Api/Mastodon/Tag.php index 1e74eae00f..c83c3bcf35 100644 --- a/src/Object/Api/Mastodon/Tag.php +++ b/src/Object/Api/Mastodon/Tag.php @@ -22,14 +22,14 @@ namespace Friendica\Object\Api\Mastodon; use Friendica\App\BaseURL; -use Friendica\BaseEntity; +use Friendica\BaseDataTransferObject; /** * Class Tag * * @see https://docs.joinmastodon.org/entities/tag */ -class Tag extends BaseEntity +class Tag extends BaseDataTransferObject { /** @var string */ protected $name; diff --git a/src/Object/Api/Twitter/User.php b/src/Object/Api/Twitter/User.php index 1cdd699de8..7793fbc26f 100644 --- a/src/Object/Api/Twitter/User.php +++ b/src/Object/Api/Twitter/User.php @@ -21,14 +21,14 @@ namespace Friendica\Object\Api\Twitter; -use Friendica\BaseEntity; +use Friendica\BaseDataTransferObject; use Friendica\Content\ContactSelector; use Friendica\Content\Text\BBCode; /** * @see https://developer.twitter.com/en/docs/tweets/data-dictionary/overview/user-object */ -class User extends BaseEntity +class User extends BaseDataTransferObject { /** @var int */ protected $id; From b633d75e6c7c87a57367777703578b1d34dbffe3 Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Mon, 18 Jan 2021 23:27:44 -0500 Subject: [PATCH 4/8] [Database version 1394] Add 2fa_trusted_browser table --- database.sql | 16 +++++++++++++++- static/dbstructure.config.php | 14 ++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/database.sql b/database.sql index c9017fd2ef..4a8a9d6fc8 100644 --- a/database.sql +++ b/database.sql @@ -283,10 +283,24 @@ CREATE TABLE IF NOT EXISTS `2fa_recovery_codes` ( `code` varchar(50) NOT NULL COMMENT 'Recovery code string', `generated` datetime NOT NULL COMMENT 'Datetime the code was generated', `used` datetime COMMENT 'Datetime the code was used', - PRIMARY KEY(`uid`,`code`), + PRIMARY KEY(`uid`,`code`), FOREIGN KEY (`uid`) REFERENCES `user` (`uid`) ON UPDATE RESTRICT ON DELETE CASCADE ) DEFAULT COLLATE utf8mb4_general_ci COMMENT='Two-factor authentication recovery codes'; +-- +-- TABLE 2fa_trusted_browser +-- +CREATE TABLE IF NOT EXISTS `2fa_trusted_browser` ( + `cookie_hash` varchar(80) NOT NULL COMMENT 'Trusted cookie hash', + `uid` mediumint unsigned NOT NULL COMMENT 'User ID', + `user_agent` text COMMENT 'User agent string', + `created` datetime NOT NULL COMMENT 'Datetime the trusted browser was recorded', + `last_used` datetime COMMENT 'Datetime the trusted browser was last used', + PRIMARY KEY(`cookie_hash`), + INDEX `uid` (`uid`), + FOREIGN KEY (`uid`) REFERENCES `user` (`uid`) ON UPDATE RESTRICT ON DELETE CASCADE +) DEFAULT COLLATE utf8mb4_general_ci COMMENT='Two-factor authentication trusted browsers'; + -- -- TABLE addon -- diff --git a/static/dbstructure.config.php b/static/dbstructure.config.php index feba3c920b..5c9d454ead 100644 --- a/static/dbstructure.config.php +++ b/static/dbstructure.config.php @@ -348,6 +348,20 @@ return [ "PRIMARY" => ["uid", "code"] ] ], + "2fa_trusted_browser" => [ + "comment" => "Two-factor authentication trusted browsers", + "fields" => [ + "cookie_hash" => ["type" => "varchar(80)", "not null" => "1", "primary" => "1", "comment" => "Trusted cookie hash"], + "uid" => ["type" => "mediumint unsigned", "not null" => "1", "foreign" => ["user" => "uid"], "comment" => "User ID"], + "user_agent" => ["type" => "text", "comment" => "User agent string"], + "created" => ["type" => "datetime", "not null" => "1", "comment" => "Datetime the trusted browser was recorded"], + "last_used" => ["type" => "datetime", "comment" => "Datetime the trusted browser was last used"], + ], + "indexes" => [ + "PRIMARY" => ["cookie_hash"], + "uid" => ["uid"], + ] + ], "addon" => [ "comment" => "registered addons", "fields" => [ From 72bb3bce3460a483d07a0f004cc66e799236ade6 Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Mon, 18 Jan 2021 23:32:28 -0500 Subject: [PATCH 5/8] Add trusted browser classes - Added some tests --- src/BaseEntity.php | 56 +++++++++++ .../TwoFactor/Collection/TrustedBrowsers.php | 10 ++ .../TwoFactor/Factory/TrustedBrowser.php | 33 +++++++ .../TwoFactor/Model/TrustedBrowser.php | 51 ++++++++++ .../TwoFactor/Repository/TrustedBrowser.php | 98 +++++++++++++++++++ .../TwoFactor/Factory/TrustedBrowserTest.php | 62 ++++++++++++ .../TwoFactor/Model/TrustedBrowserTest.php | 46 +++++++++ 7 files changed, 356 insertions(+) create mode 100644 src/BaseEntity.php create mode 100644 src/Security/TwoFactor/Collection/TrustedBrowsers.php create mode 100644 src/Security/TwoFactor/Factory/TrustedBrowser.php create mode 100644 src/Security/TwoFactor/Model/TrustedBrowser.php create mode 100644 src/Security/TwoFactor/Repository/TrustedBrowser.php create mode 100644 tests/src/Security/TwoFactor/Factory/TrustedBrowserTest.php create mode 100644 tests/src/Security/TwoFactor/Model/TrustedBrowserTest.php diff --git a/src/BaseEntity.php b/src/BaseEntity.php new file mode 100644 index 0000000000..66acab1bb0 --- /dev/null +++ b/src/BaseEntity.php @@ -0,0 +1,56 @@ +. + * + */ + +namespace Friendica; + +use Friendica\Network\HTTPException; + +/** + * The Entity classes directly inheriting from this abstract class are meant to represent a single business entity. + * Their properties may or may not correspond with the database fields of the table we use to represent it. + * Each model method must correspond to a business action being performed on this entity. + * Only these methods will be allowed to alter the model data. + * + * To persist such a model, the associated Repository must be instantiated and the "save" method must be called + * and passed the entity as a parameter. + * + * Ideally, the constructor should only be called in the associated Factory which will instantiate entities depending + * on the provided data. + * + * Since these objects aren't meant to be using any dependency, including logging, unit tests can and must be + * written for each and all of their methods + */ +abstract class BaseEntity extends BaseDataTransferObject +{ + /** + * @param string $name + * @return mixed + * @throws HTTPException\InternalServerErrorException + */ + public function __get(string $name) + { + if (!property_exists($this, $name)) { + throw new HTTPException\InternalServerErrorException('Unknown property ' . $name . ' in Entity ' . static::class); + } + + return $this->$name; + } +} diff --git a/src/Security/TwoFactor/Collection/TrustedBrowsers.php b/src/Security/TwoFactor/Collection/TrustedBrowsers.php new file mode 100644 index 0000000000..659e16a5b5 --- /dev/null +++ b/src/Security/TwoFactor/Collection/TrustedBrowsers.php @@ -0,0 +1,10 @@ +cookie_hash = $cookie_hash; + $this->uid = $uid; + $this->user_agent = $user_agent; + $this->created = $created; + $this->last_used = $last_used; + } + + public function recordUse() + { + $this->last_used = DateTimeFormat::utcNow(); + } +} diff --git a/src/Security/TwoFactor/Repository/TrustedBrowser.php b/src/Security/TwoFactor/Repository/TrustedBrowser.php new file mode 100644 index 0000000000..6d708d3673 --- /dev/null +++ b/src/Security/TwoFactor/Repository/TrustedBrowser.php @@ -0,0 +1,98 @@ +db = $database; + $this->logger = $logger; + $this->factory = $factory ?? new \Friendica\Security\TwoFactor\Factory\TrustedBrowser($logger); + } + + /** + * @param string $cookie_hash + * @return Model\TrustedBrowser|null + * @throws \Exception + */ + public function selectOneByHash(string $cookie_hash): Model\TrustedBrowser + { + $fields = $this->db->selectFirst(self::$table_name, [], ['cookie_hash' => $cookie_hash]); + if (!$this->db->isResult($fields)) { + throw new NotFoundException(''); + } + + return $this->factory->createFromTableRow($fields); + } + + public function selectAllByUid(int $uid): TrustedBrowsers + { + $rows = $this->db->selectToArray(self::$table_name, [], ['uid' => $uid]); + + $trustedBrowsers = []; + foreach ($rows as $fields) { + $trustedBrowsers[] = $this->factory->createFromTableRow($fields); + } + + return new TrustedBrowsers($trustedBrowsers); + } + + /** + * @param Model\TrustedBrowser $trustedBrowser + * @return bool + * @throws \Exception + */ + public function save(Model\TrustedBrowser $trustedBrowser): bool + { + return $this->db->insert(self::$table_name, $trustedBrowser->toArray(), $this->db::INSERT_UPDATE); + } + + /** + * @param Model\TrustedBrowser $trustedBrowser + * @return bool + * @throws \Exception + */ + public function remove(Model\TrustedBrowser $trustedBrowser): bool + { + return $this->db->delete(self::$table_name, ['cookie_hash' => $trustedBrowser->cookie_hash]); + } + + /** + * @param int $local_user + * @param string $cookie_hash + * @return bool + * @throws \Exception + */ + public function removeForUser(int $local_user, string $cookie_hash): bool + { + return $this->db->delete(self::$table_name, ['cookie_hash' => $cookie_hash,'uid' => $local_user]); + } + + /** + * @param int $local_user + * @return bool + * @throws \Exception + */ + public function removeAllForUser(int $local_user): bool + { + return $this->db->delete(self::$table_name, ['uid' => $local_user]); + } +} diff --git a/tests/src/Security/TwoFactor/Factory/TrustedBrowserTest.php b/tests/src/Security/TwoFactor/Factory/TrustedBrowserTest.php new file mode 100644 index 0000000000..5b2b6111c9 --- /dev/null +++ b/tests/src/Security/TwoFactor/Factory/TrustedBrowserTest.php @@ -0,0 +1,62 @@ + Strings::getRandomHex(), + 'uid' => 42, + 'user_agent' => 'PHPUnit', + 'created' => DateTimeFormat::utcNow(), + 'last_used' => null, + ]; + + $trustedBrowser = $factory->createFromTableRow($row); + + $this->assertEquals($row, $trustedBrowser->toArray()); + } + + public function testCreateFromTableRowMissingData() + { + $this->expectException(\TypeError::class); + + $factory = new TrustedBrowser(new VoidLogger()); + + $row = [ + 'cookie_hash' => null, + 'uid' => null, + 'user_agent' => null, + 'created' => null, + 'last_used' => null, + ]; + + $trustedBrowser = $factory->createFromTableRow($row); + + $this->assertEquals($row, $trustedBrowser->toArray()); + } + + public function testCreateForUserWithUserAgent() + { + $factory = new TrustedBrowser(new VoidLogger()); + + $uid = 42; + $userAgent = 'PHPUnit'; + + $trustedBrowser = $factory->createForUserWithUserAgent($uid, $userAgent); + + $this->assertNotEmpty($trustedBrowser->cookie_hash); + $this->assertEquals($uid, $trustedBrowser->uid); + $this->assertEquals($userAgent, $trustedBrowser->user_agent); + $this->assertNotEmpty($trustedBrowser->created); + } +} diff --git a/tests/src/Security/TwoFactor/Model/TrustedBrowserTest.php b/tests/src/Security/TwoFactor/Model/TrustedBrowserTest.php new file mode 100644 index 0000000000..d895273744 --- /dev/null +++ b/tests/src/Security/TwoFactor/Model/TrustedBrowserTest.php @@ -0,0 +1,46 @@ +assertEquals($hash, $trustedBrowser->cookie_hash); + $this->assertEquals(42, $trustedBrowser->uid); + $this->assertEquals('PHPUnit', $trustedBrowser->user_agent); + $this->assertNotEmpty($trustedBrowser->created); + } + + public function testRecordUse() + { + $hash = Strings::getRandomHex(); + $past = DateTimeFormat::utc('now - 5 minutes'); + + $trustedBrowser = new TrustedBrowser( + $hash, + 42, + 'PHPUnit', + $past, + $past + ); + + $trustedBrowser->recordUse(); + + $this->assertEquals($past, $trustedBrowser->created); + $this->assertGreaterThan($past, $trustedBrowser->last_used); + } +} From 50f97e977ae364f20f6cb0cfb15759e97d328d62 Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Mon, 18 Jan 2021 23:32:48 -0500 Subject: [PATCH 6/8] Added support for trusted browser during authentication --- src/Module/Security/Logout.php | 8 ++++++ src/Module/Security/TwoFactor/Verify.php | 15 ++++++++++++ src/Security/Authentication.php | 31 ++++++++++++++++++++++-- view/templates/twofactor/verify.tpl | 2 ++ 4 files changed, 54 insertions(+), 2 deletions(-) diff --git a/src/Module/Security/Logout.php b/src/Module/Security/Logout.php index c698dd00b0..2a4b33e2f8 100644 --- a/src/Module/Security/Logout.php +++ b/src/Module/Security/Logout.php @@ -26,6 +26,7 @@ use Friendica\Core\Hook; use Friendica\Core\System; use Friendica\DI; use Friendica\Model\Profile; +use Friendica\Security\TwoFactor; /** * Logout module @@ -44,6 +45,13 @@ class Logout extends BaseModule } Hook::callAll("logging_out"); + + // Remove this trusted browser as it won't be able to be used ever again after the cookie is cleared + if (DI::cookie()->get('trusted')) { + $trustedBrowserRepository = new TwoFactor\Repository\TrustedBrowser(DI::dba(), DI::logger()); + $trustedBrowserRepository->removeForUser(local_user(), DI::cookie()->get('trusted')); + } + DI::cookie()->clear(); DI::session()->clear(); diff --git a/src/Module/Security/TwoFactor/Verify.php b/src/Module/Security/TwoFactor/Verify.php index d7a44f0c56..8e3c75c01c 100644 --- a/src/Module/Security/TwoFactor/Verify.php +++ b/src/Module/Security/TwoFactor/Verify.php @@ -26,6 +26,7 @@ use Friendica\Core\Renderer; use Friendica\Core\Session; use Friendica\DI; use PragmaRX\Google2FA\Google2FA; +use Friendica\Security\TwoFactor; /** * Page 1: Authenticator code verification @@ -55,6 +56,19 @@ class Verify extends BaseModule if ($valid && Session::get('2fa') !== $code) { Session::set('2fa', $code); + // Trust this browser feature + if (!empty($_REQUEST['trust_browser'])) { + $trustedBrowserFactory = new TwoFactor\Factory\TrustedBrowser(DI::logger()); + $trustedBrowserRepository = new TwoFactor\Repository\TrustedBrowser(DI::dba(), DI::logger(), $trustedBrowserFactory); + + $trustedBrowser = $trustedBrowserFactory->createForUserWithUserAgent(local_user(), $_SERVER['HTTP_USER_AGENT']); + + $trustedBrowserRepository->save($trustedBrowser); + + // The string is sent to the browser to be sent back with each request + DI::cookie()->set('trusted', $trustedBrowser->cookie_hash); + } + // Resume normal login workflow DI::auth()->setForUser($a, $a->user, true, true); } else { @@ -83,6 +97,7 @@ class Verify extends BaseModule '$errors' => self::$errors, '$recovery_message' => DI::l10n()->t('Don’t have your phone? Enter a two-factor recovery code', '2fa/recovery'), '$verify_code' => ['verify_code', DI::l10n()->t('Please enter a code from your authentication app'), '', '', DI::l10n()->t('Required'), 'autofocus autocomplete="off" placeholder="000000"', 'tel'], + '$trust_browser' => ['trust_browser', DI::l10n()->t('This is my two-factor authenticator app device'), !empty($_REQUEST['trust_browser'])], '$verify_label' => DI::l10n()->t('Verify code and complete login'), ]); } diff --git a/src/Security/Authentication.php b/src/Security/Authentication.php index 089035bb7f..a36341a1d8 100644 --- a/src/Security/Authentication.php +++ b/src/Security/Authentication.php @@ -33,7 +33,7 @@ use Friendica\Database\DBA; use Friendica\DI; use Friendica\Model\User; use Friendica\Network\HTTPException; -use Friendica\Repository\TwoFactor\TrustedBrowser; +use Friendica\Security\TwoFactor\Repository\TrustedBrowser; use Friendica\Util\DateTimeFormat; use Friendica\Util\Network; use Friendica\Util\Strings; @@ -427,11 +427,38 @@ class Authentication return; } - // Case 1: 2FA session present and valid: return + // Case 1a: 2FA session already present: return if ($this->session->get('2fa')) { return; } + // Case 1b: Check for trusted browser + if ($this->cookie->get('trusted')) { + // Retrieve a trusted_browser model based on cookie hash + $trustedBrowserRepository = new TrustedBrowser($this->dba, $this->logger); + try { + $trustedBrowser = $trustedBrowserRepository->selectOneByHash($this->cookie->get('trusted')); + // Verify record ownership + if ($trustedBrowser->uid === $uid) { + // Update last_used date + $trustedBrowser->recordUse(); + + // Save it to the database + $trustedBrowserRepository->save($trustedBrowser); + + // Set 2fa session key and return + $this->session->set('2fa', true); + + return; + } else { + // Invalid trusted cookie value, removing it + $this->cookie->unset('trusted'); + } + } catch (\Throwable $e) { + // Local trusted browser record was probably removed by the user, we carry on with 2FA + } + } + // Case 2: No valid 2FA session: redirect to code verification page if ($this->mode->isAjax()) { throw new HTTPException\ForbiddenException(); diff --git a/view/templates/twofactor/verify.tpl b/view/templates/twofactor/verify.tpl index 2b1fe31421..938f98da09 100644 --- a/view/templates/twofactor/verify.tpl +++ b/view/templates/twofactor/verify.tpl @@ -18,6 +18,8 @@ {{include file="field_input.tpl" field=$verify_code}} + {{include file="field_checkbox.tpl" field=$trust_browser}} +
From 54fa2e8f121d4f7b4d2a893331add935bff99f93 Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Tue, 19 Jan 2021 23:39:09 -0500 Subject: [PATCH 7/8] [Composer] Add dependency ua-parser/uap-php --- composer.json | 1 + composer.lock | 129 +++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 129 insertions(+), 1 deletion(-) diff --git a/composer.json b/composer.json index c242924543..ae4ebf1593 100644 --- a/composer.json +++ b/composer.json @@ -48,6 +48,7 @@ "psr/container": "^1.0", "seld/cli-prompt": "^1.0", "smarty/smarty": "^3.1", + "ua-parser/uap-php": "^3.9", "xemlock/htmlpurifier-html5": "^0.1.11", "fxp/composer-asset-plugin": "^1.4", "bower-asset/base64": "^1.0", diff --git a/composer.lock b/composer.lock index 779c3b51b3..a20c92baa5 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "7d8031c9b95fd94d8872804759a26509", + "content-hash": "c66de8307eed717b4e23fcf386aa17ef", "packages": [ { "name": "asika/simple-console", @@ -253,6 +253,76 @@ }, "type": "bower-asset-library" }, + { + "name": "composer/ca-bundle", + "version": "1.2.8", + "source": { + "type": "git", + "url": "https://github.com/composer/ca-bundle.git", + "reference": "8a7ecad675253e4654ea05505233285377405215" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/composer/ca-bundle/zipball/8a7ecad675253e4654ea05505233285377405215", + "reference": "8a7ecad675253e4654ea05505233285377405215", + "shasum": "" + }, + "require": { + "ext-openssl": "*", + "ext-pcre": "*", + "php": "^5.3.2 || ^7.0 || ^8.0" + }, + "require-dev": { + "phpunit/phpunit": "^4.8.35 || ^5.7 || 6.5 - 8", + "psr/log": "^1.0", + "symfony/process": "^2.5 || ^3.0 || ^4.0 || ^5.0" + }, + "type": "library", + "extra": { + "branch-alias": { + "dev-master": "1.x-dev" + } + }, + "autoload": { + "psr-4": { + "Composer\\CaBundle\\": "src" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Jordi Boggiano", + "email": "j.boggiano@seld.be", + "homepage": "http://seld.be" + } + ], + "description": "Lets you find a path to the system CA bundle, and includes a fallback to the Mozilla CA bundle.", + "keywords": [ + "cabundle", + "cacert", + "certificate", + "ssl", + "tls" + ], + "funding": [ + { + "url": "https://packagist.com", + "type": "custom" + }, + { + "url": "https://github.com/composer", + "type": "github" + }, + { + "url": "https://tidelift.com/funding/github/packagist/composer/composer", + "type": "tidelift" + } + ], + "time": "2020-08-23T12:54:47+00:00" + }, { "name": "divineomega/do-file-cache", "version": "v2.0.6", @@ -3432,6 +3502,63 @@ ], "time": "2020-05-12T16:14:59+00:00" }, + { + "name": "ua-parser/uap-php", + "version": "v3.9.7", + "source": { + "type": "git", + "url": "https://github.com/ua-parser/uap-php.git", + "reference": "7efc2f05b7d9817a59132e5d2e5ca91a1c071f6a" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/ua-parser/uap-php/zipball/7efc2f05b7d9817a59132e5d2e5ca91a1c071f6a", + "reference": "7efc2f05b7d9817a59132e5d2e5ca91a1c071f6a", + "shasum": "" + }, + "require": { + "composer/ca-bundle": "^1.1", + "php": ">=5.3.0" + }, + "require-dev": { + "phpunit/phpunit": "<8", + "symfony/console": "^2.0 || ^3.0 || ^4.0", + "symfony/filesystem": "^2.0 || ^3.0 || ^4.0", + "symfony/finder": "^2.0 || ^3.0 || ^4.0", + "symfony/yaml": "^2.0 || ^3.0 || ^4.0" + }, + "suggest": { + "symfony/console": "Required for CLI usage - ^2.0 || ^3.0 || ^4.0", + "symfony/filesystem": "Required for CLI usage - 2.0 || ^3.0 || ^4.0", + "symfony/finder": "Required for CLI usage - ^2.0 || ^3.0 || ^4.0", + "symfony/yaml": "Required for CLI usage - ^4.0 || ^5.0" + }, + "bin": [ + "bin/uaparser" + ], + "type": "library", + "autoload": { + "psr-4": { + "UAParser\\": "src" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Dave Olsen", + "email": "dmolsen@gmail.com" + }, + { + "name": "Lars Strojny", + "email": "lars@strojny.net" + } + ], + "description": "A multi-language port of Browserscope's user agent parser.", + "time": "2020-02-21T09:54:14+00:00" + }, { "name": "xemlock/htmlpurifier-html5", "version": "v0.1.11", From 5a949911ba22caefc46648d7abfcd0cd1540ad5f Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Tue, 19 Jan 2021 23:44:19 -0500 Subject: [PATCH 8/8] Add trusted browsers user setting module - Add trusted browsers help section --- doc/Two-Factor-Authentication.md | 9 +- src/Module/Settings/TwoFactor/Index.php | 6 + src/Module/Settings/TwoFactor/Trusted.php | 110 ++++++++++++++++++ static/routes.config.php | 1 + view/templates/settings/twofactor/index.tpl | 1 + .../settings/twofactor/trusted_browsers.tpl | 48 ++++++++ 6 files changed, 174 insertions(+), 1 deletion(-) create mode 100644 src/Module/Settings/TwoFactor/Trusted.php create mode 100644 view/templates/settings/twofactor/trusted_browsers.tpl diff --git a/doc/Two-Factor-Authentication.md b/doc/Two-Factor-Authentication.md index eca6f598c9..2ec33ed29c 100644 --- a/doc/Two-Factor-Authentication.md +++ b/doc/Two-Factor-Authentication.md @@ -71,4 +71,11 @@ Just copy and paste it in your third-party app in the Friendica account password We recommend generating a single app-specific password for each separate third-party app you are using, using a meaningul description of the target app (like "Frienqa on my Fairphone 2"). You can also revoke any and all app-specific password you generated this way. -This may log you out of the third-party application(s) you used the revoked app-specific password to log in with. \ No newline at end of file +This may log you out of the third-party application(s) you used the revoked app-specific password to log in with. + +## Trusted browsers + +As a convenience, during two-factor authentication it is possible to identify a browser as trusted. +This will skip all further two-factor authentication prompt on this browser. + +You can remove any or all of these trusted browsers in the two-factor authentication settings. diff --git a/src/Module/Settings/TwoFactor/Index.php b/src/Module/Settings/TwoFactor/Index.php index bd3840485f..81b4639c78 100644 --- a/src/Module/Settings/TwoFactor/Index.php +++ b/src/Module/Settings/TwoFactor/Index.php @@ -78,6 +78,11 @@ class Index extends BaseSettings DI::baseUrl()->redirect('settings/2fa/app_specific?t=' . self::getFormSecurityToken('settings_2fa_password')); } break; + case 'trusted': + if ($has_secret) { + DI::baseUrl()->redirect('settings/2fa/trusted?t=' . self::getFormSecurityToken('settings_2fa_password')); + } + break; case 'configure': if (!$verified) { DI::baseUrl()->redirect('settings/2fa/verify?t=' . self::getFormSecurityToken('settings_2fa_password')); @@ -130,6 +135,7 @@ class Index extends BaseSettings '$disable_label' => DI::l10n()->t('Disable two-factor authentication'), '$recovery_codes_label' => DI::l10n()->t('Show recovery codes'), '$app_specific_passwords_label' => DI::l10n()->t('Manage app-specific passwords'), + '$trusted_browsers_label' => DI::l10n()->t('Manage trusted browsers'), '$configure_label' => DI::l10n()->t('Finish app configuration'), ]); } diff --git a/src/Module/Settings/TwoFactor/Trusted.php b/src/Module/Settings/TwoFactor/Trusted.php new file mode 100644 index 0000000000..7532509417 --- /dev/null +++ b/src/Module/Settings/TwoFactor/Trusted.php @@ -0,0 +1,110 @@ +get(local_user(), '2fa', 'verified'); + + if (!$verified) { + DI::baseUrl()->redirect('settings/2fa'); + } + + if (!self::checkFormSecurityToken('settings_2fa_password', 't')) { + notice(DI::l10n()->t('Please enter your password to access this page.')); + DI::baseUrl()->redirect('settings/2fa'); + } + } + + public static function post(array $parameters = []) + { + if (!local_user()) { + return; + } + + $trustedBrowserRepository = new TwoFactor\Repository\TrustedBrowser(DI::dba(), DI::logger()); + + if (!empty($_POST['action'])) { + self::checkFormSecurityTokenRedirectOnError('settings/2fa/trusted', 'settings_2fa_trusted'); + + switch ($_POST['action']) { + case 'remove_all' : + $trustedBrowserRepository->removeAllForUser(local_user()); + info(DI::l10n()->t('Trusted browsers successfully removed.')); + DI::baseUrl()->redirect('settings/2fa/trusted?t=' . self::getFormSecurityToken('settings_2fa_password')); + break; + } + } + + if (!empty($_POST['remove_id'])) { + self::checkFormSecurityTokenRedirectOnError('settings/2fa/trusted', 'settings_2fa_trusted'); + + if ($trustedBrowserRepository->removeForUser(local_user(), $_POST['remove_id'])) { + info(DI::l10n()->t('Trusted browser successfully removed.')); + } + + DI::baseUrl()->redirect('settings/2fa/trusted?t=' . self::getFormSecurityToken('settings_2fa_password')); + } + } + + + public static function content(array $parameters = []): string + { + parent::content($parameters); + + $trustedBrowserRepository = new TwoFactor\Repository\TrustedBrowser(DI::dba(), DI::logger()); + $trustedBrowsers = $trustedBrowserRepository->selectAllByUid(local_user()); + + $parser = Parser::create(); + + $trustedBrowserDisplay = array_map(function (TwoFactor\Model\TrustedBrowser $trustedBrowser) use ($parser) { + $dates = [ + 'created_ago' => Temporal::getRelativeDate($trustedBrowser->created), + 'last_used_ago' => Temporal::getRelativeDate($trustedBrowser->last_used), + ]; + + $result = $parser->parse($trustedBrowser->user_agent); + + $uaData = [ + 'os' => $result->os->family, + 'device' => $result->device->family, + 'browser' => $result->ua->family, + ]; + + return $trustedBrowser->toArray() + $dates + $uaData; + }, $trustedBrowsers->getArrayCopy()); + + return Renderer::replaceMacros(Renderer::getMarkupTemplate('settings/twofactor/trusted_browsers.tpl'), [ + '$form_security_token' => self::getFormSecurityToken('settings_2fa_trusted'), + '$password_security_token' => self::getFormSecurityToken('settings_2fa_password'), + + '$title' => DI::l10n()->t('Two-factor Trusted Browsers'), + '$message' => DI::l10n()->t('Trusted browsers are individual browsers you chose to skip two-factor authentication to access Friendica. Please use this feature sparingly, as it can negate the benefit of two-factor authentication.'), + '$device_label' => DI::l10n()->t('Device'), + '$os_label' => DI::l10n()->t('OS'), + '$browser_label' => DI::l10n()->t('Browser'), + '$created_label' => DI::l10n()->t('Trusted'), + '$last_used_label' => DI::l10n()->t('Last Use'), + '$remove_label' => DI::l10n()->t('Remove'), + '$remove_all_label' => DI::l10n()->t('Remove All'), + + '$trusted_browsers' => $trustedBrowserDisplay, + ]); + } +} diff --git a/static/routes.config.php b/static/routes.config.php index 4ad122fbf2..1fd54aecae 100644 --- a/static/routes.config.php +++ b/static/routes.config.php @@ -385,6 +385,7 @@ return [ '/recovery' => [Module\Settings\TwoFactor\Recovery::class, [R::GET, R::POST]], '/app_specific' => [Module\Settings\TwoFactor\AppSpecific::class, [R::GET, R::POST]], '/verify' => [Module\Settings\TwoFactor\Verify::class, [R::GET, R::POST]], + '/trusted' => [Module\Settings\TwoFactor\Trusted::class, [R::GET, R::POST]], ], '/delegation[/{action}/{user_id}]' => [Module\Settings\Delegation::class, [R::GET, R::POST]], '/display' => [Module\Settings\Display::class, [R::GET, R::POST]], diff --git a/view/templates/settings/twofactor/index.tpl b/view/templates/settings/twofactor/index.tpl index 6cf3fac119..1f8f55b6cf 100644 --- a/view/templates/settings/twofactor/index.tpl +++ b/view/templates/settings/twofactor/index.tpl @@ -30,6 +30,7 @@ {{if $has_secret && $verified}}

+

{{/if}} {{if $has_secret && !$verified}}

diff --git a/view/templates/settings/twofactor/trusted_browsers.tpl b/view/templates/settings/twofactor/trusted_browsers.tpl new file mode 100644 index 0000000000..e6d434b4fe --- /dev/null +++ b/view/templates/settings/twofactor/trusted_browsers.tpl @@ -0,0 +1,48 @@ +
+

{{$title}}

+
{{$message nofilter}}
+ +
+ + + + + + + + + + + + + +{{foreach $trusted_browsers as $trusted_browser}} + + + + + + + + +{{/foreach}} + +
{{$device_label}}{{$os_label}}{{$browser_label}}{{$created_label}}{{$last_used_label}}
+ {{$trusted_browser.device}} + + {{$trusted_browser.os}} + + {{$trusted_browser.browser}} + + + + + + + + + + +
+
+