Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CC-26461 Backporting improvements to the password reset workflow #5

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 0 additions & 21 deletions architecture-baseline.json
Original file line number Diff line number Diff line change
@@ -1,25 +1,4 @@
[
{
"fileName": "src/Spryker/Zed/Customer/Communication/Form/AddressForm.php",
"description": "Method `Spryker\\Zed\\Customer\\Communication\\Form\\AddressForm::getBlockPrefix()` must have return type.",
"rule": "ExternalMethodExtensionReturnTypeRule",
"ruleset": "Spryker",
"priority": "1"
},
{
"fileName": "src/Spryker/Zed/Customer/Communication/Form/CustomerForm.php",
"description": "Method `Spryker\\Zed\\Customer\\Communication\\Form\\CustomerForm::getBlockPrefix()` must have return type.",
"rule": "ExternalMethodExtensionReturnTypeRule",
"ruleset": "Spryker",
"priority": "1"
},
{
"fileName": "src/Spryker/Zed/Customer/Communication/Form/CustomerUpdateForm.php",
"description": "Method `Spryker\\Zed\\Customer\\Communication\\Form\\CustomerUpdateForm::getBlockPrefix()` must have return type.",
"rule": "ExternalMethodExtensionReturnTypeRule",
"ruleset": "Spryker",
"priority": "1"
},
{
"fileName": "src/Spryker/Zed/Customer/Dependency/Facade/CustomerToCountryBridge.php",
"description": "Bridges: Method `getPreferredCountryByName()` must have `public function get<DomainEntity>Collection(<DomainEntity>CriteriaTransfer): <DomainEntity>CollectionTransfer` signature.",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
<?php

/**
* Copyright © 2016-present Spryker Systems GmbH. All rights reserved.
* Use of this software requires acceptance of the Evaluation License Agreement. See LICENSE file.
*/

namespace Spryker\Zed\Customer\Business\Customer\Checker;

use DateTime;
use Generated\Shared\Transfer\CustomerErrorTransfer;
use Generated\Shared\Transfer\CustomerResponseTransfer;
use Orm\Zed\Customer\Persistence\SpyCustomer;
use Spryker\Zed\Customer\CustomerConfig;

class PasswordResetExpirationChecker implements PasswordResetExpirationCheckerInterface
{
/**
* @var \Spryker\Zed\Customer\CustomerConfig
*/
protected CustomerConfig $customerConfig;

/**
* @param \Spryker\Zed\Customer\CustomerConfig $customerConfig
*/
public function __construct(CustomerConfig $customerConfig)
{
$this->customerConfig = $customerConfig;
}

/**
* @param \Orm\Zed\Customer\Persistence\SpyCustomer$customerEntity
* @param \Generated\Shared\Transfer\CustomerResponseTransfer $customerResponseTransfer
*
* @return \Generated\Shared\Transfer\CustomerResponseTransfer
*/
public function checkPasswordResetExpiration(
SpyCustomer $customerEntity,
CustomerResponseTransfer $customerResponseTransfer
): CustomerResponseTransfer {
if (!$this->customerConfig->isCustomerPasswordResetExpirationEnabled()) {
return $customerResponseTransfer
->setIsSuccess(true);
}

/** @var \DateTime|string|null $restorePasswordDate */
$restorePasswordDate = $customerEntity->getRestorePasswordDate();

if (!$restorePasswordDate) {
return $customerResponseTransfer;
}

if (is_string($restorePasswordDate)) {
$restorePasswordDate = new DateTime($restorePasswordDate);
}

$expirationDate = clone $restorePasswordDate;
$expirationDate->modify($this->customerConfig->getCustomerPasswordResetExpirationPeriod());
$now = new DateTime();

if ($now < $expirationDate) {
return $customerResponseTransfer;
}

$customerErrorTransfer = (new CustomerErrorTransfer())->setMessage(CustomerConfig::GLOSSARY_KEY_CONFIRM_EMAIL_LINK_INVALID_OR_USED);

return $customerResponseTransfer
->setIsSuccess(false)
->addError($customerErrorTransfer);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?php

/**
* Copyright © 2016-present Spryker Systems GmbH. All rights reserved.
* Use of this software requires acceptance of the Evaluation License Agreement. See LICENSE file.
*/

namespace Spryker\Zed\Customer\Business\Customer\Checker;

use Generated\Shared\Transfer\CustomerResponseTransfer;
use Orm\Zed\Customer\Persistence\SpyCustomer;

interface PasswordResetExpirationCheckerInterface
{
/**
* @param \Orm\Zed\Customer\Persistence\SpyCustomer$customerEntity
* @param \Generated\Shared\Transfer\CustomerResponseTransfer $customerResponseTransfer
*
* @throws \RuntimeException
*
* @return \Generated\Shared\Transfer\CustomerResponseTransfer
*/
public function checkPasswordResetExpiration(
SpyCustomer $customerEntity,
CustomerResponseTransfer $customerResponseTransfer
): CustomerResponseTransfer;
}
31 changes: 19 additions & 12 deletions src/Spryker/Zed/Customer/Business/Customer/Customer.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
use Propel\Runtime\Collection\ObjectCollection;
use Spryker\Service\UtilText\UtilTextService;
use Spryker\Shared\Customer\Code\Messages;
use Spryker\Zed\Customer\Business\Customer\Checker\PasswordResetExpirationCheckerInterface;
use Spryker\Zed\Customer\Business\CustomerExpander\CustomerExpanderInterface;
use Spryker\Zed\Customer\Business\CustomerPasswordPolicy\CustomerPasswordPolicyValidatorInterface;
use Spryker\Zed\Customer\Business\Exception\CustomerNotFoundException;
Expand All @@ -34,7 +35,6 @@
use Spryker\Zed\Customer\Dependency\Facade\CustomerToMailInterface;
use Spryker\Zed\Customer\Persistence\CustomerQueryContainerInterface;
use Symfony\Component\Console\Output\OutputInterface;
use Symfony\Component\Security\Core\Encoder\BCryptPasswordEncoder;
use Symfony\Component\Security\Core\Encoder\NativePasswordEncoder;
use Symfony\Component\Security\Core\Encoder\PasswordEncoderInterface;

Expand All @@ -50,11 +50,6 @@ class Customer implements CustomerInterface
*/
protected const BCRYPT_SALT = '';

/**
* @var string
*/
protected const GLOSSARY_KEY_CONFIRM_EMAIL_LINK_INVALID_OR_USED = 'customer.error.confirm_email_link.invalid_or_used';

/**
* @var string
*/
Expand Down Expand Up @@ -115,6 +110,11 @@ class Customer implements CustomerInterface
*/
protected $localeFacade;

/**
* @var \Spryker\Zed\Customer\Business\Customer\Checker\PasswordResetExpirationCheckerInterface
*/
protected PasswordResetExpirationCheckerInterface $passwordResetExpirationChecker;

/**
* @param \Spryker\Zed\Customer\Persistence\CustomerQueryContainerInterface $queryContainer
* @param \Spryker\Zed\Customer\Business\ReferenceGenerator\CustomerReferenceGeneratorInterface $customerReferenceGenerator
Expand All @@ -125,6 +125,7 @@ class Customer implements CustomerInterface
* @param \Spryker\Zed\Customer\Dependency\Facade\CustomerToLocaleInterface $localeFacade
* @param \Spryker\Zed\Customer\Business\CustomerExpander\CustomerExpanderInterface $customerExpander
* @param \Spryker\Zed\Customer\Business\CustomerPasswordPolicy\CustomerPasswordPolicyValidatorInterface $customerPasswordPolicyValidator
* @param \Spryker\Zed\Customer\Business\Customer\Checker\PasswordResetExpirationCheckerInterface $passwordResetExpirationChecker
* @param array<\Spryker\Zed\CustomerExtension\Dependency\Plugin\PostCustomerRegistrationPluginInterface> $postCustomerRegistrationPlugins
*/
public function __construct(
Expand All @@ -137,6 +138,7 @@ public function __construct(
CustomerToLocaleInterface $localeFacade,
CustomerExpanderInterface $customerExpander,
CustomerPasswordPolicyValidatorInterface $customerPasswordPolicyValidator,
PasswordResetExpirationCheckerInterface $passwordResetExpirationChecker,
array $postCustomerRegistrationPlugins = []
) {
$this->queryContainer = $queryContainer;
Expand All @@ -149,6 +151,7 @@ public function __construct(
$this->customerPasswordPolicyValidator = $customerPasswordPolicyValidator;
$this->postCustomerRegistrationPlugins = $postCustomerRegistrationPlugins;
$this->localeFacade = $localeFacade;
$this->passwordResetExpirationChecker = $passwordResetExpirationChecker;
}

/**
Expand Down Expand Up @@ -434,7 +437,7 @@ public function confirmCustomerRegistration(CustomerTransfer $customerTransfer):
if (!$customerEntity) {
return $customerResponseTransfer
->setIsSuccess(false)
->addError((new CustomerErrorTransfer())->setMessage(static::GLOSSARY_KEY_CONFIRM_EMAIL_LINK_INVALID_OR_USED));
->addError((new CustomerErrorTransfer())->setMessage(CustomerConfig::GLOSSARY_KEY_CONFIRM_EMAIL_LINK_INVALID_OR_USED));
}

$customerEntity->setRegistered(new DateTime());
Expand Down Expand Up @@ -505,6 +508,14 @@ public function restorePassword(CustomerTransfer $customerTransfer)
return $customerResponseTransfer;
}

$customerResponseTransfer = $this
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the test file should be also copied from the original PR (Bundles/Customer/tests/SprykerTest/Zed/Customer/Business/Facade/RestorePasswordTest.php).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought we don't backport tests? 😕

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we backporting all diffs

->passwordResetExpirationChecker
->checkPasswordResetExpiration($customerEntity, $customerResponseTransfer);

if (!$customerResponseTransfer->getIsSuccess()) {
return $customerResponseTransfer;
}

$customerEntity->setRestorePasswordDate(null);
$customerEntity->setRestorePasswordKey(null);

Expand Down Expand Up @@ -887,11 +898,7 @@ protected function getEncodedPassword($currentPassword)
*/
protected function getPasswordEncoder(): PasswordEncoderInterface
{
if (class_exists(NativePasswordEncoder::class)) {
return new NativePasswordEncoder(null, null, static::BCRYPT_FACTOR);
}

return new BCryptPasswordEncoder(static::BCRYPT_FACTOR);
return new NativePasswordEncoder(null, null, static::BCRYPT_FACTOR);
}

/**
Expand Down
13 changes: 13 additions & 0 deletions src/Spryker/Zed/Customer/Business/CustomerBusinessFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
use Spryker\Zed\Customer\Business\Checkout\CustomerOrderSaverInterface;
use Spryker\Zed\Customer\Business\Checkout\CustomerOrderSaverWithMultiShippingAddress;
use Spryker\Zed\Customer\Business\Customer\Address;
use Spryker\Zed\Customer\Business\Customer\Checker\PasswordResetExpirationChecker;
use Spryker\Zed\Customer\Business\Customer\Checker\PasswordResetExpirationCheckerInterface;
use Spryker\Zed\Customer\Business\Customer\Customer;
use Spryker\Zed\Customer\Business\Customer\CustomerReader;
use Spryker\Zed\Customer\Business\Customer\CustomerReaderInterface;
Expand Down Expand Up @@ -66,12 +68,23 @@ public function createCustomer()
$this->getLocaleFacade(),
$this->createCustomerExpander(),
$this->createCustomerPasswordPolicyValidator(),
$this->createPasswordResetExpirationChecker(),
$this->getPostCustomerRegistrationPlugins(),
);

return $customer;
}

/**
* @return \Spryker\Zed\Customer\Business\Customer\Checker\PasswordResetExpirationCheckerInterface
*/
public function createPasswordResetExpirationChecker(): PasswordResetExpirationCheckerInterface
{
return new PasswordResetExpirationChecker(
$this->getConfig(),
);
}

/**
* @return \Spryker\Zed\Customer\Business\Customer\CustomerReaderInterface
*/
Expand Down
10 changes: 5 additions & 5 deletions src/Spryker/Zed/Customer/Communication/Form/AddressForm.php
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ protected function addFkCustomerField(FormBuilderInterface $builder)

/**
* @param \Symfony\Component\Form\FormBuilderInterface $builder
* @param array $choices
* @param array<mixed, mixed> $choices
*
* @return $this
*/
Expand Down Expand Up @@ -331,8 +331,8 @@ protected function addZipCodeField(FormBuilderInterface $builder)

/**
* @param \Symfony\Component\Form\FormBuilderInterface $builder
* @param array $choices
* @param array $preferredChoices
* @param array<mixed, mixed> $choices
* @param array<mixed, mixed> $preferredChoices
*
* @return $this
*/
Expand Down Expand Up @@ -444,7 +444,7 @@ protected function createLastNameRegexConstraint(): Regex
/**
* @return string
*/
public function getBlockPrefix()
public function getBlockPrefix(): string
{
return 'customer_address';
}
Expand All @@ -454,7 +454,7 @@ public function getBlockPrefix()
*
* @return string
*/
public function getName()
public function getName(): string
{
return $this->getBlockPrefix();
}
Expand Down
21 changes: 11 additions & 10 deletions src/Spryker/Zed/Customer/Communication/Form/CustomerForm.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
use Symfony\Component\Form\Extension\Core\Type\TextType;
use Symfony\Component\Form\FormBuilderInterface;
use Symfony\Component\OptionsResolver\OptionsResolver;
use Symfony\Component\Validator\Constraint;
use Symfony\Component\Validator\Constraints\Callback;
use Symfony\Component\Validator\Constraints\Email;
use Symfony\Component\Validator\Constraints\Length;
Expand Down Expand Up @@ -118,7 +119,7 @@ class CustomerForm extends AbstractType
/**
* @return string
*/
public function getBlockPrefix()
public function getBlockPrefix(): string
{
return 'customer';
}
Expand Down Expand Up @@ -188,7 +189,7 @@ protected function addEmailField(FormBuilderInterface $builder)

/**
* @param \Symfony\Component\Form\FormBuilderInterface $builder
* @param array $choices
* @param array<mixed, mixed> $choices
*
* @return $this
*/
Expand Down Expand Up @@ -246,7 +247,7 @@ protected function addLastNameField(FormBuilderInterface $builder)

/**
* @param \Symfony\Component\Form\FormBuilderInterface $builder
* @param array $choices
* @param array<mixed, mixed> $choices
*
* @return $this
*/
Expand Down Expand Up @@ -319,7 +320,7 @@ protected function addPhoneField(FormBuilderInterface $builder)

/**
* @param \Symfony\Component\Form\FormBuilderInterface $builder
* @param array $choices
* @param array<mixed, mixed> $choices
*
* @return $this
*/
Expand Down Expand Up @@ -351,7 +352,7 @@ protected function addStoreField(FormBuilderInterface $builder, array $choices)

/**
* @param \Symfony\Component\Form\FormBuilderInterface $builder
* @param array $choices
* @param array<mixed, mixed> $choices
*
* @return $this
*/
Expand Down Expand Up @@ -394,9 +395,9 @@ protected function addDateOfBirthField(FormBuilderInterface $builder)
}

/**
* @return array
* @return list<\Symfony\Component\Validator\Constraint>
*/
protected function createEmailConstraints()
protected function createEmailConstraints(): array
{
$emailConstraints = [
new NotBlank(),
Expand Down Expand Up @@ -456,7 +457,7 @@ protected function createLastNameRegexConstraint(): Regex
/**
* @return \Symfony\Component\Form\CallbackTransformer
*/
protected function createDateTimeModelTransformer()
protected function createDateTimeModelTransformer(): CallbackTransformer
{
return new CallbackTransformer(
function ($dateAsString) {
Expand All @@ -473,7 +474,7 @@ function ($dateAsObject) {
/**
* @return \Symfony\Component\Form\CallbackTransformer
*/
protected function createLocaleModelTransformer()
protected function createLocaleModelTransformer(): CallbackTransformer
{
return new CallbackTransformer(
function ($localeAsObject) {
Expand All @@ -494,7 +495,7 @@ function ($localeAsInt) {
*
* @return string
*/
public function getName()
public function getName(): string
{
return $this->getBlockPrefix();
}
Expand Down
Loading
Loading