MC-22950: Enable 2FA by default for Admins

- Code review feedback
- Removed extra code
- Fixed ACL check for unconfigured site with disallowed user role
- Moved session data into dedicated locations
- Refactored userId's to user UserContext

Author: nathanjosiah

TwoFactorAuth/Api/TfaSessionInterface.php

@@ -27,4 +27,18 @@ public function grantAccess(): void;
      * @return bool
      */
     public function isGranted(): bool;
+
+    /**
+     * Get the current configuration for skipped providers
+     *
+     * @return array
+     */
+    public function getSkippedProviderConfig(): array;
+
+    /**
+     * Set the configuration of skipped providers
+     *
+     * @param array $config
+     */
+    public function setSkippedProviderConfig(array $config): void;
 }

TwoFactorAuth/Block/Adminhtml/System/Config/Providers.php

@@ -10,30 +10,30 @@
 use Magento\Framework\Data\Form\Element\AbstractElement;
 use Magento\Backend\Block\Template\Context;
 use Magento\Framework\Phrase;
-use Magento\Framework\Serialize\SerializerInterface;
+use Magento\Framework\Serialize\Serializer\Json;
 
 /**
- * Providers field
+ * Displays a warning modal if the all currently available providers are deselected
  */
 class Providers extends Field
 {
     /**
-     * @var SerializerInterface
+     * @var Json
      */
-    private $serializer;
+    private $json;
 
     /**
      * @param Context $context
-     * @param SerializerInterface $serializer
+     * @param Json $json
      * @param array $data
      */
     public function __construct(
         Context $context,
-        SerializerInterface $serializer,
+        Json $json,
         array $data = []
     ) {
         parent::__construct($context, $data);
-        $this->serializer = $serializer;
+        $this->json = $json;
     }
 
     /**
@@ -50,7 +50,7 @@ protected function _getElementHtml(AbstractElement $element)
                 ]
             ]
         ];
-        $html .= '<script type="text/x-magento-init">' . $this->serializer->serialize($config) . '</script>';
+        $html .= '<script type="text/x-magento-init">' . $this->json->serialize($config) . '</script>';
 
         return $html;
     }

TwoFactorAuth/Block/ChangeProvider.php

@@ -7,10 +7,9 @@
 
 namespace Magento\TwoFactorAuth\Block;
 
+use Magento\Authorization\Model\UserContextInterface;
 use Magento\Backend\Block\Template;
 use Magento\Backend\Model\Auth\Session;
-use Magento\TwoFactorAuth\Api\TfaSessionInterface;
-use Magento\User\Model\User;
 use Magento\TwoFactorAuth\Api\TfaInterface;
 use Magento\TwoFactorAuth\Api\ProviderInterface;
 
@@ -24,6 +23,11 @@ class ChangeProvider extends Template
      */
     private $tfa;
 
+    /**
+     * @var UserContextInterface
+     */
+    private $userContext;
+
     /**
      * @var Session
      */
@@ -33,27 +37,29 @@ class ChangeProvider extends Template
      * ChangeProvider constructor.
      * @param Template\Context $context
      * @param Session $session
+     * @param UserContextInterface $userContext
      * @param TfaInterface $tfa
      * @param array $data
      */
     public function __construct(
         Template\Context $context,
         Session $session,
+        UserContextInterface $userContext,
         TfaInterface $tfa,
         array $data = []
     ) {
         parent::__construct($context, $data);
         $this->tfa = $tfa;
         $this->session = $session;
+        $this->userContext = $userContext;
     }
 
     /**
      * @inheritDoc
      */
     protected function _toHtml()
     {
-        $userId = (int)$this->session->getUser()->getId();
-        $toActivate = $this->tfa->getProvidersToActivate($userId);
+        $toActivate = $this->tfa->getProvidersToActivate($this->userContext->getUserId());
 
         foreach ($toActivate as $toActivateProvider) {
             if ($toActivateProvider->getCode() === $this->getData('provider')) {
@@ -71,7 +77,7 @@ public function getJsLayout()
     {
         $providers = [];
         foreach ($this->getProvidersList() as $provider) {
-            if (!$provider->isActive((int)$this->session->getUser()->getId())) {
+            if (!$provider->isActive($this->userContext->getUserId())) {
                 continue;
             }
             $providers[] = [
@@ -89,15 +95,6 @@ public function getJsLayout()
         return parent::getJsLayout();
     }
 
-    /**
-     * Get user
-     * @return User|null
-     */
-    private function getUser(): ?User
-    {
-        return $this->session->getUser();
-    }
-
     /**
      * Get a list of available providers
      * @return ProviderInterface[]
@@ -106,7 +103,7 @@ private function getProvidersList(): array
     {
         $res = [];
 
-        $providers = $this->tfa->getUserProviders((int) $this->getUser()->getId());
+        $providers = $this->tfa->getUserProviders((int) $this->userContext->getUserId());
         foreach ($providers as $provider) {
             if ($provider->getCode() !== $this->getData('provider')) {
                 $res[] = $provider;

TwoFactorAuth/Block/ConfigureLater.php

@@ -7,14 +7,13 @@
 
 namespace Magento\TwoFactorAuth\Block;
 
+use Magento\Authorization\Model\UserContextInterface;
 use Magento\Backend\Block\Template;
 use Magento\Backend\Block\Template\Context;
 use Magento\Backend\Model\Auth\Session;
 use Magento\Framework\Data\Form\FormKey;
 use Magento\Framework\Serialize\SerializerInterface;
-use Magento\User\Model\User;
 use Magento\TwoFactorAuth\Api\TfaInterface;
-use Magento\TwoFactorAuth\Api\ProviderInterface;
 
 /**
  * @api
@@ -36,13 +35,19 @@ class ConfigureLater extends Template
      */
     private $serializer;
 
+    /**
+     * @var UserContextInterface
+     */
+    private $userContext;
+
     /**
      * ChangeProvider constructor.
      * @param Context $context
      * @param Session $session
      * @param TfaInterface $tfa
      * @param SerializerInterface $serializer
      * @param FormKey $formKey
+     * @param UserContextInterface $userContext
      * @param array $data
      */
     public function __construct(
@@ -51,21 +56,23 @@ public function __construct(
         TfaInterface $tfa,
         SerializerInterface $serializer,
         FormKey $formKey,
+        UserContextInterface $userContext,
         array $data = []
     ) {
         parent::__construct($context, $data);
         $this->tfa = $tfa;
         $this->session = $session;
         $this->serializer = $serializer;
         $this->formKey = $formKey;
+        $this->userContext = $userContext;
     }
 
     /**
      * @inheritDoc
      */
     protected function _toHtml()
     {
-        $userId = (int)$this->session->getUser()->getId();
+        $userId = $this->userContext->getUserId();
         $providers = $this->tfa->getUserProviders($userId);
         $toActivate = $this->tfa->getProvidersToActivate($userId);
 

TwoFactorAuth/Block/Provider/U2fKey/Auth.php

@@ -10,6 +10,7 @@
 use Magento\Backend\Block\Template;
 use Magento\Backend\Model\Auth\Session;
 use Magento\TwoFactorAuth\Model\Provider\Engine\U2fKey;
+use Magento\TwoFactorAuth\Model\Provider\Engine\U2fKey\Session as U2fSession;
 
 /**
  * @api
@@ -22,29 +23,32 @@ class Auth extends Template
     private $u2fKey;
 
     /**
-     * @var Session
+     * @var U2fSession
      */
-    private $session;
+    private $u2fSession;
 
     /**
-     * @var array
+     * @var Session
      */
-    private $authenticateData;
+    private $session;
 
     /**
      * @param Template\Context $context
-     * @param Session $session
+     * @param U2fSession $u2fSession
      * @param U2fKey $u2fKey
+     * @param Session $session
      * @param array $data
      */
     public function __construct(
         Template\Context $context,
-        Session $session,
+        U2fSession $u2fSession,
         U2fKey $u2fKey,
+        Session $session,
         array $data = []
     ) {
         parent::__construct($context, $data);
         $this->u2fKey = $u2fKey;
+        $this->u2fSession = $u2fSession;
         $this->session = $session;
     }
 
@@ -74,7 +78,7 @@ public function getJsLayout()
     public function generateAuthenticateData(): array
     {
         $authenticateData = $this->u2fKey->getAuthenticateData($this->session->getUser());
-        $this->session->setTfaU2fChallenge($authenticateData['credentialRequestOptions']['challenge']);
+        $this->u2fSession->setU2fChallenge($authenticateData['credentialRequestOptions']['challenge']);
 
         return $authenticateData;
     }

TwoFactorAuth/Block/Provider/U2fKey/Configure.php

@@ -10,6 +10,7 @@
 use Magento\Backend\Block\Template;
 use Magento\Backend\Model\Auth\Session;
 use Magento\TwoFactorAuth\Model\Provider\Engine\U2fKey;
+use Magento\TwoFactorAuth\Model\Provider\Engine\U2fKey\Session as U2fSession;
 
 /**
  * @api
@@ -21,6 +22,11 @@ class Configure extends Template
      */
     private $u2fKey;
 
+    /**
+     * @var U2fSession
+     */
+    private $u2fSession;
+
     /**
      * @var Session
      */
@@ -29,18 +35,21 @@ class Configure extends Template
     /**
      * @param Template\Context $context
      * @param U2fKey $u2fKey
+     * @param U2fSession $u2fSession
      * @param Session $session
      * @param array $data
      */
     public function __construct(
         Template\Context $context,
         U2fKey $u2fKey,
+        U2fSession $u2fSession,
         Session $session,
         array $data = []
     ) {
 
         parent::__construct($context, $data);
         $this->u2fKey = $u2fKey;
+        $this->u2fSession = $u2fSession;
         $this->session = $session;
     }
 
@@ -63,10 +72,15 @@ public function getJsLayout()
         return parent::getJsLayout();
     }
 
+    /**
+     * Get the data required to issue a WebAuthn request
+     *
+     * @return array
+     */
     public function getRegisterData(): array
     {
         $registerData = $this->u2fKey->getRegisterData($this->session->getUser());
-        $this->session->setTfaU2fChallenge($registerData['publicKey']['challenge']);
+        $this->u2fSession->setU2fChallenge($registerData['publicKey']['challenge']);
 
         return $registerData;
     }