From d5c472e8f698c768c3da64beacc0ba673599b096 Mon Sep 17 00:00:00 2001 From: Oleksandr Gorkun Date: Thu, 27 Feb 2020 16:53:51 -0600 Subject: [PATCH 01/26] MC-22950: Enable 2FA by default for Admins --- TwoFactorAuth/Api/Data/TrustedInterface.php | 3 + .../Data/TrustedSearchResultsInterface.php | 2 + TwoFactorAuth/Api/EngineInterface.php | 3 + .../NotificationExceptionInterface.php | 17 ++ TwoFactorAuth/Api/ProviderInterface.php | 3 + TwoFactorAuth/Api/TfaInterface.php | 7 +- TwoFactorAuth/Api/TrustedManagerInterface.php | 10 + .../Api/TrustedRepositoryInterface.php | 12 + .../Api/UserConfigRequestManagerInterface.php | 37 +++ .../Api/UserConfigTokenManagerInterface.php | 32 +++ TwoFactorAuth/Api/UserNotifierInterface.php | 38 +++ TwoFactorAuth/Block/Configure.php | 69 +++++ TwoFactorAuth/Command/TfaDisable.php | 18 +- .../Adminhtml/AbstractConfigureAction.php | 46 ++++ .../Controller/Adminhtml/Authy/Authpost.php | 12 +- .../Controller/Adminhtml/Authy/Configure.php | 15 +- .../Adminhtml/Authy/Configurepost.php | 15 +- .../Adminhtml/Authy/Configureverifypost.php | 15 +- .../Adminhtml/Authy/Verifyonetouch.php | 12 +- .../Controller/Adminhtml/Duo/Auth.php | 24 +- .../Controller/Adminhtml/Duo/Authpost.php | 32 ++- .../Controller/Adminhtml/Google/Authpost.php | 41 +-- .../Controller/Adminhtml/Google/Configure.php | 15 +- .../Adminhtml/Google/Configurepost.php | 15 +- .../Controller/Adminhtml/Tfa/Configure.php | 71 ++++++ .../Adminhtml/Tfa/Configurepost.php | 137 ++++++++++ .../Controller/Adminhtml/Tfa/Index.php | 26 +- .../Adminhtml/Tfa/Requestconfig.php | 103 ++++++++ .../Controller/Adminhtml/Tfa/Revoke.php | 22 +- .../Controller/Adminhtml/U2f/Auth.php | 17 +- .../Controller/Adminhtml/U2f/Authpost.php | 10 +- .../Controller/Adminhtml/U2f/Configure.php | 19 +- .../Adminhtml/U2f/Configurepost.php | 21 +- .../Model/Config/Backend/ForceProviders.php | 31 +++ TwoFactorAuth/Model/EmailUserNotifier.php | 115 +++++++++ .../Model/Exception/NotificationException.php | 19 ++ TwoFactorAuth/Model/Provider.php | 2 +- TwoFactorAuth/Model/Provider/Engine/Authy.php | 19 +- .../Model/Provider/Engine/DuoSecurity.php | 21 +- .../Model/Provider/Engine/Google.php | 52 ++-- .../Model/Provider/Engine/U2fKey.php | 23 +- .../ResourceModel/Trusted/Collection.php | 1 + .../Model/ResourceModel/TrustedRepository.php | 11 +- TwoFactorAuth/Model/Tfa.php | 79 +++--- TwoFactorAuth/Model/TrustedManager.php | 8 + TwoFactorAuth/Model/TrustedRegistry.php | 1 + .../UserConfig/HtmlAreaTokenVerifier.php | 116 +++++++++ .../Model/UserConfig/SignedTokenManager.php | 86 +++++++ .../UserConfig/UserConfigRequestManager.php | 88 +++++++ .../Observer/BackendAuthUserLoginSuccess.php | 53 ---- .../Observer/ControllerActionPredispatch.php | 88 ++++--- .../Adminhtml/Authy/ConfigureTest.php | 55 ++++ .../Adminhtml/Authy/ConfigurepostTest.php | 55 ++++ .../Authy/ConfigureverifypostTest.php | 55 ++++ .../Controller/Adminhtml/Duo/AuthTest.php | 64 +++++ .../Controller/Adminhtml/Duo/AuthpostTest.php | 80 ++++++ .../Adminhtml/Google/ConfigureTest.php | 52 ++++ .../Adminhtml/Google/ConfigurepostTest.php | 52 ++++ .../Adminhtml/Tfa/ConfigureTest.php | 92 +++++++ .../Adminhtml/Tfa/ConfigurepostTest.php | 154 ++++++++++++ .../Controller/Adminhtml/Tfa/IndexTest.php | 77 ++++++ .../Adminhtml/Tfa/RequestconfigTest.php | 110 ++++++++ .../Adminhtml/U2f/ConfigureTest.php | 52 ++++ .../Adminhtml/U2f/ConfigurepostTest.php | 52 ++++ .../ControllerActionPredispatchTest.php | 153 ++++++++++++ .../UserConfigRequestManagerTest.php | 171 +++++++++++++ .../UserConfigTokenManagerTest.php | 84 +++++++ .../Config/Backend/ForceProvidersTest.php | 48 ++++ .../Unit/Model/Provider/Engine/AuthyTest.php | 77 ++++++ .../Model/Provider/Engine/DuoSecurityTest.php | 127 ++++++++++ .../Unit/Model/Provider/Engine/GoogleTest.php | 45 ++++ .../Unit/Model/Provider/Engine/U2fKeyTest.php | 45 ++++ .../Test/Unit/Model/ProviderTest.php | 44 ++++ TwoFactorAuth/Test/Unit/Model/TfaTest.php | 236 ++++++++++++++++++ .../UserConfig/HtmlAreaTokenVerifierTest.php | 200 +++++++++++++++ .../ControllerActionPredispatch.php | 32 +++ .../TestCase/AbstractBackendController.php | 63 +++++ .../AbstractConfigureBackendController.php | 72 ++++++ .../Ui/Component/Form/User/DataProvider.php | 32 +-- TwoFactorAuth/etc/adminhtml/events.xml | 3 - TwoFactorAuth/etc/adminhtml/system.xml | 62 +---- TwoFactorAuth/etc/config.xml | 8 - TwoFactorAuth/etc/di.xml | 7 +- TwoFactorAuth/etc/email_templates.xml | 11 + .../adminhtml/email/app_config_required.html | 22 ++ .../adminhtml/email/user_config_required.html | 25 ++ .../view/adminhtml/layout/tfa_authy_auth.xml | 6 - .../view/adminhtml/layout/tfa_google_auth.xml | 6 - .../adminhtml/layout/tfa_tfa_configure.xml | 23 ++ .../layout/tfa_tfa_requestconfig.xml | 18 ++ .../view/adminhtml/layout/tfa_u2f_auth.xml | 6 - .../adminhtml/templates/tfa/configure.phtml | 40 +++ .../templates/tfa/request_config.phtml | 12 + .../ui_component/tfa_edit_user_form.xml | 19 -- TwoFactorAuth/view/adminhtml/web/css/auth.css | 6 + .../view/adminhtml/web/js/authy/auth.js | 4 +- .../view/adminhtml/web/js/authy/configure.js | 4 +- .../web/js/authy/configure/register.js | 4 +- .../web/js/authy/configure/registry.js | 4 +- .../web/js/authy/configure/verify.js | 4 +- .../view/adminhtml/web/js/change_provider.js | 4 +- .../view/adminhtml/web/js/duo/api.js | 3 +- .../view/adminhtml/web/js/duo/auth.js | 4 +- TwoFactorAuth/view/adminhtml/web/js/error.js | 4 +- .../web/js/form/element/providers.js | 6 +- .../web/js/form/element/reset_providers.js | 4 +- .../web/js/form/element/trusted_devices.js | 4 +- .../view/adminhtml/web/js/form/provider.js | 4 +- .../view/adminhtml/web/js/google/auth.js | 8 +- .../view/adminhtml/web/js/registry.js | 16 +- .../view/adminhtml/web/js/trust_device.js | 4 +- .../view/adminhtml/web/js/u2fkey/auth.js | 5 +- .../view/adminhtml/web/js/u2fkey/configure.js | 4 +- 113 files changed, 3956 insertions(+), 519 deletions(-) create mode 100644 TwoFactorAuth/Api/Exception/NotificationExceptionInterface.php create mode 100644 TwoFactorAuth/Api/UserConfigRequestManagerInterface.php create mode 100644 TwoFactorAuth/Api/UserConfigTokenManagerInterface.php create mode 100644 TwoFactorAuth/Api/UserNotifierInterface.php create mode 100644 TwoFactorAuth/Block/Configure.php create mode 100644 TwoFactorAuth/Controller/Adminhtml/AbstractConfigureAction.php create mode 100644 TwoFactorAuth/Controller/Adminhtml/Tfa/Configure.php create mode 100644 TwoFactorAuth/Controller/Adminhtml/Tfa/Configurepost.php create mode 100644 TwoFactorAuth/Controller/Adminhtml/Tfa/Requestconfig.php create mode 100644 TwoFactorAuth/Model/Config/Backend/ForceProviders.php create mode 100644 TwoFactorAuth/Model/EmailUserNotifier.php create mode 100644 TwoFactorAuth/Model/Exception/NotificationException.php create mode 100644 TwoFactorAuth/Model/UserConfig/HtmlAreaTokenVerifier.php create mode 100644 TwoFactorAuth/Model/UserConfig/SignedTokenManager.php create mode 100644 TwoFactorAuth/Model/UserConfig/UserConfigRequestManager.php delete mode 100644 TwoFactorAuth/Observer/BackendAuthUserLoginSuccess.php create mode 100644 TwoFactorAuth/Test/Integration/Controller/Adminhtml/Authy/ConfigureTest.php create mode 100644 TwoFactorAuth/Test/Integration/Controller/Adminhtml/Authy/ConfigurepostTest.php create mode 100644 TwoFactorAuth/Test/Integration/Controller/Adminhtml/Authy/ConfigureverifypostTest.php create mode 100644 TwoFactorAuth/Test/Integration/Controller/Adminhtml/Duo/AuthTest.php create mode 100644 TwoFactorAuth/Test/Integration/Controller/Adminhtml/Duo/AuthpostTest.php create mode 100644 TwoFactorAuth/Test/Integration/Controller/Adminhtml/Google/ConfigureTest.php create mode 100644 TwoFactorAuth/Test/Integration/Controller/Adminhtml/Google/ConfigurepostTest.php create mode 100644 TwoFactorAuth/Test/Integration/Controller/Adminhtml/Tfa/ConfigureTest.php create mode 100644 TwoFactorAuth/Test/Integration/Controller/Adminhtml/Tfa/ConfigurepostTest.php create mode 100644 TwoFactorAuth/Test/Integration/Controller/Adminhtml/Tfa/IndexTest.php create mode 100644 TwoFactorAuth/Test/Integration/Controller/Adminhtml/Tfa/RequestconfigTest.php create mode 100644 TwoFactorAuth/Test/Integration/Controller/Adminhtml/U2f/ConfigureTest.php create mode 100644 TwoFactorAuth/Test/Integration/Controller/Adminhtml/U2f/ConfigurepostTest.php create mode 100644 TwoFactorAuth/Test/Integration/ControllerActionPredispatchTest.php create mode 100644 TwoFactorAuth/Test/Integration/UserConfigRequestManagerTest.php create mode 100644 TwoFactorAuth/Test/Integration/UserConfigTokenManagerTest.php create mode 100644 TwoFactorAuth/Test/Unit/Model/Config/Backend/ForceProvidersTest.php create mode 100644 TwoFactorAuth/Test/Unit/Model/Provider/Engine/AuthyTest.php create mode 100644 TwoFactorAuth/Test/Unit/Model/Provider/Engine/DuoSecurityTest.php create mode 100644 TwoFactorAuth/Test/Unit/Model/Provider/Engine/GoogleTest.php create mode 100644 TwoFactorAuth/Test/Unit/Model/Provider/Engine/U2fKeyTest.php create mode 100644 TwoFactorAuth/Test/Unit/Model/ProviderTest.php create mode 100644 TwoFactorAuth/Test/Unit/Model/TfaTest.php create mode 100644 TwoFactorAuth/Test/Unit/Model/UserConfig/HtmlAreaTokenVerifierTest.php create mode 100644 TwoFactorAuth/TestFramework/ControllerActionPredispatch.php create mode 100644 TwoFactorAuth/TestFramework/TestCase/AbstractBackendController.php create mode 100644 TwoFactorAuth/TestFramework/TestCase/AbstractConfigureBackendController.php create mode 100644 TwoFactorAuth/etc/email_templates.xml create mode 100644 TwoFactorAuth/view/adminhtml/email/app_config_required.html create mode 100644 TwoFactorAuth/view/adminhtml/email/user_config_required.html create mode 100644 TwoFactorAuth/view/adminhtml/layout/tfa_tfa_configure.xml create mode 100644 TwoFactorAuth/view/adminhtml/layout/tfa_tfa_requestconfig.xml create mode 100644 TwoFactorAuth/view/adminhtml/templates/tfa/configure.phtml create mode 100644 TwoFactorAuth/view/adminhtml/templates/tfa/request_config.phtml diff --git a/TwoFactorAuth/Api/Data/TrustedInterface.php b/TwoFactorAuth/Api/Data/TrustedInterface.php index 6633ef95..5cc48592 100644 --- a/TwoFactorAuth/Api/Data/TrustedInterface.php +++ b/TwoFactorAuth/Api/Data/TrustedInterface.php @@ -11,6 +11,8 @@ /** * Trusted platform entity interface + * + * @deprecated Trusted Devices functionality was removed. */ interface TrustedInterface extends ExtensibleDataInterface { @@ -51,6 +53,7 @@ interface TrustedInterface extends ExtensibleDataInterface /** * Get value for tfa_trusted_id + * * @return int */ public function getId(): int; diff --git a/TwoFactorAuth/Api/Data/TrustedSearchResultsInterface.php b/TwoFactorAuth/Api/Data/TrustedSearchResultsInterface.php index e7ad6cb4..825508a3 100644 --- a/TwoFactorAuth/Api/Data/TrustedSearchResultsInterface.php +++ b/TwoFactorAuth/Api/Data/TrustedSearchResultsInterface.php @@ -11,6 +11,8 @@ /** * Trusted devices search results interface + * + * @deprecated Trusted Devices functionality was removed. */ interface TrustedSearchResultsInterface extends SearchResultsInterface { diff --git a/TwoFactorAuth/Api/EngineInterface.php b/TwoFactorAuth/Api/EngineInterface.php index 4e3f4b8b..77bc72bb 100644 --- a/TwoFactorAuth/Api/EngineInterface.php +++ b/TwoFactorAuth/Api/EngineInterface.php @@ -17,13 +17,16 @@ interface EngineInterface { /** * Return true if this provider has been enabled by admin + * * @return bool */ public function isEnabled(): bool; /** * Return true if this provider allows trusted devices + * * @return bool + * @deprecated Trusted Devices functionality is deprecated. */ public function isTrustedDevicesAllowed(): bool; diff --git a/TwoFactorAuth/Api/Exception/NotificationExceptionInterface.php b/TwoFactorAuth/Api/Exception/NotificationExceptionInterface.php new file mode 100644 index 00000000..86bfed07 --- /dev/null +++ b/TwoFactorAuth/Api/Exception/NotificationExceptionInterface.php @@ -0,0 +1,17 @@ +tfa = $tfa; + } + + /** + * Create list of providers for user to choose. + * + * @return array + */ + public function getProvidersList(): array + { + $selected = $this->tfa->getForcedProviders(); + $list = []; + foreach ($this->tfa->getAllEnabledProviders() as $provider) { + $list[] = [ + 'code' => $provider->getCode(), + 'name' => $provider->getName(), + 'icon' => $this->getViewFileUrl($provider->getIcon()), + 'selected' => in_array($provider, $selected, true) + ]; + } + + return $list; + } + + /** + * Get the form's action URL. + * + * @return string + */ + public function getActionUrl(): string + { + return $this->getUrl('tfa/tfa/configurepost'); + } +} diff --git a/TwoFactorAuth/Command/TfaDisable.php b/TwoFactorAuth/Command/TfaDisable.php index e110e5ae..cd2a3427 100644 --- a/TwoFactorAuth/Command/TfaDisable.php +++ b/TwoFactorAuth/Command/TfaDisable.php @@ -12,10 +12,11 @@ use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; -use Magento\TwoFactorAuth\Api\TfaInterface; /** - * Disable 2FA commandline + * Disable 2FA command + * + * @deprecated 2FA cannot be disabled anymore. */ class TfaDisable extends Command { @@ -48,23 +49,18 @@ public function __construct( protected function configure() { $this->setName('security:tfa:disable'); - $this->setDescription('Globally disable two factor auth'); + $this->setDescription('[DEPRECATED] Globally disable two factor auth'); parent::configure(); } /** + * @inheritDoc + * * @SuppressWarnings("PHPMD.UnusedFormalParameter") */ protected function execute(InputInterface $input, OutputInterface $output) { - $this->config->saveConfig( - TfaInterface::XML_PATH_ENABLED, - '0', - 'default', - 0 - ); - - $this->cacheManager->flush(['config']); + trigger_error('Disabling 2FA is deprecated', E_USER_DEPRECATED); } } diff --git a/TwoFactorAuth/Controller/Adminhtml/AbstractConfigureAction.php b/TwoFactorAuth/Controller/Adminhtml/AbstractConfigureAction.php new file mode 100644 index 00000000..e7892d60 --- /dev/null +++ b/TwoFactorAuth/Controller/Adminhtml/AbstractConfigureAction.php @@ -0,0 +1,46 @@ +tokenVerifier = $tokenVerifier; + } + + /** + * @inheritDoc + */ + protected function _isAllowed() + { + $isAllowed = parent::_isAllowed(); + if ($isAllowed) { + $isAllowed = $this->tokenVerifier->isConfigTokenProvided(); + } + + return $isAllowed; + } +} diff --git a/TwoFactorAuth/Controller/Adminhtml/Authy/Authpost.php b/TwoFactorAuth/Controller/Adminhtml/Authy/Authpost.php index 72e7085a..c4a2874c 100644 --- a/TwoFactorAuth/Controller/Adminhtml/Authy/Authpost.php +++ b/TwoFactorAuth/Controller/Adminhtml/Authy/Authpost.php @@ -16,7 +16,6 @@ use Magento\TwoFactorAuth\Model\AlertInterface; use Magento\TwoFactorAuth\Api\TfaInterface; use Magento\TwoFactorAuth\Api\TfaSessionInterface; -use Magento\TwoFactorAuth\Api\TrustedManagerInterface; use Magento\TwoFactorAuth\Controller\Adminhtml\AbstractAction; use Magento\TwoFactorAuth\Model\Provider\Engine\Authy; use Magento\User\Model\User; @@ -46,11 +45,6 @@ class Authpost extends AbstractAction implements HttpPostActionInterface */ private $tfaSession; - /** - * @var TrustedManagerInterface - */ - private $trustedManager; - /** * @var Authy */ @@ -72,7 +66,7 @@ class Authpost extends AbstractAction implements HttpPostActionInterface * @param JsonFactory $jsonFactory * @param Authy $authy * @param TfaSessionInterface $tfaSession - * @param TrustedManagerInterface $trustedManager + * @param \Magento\TwoFactorAuth\Api\TrustedManagerInterface $trustedManager * @param TfaInterface $tfa * @param AlertInterface $alert * @param DataObjectFactory $dataObjectFactory @@ -83,7 +77,7 @@ public function __construct( JsonFactory $jsonFactory, Authy $authy, TfaSessionInterface $tfaSession, - TrustedManagerInterface $trustedManager, + \Magento\TwoFactorAuth\Api\TrustedManagerInterface $trustedManager, TfaInterface $tfa, AlertInterface $alert, DataObjectFactory $dataObjectFactory @@ -93,7 +87,6 @@ public function __construct( $this->session = $session; $this->jsonFactory = $jsonFactory; $this->tfaSession = $tfaSession; - $this->trustedManager = $trustedManager; $this->authy = $authy; $this->dataObjectFactory = $dataObjectFactory; $this->alert = $alert; @@ -120,7 +113,6 @@ public function execute() $this->authy->verify($user, $this->dataObjectFactory->create([ 'data' => $this->getRequest()->getParams(), ])); - $this->trustedManager->handleTrustDeviceRequest(Authy::CODE, $this->getRequest()); $this->tfaSession->grantAccess(); $result->setData(['success' => true]); } catch (Exception $e) { diff --git a/TwoFactorAuth/Controller/Adminhtml/Authy/Configure.php b/TwoFactorAuth/Controller/Adminhtml/Authy/Configure.php index 0f28e30b..a35c52ab 100644 --- a/TwoFactorAuth/Controller/Adminhtml/Authy/Configure.php +++ b/TwoFactorAuth/Controller/Adminhtml/Authy/Configure.php @@ -12,14 +12,15 @@ use Magento\Framework\App\Action\HttpGetActionInterface; use Magento\Framework\View\Result\PageFactory; use Magento\TwoFactorAuth\Api\TfaInterface; -use Magento\TwoFactorAuth\Controller\Adminhtml\AbstractAction; +use Magento\TwoFactorAuth\Controller\Adminhtml\AbstractConfigureAction; use Magento\TwoFactorAuth\Model\Provider\Engine\Authy; use Magento\User\Model\User; +use Magento\TwoFactorAuth\Model\UserConfig\HtmlAreaTokenVerifier; /** * @SuppressWarnings(PHPMD.CamelCaseMethodName) */ -class Configure extends AbstractAction implements HttpGetActionInterface +class Configure extends AbstractConfigureAction implements HttpGetActionInterface { /** * @var PageFactory @@ -41,14 +42,16 @@ class Configure extends AbstractAction implements HttpGetActionInterface * @param Session $session * @param TfaInterface $tfa * @param PageFactory $pageFactory + * @param HtmlAreaTokenVerifier $tokenVerifier */ public function __construct( Action\Context $context, Session $session, TfaInterface $tfa, - PageFactory $pageFactory + PageFactory $pageFactory, + HtmlAreaTokenVerifier $tokenVerifier ) { - parent::__construct($context); + parent::__construct($context, $tokenVerifier); $this->pageFactory = $pageFactory; $this->session = $session; $this->tfa = $tfa; @@ -76,6 +79,10 @@ public function execute() */ protected function _isAllowed() { + if (!parent::_isAllowed()) { + return false; + } + $user = $this->getUser(); return diff --git a/TwoFactorAuth/Controller/Adminhtml/Authy/Configurepost.php b/TwoFactorAuth/Controller/Adminhtml/Authy/Configurepost.php index b8039475..63efa4f6 100644 --- a/TwoFactorAuth/Controller/Adminhtml/Authy/Configurepost.php +++ b/TwoFactorAuth/Controller/Adminhtml/Authy/Configurepost.php @@ -15,14 +15,15 @@ use Magento\Framework\View\Result\PageFactory; use Magento\TwoFactorAuth\Model\AlertInterface; use Magento\TwoFactorAuth\Api\TfaInterface; -use Magento\TwoFactorAuth\Controller\Adminhtml\AbstractAction; +use Magento\TwoFactorAuth\Controller\Adminhtml\AbstractConfigureAction; use Magento\TwoFactorAuth\Model\Provider\Engine\Authy; use Magento\User\Model\User; +use Magento\TwoFactorAuth\Model\UserConfig\HtmlAreaTokenVerifier; /** * @SuppressWarnings(PHPMD.CamelCaseMethodName) */ -class Configurepost extends AbstractAction implements HttpPostActionInterface +class Configurepost extends AbstractConfigureAction implements HttpPostActionInterface { /** * @var PageFactory @@ -56,6 +57,7 @@ class Configurepost extends AbstractAction implements HttpPostActionInterface * @param TfaInterface $tfa * @param AlertInterface $alert * @param JsonFactory $jsonFactory + * @param HtmlAreaTokenVerifier $tokenVerifier */ public function __construct( Action\Context $context, @@ -63,9 +65,10 @@ public function __construct( Authy\Verification $verification, TfaInterface $tfa, AlertInterface $alert, - JsonFactory $jsonFactory + JsonFactory $jsonFactory, + HtmlAreaTokenVerifier $tokenVerifier ) { - parent::__construct($context); + parent::__construct($context, $tokenVerifier); $this->jsonFactory = $jsonFactory; $this->session = $session; $this->tfa = $tfa; @@ -131,6 +134,10 @@ public function execute() */ protected function _isAllowed() { + if (!parent::_isAllowed()) { + return false; + } + $user = $this->getUser(); return diff --git a/TwoFactorAuth/Controller/Adminhtml/Authy/Configureverifypost.php b/TwoFactorAuth/Controller/Adminhtml/Authy/Configureverifypost.php index 082298f3..6b6c091e 100644 --- a/TwoFactorAuth/Controller/Adminhtml/Authy/Configureverifypost.php +++ b/TwoFactorAuth/Controller/Adminhtml/Authy/Configureverifypost.php @@ -15,14 +15,15 @@ use Magento\TwoFactorAuth\Model\AlertInterface; use Magento\TwoFactorAuth\Api\TfaInterface; use Magento\TwoFactorAuth\Api\TfaSessionInterface; -use Magento\TwoFactorAuth\Controller\Adminhtml\AbstractAction; +use Magento\TwoFactorAuth\Controller\Adminhtml\AbstractConfigureAction; use Magento\TwoFactorAuth\Model\Provider\Engine\Authy; use Magento\User\Model\User; +use Magento\TwoFactorAuth\Model\UserConfig\HtmlAreaTokenVerifier; /** * @SuppressWarnings(PHPMD.CamelCaseMethodName) */ -class Configureverifypost extends AbstractAction implements HttpPostActionInterface +class Configureverifypost extends AbstractConfigureAction implements HttpPostActionInterface { /** * @var JsonFactory @@ -68,6 +69,7 @@ class Configureverifypost extends AbstractAction implements HttpPostActionInterf * @param Authy $authy * @param Authy\Verification $verification * @param JsonFactory $jsonFactory + * @param HtmlAreaTokenVerifier $tokenVerifier */ public function __construct( Action\Context $context, @@ -77,9 +79,10 @@ public function __construct( AlertInterface $alert, Authy $authy, Authy\Verification $verification, - JsonFactory $jsonFactory + JsonFactory $jsonFactory, + HtmlAreaTokenVerifier $tokenVerifier ) { - parent::__construct($context); + parent::__construct($context, $tokenVerifier); $this->jsonFactory = $jsonFactory; $this->session = $session; $this->tfa = $tfa; @@ -144,6 +147,10 @@ public function execute() */ protected function _isAllowed() { + if (!parent::_isAllowed()) { + return false; + } + $user = $this->getUser(); return diff --git a/TwoFactorAuth/Controller/Adminhtml/Authy/Verifyonetouch.php b/TwoFactorAuth/Controller/Adminhtml/Authy/Verifyonetouch.php index 9f11b136..b44b0b12 100644 --- a/TwoFactorAuth/Controller/Adminhtml/Authy/Verifyonetouch.php +++ b/TwoFactorAuth/Controller/Adminhtml/Authy/Verifyonetouch.php @@ -16,7 +16,6 @@ use Magento\TwoFactorAuth\Model\AlertInterface; use Magento\TwoFactorAuth\Api\TfaInterface; use Magento\TwoFactorAuth\Api\TfaSessionInterface; -use Magento\TwoFactorAuth\Api\TrustedManagerInterface; use Magento\TwoFactorAuth\Controller\Adminhtml\AbstractAction; use Magento\TwoFactorAuth\Model\Provider\Engine\Authy; use Magento\User\Model\User; @@ -41,11 +40,6 @@ class Verifyonetouch extends AbstractAction implements HttpGetActionInterface, H */ private $tfa; - /** - * @var TrustedManagerInterface - */ - private $trustedManager; - /** * @var TfaSessionInterface */ @@ -65,7 +59,7 @@ class Verifyonetouch extends AbstractAction implements HttpGetActionInterface, H * Verifyonetouch constructor. * @param Action\Context $context * @param JsonFactory $jsonFactory - * @param TrustedManagerInterface $trustedManager + * @param \Magento\TwoFactorAuth\Api\TrustedManagerInterface $trustedManager * @param TfaSessionInterface $tfaSession * @param TfaInterface $tfa * @param AlertInterface $alert @@ -75,7 +69,7 @@ class Verifyonetouch extends AbstractAction implements HttpGetActionInterface, H public function __construct( Action\Context $context, JsonFactory $jsonFactory, - TrustedManagerInterface $trustedManager, + \Magento\TwoFactorAuth\Api\TrustedManagerInterface $trustedManager, TfaSessionInterface $tfaSession, TfaInterface $tfa, AlertInterface $alert, @@ -86,7 +80,6 @@ public function __construct( $this->session = $session; $this->jsonFactory = $jsonFactory; $this->tfa = $tfa; - $this->trustedManager = $trustedManager; $this->tfaSession = $tfaSession; $this->alert = $alert; $this->oneTouch = $oneTouch; @@ -111,7 +104,6 @@ public function execute() try { $res = $this->oneTouch->verify($this->getUser()); if ($res === 'approved') { - $this->trustedManager->handleTrustDeviceRequest(Authy::CODE, $this->getRequest()); $this->tfaSession->grantAccess(); $res = ['success' => true, 'status' => 'approved']; } else { diff --git a/TwoFactorAuth/Controller/Adminhtml/Duo/Auth.php b/TwoFactorAuth/Controller/Adminhtml/Duo/Auth.php index d04a2726..6c9bb734 100644 --- a/TwoFactorAuth/Controller/Adminhtml/Duo/Auth.php +++ b/TwoFactorAuth/Controller/Adminhtml/Duo/Auth.php @@ -15,6 +15,7 @@ use Magento\TwoFactorAuth\Api\UserConfigManagerInterface; use Magento\TwoFactorAuth\Controller\Adminhtml\AbstractAction; use Magento\TwoFactorAuth\Model\Provider\Engine\DuoSecurity; +use Magento\TwoFactorAuth\Model\UserConfig\HtmlAreaTokenVerifier; /** * Duo security authentication page @@ -42,18 +43,25 @@ class Auth extends AbstractAction implements HttpGetActionInterface */ private $userConfigManager; + /** + * @var HtmlAreaTokenVerifier + */ + private $tokenVerifier; + public function __construct( Action\Context $context, Session $session, PageFactory $pageFactory, UserConfigManagerInterface $userConfigManager, - TfaInterface $tfa + TfaInterface $tfa, + HtmlAreaTokenVerifier $tokenVerifier ) { parent::__construct($context); $this->tfa = $tfa; $this->session = $session; $this->pageFactory = $pageFactory; $this->userConfigManager = $userConfigManager; + $this->tokenVerifier = $tokenVerifier; } /** @@ -70,7 +78,7 @@ private function getUser() */ public function execute() { - $this->userConfigManager->setDefaultProvider((int) $this->getUser()->getId(), DuoSecurity::CODE); + $this->userConfigManager->setDefaultProvider((int)$this->getUser()->getId(), DuoSecurity::CODE); return $this->pageFactory->create(); } @@ -81,11 +89,19 @@ public function execute() */ protected function _isAllowed() { - // Do not check for activation + if (!parent::_isAllowed()) { + return false; + } + + // 1st time users must have the token. $user = $this->getUser(); return $user && - $this->tfa->getProviderIsAllowed((int) $user->getId(), DuoSecurity::CODE); + $this->tfa->getProviderIsAllowed((int)$user->getId(), DuoSecurity::CODE) + && ( + $this->userConfigManager->isProviderConfigurationActive((int)$user->getId(), DuoSecurity::CODE) + || $this->tokenVerifier->isConfigTokenProvided() + ); } } diff --git a/TwoFactorAuth/Controller/Adminhtml/Duo/Authpost.php b/TwoFactorAuth/Controller/Adminhtml/Duo/Authpost.php index 8527466d..f21d5a8a 100644 --- a/TwoFactorAuth/Controller/Adminhtml/Duo/Authpost.php +++ b/TwoFactorAuth/Controller/Adminhtml/Duo/Authpost.php @@ -16,6 +16,8 @@ use Magento\TwoFactorAuth\Api\TfaSessionInterface; use Magento\TwoFactorAuth\Controller\Adminhtml\AbstractAction; use Magento\TwoFactorAuth\Model\Provider\Engine\DuoSecurity; +use Magento\TwoFactorAuth\Api\UserConfigManagerInterface; +use Magento\TwoFactorAuth\Model\UserConfig\HtmlAreaTokenVerifier; /** * Duo security authentication post controller @@ -58,6 +60,16 @@ class Authpost extends AbstractAction implements HttpPostActionInterface */ private $context; + /** + * @var HtmlAreaTokenVerifier + */ + private $tokenVerifier; + + /** + * @var UserConfigManagerInterface + */ + private $userConfig; + /** * Authpost constructor. * @param Action\Context $context @@ -67,6 +79,8 @@ class Authpost extends AbstractAction implements HttpPostActionInterface * @param DataObjectFactory $dataObjectFactory * @param AlertInterface $alert * @param TfaInterface $tfa + * @param HtmlAreaTokenVerifier $tokenVerifier + * @param UserConfigManagerInterface $userConfig */ public function __construct( Action\Context $context, @@ -75,7 +89,9 @@ public function __construct( TfaSessionInterface $tfaSession, DataObjectFactory $dataObjectFactory, AlertInterface $alert, - TfaInterface $tfa + TfaInterface $tfa, + HtmlAreaTokenVerifier $tokenVerifier, + UserConfigManagerInterface $userConfig ) { parent::__construct($context); $this->tfa = $tfa; @@ -85,6 +101,8 @@ public function __construct( $this->dataObjectFactory = $dataObjectFactory; $this->alert = $alert; $this->context = $context; + $this->tokenVerifier = $tokenVerifier; + $this->userConfig = $userConfig; } /** @@ -128,11 +146,19 @@ public function execute() */ protected function _isAllowed() { - // Do not check for activation + if (!parent::_isAllowed()) { + return false; + } + + // 1st time users must have the token. $user = $this->getUser(); return $user && - $this->tfa->getProviderIsAllowed((int) $user->getId(), DuoSecurity::CODE); + $this->tfa->getProviderIsAllowed((int)$user->getId(), DuoSecurity::CODE) + && ( + $this->userConfig->isProviderConfigurationActive((int)$user->getId(), DuoSecurity::CODE) + || $this->tokenVerifier->isConfigTokenProvided() + ); } } diff --git a/TwoFactorAuth/Controller/Adminhtml/Google/Authpost.php b/TwoFactorAuth/Controller/Adminhtml/Google/Authpost.php index 6357576a..691317af 100644 --- a/TwoFactorAuth/Controller/Adminhtml/Google/Authpost.php +++ b/TwoFactorAuth/Controller/Adminhtml/Google/Authpost.php @@ -10,15 +10,12 @@ use Magento\Backend\Model\Auth\Session; use Magento\Backend\App\Action; use Magento\Framework\App\Action\HttpPostActionInterface; -use Magento\Framework\App\ResponseInterface; use Magento\Framework\Controller\Result\JsonFactory; -use Magento\Framework\Controller\ResultInterface; use Magento\Framework\DataObjectFactory; use Magento\Framework\Exception\NoSuchEntityException; use Magento\TwoFactorAuth\Model\AlertInterface; use Magento\TwoFactorAuth\Api\TfaInterface; use Magento\TwoFactorAuth\Api\TfaSessionInterface; -use Magento\TwoFactorAuth\Api\TrustedManagerInterface; use Magento\TwoFactorAuth\Controller\Adminhtml\AbstractAction; use Magento\TwoFactorAuth\Model\Provider\Engine\Google; use Magento\User\Model\User; @@ -53,11 +50,6 @@ class Authpost extends AbstractAction implements HttpPostActionInterface */ private $tfaSession; - /** - * @var TrustedManagerInterface - */ - private $trustedManager; - /** * @var DataObjectFactory */ @@ -74,7 +66,7 @@ class Authpost extends AbstractAction implements HttpPostActionInterface * @param JsonFactory $jsonFactory * @param Google $google * @param TfaSessionInterface $tfaSession - * @param TrustedManagerInterface $trustedManager + * @param \Magento\TwoFactorAuth\Api\TrustedManagerInterface $trustedManager * @param TfaInterface $tfa * @param AlertInterface $alert * @param DataObjectFactory $dataObjectFactory @@ -85,7 +77,7 @@ public function __construct( JsonFactory $jsonFactory, Google $google, TfaSessionInterface $tfaSession, - TrustedManagerInterface $trustedManager, + \Magento\TwoFactorAuth\Api\TrustedManagerInterface $trustedManager, TfaInterface $tfa, AlertInterface $alert, DataObjectFactory $dataObjectFactory @@ -101,30 +93,18 @@ public function __construct( $this->alert = $alert; } - /** - * Get current user - * @return User|null - */ - private function getUser(): ?User - { - return $this->session->getUser(); - } - /** * @inheritdoc - * @return ResponseInterface|ResultInterface * @throws NoSuchEntityException */ public function execute() { + $user = $this->session->getUser(); $response = $this->jsonFactory->create(); + /** @var \Magento\Framework\DataObject $request */ + $request = $this->dataObjectFactory->create(['data' => $this->getRequest()->getParams()]); - $user = $this->getUser(); - - if ($this->google->verify($user, $this->dataObjectFactory->create([ - 'data' => $this->getRequest()->getParams(), - ]))) { - $this->trustedManager->handleTrustDeviceRequest(Google::CODE, $this->getRequest()); + if ($this->google->verify($user, $request)) { $this->tfaSession->grantAccess(); $response->setData(['success' => true]); } else { @@ -148,11 +128,10 @@ public function execute() */ protected function _isAllowed() { - $user = $this->getUser(); + $user = $this->session->getUser(); - return - $user && - $this->tfa->getProviderIsAllowed((int) $user->getId(), Google::CODE) && - $this->tfa->getProvider(Google::CODE)->isActive((int) $user->getId()); + return $user + && $this->tfa->getProviderIsAllowed((int)$user->getId(), Google::CODE) + && $this->tfa->getProvider(Google::CODE)->isActive((int)$user->getId()); } } diff --git a/TwoFactorAuth/Controller/Adminhtml/Google/Configure.php b/TwoFactorAuth/Controller/Adminhtml/Google/Configure.php index 4afde0f8..121469b5 100644 --- a/TwoFactorAuth/Controller/Adminhtml/Google/Configure.php +++ b/TwoFactorAuth/Controller/Adminhtml/Google/Configure.php @@ -12,15 +12,16 @@ use Magento\Framework\App\Action\HttpGetActionInterface; use Magento\Framework\View\Result\PageFactory; use Magento\TwoFactorAuth\Api\TfaInterface; -use Magento\TwoFactorAuth\Controller\Adminhtml\AbstractAction; +use Magento\TwoFactorAuth\Controller\Adminhtml\AbstractConfigureAction; use Magento\TwoFactorAuth\Model\Provider\Engine\Google; use Magento\User\Model\User; +use Magento\TwoFactorAuth\Model\UserConfig\HtmlAreaTokenVerifier; /** * Google authenticator configuration page * @SuppressWarnings(PHPMD.CamelCaseMethodName) */ -class Configure extends AbstractAction implements HttpGetActionInterface +class Configure extends AbstractConfigureAction implements HttpGetActionInterface { /** * @var TfaInterface @@ -42,14 +43,16 @@ class Configure extends AbstractAction implements HttpGetActionInterface * @param Session $session * @param PageFactory $pageFactory * @param TfaInterface $tfa + * @param HtmlAreaTokenVerifier $tokenVerifier */ public function __construct( Action\Context $context, Session $session, PageFactory $pageFactory, - TfaInterface $tfa + TfaInterface $tfa, + HtmlAreaTokenVerifier $tokenVerifier ) { - parent::__construct($context); + parent::__construct($context, $tokenVerifier); $this->tfa = $tfa; $this->session = $session; $this->pageFactory = $pageFactory; @@ -77,6 +80,10 @@ public function execute() */ protected function _isAllowed() { + if (!parent::_isAllowed()) { + return false; + } + $user = $this->getUser(); return diff --git a/TwoFactorAuth/Controller/Adminhtml/Google/Configurepost.php b/TwoFactorAuth/Controller/Adminhtml/Google/Configurepost.php index fb39c081..7467660b 100644 --- a/TwoFactorAuth/Controller/Adminhtml/Google/Configurepost.php +++ b/TwoFactorAuth/Controller/Adminhtml/Google/Configurepost.php @@ -18,15 +18,16 @@ use Magento\TwoFactorAuth\Model\AlertInterface; use Magento\TwoFactorAuth\Api\TfaInterface; use Magento\TwoFactorAuth\Api\TfaSessionInterface; -use Magento\TwoFactorAuth\Controller\Adminhtml\AbstractAction; +use Magento\TwoFactorAuth\Controller\Adminhtml\AbstractConfigureAction; use Magento\TwoFactorAuth\Model\Provider\Engine\Google; use Magento\User\Model\User; +use Magento\TwoFactorAuth\Model\UserConfig\HtmlAreaTokenVerifier; /** * Google authenticator configuration post controller * @SuppressWarnings(PHPMD.CamelCaseMethodName) */ -class Configurepost extends AbstractAction implements HttpPostActionInterface +class Configurepost extends AbstractConfigureAction implements HttpPostActionInterface { /** * @var TfaInterface @@ -72,6 +73,7 @@ class Configurepost extends AbstractAction implements HttpPostActionInterface * @param TfaInterface $tfa * @param AlertInterface $alert * @param DataObjectFactory $dataObjectFactory + * @param HtmlAreaTokenVerifier $tokenVerifier */ public function __construct( Action\Context $context, @@ -81,9 +83,10 @@ public function __construct( TfaSessionInterface $tfaSession, TfaInterface $tfa, AlertInterface $alert, - DataObjectFactory $dataObjectFactory + DataObjectFactory $dataObjectFactory, + HtmlAreaTokenVerifier $tokenVerifier ) { - parent::__construct($context); + parent::__construct($context, $tokenVerifier); $this->tfa = $tfa; $this->session = $session; $this->jsonFactory = $jsonFactory; @@ -144,6 +147,10 @@ public function execute() */ protected function _isAllowed() { + if (!parent::_isAllowed()) { + return false; + } + $user = $this->getUser(); return diff --git a/TwoFactorAuth/Controller/Adminhtml/Tfa/Configure.php b/TwoFactorAuth/Controller/Adminhtml/Tfa/Configure.php new file mode 100644 index 00000000..6643e85c --- /dev/null +++ b/TwoFactorAuth/Controller/Adminhtml/Tfa/Configure.php @@ -0,0 +1,71 @@ +tfa = $tfa; + $this->tokenVerifier = $tokenVerifier; + $this->session = $session; + } + + /** + * @inheritDoc + */ + public function execute() + { + $user = $this->session->getUser(); + if (!$this->tfa->getUserProviders((int)$user->getId()) && !$this->tokenVerifier->isConfigTokenProvided()) { + return $this->_redirect('tfa/tfa/requestconfig'); + } + + return $this->resultFactory->create(ResultFactory::TYPE_PAGE); + } +} diff --git a/TwoFactorAuth/Controller/Adminhtml/Tfa/Configurepost.php b/TwoFactorAuth/Controller/Adminhtml/Tfa/Configurepost.php new file mode 100644 index 00000000..df8c1a87 --- /dev/null +++ b/TwoFactorAuth/Controller/Adminhtml/Tfa/Configurepost.php @@ -0,0 +1,137 @@ +startUpUrl = $context->getBackendUrl()->getStartupPageUrl(); + $this->config = $config; + $this->configResource = $configResource; + $this->tfa = $tfa; + $this->tokenVerifier = $tokenVerifier; + $this->session = $session; + } + + /** + * Validate user input + * + * @param mixed $selected + * @return bool + */ + private function validate($selected): bool + { + $providerCodes = array_map( + function (ProviderInterface $provider): string { + return $provider->getCode(); + }, + $this->tfa->getAllEnabledProviders() + ); + + return is_array($selected) && !array_diff(array_keys($selected), $providerCodes); + } + + /** + * @inheritDoc + */ + protected function _isAllowed() + { + $user = $this->session->getUser(); + if ($user && !$this->tokenVerifier->isConfigTokenProvided()) { + return false; + } + + return parent::_isAllowed(); + } + + /** + * @inheritDoc + */ + public function execute() + { + $selected = $this->getRequest()->getParam('tfa_selected'); + if ($this->validate($selected)) { + $this->configResource->saveConfig( + TfaInterface::XML_PATH_FORCED_PROVIDERS, + implode(', ',array_keys($selected)) + ); + $this->config->reinit(); + $this->getMessageManager()->addSuccessMessage( + __('Two-Factory Authorization providers have been successfully configured') + ); + + return $this->_redirect($this->startUpUrl); + } else { + $this->getMessageManager()->addErrorMessage(__('Please select valid providers.')); + + return $this->_redirect('tfa/tfa/configure'); + } + } +} diff --git a/TwoFactorAuth/Controller/Adminhtml/Tfa/Index.php b/TwoFactorAuth/Controller/Adminhtml/Tfa/Index.php index c80b9554..17568b91 100644 --- a/TwoFactorAuth/Controller/Adminhtml/Tfa/Index.php +++ b/TwoFactorAuth/Controller/Adminhtml/Tfa/Index.php @@ -8,13 +8,14 @@ namespace Magento\TwoFactorAuth\Controller\Adminhtml\Tfa; use Magento\Backend\Model\Auth\Session; -use Magento\Backend\App\Action; +use Magento\Backend\App\Action\Context; use Magento\Framework\App\Action\HttpGetActionInterface; use Magento\Framework\Exception\LocalizedException; use Magento\TwoFactorAuth\Api\TfaInterface; use Magento\TwoFactorAuth\Api\UserConfigManagerInterface; use Magento\TwoFactorAuth\Controller\Adminhtml\AbstractAction; use Magento\User\Model\User; +use Magento\TwoFactorAuth\Api\UserConfigRequestManagerInterface; /** * 2FA entry point controller @@ -42,27 +43,35 @@ class Index extends AbstractAction implements HttpGetActionInterface private $userConfigManager; /** - * @var Action\Context + * @var Context */ private $context; + /** + * @var UserConfigRequestManagerInterface + */ + private $userConfigRequest; + /** * @param Action\Context $context * @param Session $session * @param UserConfigManagerInterface $userConfigManager * @param TfaInterface $tfa + * @param UserConfigRequestManagerInterface $userConfigRequestManager */ public function __construct( - Action\Context $context, + Context $context, Session $session, UserConfigManagerInterface $userConfigManager, - TfaInterface $tfa + TfaInterface $tfa, + UserConfigRequestManagerInterface $userConfigRequestManager ) { parent::__construct($context); $this->tfa = $tfa; $this->session = $session; $this->userConfigManager = $userConfigManager; $this->context = $context; + $this->userConfigRequest = $userConfigRequestManager; } /** @@ -82,8 +91,13 @@ public function execute() { $user = $this->getUser(); + if (!$this->tfa->getUserProviders((int)$user->getId())) { + //If 2FA is not configured - request configuration. + return $this->_redirect('tfa/tfa/requestconfig'); + } $providersToConfigure = $this->tfa->getProvidersToActivate((int) $user->getId()); if (!empty($providersToConfigure)) { + //2FA provider not activated - redirect to the provider form. return $this->_redirect($providersToConfigure[0]->getConfigureAction()); } @@ -91,10 +105,12 @@ public function execute() $defaultProviderCode = $this->userConfigManager->getDefaultProvider((int) $user->getId()); if ($this->tfa->getProviderIsAllowed((int) $user->getId(), $defaultProviderCode)) { + //If default provider was configured - select it. $providerCode = $defaultProviderCode; } if (!$providerCode) { + //Select one random provider. $providers = $this->tfa->getUserProviders((int) $user->getId()); if (!empty($providers)) { $providerCode = $providers[0]->getCode(); @@ -102,11 +118,13 @@ public function execute() } if (!$providerCode) { + //Couldn't find provider - perhaps something is not configured properly. return $this->_redirect($this->context->getBackendUrl()->getStartupPageUrl()); } $provider = $this->tfa->getProvider($providerCode); if ($provider) { + //Provider found, user will be challenged. return $this->_redirect($provider->getAuthAction()); } diff --git a/TwoFactorAuth/Controller/Adminhtml/Tfa/Requestconfig.php b/TwoFactorAuth/Controller/Adminhtml/Tfa/Requestconfig.php new file mode 100644 index 00000000..ad21f41b --- /dev/null +++ b/TwoFactorAuth/Controller/Adminhtml/Tfa/Requestconfig.php @@ -0,0 +1,103 @@ +configRequestManager = $configRequestManager; + $this->tokenVerifier = $tokenVerifier; + $this->tfa = $tfa; + $this->session = $session; + } + + /** + * @inheritDoc + */ + public function execute() + { + $user = $this->session->getUser(); + if (!$this->configRequestManager->isConfigurationRequiredFor((int)$user->getId())) { + throw new AuthorizationException(__('2FA is already configured for the user.')); + } + if ($this->tokenVerifier->isConfigTokenProvided()) { + if (!$this->tfa->getForcedProviders()) { + return $this->_redirect('tfa/tfa/configure'); + } else { + return $this->_redirect('tfa/tfa/index'); + } + } + + try { + $this->configRequestManager->sendConfigRequestTo($user); + } catch (AuthorizationException $exception) { + $this->messageManager->addErrorMessage( + 'Please ask an administrator with sufficient access to configure 2FA first' + ); + } catch (NotificationExceptionInterface $exception) { + $this->messageManager->addErrorMessage('Failed to send the message. Please contact the administrator'); + } + + return $this->resultFactory->create(ResultFactory::TYPE_PAGE); + } +} diff --git a/TwoFactorAuth/Controller/Adminhtml/Tfa/Revoke.php b/TwoFactorAuth/Controller/Adminhtml/Tfa/Revoke.php index 74dd5d51..8238d575 100644 --- a/TwoFactorAuth/Controller/Adminhtml/Tfa/Revoke.php +++ b/TwoFactorAuth/Controller/Adminhtml/Tfa/Revoke.php @@ -7,30 +7,29 @@ namespace Magento\TwoFactorAuth\Controller\Adminhtml\Tfa; -use Magento\Backend\App\Action; +use Magento\Backend\App\Action\Context; use Magento\Framework\App\Action\HttpGetActionInterface; use Magento\Framework\App\Action\HttpPostActionInterface; use Magento\Framework\Exception\NoSuchEntityException; -use Magento\TwoFactorAuth\Api\TrustedManagerInterface; use Magento\TwoFactorAuth\Controller\Adminhtml\AbstractAction; /** * Revoke 2FA trusted host authorization controller + * + * @deprecated Trusted Devices functionality was removed. * @SuppressWarnings(PHPMD.CamelCaseMethodName) */ class Revoke extends AbstractAction implements HttpGetActionInterface, HttpPostActionInterface { /** - * @var TrustedManagerInterface + * @param Context $context + * @param \Magento\TwoFactorAuth\Api\TrustedManagerInterface $trustedManager */ - private $trustedManager; - public function __construct( - Action\Context $context, - TrustedManagerInterface $trustedManager + Context $context, + \Magento\TwoFactorAuth\Api\TrustedManagerInterface $trustedManager ) { parent::__construct($context); - $this->trustedManager = $trustedManager; } /** @@ -39,12 +38,7 @@ public function __construct( */ public function execute() { - $tokenId = (int) $this->getRequest()->getParam('id'); - $userId = (int) $this->getRequest()->getParam('user_id'); - $this->trustedManager->revokeTrustedDevice($tokenId); - - $this->messageManager->addSuccessMessage(__('Device authorization revoked')); - return $this->_redirect('adminhtml/user/edit', ['user_id' => $userId]); + trigger_error('Trusted devices are no longer supported', E_USER_DEPRECATED); } /** diff --git a/TwoFactorAuth/Controller/Adminhtml/U2f/Auth.php b/TwoFactorAuth/Controller/Adminhtml/U2f/Auth.php index 46148d0e..a0b3ee1c 100644 --- a/TwoFactorAuth/Controller/Adminhtml/U2f/Auth.php +++ b/TwoFactorAuth/Controller/Adminhtml/U2f/Auth.php @@ -64,21 +64,12 @@ public function __construct( $this->userConfigManager = $userConfigManager; } - /** - * Get current user - * @return User|null - */ - private function getUser(): ?User - { - return $this->session->getUser(); - } - /** * @inheritDoc */ public function execute() { - $this->userConfigManager->setDefaultProvider((int) $this->getUser()->getId(), U2fKey::CODE); + $this->userConfigManager->setDefaultProvider((int) $this->session->getUser()->getId(), U2fKey::CODE); return $this->pageFactory->create(); } @@ -87,11 +78,11 @@ public function execute() */ protected function _isAllowed() { - $user = $this->getUser(); + $user = $this->session->getUser(); return $user && - $this->tfa->getProviderIsAllowed((int) $this->getUser()->getId(), U2fKey::CODE) && - $this->tfa->getProvider(U2fKey::CODE)->isActive((int) $this->getUser()->getId()); + $this->tfa->getProviderIsAllowed((int) $user->getId(), U2fKey::CODE) && + $this->tfa->getProvider(U2fKey::CODE)->isActive((int) $user->getId()); } } diff --git a/TwoFactorAuth/Controller/Adminhtml/U2f/Authpost.php b/TwoFactorAuth/Controller/Adminhtml/U2f/Authpost.php index 9af9d317..944888c5 100644 --- a/TwoFactorAuth/Controller/Adminhtml/U2f/Authpost.php +++ b/TwoFactorAuth/Controller/Adminhtml/U2f/Authpost.php @@ -16,7 +16,6 @@ use Magento\Framework\DataObjectFactory; use Magento\TwoFactorAuth\Model\AlertInterface; use Magento\TwoFactorAuth\Api\TfaSessionInterface; -use Magento\TwoFactorAuth\Api\TrustedManagerInterface; use Magento\TwoFactorAuth\Controller\Adminhtml\AbstractAction; use Magento\TwoFactorAuth\Model\Provider\Engine\U2fKey; use Magento\TwoFactorAuth\Model\Tfa; @@ -54,11 +53,6 @@ class Authpost extends AbstractAction implements HttpPostActionInterface */ private $tfaSession; - /** - * @var TrustedManagerInterface - */ - private $trustedManager; - /** * @var DataObjectFactory */ @@ -74,7 +68,7 @@ public function __construct( Session $session, JsonFactory $jsonFactory, TfaSessionInterface $tfaSession, - TrustedManagerInterface $trustedManager, + \Magento\TwoFactorAuth\Api\TrustedManagerInterface $trustedManager, U2fKey $u2fKey, DataObjectFactory $dataObjectFactory, AlertInterface $alert, @@ -87,7 +81,6 @@ public function __construct( $this->u2fKey = $u2fKey; $this->jsonFactory = $jsonFactory; $this->tfaSession = $tfaSession; - $this->trustedManager = $trustedManager; $this->dataObjectFactory = $dataObjectFactory; $this->alert = $alert; } @@ -104,7 +97,6 @@ public function execute() 'data' => $this->getRequest()->getParams(), ])); $this->tfaSession->grantAccess(); - $this->trustedManager->handleTrustDeviceRequest(U2fKey::CODE, $this->getRequest()); $res = ['success' => true]; } catch (Exception $e) { diff --git a/TwoFactorAuth/Controller/Adminhtml/U2f/Configure.php b/TwoFactorAuth/Controller/Adminhtml/U2f/Configure.php index 523ae8d4..072492f2 100644 --- a/TwoFactorAuth/Controller/Adminhtml/U2f/Configure.php +++ b/TwoFactorAuth/Controller/Adminhtml/U2f/Configure.php @@ -7,20 +7,21 @@ namespace Magento\TwoFactorAuth\Controller\Adminhtml\U2f; -use Magento\Backend\App\Action; +use Magento\Backend\App\Action\Context; use Magento\Backend\Model\Auth\Session; use Magento\Framework\App\Action\HttpGetActionInterface; use Magento\Framework\View\Result\PageFactory; -use Magento\TwoFactorAuth\Controller\Adminhtml\AbstractAction; +use Magento\TwoFactorAuth\Controller\Adminhtml\AbstractConfigureAction; use Magento\TwoFactorAuth\Model\Provider\Engine\U2fKey; use Magento\TwoFactorAuth\Model\Tfa; use Magento\User\Model\User; +use Magento\TwoFactorAuth\Model\UserConfig\HtmlAreaTokenVerifier; /** * CUbiKey configuration page controller * @SuppressWarnings(PHPMD.CamelCaseMethodName) */ -class Configure extends AbstractAction implements HttpGetActionInterface +class Configure extends AbstractConfigureAction implements HttpGetActionInterface { /** * @var Tfa @@ -41,18 +42,20 @@ class Configure extends AbstractAction implements HttpGetActionInterface * @param Tfa $tfa * @param Session $session * @param PageFactory $pageFactory - * @param Action\Context $context + * @param Context $context + * @param HtmlAreaTokenVerifier $tokenVerifier */ public function __construct( Tfa $tfa, Session $session, PageFactory $pageFactory, - Action\Context $context + Context $context, + HtmlAreaTokenVerifier $tokenVerifier ) { $this->tfa = $tfa; $this->session = $session; - parent::__construct($context); + parent::__construct($context, $tokenVerifier); $this->pageFactory = $pageFactory; } @@ -77,6 +80,10 @@ private function getUser(): ?User */ protected function _isAllowed() { + if (!parent::_isAllowed()) { + return false; + } + $user = $this->getUser(); return diff --git a/TwoFactorAuth/Controller/Adminhtml/U2f/Configurepost.php b/TwoFactorAuth/Controller/Adminhtml/U2f/Configurepost.php index 5fc69342..cef3564e 100644 --- a/TwoFactorAuth/Controller/Adminhtml/U2f/Configurepost.php +++ b/TwoFactorAuth/Controller/Adminhtml/U2f/Configurepost.php @@ -8,22 +8,23 @@ namespace Magento\TwoFactorAuth\Controller\Adminhtml\U2f; use Exception; -use Magento\Backend\App\Action; +use Magento\Backend\App\Action\Context; use Magento\Backend\Model\Auth\Session; use Magento\Framework\App\Action\HttpPostActionInterface; use Magento\Framework\Controller\Result\JsonFactory; use Magento\TwoFactorAuth\Model\AlertInterface; use Magento\TwoFactorAuth\Api\TfaSessionInterface; -use Magento\TwoFactorAuth\Controller\Adminhtml\AbstractAction; +use Magento\TwoFactorAuth\Controller\Adminhtml\AbstractConfigureAction; use Magento\TwoFactorAuth\Model\Provider\Engine\U2fKey; use Magento\TwoFactorAuth\Model\Tfa; use Magento\User\Model\User; +use Magento\TwoFactorAuth\Model\UserConfig\HtmlAreaTokenVerifier; /** * UbiKey configuration post controller * @SuppressWarnings(PHPMD.CamelCaseMethodName) */ -class Configurepost extends AbstractAction implements HttpPostActionInterface +class Configurepost extends AbstractConfigureAction implements HttpPostActionInterface { /** * @var Tfa @@ -62,7 +63,8 @@ class Configurepost extends AbstractAction implements HttpPostActionInterface * @param TfaSessionInterface $tfaSession * @param U2fKey $u2fKey * @param AlertInterface $alert - * @param Action\Context $context + * @param Context $context + * @param HtmlAreaTokenVerifier $tokenVerifier */ public function __construct( Tfa $tfa, @@ -71,9 +73,10 @@ public function __construct( TfaSessionInterface $tfaSession, U2fKey $u2fKey, AlertInterface $alert, - Action\Context $context + Context $context, + HtmlAreaTokenVerifier $tokenVerifier ) { - parent::__construct($context); + parent::__construct($context, $tokenVerifier); $this->tfa = $tfa; $this->session = $session; @@ -105,7 +108,7 @@ public function execute() ); $res = ['success' => true]; - } catch (Exception $e) { + } catch (\Throwable $e) { $this->alert->event( 'Magento_TwoFactorAuth', 'U2F error while adding device', @@ -134,6 +137,10 @@ private function getUser(): ?User */ protected function _isAllowed() { + if (!parent::_isAllowed()) { + return false; + } + $user = $this->getUser(); return diff --git a/TwoFactorAuth/Model/Config/Backend/ForceProviders.php b/TwoFactorAuth/Model/Config/Backend/ForceProviders.php new file mode 100644 index 00000000..11794347 --- /dev/null +++ b/TwoFactorAuth/Model/Config/Backend/ForceProviders.php @@ -0,0 +1,31 @@ +getValue(); + if (!$value) { + throw new ValidatorException(__('You have to select at least one Two-Factor Authorization provider')); + } + + return parent::beforeSave(); + } +} diff --git a/TwoFactorAuth/Model/EmailUserNotifier.php b/TwoFactorAuth/Model/EmailUserNotifier.php new file mode 100644 index 00000000..2cee8964 --- /dev/null +++ b/TwoFactorAuth/Model/EmailUserNotifier.php @@ -0,0 +1,115 @@ +scopeConfig = $scopeConfig; + $this->transportBuilder = $transportBuilder; + $this->storeManager = $storeManager; + $this->logger = $logger; + } + + /** + * Send configuration related message to the admin user. + * + * @param User $user + * @param string $token + * @param string $emailTemplateId + * @return void + * @throws NotificationExceptionInterface + */ + private function sendConfigRequired(User $user, string $token, string $emailTemplateId): void + { + try { + $transport = $this->transportBuilder + ->setTemplateIdentifier($emailTemplateId) + ->setTemplateOptions([ + 'area' => 'adminhtml', + 'store' => 0 + ]) + ->setTemplateVars( + [ + 'username' => $user->getFirstName() . ' ' . $user->getLastName(), + 'token' => $token, + 'store_name' => $this->storeManager->getStore()->getFrontendName() + ] + ) + ->setFromByScope( + $this->scopeConfig->getValue('admin/emails/forgot_email_identity') + ) + ->addTo($user->getEmail(), $user->getFirstName() . ' ' . $user->getLastName()) + ->getTransport(); + $transport->sendMessage(); + } catch (\Throwable $exception) { + $this->logger->critical($exception); + throw new NotificationException('Failed to send 2FA E-mail to a user', 0, $exception); + } + } + + /** + * @inheritDoc + */ + public function sendUserConfigRequestMessage(User $user, string $token): void + { + $this->sendConfigRequired($user, $token, 'tfa_admin_user_config_required'); + } + + /** + * @inheritDoc + */ + public function sendAppConfigRequestMessage(User $user, string $token): void + { + $this->sendConfigRequired($user, $token, 'tfa_admin_app_config_required'); + } +} diff --git a/TwoFactorAuth/Model/Exception/NotificationException.php b/TwoFactorAuth/Model/Exception/NotificationException.php new file mode 100644 index 00000000..679cac51 --- /dev/null +++ b/TwoFactorAuth/Model/Exception/NotificationException.php @@ -0,0 +1,19 @@ +engine->isTrustedDevicesAllowed(); + return false; } /** diff --git a/TwoFactorAuth/Model/Provider/Engine/Authy.php b/TwoFactorAuth/Model/Provider/Engine/Authy.php index ac7e8146..cac59db3 100644 --- a/TwoFactorAuth/Model/Provider/Engine/Authy.php +++ b/TwoFactorAuth/Model/Provider/Engine/Authy.php @@ -24,17 +24,21 @@ class Authy implements EngineInterface { /** - * Engine code + * Must be the same as defined in di.xml */ - public const CODE = 'authy'; // Must be the same as defined in di.xml + public const CODE = 'authy'; /** * Configuration XML path for enabled flag + * + * @deprecated Providers are now enabled via "forced_providers" config */ public const XML_PATH_ENABLED = 'twofactorauth/authy/enabled'; /** * Configuration XML path to allow trusted devices + * + * @deprecated Trusted devices functionality is now deprecated */ public const XML_PATH_ALLOW_TRUSTED_DEVICES = 'twofactorauth/authy/allow_trusted_devices'; @@ -128,9 +132,12 @@ public function enroll(UserInterface $user): bool */ public function isEnabled(): bool { - return - (bool) $this->scopeConfig->getValue(static::XML_PATH_ENABLED) && - (bool) $this->service->getApiKey(); + try { + return !!$this->service->getApiKey(); + } catch (\TypeError $exception) { + //API key is empty, returned null instead of a string + return false; + } } /** @@ -146,6 +153,6 @@ public function verify(UserInterface $user, DataObject $request): bool */ public function isTrustedDevicesAllowed(): bool { - return (bool) $this->scopeConfig->getValue(static::XML_PATH_ALLOW_TRUSTED_DEVICES); + return false; } } diff --git a/TwoFactorAuth/Model/Provider/Engine/DuoSecurity.php b/TwoFactorAuth/Model/Provider/Engine/DuoSecurity.php index 5f693655..b99641f2 100644 --- a/TwoFactorAuth/Model/Provider/Engine/DuoSecurity.php +++ b/TwoFactorAuth/Model/Provider/Engine/DuoSecurity.php @@ -223,7 +223,11 @@ public function verify(UserInterface $user, DataObject $request): bool { $time = time(); - [$authSig, $appSig] = explode(':', $request->getData('sig_response')); + $signatures = explode(':', (string)$request->getData('sig_response')); + if (count($signatures) !== 2) { + return false; + } + [$authSig, $appSig] = $signatures; $authUser = $this->parseValues($this->getSecretKey(), $authSig, static::AUTH_PREFIX, $time); $appUser = $this->parseValues($this->getApplicationKey(), $appSig, static::APP_PREFIX, $time); @@ -236,12 +240,15 @@ public function verify(UserInterface $user, DataObject $request): bool */ public function isEnabled(): bool { - return - (bool) $this->scopeConfig->getValue(static::XML_PATH_ENABLED) && - (bool) $this->getApiHostname() && - (bool) $this->getIntegrationKey() && - (bool) $this->getApiHostname() && - (bool) $this->getSecretKey(); + try { + return !!$this->getApiHostname() && + !!$this->getIntegrationKey() && + !!$this->getApiHostname() && + !!$this->getSecretKey(); + } catch (\TypeError $exception) { + //At least one of the methods returned null instead of a string + return false; + } } /** diff --git a/TwoFactorAuth/Model/Provider/Engine/Google.php b/TwoFactorAuth/Model/Provider/Engine/Google.php index 4027e901..c0819177 100644 --- a/TwoFactorAuth/Model/Provider/Engine/Google.php +++ b/TwoFactorAuth/Model/Provider/Engine/Google.php @@ -11,7 +11,6 @@ use Endroid\QrCode\QrCode; use Endroid\QrCode\Writer\PngWriter; use Exception; -use Magento\Framework\App\Config\ScopeConfigInterface; use Magento\Framework\DataObject; use Magento\Framework\Exception\NoSuchEntityException; use Magento\Store\Model\StoreManagerInterface; @@ -28,16 +27,22 @@ class Google implements EngineInterface { /** * Engine code + * + * Must be the same as defined in di.xml */ - public const CODE = 'google'; // Must be the same as defined in di.xml + public const CODE = 'google'; /** * Configuration XML path for enabled flag + * + * @deprecated Providers are now enabled via "forced_providers" config */ public const XML_PATH_ENABLED = 'twofactorauth/google/enabled'; /** * Configuration XML path to allow trusted devices + * + * @deprecated Trusted devices functionality is now deprecated */ public const XML_PATH_ALLOW_TRUSTED_DEVICES = 'twofactorauth/google/allow_trusted_devices'; @@ -56,24 +61,18 @@ class Google implements EngineInterface */ private $storeManager; - /** - * @var ScopeConfigInterface - */ - private $scopeConfig; - /** * @param StoreManagerInterface $storeManager - * @param ScopeConfigInterface $scopeConfig + * @param \Magento\Framework\App\Config\ScopeConfigInterface $scopeConfig * @param UserConfigManagerInterface $configManager */ public function __construct( StoreManagerInterface $storeManager, - ScopeConfigInterface $scopeConfig, + \Magento\Framework\App\Config\ScopeConfigInterface $scopeConfig, UserConfigManagerInterface $configManager ) { $this->configManager = $configManager; $this->storeManager = $storeManager; - $this->scopeConfig = $scopeConfig; } /** @@ -95,22 +94,16 @@ private function generateSecret(): string */ private function getTotp(UserInterface $user): TOTP { - if ($this->totp === null) { - $config = $this->configManager->getProviderConfig((int) $user->getId(), static::CODE); - - if (!isset($config['secret'])) { - $config['secret'] = $this->getSecretCode($user); - } - - // @codingStandardsIgnoreStart - $this->totp = new TOTP( - $user->getEmail(), - $config['secret'] - ); - // @codingStandardsIgnoreEnd + $config = $this->configManager->getProviderConfig((int)$user->getId(), static::CODE); + if (!isset($config['secret'])) { + $config['secret'] = $this->getSecretCode($user); + } + if (!$config['secret']) { + throw new NoSuchEntityException(__('Secret for user with ID#%1 was not found', $user->getId())); } + $totp = new TOTP($user->getEmail(), $config['secret']); - return $this->totp; + return $totp; } /** @@ -122,11 +115,11 @@ private function getTotp(UserInterface $user): TOTP */ public function getSecretCode(UserInterface $user): ?string { - $config = $this->configManager->getProviderConfig((int) $user->getId(), static::CODE); + $config = $this->configManager->getProviderConfig((int)$user->getId(), static::CODE); if (!isset($config['secret'])) { $config['secret'] = $this->generateSecret(); - $this->configManager->setProviderConfig((int) $user->getId(), static::CODE, $config); + $this->configManager->setProviderConfig((int)$user->getId(), static::CODE, $config); } return $config['secret'] ?? null; @@ -158,6 +151,9 @@ private function getProvisioningUrl(UserInterface $user): string public function verify(UserInterface $user, DataObject $request): bool { $token = $request->getData('tfa_code'); + if (!$token) { + return false; + } $totp = $this->getTotp($user); $totp->now(); @@ -195,7 +191,7 @@ public function getQrCodeAsPng(UserInterface $user): string */ public function isEnabled(): bool { - return (bool) $this->scopeConfig->getValue(static::XML_PATH_ENABLED); + return true; } /** @@ -203,6 +199,6 @@ public function isEnabled(): bool */ public function isTrustedDevicesAllowed(): bool { - return (bool) $this->scopeConfig->getValue(static::XML_PATH_ALLOW_TRUSTED_DEVICES); + return false; } } diff --git a/TwoFactorAuth/Model/Provider/Engine/U2fKey.php b/TwoFactorAuth/Model/Provider/Engine/U2fKey.php index 70a64c99..4beb0c61 100644 --- a/TwoFactorAuth/Model/Provider/Engine/U2fKey.php +++ b/TwoFactorAuth/Model/Provider/Engine/U2fKey.php @@ -7,7 +7,6 @@ namespace Magento\TwoFactorAuth\Model\Provider\Engine; -use Magento\Framework\App\Config\ScopeConfigInterface; use Magento\Framework\DataObject; use Magento\Framework\Exception\LocalizedException; use Magento\Framework\Exception\NoSuchEntityException; @@ -28,16 +27,22 @@ class U2fKey implements EngineInterface { /** * Engine code + * + * Must be the same as defined in di.xml */ - public const CODE = 'u2fkey'; // Must be the same as defined in di.xml + public const CODE = 'u2fkey'; /** * Configuration XML path for enabled flag + * + * @deprecated Providers are now enabled via "forced_providers" config */ public const XML_PATH_ENABLED = 'twofactorauth/u2fkey/enabled'; /** * Configuration XML path to allow trusted devices + * + * @deprecated Trusted devices functionality is now deprecated */ public const XML_PATH_ALLOW_TRUSTED_DEVICES = 'twofactorauth/u2fkey/allow_trusted_devices'; @@ -51,24 +56,18 @@ class U2fKey implements EngineInterface */ private $storeManager; - /** - * @var ScopeConfigInterface - */ - private $scopeConfig; - /** * @param StoreManagerInterface $storeManager - * @param ScopeConfigInterface $scopeConfig + * @param \Magento\Framework\App\Config\ScopeConfigInterface $scopeConfig * @param UserConfigManagerInterface $userConfigManager */ public function __construct( StoreManagerInterface $storeManager, - ScopeConfigInterface $scopeConfig, + \Magento\Framework\App\Config\ScopeConfigInterface $scopeConfig, UserConfigManagerInterface $userConfigManager ) { $this->userConfigManager = $userConfigManager; $this->storeManager = $storeManager; - $this->scopeConfig = $scopeConfig; } /** @@ -195,7 +194,7 @@ public function registerDevice(UserInterface $user, array $request, array $respo */ public function isEnabled(): bool { - return (bool) $this->scopeConfig->getValue(static::XML_PATH_ENABLED); + return true; } /** @@ -203,7 +202,7 @@ public function isEnabled(): bool */ public function isTrustedDevicesAllowed(): bool { - return (bool) $this->scopeConfig->getValue(static::XML_PATH_ALLOW_TRUSTED_DEVICES); + return false; } /** diff --git a/TwoFactorAuth/Model/ResourceModel/Trusted/Collection.php b/TwoFactorAuth/Model/ResourceModel/Trusted/Collection.php index d1a7280f..c83634c0 100644 --- a/TwoFactorAuth/Model/ResourceModel/Trusted/Collection.php +++ b/TwoFactorAuth/Model/ResourceModel/Trusted/Collection.php @@ -14,6 +14,7 @@ * Trusted devices collection * @SuppressWarnings(PHPMD.CamelCaseMethodName) * @SuppressWarnings(PHPMD.CamelCasePropertyName) + * @deprecated Trusted Devices functionality was removed. */ class Collection extends AbstractCollection { diff --git a/TwoFactorAuth/Model/ResourceModel/TrustedRepository.php b/TwoFactorAuth/Model/ResourceModel/TrustedRepository.php index 2a7307e5..cd8832c2 100644 --- a/TwoFactorAuth/Model/ResourceModel/TrustedRepository.php +++ b/TwoFactorAuth/Model/ResourceModel/TrustedRepository.php @@ -103,6 +103,8 @@ public function __construct( */ public function save(TrustedInterface $trusted): TrustedInterface { + trigger_error('Trusted devices are no longer supported', E_USER_DEPRECATED); + $trustedData = $this->extensibleDataObjectConverter->toNestedArray( $trusted, [], @@ -126,6 +128,8 @@ public function save(TrustedInterface $trusted): TrustedInterface */ public function getById(int $id): TrustedInterface { + trigger_error('Trusted devices are no longer supported', E_USER_DEPRECATED); + $fromRegistry = $this->registry->retrieveById($id); if ($fromRegistry === null) { $trusted = $this->trustedFactory->create(); @@ -147,6 +151,8 @@ public function getById(int $id): TrustedInterface */ public function getByUserId(int $value): TrustedInterface { + trigger_error('Trusted devices are no longer supported', E_USER_DEPRECATED); + $fromRegistry = $this->registry->retrieveByUserId($value); if ($fromRegistry === null) { $trusted = $this->trustedFactory->create(); @@ -167,6 +173,8 @@ public function getByUserId(int $value): TrustedInterface */ public function delete(TrustedInterface $trusted): void { + trigger_error('Trusted devices are no longer supported', E_USER_DEPRECATED); + $trustedData = $this->extensibleDataObjectConverter->toNestedArray( $trusted, [], @@ -186,8 +194,9 @@ public function delete(TrustedInterface $trusted): void */ public function getList(SearchCriteriaInterface $searchCriteria): SearchResultsInterface { - $collection = $this->collectionFactory->create(); + trigger_error('Trusted devices are no longer supported', E_USER_DEPRECATED); + $collection = $this->collectionFactory->create(); if (null === $searchCriteria) { $searchCriteria = $this->searchCriteriaBuilder->create(); } else { diff --git a/TwoFactorAuth/Model/Tfa.php b/TwoFactorAuth/Model/Tfa.php index b89493df..f1633af0 100644 --- a/TwoFactorAuth/Model/Tfa.php +++ b/TwoFactorAuth/Model/Tfa.php @@ -13,17 +13,19 @@ use Magento\TwoFactorAuth\Api\ProviderInterface; use Magento\TwoFactorAuth\Api\ProviderPoolInterface; use Magento\TwoFactorAuth\Api\TfaInterface; -use Magento\TwoFactorAuth\Api\TrustedRepositoryInterface; use Magento\TwoFactorAuth\Api\UserConfigManagerInterface; /** + * @inheritDoc + * * @SuppressWarnings(PHPMD.LongVariable) */ class Tfa implements TfaInterface { - private $forcedProviders = null; + /** + * @var null|string[] + */ private $allowedUrls = null; - private $enabledProviders = null; /** * @var ScopeConfigInterface @@ -40,11 +42,6 @@ class Tfa implements TfaInterface */ private $searchCriteriaBuilder; - /** - * @var TrustedRepositoryInterface - */ - private $trustedRepository; - /** * @var ProviderPoolInterface */ @@ -52,14 +49,14 @@ class Tfa implements TfaInterface /** * @param ScopeConfigInterface $scopeConfig - * @param TrustedRepositoryInterface $trustedRepository + * @param \Magento\TwoFactorAuth\Api\TrustedRepositoryInterface $trustedRepository * @param SearchCriteriaBuilder $searchCriteriaBuilder * @param UserConfigManagerInterface $userConfigManager * @param ProviderPoolInterface $providerPool */ public function __construct( ScopeConfigInterface $scopeConfig, - TrustedRepositoryInterface $trustedRepository, + \Magento\TwoFactorAuth\Api\TrustedRepositoryInterface $trustedRepository, SearchCriteriaBuilder $searchCriteriaBuilder, UserConfigManagerInterface $userConfigManager, ProviderPoolInterface $providerPool @@ -67,7 +64,6 @@ public function __construct( $this->scopeConfig = $scopeConfig; $this->userConfigManager = $userConfigManager; $this->searchCriteriaBuilder = $searchCriteriaBuilder; - $this->trustedRepository = $trustedRepository; $this->providerPool = $providerPool; } @@ -100,20 +96,15 @@ public function getProviderByCode(string $code): ?ProviderInterface */ public function getAllEnabledProviders(): array { - if ($this->enabledProviders === null) { - $this->enabledProviders = []; - - if ($this->isEnabled()) { - $providers = $this->getAllProviders(); - foreach ($providers as $provider) { - if ($provider->isEnabled()) { - $this->enabledProviders[] = $provider; - } - } + $enabledProviders = []; + $providers = $this->getAllProviders(); + foreach ($providers as $provider) { + if ($provider->isEnabled()) { + $enabledProviders[] = $provider; } } - return $this->enabledProviders; + return $enabledProviders; } /** @@ -139,21 +130,25 @@ public function getProvider(string $providerCode, bool $onlyEnabled = true): ?Pr */ public function getForcedProviders(): array { - if ($this->forcedProviders === null) { - $providersString = (string) $this->scopeConfig->getValue(TfaInterface::XML_PATH_FORCED_PROVIDERS); - $forcedProvidersCodes = preg_split('/\s*,\s*/', $providersString); + $forcedProviders = []; - $this->forcedProviders = []; + $configValue = $this->scopeConfig->getValue(TfaInterface::XML_PATH_FORCED_PROVIDERS); + if (!is_array($configValue) && $configValue) { + $forcedProvidersCodes = preg_split('/\s*,\s*/', $configValue); + } else { + $forcedProvidersCodes = $configValue; + } + if ($forcedProvidersCodes) { foreach ($forcedProvidersCodes as $forcedProviderCode) { $provider = $this->getProvider($forcedProviderCode); if ($provider) { - $this->forcedProviders[] = $provider; + $forcedProviders[] = $provider; } } } - return $this->forcedProviders; + return $forcedProviders; } /** @@ -161,23 +156,7 @@ public function getForcedProviders(): array */ public function getUserProviders(int $userId): array { - $forcedProviders = $this->getForcedProviders(); - - if (!empty($forcedProviders)) { - return $forcedProviders; - } - - $providersCodes = $this->userConfigManager->getProvidersCodes($userId); - - $res = []; - foreach ($providersCodes as $providerCode) { - $provider = $this->getProvider($providerCode); - if ($provider) { - $res[] = $provider; - } - } - - return $res; + return $this->getForcedProviders(); } /** @@ -185,10 +164,7 @@ public function getUserProviders(int $userId): array */ public function getTrustedDevices(int $userId): array { - $criteria = $this->searchCriteriaBuilder->addFilter('user_id', $userId)->create(); - $results = $this->trustedRepository->getList($criteria); - - return $results->getItems(); + return []; } /** @@ -201,6 +177,9 @@ public function getAllowedUrls(): array 'adminhtml_auth_login', 'adminhtml_auth_logout', 'adminhtml_auth_forgotpassword', + 'tfa_tfa_requestconfig', + 'tfa_tfa_configure', + 'tfa_tfa_configurepost', 'tfa_tfa_index' ]; @@ -255,7 +234,7 @@ public function getProviderIsAllowed(int $userId, string $providerCode): bool */ public function isEnabled(): bool { - return (bool) $this->scopeConfig->getValue(TfaInterface::XML_PATH_ENABLED); + return true; } /** diff --git a/TwoFactorAuth/Model/TrustedManager.php b/TwoFactorAuth/Model/TrustedManager.php index 48f2de13..5529b083 100644 --- a/TwoFactorAuth/Model/TrustedManager.php +++ b/TwoFactorAuth/Model/TrustedManager.php @@ -183,6 +183,8 @@ private function sendTokenCookie(string $token): void */ public function rotateTrustedDeviceToken(): void { + trigger_error('Trusted devices are no longer supported', E_USER_DEPRECATED); + $user = $this->getUser(); $tokenCollection = $this->getTokenCollection(); @@ -208,6 +210,8 @@ public function rotateTrustedDeviceToken(): void */ public function isTrustedDevice(): bool { + trigger_error('Trusted devices are no longer supported', E_USER_DEPRECATED); + if ($this->isTrustedDevice === null) { // Must cache this in a single session to avoid rotation issues $user = $this->getUser(); $tokenCollection = $this->getTokenCollection(); @@ -234,6 +238,8 @@ public function isTrustedDevice(): bool */ public function revokeTrustedDevice(int $tokenId): void { + trigger_error('Trusted devices are no longer supported', E_USER_DEPRECATED); + $token = $this->trustedRepository->getById($tokenId); $this->trustedRepository->delete($token); } @@ -243,6 +249,8 @@ public function revokeTrustedDevice(int $tokenId): void */ public function handleTrustDeviceRequest(string $providerCode, RequestInterface $request): bool { + trigger_error('Trusted devices are no longer supported', E_USER_DEPRECATED); + $provider = $this->tfa->getProvider($providerCode); if ($provider) { if ($provider->isTrustedDevicesAllowed() && diff --git a/TwoFactorAuth/Model/TrustedRegistry.php b/TwoFactorAuth/Model/TrustedRegistry.php index 4af05b49..e079ecb8 100644 --- a/TwoFactorAuth/Model/TrustedRegistry.php +++ b/TwoFactorAuth/Model/TrustedRegistry.php @@ -12,6 +12,7 @@ /** * Trusted hosts registry * @SuppressWarnings(PHPMD.ShortVariable) + * @deprecated Trusted Devices functionality was removed. */ class TrustedRegistry { diff --git a/TwoFactorAuth/Model/UserConfig/HtmlAreaTokenVerifier.php b/TwoFactorAuth/Model/UserConfig/HtmlAreaTokenVerifier.php new file mode 100644 index 00000000..16ab69d6 --- /dev/null +++ b/TwoFactorAuth/Model/UserConfig/HtmlAreaTokenVerifier.php @@ -0,0 +1,116 @@ +request = $request; + $this->tokenManager = $tokenManager; + $this->cookies = $cookies; + $this->cookieMetadataFactory = $cookieMetadataFactory; + $this->session = $session; + $this->sessionManager = $sessionManager; + } + + /** + * Was config token provided by current user?. + * + * @return bool + */ + public function isConfigTokenProvided(): bool + { + return !!$this->readConfigToken(); + } + + /** + * Read configuration token provided by user. + * + * @return string|null + */ + public function readConfigToken(): ?string + { + + $user = $this->session->getUser(); + if (!$user) { + return null; + } + $token = $this->cookies->getCookie('tfa-ct'); + if (!$token) { + $token = $this->request->getParam('tfat'); + } + if (!$token || !$this->tokenManager->isValidFor((int)$user->getId(), $token)) { + return null; + } + if (!$this->cookies->getCookie('tfa-ct')) { + $metadata = $this->cookieMetadataFactory->createSensitiveCookieMetadata() + ->setPath($this->sessionManager->getCookiePath()); + $this->cookies->setSensitiveCookie('tfa-ct', $token, $metadata); + } + + return $token; + } +} diff --git a/TwoFactorAuth/Model/UserConfig/SignedTokenManager.php b/TwoFactorAuth/Model/UserConfig/SignedTokenManager.php new file mode 100644 index 00000000..d0d3e93c --- /dev/null +++ b/TwoFactorAuth/Model/UserConfig/SignedTokenManager.php @@ -0,0 +1,86 @@ +encryptor = $encryptor; + $this->json = $json; + $this->dateTime = $dateTime; + } + + /** + * @inheritDoc + */ + public function issueFor(int $userId): string + { + $data = ['user_id' => $userId, 'tfa_configuration' => true, 'iss' => $this->dateTime->timestamp()]; + $encodedData = $this->json->serialize($data); + $signature = base64_encode($this->encryptor->hash($encodedData)); + + return base64_encode($encodedData .'.' .$signature); + } + + /** + * @inheritDoc + */ + public function isValidFor(int $userId, string $token): bool + { + $isValid = false; + [$encodedData, $signatureProvided] = explode('.', base64_decode($token)); + try { + $data = $this->json->unserialize($encodedData); + if (array_key_exists('user_id', $data) + && array_key_exists('tfa_configuration', $data) + && array_key_exists('iss', $data) + && $data['user_id'] === $userId + && $data['tfa_configuration'] + && ($this->dateTime->timestamp() - (int)$data['iss']) < 3600 + && Security::compareStrings(base64_encode($this->encryptor->hash($encodedData)), $signatureProvided) + ) { + $isValid = true; + } + } catch (\Throwable $exception) { + $isValid = false; + } + + return $isValid; + } +} diff --git a/TwoFactorAuth/Model/UserConfig/UserConfigRequestManager.php b/TwoFactorAuth/Model/UserConfig/UserConfigRequestManager.php new file mode 100644 index 00000000..07f0460b --- /dev/null +++ b/TwoFactorAuth/Model/UserConfig/UserConfigRequestManager.php @@ -0,0 +1,88 @@ +tfa = $tfa; + $this->notifier = $notifier; + $this->tokenManager = $tokenManager; + $this->auth = $auth; + } + + /** + * @inheritDoc + */ + public function isConfigurationRequiredFor(int $userId): bool + { + return empty($this->tfa->getUserProviders($userId)) + || !empty($this->tfa->getProvidersToActivate($userId)); + } + + /** + * @inheritDoc + */ + public function sendConfigRequestTo(User $user): void + { + $userId = (int)$user->getId(); + if (empty($this->tfa->getUserProviders($userId))) { + //Application level configuration is required. + if (!$this->auth->isAllowed($user->getAclRole(), 'Magento_TwoFactorAuth::config')) { + throw new AuthorizationException(__('User is not authorized to edit 2FA configuration')); + } + $this->notifier->sendAppConfigRequestMessage($user, $this->tokenManager->issueFor($userId)); + } else { + //Personal provider config required. + $this->notifier->sendUserConfigRequestMessage($user, $this->tokenManager->issueFor($userId)); + } + } +} diff --git a/TwoFactorAuth/Observer/BackendAuthUserLoginSuccess.php b/TwoFactorAuth/Observer/BackendAuthUserLoginSuccess.php deleted file mode 100644 index a53fde5a..00000000 --- a/TwoFactorAuth/Observer/BackendAuthUserLoginSuccess.php +++ /dev/null @@ -1,53 +0,0 @@ -trustedManager = $trustedManager; - $this->tfa = $tfa; - } - - /** - * @param Observer $observer - * @return void - * @SuppressWarnings("PHPMD.UnusedFormalParameter") - */ - public function execute(Observer $observer) - { - if (!$this->tfa->isEnabled()) { - return; - } - - if ($this->trustedManager->isTrustedDevice()) { - $this->trustedManager->rotateTrustedDeviceToken(); - } - } -} diff --git a/TwoFactorAuth/Observer/ControllerActionPredispatch.php b/TwoFactorAuth/Observer/ControllerActionPredispatch.php index 9324a898..4a393649 100644 --- a/TwoFactorAuth/Observer/ControllerActionPredispatch.php +++ b/TwoFactorAuth/Observer/ControllerActionPredispatch.php @@ -9,15 +9,14 @@ use Magento\Backend\App\AbstractAction; use Magento\Backend\Model\Auth\Session; -use Magento\Backend\Model\UrlInterface; -use Magento\Framework\App\ActionFlag; use Magento\Framework\App\Action\Action; use Magento\Framework\Event\Observer; use Magento\Framework\Event\ObserverInterface; +use Magento\User\Model\User; use Magento\TwoFactorAuth\Api\TfaInterface; use Magento\TwoFactorAuth\Api\TfaSessionInterface; -use Magento\TwoFactorAuth\Api\TrustedManagerInterface; -use Magento\User\Model\User; +use Magento\TwoFactorAuth\Api\UserConfigRequestManagerInterface; +use Magento\TwoFactorAuth\Model\UserConfig\HtmlAreaTokenVerifier; /** * Handle redirection to 2FA page if required @@ -30,52 +29,49 @@ class ControllerActionPredispatch implements ObserverInterface private $tfa; /** - * @var ActionFlag + * @var TfaSessionInterface */ - private $actionFlag; + private $tfaSession; /** - * @var UrlInterface + * @var Session */ - private $url; + private $session; /** - * @var TfaSessionInterface + * @var UserConfigRequestManagerInterface */ - private $tfaSession; + private $configRequestManager; /** - * @var Session + * @var AbstractAction|null */ - private $session; + private $action; /** - * @var TrustedManagerInterface + * @var HtmlAreaTokenVerifier */ - private $trustedManager; + private $tokenManager; /** * @param TfaInterface $tfa - * @param ActionFlag $actionFlag - * @param UrlInterface $url - * @param Session $session * @param TfaSessionInterface $tfaSession - * @param TrustedManagerInterface $trustedManager + * @param Session $session + * @param UserConfigRequestManagerInterface $configRequestManager + * @param HtmlAreaTokenVerifier $tokenManager */ public function __construct( TfaInterface $tfa, - ActionFlag $actionFlag, - UrlInterface $url, - Session $session, TfaSessionInterface $tfaSession, - TrustedManagerInterface $trustedManager + Session $session, + UserConfigRequestManagerInterface $configRequestManager, + HtmlAreaTokenVerifier $tokenManager ) { $this->tfa = $tfa; - $this->actionFlag = $actionFlag; - $this->url = $url; $this->tfaSession = $tfaSession; $this->session = $session; - $this->trustedManager = $trustedManager; + $this->configRequestManager = $configRequestManager; + $this->tokenManager = $tokenManager; } /** @@ -87,32 +83,46 @@ private function getUser(): ?User return $this->session->getUser(); } + /** + * Redirect user to given URL. + * + * @param string $url + * @return void + */ + private function redirect(string $url): void + { + $this->action->getActionFlag()->set('', Action::FLAG_NO_DISPATCH, true); + $this->action->getResponse()->setRedirect($this->action->getUrl($url)); + } + /** * @inheritDoc */ public function execute(Observer $observer) { - if (!$this->tfa->isEnabled()) { - return; - } - /** @var $controllerAction AbstractAction */ - $controllerAction = $observer->getEvent()->getControllerAction(); + $controllerAction = $observer->getEvent()->getData('controller_action'); + $this->action = $controllerAction; $fullActionName = $controllerAction->getRequest()->getFullActionName(); + $user = $this->getUser(); if (in_array($fullActionName, $this->tfa->getAllowedUrls(), true)) { + //Actions that are used for 2FA must remain accessible. return; } - $user = $this->getUser(); - if ($user && !empty($this->tfa->getUserProviders((int) $user->getId()))) { - $accessGranted = ($this->tfaSession->isGranted() || $this->trustedManager->isTrustedDevice()) && - empty($this->tfa->getProvidersToActivate((int) $user->getId())); - - if (!$accessGranted) { - $this->actionFlag->set('', Action::FLAG_NO_DISPATCH, true); - $url = $this->url->getUrl('tfa/tfa/index'); - $controllerAction->getResponse()->setRedirect($url); + if ($user) { + if ($this->configRequestManager->isConfigurationRequiredFor((int)$user->getId())) { + //User must configure 2FA first + $this->tokenManager->readConfigToken(); + //User needs special link with a token to be allowed to configure 2FA + $this->redirect('tfa/tfa/requestconfig'); + } else { + //2FA required + $accessGranted = $this->tfaSession->isGranted(); + if (!$accessGranted) { + $this->redirect('tfa/tfa/index'); + } } } } diff --git a/TwoFactorAuth/Test/Integration/Controller/Adminhtml/Authy/ConfigureTest.php b/TwoFactorAuth/Test/Integration/Controller/Adminhtml/Authy/ConfigureTest.php new file mode 100644 index 00000000..d2509c54 --- /dev/null +++ b/TwoFactorAuth/Test/Integration/Controller/Adminhtml/Authy/ConfigureTest.php @@ -0,0 +1,55 @@ +expectedNoAccessResponseCode = 302; + } + + /** + * @inheritDoc + * @magentoConfigFixture default/twofactorauth/general/force_providers duo_security + * @magentoConfigFixture default/twofactorauth/duo/integration_key duo_security + * @magentoConfigFixture default/twofactorauth/duo/secret_key duo_security + * @magentoConfigFixture default/twofactorauth/duo/api_hostname duo_security + * @magentoConfigFixture default/twofactorauth/duo/application_key duo_security + */ + public function testTokenAccess(): void + { + parent::testTokenAccess(); + //Redirect when isAllowed returns false + $this->assertRedirect($this->stringContains('auth/login')); + } + + /** + * @inheritDoc + * @magentoConfigFixture default/twofactorauth/general/force_providers duo_security + * @magentoConfigFixture default/twofactorauth/duo/integration_key duo_security + * @magentoConfigFixture default/twofactorauth/duo/secret_key duo_security + * @magentoConfigFixture default/twofactorauth/duo/api_hostname duo_security + * @magentoConfigFixture default/twofactorauth/duo/application_key duo_security + */ + public function testAclHasAccess() + { + $this->expectedNoAccessResponseCode = 200; + parent::testAclHasAccess(); + //Redirect that Authpost supplies when signatures is not provided in a request. + $this->assertRedirect($this->stringContains('duo/auth')); + } + + /** + * @inheritDoc + * @magentoConfigFixture default/twofactorauth/general/force_providers duo_security + * @magentoConfigFixture default/twofactorauth/duo/integration_key duo_security + * @magentoConfigFixture default/twofactorauth/duo/secret_key duo_security + * @magentoConfigFixture default/twofactorauth/duo/api_hostname duo_security + * @magentoConfigFixture default/twofactorauth/duo/application_key duo_security + */ + public function testAclNoAccess() + { + parent::testAclNoAccess(); + //Redirect when isAllowed returns false + $this->assertRedirect($this->stringContains('auth/login')); + } +} diff --git a/TwoFactorAuth/Test/Integration/Controller/Adminhtml/Google/ConfigureTest.php b/TwoFactorAuth/Test/Integration/Controller/Adminhtml/Google/ConfigureTest.php new file mode 100644 index 00000000..453cfa0f --- /dev/null +++ b/TwoFactorAuth/Test/Integration/Controller/Adminhtml/Google/ConfigureTest.php @@ -0,0 +1,52 @@ +tokenManager = Bootstrap::getObjectManager()->get(UserConfigTokenManagerInterface::class); + } + + /** + * Verify that 2FA providers form is shown to users when 2FA for the app is not configured and token is present. + * + * @return void + */ + public function testList(): void + { + $this->getRequest() + ->setQueryValue('tfat', $this->tokenManager->issueFor((int)$this->_session->getUser()->getId())); + $this->dispatch($this->uri); + $this->assertRegExp('/google/', $this->getResponse()->getBody()); + } + + /** + * Verify that 2FA config request is displayed for users when 2FA is not configured for the user. + * + * @return void + */ + public function testWithoutToken(): void + { + $this->dispatch($this->uri); + $this->assertRedirect($this->stringContains('requestconfig')); + } + + /** + * @inheritDoc + */ + public function testAclHasAccess() + { + $this->getRequest() + ->setQueryValue('tfat', $this->tokenManager->issueFor((int)$this->_session->getUser()->getId())); + parent::testAclHasAccess(); + } + + /** + * @inheritDoc + */ + public function testAclNoAccess() + { + $this->getRequest() + ->setQueryValue('tfat', $this->tokenManager->issueFor((int)$this->_session->getUser()->getId())); + parent::testAclNoAccess(); + $this->assertRedirect($this->stringContains('login')); + } +} diff --git a/TwoFactorAuth/Test/Integration/Controller/Adminhtml/Tfa/ConfigurepostTest.php b/TwoFactorAuth/Test/Integration/Controller/Adminhtml/Tfa/ConfigurepostTest.php new file mode 100644 index 00000000..dd0ac7a6 --- /dev/null +++ b/TwoFactorAuth/Test/Integration/Controller/Adminhtml/Tfa/ConfigurepostTest.php @@ -0,0 +1,154 @@ +tokenManager = Bootstrap::getObjectManager()->get(UserConfigTokenManagerInterface::class); + $this->tfa = Bootstrap::getObjectManager()->get(TfaInterface::class); + } + + /** + * Verify that 2FA providers are updated when a user submits the form. + * + * @return void + */ + public function testUpdated(): void + { + $providerCode = Google::CODE; + $this->getRequest()->setMethod(Request::METHOD_POST); + $this->getRequest() + ->setParam('tfat', $this->tokenManager->issueFor((int)$this->_session->getUser()->getId())); + $this->getRequest()->setPostValue([ + 'tfa_selected' => [$providerCode => 'on'] + ]); + $this->dispatch($this->uri); + $this->assertRedirect($this->stringContains('admin')); + $this->assertNotEmpty($providers = $this->tfa->getForcedProviders()); + /** @var ProviderInterface $provider */ + $provider = array_pop($providers); + $this->assertEquals($providerCode, $provider->getCode()); + } + + /** + * Verify that token is required to proceed. + * + * @return void + */ + public function testWithoutToken(): void + { + $this->getRequest()->setMethod(Request::METHOD_POST); + $this->getRequest()->setPostValue([ + 'tfa_selected' => [Google::CODE => 'on'] + ]); + $this->dispatch($this->uri); + $this->assertRedirect($this->stringContains('login')); + $this->assertEmpty($this->tfa->getForcedProviders()); + } + + /** + * Verify that token is required to proceed even if providers area already configured. + * + * @return void + * @magentoConfigFixture default/twofactorauth/general/force_providers google + */ + public function testConfiguredWithoutToken(): void + { + $this->getRequest()->setMethod(Request::METHOD_POST); + $this->getRequest()->setPostValue([ + 'tfa_selected' => ['nonExisting' => 'on'] + ]); + $this->dispatch($this->uri); + $this->assertRedirect($this->stringContains('login')); + } + + /** + * Verify that 2FA providers are validated + * + * @return void + */ + public function testValidated(): void + { + $this->getRequest()->setMethod(Request::METHOD_POST); + $this->getRequest() + ->setQueryValue('tfat', $this->tokenManager->issueFor((int)$this->_session->getUser()->getId())); + $this->getRequest()->setPostValue([ + 'tfa_selected' => ['nonExisting' => 'on'] + ]); + $this->dispatch($this->uri); + $this->assertRedirect($this->stringContains('configure')); + $this->assertEmpty($this->tfa->getForcedProviders()); + $this->assertSessionMessages($this->contains(__('Please select valid providers.')->render())); + } + + /** + * @inheritDoc + */ + public function testAclHasAccess() + { + $this->markTestSkipped('Subsequently tested with the tests above.'); + } + + /** + * @inheritDoc + */ + public function testAclNoAccess() + { + $this->getRequest() + ->setQueryValue('tfat', $this->tokenManager->issueFor((int)$this->_session->getUser()->getId())); + parent::testAclNoAccess(); + $this->assertRedirect($this->stringContains('login')); + } +} diff --git a/TwoFactorAuth/Test/Integration/Controller/Adminhtml/Tfa/IndexTest.php b/TwoFactorAuth/Test/Integration/Controller/Adminhtml/Tfa/IndexTest.php new file mode 100644 index 00000000..2580ee41 --- /dev/null +++ b/TwoFactorAuth/Test/Integration/Controller/Adminhtml/Tfa/IndexTest.php @@ -0,0 +1,77 @@ +tfa = Bootstrap::getObjectManager()->get(TfaInterface::class); + } + + /** + * Verify that user is taken to Config Request page when 2FA is not configured. + * + * @return void + */ + public function testNoneConfigured(): void + { + $this->dispatch($this->uri); + $this->assertRedirect($this->stringContains('requestconfig')); + } + + /** + * Verify that user is taken to provider's configuration page when only personal 2FA is not configured. + * + * @return void + * @magentoConfigFixture default/twofactorauth/general/force_providers google + */ + public function testUserNotConfigured(): void + { + $this->dispatch($this->uri); + $this->assertRedirect($this->stringContains('google')); + } + + /** + * Verify that user is taken to configured provider's challenge page. + * + * @return void + * @magentoConfigFixture default/twofactorauth/general/force_providers google + * @magentoDbIsolation enabled + */ + public function testConfigured(): void + { + //Activating a provider for the user. + $this->tfa->getProvider(Google::CODE)->activate((int)$this->_session->getUser()->getId()); + + $this->dispatch($this->uri); + //Taken to the provider's challenge page. + $this->assertRedirect($this->stringContains('google')); + } +} diff --git a/TwoFactorAuth/Test/Integration/Controller/Adminhtml/Tfa/RequestconfigTest.php b/TwoFactorAuth/Test/Integration/Controller/Adminhtml/Tfa/RequestconfigTest.php new file mode 100644 index 00000000..fa329029 --- /dev/null +++ b/TwoFactorAuth/Test/Integration/Controller/Adminhtml/Tfa/RequestconfigTest.php @@ -0,0 +1,110 @@ +tfa = Bootstrap::getObjectManager()->get(TfaInterface::class); + $this->tokenManager = Bootstrap::getObjectManager()->get(UserConfigTokenManagerInterface::class); + } + + /** + * Verify that 2FA config request is display for users when 2FA is not configured for the app. + * + * @return void + */ + public function testAppConfigRequested(): void + { + $this->dispatch($this->uri); + $this->assertRegExp('/You need to configure Two\-Factor Authorization/', $this->getResponse()->getBody()); + } + + /** + * Verify that 2FA config request is display for users when 2FA is not configured for the user. + * + * @return void + * @magentoConfigFixture default/twofactorauth/general/force_providers google + */ + public function testUserConfigRequested(): void + { + $this->dispatch($this->uri); + $this->assertRegExp('/You need to configure Two\-Factor Authorization/', $this->getResponse()->getBody()); + } + + + /** + * Verify that 2FA config is not requested when 2FA is configured. + * + * @return void + * @magentoConfigFixture default/twofactorauth/general/force_providers google + * @magentoDbIsolation enabled + * @expectedException \Magento\Framework\Exception\AuthorizationException + */ + public function testNotRequested(): void + { + $this->tfa->getProvider(Google::CODE)->activate((int)$this->_session->getUser()->getId()); + $this->dispatch($this->uri); + } + + /** + * Verify that users with valid tokens get redirected to the app 2FA config page. + * + * @return void + */ + public function testRedirectToAppConfig(): void + { + $this->getRequest() + ->setQueryValue('tfat', $this->tokenManager->issueFor((int)$this->_session->getUser()->getId())); + $this->dispatch($this->uri); + $this->assertRedirect($this->stringContains('tfa/configure')); + } + + /** + * Verify that users with valid tokens get redirected to the user 2FA config page. + * + * @return void + * @magentoConfigFixture default/twofactorauth/general/force_providers google + */ + public function testRedirectToUserConfig(): void + { + $this->getRequest() + ->setQueryValue('tfat', $this->tokenManager->issueFor((int)$this->_session->getUser()->getId())); + $this->dispatch($this->uri); + $this->assertRedirect($this->stringContains('tfa/index')); + } +} diff --git a/TwoFactorAuth/Test/Integration/Controller/Adminhtml/U2f/ConfigureTest.php b/TwoFactorAuth/Test/Integration/Controller/Adminhtml/U2f/ConfigureTest.php new file mode 100644 index 00000000..8ae1eaa8 --- /dev/null +++ b/TwoFactorAuth/Test/Integration/Controller/Adminhtml/U2f/ConfigureTest.php @@ -0,0 +1,52 @@ +cookieReader = Bootstrap::getObjectManager()->get(CookieReaderInterface::class); + $this->tokenManager = Bootstrap::getObjectManager()->get(UserConfigTokenManagerInterface::class); + $this->tfaSession = Bootstrap::getObjectManager()->get(TfaSessionInterface::class); + $this->tfa = Bootstrap::getObjectManager()->get(TfaInterface::class); + } + + /** + * Verify that users with configured 2FA and 2FA completed can proceed to desired page. + * + * @return void + * @magentoConfigFixture default/twofactorauth/general/force_providers google + * @magentoDbIsolation enabled + * @magentoAppIsolation enabled + */ + public function testTfaCompleted(): void + { + //Configuring 2FA for the user and completing 2FA. + $this->tfa->getProvider(Google::CODE)->activate((int)$this->_session->getUser()->getId()); + $this->tfaSession->grantAccess(); + //Accessing a page in adminhtml area + $this->dispatch('backend/admin/user/'); + //Authenticated user with 2FA configured and completed is taken to the Users page as requested. + $this->assertRegExp('/' .$this->_session->getUser()->getUserName() .'/i', $this->getResponse()->getBody()); + } + + /** + * Verify that unauthenticated user is redirected to login page. + * + * @return void + * @magentoAppIsolation enabled + */ + public function testUnauthenticated(): void + { + $this->logout(); + $this->dispatch('backend/admin/index/index'); + //Login controller redirects to full start-up URL + $this->assertRedirect($this->stringContains('index')); + $properUrl = $this->getResponse()->getHeader('Location')->getFieldValue(); + + //Login page must be rendered without redirects + $this->getRequest()->setDispatched(false); + $this->getRequest()->setUri($properUrl); + $this->dispatch($properUrl); + $this->assertContains('Welcome, please sign in', $this->getResponse()->getBody()); + } + + /** + * Verify that users would be redirected to "2FA Config Request" page when 2FA is not configured for the app. + * + * @return void + */ + public function testConfigRequested(): void + { + //Accessing a page in adminhtml area + $this->dispatch('backend/admin/user/'); + //Authenticated user gets a redirect to 2FA configuration page since none is configured. + $this->assertRedirect($this->stringContains('requestconfig')); + } + + /** + * Verify that users would be redirected to "2FA Config Request" page when 2FA is not configured for the user. + * + * @return void + * @magentoConfigFixture default/twofactorauth/general/force_providers google + */ + public function testUserConfigRequested(): void + { + //Accessing a page in adminhtml area + $this->dispatch('backend/admin/user/'); + //Authenticated user gets a redirect to 2FA configuration page since none is configured for the user. + $this->assertRedirect($this->stringContains('requestconfig')); + } + + /** + * Verify that users returning with a token from the E-mail get a new cookie with it. + * + * @return void + */ + public function testCookieSet(): void + { + //Accessing a page in adminhtml area with a valid token. + $this->getRequest() + ->setQueryValue('tfat', $token = $this->tokenManager->issueFor((int)$this->_session->getUser()->getId())); + $this->dispatch('backend/admin/user/'); + //Authenticated user gets a redirect to 2FA configuration page since none is configured. + $this->assertRedirect($this->stringContains('requestconfig')); + $this->assertNotEmpty($cookie = $this->cookieReader->getCookie('tfa-ct')); + $this->assertEquals($token, $cookie); + } + + /** + * Verify that users returning with a valid token from the E-mail and 2FA configured get redirected to 2FA form. + * + * @return void + * @magentoConfigFixture default/twofactorauth/general/force_providers google + * @magentoDbIsolation enabled + */ + public function testTfaChallenged(): void + { + $this->tfa->getProvider(Google::CODE)->activate((int)$this->_session->getUser()->getId()); + //Accessing a page in adminhtml area + $this->dispatch('backend/admin/user/'); + //Authenticated user with 2FA configured is redirected to 2FA code form. + $this->assertRedirect($this->stringContains('tfa/tfa/index')); + } +} diff --git a/TwoFactorAuth/Test/Integration/UserConfigRequestManagerTest.php b/TwoFactorAuth/Test/Integration/UserConfigRequestManagerTest.php new file mode 100644 index 00000000..c694420f --- /dev/null +++ b/TwoFactorAuth/Test/Integration/UserConfigRequestManagerTest.php @@ -0,0 +1,171 @@ +create(User::class); + $user->loadByUsername(\Magento\TestFramework\Bootstrap::ADMIN_NAME); + $this->user = $user; + $this->tfa = Bootstrap::getObjectManager()->get(TfaInterface::class); + $this->transportBuilderMock = Bootstrap::getObjectManager()->get(TransportBuilderMock::class); + $this->tokenManager = Bootstrap::getObjectManager()->get(UserConfigTokenManagerInterface::class); + $this->aclBuilder = Bootstrap::getObjectManager()->get(Builder::class); + + $this->manager = Bootstrap::getObjectManager()->get(UserConfigRequestManagerInterface::class); + } + + /** + * Check that config is required when no providers are enabled for the app. + * + * @return void + */ + public function testIsRequiredWithoutAppProviders(): void + { + $this->assertTrue($this->manager->isConfigurationRequiredFor((int)$this->user->getId())); + } + + /** + * Check that config is required when personal provider config is empty. + * + * @return void + * @magentoConfigFixture default/twofactorauth/general/force_providers google + */ + public function testIsRequiredWithoutUserProviders(): void + { + $this->assertTrue($this->manager->isConfigurationRequiredFor((int)$this->user->getId())); + } + + /** + * Check that config is not required when both app and personal provider config is present. + * + * @return void + * @magentoConfigFixture default/twofactorauth/general/force_providers google + * @magentoDbIsolation enabled + */ + public function testIsRequiredWithConfig(): void + { + $this->tfa->getProvider(Google::CODE)->activate((int)$this->user->getId()); + $this->assertFalse($this->manager->isConfigurationRequiredFor((int)$this->user->getId())); + } + + /** + * Check that app config request E-mail is NOT sent for a user that does not posses proper rights. + * + * @return void + * @expectedException \Magento\Framework\Exception\AuthorizationException + * @throws \Throwable + * @magentoAppArea adminhtml + * @magentoAppIsolation enabled + */ + public function testFailAppConfigRequest(): void + { + $this->aclBuilder->getAcl()->deny(null, 'Magento_TwoFactorAuth::config'); + $this->manager->sendConfigRequestTo($this->user); + } + + /** + * Check that app config request E-mail is sent for a user that posseses proper rights. + * + * @return void + * @throws \Throwable + * @magentoAppArea adminhtml + */ + public function testSendAppConfigRequest(): void + { + $this->manager->sendConfigRequestTo($this->user); + + $this->assertNotEmpty($message = $this->transportBuilderMock->getSentMessage()); + $messageHtml = $message->getBody()->getParts()[0]->getRawContent(); + $this->assertContains( + 'You are required to configure website-wide and personal Two-Factor Authorization in order to login to', + $messageHtml + ); + $this->assertThat( + $messageHtml, + $this->matchesRegularExpression( + '/\/s' + ) + ); + preg_match('/\/tfat\/([^\/]+)/s', $messageHtml, $tokenMatches); + $this->assertNotEmpty($tokenMatches[1]); + $token = urldecode($tokenMatches[1]); + $this->assertTrue($this->tokenManager->isValidFor((int)$this->user->getId(), $token)); + } + + /** + * Check that personal 2FA config request E-mail is sent for users. + * + * @return void + * @throws \Throwable + * @magentoAppArea adminhtml + * @magentoConfigFixture default/twofactorauth/general/force_providers google + */ + public function testSendUserConfigRequest(): void + { + $this->manager->sendConfigRequestTo($this->user); + + $this->assertNotEmpty($message = $this->transportBuilderMock->getSentMessage()); + $messageHtml = $message->getBody()->getParts()[0]->getRawContent(); + $this->assertContains( + 'You are required to configure personal Two-Factor Authorization in order to login to', + $messageHtml + ); + $this->assertThat( + $messageHtml, + $this->matchesRegularExpression( + '/\/s' + ) + ); + preg_match('/\/tfat\/([^\/]+)/s', $messageHtml, $tokenMatches); + $this->assertNotEmpty($tokenMatches[1]); + $token = urldecode($tokenMatches[1]); + $this->assertTrue($this->tokenManager->isValidFor((int)$this->user->getId(), $token)); + } +} diff --git a/TwoFactorAuth/Test/Integration/UserConfigTokenManagerTest.php b/TwoFactorAuth/Test/Integration/UserConfigTokenManagerTest.php new file mode 100644 index 00000000..24165295 --- /dev/null +++ b/TwoFactorAuth/Test/Integration/UserConfigTokenManagerTest.php @@ -0,0 +1,84 @@ +dateTimeMock = $this->getMockBuilder(DateTime::class)->disableOriginalConstructor()->getMock(); + $this->userFactory = Bootstrap::getObjectManager()->get(UserFactory::class); + $this->tokenManager = Bootstrap::getObjectManager()->create( + UserConfigTokenManagerInterface::class, + ['dateTime' => $this->dateTimeMock] + ); + } + + /** + * Test that issued tokens are valid for specific users and can expire. + * + * @return void + * @magentoDataFixture Magento/User/_files/user_with_role.php + */ + public function testToken(): void + { + $time = time(); + $this->dateTimeMock->method('timestamp') + ->willReturnCallback( + function () use (&$time) { + return $time; + } + ); + /** @var \Magento\User\Model\User $user1 */ + $user1 = $this->userFactory->create(); + $user1->loadByUsername(\Magento\TestFramework\Bootstrap::ADMIN_NAME); + /** @var \Magento\User\Model\User $user2 */ + $user2 = $this->userFactory->create(); + $user2->loadByUsername('adminUser'); + $this->assertNotEmpty($user1->getId()); + $this->assertNotEmpty($user2->getId()); + + //Checking that tokens for different users are different even when issued at the same time. + $token1 = $this->tokenManager->issueFor((int)$user1->getId()); + $token2 = $this->tokenManager->issueFor((int)$user2->getId()); + $this->assertNotEquals($token1, $token2); + + //Checking that valid tokens are only valid for corresponding users. + $time += 5; + $this->assertTrue($this->tokenManager->isValidFor((int)$user1->getId(), $token1)); + $this->assertTrue($this->tokenManager->isValidFor((int)$user2->getId(), $token2)); + $this->assertFalse($this->tokenManager->isValidFor((int)$user1->getId(), $token2)); + $this->assertFalse($this->tokenManager->isValidFor((int)$user2->getId(), $token1)); + + //Checking that tokens do expire + $time += 3601; + $this->assertFalse($this->tokenManager->isValidFor((int)$user1->getId(), $token1)); + $this->assertFalse($this->tokenManager->isValidFor((int)$user2->getId(), $token2)); + } +} diff --git a/TwoFactorAuth/Test/Unit/Model/Config/Backend/ForceProvidersTest.php b/TwoFactorAuth/Test/Unit/Model/Config/Backend/ForceProvidersTest.php new file mode 100644 index 00000000..ecc70a81 --- /dev/null +++ b/TwoFactorAuth/Test/Unit/Model/Config/Backend/ForceProvidersTest.php @@ -0,0 +1,48 @@ +model = $objectManager->getObject(ForceProviders::class); + } + + /** + * Check that beforeSave validates values. + * + * @return void + * @expectedException \Magento\Framework\Exception\ValidatorException + */ + public function testBeforeSaveInvalid(): void + { + $this->model->setValue(''); + $this->model->beforeSave(); + } + + /** + * Check that beforeSave validates values. + * + * @return void + */ + public function testBeforeSaveValid(): void + { + $this->model->setValue('provider1, provider2'); + $this->model->beforeSave(); + } +} diff --git a/TwoFactorAuth/Test/Unit/Model/Provider/Engine/AuthyTest.php b/TwoFactorAuth/Test/Unit/Model/Provider/Engine/AuthyTest.php new file mode 100644 index 00000000..fd6c011c --- /dev/null +++ b/TwoFactorAuth/Test/Unit/Model/Provider/Engine/AuthyTest.php @@ -0,0 +1,77 @@ +serviceMock = $this->getMockBuilder(Authy\Service::class)->disableOriginalConstructor()->getMock(); + + $this->model = $objectManager->getObject(Authy::class, ['service' => $this->serviceMock]); + } + + /** + * Check that trusted devices functionality is disabled. + * + * @return void + */ + public function testIsTrustedDevicesAllowed(): void + { + $this->assertFalse($this->model->isTrustedDevicesAllowed()); + } + + /** + * Enabled test dataset. + * + * @return array + */ + public function getIsEnabledTestDataSet(): array + { + return [ + 'api key present' => [ + 'api-key', + true + ], + 'api key not configured' => [ + null, + false + ] + ]; + } + + /** + * Check that the provider is available based on configuration. + * + * @param string|null $apiKey + * @param bool $expected + * @return void + * @dataProvider getIsEnabledTestDataSet + */ + public function testIsEnabled(?string $apiKey, bool $expected): void + { + $this->serviceMock->method('getApiKey')->willReturn($apiKey); + + $this->assertEquals($expected, $this->model->isEnabled()); + } +} diff --git a/TwoFactorAuth/Test/Unit/Model/Provider/Engine/DuoSecurityTest.php b/TwoFactorAuth/Test/Unit/Model/Provider/Engine/DuoSecurityTest.php new file mode 100644 index 00000000..495380ea --- /dev/null +++ b/TwoFactorAuth/Test/Unit/Model/Provider/Engine/DuoSecurityTest.php @@ -0,0 +1,127 @@ +configMock = $this->getMockBuilder(ScopeConfigInterface::class)->disableOriginalConstructor()->getMock(); + + $this->model = $objectManager->getObject(DuoSecurity::class, ['scopeConfig' => $this->configMock]); + } + + /** + * Check that trusted devices functionality is disabled. + * + * @return void + */ + public function testIsTrustedDevicesAllowed(): void + { + $this->assertFalse($this->model->isTrustedDevicesAllowed()); + } + + /** + * Enabled test dataset. + * + * @return array + */ + public function getIsEnabledTestDataSet(): array + { + return [ + [ + 'value', + 'value', + 'value', + 'value', + true + ], + [ + null, + null, + null, + null, + false + ], + [ + 'value', + null, + null, + null, + false + ], + [ + null, + 'value', + null, + null, + false + ], + [ + null, + null, + 'value', + null, + false + ], + [ + null, + null, + null, + 'value', + false + ] + ]; + } + + /** + * Check that the provider is available based on configuration. + * + * @param string|null $apiHostname + * @param string|null $appKey + * @param string|null $secretKey + * @param string|null $integrationKey + * @param bool $expected + * @return void + * @dataProvider getIsEnabledTestDataSet + */ + public function testIsEnabled( + ?string $apiHostname, + ?string $appKey, + ?string $secretKey, + ?string $integrationKey, + bool $expected + ): void { + $this->configMock->method('getValue')->willReturnMap( + [ + [DuoSecurity::XML_PATH_API_HOSTNAME, 'default', null, $apiHostname], + [DuoSecurity::XML_PATH_APPLICATION_KEY, 'default', null, $appKey], + [DuoSecurity::XML_PATH_SECRET_KEY, 'default', null, $secretKey], + [DuoSecurity::XML_PATH_INTEGRATION_KEY, 'default', null, $integrationKey] + ] + ); + + $this->assertEquals($expected, $this->model->isEnabled()); + } +} diff --git a/TwoFactorAuth/Test/Unit/Model/Provider/Engine/GoogleTest.php b/TwoFactorAuth/Test/Unit/Model/Provider/Engine/GoogleTest.php new file mode 100644 index 00000000..c0cffd8a --- /dev/null +++ b/TwoFactorAuth/Test/Unit/Model/Provider/Engine/GoogleTest.php @@ -0,0 +1,45 @@ +model = $objectManager->getObject(Google::class); + } + + /** + * Check that trusted devices functionality is disabled. + * + * @return void + */ + public function testIsTrustedDevicesAllowed(): void + { + $this->assertFalse($this->model->isTrustedDevicesAllowed()); + } + + /** + * Check that the provider is available based on configuration. + * + * @return void + */ + public function testIsEnabled(): void { + $this->assertTrue($this->model->isEnabled()); + } +} diff --git a/TwoFactorAuth/Test/Unit/Model/Provider/Engine/U2fKeyTest.php b/TwoFactorAuth/Test/Unit/Model/Provider/Engine/U2fKeyTest.php new file mode 100644 index 00000000..282221cb --- /dev/null +++ b/TwoFactorAuth/Test/Unit/Model/Provider/Engine/U2fKeyTest.php @@ -0,0 +1,45 @@ +model = $objectManager->getObject(U2fKey::class); + } + + /** + * Check that trusted devices functionality is disabled. + * + * @return void + */ + public function testIsTrustedDevicesAllowed(): void + { + $this->assertFalse($this->model->isTrustedDevicesAllowed()); + } + + /** + * Check that the provider is available based on configuration. + * + * @return void + */ + public function testIsEnabled(): void { + $this->assertTrue($this->model->isEnabled()); + } +} diff --git a/TwoFactorAuth/Test/Unit/Model/ProviderTest.php b/TwoFactorAuth/Test/Unit/Model/ProviderTest.php new file mode 100644 index 00000000..14737936 --- /dev/null +++ b/TwoFactorAuth/Test/Unit/Model/ProviderTest.php @@ -0,0 +1,44 @@ +model = $objectManager->getObject( + Provider::class, + [ + 'code' => 'test', + 'name' => 'test', + 'icon' => 'test.jpg', + 'configureAction' =>'configure', + 'authAction' => 'auth' + ] + ); + } + + /** + * Check that trusted devices functionality is disabled + * + * @return void + */ + public function testTrustedDevicesEnabled(): void + { + $this->assertFalse($this->model->isTrustedDevicesAllowed()); + } +} diff --git a/TwoFactorAuth/Test/Unit/Model/TfaTest.php b/TwoFactorAuth/Test/Unit/Model/TfaTest.php new file mode 100644 index 00000000..d115e932 --- /dev/null +++ b/TwoFactorAuth/Test/Unit/Model/TfaTest.php @@ -0,0 +1,236 @@ +pool = $this->getMockForAbstractClass(ProviderPoolInterface::class); + $this->pool->method('getProviders') + ->willReturnCallback( + function (): array { + return $this->providersMockList; + } + ); + $this->pool->method('getProviderByCode') + ->willReturnCallback( + function (string $code): ProviderInterface { + if (array_key_exists($code, $this->providersMockList)) { + return $this->providersMockList[$code]; + } + throw new NoSuchEntityException(); + } + ); + $this->configMock = $this->getMockForAbstractClass(ScopeConfigInterface::class); + + $this->model = $objectManager->getObject( + Tfa::class, + ['providerPool' => $this->pool, 'scopeConfig' => $this->configMock] + ); + } + + /** + * Define existing providers. + * + * @param array $providersData Keys - codes, values - {enabled: bool}. + * @return void + */ + private function defineProvidersList(array $providersData): void + { + $providers = []; + foreach ($providersData as $code => $providerData) { + $provider = $this->getMockForAbstractClass(ProviderInterface::class); + $provider->method('getCode')->willReturn($code); + $provider->method('isEnabled')->willReturn($providerData['enabled']); + $providers[$code] = $provider; + } + + $this->providersMockList = $providers; + } + + /** + * Extract codes from a providers list. + * + * @param ProviderInterface[] $providers + * @return string[] + */ + private function extractProviderCodes(array $providers): array + { + return array_map( + function (ProviderInterface $provider): string { + return $provider->getCode(); + }, + $providers + ); + } + + /** + * Check that enabled providers list updates when the pool or providers update. + * + * @return void + */ + public function testAllEnabledProvidersUpdates(): void + { + $providersData = ['provider1' => ['enabled' => true], 'provider2' => ['enabled' => true]]; + $this->defineProvidersList($providersData); + $this->assertEquals( + ['provider1', 'provider2'], + $this->extractProviderCodes($this->model->getAllEnabledProviders()) + ); + + $providersData = ['provider1' => ['enabled' => true], 'provider2' => ['enabled' => false]]; + $this->defineProvidersList($providersData); + $this->assertEquals( + ['provider1'], + $this->extractProviderCodes($this->model->getAllEnabledProviders()) + ); + + $providersData = [ + 'provider1' => ['enabled' => true], + 'provider2' => ['enabled' => true], + 'provider3' => ['enabled' => true] + ]; + $this->defineProvidersList($providersData); + $this->assertEquals( + ['provider1', 'provider2', 'provider3'], + $this->extractProviderCodes($this->model->getAllEnabledProviders()) + ); + } + + /** + * Data set for the forcedProviders test. + * + * @return array + */ + public function getForcedProvidersDataSet(): array + { + return [ + 'not defined' => [ + null, + ['provider1' => ['enabled' => true]], + [] + ], + 'not defined string' => [ + '', + ['provider1' => ['enabled' => true]], + [] + ], + 'not defined array' => [ + [], + ['provider1' => ['enabled' => true]], + [] + ], + 'valid array' => [ + ['provider1'], + ['provider1' => ['enabled' => true], 'provider2' => ['enabled' => true]], + ['provider1'] + ], + 'valid string' => [ + 'provider1, provider2', + ['provider1' => ['enabled' => true], 'provider2' => ['enabled' => true]], + ['provider1', 'provider2'] + ], + 'invalid code' => [ + 'nonExistingProvider', + ['provider1' => ['enabled' => true], 'provider2' => ['enabled' => true]], + [] + ], + 'disabledProvider' => [ + 'provider1', + ['provider1' => ['enabled' => false], 'provider2' => ['enabled' => true]], + [] + ] + ]; + } + + /** + * Test getForcedProviders method. + * + * @param string|null|array $configValue + * @param array $providersList + * @param array $expected + * @return void + * @dataProvider getForcedProvidersDataSet + */ + public function testGetForcedProviders($configValue, array $providersList, $expected): void + { + $this->configMock->method('getValue')->willReturn($configValue); + $this->defineProvidersList($providersList); + + $result = $this->model->getForcedProviders(); + + $this->assertEquals($expected, $this->extractProviderCodes($result)); + } + + /** + * Check that user providers = forced providers. + * + * @return void + */ + public function testGetUserProviders(): void + { + $this->configMock->method('getValue')->willReturnReference($configValue); + $this->defineProvidersList(['provider1' => ['enabled' => true]]); + + $configValue = 'provider1'; + $this->assertEquals(['provider1'], $this->extractProviderCodes($this->model->getUserProviders(1))); + + $configValue = ''; + $this->assertEmpty($this->model->getUserProviders(1)); + } + + /** + * Verify that Trusted Devices functionality was removed. + * + * @return void + */ + public function testGetTrustedDevices(): void + { + $this->assertEmpty($this->model->getTrustedDevices(1)); + } + + /** + * Verify that 2FA is always enabled + * + * @return void + */ + public function testIsEnabled(): void + { + $this->assertTrue($this->model->isEnabled()); + } +} diff --git a/TwoFactorAuth/Test/Unit/Model/UserConfig/HtmlAreaTokenVerifierTest.php b/TwoFactorAuth/Test/Unit/Model/UserConfig/HtmlAreaTokenVerifierTest.php new file mode 100644 index 00000000..501e60a5 --- /dev/null +++ b/TwoFactorAuth/Test/Unit/Model/UserConfig/HtmlAreaTokenVerifierTest.php @@ -0,0 +1,200 @@ +requestMock = $this->getMockForAbstractClass(RequestInterface::class); + $this->tokenManagerMock = $this->getMockForAbstractClass(UserConfigTokenManagerInterface::class); + $this->cookiesMock = $this->getMockForAbstractClass(CookieManagerInterface::class); + $this->cookiesMetaFactoryMock = $this->getMockBuilder(CookieMetadataFactory::class) + ->disableOriginalConstructor() + ->getMock(); + $this->sessionMock = $this->getMockBuilder(Session::class) + ->disableOriginalConstructor() + ->getMock(); + $this->sessionManagerMock = $this->getMockBuilder(SessionManager::class) + ->disableOriginalConstructor() + ->getMock(); + + $this->model = $objectManager->getObject( + HtmlAreaTokenVerifier::class, + [ + 'request' => $this->requestMock, + 'tokenManager' => $this->tokenManagerMock, + 'cookies' => $this->cookiesMock, + 'cookieMetadataFactory' => $this->cookiesMetaFactoryMock, + 'session' => $this->sessionMock, + 'sessionManager' => $this->sessionManagerMock + ] + ); + } + + /** + * Request/cookies data sets. + * + * @return array + */ + public function getTokenRequestData(): array + { + return [ + 'token in query' => [ + true, + 'token', + null, + true, + true, + 'token' + ], + 'token in cookies' => [ + true, + null, + 'token', + true, + false, + 'token' + ], + 'token is absent' => [ + true, + null, + null, + false, + false, + null + ], + 'invalid token' => [ + true, + 'token', + null, + false, + false, + null + ], + 'invalid token from cookies' => [ + true, + null, + 'token', + false, + false, + null + ], + 'token in both' => [ + true, + 'token', + 'token', + true, + false, + 'token' + ], + 'no user' => [ + false, + 'token', + 'token', + true, + false, + null + ] + ]; + } + + /** + * Test "readConfigToken" method with different variation of request/cookie parameters provided. + * + * @param bool $userPresent + * @param string|null $fromRequest + * @param string|null $fromCookies + * @param bool $isValid + * @param bool $cookieSet + * @param string|null $expected + * @return void + * @dataProvider getTokenRequestData + */ + public function testReadConfigToken( + bool $userPresent, + ?string $fromRequest, + ?string $fromCookies, + bool $isValid, + bool $cookieSet, + ?string $expected + ): void { + $this->sessionMock->method('__call') + ->willReturn( + $userPresent ? $this->getMockBuilder(User::class)->disableOriginalConstructor()->getMock() : null + ); + $this->requestMock->method('getParam')->with('tfat')->willReturn($fromRequest); + $this->cookiesMock->method('getCookie')->with('tfa-ct')->willReturn($fromCookies); + $this->tokenManagerMock->method('isValidFor')->willReturn($isValid); + $this->sessionManagerMock->method('getCookiePath')->willReturn('admin_path'); + $this->cookiesMetaFactoryMock->method('createSensitiveCookieMetadata') + ->willReturn( + $metaMock = $this->getMockBuilder(SensitiveCookieMetadata::class) + ->disableOriginalConstructor() + ->getMock() + ); + if ($cookieSet) { + $metaMock->expects($this->atLeastOnce())->method('setPath')->with('admin_path')->willReturn($metaMock); + $this->cookiesMock->expects($this->once()) + ->method('setSensitiveCookie') + ->with('tfa-ct', $fromRequest, $metaMock); + } else { + $this->cookiesMock->expects($this->never())->method('setSensitiveCookie'); + } + + $this->assertEquals($expected, $this->model->readConfigToken()); + } +} diff --git a/TwoFactorAuth/TestFramework/ControllerActionPredispatch.php b/TwoFactorAuth/TestFramework/ControllerActionPredispatch.php new file mode 100644 index 00000000..dc4353e4 --- /dev/null +++ b/TwoFactorAuth/TestFramework/ControllerActionPredispatch.php @@ -0,0 +1,32 @@ +getEvent()->getData('controller_action'); + if (class_exists('Magento\TestFramework\Request') + && $controllerAction->getRequest() instanceof \Magento\TestFramework\Request + && !$controllerAction->getRequest()->getParam('tfa_enabled') + ) { + //Hack that allows integration controller tests that are not aware of 2FA to run + return; + } + + parent::execute($observer); + } +} diff --git a/TwoFactorAuth/TestFramework/TestCase/AbstractBackendController.php b/TwoFactorAuth/TestFramework/TestCase/AbstractBackendController.php new file mode 100644 index 00000000..05a91a43 --- /dev/null +++ b/TwoFactorAuth/TestFramework/TestCase/AbstractBackendController.php @@ -0,0 +1,63 @@ +_objectManager->get(\Magento\Backend\Model\UrlInterface::class)->turnOffSecretKey(); + $this->_auth = $this->_objectManager->get(\Magento\Backend\Model\Auth::class); + $this->_session = $this->_auth->getAuthStorage(); + $this->login(); + } + + /** + * Perform logout + * + * @return void + */ + protected function logout(): void + { + $this->_auth->getAuthStorage()->destroy(['send_expire_cookie' => false]); + } + + /** + * Login as default admin user. + * + * @return void + */ + protected function login(): void + { + $credentials = $this->_getAdminCredentials(); + $this->_auth->login($credentials['user'], $credentials['password']); + /** @var \Magento\Security\Model\Plugin\Auth $authPlugin */ + $authPlugin = $this->_objectManager->get(\Magento\Security\Model\Plugin\Auth::class); + $authPlugin->afterLogin($this->_auth); + } + + /** + * @inheritDoc + */ + public function dispatch($uri) + { + if ($this->getRequest()->getParam('tfa_enabled') === null) { + $this->getRequest()->setParam('tfa_enabled', true); + } + + parent::dispatch($uri); + } +} diff --git a/TwoFactorAuth/TestFramework/TestCase/AbstractConfigureBackendController.php b/TwoFactorAuth/TestFramework/TestCase/AbstractConfigureBackendController.php new file mode 100644 index 00000000..040f58b0 --- /dev/null +++ b/TwoFactorAuth/TestFramework/TestCase/AbstractConfigureBackendController.php @@ -0,0 +1,72 @@ +tokenManager = Bootstrap::getObjectManager()->get(UserConfigTokenManagerInterface::class); + } + + /** + * Verify that even with ACL an admin user needs token to access the page. + * + * @return void + */ + public function testTokenAccess(): void + { + $this->getRequest()->setMethod($this->httpMethod); + $this->dispatch($this->uri); + $this->assertRedirect($this->stringContains('login')); + } + + /** + * Check that a user with proper token and rights can access the page. + */ + public function testAclHasAccess() + { + $this->getRequest() + ->setParam('tfat', $this->tokenManager->issueFor((int)$this->_session->getUser()->getId())); + + parent::testAclHasAccess(); + } + + /** + * Check that a user with proper token but without rights cannot access the page. + */ + public function testAclNoAccess() + { + $this->getRequest() + ->setParam('tfat', $this->tokenManager->issueFor((int)$this->_session->getUser()->getId())); + + parent::testAclNoAccess(); + } +} diff --git a/TwoFactorAuth/Ui/Component/Form/User/DataProvider.php b/TwoFactorAuth/Ui/Component/Form/User/DataProvider.php index 31d32ccb..5bb5289c 100644 --- a/TwoFactorAuth/Ui/Component/Form/User/DataProvider.php +++ b/TwoFactorAuth/Ui/Component/Form/User/DataProvider.php @@ -14,7 +14,6 @@ use Magento\TwoFactorAuth\Api\TfaInterface; use Magento\TwoFactorAuth\Api\UserConfigManagerInterface; use Magento\TwoFactorAuth\Model\Config\Source\EnabledProvider; -use Magento\TwoFactorAuth\Model\Trusted; class DataProvider extends AbstractDataProvider { @@ -117,34 +116,6 @@ private function getResetProviderUrls(User $user) return $resetProviders; } - /** - * Get a list of trusted devices as array - * @param User $user - * @return array - */ - private function getTrustedDevices(User $user) - { - $trustedDevices = $this->tfa->getTrustedDevices((int) $user->getId()); - $res = []; - - foreach ($trustedDevices as $trustedDevice) { - /** @var Trusted $trustedDevice */ - $revokeUrl = $this->url->getUrl('tfa/tfa/revoke', [ - 'id' => $trustedDevice->getId(), - 'user_id' => (int) $user->getId(), - ]); - - $res[] = [ - 'last_ip' => $trustedDevice->getLastIp(), - 'date_time' => $trustedDevice->getDateTime(), - 'device_name' => $trustedDevice->getDeviceName(), - 'revoke_url' => $revokeUrl, - ]; - } - - return $res; - } - /** * @inheritdoc */ @@ -173,11 +144,10 @@ public function getData() foreach ($items as $user) { $providerCodes = $this->userConfigManager->getProvidersCodes((int) $user->getId()); $resetProviders = $this->getResetProviderUrls($user); - $trustedDevices = $this->getTrustedDevices($user); $data = [ 'reset_providers' => $resetProviders, - 'trusted_devices' => $trustedDevices, + 'trusted_devices' => [], 'tfa_providers' => $providerCodes, ]; $this->loadedData[(int) $user->getId()] = $data; diff --git a/TwoFactorAuth/etc/adminhtml/events.xml b/TwoFactorAuth/etc/adminhtml/events.xml index e68010f6..0d5c1f23 100644 --- a/TwoFactorAuth/etc/adminhtml/events.xml +++ b/TwoFactorAuth/etc/adminhtml/events.xml @@ -17,7 +17,4 @@ - - - diff --git a/TwoFactorAuth/etc/adminhtml/system.xml b/TwoFactorAuth/etc/adminhtml/system.xml index 256a6b1e..e94a81c7 100644 --- a/TwoFactorAuth/etc/adminhtml/system.xml +++ b/TwoFactorAuth/etc/adminhtml/system.xml @@ -22,61 +22,18 @@ - - - Magento\Config\Model\Config\Source\Yesno - - - + Magento\TwoFactorAuth\Model\Config\Source\Provider - Force providers to be used by all users in this system. Leave empty to allow individual - selection. - - - - - - - - - Magento\Config\Model\Config\Source\Yesno - - - - NOTE: This option should be used in HTTPS only environments - Magento\Config\Model\Config\Source\Yesno - - - - - - - - Magento\Config\Model\Config\Source\Yesno - Requires HTTPS to work - - - - Magento\Config\Model\Config\Source\Yesno + Two-factor authorization providers for admin users to use during login + Magento\TwoFactorAuth\Model\Config\Backend\ForceProviders - - - Magento\Config\Model\Config\Source\Yesno - @@ -94,21 +51,10 @@ - - - Magento\Config\Model\Config\Source\Yesno - - - - NOTE: This option should be used in HTTPS only environments - Magento\Config\Model\Config\Source\Yesno - diff --git a/TwoFactorAuth/etc/config.xml b/TwoFactorAuth/etc/config.xml index 9a68ca27..7f5571b9 100644 --- a/TwoFactorAuth/etc/config.xml +++ b/TwoFactorAuth/etc/config.xml @@ -10,19 +10,11 @@ - 0 - - 1 - - 1 Login request to your Magento Admin - - 1 - diff --git a/TwoFactorAuth/etc/di.xml b/TwoFactorAuth/etc/di.xml index 4fbc0aa5..6e22186e 100644 --- a/TwoFactorAuth/etc/di.xml +++ b/TwoFactorAuth/etc/di.xml @@ -31,7 +31,7 @@ - + Magento\TwoFactorAuth\Command\TfaDisable @@ -140,4 +140,9 @@ true + + + + + diff --git a/TwoFactorAuth/etc/email_templates.xml b/TwoFactorAuth/etc/email_templates.xml new file mode 100644 index 00000000..addfd7bd --- /dev/null +++ b/TwoFactorAuth/etc/email_templates.xml @@ -0,0 +1,11 @@ + + + +