From e4a83eafb80c4613ca15640ff79b7e74d0fc5eae Mon Sep 17 00:00:00 2001 From: Philipp Date: Thu, 7 Jul 2022 21:33:41 +0200 Subject: [PATCH] Add a lot of log-points --- src/Module/Security/TwoFactor/SignOut.php | 2 +- src/Module/Security/TwoFactor/Trust.php | 44 +++++--- src/Module/Settings/TwoFactor/Trusted.php | 32 +++--- .../TrustedBrowserNotFoundException.php | 32 ++++++ .../TrustedBrowserPersistenceException.php | 32 ++++++ .../TwoFactor/Factory/TrustedBrowser.php | 5 + .../TwoFactor/Model/TrustedBrowser.php | 6 ++ .../TwoFactor/Repository/TrustedBrowser.php | 101 ++++++++++++------ 8 files changed, 189 insertions(+), 65 deletions(-) create mode 100644 src/Security/TwoFactor/Exception/TrustedBrowserNotFoundException.php create mode 100644 src/Security/TwoFactor/Exception/TrustedBrowserPersistenceException.php diff --git a/src/Module/Security/TwoFactor/SignOut.php b/src/Module/Security/TwoFactor/SignOut.php index 20b7f039d6..75efc9e60b 100644 --- a/src/Module/Security/TwoFactor/SignOut.php +++ b/src/Module/Security/TwoFactor/SignOut.php @@ -108,7 +108,7 @@ class SignOut extends BaseModule info($this->t('Logged out.')); $this->baseUrl->redirect(); } - } catch (NotFoundException $exception) { + } catch (TwoFactor\Exception\TrustedBrowserNotFoundException $exception) { $this->cookie->clear(); $this->session->clear(); diff --git a/src/Module/Security/TwoFactor/Trust.php b/src/Module/Security/TwoFactor/Trust.php index a245e6be56..e2f54d152a 100644 --- a/src/Module/Security/TwoFactor/Trust.php +++ b/src/Module/Security/TwoFactor/Trust.php @@ -29,7 +29,6 @@ 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; @@ -53,23 +52,24 @@ class Trust extends BaseModule /** @var TwoFactor\Factory\TrustedBrowser */ protected $trustedBrowserFactory; /** @var TwoFactor\Repository\TrustedBrowser */ - protected $trustedBrowserRepositoy; + protected $trustedBrowserRepository; 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; + $this->app = $app; + $this->auth = $auth; + $this->session = $session; + $this->cookie = $cookie; + $this->trustedBrowserFactory = $trustedBrowserFactory; + $this->trustedBrowserRepository = $trustedBrowserRepositoy; } protected function post(array $request = []) { if (!local_user() || !$this->session->get('2fa')) { + $this->logger->info('Invalid call', ['request' => $request]); return; } @@ -82,16 +82,24 @@ class Trust extends BaseModule case 'trust': case 'dont_trust': $trustedBrowser = $this->trustedBrowserFactory->createForUserWithUserAgent(local_user(), $this->server['HTTP_USER_AGENT'], $action === 'trust'); - $this->trustedBrowserRepositoy->save($trustedBrowser); + try { + $this->trustedBrowserRepository->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.')); - }; + // 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.')); + }; + } catch (TwoFactor\Exception\TrustedBrowserPersistenceException $exception) { + $this->logger->warning('Unexpected error when saving the trusted browser.', ['trustedBrowser' => $trustedBrowser, 'exception' => $exception]); + } break; } - $this->auth->setForUser($this->app, User::getById($this->app->getLoggedInUserId()), true, true); + try { + $this->auth->setForUser($this->app, User::getById($this->app->getLoggedInUserId()), true, true); + } catch (\Exception $exception) { + $this->logger->warning('Unexpected error during authentication.', ['user' => $this->app->getLoggedInUserId(), 'exception' => $exception]); + } } } @@ -103,13 +111,17 @@ class Trust extends BaseModule if ($this->cookie->get('2fa_cookie_hash')) { try { - $trustedBrowser = $this->trustedBrowserRepositoy->selectOneByHash($this->cookie->get('2fa_cookie_hash')); + $trustedBrowser = $this->trustedBrowserRepository->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) { + } catch (TwoFactor\Exception\TrustedBrowserNotFoundException $exception) { $this->logger->notice('Trusted Browser of the cookie not found.', ['cookie_hash' => $this->cookie->get('trusted'), 'uid' => $this->app->getLoggedInUserId(), 'exception' => $exception]); + } catch (TwoFactor\Exception\TrustedBrowserPersistenceException $exception) { + $this->logger->warning('Unexpected persistence exception.', ['cookie_hash' => $this->cookie->get('trusted'), 'uid' => $this->app->getLoggedInUserId(), 'exception' => $exception]); + } catch (\Exception $exception) { + $this->logger->warning('Unexpected exception.', ['cookie_hash' => $this->cookie->get('trusted'), 'uid' => $this->app->getLoggedInUserId(), 'exception' => $exception]); } } diff --git a/src/Module/Settings/TwoFactor/Trusted.php b/src/Module/Settings/TwoFactor/Trusted.php index d1c0476de5..140394e077 100644 --- a/src/Module/Settings/TwoFactor/Trusted.php +++ b/src/Module/Settings/TwoFactor/Trusted.php @@ -77,7 +77,7 @@ class Trusted extends BaseSettings self::checkFormSecurityTokenRedirectOnError('settings/2fa/trusted', 'settings_2fa_trusted'); switch ($_POST['action']) { - case 'remove_all' : + case 'remove_all': $this->trustedBrowserRepo->removeAllForUser(local_user()); info($this->t('Trusted browsers successfully removed.')); $this->baseUrl->redirect('settings/2fa/trusted?t=' . self::getFormSecurityToken('settings_2fa_password')); @@ -118,9 +118,9 @@ class Trusted extends BaseSettings $result = $parser->parse($trustedBrowser->user_agent); $uaData = [ - 'os' => $result->os->family, - 'device' => $result->device->family, - 'browser' => $result->ua->family, + 'os' => $result->os->family, + 'device' => $result->device->family, + 'browser' => $result->ua->family, 'trusted_labeled' => $trustedBrowser->trusted ? $this->t('Yes') : $this->t('No'), ]; @@ -128,21 +128,21 @@ class Trusted extends BaseSettings }, $trustedBrowsers->getArrayCopy()); return Renderer::replaceMacros(Renderer::getMarkupTemplate('settings/twofactor/trusted_browsers.tpl'), [ - '$form_security_token' => self::getFormSecurityToken('settings_2fa_trusted'), + '$form_security_token' => self::getFormSecurityToken('settings_2fa_trusted'), '$password_security_token' => self::getFormSecurityToken('settings_2fa_password'), - '$title' => $this->t('Two-factor Trusted Browsers'), - '$message' => $this->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' => $this->t('Device'), - '$os_label' => $this->t('OS'), - '$browser_label' => $this->t('Browser'), - '$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'), + '$title' => $this->t('Two-factor Trusted Browsers'), + '$message' => $this->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' => $this->t('Device'), + '$os_label' => $this->t('OS'), + '$browser_label' => $this->t('Browser'), + '$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'), - '$trusted_browsers' => $trustedBrowserDisplay, + '$trusted_browsers' => $trustedBrowserDisplay, ]); } } diff --git a/src/Security/TwoFactor/Exception/TrustedBrowserNotFoundException.php b/src/Security/TwoFactor/Exception/TrustedBrowserNotFoundException.php new file mode 100644 index 0000000000..4bd0e6db0f --- /dev/null +++ b/src/Security/TwoFactor/Exception/TrustedBrowserNotFoundException.php @@ -0,0 +1,32 @@ +. + * + */ + +namespace Friendica\Security\TwoFactor\Exception; + +use Exception; + +class TrustedBrowserNotFoundException extends \RuntimeException +{ + public function __construct($message = '', Exception $previous = null) + { + parent::__construct($message, 404, $previous); + } +} diff --git a/src/Security/TwoFactor/Exception/TrustedBrowserPersistenceException.php b/src/Security/TwoFactor/Exception/TrustedBrowserPersistenceException.php new file mode 100644 index 0000000000..fb8a4104de --- /dev/null +++ b/src/Security/TwoFactor/Exception/TrustedBrowserPersistenceException.php @@ -0,0 +1,32 @@ +. + * + */ + +namespace Friendica\Security\TwoFactor\Exception; + +use Throwable; + +class TrustedBrowserPersistenceException extends \RuntimeException +{ + public function __construct($message = "", Throwable $previous = null) + { + parent::__construct($message, 500, $previous); + } +} diff --git a/src/Security/TwoFactor/Factory/TrustedBrowser.php b/src/Security/TwoFactor/Factory/TrustedBrowser.php index 21961f7ef3..13e5925006 100644 --- a/src/Security/TwoFactor/Factory/TrustedBrowser.php +++ b/src/Security/TwoFactor/Factory/TrustedBrowser.php @@ -27,6 +27,11 @@ use Friendica\Util\Strings; class TrustedBrowser extends BaseFactory { + /** + * Creates a new Trusted Browser based on the current user environment + * + * @throws \Exception In case something really unexpected happens + */ public function createForUserWithUserAgent(int $uid, string $userAgent, bool $trusted): \Friendica\Security\TwoFactor\Model\TrustedBrowser { $trustedHash = Strings::getRandomHex(); diff --git a/src/Security/TwoFactor/Model/TrustedBrowser.php b/src/Security/TwoFactor/Model/TrustedBrowser.php index cd9d2007e6..7d2bfca30a 100644 --- a/src/Security/TwoFactor/Model/TrustedBrowser.php +++ b/src/Security/TwoFactor/Model/TrustedBrowser.php @@ -67,6 +67,12 @@ class TrustedBrowser extends BaseEntity $this->last_used = $last_used; } + /** + * Records if the trusted browser was used + * + * @return void + * @throws \Exception unexpected DateTime exception happened + */ public function recordUse() { $this->last_used = DateTimeFormat::utcNow(); diff --git a/src/Security/TwoFactor/Repository/TrustedBrowser.php b/src/Security/TwoFactor/Repository/TrustedBrowser.php index 2a62fa7007..d8508bf941 100644 --- a/src/Security/TwoFactor/Repository/TrustedBrowser.php +++ b/src/Security/TwoFactor/Repository/TrustedBrowser.php @@ -21,10 +21,8 @@ namespace Friendica\Security\TwoFactor\Repository; -use Friendica\Security\TwoFactor\Model; -use Friendica\Security\TwoFactor\Collection\TrustedBrowsers; +use Friendica\Security\TwoFactor; use Friendica\Database\Database; -use Friendica\Network\HTTPException\NotFoundException; use Psr\Log\LoggerInterface; class TrustedBrowser @@ -35,83 +33,122 @@ class TrustedBrowser /** @var LoggerInterface */ protected $logger; - /** @var \Friendica\Security\TwoFactor\Factory\TrustedBrowser */ + /** @var TwoFactor\Factory\TrustedBrowser */ protected $factory; protected static $table_name = '2fa_trusted_browser'; - public function __construct(Database $database, LoggerInterface $logger, \Friendica\Security\TwoFactor\Factory\TrustedBrowser $factory = null) + public function __construct(Database $database, LoggerInterface $logger, TwoFactor\Factory\TrustedBrowser $factory = null) { - $this->db = $database; - $this->logger = $logger; - $this->factory = $factory ?? new \Friendica\Security\TwoFactor\Factory\TrustedBrowser($logger); + $this->db = $database; + $this->logger = $logger; + $this->factory = $factory ?? new TwoFactor\Factory\TrustedBrowser($logger); } /** * @param string $cookie_hash - * @return Model\TrustedBrowser|null - * @throws \Exception + * + * @return TwoFactor\Model\TrustedBrowser|null + * + * @throws TwoFactor\Exception\TrustedBrowserPersistenceException + * @throws TwoFactor\Exception\TrustedBrowserNotFoundException */ - public function selectOneByHash(string $cookie_hash): Model\TrustedBrowser + public function selectOneByHash(string $cookie_hash): TwoFactor\Model\TrustedBrowser { - $fields = $this->db->selectFirst(self::$table_name, [], ['cookie_hash' => $cookie_hash]); + try { + $fields = $this->db->selectFirst(self::$table_name, [], ['cookie_hash' => $cookie_hash]); + } catch (\Exception $exception) { + throw new TwoFactor\Exception\TrustedBrowserPersistenceException(sprintf('Internal server error when retrieving cookie hash \'%s\'', $cookie_hash)); + } if (!$this->db->isResult($fields)) { - throw new NotFoundException(''); + throw new TwoFactor\Exception\TrustedBrowserNotFoundException(sprintf('Cookie hash \'%s\' not found', $cookie_hash)); } return $this->factory->createFromTableRow($fields); } - public function selectAllByUid(int $uid): TrustedBrowsers + /** + * @param int $uid + * + * @return TwoFactor\Collection\TrustedBrowsers + * + * @throws TwoFactor\Exception\TrustedBrowserPersistenceException + */ + public function selectAllByUid(int $uid): TwoFactor\Collection\TrustedBrowsers { - $rows = $this->db->selectToArray(self::$table_name, [], ['uid' => $uid]); + try { + $rows = $this->db->selectToArray(self::$table_name, [], ['uid' => $uid]); - $trustedBrowsers = []; - foreach ($rows as $fields) { - $trustedBrowsers[] = $this->factory->createFromTableRow($fields); + $trustedBrowsers = []; + foreach ($rows as $fields) { + $trustedBrowsers[] = $this->factory->createFromTableRow($fields); + } + return new TwoFactor\Collection\TrustedBrowsers($trustedBrowsers); + + } catch (\Exception $exception) { + throw new TwoFactor\Exception\TrustedBrowserPersistenceException(sprintf('selection for uid \'%s\' wasn\'t successful.', $uid)); } - - return new TrustedBrowsers($trustedBrowsers); } /** - * @param Model\TrustedBrowser $trustedBrowser + * @param TwoFactor\Model\TrustedBrowser $trustedBrowser + * * @return bool - * @throws \Exception + * + * @throws TwoFactor\Exception\TrustedBrowserPersistenceException */ - public function save(Model\TrustedBrowser $trustedBrowser): bool + public function save(TwoFactor\Model\TrustedBrowser $trustedBrowser): bool { - return $this->db->insert(self::$table_name, $trustedBrowser->toArray(), $this->db::INSERT_UPDATE); + try { + return $this->db->insert(self::$table_name, $trustedBrowser->toArray(), $this->db::INSERT_UPDATE); + } catch (\Exception $exception) { + throw new TwoFactor\Exception\TrustedBrowserPersistenceException(sprintf('Couldn\'t save trusted Browser with cookie_hash \'%s\'', $trustedBrowser->cookie_hash)); + } } /** - * @param Model\TrustedBrowser $trustedBrowser + * @param TwoFactor\Model\TrustedBrowser $trustedBrowser + * * @return bool - * @throws \Exception + * + * @throws TwoFactor\Exception\TrustedBrowserPersistenceException */ - public function remove(Model\TrustedBrowser $trustedBrowser): bool + public function remove(TwoFactor\Model\TrustedBrowser $trustedBrowser): bool { - return $this->db->delete(self::$table_name, ['cookie_hash' => $trustedBrowser->cookie_hash]); + try { + return $this->db->delete(self::$table_name, ['cookie_hash' => $trustedBrowser->cookie_hash]); + } catch (\Exception $exception) { + throw new TwoFactor\Exception\TrustedBrowserPersistenceException(sprintf('Couldn\'t delete trusted Browser with cookie hash \'%s\'', $trustedBrowser->cookie_hash)); + } } /** * @param int $local_user * @param string $cookie_hash + * * @return bool - * @throws \Exception + * + * @throws TwoFactor\Exception\TrustedBrowserPersistenceException */ public function removeForUser(int $local_user, string $cookie_hash): bool { - return $this->db->delete(self::$table_name, ['cookie_hash' => $cookie_hash,'uid' => $local_user]); + try { + return $this->db->delete(self::$table_name, ['cookie_hash' => $cookie_hash, 'uid' => $local_user]); + } catch (\Exception $exception) { + throw new TwoFactor\Exception\TrustedBrowserPersistenceException(sprintf('Couldn\'t delete trusted Browser for user \'%s\' and cookie hash \'%s\'', $local_user, $cookie_hash)); + } } /** * @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]); + try { + return $this->db->delete(self::$table_name, ['uid' => $local_user]); + } catch (\Exception $exception) { + throw new TwoFactor\Exception\TrustedBrowserPersistenceException(sprintf('Couldn\'t delete trusted Browsers for user \'%s\'', $local_user)); + } } }