Skip to content

Commit e0f1e46

Browse files
admanesachinadmanesachin
committed
AC-1323: Reset Token Improvement.
1 parent ea6c573 commit e0f1e46

File tree

9 files changed

+77
-59
lines changed

9 files changed

+77
-59
lines changed

‎app/code/Magento/Customer/Controller/Account/CreatePassword.php‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use Magento\Framework\Controller\Result\Redirect;
1818
use Magento\Framework\View\Result\Page;
1919
use Magento\Framework\View\Result\PageFactory;
20+
use Magento\Customer\Api\CustomerRepositoryInterface;
2021

2122
/**
2223
* Controller for front-end customer password reset form
@@ -48,21 +49,28 @@ class CreatePassword extends \Magento\Customer\Controller\AbstractAccount implem
4849
*/
4950
private $getByToken;
5051

52+
/**
53+
* @var CustomerRepositoryInterface
54+
*/
55+
private $customerRepository;
56+
5157
/**
5258
* @param Context $context
5359
* @param Session $customerSession
5460
* @param PageFactory $resultPageFactory
5561
* @param AccountManagementInterface $accountManagement
5662
* @param ConfirmCustomerByToken|null $confirmByToken
5763
* @param GetCustomerByToken|null $getByToken
64+
* @param CustomerRepositoryInterface|null $customerRepository
5865
*/
5966
public function __construct(
6067
Context $context,
6168
Session $customerSession,
6269
PageFactory $resultPageFactory,
6370
AccountManagementInterface $accountManagement,
6471
ConfirmCustomerByToken $confirmByToken = null,
65-
GetCustomerByToken $getByToken = null
72+
GetCustomerByToken $getByToken = null,
73+
CustomerRepositoryInterface $customerRepository = null
6674
) {
6775
$this->session = $customerSession;
6876
$this->resultPageFactory = $resultPageFactory;
@@ -71,6 +79,8 @@ public function __construct(
7179
?? ObjectManager::getInstance()->get(ConfirmCustomerByToken::class);
7280
$this->getByToken = $getByToken
7381
?? ObjectManager::getInstance()->get(GetCustomerByToken::class);
82+
$this->customerRepository = $customerRepository
83+
?? ObjectManager::getInstance()->get(CustomerRepositoryInterface::class);
7484

7585
parent::__construct($context);
7686
}
@@ -83,23 +93,24 @@ public function __construct(
8393
public function execute()
8494
{
8595
$resetPasswordToken = (string)$this->getRequest()->getParam('token');
96+
$customerId = (int)$this->getRequest()->getParam('id');
8697
$isDirectLink = $resetPasswordToken != '';
8798
if (!$isDirectLink) {
8899
$resetPasswordToken = (string)$this->session->getRpToken();
100+
$customerId = (int)$this->session->getRpCustomerId();
89101
}
90102

91103
try {
92-
$this->accountManagement->validateResetPasswordLinkToken(null, $resetPasswordToken);
93-
94-
$this->confirmByToken->execute($resetPasswordToken);
104+
$this->accountManagement->validateResetPasswordLinkToken($customerId, $resetPasswordToken);
95105

96106
// Extend token validity to avoid expiration while this form is
97107
// being completed by the user.
98-
$customer = $this->getByToken->execute($resetPasswordToken);
108+
$customer = $this->customerRepository->getById($customerId);
99109
$this->accountManagement->changeResetPasswordLinkToken($customer, $resetPasswordToken);
100110

101111
if ($isDirectLink) {
102112
$this->session->setRpToken($resetPasswordToken);
113+
$this->session->setRpCustomerId($customerId);
103114
$resultRedirect = $this->resultRedirectFactory->create();
104115
$resultRedirect->setPath('*/*/createpassword');
105116

@@ -109,7 +120,8 @@ public function execute()
109120
$resultPage = $this->resultPageFactory->create();
110121
$resultPage->getLayout()
111122
->getBlock('resetPassword')
112-
->setResetPasswordLinkToken($resetPasswordToken);
123+
->setResetPasswordLinkToken($resetPasswordToken)
124+
->setRpCustomerId($customerId);
113125

114126
return $resultPage;
115127
}

‎app/code/Magento/Customer/Controller/Account/ResetPasswordPost.php‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,10 @@ public function execute()
6767
/** @var \Magento\Framework\Controller\Result\Redirect $resultRedirect */
6868
$resultRedirect = $this->resultRedirectFactory->create();
6969
$resetPasswordToken = (string)$this->getRequest()->getQuery('token');
70+
$customerId = (string)$this->getRequest()->getQuery('id');
7071
$password = (string)$this->getRequest()->getPost('password');
7172
$passwordConfirmation = (string)$this->getRequest()->getPost('password_confirmation');
73+
$email = null;
7274

7375
if ($password !== $passwordConfirmation) {
7476
$this->messageManager->addErrorMessage(__("New Password and Confirm New Password values didn't match."));
@@ -83,9 +85,13 @@ public function execute()
8385
return $resultRedirect;
8486
}
8587

88+
if ($customerId && $this->customerRepository->getById($customerId)) {
89+
$email = $this->customerRepository->getById($customerId)->getEmail();
90+
}
91+
8692
try {
8793
$this->accountManagement->resetPassword(
88-
null,
94+
$email,
8995
$resetPasswordToken,
9096
$password
9197
);
@@ -95,6 +101,7 @@ public function execute()
95101
$this->session->start();
96102
}
97103
$this->session->unsRpToken();
104+
$this->session->unsRpCustomerId();
98105
$this->messageManager->addSuccessMessage(__('You updated your password.'));
99106
$resultRedirect->setPath('*/*/login');
100107

‎app/code/Magento/Customer/Model/AccountManagement.php‎

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -714,8 +714,8 @@ private function handleUnknownTemplate($template)
714714
public function resetPassword($email, $resetToken, $newPassword)
715715
{
716716
if (!$email) {
717-
$customer = $this->getByToken->execute($resetToken);
718-
$email = $customer->getEmail();
717+
$params = ['fieldName' => 'email'];
718+
throw new InputException(__('"%fieldName" is required. Enter and try again.', $params));
719719
} else {
720720
$customer = $this->customerRepository->get($email);
721721
}
@@ -1186,24 +1186,17 @@ public function validateCustomerStoreIdByWebsiteId(CustomerInterface $customer)
11861186
* @throws NoSuchEntityException If customer doesn't exist
11871187
* @SuppressWarnings(PHPMD.LongVariable)
11881188
*/
1189-
private function validateResetPasswordToken($customerId, $resetPasswordLinkToken)
1189+
private function validateResetPasswordToken(int $customerId, string $resetPasswordLinkToken): bool
11901190
{
1191-
if ($customerId !== null && $customerId <= 0) {
1191+
if (!$customerId) {
11921192
throw new InputException(
11931193
__(
11941194
'Invalid value of "%value" provided for the %fieldName field.',
11951195
['value' => $customerId, 'fieldName' => 'customerId']
11961196
)
11971197
);
11981198
}
1199-
1200-
if ($customerId === null) {
1201-
//Looking for the customer.
1202-
$customerId = $this->getByToken
1203-
->execute($resetPasswordLinkToken)
1204-
->getId();
1205-
}
1206-
if (!is_string($resetPasswordLinkToken) || empty($resetPasswordLinkToken)) {
1199+
if (!$resetPasswordLinkToken) {
12071200
$params = ['fieldName' => 'resetPasswordLinkToken'];
12081201
throw new InputException(__('"%fieldName" is required. Enter and try again.', $params));
12091202
}

‎app/code/Magento/Customer/Model/ForgotPasswordToken/GetCustomerByToken.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
/**
1818
* Get Customer By reset password token
1919
* @SuppressWarnings(PHPMD.LongVariable)
20+
* @deprecated Rp Tokens cannot be looked up directly
2021
*/
2122
class GetCustomerByToken
2223
{

‎app/code/Magento/Customer/Model/ResourceModel/Customer.php‎

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
use Magento\Framework\App\ObjectManager;
1313
use Magento\Framework\Exception\AlreadyExistsException;
1414
use Magento\Framework\Validator\Exception as ValidatorException;
15+
use Magento\Framework\Encryption\EncryptorInterface;
1516

1617
/**
1718
* Customer entity resource model
@@ -54,6 +55,11 @@ class Customer extends \Magento\Eav\Model\Entity\VersionControl\AbstractEntity
5455
*/
5556
private $notificationStorage;
5657

58+
/**
59+
* @var EncryptorInterface
60+
*/
61+
private $encryptor;
62+
5763
/**
5864
* Customer constructor.
5965
*
@@ -66,6 +72,7 @@ class Customer extends \Magento\Eav\Model\Entity\VersionControl\AbstractEntity
6672
* @param \Magento\Store\Model\StoreManagerInterface $storeManager
6773
* @param array $data
6874
* @param AccountConfirmation $accountConfirmation
75+
* @param EncryptorInterface|null $encryptor
6976
*/
7077
public function __construct(
7178
\Magento\Eav\Model\Entity\Context $context,
@@ -76,7 +83,8 @@ public function __construct(
7683
\Magento\Framework\Stdlib\DateTime $dateTime,
7784
\Magento\Store\Model\StoreManagerInterface $storeManager,
7885
$data = [],
79-
AccountConfirmation $accountConfirmation = null
86+
AccountConfirmation $accountConfirmation = null,
87+
EncryptorInterface $encryptor = null
8088
) {
8189
parent::__construct($context, $entitySnapshot, $entityRelationComposite, $data);
8290

@@ -88,6 +96,8 @@ public function __construct(
8896
$this->setType('customer');
8997
$this->setConnection('customer_read');
9098
$this->storeManager = $storeManager;
99+
$this->encryptor = $encryptor ?? ObjectManager::getInstance()
100+
->get(EncryptorInterface::class);
91101
}
92102

93103
/**
@@ -176,6 +186,11 @@ protected function _beforeSave(\Magento\Framework\DataObject $customer)
176186
$this->_validate($customer);
177187
}
178188

189+
if ($customer->getData('rp_token')) {
190+
$rpToken = $customer->getData('rp_token');
191+
$customer->setRpToken($this->encryptor->encrypt($rpToken));
192+
}
193+
179194
return $this;
180195
}
181196

@@ -224,6 +239,10 @@ protected function _afterSave(\Magento\Framework\DataObject $customer)
224239
NotificationStorage::UPDATE_CUSTOMER_SESSION,
225240
$customer->getId()
226241
);
242+
if ($customer->getData('rp_token')) {
243+
$rpToken = $customer->getData('rp_token');
244+
$customer->setRpToken($this->encryptor->decrypt($rpToken));
245+
}
227246
return parent::_afterSave($customer);
228247
}
229248

@@ -445,4 +464,16 @@ public function updateSessionCutOff(int $customerId, int $timestamp): void
445464
$this->getConnection()->quoteInto('entity_id = ?', $customerId)
446465
);
447466
}
467+
468+
/**
469+
* @inheritDoc
470+
*/
471+
protected function _afterLoad(\Magento\Framework\DataObject $customer)
472+
{
473+
if ($customer->getData('rp_token')) {
474+
$rpToken = $customer->getData('rp_token');
475+
$customer->setRpToken($this->encryptor->decrypt($rpToken));
476+
}
477+
return parent::_afterLoad($customer); //
478+
}
448479
}

‎app/code/Magento/Customer/Test/Unit/Model/AccountManagementTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1751,7 +1751,7 @@ public function testValidateResetPasswordTokenBadResetPasswordLinkToken(): void
17511751
$this->expectException(InputException::class);
17521752
$this->expectExceptionMessage('"resetPasswordLinkToken" is required. Enter and try again.');
17531753

1754-
$this->accountManagement->validateResetPasswordLinkToken(22, null);
1754+
$this->accountManagement->validateResetPasswordLinkToken(22, '');
17551755
}
17561756

17571757
/**

‎app/code/Magento/Customer/view/frontend/email/password_reset_confirmation.html‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
<!--@vars {
99
"var store.frontend_name":"Store Name",
1010
"var customer.name":"Customer Name",
11-
"var this.getUrl($store,'customer/account/createPassword/',[_query:[token:$customer.rp_token],_nosid:1])":"Reset Password URL"
11+
"var this.getUrl($store,'customer/account/createPassword/',[_query:[id:$customer.id,token:$customer.rp_token],_nosid:1])":"Reset Password URL"
1212
} @-->
1313
{{template config_path="design/email/header_template"}}
1414

@@ -22,7 +22,7 @@
2222
<table class="inner-wrapper" border="0" cellspacing="0" cellpadding="0" align="center">
2323
<tr>
2424
<td align="center">
25-
<a href="{{var this.getUrl($store,'customer/account/createPassword/',[_query:[token:$customer.rp_token],_nosid:1])}}" target="_blank">{{trans "Set a New Password"}}</a>
25+
<a href="{{var this.getUrl($store,'customer/account/createPassword/',[_query:[id:$customer.id,token:$customer.rp_token],_nosid:1])}}" target="_blank">{{trans "Set a New Password"}}</a>
2626
</td>
2727
</tr>
2828
</table>

‎app/code/Magento/Customer/view/frontend/templates/form/resetforgottenpassword.phtml‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,13 @@
33
* Copyright © Magento, Inc. All rights reserved.
44
* See COPYING.txt for license details.
55
*/
6-
6+
// phpcs:disable Generic.Files.LineLength.TooLong
7+
//phpcs:disable Magento2.Legacy.PhtmlTemplate
78
/** @var \Magento\Customer\Block\Account\Resetpassword $block */
89
?>
9-
<form action="<?= $block->escapeUrl($block->getUrl('*/*/resetpasswordpost', ['_query' => ['token' => $block->getResetPasswordLinkToken()]])) ?>"
10+
<form action="<?= $block->escapeUrl($block->getUrl('*/*/resetpasswordpost', ['_query' => ['id'=>$block->getRpCustomerId(),'token' => $block->getResetPasswordLinkToken()]])) ?>"
1011
method="post"
11-
<?php if ($block->isAutocompleteDisabled()) : ?> autocomplete="off"<?php endif; ?>
12+
<?php if ($block->isAutocompleteDisabled()): ?> autocomplete="off"<?php endif; ?>
1213
id="form-validate"
1314
class="form password reset"
1415
data-mage-init='{"validation":{}}'>

‎dev/tests/integration/testsuite/Magento/Customer/Model/AccountManagementTest.php‎

Lines changed: 6 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -375,10 +375,10 @@ public function testResetPasswordTokenSecondTime()
375375
* @magentoDataFixture Magento/Customer/_files/customer.php
376376
*
377377
*/
378-
public function testValidateResetPasswordLinkTokenNull()
378+
public function testValidateResetPasswordLinkTokenEmpty()
379379
{
380380
try {
381-
$this->accountManagement->validateResetPasswordLinkToken(1, null);
381+
$this->accountManagement->validateResetPasswordLinkToken(1, '');
382382
$this->fail('Expected exception not thrown.');
383383
} catch (InputException $ie) {
384384
$this->assertEquals('"%fieldName" is required. Enter and try again.', $ie->getRawMessage());
@@ -391,25 +391,12 @@ public function testValidateResetPasswordLinkTokenNull()
391391
/**
392392
* @magentoDataFixture Magento/Customer/_files/customer.php
393393
*/
394-
public function testValidateResetPasswordLinkTokenWithoutId()
394+
public function testValidateResetPasswordLinkTokenInvalidId()
395395
{
396396
$token = 'randomStr123';
397397
$this->setResetPasswordData($token, 'Y-m-d H:i:s');
398-
$this->assertTrue(
399-
$this->accountManagement->validateResetPasswordLinkToken(null, $token)
400-
);
401-
}
402-
/**
403-
* @magentoDataFixture Magento/Customer/_files/two_customers.php
404-
*/
405-
public function testValidateResetPasswordLinkTokenAmbiguous()
406-
{
407-
$this->expectException(\Magento\Framework\Exception\State\ExpiredException::class);
408-
409-
$token = 'randomStr123';
410-
$this->setResetPasswordData($token, 'Y-m-d H:i:s', 1);
411-
$this->setResetPasswordData($token, 'Y-m-d H:i:s', 2);
412-
$this->accountManagement->validateResetPasswordLinkToken(null, $token);
398+
$this->expectException(\Magento\Framework\Exception\InputException::class);
399+
$this->accountManagement->validateResetPasswordLinkToken(0, $token);
413400
}
414401

415402
/**
@@ -542,21 +529,7 @@ public function testResetPasswordWithoutEmail()
542529
$resetToken = 'lsdj579slkj5987slkj595lkj';
543530
$password = 'new_Password123';
544531
$this->setResetPasswordData($resetToken, 'Y-m-d H:i:s');
545-
$this->assertTrue(
546-
$this->accountManagement->resetPassword(null, $resetToken, $password)
547-
);
548-
}
549-
/**
550-
* @magentoDataFixture Magento/Customer/_files/two_customers.php
551-
*/
552-
public function testResetPasswordAmbiguousToken()
553-
{
554-
$this->expectException(\Magento\Framework\Exception\State\ExpiredException::class);
555-
556-
$resetToken = 'lsdj579slkj5987slkj595lkj';
557-
$password = 'new_Password123';
558-
$this->setResetPasswordData($resetToken, 'Y-m-d H:i:s', 1);
559-
$this->setResetPasswordData($resetToken, 'Y-m-d H:i:s', 2);
532+
$this->expectException(InputException::class);
560533
$this->accountManagement->resetPassword(null, $resetToken, $password);
561534
}
562535

0 commit comments

Comments
 (0)