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

[adrian-martinez-interactiv4]

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.

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)

[dmanners]

Does this work with multiple stores each with a different value?

[adrian-martinez-interactiv4]

This has many considerations, I'll explain them in a separate comment in the main thread.

[dmanners]

My thoughts here would be why should the admin be limited when resetting accounts to the customer restrictions. Imagine the case that a user has a problem with a reset and then calls the shop owner, the shop owner as a logged in admin, in my opinion, should be able to reset the password regardless of if the frontend reset timer limit has been reached.

I would be interested to hear other thoughts here though.

[adrian-martinez-interactiv4]

Hi @dmanners , in first place, I agree with you in last comment, shop owner logged as admin should be always able to reset customer password email, I could simply skip security checks for admin scope when password request is for a customer. That would solve part of the problem, or at least, the problem mentioned in issue #11409.

If we agree in the previous paragraph, and I do it this way, security checks will only be applied for admin user password requests from admin area, and for customer password requests from frontend area. Then you ask me: Does this work with multiple stores each with a different value?.

Yes, but with some considerations. In Magento/Security/Model/Config.php we can see this:

It will take proper settings from current store; if we don't check this fields from admin scope for customer password reset requests, since this values are not used anymore for this case and they are retrieved properly for the rest of cases, it should work for multiples stores with different values.

What I can see digging here is another design issue, maybe for other PR, that may start from applied changes from this PR. This security checks share a table, password_reset_request_event, for admin users and customers password reset requests:

Where request_type == 0 for admin users, and request_type == 1 for customers. The problem here is that this structure is enough for admin users, and also for customers, but only if customer account sharing option is global. Let's say I have 2 different websites and I have a separate account for each of them with the same email. Requesting password in the first website will affect requesting password in the second one, even if they are independent customers that may have different passwords, due that only account reference is the email.

Let me know what you think about this all.

[dmanners]

HI @adrian-martinez-interactiv4 I agree on the statement security checks will only be applied for admin user password requests from admin area, and for customer password requests from frontend area.. I would suggest that if you make that change then I would be happy with the feature and we can progress this PR. As for the other points you raised, I see the design issue but think it could/should be sorted in a different PR. Could you please raise/check for an existing issue in github for this case. We will sort the first issue in this pr and can deal with the second issue in a second PR.

Thanks

[adrian-martinez-interactiv4]

@dmanners Done, could you check it please? Thank you

[dmanners]

One more change to the constant and I think we are there.

[dmanners]

I would also suggest using self::XML_PATH_FRONTEND_AREA as the value then we will not need to maintain two versions of the constant.

[adrian-martinez-interactiv4]

@dmanners Done

[adrian-martinez-interactiv4]

@dmanners Test passed, moving changes to backports

[aabumuslimov]

related internal ticket MAGETWO-71194