Skip to content

Commit

Permalink
CC-26451: Password reset improvement (#10628)
Browse files Browse the repository at this point in the history
CC-26451 Fixed Insecure Password Reset Workflow
  • Loading branch information
abitskil authored Nov 24, 2023
1 parent 1528274 commit 3a20114
Show file tree
Hide file tree
Showing 10 changed files with 269 additions and 58 deletions.
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
->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

0 comments on commit 3a20114

Please sign in to comment.