Prestashop: Payment module restrictions suddenly stop working due to ghost employee in FO context

[Hlavtox]

Describe the bug
Hi, I noticed that restricting payment methods stopped working all of our Prestashop 1.7.6.2 installs. It always shows all payment modules.

The cause

• I traced the issue to hook:getHookModuleExecList();
• It checks, if it's FO or BO by checking if isset($context->employee) and it only makes the payment restrictions, if it is on front end. (Employee is not set.)
• I don't know why, but in all our shops, some ghost employee got created in the context, with no id and no email. - https://github.com/PrestaShop/PrestaShop/files/4031515/file.txt

Solution
I changed isset($context->employee) to isset($context->employee) && !empty($context->employee->id) and it solved the issue. Is this an acceptable solution worth implementing?

Additional information
PrestaShop version: 1.7.6.2
PHP version: 7.1

[khouloudbelguith]

Hi @Hlavtox,

I did not manage to reproduce the issue with PS1.7.6.2
I attached a screen record
https://drive.google.com/file/d/1HAlNXveWXJM5lfwvtOsg7b5XV3xtD3U1/view

Thanks to check & feedback.

[Hlavtox]

@khouloudbelguith It works fine in clean install, but doesn't work in my case.

Do you want me to provide you with access rights to debug it?

[khouloudbelguith]

Could you please provide us with more info? We need more details to understand how we can reproduce your issue:

• server setup and configuration
• PrestaShop version (source)
• debug mode report
• PHP error logs
• apache error log
• javascript console log

Don't you know how to get this information? Please read the following article:
http://build.prestashop.com/howtos/misc/how-to-create-bug-report/

Thanks!

[Hlavtox]

@khouloudbelguith I traced the bug to the source. Country restrictions does not work, currency restriction does not work, carrier restriction does not work.

The problematic code is in Hook:getHookModuleExecList();

It badly detects front end:

if (isset($context->employee)) {
    $frontend = false;
} else {

Then all the checks don't get applied:

 } elseif ($frontend) {
    // For payment modules, we check that they are available in the contextual country
    if (Validate::isLoadedObject($context->country)) {
    ......

[khouloudbelguith]

@Hlavtox, We need to retrieve the PHP error log and the debug mode report in order to find out what's wrong.
Don't you know how to get this information? Please read the following article:
http://build.prestashop.com/howtos/misc/how-to-create-bug-report/

Thanks!

[Hlavtox]

@khouloudbelguith

1. There are no errors.
2. There are no errors in PHP error log.
3. The code does what it's supposed to do.

It check's by looking to $context->employee if the hook is loaded from the front to make checks module access to carrier, currency and countries. But in my case there is some (ghost?) employee.

This is the dump of $context->employee:
file.txt

[khouloudbelguith]

@Hlavtox, this is a fresh install? or an upgrade from a previous PrestaShop version?
If yes, it is an upgrade? what is the exact version of the 1-click upgrade did you use? have you any errors during the upgrade?
Thanks to provide me errors in the Project_Folder/var/logs & log.txt file, available in<admin folder>/autoupgrade/tmp/log.txt

Thanks!

[Hlavtox]

@khouloudbelguith

• This is an upgrade from 1.7.6.0. to 1.7.6.2
• 1-Click Upgrade v4.10.0
• Symfony log for last 2 months in the attachment, but there is nothing.

prod.log

I added && !empty($context->employee->id) to check if it's a real user and not some empty ghost, but maybe it would be good to check, how this situation can be created.

Can you ping someone? I would make a PR if it's an acceptable fix.

[Hlavtox]

@khouloudbelguith I updated the issue description. Thank you.

[khouloudbelguith]

@Hlavtox, yesterday, you have those errors.

[2020-01-07 12:26:40] request.INFO: Matched route "admin_system_information_check_files". {"route":"admin_system_information_check_files","route_parameters":{"_controller":"PrestaShopBundle\Controller\Admin\Configure\AdvancedParameters\SystemInformationController::displayCheckFilesAction","_legacy_controller":"AdminInformation","_legacy_link":"AdminInformation:checkFiles","_route":"admin_system_information_check_files"},"request_uri":"https://eshop-teka.cz/admin258ytdyrs/index.php/configure/advanced/system-information/files?_token=","method":"POST"} []
[2020-01-07 12:26:40] security.DEBUG: Read existing security token from the session. {"key":"_security_main","token_class":"Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken"} []
[2020-01-07 12:26:40] security.DEBUG: User was reloaded from a user provider. {"provider":"PrestaShopBundle\Security\Admin\EmployeeProvider","username":"REMOVED"} []
[2020-01-07 12:26:40] request.CRITICAL: Uncaught PHP Exception Symfony\Component\Debug\Exception\FatalThrowableError: "Parse error: syntax error, unexpected '='" at /srv/www/eshop-teka.cz/public/www/src/Adapter/Requirement/CheckMissingOrUpdatedFiles.php line 60 {"exception":"[object] (Symfony\Component\Debug\Exception\FatalThrowableError(code: 0): Parse error: syntax error, unexpected '=' at /srv/www/eshop-teka.cz/public/www/src/Adapter/Requirement/CheckMissingOrUpdatedFiles.php:60)"} []
[2020-01-07 12:26:40] security.DEBUG: Stored the security token in the session. {"key":"_security_main"} []

In my case, after an upgrade to PS1.7.6.2 using the 1-click upgrade v4.10.1(new release)
=> OK

But in my case there is some (ghost?) employee.

I need to reproduce this issue, why there are ghost employees in your case?
The only solution to create employees in from the BO => Advanced Parameters => Team => Employees.

Those employees in your case, they have a SuperAdmin profile?

Thanks!

[Hlavtox]

@khouloudbelguith The employee in the context has no ID, no name, no email, no nothing.
https://github.com/PrestaShop/PrestaShop/files/4031515/file.txt

This is a question for the programming team, how something like this can happen.

[khouloudbelguith]

@Hlavtox, this issue occurs only after the upgrade?
Can you please provide us the log of your upgrade? It can be found in the log.txt file, available in <admin folder>/autoupgrade/tmp/log.txt
It could be a problem with the employee table, could you please try to re-create it.
Creating a backup is necessary for your safety.
Thanks!

[khouloudbelguith]

Hi @Hlavtox,

Since we had no news from you for more than 30 days, I'll close this ticket. Feel free to open another one if you can give specific details.

Thanks!

[Hlavtox]

@khouloudbelguith Can you please reopen this and ping someone from the core team?

I am 100% sure it has nothing to do with my employees.

[khouloudbelguith]

Hi @Hlavtox,

I re-opened the issue.

some ghost employee got created in the context, with no id and no email

I need to reproduce this issue.
Thanks to provide the exact steps to get ghost employees created.

Thanks!

[Hlavtox]

@khouloudbelguith I don't know, but I have 9 Prestashop 1.7.6.2 installs and it happened on every single one of them.

I am currently searching in modules, if any module does something with employees.

[khouloudbelguith]

@Hlavtox, thanks!
Waiting for your feedback.

[Hlavtox]

@khouloudbelguith I found out that the cause of this issue was a bad-written module.

I have searched the code and there are multiple places, where Prestashop detects, if it's BO or FO by checking the $context->employee variable.

Would you ask the developers, if it would be good to add additional checks to these places? Or to change the way it detects the FO and BO. Or, it's a user's problem if he installs a malfunctioning module.

Thank you for your time.

[khouloudbelguith]

@Hlavtox,

the cause of this issue was a bad-written module.

So, this issue is caused by a custom module?
What is the exact name of this module?

Thanks!

[Hlavtox]

@khouloudbelguith Yes. This module is not on PS Addons.

I asked the developer and he said that the line.
$this->context->employee = new Employee(0);
was there, because getPriceStatic in Product.php somehow did not work properly in older versions of Prestashop without this.

Nobody noticed this bug in his module in years, because no other restrictions than carrier|payment are much used, and carrier|payment had to be solved by external modules in 1.6, so it was not affected much by this.

[khouloudbelguith]

@Hlavtox, thanks!

I asked the developer and he said that the line.
$this->context->employee = new Employee(0);

This line is added by your module so I think, it is a module issue.

Would you ask the developers, if it would be good to add additional checks to these places? Or to change the way it detects the FO and BO. Or, it's a user's problem if he installs a malfunctioning module.

Ping @PrestaShop/prestashop-core-developers what do you think?

Thanks!

[Hlavtox]

@khouloudbelguith Yes, it's definitely a module issue. The question is, should Prestashop somehow protect itself against the module behavior? :-)

[khouloudbelguith]

@Hlavtox, so I need to close this issue since it is a module issue.
I just pinged our devs to answer this question.

What do you think?

Thanks!

[Hlavtox]

@khouloudbelguith Let's get feedback from the developer team and then you can close it. Thank you for your time. 🌹

[Hlavtox]

Bump @matks, should I make a PR with the fix I proposed in my first comment, or we close this?
(The problem is, that badly written module can affect the behavior of important stuff.)