From 0223c030a9e7918c055d382ebea44003de289b0f Mon Sep 17 00:00:00 2001 From: Philipp Date: Sat, 25 Jun 2022 14:45:33 +0200 Subject: [PATCH] Improve 2 factor usage --- database.sql | 3 +- doc/database/db_2fa_trusted_browser.md | 15 +- src/Model/User/Cookie.php | 17 ++- src/Module/Security/Logout.php | 18 +-- src/Module/Security/TwoFactor/Signout.php | 129 ++++++++++++++++++ src/Module/Security/TwoFactor/Trust.php | 126 +++++++++++++++++ src/Module/Security/TwoFactor/Verify.php | 76 +++++------ src/Module/Settings/TwoFactor/Index.php | 5 +- src/Module/Settings/TwoFactor/Trusted.php | 4 +- src/Security/Authentication.php | 19 ++- .../TwoFactor/Factory/TrustedBrowser.php | 4 +- .../TwoFactor/Model/TrustedBrowser.php | 14 +- static/dbstructure.config.php | 3 +- static/routes.config.php | 2 + .../TwoFactor/Factory/TrustedBrowserTest.php | 5 +- .../TwoFactor/Model/TrustedBrowserTest.php | 3 + .../settings/twofactor/trusted_browsers.tpl | 4 + view/templates/twofactor/signout.tpl | 14 ++ view/templates/twofactor/trust.tpl | 14 ++ view/templates/twofactor/verify.tpl | 2 - 20 files changed, 400 insertions(+), 77 deletions(-) create mode 100644 src/Module/Security/TwoFactor/Signout.php create mode 100644 src/Module/Security/TwoFactor/Trust.php create mode 100644 view/templates/twofactor/signout.tpl create mode 100644 view/templates/twofactor/trust.tpl diff --git a/database.sql b/database.sql index 10b18d4cae..01bd84b00b 100644 --- a/database.sql +++ b/database.sql @@ -1,6 +1,6 @@ -- ------------------------------------------ -- Friendica 2022.09-dev (Giant Rhubarb) --- DB_UPDATE_VERSION 1472 +-- DB_UPDATE_VERSION 1473 -- ------------------------------------------ @@ -297,6 +297,7 @@ 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', + `trusted` boolean NOT NULL DEFAULT '1' COMMENT 'Whenever this browser should be trusted or not', `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`), diff --git a/doc/database/db_2fa_trusted_browser.md b/doc/database/db_2fa_trusted_browser.md index d12d9e1fc0..18126b49fc 100644 --- a/doc/database/db_2fa_trusted_browser.md +++ b/doc/database/db_2fa_trusted_browser.md @@ -6,13 +6,14 @@ Two-factor authentication trusted browsers Fields ------ -| Field | Description | Type | Null | Key | Default | Extra | -| ----------- | ------------------------------------------ | ------------------ | ---- | --- | ------- | ----- | -| cookie_hash | Trusted cookie hash | varchar(80) | NO | PRI | NULL | | -| uid | User ID | mediumint unsigned | NO | | NULL | | -| user_agent | User agent string | text | YES | | NULL | | -| created | Datetime the trusted browser was recorded | datetime | NO | | NULL | | -| last_used | Datetime the trusted browser was last used | datetime | YES | | NULL | | +| Field | Description | Type | Null | Key | Default | Extra | +| ----------- | ---------------------------------------------- | ------------------ | ---- | --- | ------- | ----- | +| cookie_hash | Trusted cookie hash | varchar(80) | NO | PRI | NULL | | +| uid | User ID | mediumint unsigned | NO | | NULL | | +| user_agent | User agent string | text | YES | | NULL | | +| trusted | Whenever this browser should be trusted or not | boolean | NO | | 1 | | +| created | Datetime the trusted browser was recorded | datetime | NO | | NULL | | +| last_used | Datetime the trusted browser was last used | datetime | YES | | NULL | | Indexes ------------ diff --git a/src/Model/User/Cookie.php b/src/Model/User/Cookie.php index aabb028209..4359d21071 100644 --- a/src/Model/User/Cookie.php +++ b/src/Model/User/Cookie.php @@ -124,6 +124,19 @@ class Cookie } } + /** + * Resets the cookie to a given data set + * + * @param array $data + * + * @return bool + */ + public function reset(array $data): bool + { + return $this->clear() && + $this->setMultiple($data); + } + /** * Clears the Friendica cookie */ @@ -131,7 +144,7 @@ class Cookie { $this->data = []; // make sure cookie is deleted on browser close, as a security measure - return $this->setCookie( '', -3600, $this->sslEnabled); + return $this->setCookie('', -3600, $this->sslEnabled); } /** @@ -161,7 +174,7 @@ class Cookie * */ protected function setCookie(string $value = null, int $expire = null, - bool $secure = null): bool + bool $secure = null): bool { return setcookie(self::NAME, $value, $expire, self::PATH, self::DOMAIN, $secure, self::HTTPONLY); } diff --git a/src/Module/Security/Logout.php b/src/Module/Security/Logout.php index 1ec0764834..004292cb5c 100644 --- a/src/Module/Security/Logout.php +++ b/src/Module/Security/Logout.php @@ -31,7 +31,6 @@ use Friendica\Core\System; use Friendica\Model\Profile; use Friendica\Model\User\Cookie; use Friendica\Module\Response; -use Friendica\Security\TwoFactor; use Friendica\Util\Profiler; use Psr\Log\LoggerInterface; @@ -46,17 +45,14 @@ class Logout extends BaseModule protected $cookie; /** @var IHandleSessions */ protected $session; - /** @var TwoFactor\Repository\TrustedBrowser */ - protected $trustedBrowserRepo; - public function __construct(L10n $l10n, App\BaseURL $baseUrl, App\Arguments $args, LoggerInterface $logger, Profiler $profiler, Response $response, TwoFactor\Repository\TrustedBrowser $trustedBrowserRepo, ICanCache $cache, Cookie $cookie, IHandleSessions $session, array $server, array $parameters = []) + public function __construct(L10n $l10n, App\BaseURL $baseUrl, App\Arguments $args, LoggerInterface $logger, Profiler $profiler, Response $response, ICanCache $cache, Cookie $cookie, IHandleSessions $session, array $server, array $parameters = []) { parent::__construct($l10n, $baseUrl, $args, $logger, $profiler, $response, $server, $parameters); - $this->cache = $cache; - $this->cookie = $cookie; - $this->session = $session; - $this->trustedBrowserRepo = $trustedBrowserRepo; + $this->cache = $cache; + $this->cookie = $cookie; + $this->session = $session; } @@ -73,9 +69,9 @@ 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 ($this->cookie->get('trusted')) { - $this->trustedBrowserRepo->removeForUser(local_user(), $this->cookie->get('trusted')); + // If this is a trusted browser, redirect to the 2fa signout page + if ($this->cookie->get('2fa_cookie_hash')) { + $this->baseUrl->redirect('2fa/signout'); } $this->cookie->clear(); diff --git a/src/Module/Security/TwoFactor/Signout.php b/src/Module/Security/TwoFactor/Signout.php new file mode 100644 index 0000000000..3e52b27e71 --- /dev/null +++ b/src/Module/Security/TwoFactor/Signout.php @@ -0,0 +1,129 @@ +. + * + */ + +namespace Friendica\Module\Security\TwoFactor; + +use Friendica\App; +use Friendica\BaseModule; +use Friendica\Core\L10n; +use Friendica\Core\Renderer; +use Friendica\Core\Session\Capability\IHandleSessions; +use Friendica\Model\User\Cookie; +use Friendica\Module\Response; +use Friendica\Network\HTTPException\NotFoundException; +use Friendica\Util\Profiler; +use Friendica\Security\TwoFactor; +use Psr\Log\LoggerInterface; + +/** + * Page 4: Logout dialog for trusted browsers + * + * @package Friendica\Module\TwoFactor + */ +class Signout extends BaseModule +{ + protected $errors = []; + + /** @var IHandleSessions */ + protected $session; + /** @var Cookie */ + protected $cookie; + /** @var TwoFactor\Repository\TrustedBrowser */ + protected $trustedBrowserRepositoy; + + public function __construct(L10n $l10n, App\BaseURL $baseUrl, App\Arguments $args, LoggerInterface $logger, IHandleSessions $session, Cookie $cookie, TwoFactor\Repository\TrustedBrowser $trustedBrowserRepositoy, Profiler $profiler, Response $response, array $server, array $parameters = []) + { + parent::__construct($l10n, $baseUrl, $args, $logger, $profiler, $response, $server, $parameters); + + $this->session = $session; + $this->cookie = $cookie; + $this->trustedBrowserRepositoy = $trustedBrowserRepositoy; + } + + protected function post(array $request = []) + { + if (!local_user() || !($this->cookie->get('2fa_cookie_hash'))) { + return; + } + + $action = $request['action'] ?? ''; + + if (!empty($action)) { + self::checkFormSecurityTokenRedirectOnError('2fa', 'twofactor_signout'); + + switch ($action) { + case 'trust_and_sign_out': + $trusted = $this->cookie->get('2fa_cookie_hash'); + $this->cookie->reset(['2fa_cookie_hash' => $trusted]); + $this->session->clear(); + + info($this->t('Logged out.')); + $this->baseUrl->redirect(); + break; + case 'sign_out': + $this->trustedBrowserRepositoy->removeForUser(local_user(), $this->cookie->get('2fa_cookie_hash')); + $this->cookie->clear(); + $this->session->clear(); + + info($this->t('Logged out.')); + $this->baseUrl->redirect(); + break; + default: + $this->baseUrl->redirect(); + } + } + } + + protected function content(array $request = []): string + { + if (!local_user() || !($this->cookie->get('2fa_cookie_hash'))) { + $this->baseUrl->redirect(); + } + + try { + $trustedBrowser = $this->trustedBrowserRepositoy->selectOneByHash($this->cookie->get('2fa_cookie_hash')); + if (!$trustedBrowser->trusted) { + $trusted = $this->cookie->get('2fa_cookie_hash'); + $this->cookie->reset(['2fa_cookie_hash' => $trusted]); + $this->session->clear(); + + info($this->t('Logged out.')); + $this->baseUrl->redirect(); + } + } catch (NotFoundException $exception) { + $this->cookie->clear(); + $this->session->clear(); + + info($this->t('Logged out.')); + $this->baseUrl->redirect(); + } + + return Renderer::replaceMacros(Renderer::getMarkupTemplate('twofactor/signout.tpl'), [ + '$form_security_token' => self::getFormSecurityToken('twofactor_signout'), + + '$title' => $this->t('Sign out of this browser?'), + '$message' => $this->t('

If you trust this browser, you will not be asked for verification code the next time you sign in.

'), + '$sign_out_label' => $this->t('Sign out'), + '$cancel_label' => $this->t('Cancel'), + '$trust_and_sign_out_label' => $this->t('Trust and sign out'), + ]); + } +} diff --git a/src/Module/Security/TwoFactor/Trust.php b/src/Module/Security/TwoFactor/Trust.php new file mode 100644 index 0000000000..a245e6be56 --- /dev/null +++ b/src/Module/Security/TwoFactor/Trust.php @@ -0,0 +1,126 @@ +. + * + */ + +namespace Friendica\Module\Security\TwoFactor; + +use Friendica\App; +use Friendica\BaseModule; +use Friendica\Core\L10n; +use Friendica\Core\Renderer; +use Friendica\Core\Session\Capability\IHandleSessions; +use Friendica\Model\User; +use Friendica\Model\User\Cookie; +use Friendica\Module\Response; +use Friendica\Network\HTTPException\NotFoundException; +use Friendica\Security\Authentication; +use Friendica\Util\Profiler; +use Friendica\Security\TwoFactor; +use Psr\Log\LoggerInterface; + +/** + * Page 2: Trust Browser dialog + * + * @package Friendica\Module\TwoFactor + */ +class Trust extends BaseModule +{ + /** @var App */ + protected $app; + /** @var Authentication */ + protected $auth; + /** @var IHandleSessions */ + protected $session; + /** @var Cookie */ + protected $cookie; + /** @var TwoFactor\Factory\TrustedBrowser */ + protected $trustedBrowserFactory; + /** @var TwoFactor\Repository\TrustedBrowser */ + protected $trustedBrowserRepositoy; + + public function __construct(App $app, Authentication $auth, L10n $l10n, App\BaseURL $baseUrl, App\Arguments $args, LoggerInterface $logger, Profiler $profiler, IHandleSessions $session, Cookie $cookie, TwoFactor\Factory\TrustedBrowser $trustedBrowserFactory, TwoFactor\Repository\TrustedBrowser $trustedBrowserRepositoy, Response $response, array $server, array $parameters = []) + { + parent::__construct($l10n, $baseUrl, $args, $logger, $profiler, $response, $server, $parameters); + + $this->app = $app; + $this->auth = $auth; + $this->session = $session; + $this->cookie = $cookie; + $this->trustedBrowserFactory = $trustedBrowserFactory; + $this->trustedBrowserRepositoy = $trustedBrowserRepositoy; + } + + protected function post(array $request = []) + { + if (!local_user() || !$this->session->get('2fa')) { + return; + } + + $action = $request['action'] ?? ''; + + if (!empty($action)) { + self::checkFormSecurityTokenRedirectOnError('2fa', 'twofactor_trust'); + + switch ($action) { + case 'trust': + case 'dont_trust': + $trustedBrowser = $this->trustedBrowserFactory->createForUserWithUserAgent(local_user(), $this->server['HTTP_USER_AGENT'], $action === 'trust'); + $this->trustedBrowserRepositoy->save($trustedBrowser); + + // The string is sent to the browser to be sent back with each request + if (!$this->cookie->set('2fa_cookie_hash', $trustedBrowser->cookie_hash)) { + notice($this->t('Couldn\'t save browser to Cookie.')); + }; + break; + } + + $this->auth->setForUser($this->app, User::getById($this->app->getLoggedInUserId()), true, true); + } + } + + protected function content(array $request = []): string + { + if (!local_user() || !$this->session->get('2fa')) { + $this->baseUrl->redirect(); + } + + if ($this->cookie->get('2fa_cookie_hash')) { + try { + $trustedBrowser = $this->trustedBrowserRepositoy->selectOneByHash($this->cookie->get('2fa_cookie_hash')); + if (!$trustedBrowser->trusted) { + $this->auth->setForUser($this->app, User::getById($this->app->getLoggedInUserId()), true, true); + $this->baseUrl->redirect(); + } + } catch (NotFoundException $exception) { + $this->logger->notice('Trusted Browser of the cookie not found.', ['cookie_hash' => $this->cookie->get('trusted'), 'uid' => $this->app->getLoggedInUserId(), 'exception' => $exception]); + } + } + + return Renderer::replaceMacros(Renderer::getMarkupTemplate('twofactor/trust.tpl'), [ + '$form_security_token' => self::getFormSecurityToken('twofactor_trust'), + + '$title' => $this->t('Trust this browser?'), + '$message' => $this->t('

If you choose to trust this browser, you will not be asked for a verification code the next time you sign in.

'), + '$not_now_label' => $this->t('Not now'), + '$dont_trust_label' => $this->t('Don\'t trust'), + '$trust_label' => $this->t('Trust'), + ]); + } +} diff --git a/src/Module/Security/TwoFactor/Verify.php b/src/Module/Security/TwoFactor/Verify.php index b102823576..d0850aedcb 100644 --- a/src/Module/Security/TwoFactor/Verify.php +++ b/src/Module/Security/TwoFactor/Verify.php @@ -21,13 +21,17 @@ namespace Friendica\Module\Security\TwoFactor; +use Friendica\App; use Friendica\BaseModule; +use Friendica\Core\L10n; +use Friendica\Core\PConfig\Capability\IManagePersonalConfigValues; use Friendica\Core\Renderer; -use Friendica\Core\Session; -use Friendica\DI; -use Friendica\Model\User; +use Friendica\Core\Session\Capability\IHandleSessions; +use Friendica\Module\Response; +use Friendica\Util\Profiler; use PragmaRX\Google2FA\Google2FA; use Friendica\Security\TwoFactor; +use Psr\Log\LoggerInterface; /** * Page 1: Authenticator code verification @@ -36,7 +40,20 @@ use Friendica\Security\TwoFactor; */ class Verify extends BaseModule { - private static $errors = []; + protected $errors = []; + + /** @var IHandleSessions */ + protected $session; + /** @var IManagePersonalConfigValues */ + protected $pConfig; + + public function __construct(L10n $l10n, App\BaseURL $baseUrl, App\Arguments $args, LoggerInterface $logger, Profiler $profiler, Response $response, IManagePersonalConfigValues $pConfig, IHandleSessions $session, array $server, array $parameters = []) + { + parent::__construct($l10n, $baseUrl, $args, $logger, $profiler, $response, $server, $parameters); + + $this->session = $session; + $this->pConfig = $pConfig; + } protected function post(array $request = []) { @@ -44,36 +61,20 @@ class Verify extends BaseModule return; } - if (($_POST['action'] ?? '') == 'verify') { + if (($request['action'] ?? '') === 'verify') { self::checkFormSecurityTokenRedirectOnError('2fa', 'twofactor_verify'); - $a = DI::app(); + $code = $request['verify_code'] ?? ''; - $code = $_POST['verify_code'] ?? ''; - - $valid = (new Google2FA())->verifyKey(DI::pConfig()->get(local_user(), '2fa', 'secret'), $code); + $valid = (new Google2FA())->verifyKey($this->pConfig->get(local_user(), '2fa', 'secret'), $code); // The same code can't be used twice even if it's valid - 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); - } + if ($valid && $this->session->get('2fa') !== $code) { + $this->session->set('2fa', $code); - // Resume normal login workflow - DI::auth()->setForUser($a, User::getById($a->getLoggedInUserId()), true, true); + $this->baseUrl->redirect('2fa/trust'); } else { - self::$errors[] = DI::l10n()->t('Invalid code, please retry.'); + $this->errors[] = $this->t('Invalid code, please retry.'); } } } @@ -81,25 +82,24 @@ class Verify extends BaseModule protected function content(array $request = []): string { if (!local_user()) { - DI::baseUrl()->redirect(); + $this->baseUrl->redirect(); } // Already authenticated with 2FA token - if (Session::get('2fa')) { - DI::baseUrl()->redirect(); + if ($this->session->get('2fa')) { + $this->baseUrl->redirect(); } return Renderer::replaceMacros(Renderer::getMarkupTemplate('twofactor/verify.tpl'), [ '$form_security_token' => self::getFormSecurityToken('twofactor_verify'), - '$title' => DI::l10n()->t('Two-factor authentication'), - '$message' => DI::l10n()->t('

Open the two-factor authentication app on your device to get an authentication code and verify your identity.

'), - '$errors_label' => DI::l10n()->tt('Error', 'Errors', count(self::$errors)), - '$errors' => self::$errors, - '$recovery_message' => DI::l10n()->t('If you do not have access to your authentication code you can use 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="one-time-code" placeholder="000000" inputmode="numeric" pattern="[0-9]*"'], - '$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'), + '$title' => $this->t('Two-factor authentication'), + '$message' => $this->t('

Open the two-factor authentication app on your device to get an authentication code and verify your identity.

'), + '$errors_label' => $this->tt('Error', 'Errors', count($this->errors)), + '$errors' => $this->errors, + '$recovery_message' => $this->t('If you do not have access to your authentication code you can use a two-factor recovery code.', '2fa/recovery'), + '$verify_code' => ['verify_code', $this->t('Please enter a code from your authentication app'), '', '', $this->t('Required'), 'autofocus autocomplete="one-time-code" placeholder="000000" inputmode="numeric" pattern="[0-9]*"'], + '$verify_label' => $this->t('Verify code and complete login'), ]); } } diff --git a/src/Module/Settings/TwoFactor/Index.php b/src/Module/Settings/TwoFactor/Index.php index 35c5d3cf9c..98826824b9 100644 --- a/src/Module/Settings/TwoFactor/Index.php +++ b/src/Module/Settings/TwoFactor/Index.php @@ -24,6 +24,7 @@ namespace Friendica\Module\Settings\TwoFactor; use Friendica\Core\Renderer; use Friendica\Core\Session; use Friendica\DI; +use Friendica\Network\HTTPException\FoundException; use Friendica\Security\TwoFactor\Model\AppSpecificPassword; use Friendica\Security\TwoFactor\Model\RecoveryCode; use Friendica\Model\User; @@ -90,7 +91,9 @@ class Index extends BaseSettings break; } } catch (\Exception $e) { - notice(DI::l10n()->t('Wrong Password')); + if (!($e instanceof FoundException)) { + notice(DI::l10n()->t($e->getMessage())); + } } } diff --git a/src/Module/Settings/TwoFactor/Trusted.php b/src/Module/Settings/TwoFactor/Trusted.php index 12327a5918..d1c0476de5 100644 --- a/src/Module/Settings/TwoFactor/Trusted.php +++ b/src/Module/Settings/TwoFactor/Trusted.php @@ -121,6 +121,7 @@ class Trusted extends BaseSettings 'os' => $result->os->family, 'device' => $result->device->family, 'browser' => $result->ua->family, + 'trusted_labeled' => $trustedBrowser->trusted ? $this->t('Yes') : $this->t('No'), ]; return $trustedBrowser->toArray() + $dates + $uaData; @@ -135,7 +136,8 @@ class Trusted extends BaseSettings '$device_label' => $this->t('Device'), '$os_label' => $this->t('OS'), '$browser_label' => $this->t('Browser'), - '$created_label' => $this->t('Trusted'), + '$trusted_label' => $this->t('Trusted'), + '$created_label' => $this->t('Created At'), '$last_used_label' => $this->t('Last Use'), '$remove_label' => $this->t('Remove'), '$remove_all_label' => $this->t('Remove All'), diff --git a/src/Security/Authentication.php b/src/Security/Authentication.php index aca4f2c23e..a23d0c9557 100644 --- a/src/Security/Authentication.php +++ b/src/Security/Authentication.php @@ -144,7 +144,7 @@ class Authentication // Renew the cookie $this->cookie->send(); - // Do the authentification if not done by now + // Do the authentication if not done by now if (!$this->session->get('authenticated')) { $this->setForUser($a, $user); @@ -269,7 +269,11 @@ class Authentication } if (!$remember) { + $trusted = $this->cookie->get('2fa_cookie_hash') ?? null; $this->cookie->clear(); + if ($trusted) { + $this->cookie->set('2fa_cookie_hash', $trusted); + } } // if we haven't failed up this point, log them in. @@ -407,11 +411,11 @@ class Authentication } // Case 1b: Check for trusted browser - if ($this->cookie->get('trusted')) { + if ($this->cookie->get('2fa_cookie_hash')) { // Retrieve a trusted_browser model based on cookie hash $trustedBrowserRepository = new TrustedBrowser($this->dba, $this->logger); try { - $trustedBrowser = $trustedBrowserRepository->selectOneByHash($this->cookie->get('trusted')); + $trustedBrowser = $trustedBrowserRepository->selectOneByHash($this->cookie->get('2fa_cookie_hash')); // Verify record ownership if ($trustedBrowser->uid === $uid) { // Update last_used date @@ -420,10 +424,13 @@ class Authentication // Save it to the database $trustedBrowserRepository->save($trustedBrowser); - // Set 2fa session key and return - $this->session->set('2fa', true); + // Only use this entry, if its really trusted, otherwise just update the record and proceed + if ($trustedBrowser->trusted) { + // Set 2fa session key and return + $this->session->set('2fa', true); - return; + return; + } } else { // Invalid trusted cookie value, removing it $this->cookie->unset('trusted'); diff --git a/src/Security/TwoFactor/Factory/TrustedBrowser.php b/src/Security/TwoFactor/Factory/TrustedBrowser.php index 61ec154fcc..21961f7ef3 100644 --- a/src/Security/TwoFactor/Factory/TrustedBrowser.php +++ b/src/Security/TwoFactor/Factory/TrustedBrowser.php @@ -27,7 +27,7 @@ use Friendica\Util\Strings; class TrustedBrowser extends BaseFactory { - public function createForUserWithUserAgent($uid, $userAgent): \Friendica\Security\TwoFactor\Model\TrustedBrowser + public function createForUserWithUserAgent(int $uid, string $userAgent, bool $trusted): \Friendica\Security\TwoFactor\Model\TrustedBrowser { $trustedHash = Strings::getRandomHex(); @@ -35,6 +35,7 @@ class TrustedBrowser extends BaseFactory $trustedHash, $uid, $userAgent, + $trusted, DateTimeFormat::utcNow() ); } @@ -45,6 +46,7 @@ class TrustedBrowser extends BaseFactory $row['cookie_hash'], $row['uid'], $row['user_agent'], + $row['trusted'], $row['created'], $row['last_used'] ); diff --git a/src/Security/TwoFactor/Model/TrustedBrowser.php b/src/Security/TwoFactor/Model/TrustedBrowser.php index d0a654d5f2..cd9d2007e6 100644 --- a/src/Security/TwoFactor/Model/TrustedBrowser.php +++ b/src/Security/TwoFactor/Model/TrustedBrowser.php @@ -31,6 +31,7 @@ use Friendica\Util\DateTimeFormat; * @property-read $cookie_hash * @property-read $uid * @property-read $user_agent + * @property-read $trusted * @property-read $created * @property-read $last_used * @package Friendica\Model\TwoFactor @@ -40,6 +41,7 @@ class TrustedBrowser extends BaseEntity protected $cookie_hash; protected $uid; protected $user_agent; + protected $trusted; protected $created; protected $last_used; @@ -51,16 +53,18 @@ class TrustedBrowser extends BaseEntity * @param string $cookie_hash * @param int $uid * @param string $user_agent + * @param bool $trusted * @param string $created * @param string|null $last_used */ - public function __construct(string $cookie_hash, int $uid, string $user_agent, string $created, string $last_used = null) + public function __construct(string $cookie_hash, int $uid, string $user_agent, bool $trusted, string $created, string $last_used = null) { $this->cookie_hash = $cookie_hash; - $this->uid = $uid; - $this->user_agent = $user_agent; - $this->created = $created; - $this->last_used = $last_used; + $this->uid = $uid; + $this->user_agent = $user_agent; + $this->trusted = $trusted; + $this->created = $created; + $this->last_used = $last_used; } public function recordUse() diff --git a/static/dbstructure.config.php b/static/dbstructure.config.php index c48da6a09a..758c33d0da 100644 --- a/static/dbstructure.config.php +++ b/static/dbstructure.config.php @@ -55,7 +55,7 @@ use Friendica\Database\DBA; if (!defined('DB_UPDATE_VERSION')) { - define('DB_UPDATE_VERSION', 1472); + define('DB_UPDATE_VERSION', 1473); } return [ @@ -358,6 +358,7 @@ return [ "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"], + "trusted" => ["type" => "boolean", "not null" => "1", "default" => "1", "comment" => "Whenever this browser should be trusted or not"], "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"], ], diff --git a/static/routes.config.php b/static/routes.config.php index 72964d2308..9c82c8e1f1 100644 --- a/static/routes.config.php +++ b/static/routes.config.php @@ -165,6 +165,8 @@ return [ '/2fa' => [ '[/]' => [Module\Security\TwoFactor\Verify::class, [R::GET, R::POST]], '/recovery' => [Module\Security\TwoFactor\Recovery::class, [R::GET, R::POST]], + '/trust' => [Module\Security\TwoFactor\Trust::class, [R::GET, R::POST]], + '/signout' => [Module\Security\TwoFactor\Signout::class, [R::GET, R::POST]], ], '/api' => [ diff --git a/tests/src/Security/TwoFactor/Factory/TrustedBrowserTest.php b/tests/src/Security/TwoFactor/Factory/TrustedBrowserTest.php index 84a118eb80..baddfb3791 100644 --- a/tests/src/Security/TwoFactor/Factory/TrustedBrowserTest.php +++ b/tests/src/Security/TwoFactor/Factory/TrustedBrowserTest.php @@ -38,6 +38,7 @@ class TrustedBrowserTest extends MockedTest 'uid' => 42, 'user_agent' => 'PHPUnit', 'created' => DateTimeFormat::utcNow(), + 'trusted' => true, 'last_used' => null, ]; @@ -57,6 +58,7 @@ class TrustedBrowserTest extends MockedTest 'uid' => null, 'user_agent' => null, 'created' => null, + 'trusted' => true, 'last_used' => null, ]; @@ -72,11 +74,12 @@ class TrustedBrowserTest extends MockedTest $uid = 42; $userAgent = 'PHPUnit'; - $trustedBrowser = $factory->createForUserWithUserAgent($uid, $userAgent); + $trustedBrowser = $factory->createForUserWithUserAgent($uid, $userAgent, true); $this->assertNotEmpty($trustedBrowser->cookie_hash); $this->assertEquals($uid, $trustedBrowser->uid); $this->assertEquals($userAgent, $trustedBrowser->user_agent); + $this->assertTrue($trustedBrowser->trusted); $this->assertNotEmpty($trustedBrowser->created); } } diff --git a/tests/src/Security/TwoFactor/Model/TrustedBrowserTest.php b/tests/src/Security/TwoFactor/Model/TrustedBrowserTest.php index 0c91ea5e5e..cf5db0ffd8 100644 --- a/tests/src/Security/TwoFactor/Model/TrustedBrowserTest.php +++ b/tests/src/Security/TwoFactor/Model/TrustedBrowserTest.php @@ -36,12 +36,14 @@ class TrustedBrowserTest extends MockedTest $hash, 42, 'PHPUnit', + true, DateTimeFormat::utcNow() ); $this->assertEquals($hash, $trustedBrowser->cookie_hash); $this->assertEquals(42, $trustedBrowser->uid); $this->assertEquals('PHPUnit', $trustedBrowser->user_agent); + $this->assertTrue($trustedBrowser->trusted); $this->assertNotEmpty($trustedBrowser->created); } @@ -54,6 +56,7 @@ class TrustedBrowserTest extends MockedTest $hash, 42, 'PHPUnit', + true, $past, $past ); diff --git a/view/templates/settings/twofactor/trusted_browsers.tpl b/view/templates/settings/twofactor/trusted_browsers.tpl index 29d2ab29c6..1a9ae91fe5 100644 --- a/view/templates/settings/twofactor/trusted_browsers.tpl +++ b/view/templates/settings/twofactor/trusted_browsers.tpl @@ -10,6 +10,7 @@ {{$device_label}} {{$os_label}} {{$browser_label}} + {{$trusted_label}} {{$created_label}} {{$last_used_label}} @@ -28,6 +29,9 @@ {{$trusted_browser.browser}} + {{$trusted_browser.trusted_labeled}} + + diff --git a/view/templates/twofactor/signout.tpl b/view/templates/twofactor/signout.tpl new file mode 100644 index 0000000000..f2a211102e --- /dev/null +++ b/view/templates/twofactor/signout.tpl @@ -0,0 +1,14 @@ +
+

{{$title}}

+
{{$message nofilter}}
+ +
+ + +
+ + + +
+
+
diff --git a/view/templates/twofactor/trust.tpl b/view/templates/twofactor/trust.tpl new file mode 100644 index 0000000000..6bfe307d5e --- /dev/null +++ b/view/templates/twofactor/trust.tpl @@ -0,0 +1,14 @@ +
+

{{$title}}

+
{{$message nofilter}}
+ +
+ + +
+ + + +
+
+
diff --git a/view/templates/twofactor/verify.tpl b/view/templates/twofactor/verify.tpl index 938f98da09..2b1fe31421 100644 --- a/view/templates/twofactor/verify.tpl +++ b/view/templates/twofactor/verify.tpl @@ -18,8 +18,6 @@ {{include file="field_input.tpl" field=$verify_code}} - {{include file="field_checkbox.tpl" field=$trust_browser}} -
-- 2.39.5