#4933 fix: check email validity before attempting to save email or check if password request is allowed

[pquerner]

Description (*)

This pull request moved the email address validity check to the top, which means that invalid email addresses are no longer saved nor checked in the database, before making sure it is even a valid email address.

Fixed Issues (if relevant)

• fixes Validate email address for forgottenPassword #4933

Manual testing scenarios (*)

1. Go to password reset page in frontend
2. Enter invalid email address (disable frontend javascript validation by removing the CSS classes)
3. Backend should disallow invalid email addresses now

Questions or comments

Contribution checklist (*)

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All automated tests passed successfully (all builds are green)

[kiatng]

I tested with a bogus email with valid format; the system saves on every submission beyond the allowed times. Shouldn't we avoid saving beyond the allowed times for this type of emails as well, besides invalid formatted emails?

[pquerner]

I tested with a bogus email with valid format; the system saves on every submission beyond the allowed times. Shouldn't we avoid saving beyond the allowed times for this type of emails as well, besides invalid formatted emails?

It is true that inserts are done first and then checked afterwards. But I didn't want to touch that logic part.

[pquerner]

PHPStan fails, but not because of my changes. No idea what to do in that case. :D Anyone?

[Hanmac]

PHPStan fails, but not because of my changes. No idea what to do in that case. :D Anyone?

I added an extra MR #4937

[kiatng]

If the intent of this PR is to lower write amplification: fewer DB writes under spam/abuse, then we need to consider that correctly formatted emails would still get written to the DB until it exceeded 24 hours or per IP limits.

[pquerner]

Yes, but please lets do 1 issue 1 pull request. I didnt want to get this all too bloated.

[kiatng]

I tasked the AI for a solution, it came up with this:

    public function forgotPasswordPostAction()
    {
        $email = (string) $this->getRequest()->getPost('email');
        if ($email) {
            $flowPassword = Mage::getModel('customer/flowpassword');
            $flowPassword->setEmail($email); // kiat: no save

            if (!$flowPassword->checkCustomerForgotPasswordFlowEmail($email)) {
                // ...
            }

            if (!$flowPassword->checkCustomerForgotPasswordFlowIp()) {
                // ...
            }

            if (!Zend_Validate::is($email, 'EmailAddress')) {
                // ...
            }

            // ...

            try {
                $flowPassword->save(); // kiat: insert save after try before if
                if ($customerId) {
                    /** @var Helper $helper */ // kiat: should remove
                    // ...
                }
            } catch (Exception $exception) {
                // ...
            }
//....

I tested it and it works! You can still move the email validation up if desired. The log only saved up to the limit set in the system config.

[pquerner]

As I said, this wasnt the intend of this pull request and I feel like it should be done in its own PR.
Of course if you or others feel like it absolutely must be included here, then please go ahead and add it.

[addison74]

PHPStan errors.

[Copilot]

Pull Request Overview

This PR improves the password reset flow by validating email addresses before performing any database operations or flow checks. The change prevents invalid email addresses from being processed or stored during password reset attempts.

Key changes:

• Email validation moved to the beginning of the process
• Database operations only occur after email validation passes
• Improved error handling flow for invalid emails

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud