validation of unique email when creating a user

[vitek-rostislav]

What is happening

What is wrong in code

The UniqueEmail constraint accepts domainId parameter that is taken from the form's options. The problem is that admin can change the domain when creating a new user and hence the validation can fail on false positive result, or even worse, the validation may pass but the request fails on duplicate email exception.

Steps to reproduce - scenario 1

• go to admin/customer/new/
• change domain to non-default value
• fill in an existing email from the first domain (e.g. "no-reply@shopsys.com")
• fill in other required fields
• submit form
• You see a validation error that the email already is already registered (even though it is not registered on the second domain)

Steps to reproduce - scenario 2

• go to admin/customer/new/
• change domain to non-default value
• fill in an existing email from the first domain (e.g. "no-reply.2@shopsys.com")
• fill in other required fields
• submit form
• You see error 500

Expected result

• Validation of the email uniqueness is fixed - UniqueEmail constraint should not be probably used in the admin form at all. Instead, it is possible to use Callback constraint where the submitted data (i.e. actually selected domain) can be accessed. It should look something like that:

'constraints' => [
                    new Callback([$this, 'validateUniqueEmail']),
                ],

...

public function validateUniqueLevel(string $email, ExecutionContextInterface $context): void
    {
        /** @var \Symfony\Component\Form\Form $form */
        $form = $context->getRoot();
        /** @var Shopsys\FrameworkBundle\Model\Customer\User\CustomerUserData $customerUserData */
        $customerUserData = $form->getData();

        $domainId = $customerUserData->domainId;
        if ($this->customerUserFacade->findCustomerUserByEmailAndDomain($email, $domainId) !== null) { 
            $context->addViolation('The email is already registered on given domain.')
        }
    }

• Also, the framework should be examined for similar constraints that should take the form changes into account

[stale]

This issue has been automatically marked as stale because there was no activity within the last 4 months (and it is quite a long time). It will be closed if no further activity occurs. Thank you for your contributions.

[henzigo]

I see that PR #1868 is merged. @TomasLudvik can you confirm that this Issue is already solved?

[s3tezsky]

I see that PR #1868 is merged. @TomasLudvik can you confirm that this Issue is already solved?

It is because the PR is not merged into master yet. The issue will be closed when it is done.

[TomasLudvik]

@henzigo thank you for pointing out, you are right, this has been already fixed
@s3tezsky it is already merged, from 7.3 to master, problem is, that because of changes in User / CustomerUser namespaces Github does not know that