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.

Sign up for GitHub

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

Jump to bottom

[Backport 2.2-develop] #11409: Too many password reset requests even when disabled in settings #11435

Merged
merged 1 commit into from
Dec 7, 2017
Merged

[Backport 2.2-develop] #11409: Too many password reset requests even when disabled in settings #11435

merged 1 commit into from
Dec 7, 2017

Conversation

adrian-martinez-interactiv4
Copy link
Contributor

When attempting to reset a customer's password via the admin, the system tells me 'Too many password reset requests' even when I have disabled the 'max wait time between password resets' in the store configuration settings.

Related with PR#11434

Description

\Magento\Security\Model\Config::getXmlPathPrefix() method fails to use the customer configuration customer/password/, using the admin/security/ settings instead, when reset password is triggered from admin, due to current scope:

    /**
     * {@inheritDoc}
     *
     * @return string
     */
    protected function getXmlPathPrefix()
    {
        if ($this->scope->getCurrentScope() == \Magento\Framework\App\Area::AREA_ADMINHTML) {
            return self::XML_PATH_ADMIN_AREA;
        }
        return self::XML_PATH_FRONTEND_AREA;
    }

Emulated frontend area in scope in plugin method \Magento\Security\Model\Plugin\AccountManagement::beforeInitiatePasswordReset, also fixed di.xml parameter injection that caused customer password reset requests also counting as admin user requests when emails coincide for admin user and customer.

Fixed Issues (if relevant)

  1. Too many password reset requests even when disabled in settings #11409: Too many password reset requests even when disabled in settings

Manual testing scenarios

  • Login to the admin
  • Go to Customers > All Customers
  • Click to edit a customer and click 'Reset Password' from the options in the top
  • This works the first time, but only then. Every subsequent request fails.

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

@adrian-martinez-interactiv4 adrian-martinez-interactiv4 changed the title [Backport 2.2-develop] Issue #11409: Too many password reset requests even when disabled in settings [Backport 2.2-develop] #11409: Too many password reset requests even when disabled in settings Oct 14, 2017
@dmanners dmanners self-assigned this Oct 24, 2017
@dmanners
Copy link
Contributor

See #11434 (comment) for comment on change.

dmanners
dmanners reviewed Oct 26, 2017
View reviewed changes
app/code/Magento/Security/Model/Config.php Outdated
@@ -27,7 +27,7 @@ class Config implements ConfigInterface
/**
* Configuration path to fronted area
*/
const XML_PATH_FRONTED_AREA = 'customer/password/';
const XML_PATH_FRONTEND_AREA = 'customer/password/';
Copy link
Contributor

Choose a reason for hiding this comment

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

Unfortunately we cannot rename constants even if it contained a spelling mistake before :( http://devdocs.magento.com/guides/v2.2/contributor-guide/backward-compatible-development/index.html#removing-or-renaming-constants

Copy link
Contributor

Choose a reason for hiding this comment

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

You could always deprecate the constant with the spelling mistake, create a new constant and use that. Then update the old constant to use the new one, I think that would work.

dmanners
dmanners suggested changes Oct 26, 2017
View reviewed changes
Copy link
Contributor

@dmanners dmanners left a comment

Choose a reason for hiding this comment

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

As discussed it would be great to get this to include the same changes with #11434 and also to update the constants so that we do not break BC.

@adrian-martinez-interactiv4 adrian-martinez-interactiv4 force-pushed the FR22#11409-TOO-MANY-PASSWORD-RESET-REQUESTS branch from 057524e to 84c9c91 Compare October 28, 2017 13:38
@dmanners
Copy link
Contributor

dmanners commented Nov 1, 2017

I will put this on hold until #11434 has been merged into 2.3-develop

@magento-engcom-team magento-engcom-team added 2.2.x Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release labels Nov 7, 2017
@dmanners
Copy link
Contributor

Now that #11434 has been sorted can you make the same changes into the backported PRs @adrian-martinez-interactiv4

@adrian-martinez-interactiv4 adrian-martinez-interactiv4 force-pushed the FR22#11409-TOO-MANY-PASSWORD-RESET-REQUESTS branch from 84c9c91 to 8e29dc7 Compare November 30, 2017 17:22
@adrian-martinez-interactiv4 adrian-martinez-interactiv4 force-pushed the FR22#11409-TOO-MANY-PASSWORD-RESET-REQUESTS branch from 8e29dc7 to d06190d Compare December 1, 2017 10:13
@dmanners dmanners added this to the December 2017 milestone Dec 1, 2017
@magento-team magento-team merged commit d06190d into magento:2.2-develop Dec 7, 2017
magento-team pushed a commit that referenced this pull request Dec 7, 2017
@ishakhsuvarov
MAGETWO-84862: [Backport 2.2-develop] #11409: Too many password reset…
9fe7383 
… requests even when disabled in settings #11435
magento-team pushed a commit that referenced this pull request Dec 7, 2017
@ishakhsuvarov
Merge pull request #1808 from magento-engcom/2.2-develop-prs
321278b 
[EngCom] Public Pull Requests - 2.2-develop
 - MAGETWO-84981: Trying to get data from non existent products #12539
 - MAGETWO-84979: [Backport 2.2-develop] Fix swagger-ui on instances of Magento running on a non-standard port #12541
 - MAGETWO-84903: Added namespace to product videos fotorama events #12469
 - MAGETWO-84862: [Backport 2.2-develop] #11409: Too many password reset requests even when disabled in settings #11435
 - MAGETWO-84856: Issue 12506: Fixup typo getDispretionPath -> getDispersionPath #12507
 - MAGETWO-84808: 12110: Missing cascade into attribute set deletion. #12167
 - MAGETWO-83503: [2.2] - Add command to view mview state and queue #12122
 - MAGETWO-80223: Fix syntax of expectException() calls #11099
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dmanners dmanners dmanners requested changes

Assignees

@dmanners dmanners

Labels
Award: complex Award: test coverage Progress: accept Progress: on hold Release Line: 2.2 Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release
Projects
None yet
Milestone
December 2017
Development

Successfully merging this pull request may close these issues.
5 participants
@adrian-martinez-interactiv4 @dmanners @magento-team @vkublytskyi @magento-engcom-team