From 4e11099e6163b5be250725c4727f85c0bba4847b Mon Sep 17 00:00:00 2001 From: Maksym Novik Date: Thu, 16 May 2019 11:33:28 +0300 Subject: [PATCH] Creating Customer without password is directly confirmed #14492 --- .../Controller/Account/CreatePassword.php | 33 +++++-- .../Customer/Model/AccountManagement.php | 99 +++++++++---------- .../ConfirmCustomerByToken.php | 58 +++++++++++ .../GetCustomerByToken.php | 89 +++++++++++++++++ .../Customer/Model/ResourceModel/Customer.php | 13 +-- .../Customer/Controller/AccountTest.php | 29 +++++- 6 files changed, 247 insertions(+), 74 deletions(-) create mode 100644 app/code/Magento/Customer/Model/ForgotPasswordToken/ConfirmCustomerByToken.php create mode 100644 app/code/Magento/Customer/Model/ForgotPasswordToken/GetCustomerByToken.php diff --git a/app/code/Magento/Customer/Controller/Account/CreatePassword.php b/app/code/Magento/Customer/Controller/Account/CreatePassword.php index 124ac912a7ccf..d12ec57d3c339 100644 --- a/app/code/Magento/Customer/Controller/Account/CreatePassword.php +++ b/app/code/Magento/Customer/Controller/Account/CreatePassword.php @@ -3,13 +3,17 @@ * Copyright © Magento, Inc. All rights reserved. * See COPYING.txt for license details. */ +declare(strict_types=1); + namespace Magento\Customer\Controller\Account; use Magento\Customer\Api\AccountManagementInterface; +use Magento\Customer\Model\ForgotPasswordToken\ConfirmCustomerByToken; use Magento\Customer\Model\Session; use Magento\Framework\App\Action\HttpGetActionInterface; use Magento\Framework\View\Result\PageFactory; use Magento\Framework\App\Action\Context; +use Magento\Framework\App\ObjectManager; /** * Class CreatePassword @@ -34,20 +38,30 @@ class CreatePassword extends \Magento\Customer\Controller\AbstractAccount implem protected $resultPageFactory; /** - * @param Context $context - * @param Session $customerSession - * @param PageFactory $resultPageFactory - * @param AccountManagementInterface $accountManagement + * @var \Magento\Customer\Model\ForgotPasswordToken\ConfirmCustomerByToken + */ + private $confirmByToken; + + /** + * @param \Magento\Framework\App\Action\Context $context + * @param \Magento\Customer\Model\Session $customerSession + * @param \Magento\Framework\View\Result\PageFactory $resultPageFactory + * @param \Magento\Customer\Api\AccountManagementInterface $accountManagement + * @param \Magento\Customer\Model\ForgotPasswordToken\ConfirmCustomerByToken $confirmByToken */ public function __construct( Context $context, Session $customerSession, PageFactory $resultPageFactory, - AccountManagementInterface $accountManagement + AccountManagementInterface $accountManagement, + ConfirmCustomerByToken $confirmByToken = null ) { $this->session = $customerSession; $this->resultPageFactory = $resultPageFactory; $this->accountManagement = $accountManagement; + $this->confirmByToken = $confirmByToken + ?? ObjectManager::getInstance()->get(ConfirmCustomerByToken::class); + parent::__construct($context); } @@ -67,6 +81,8 @@ public function execute() try { $this->accountManagement->validateResetPasswordLinkToken(null, $resetPasswordToken); + $this->confirmByToken->execute($resetPasswordToken); + if ($isDirectLink) { $this->session->setRpToken($resetPasswordToken); $resultRedirect = $this->resultRedirectFactory->create(); @@ -77,16 +93,17 @@ public function execute() /** @var \Magento\Framework\View\Result\Page $resultPage */ $resultPage = $this->resultPageFactory->create(); $resultPage->getLayout() - ->getBlock('resetPassword') - ->setResetPasswordLinkToken($resetPasswordToken); + ->getBlock('resetPassword') + ->setResetPasswordLinkToken($resetPasswordToken); return $resultPage; } } catch (\Exception $exception) { - $this->messageManager->addError(__('Your password reset link has expired.')); + $this->messageManager->addErrorMessage(__('Your password reset link has expired.')); /** @var \Magento\Framework\Controller\Result\Redirect $resultRedirect */ $resultRedirect = $this->resultRedirectFactory->create(); $resultRedirect->setPath('*/*/forgotpassword'); + return $resultRedirect; } } diff --git a/app/code/Magento/Customer/Model/AccountManagement.php b/app/code/Magento/Customer/Model/AccountManagement.php index 8439a3f2308c2..393a342420246 100644 --- a/app/code/Magento/Customer/Model/AccountManagement.php +++ b/app/code/Magento/Customer/Model/AccountManagement.php @@ -17,6 +17,7 @@ use Magento\Customer\Model\Config\Share as ConfigShare; use Magento\Customer\Model\Customer as CustomerModel; use Magento\Customer\Model\Customer\CredentialsValidator; +use Magento\Customer\Model\ForgotPasswordToken\GetCustomerByToken; use Magento\Customer\Model\Metadata\Validator; use Magento\Customer\Model\ResourceModel\Visitor\CollectionFactory; use Magento\Directory\Model\AllowedCountries; @@ -44,7 +45,6 @@ use Magento\Framework\Intl\DateTimeFactory; use Magento\Framework\Mail\Template\TransportBuilder; use Magento\Framework\Math\Random; -use Magento\Framework\Phrase; use Magento\Framework\Reflection\DataObjectProcessor; use Magento\Framework\Registry; use Magento\Framework\Session\SaveHandlerInterface; @@ -345,6 +345,11 @@ class AccountManagement implements AccountManagementInterface */ private $allowedCountriesReader; + /** + * @var GetCustomerByToken + */ + private $getByToken; + /** * @param CustomerFactory $customerFactory * @param ManagerInterface $eventManager @@ -377,10 +382,12 @@ class AccountManagement implements AccountManagementInterface * @param CollectionFactory|null $visitorCollectionFactory * @param SearchCriteriaBuilder|null $searchCriteriaBuilder * @param AddressRegistry|null $addressRegistry + * @param GetCustomerByToken|null $getByToken * @param AllowedCountries|null $allowedCountriesReader + * @SuppressWarnings(PHPMD.CyclomaticComplexity) * @SuppressWarnings(PHPMD.ExcessiveParameterList) * @SuppressWarnings(PHPMD.NPathComplexity) - * @SuppressWarnings(PHPMD.CyclomaticComplexity) + * @SuppressWarnings(PHPMD.LongVariable) */ public function __construct( CustomerFactory $customerFactory, @@ -414,6 +421,7 @@ public function __construct( CollectionFactory $visitorCollectionFactory = null, SearchCriteriaBuilder $searchCriteriaBuilder = null, AddressRegistry $addressRegistry = null, + GetCustomerByToken $getByToken = null, AllowedCountries $allowedCountriesReader = null ) { $this->customerFactory = $customerFactory; @@ -439,23 +447,26 @@ public function __construct( $this->customerModel = $customerModel; $this->objectFactory = $objectFactory; $this->extensibleDataObjectConverter = $extensibleDataObjectConverter; + $objectManager = ObjectManager::getInstance(); $this->credentialsValidator = - $credentialsValidator ?: ObjectManager::getInstance()->get(CredentialsValidator::class); - $this->dateTimeFactory = $dateTimeFactory ?: ObjectManager::getInstance()->get(DateTimeFactory::class); - $this->accountConfirmation = $accountConfirmation ?: ObjectManager::getInstance() + $credentialsValidator ?: $objectManager->get(CredentialsValidator::class); + $this->dateTimeFactory = $dateTimeFactory ?: $objectManager->get(DateTimeFactory::class); + $this->accountConfirmation = $accountConfirmation ?: $objectManager ->get(AccountConfirmation::class); $this->sessionManager = $sessionManager - ?: ObjectManager::getInstance()->get(SessionManagerInterface::class); + ?: $objectManager->get(SessionManagerInterface::class); $this->saveHandler = $saveHandler - ?: ObjectManager::getInstance()->get(SaveHandlerInterface::class); + ?: $objectManager->get(SaveHandlerInterface::class); $this->visitorCollectionFactory = $visitorCollectionFactory - ?: ObjectManager::getInstance()->get(CollectionFactory::class); + ?: $objectManager->get(CollectionFactory::class); $this->searchCriteriaBuilder = $searchCriteriaBuilder - ?: ObjectManager::getInstance()->get(SearchCriteriaBuilder::class); + ?: $objectManager->get(SearchCriteriaBuilder::class); $this->addressRegistry = $addressRegistry - ?: ObjectManager::getInstance()->get(AddressRegistry::class); + ?: $objectManager->get(AddressRegistry::class); + $this->getByToken = $getByToken + ?: $objectManager->get(GetCustomerByToken::class); $this->allowedCountriesReader = $allowedCountriesReader - ?: ObjectManager::getInstance()->get(AllowedCountries::class); + ?: $objectManager->get(AllowedCountries::class); } /** @@ -521,8 +532,11 @@ public function activateById($customerId, $confirmationKey) * @param \Magento\Customer\Api\Data\CustomerInterface $customer * @param string $confirmationKey * @return \Magento\Customer\Api\Data\CustomerInterface - * @throws \Magento\Framework\Exception\State\InvalidTransitionException - * @throws \Magento\Framework\Exception\State\InputMismatchException + * @throws InputException + * @throws InputMismatchException + * @throws InvalidTransitionException + * @throws LocalizedException + * @throws NoSuchEntityException */ private function activateCustomer($customer, $confirmationKey) { @@ -630,42 +644,6 @@ public function initiatePasswordReset($email, $template, $websiteId = null) return false; } - /** - * Match a customer by their RP token. - * - * @param string $rpToken - * @throws ExpiredException - * @throws NoSuchEntityException - * @return CustomerInterface - * @throws LocalizedException - */ - private function matchCustomerByRpToken(string $rpToken): CustomerInterface - { - $this->searchCriteriaBuilder->addFilter( - 'rp_token', - $rpToken - ); - $this->searchCriteriaBuilder->setPageSize(1); - $found = $this->customerRepository->getList( - $this->searchCriteriaBuilder->create() - ); - if ($found->getTotalCount() > 1) { - //Failed to generated unique RP token - throw new ExpiredException( - new Phrase('Reset password token expired.') - ); - } - if ($found->getTotalCount() === 0) { - //Customer with such token not found. - throw NoSuchEntityException::singleField( - 'rp_token', - $rpToken - ); - } - //Unique customer found. - return $found->getItems()[0]; - } - /** * Handle not supported template * @@ -691,7 +669,7 @@ private function handleUnknownTemplate($template) public function resetPassword($email, $resetToken, $newPassword) { if (!$email) { - $customer = $this->matchCustomerByRpToken($resetToken); + $customer = $this->getByToken->execute($resetToken); $email = $customer->getEmail(); } else { $customer = $this->customerRepository->get($email); @@ -830,6 +808,8 @@ public function getConfirmationStatus($customerId) /** * @inheritdoc + * + * @throws LocalizedException */ public function createAccount(CustomerInterface $customer, $password = null, $redirectUrl = '') { @@ -852,6 +832,8 @@ public function createAccount(CustomerInterface $customer, $password = null, $re /** * @inheritdoc + * + * @throws InputMismatchException * @SuppressWarnings(PHPMD.CyclomaticComplexity) * @SuppressWarnings(PHPMD.NPathComplexity) */ @@ -987,6 +969,8 @@ protected function sendEmailConfirmation(CustomerInterface $customer, $redirectU /** * @inheritdoc + * + * @throws InvalidEmailOrPasswordException */ public function changePassword($email, $currentPassword, $newPassword) { @@ -1000,6 +984,8 @@ public function changePassword($email, $currentPassword, $newPassword) /** * @inheritdoc + * + * @throws InvalidEmailOrPasswordException */ public function changePasswordById($customerId, $currentPassword, $newPassword) { @@ -1137,12 +1123,14 @@ public function isCustomerInStore($customerWebsiteId, $storeId) * * @param int $customerId * @param string $resetPasswordLinkToken + * * @return bool - * @throws \Magento\Framework\Exception\State\InputMismatchException If token is mismatched - * @throws \Magento\Framework\Exception\State\ExpiredException If token is expired - * @throws \Magento\Framework\Exception\InputException If token or customer id is invalid - * @throws \Magento\Framework\Exception\NoSuchEntityException If customer doesn't exist + * @throws ExpiredException If token is expired + * @throws InputException If token or customer id is invalid + * @throws InputMismatchException If token is mismatched * @throws LocalizedException + * @throws NoSuchEntityException If customer doesn't exist + * @SuppressWarnings(PHPMD.LongVariable) */ private function validateResetPasswordToken($customerId, $resetPasswordLinkToken) { @@ -1157,7 +1145,8 @@ private function validateResetPasswordToken($customerId, $resetPasswordLinkToken if ($customerId === null) { //Looking for the customer. - $customerId = $this->matchCustomerByRpToken($resetPasswordLinkToken) + $customerId = $this->getByToken + ->execute($resetPasswordLinkToken) ->getId(); } if (!is_string($resetPasswordLinkToken) || empty($resetPasswordLinkToken)) { diff --git a/app/code/Magento/Customer/Model/ForgotPasswordToken/ConfirmCustomerByToken.php b/app/code/Magento/Customer/Model/ForgotPasswordToken/ConfirmCustomerByToken.php new file mode 100644 index 0000000000000..6aadc814a4b9b --- /dev/null +++ b/app/code/Magento/Customer/Model/ForgotPasswordToken/ConfirmCustomerByToken.php @@ -0,0 +1,58 @@ +getByToken = $getByToken; + $this->customerRepository = $customerRepository; + } + + /** + * Confirm customer account my rp_token + * + * @param string $resetPasswordToken + * + * @return void + * @throws \Magento\Framework\Exception\LocalizedException + */ + public function execute(string $resetPasswordToken): void + { + $customer = $this->getByToken->execute($resetPasswordToken); + if ($customer->getConfirmation()) { + $this->customerRepository->save( + $customer->setConfirmation(null) + ); + } + } +} diff --git a/app/code/Magento/Customer/Model/ForgotPasswordToken/GetCustomerByToken.php b/app/code/Magento/Customer/Model/ForgotPasswordToken/GetCustomerByToken.php new file mode 100644 index 0000000000000..09af4e296bd92 --- /dev/null +++ b/app/code/Magento/Customer/Model/ForgotPasswordToken/GetCustomerByToken.php @@ -0,0 +1,89 @@ +customerRepository = $customerRepository; + $this->searchCriteriaBuilder = $searchCriteriaBuilder; + } + + /** + * Get customer by rp_token + * + * @param string $resetPasswordToken + * + * @return \Magento\Customer\Api\Data\CustomerInterface + * @throws ExpiredException + * @throws NoSuchEntityException + * @throws \Magento\Framework\Exception\LocalizedException + */ + public function execute(string $resetPasswordToken):CustomerInterface + { + $this->searchCriteriaBuilder->addFilter( + 'rp_token', + $resetPasswordToken + ); + $this->searchCriteriaBuilder->setPageSize(1); + $found = $this->customerRepository->getList( + $this->searchCriteriaBuilder->create() + ); + + if ($found->getTotalCount() > 1) { + //Failed to generated unique RP token + throw new ExpiredException( + new Phrase('Reset password token expired.') + ); + } + if ($found->getTotalCount() === 0) { + //Customer with such token not found. + new NoSuchEntityException( + new Phrase( + 'No such entity with rp_token = %value', + [ + 'value' => $resetPasswordToken + ] + ) + ); + } + + //Unique customer found. + return $found->getItems()[0]; + } +} diff --git a/app/code/Magento/Customer/Model/ResourceModel/Customer.php b/app/code/Magento/Customer/Model/ResourceModel/Customer.php index 2eb1ef897e70e..94196df6fe093 100644 --- a/app/code/Magento/Customer/Model/ResourceModel/Customer.php +++ b/app/code/Magento/Customer/Model/ResourceModel/Customer.php @@ -95,9 +95,12 @@ protected function _getDefaultAttributes() /** * Check customer scope, email and confirmation key before saving * - * @param \Magento\Framework\DataObject $customer + * @param \Magento\Framework\DataObject|\Magento\Customer\Api\Data\CustomerInterface $customer + * * @return $this - * @throws \Magento\Framework\Exception\LocalizedException + * @throws AlreadyExistsException + * @throws ValidatorException + * @throws \Magento\Framework\Exception\NoSuchEntityException * @SuppressWarnings(PHPMD.CyclomaticComplexity) * @SuppressWarnings(PHPMD.NPathComplexity) */ @@ -141,9 +144,7 @@ protected function _beforeSave(\Magento\Framework\DataObject $customer) } // set confirmation key logic - if ($customer->getForceConfirmed() || $customer->getPasswordHash() == '') { - $customer->setConfirmation(null); - } elseif (!$customer->getId() && $customer->isConfirmationRequired()) { + if (!$customer->getId() && $customer->isConfirmationRequired()) { $customer->setConfirmation($customer->getRandomConfirmationKey()); } // remove customer confirmation key from database, if empty @@ -163,7 +164,7 @@ protected function _beforeSave(\Magento\Framework\DataObject $customer) * * @param \Magento\Customer\Model\Customer $customer * @return void - * @throws \Magento\Framework\Validator\Exception + * @throws ValidatorException */ protected function _validate($customer) { diff --git a/dev/tests/integration/testsuite/Magento/Customer/Controller/AccountTest.php b/dev/tests/integration/testsuite/Magento/Customer/Controller/AccountTest.php index 10b632c002475..9377c872517ce 100644 --- a/dev/tests/integration/testsuite/Magento/Customer/Controller/AccountTest.php +++ b/dev/tests/integration/testsuite/Magento/Customer/Controller/AccountTest.php @@ -15,17 +15,17 @@ use Magento\Framework\App\Config\ScopeConfigInterface; use Magento\Framework\App\Config\Value; use Magento\Framework\App\Http; +use Magento\Framework\App\Request\Http as HttpRequest; use Magento\Framework\Data\Form\FormKey; use Magento\Framework\Message\MessageInterface; +use Magento\Framework\Serialize\Serializer\Json; +use Magento\Framework\Stdlib\CookieManagerInterface; use Magento\TestFramework\Helper\Bootstrap; +use Magento\TestFramework\Mail\Template\TransportBuilderMock; use Magento\TestFramework\Request; use Magento\TestFramework\Response; -use Zend\Stdlib\Parameters; -use Magento\Framework\App\Request\Http as HttpRequest; -use Magento\TestFramework\Mail\Template\TransportBuilderMock; -use Magento\Framework\Serialize\Serializer\Json; -use Magento\Framework\Stdlib\CookieManagerInterface; use Magento\Theme\Controller\Result\MessagePlugin; +use Zend\Stdlib\Parameters; /** * @SuppressWarnings(PHPMD.CouplingBetweenObjects) @@ -169,6 +169,7 @@ public function testCreatepasswordActionWithDirectLink() $token = Bootstrap::getObjectManager()->get(\Magento\Framework\Math\Random::class) ->getUniqueHash(); $customer->changeResetPasswordLinkToken($token); + $customer->setData('confirmation', 'confirmation'); $customer->save(); $this->getRequest()->setParam('token', $token); @@ -187,6 +188,7 @@ public function testCreatepasswordActionWithDirectLink() $session = Bootstrap::getObjectManager()->get(Session::class); $this->assertEquals($token, $session->getRpToken()); $this->assertNotContains($token, $response->getHeader('Location')->getFieldValue()); + $this->assertCustomerConfirmationEquals(1, null); } /** @@ -201,6 +203,7 @@ public function testCreatepasswordActionWithSession() $token = Bootstrap::getObjectManager()->get(\Magento\Framework\Math\Random::class) ->getUniqueHash(); $customer->changeResetPasswordLinkToken($token); + $customer->setData('confirmation', 'confirmation'); $customer->save(); /** @var \Magento\Customer\Model\Session $customer */ @@ -213,6 +216,7 @@ public function testCreatepasswordActionWithSession() $response = $this->getResponse(); $text = $response->getBody(); $this->assertTrue((bool)preg_match('/' . $token . '/m', $text)); + $this->assertCustomerConfirmationEquals(1, null); } /** @@ -227,6 +231,7 @@ public function testCreatepasswordActionInvalidToken() $token = Bootstrap::getObjectManager()->get(\Magento\Framework\Math\Random::class) ->getUniqueHash(); $customer->changeResetPasswordLinkToken($token); + $customer->setData('confirmation', 'confirmation'); $customer->save(); $this->getRequest()->setParam('token', 'INVALIDTOKEN'); @@ -238,6 +243,19 @@ public function testCreatepasswordActionInvalidToken() $response = $this->getResponse(); $this->assertEquals(302, $response->getHttpResponseCode()); $this->assertContains('customer/account/forgotpassword', $response->getHeader('Location')->getFieldValue()); + $this->assertCustomerConfirmationEquals(1, 'confirmation'); + } + + /** + * @param int $customerId + * @param string|null $confirmation + */ + private function assertCustomerConfirmationEquals(int $customerId, string $confirmation = null) + { + /** @var \Magento\Customer\Model\Customer $customer */ + $customer = Bootstrap::getObjectManager() + ->create(\Magento\Customer\Model\Customer::class)->load($customerId); + $this->assertEquals($confirmation, $customer->getConfirmation()); } /** @@ -913,6 +931,7 @@ private function getConfirmationUrlFromMessageContent(string $content): string if (preg_match('.*?)".*>', $content, $matches)) { $confirmationUrl = $matches['url']; $confirmationUrl = str_replace('http://localhost/index.php/', '', $confirmationUrl); + // phpcs:ignore Magento2.Functions.DiscouragedFunction $confirmationUrl = html_entity_decode($confirmationUrl); }