magento2: Added encode diacritics breaks on rp_token

hello,

straight off the bat I want to say the “old version” thing is a red herring and I see this with a value in the format 0:3:<base64 string>.

@magento-engcom-team / @engcom-Bravo you are saying you cannot reproduce this issue which demonstrates that you did not first try to understand the bug report.

(a) the field holds encrypted data which means the tokens that people are giving you will result in different binary data within the customer model depending on value of crypt/key - you can’t expect the behaviour to be reproduced by simply entering the values provided by users in this issue without also having their crypt/key which they obviously should not be sharing. i’m not surprised by random users posting these values here as they might not be aware of the inner workings of magento, but it’s a really basic oversight for someone who works for adobe

(b) the issue is obviously not going to be reproducible every time since the data being validated are by definition random bytes. not all random byte sequences result in iconv heuristics thinking it is a malformed multibyte sequence rather than random binary data. you would need to be doing password reset more than 10 times and trying to save the customer between each attempt to be guaranteed to reproduce the bug. if you don’t understand, i’ve provided an 3v4l which demonstrates the principle

© even considering the above it is conceptually trivial to understand that the “text” validator should not be used on arbitrary binary data such as random keys, it is not necessary to attempt to reproduce the bug to grasp that the core has bad behaviour

I reckon a dev rather than a tester needs to have a proper look at this. For now the patch in Text provided by @mischabraam is sufficient to workaround the bug. But there is a lot more to consider for a proper fix:

• from the perspective of the core, the frontend model of this being hidden is essentially a text type eav attribute which is not conceptually the same as binary data and shouldn’t be using the text validateValue function anyway - it should be encouraged to have a custom model used which overrides this function to return true when there is no possible validation to be made
• hidden in general automatically becoming Text when the backend_type is static is a bit lame anyway, because the type of the column on the entity table could be anything. failures_num is a hidden “static” field which is a smallint but being validated as if it’s text
• iconv can throw an E_NOTICE error but there is no try/catch block around the call to the function which is why the error message is opaque - in case of text where there actually is malformed UTF-8 data being provided, it would actually be a useful validation result to say so
• I don’t think this transliterate diacritic step is actually necessary because the strlen check uses mb_strlen which is appropriate for if the string contains diacritics. However this may be an issue of backwards compatibility being broken for custom input rules which may not use multibyte functions - and I think it’s fair enough if you don’t expect the ecosystem of third party module providers to properly understand this issue
• In the specific case of rp_token, the transliterate step happens before calling validateLength and validateInputRule functions. Those functions both check $attribute->getValidateRules() before doing anything. In the case of rp_token, that is an empty array, so no further processing actually takes place and the validation always returns successful. So there is an opportunity to do an early return here whilst retaining the transliteration function for strings where it is actually required. personally I’ve patched it like this:

--- Model/Attribute/Data/Text.php	2023-07-06 14:19:18.505501382 +0100
+++ Model/Attribute/Data/Text.php	2023-07-06 14:11:21.857329218 +0100
@@ -79,6 +79,10 @@
             return $errors;
         }
 
+        if (empty($attribute->getValidateRules())) {
+            return true;
+        }
+
         // if string with diacritics encode it.
         $value = $this->encodeDiacritics($value);

this is a general performance microimprovement which I think fixes the bug as a side effect, and there still needs to be a more comprehensive investigation into the whole system design here.

cheers, M

I was able to make the issue go away by modifying _beforeSave() in Magento\Customer\Model\ResourceModel\Customer. I just moved this:

    if (!$customer->getData('ignore_validation_flag')) {
        $this->_validate($customer);
    }

so that it is called after the next block:

     if ($customer->getData('rp_token')) {
        $rpToken = $customer->getData('rp_token');
        $customer->setRpToken($this->encryptor->encrypt($rpToken));
    }

That next block sets the token to a serialized string, which isn’t going to contain any diacritics, and therefore won’t cause any issues when iconv() is called against it.

Thank @jonathanribas . I think we can close this issue

Totally agree with the above, I lost the energy to write a similar comment before because I don’t think anyone who works for Adobe will be able to understand it.

TBH I would be surprised if there is not some kind of security bug introduced by the allow_diacritics “”“feature”“” - it means that the email address as seen by the validator is different than the e-mail address which gets stored; and it would allow arbitrary binary bytes to be inserted to the email column so long as they don’t throw an error during iconv/translit. It’s hard to imagine there is not some kind of CVE that fall out of the back of “we allow binary data in an email column”.

But who cares, I’m approaching the point where I’m thinking about becoming abusive on this repo so I get banned and I can tell my clients “I’m literally unable to work with Magento because I am banned from the project by Adobe”. Then I will never have to think about it ever again.

We were facing this issue as well. One oddity we faced was that the issue was not reproducible in any other environment (staging or local) using an exact database dump and the same code base.

Our solution was to nullify all rp_token and rp_token_create_at values in the customer_entity table.

Ok, I have a project with this issue. I have not been able to exactly track the origin of the issue. But I have a working DB dump you can use to trace and fix it. One side note: this specific project is a Magento Commerce project, so you’ll need a license for this. (I’ve tried to downscale the DB to OpenSource but haven’t succeeded in that yet).

Steps to get it working.

• Install Magento Commerce 2.4.6.
• Replace the created DB with the dump to find below.
• Replace the config.php with the one to find below. (Not sure if this is required)
• Run bin/magento set:up and bin/magento inde:rein. (You’ll need a working Elasticsearch instance running on localhost:9200 or change the config)
• Use Postman to GET a customer to verify it’s working.
• Use Postman to PUT a change to that customer to see the error.

magento-commerce.sql.gz

config.php.zip

https://api.postman.com/collections/371535-4de80353-35ea-458b-8b7a-28e9f3e7e776?access_key=PMAT-01GYYHFGFWDH6E7WN6ZQZSYQGN

@cschuler-mocap Yep that is correct. But the customers in my case aren’t that old 😉, I mean not coming from Magento1. This project started with Magento 2.3.5-p1 and periodically upgraded to the current version 2.4.6. Since we upgraded from 2.4.5-p1 to 2.4.6 we are having these issues. So, new customers created with 2.4.6 can be updated using the REST API (or admin grid, see @jeffyl 's comment). But, customers created with 2.4.5-p1 or an older Magento version can NOT be updated.

@engcom-Bravo did you test correct?

• You should install Magento 2.4.5
• Create a customer, and use it (so activate its account, do some stuff like password resets and such, maybe order something… dunno)
• Upgrade Magento to 2.4.6
• Update the customer using the admin grid or REST API

EDIT: I see that in my initial comment I didn’t mention this, excuse moi for that.

I’ve just run into this issue myself. Not even using the API, just trying to edit the customer in the Magento backend will fail. Logs for that:

[2023-04-03T20:37:52.428761+00:00] main.CRITICAL: Exception message: Notice: iconv(): Detected an incomplete multibyte character in input string in /home/forge/magento.store/vendor/magento/module-eav/Model/Attribute/Data/Text.php on line 190
Trace: <pre>#1 iconv() called at [vendor/magento/module-eav/Model/Attribute/Data/Text.php:190]
#2 Magento\Eav\Model\Attribute\Data\Text->encodeDiacritics() called at [vendor/magento/module-eav/Model/Attribute/Data/Text.php:83]
#3 Magento\Eav\Model\Attribute\Data\Text->validateValue() called at [vendor/magento/module-eav/Model/Validator/Attribute/Data.php:141]
#4 Magento\Eav\Model\Validator\Attribute\Data->isValid() called at [vendor/magento/framework/Validator/Constraint.php:51]
#5 Magento\Framework\Validator\Constraint->isValid() called at [vendor/magento/framework/Validator.php:61]
#6 Magento\Framework\Validator->isValid() called at [vendor/magento/module-customer/Model/ResourceModel/Customer.php:261]
#7 Magento\Customer\Model\ResourceModel\Customer->_validate() called at [vendor/magento/module-customer/Model/ResourceModel/Customer.php:194]
#8 Magento\Customer\Model\ResourceModel\Customer->_beforeSave() called at [vendor/magento/module-eav/Model/Entity/VersionControl/AbstractEntity.php:90]
#9 Magento\Eav\Model\Entity\VersionControl\AbstractEntity->save() called at [vendor/magento/framework/Interception/Interceptor.php:58]
#10 Magento\Customer\Model\ResourceModel\Customer\Interceptor->___callParent() called at [vendor/magento/framework/Interception/Interceptor.php:138]
#11 Magento\Customer\Model\ResourceModel\Customer\Interceptor->Magento\Framework\Interception\{closure}() called at [vendor/magento/module-checkout/Model/Plugin/RecollectQuoteOnCustomerGroupChange.php:77]
#12 Magento\Checkout\Model\Plugin\RecollectQuoteOnCustomerGroupChange->aroundSave() called at [vendor/magento/framework/Interception/Interceptor.php:135]
#13 Magento\Customer\Model\ResourceModel\Customer\Interceptor->Magento\Framework\Interception\{closure}() called at [vendor/magento/framework/Interception/Interceptor.php:153]
#14 Magento\Customer\Model\ResourceModel\Customer\Interceptor->___callPlugins() called at [generated/code/Magento/Customer/Model/ResourceModel/Customer/Interceptor.php:104]
#15 Magento\Customer\Model\ResourceModel\Customer\Interceptor->save() called at [vendor/magento/framework/Model/AbstractModel.php:663]
#16 Magento\Framework\Model\AbstractModel->save() called at [vendor/magento/framework/Interception/Interceptor.php:58]
#17 Magento\Customer\Model\Backend\Customer\Interceptor->___callParent() called at [vendor/magento/framework/Interception/Interceptor.php:138]
#18 Magento\Customer\Model\Backend\Customer\Interceptor->Magento\Framework\Interception\{closure}() called at [vendor/magento/framework/Interception/Interceptor.php:153]
#19 Magento\Customer\Model\Backend\Customer\Interceptor->___callPlugins() called at [generated/code/Magento/Customer/Model/Backend/Customer/Interceptor.php:23]
#20 Magento\Customer\Model\Backend\Customer\Interceptor->save() called at [vendor/magento/module-customer/Model/ResourceModel/CustomerRepository.php:257]
#21 Magento\Customer\Model\ResourceModel\CustomerRepository->save() called at [vendor/magento/framework/Interception/Interceptor.php:58]
#22 Magento\Customer\Model\ResourceModel\CustomerRepository\Interceptor->___callParent() called at [vendor/magento/framework/Interception/Interceptor.php:138]
#23 Magento\Customer\Model\ResourceModel\CustomerRepository\Interceptor->Magento\Framework\Interception\{closure}() called at [vendor/magento/module-customer/Model/Plugin/CustomerRepository/TransactionWrapper.php:44]
#24 Magento\Customer\Model\Plugin\CustomerRepository\TransactionWrapper->aroundSave() called at [vendor/magento/framework/Interception/Interceptor.php:135]
#25 Magento\Customer\Model\ResourceModel\CustomerRepository\Interceptor->Magento\Framework\Interception\{closure}() called at [vendor/magento/framework/Interception/Interceptor.php:153]
#26 Magento\Customer\Model\ResourceModel\CustomerRepository\Interceptor->___callPlugins() called at [generated/code/Magento/Customer/Model/ResourceModel/CustomerRepository/Interceptor.php:23]
#27 Magento\Customer\Model\ResourceModel\CustomerRepository\Interceptor->save() called at [vendor/magento/module-customer/Controller/Adminhtml/Index/Save.php:382]
#28 Magento\Customer\Controller\Adminhtml\Index\Save->execute() called at [vendor/magento/framework/Interception/Interceptor.php:58]
#29 Magento\Customer\Controller\Adminhtml\Index\Save\Interceptor->___callParent() called at [vendor/magento/framework/Interception/Interceptor.php:138]
#30 Magento\Customer\Controller\Adminhtml\Index\Save\Interceptor->Magento\Framework\Interception\{closure}() called at [vendor/magento/framework/Interception/Interceptor.php:153]
#31 Magento\Customer\Controller\Adminhtml\Index\Save\Interceptor->___callPlugins() called at [generated/code/Magento/Customer/Controller/Adminhtml/Index/Save/Interceptor.php:23]
#32 Magento\Customer\Controller\Adminhtml\Index\Save\Interceptor->execute() called at [vendor/magento/framework/App/Action/Action.php:111]
#33 Magento\Framework\App\Action\Action->dispatch() called at [vendor/magento/module-backend/App/AbstractAction.php:151]
#34 Magento\Backend\App\AbstractAction->dispatch() called at [vendor/magento/framework/Interception/Interceptor.php:58]
#35 Magento\Customer\Controller\Adminhtml\Index\Save\Interceptor->___callParent() called at [vendor/magento/framework/Interception/Interceptor.php:138]
#36 Magento\Customer\Controller\Adminhtml\Index\Save\Interceptor->Magento\Framework\Interception\{closure}() called at [vendor/magento/module-backend/App/Action/Plugin/Authentication.php:145]
#37 Magento\Backend\App\Action\Plugin\Authentication->aroundDispatch() called at [vendor/magento/framework/Interception/Interceptor.php:135]
#38 Magento\Customer\Controller\Adminhtml\Index\Save\Interceptor->Magento\Framework\Interception\{closure}() called at [vendor/magento/framework/Interception/Interceptor.php:153]
#39 Magento\Customer\Controller\Adminhtml\Index\Save\Interceptor->___callPlugins() called at [generated/code/Magento/Customer/Controller/Adminhtml/Index/Save/Interceptor.php:32]
#40 Magento\Customer\Controller\Adminhtml\Index\Save\Interceptor->dispatch() called at [vendor/magento/framework/App/FrontController.php:245]
#41 Magento\Framework\App\FrontController->getActionResponse() called at [vendor/magento/framework/App/FrontController.php:212]
#42 Magento\Framework\App\FrontController->processRequest() called at [vendor/magento/framework/App/FrontController.php:147]
#43 Magento\Framework\App\FrontController->dispatch() called at [vendor/magento/framework/Interception/Interceptor.php:58]
#44 Magento\Framework\App\FrontController\Interceptor->___callParent() called at [vendor/magento/framework/Interception/Interceptor.php:138]
#45 Magento\Framework\App\FrontController\Interceptor->Magento\Framework\Interception\{closure}() called at [vendor/magento/framework/Interception/Interceptor.php:153]
#46 Magento\Framework\App\FrontController\Interceptor->___callPlugins() called at [generated/code/Magento/Framework/App/FrontController/Interceptor.php:23]
#47 Magento\Framework\App\FrontController\Interceptor->dispatch() called at [vendor/magento/framework/App/Http.php:116]
#48 Magento\Framework\App\Http->launch() called at [generated/code/Magento/Framework/App/Http/Interceptor.php:23]
#49 Magento\Framework\App\Http\Interceptor->launch() called at [vendor/magento/framework/App/Bootstrap.php:264]
#50 Magento\Framework\App\Bootstrap->run() called at [pub/index.php:30]

The original problem (https://github.com/magento/magento2/issues/12075) was that valid emails got flagged as invalid if they contained a diacritic.

This was because the Magento custom email validator was overwriting the validateInternationalizedLocalPart of Zend email validator to always return false:

    protected function validateInternationalizedLocalPart($localPart)
    {
        return false;
    }

In this MR this overwrite has been removed and the old Zend email validator was replaced with a new Laminas email validator: https://github.com/magento/magento2/commit/f9bfcb172ff2b31ac0f5aa2186732ea5f3046017#diff-339a22f694ac5f034a3bcfce9b64a2ac1026112da8487f85f2866fa8279ec51d

So now there is a workaround for a broken workaround for a bug that does not even exist anymore. Email validation by default should be able to handle diacritics just fine.

These two commits just add unnecessary complexity to solve a problem that has already been solved elsewhere: https://github.com/magento/magento2/pull/36027/commits/f10dc0483dae38eca7c20540e04e8e23329f3884 https://github.com/magento/magento2/commit/b2f10630626a162934398e4d8225d08d4f38fa70

Can’t believe this issue has been closed. @mischabraam can you reopen it?

Ran into this with sporadic customer saves on a site recently updated to 2.4.6-p3, also caused by false positives/attempts to convert rp_token binary values.

Same issue on current project 2.4.5-p5

Can confirm https://github.com/magento/magento2/commit/b2f10630626a162934398e4d8225d08d4f38fa70 works so far. I am using it as a patch to 2.4.6-p2

Ah, a workaround was made here: https://github.com/magento/magento2/commit/b2f10630626a162934398e4d8225d08d4f38fa70 which was merged to 2.4-develop here https://github.com/magento/magento2/commit/eb34e3477994257e0601bd1a92a04f92152c1919 on September 1st. So hopefully this will drop in the next release.

It uses the patch suggested in this thread of not calling encode diacritics, unfortunately ignoring the advice where translit shouldn’t be called anyway - the appropriate bug fix for #12075 was actually just to remove an override on Symfony\Mailer (from memory) - but that’s a discussion for another thread I suppose. For now this is fine closed as it is worked around.

Hello, In my case I needed to transfer the ecommerce data to a new clean M2.4.6 install and forgot to update the encryption key in env.php based on the first install. that fixed the problem. Good day

@sonbui00, I see but rp_token should have a few days validity for security reasons.

Verified the issue while upgrading from Magento 2.4.5 to Magento 2.4.6 instance and the issue is not reproducible.

@engcom-Bravo Did you try setting the rp_token to f37ce941d293e34d25e635ee0b0d6f16?

As I mentioned in a previous comment, the issue here is that the code is calling the validator:

    if (!$customer->getData('ignore_validation_flag')) {
        $this->_validate($customer);
    }

before calling the function to encrpyt the rp_token:

     if ($customer->getData('rp_token')) {
        $rpToken = $customer->getData('rp_token');
        $customer->setRpToken($this->encryptor->encrypt($rpToken));
    }

Which means that the validator is is attempting to do a character conversion on raw binary data. And I think this might be why it is hard to reproduce – the contents of that binary data are entirely random, and only certain combinations of random bytes cause an issue for the character conversion function. I had to test a dozen different customer’s rp_tokens before I found one that caused the error.

Could the failure to reproduce this error be caused by the fact that it seems to only happen on older customers that were converted from Magento V1 to V2? mischabraam mentioned that he was having this problem with customers that came from earlier versions of Magento, and the customer who brought error this to my attention was an older customer who had been converted from V1.

If I look at the rp_token field on older customers, I see a hex value, which looks something like this: b62cb2863a181348b584453c9c8d11d2

But on newer customers, the value has been encrypted differently: 0:3:pquLCjwnjqXRPAIrH8yFvgUv8IIknfEy4oN7ysOqFW5n0xJ+qXY+G8Vwdlofa/CHWAoNRCLmpRmT6RxS

Maybe this is what is causing the issue to only affect older customers?