Fixed 'Delete operation is forbidden for current area' error. #31318

vovayatsyuk · 2020-12-17T10:36:06Z

Description

This error happens if the customer address doesn't pass validation during account creation.

Fixed Issues (if relevant)

Fixes Delete operation is forbidden for current area when creating account from checkout success page. #9736
Fixes Delete operation is forbidden for current area #8631
Fixes Create Account not working properly for Guest User : Delete operation is forbidden for current area (Enterprise Magento ver. 2.1.0) #5699

Manual testing scenarios

Open Magento/Customer/Model/AccountManagement.php and find the following lines:

        try {
            foreach ($customerAddresses as $address) {
                if (!$this->isAddressAllowedForWebsite($address, $customer->getStoreId())) {
                    continue;
                }
                if ($address->getId()) {
                    $newAddress = clone $address;
                    $newAddress->setId(null);
                    $newAddress->setCustomerId($customer->getId());
                    $this->addressRepository->save($newAddress);
                } else {
                    $address->setCustomerId($customer->getId());
                    $this->addressRepository->save($address);
                }
            }
            $this->customerRegistry->remove($customer->getId());
        } catch (InputException $e) {
            $this->customerRepository->delete($customer);
            throw $e;
        }

Change them as shown below (Emulate InputException from addressRepository->save method):

        try {
            throw new InputException(__("Test"));                     // <-- this line was added
            foreach ($customerAddresses as $address) {
                if (!$this->isAddressAllowedForWebsite($address, $customer->getStoreId())) {
                    continue;
                }
                if ($address->getId()) {
                    $newAddress = clone $address;
                    $newAddress->setId(null);
                    $newAddress->setCustomerId($customer->getId());
                    $this->addressRepository->save($newAddress);
                } else {
                    $address->setCustomerId($customer->getId());
                    $this->addressRepository->save($address);
                }
            }
            $this->customerRegistry->remove($customer->getId());
        } catch (InputException $e) {
            $this->customerRepository->delete($customer);
            throw $e;
        }

Add product to cart as a guest and navigate to multishipping checkout.
Press "Create account"
Fill the form and submit it.
An error "Test" will be shown.
If you will try to register one more time, you'll be informed that this email is already taken.
You can also check the customer's grid in the magento backend and see that your customer was just created.

Expected Behavior

The client should see the "Test" error message every time he submits the form.

Actual Behavior

The client sees the "Delete operation is forbidden for current area" error message for first time. Next submits tells that his email is already in use.

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 are green)

Resolved issues:

resolves [Issue] Fixed 'Delete operation is forbidden for current area' error. #31540: Fixed 'Delete operation is forbidden for current area' error.

This error happens if customer address doesn't pass validation during account creation.

m2-assistant · 2020-12-17T10:36:09Z

Hi @vovayatsyuk. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

@magento give me test instance - deploy test instance based on PR changes
@magento give me 2.4-develop instance - deploy vanilla Magento instance

❗ Automated tests can be triggered manually with an appropriate comment:

@magento run all tests - run or re-run all required tests against the PR changes
@magento run <test-build(s)> - run or re-run specific test build(s)
For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names. Allowed build names are:

Database Compare
Functional Tests CE
Functional Tests EE,
Functional Tests B2B
Integration Tests
Magento Health Index
Sample Data Tests CE
Sample Data Tests EE
Sample Data Tests B2B
Static Tests
Unit Tests
WebAPI Tests

You can find more information about the builds here

ℹ️ Please run only needed test builds instead of all when developing. Please run all test builds before sending your PR for review.

For more details, please, review the Magento Contributor Guide documentation.

⚠️ According to the Magento Contribution requirements, all Pull Requests must go through the Community Contributions Triage process. Community Contributions Triage is a public meeting.

🕙 You can find the schedule on the Magento Community Calendar page.

📞 The triage of Pull Requests happens in the queue order. If you want to speed up the delivery of your contribution, please join the Community Contributions Triage session to discuss the appropriate ticket.

🎥 You can find the recording of the previous Community Contributions Triage on the Magento Youtube Channel

✏️ Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel

rogyar

Hi @vovayatsyuk. The fix totally makes sense for me. Thank you.

Could I ask you to cover this case with an integration test that will trigger the InputException, please?

Thanks

engcom-Charlie · 2020-12-21T11:14:22Z

Hi @vovayatsyuk, could you please cover your changes by integration test?
Thank you.

…dation-was-failed

vovayatsyuk · 2020-12-21T15:55:58Z

There is a test that already checks account creation with an exception during address save.

The only part that is missing in the test is to check if delete wasn't blocked in beforeDelete.

I don't know how to do that since $this->customerRepository is a mocked object and no validation is made for this mock.

My zero testing knowledge doesn't allow me to understand how to do this properly. Can you help?

vovayatsyuk · 2020-12-21T16:01:52Z

I think I got it. I should add a test to dev/tests/integration/testsuite/Magento/Customer/Model/AccountManagementTest.php

The test checks if email is still available after failed try with incorrect address.

engcom-Charlie · 2020-12-22T08:55:28Z

@magento run all tests

engcom-Charlie · 2020-12-22T10:52:17Z

Hello, @vovayatsyuk looks like the introduced test falls.
Could you please take a look?
Thank you.

vovayatsyuk · 2020-12-22T11:04:32Z

Can you please advise? When I'm running it as a single test is works fine:

../../../vendor/bin/phpunit --filter testCreateAccountWithInvalidAddress testsuite/Magento/Customer/Model/AccountManagementTest.php

But when I run whole class:

../../../vendor/bin/phpunit testsuite/Magento/Customer/Model/AccountManagementTest.php

It fails. Do you have any idea?

engcom-Charlie · 2020-12-22T11:20:22Z

@vovayatsyuk you can check the previous test cases in the test class looks like they affect your test case.
Please let me know if you will have any questions.

vovayatsyuk · 2020-12-22T11:25:14Z

Okay. When I run all tests my fix is not working and I still get a Delete operation is forbidden for current area exception.

I've updated the test. It still doesn't work, but at least it shows the real issue of why it's failed.

engcom-Charlie · 2020-12-22T11:33:29Z

It is necessary to make that all tests passed successfully.
Could you do it?
Thank you.

vovayatsyuk · 2020-12-22T12:22:56Z

I found the reason for the failure (https://github.com/magento/magento2/blob/2.4-develop/dev/tests/integration/testsuite/Magento/Customer/_files/customer_rollback.php#L24):

$registry->register('isSecureArea', false);

p.s. There are 398 other test files that use this code too. I believe all of these lines should be removed if test doesn't change it back to unregistered state.

engcom-Charlie · 2020-12-22T12:47:14Z

@magento run Integration Tests

vovayatsyuk · 2020-12-22T12:51:14Z

I didn't fix it yet.

The easiest way to fix the test is to unregister 'isSecureArea' flag before setting it. But I think it's not correct because the real problem is registered flags from previous tests.

However, if you want me to add ->unregister I'll do that.

engcom-Charlie · 2020-12-22T13:03:31Z

I found the reason for the failure (https://github.com/magento/magento2/blob/2.4-develop/dev/tests/integration/testsuite/Magento/Customer/_files/customer_rollback.php#L24):
$registry->register('isSecureArea', false);
p.s. There are 398 other test files that use this code too. I believe all of these lines should be removed if test doesn't change it back to unregistered state.

Could you try to fix it? Maybe create the right fixture and use it instead of the old one whenever it needs.

ishakhsuvarov · 2022-10-20T17:11:26Z

@magento run all tests

magento-automated-testing · 2022-10-20T17:11:37Z

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

ihor-sviziev · 2023-04-25T08:38:17Z

Hi @vovayatsyuk,
I see all linked issues are already closed as non-reproducible or already fixed. Could you please update us, does the problem still there?

vovayatsyuk · 2023-04-25T09:01:31Z

Yes, the problem is still here.

…dation-was-failed

ihor-sviziev · 2023-04-25T09:46:24Z

@magento run all tests

magento-automated-testing · 2023-04-25T09:46:34Z

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

ihor-sviziev

Hi @vovayatsyuk,
Please provide more context on when this issue might happen. I see all linked issues already closed.

In the past, I had a similar issue on frontend when the customer was automatically created during the checkout process (using a custom checkout).
But then I I analyzed deeper and it seems like address validation can be simply skipped by adding the following flag ignore_validation_flag to the customer model:

magento2/app/code/Magento/Customer/Model/AccountManagement.php

Lines 1649 to 1658 in 5676386

    
               /** 
        
                * Set ignore_validation_flag for reset password flow to skip unnecessary address and customer validation 
        
                * 
        
                * @param Customer $customer 
        
                * @return void 
        
                */ 
        
               private function setIgnoreValidationFlag($customer) 
        
               { 
        
                   $customer->setData('ignore_validation_flag', true); 
        
               }

magento2/app/code/Magento/Customer/Model/ResourceModel/Customer/Relation.php

Lines 26 to 65 in 5676386

    
           if (!$customer->getData('ignore_validation_flag')) { 
        
               /** @var \Magento\Customer\Model\Address $address */ 
        
               foreach ($customer->getAddresses() as $address) { 
        
                   if ($address->getData('_deleted')) { 
        
                       if ($address->getId() == $defaultBillingId) { 
        
                           $customer->setData('default_billing', null); 
        
                       } 
        
                       if ($address->getId() == $defaultShippingId) { 
        
                           $customer->setData('default_shipping', null); 
        
                       } 
        
                       $removedAddressId = $address->getId(); 
        
                       $address->delete(); 
        
                       // Remove deleted address from customer address collection 
        
                       $customer->getAddressesCollection()->removeItemByKey($removedAddressId); 
        
                   } else { 
        
                       $address->setParentId( 
        
                           $customer->getId() 
        
                       )->setStoreId( 
        
                           $customer->getStoreId() 
        
                       )->setIsCustomerSaveTransaction( 
        
                           true 
        
                       )->save(); 
        
                       if (($address->getIsPrimaryBilling() || 
        
                               $address->getIsDefaultBilling()) && $address->getId() != $defaultBillingId 
        
                       ) { 
        
                           $customer->setData('default_billing', $address->getId()); 
        
                       } 
        
                       if (($address->getIsPrimaryShipping() || 
        
                               $address->getIsDefaultShipping()) && $address->getId() != $defaultShippingId 
        
                       ) { 
        
                           $customer->setData('default_shipping', $address->getId()); 
        
                       } 
        
                   } 
        
               } 
        
           }

magento2/app/code/Magento/Customer/Model/ResourceModel/CustomerRepository.php

Lines 265 to 285 in 5676386

    
           if ($customer->getAddresses() !== null && !$customerModel->getData('ignore_validation_flag')) { 
        
               if ($customer->getId()) { 
        
                   $existingAddresses = $this->getById($customer->getId())->getAddresses(); 
        
                   $getIdFunc = function ($address) { 
        
                       return $address->getId(); 
        
                   }; 
        
                   $existingAddressIds = array_map($getIdFunc, $existingAddresses); 
        
               } else { 
        
                   $existingAddressIds = []; 
        
               } 
        
               $savedAddressIds = []; 
        
               foreach ($customer->getAddresses() as $address) { 
        
                   $address->setCustomerId($customerId) 
        
                       ->setRegion($address->getRegion()); 
        
                   $this->addressRepository->save($address); 
        
                   if ($address->getId()) { 
        
                       $savedAddressIds[] = $address->getId(); 
        
                   } 
        
               } 
        
               $this->deleteAddressesByIds(array_diff($existingAddressIds, $savedAddressIds)); 
        
           }

So having that in mind, it looks like we shouldn't fix this issue because we already have the mechanism for skipping address validation.

vovayatsyuk · 2023-05-25T08:30:12Z

The problem appears when the exception is thrown which leads to the $this->customerRepository->delete($customer); method that is restricted on frontend.

So, yes, if we can guarantee that exception will not be thrown we will never see this error.

Unfortunately, I don't remember why it happened in the first place, maybe it was a custom theme that removed the address form from multishipping registration page.

ihor-sviziev · 2023-05-25T08:49:30Z

@vovayatsyuk, so please double-check it from your side, if adding such a flag fixes the issue for you - I think we can close this Pull Request as fixed. If it's not - please provide some steps to reproduce.

vovayatsyuk · 2023-05-25T08:52:26Z

Ok, let's close this PR

ihor-sviziev · 2023-05-25T08:54:37Z

@vovayatsyuk, thank you so much for contributing and trying to improve Magento! I'm very sorry to not process your pull request for three years 🤦‍♂️

Fixed 'Delete operation is forbidden for current area' error.

46f9fff

This error happens if customer address doesn't pass validation during account creation.

magento-engcom-team added Component: Customer Release Line: 2.4 labels Dec 17, 2020

m2-community-project bot added the Progress: pending review label Dec 17, 2020

rogyar requested changes Dec 17, 2020

View reviewed changes

m2-community-project bot assigned rogyar Dec 17, 2020

m2-community-project bot added Progress: needs update and removed Progress: pending review labels Dec 17, 2020

gabrieldagama added Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Severity: S2 Major restrictions or short-term circumventions are required until a fix is available. labels Dec 17, 2020

engcom-Charlie self-assigned this Dec 21, 2020

Merge branch '2.4-develop' into fix-delete-customer-when-address-vali…

22a409b

…dation-was-failed

Integration test added.

db0cbcd

The test checks if email is still available after failed try with incorrect address.

Improve test: wait for exact exception

1aef161

Added area name to use when running test

35689d6

Do not register 'isSecureArea' flag in the end of test

e0f8f3d

m2-community-project bot added Progress: pending review and removed Progress: review labels Dec 7, 2022

ihor-sviziev assigned ihor-sviziev and unassigned rogyar, gabrieldagama, engcom-Alfa and engcom-Charlie Apr 25, 2023

ihor-sviziev added the Progress: needs update label Apr 25, 2023

m2-community-project bot added Progress: review and removed Progress: pending review labels Apr 25, 2023

Merge branch '2.4-develop' into fix-delete-customer-when-address-vali…

7d1215d

…dation-was-failed

m2-community-project bot added Progress: pending review and removed Progress: needs update Progress: review labels Apr 25, 2023

This comment was marked as duplicate.

Sign in to view

ihor-sviziev requested changes May 25, 2023

View reviewed changes

m2-community-project bot added Progress: needs update and removed Progress: pending review labels May 25, 2023

vovayatsyuk closed this May 25, 2023

m2-community-project bot removed the Progress: needs update label May 25, 2023

	/**
	* Set ignore_validation_flag for reset password flow to skip unnecessary address and customer validation
	*
	* @param Customer $customer
	* @return void
	*/
	private function setIgnoreValidationFlag($customer)
	{
	$customer->setData('ignore_validation_flag', true);
	}

	if (!$customer->getData('ignore_validation_flag')) {
	/** @var \Magento\Customer\Model\Address $address */
	foreach ($customer->getAddresses() as $address) {
	if ($address->getData('_deleted')) {
	if ($address->getId() == $defaultBillingId) {
	$customer->setData('default_billing', null);
	}

	if ($address->getId() == $defaultShippingId) {
	$customer->setData('default_shipping', null);
	}

	$removedAddressId = $address->getId();
	$address->delete();

	// Remove deleted address from customer address collection
	$customer->getAddressesCollection()->removeItemByKey($removedAddressId);
	} else {
	$address->setParentId(
	$customer->getId()
	)->setStoreId(
	$customer->getStoreId()
	)->setIsCustomerSaveTransaction(
	true
	)->save();

	if (($address->getIsPrimaryBilling() \|\|
	$address->getIsDefaultBilling()) && $address->getId() != $defaultBillingId
	) {
	$customer->setData('default_billing', $address->getId());
	}

	if (($address->getIsPrimaryShipping() \|\|
	$address->getIsDefaultShipping()) && $address->getId() != $defaultShippingId
	) {
	$customer->setData('default_shipping', $address->getId());
	}
	}
	}
	}

	if ($customer->getAddresses() !== null && !$customerModel->getData('ignore_validation_flag')) {
	if ($customer->getId()) {
	$existingAddresses = $this->getById($customer->getId())->getAddresses();
	$getIdFunc = function ($address) {
	return $address->getId();
	};
	$existingAddressIds = array_map($getIdFunc, $existingAddresses);
	} else {
	$existingAddressIds = [];
	}
	$savedAddressIds = [];
	foreach ($customer->getAddresses() as $address) {
	$address->setCustomerId($customerId)
	->setRegion($address->getRegion());
	$this->addressRepository->save($address);
	if ($address->getId()) {
	$savedAddressIds[] = $address->getId();
	}
	}
	$this->deleteAddressesByIds(array_diff($existingAddressIds, $savedAddressIds));
	}

Fixed 'Delete operation is forbidden for current area' error. #31318

Fixed 'Delete operation is forbidden for current area' error. #31318

Uh oh!

Conversation

vovayatsyuk commented Dec 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Fixed Issues (if relevant)

Manual testing scenarios

Expected Behavior

Actual Behavior

Contribution checklist (*)

Resolved issues:

Uh oh!

m2-assistant bot commented Dec 17, 2020

Uh oh!

rogyar left a comment

Choose a reason for hiding this comment

Uh oh!

engcom-Charlie commented Dec 21, 2020

Uh oh!

vovayatsyuk commented Dec 21, 2020

Uh oh!

vovayatsyuk commented Dec 21, 2020

Uh oh!

engcom-Charlie commented Dec 22, 2020

Uh oh!

engcom-Charlie commented Dec 22, 2020

Uh oh!

vovayatsyuk commented Dec 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

engcom-Charlie commented Dec 22, 2020

Uh oh!

vovayatsyuk commented Dec 22, 2020

Uh oh!

engcom-Charlie commented Dec 22, 2020

Uh oh!

vovayatsyuk commented Dec 22, 2020

Uh oh!

engcom-Charlie commented Dec 22, 2020

Uh oh!

vovayatsyuk commented Dec 22, 2020

Uh oh!

engcom-Charlie commented Dec 22, 2020

Uh oh!

ishakhsuvarov commented Oct 20, 2022

Uh oh!

magento-automated-testing bot commented Oct 20, 2022

Uh oh!

ihor-sviziev commented Apr 25, 2023

Uh oh!

vovayatsyuk commented Apr 25, 2023

Uh oh!

ihor-sviziev commented Apr 25, 2023

Uh oh!

magento-automated-testing bot commented Apr 25, 2023

Uh oh!

This comment was marked as duplicate.

Uh oh!

ihor-sviziev left a comment

Choose a reason for hiding this comment

Uh oh!

vovayatsyuk commented May 25, 2023

Uh oh!

ihor-sviziev commented May 25, 2023

Uh oh!

vovayatsyuk commented May 25, 2023

Uh oh!

ihor-sviziev commented May 25, 2023

Uh oh!

Uh oh!

vovayatsyuk commented Dec 17, 2020 •

edited

Loading

vovayatsyuk commented Dec 22, 2020 •

edited

Loading