Merge pull request #466 from magento-extensibility/MAGETWO-50209-get-rid-of-helpers

[Extensibility] MAGETWO-50209: Get rid of Helpers in Password Security Management

Author: Melnikov, Igor(imelnikov)

app/bootstrap.php

@@ -9,7 +9,6 @@
  */
 error_reporting(E_ALL);
 #ini_set('display_errors', 1);
-umask(0);
 
 /* PHP version validation */
 if (version_compare(phpversion(), '5.5.0', '<') === true) {

app/code/Magento/Backend/Controller/Adminhtml/System/Account/Save.php

@@ -10,24 +10,28 @@
 use Magento\Framework\Exception\LocalizedException;
 use Magento\Framework\Controller\ResultFactory;
 use Magento\Framework\Exception\State\UserLockedException;
+use Magento\Security\Model\SecurityCookie;
 
 class Save extends \Magento\Backend\Controller\Adminhtml\System\Account
 {
     /**
-     * @var \Magento\Security\Helper\SecurityCookie
+     * @var SecurityCookie
      */
-    protected $securityCookieHelper;
+    private $securityCookie;
 
     /**
-     * @param \Magento\Backend\App\Action\Context $context
-     * @param \Magento\Security\Helper\SecurityCookie $securityCookieHelper
+     * Get security cookie
+     *
+     * @return SecurityCookie
+     * @deprecated
      */
-    public function __construct(
-        \Magento\Backend\App\Action\Context $context,
-        \Magento\Security\Helper\SecurityCookie $securityCookieHelper
-    ) {
-        parent::__construct($context);
-        $this->securityCookieHelper = $securityCookieHelper;
+    private function getSecurityCookie()
+    {
+        if (!($this->securityCookie instanceof SecurityCookie)) {
+            return \Magento\Framework\App\ObjectManager::getInstance()->get(SecurityCookie::class);
+        } else {
+            return $this->securityCookie;
+        }
     }
 
     /**
@@ -79,7 +83,7 @@ public function execute()
             }
         } catch (UserLockedException $e) {
             $this->_auth->logout();
-            $this->securityCookieHelper->setLogoutReasonCookie(
+            $this->getSecurityCookie()->setLogoutReasonCookie(
                 \Magento\Security\Model\AdminSessionsManager::LOGOUT_REASON_USER_LOCKED
             );
         } catch (ValidatorException $e) {

app/code/Magento/Backend/i18n/en_US.csv

@@ -206,6 +206,8 @@ YTD,YTD
 "Authentication storage is incorrect.","Authentication storage is incorrect."
 "You did not sign in correctly or your account is temporarily disabled.","You did not sign in correctly or your account is temporarily disabled."
 "Authentication error occurred.","Authentication error occurred."
+"Admin session lifetime must be less than or equal to 31536000 seconds (one year)","Admin session lifetime must be less than or equal to 31536000 seconds (one year)"
+"Admin session lifetime must be greater than or equal to 60 seconds","Admin session lifetime must be greater than or equal to 60 seconds"
 Order,Order
 "Order #%1","Order #%1"
 "Access denied","Access denied"
@@ -391,7 +393,7 @@ Security,Security
 "Add Secret Key to URLs","Add Secret Key to URLs"
 "Login is Case Sensitive","Login is Case Sensitive"
 "Admin Session Lifetime (seconds)","Admin Session Lifetime (seconds)"
-"Values less than 60 are ignored.","Values less than 60 are ignored."
+"Please enter at least 60 and at most 31536000 (one year).","Please enter at least 60 and at most 31536000 (one year)."
 "Enable Charts","Enable Charts"
 Web,Web
 "Url Options","Url Options"

app/code/Magento/Backup/Model/Fs/Collection.php

@@ -91,7 +91,6 @@ protected function _hideBackupsForApache()
         $filename = '.htaccess';
         if (!$this->_varDirectory->isFile($filename)) {
             $this->_varDirectory->writeFile($filename, 'deny from all');
-            $this->_varDirectory->changePermissions($filename, 0640);
         }
     }
 

app/code/Magento/Captcha/Helper/Data.php

@@ -150,8 +150,6 @@ public function getImgDir($website = null)
         $mediaDir = $this->_filesystem->getDirectoryWrite(DirectoryList::MEDIA);
         $captchaDir = '/captcha/' . $this->_getWebsiteCode($website);
         $mediaDir->create($captchaDir);
-        $mediaDir->changePermissions($captchaDir, DriverInterface::WRITEABLE_DIRECTORY_MODE);
-
         return $mediaDir->getAbsolutePath($captchaDir) . '/';
     }
 

app/code/Magento/Captcha/Observer/CheckUserEditObserver.php

@@ -5,12 +5,10 @@
  */
 namespace Magento\Captcha\Observer;
 
+use Magento\Customer\Model\AuthenticationInterface;
 use Magento\Framework\Event\ObserverInterface;
-use Magento\Framework\Exception\NoSuchEntityException;
-use Magento\Customer\Helper\AccountManagement as AccountManagementHelper;
 use Magento\Customer\Model\Session;
 use Magento\Framework\App\Config\ScopeConfigInterface;
-use Magento\Customer\Api\CustomerRepositoryInterface;
 
 /**
  * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
@@ -48,58 +46,50 @@ class CheckUserEditObserver implements ObserverInterface
     protected $captchaStringResolver;
 
     /**
-     * Account manager
+     * Authentication
      *
-     * @var AccountManagementHelper
+     * @var AuthenticationInterface
      */
-    protected $accountManagementHelper;
+    protected $authentication;
 
     /**
      * @var Session
      */
-    protected $session;
+    protected $customerSession;
 
     /**
      * @var ScopeConfigInterface
      */
     protected $scopeConfig;
 
-    /**
-     * @var CustomerRepositoryInterface
-     */
-    protected $customerRepository;
-
     /**
      * @param \Magento\Captcha\Helper\Data $helper
      * @param \Magento\Framework\App\ActionFlag $actionFlag
      * @param \Magento\Framework\Message\ManagerInterface $messageManager
      * @param \Magento\Framework\App\Response\RedirectInterface $redirect
      * @param CaptchaStringResolver $captchaStringResolver
-     * @param AccountManagementHelper $accountManagementHelper
+     * @param AuthenticationInterface $authentication
      * @param Session $customerSession
      * @param ScopeConfigInterface $scopeConfig
-     * @param CustomerRepositoryInterface $customerRepository
      */
     public function __construct(
         \Magento\Captcha\Helper\Data $helper,
         \Magento\Framework\App\ActionFlag $actionFlag,
         \Magento\Framework\Message\ManagerInterface $messageManager,
         \Magento\Framework\App\Response\RedirectInterface $redirect,
         CaptchaStringResolver $captchaStringResolver,
-        AccountManagementHelper $accountManagementHelper,
+        AuthenticationInterface $authentication,
         Session $customerSession,
-        ScopeConfigInterface $scopeConfig,
-        CustomerRepositoryInterface $customerRepository
+        ScopeConfigInterface $scopeConfig
     ) {
         $this->helper = $helper;
         $this->actionFlag = $actionFlag;
         $this->messageManager = $messageManager;
         $this->redirect = $redirect;
         $this->captchaStringResolver = $captchaStringResolver;
-        $this->accountManagementHelper = $accountManagementHelper;
+        $this->authentication = $authentication;
         $this->customerSession = $customerSession;
         $this->scopeConfig = $scopeConfig;
-        $this->customerRepository = $customerRepository;
     }
 
     /**
@@ -120,14 +110,17 @@ public function execute(\Magento\Framework\Event\Observer $observer)
                     self::FORM_ID
                 )
             )) {
-                try {
-                    $customer = $this->customerRepository->getById($this->customerSession->getCustomerId());
-                    $this->accountManagementHelper->processCustomerLockoutData($customer->getId());
-                    $this->customerRepository->save($customer);
-                } catch (NoSuchEntityException $e) {
-                    //do nothing as customer existance is validated later in authenticate method
+                $customerId = $this->customerSession->getCustomerId();
+                $this->authentication->processAuthenticationFailure($customerId);
+                if ($this->authentication->isLocked($customerId)) {
+                    $this->customerSession->logout();
+                    $this->customerSession->start();
+                    $message = __(
+                        'The account is locked. Please wait and try again or contact %1.',
+                        $this->scopeConfig->getValue('contact/email/recipient_email')
+                    );
+                    $this->messageManager->addError($message);
                 }
-                $this->workWithLock();
                 $this->messageManager->addError(__('Incorrect CAPTCHA'));
                 $this->actionFlag->set('', \Magento\Framework\App\Action\Action::FLAG_NO_DISPATCH, true);
                 $this->redirect->redirect($controller->getResponse(), '*/*/edit');
@@ -140,24 +133,4 @@ public function execute(\Magento\Framework\Event\Observer $observer)
 
         return $this;
     }
-
-    /**
-     * Logout a user if it is locked
-     *
-     * @throws \Magento\Framework\Exception\SessionException
-     * @return void
-     */
-    protected function workWithLock()
-    {
-        $customerModel = $this->customerSession->getCustomer();
-        if ($customerModel->isCustomerLocked()) {
-            $this->customerSession->logout();
-            $this->customerSession->start();
-            $message = __(
-                'The account is locked. Please wait and try again or contact %1.',
-                $this->scopeConfig->getValue('contact/email/recipient_email')
-            );
-            $this->messageManager->addError($message);
-        }
-    }
 }

app/code/Magento/Captcha/Observer/CheckUserLoginObserver.php

@@ -5,9 +5,9 @@
  */
 namespace Magento\Captcha\Observer;
 
+use Magento\Customer\Model\AuthenticationInterface;
 use Magento\Framework\Event\ObserverInterface;
 use Magento\Framework\Exception\NoSuchEntityException;
-use Magento\Customer\Helper\AccountManagement as AccountManagementHelper;
 use Magento\Customer\Api\CustomerRepositoryInterface;
 
 /**
@@ -53,11 +53,11 @@ class CheckUserLoginObserver implements ObserverInterface
     protected $customerRepository;
 
     /**
-     * Account manager
+     * Authentication
      *
-     * @var AccountManagementHelper
+     * @var AuthenticationInterface
      */
-    protected $accountManagementHelper;
+    protected $authentication;
 
     /**
      * @param \Magento\Captcha\Helper\Data $helper
@@ -83,25 +83,10 @@ public function __construct(
         $this->_customerUrl = $customerUrl;
     }
 
-    /**
-     * Set email notification
-     *
-     * @param \Magento\Customer\Api\CustomerRepositoryInterface $customerRepository
-     * @return void
-     * @deprecated
-     * @SuppressWarnings(PHPMD.UnusedPrivateMethod)
-     */
-    private function setCustomerRepository(\Magento\Customer\Api\CustomerRepositoryInterface $customerRepository)
-    {
-
-        $this->customerRepository = $customerRepository;
-    }
-
     /**
      * Get customer repository
      *
      * @return \Magento\Customer\Api\CustomerRepositoryInterface
-     * @deprecated
      */
     private function getCustomerRepository()
     {
@@ -116,39 +101,24 @@ private function getCustomerRepository()
     }
 
     /**
-     * Set account management helper
-     *
-     * @param AccountManagementHelper $accountManagementHelper
-     * @return void
-     * @deprecated
-     * @SuppressWarnings(PHPMD.UnusedPrivateMethod)
-     */
-    private function setAccountManagementHelper(AccountManagementHelper $accountManagementHelper)
-    {
-
-        $this->accountManagementHelper = $accountManagementHelper;
-    }
-
-    /**
-     * Get account management helper
+     * Get authentication
      *
-     * @return AccountManagementHelper
-     * @deprecated
+     * @return AuthenticationInterface
      */
-    private function getAccountManagementHelper()
+    private function getAuthentication()
     {
 
-        if (!($this->accountManagementHelper instanceof \Magento\Customer\Helper\AccountManagement)) {
+        if (!($this->authentication instanceof AuthenticationInterface)) {
             return \Magento\Framework\App\ObjectManager::getInstance()->get(
-                'Magento\Customer\Helper\AccountManagement'
+                AuthenticationInterface::class
             );
         } else {
-            return $this->accountManagementHelper;
+            return $this->authentication;
         }
     }
 
     /**
-     * Check Captcha On User Login Page
+     * Check captcha on user login page
      *
      * @param \Magento\Framework\Event\Observer $observer
      * @throws NoSuchEntityException
@@ -168,8 +138,7 @@ public function execute(\Magento\Framework\Event\Observer $observer)
             if (!$captchaModel->isCorrect($word)) {
                 try {
                     $customer = $this->getCustomerRepository()->get($login);
-                    $this->getAccountManagementHelper()->processCustomerLockoutData($customer->getId());
-                    $this->getCustomerRepository()->save($customer);
+                    $this->getAuthentication()->processAuthenticationFailure($customer->getId());
                 } catch (NoSuchEntityException $e) {
                     //do nothing as customer existance is validated later in authenticate method
                 }