ticgal / mfa

MFA for GLPI
GNU Affero General Public License v3.0
2 stars 3 forks source link

Not Sending Mail Some Times #7

Closed Faizal-Hdn closed 4 months ago

Faizal-Hdn commented 1 year ago

this is emazing for plungin mfa but i'm found problem,

I'm using GLPI 10.0.6 and the plugin mfa latest version it's not sending email codes some time for user i'm configure SMTP mail with google, i'm check notification queue not available for mail on going code sent to user.

any solution for my problem...?

thanks

OscarBeiro commented 1 year ago

Hi, Thanks for your compliments :) First of all, please, update to GLPI 10.0.9. We won't consider any other bug. Then, please confirm you have working notifications. Do you receive any other mail messages from GLPI? Maybe the issue is related to that. Regards,

Faizal-Hdn commented 1 year ago

thanks for your respons,

yes, i configure other notification like new ticket, closing ticket, new followup this everything working perfectly but for notification MFA is some time sent notifcation to user some time not send mail for user.

Was wrong on my case.

im so sory for my english is very bad. :D

fgendorf commented 10 months ago

HI, we are facing the same behavior, and figure out that depends of automatic actions are running, sometimes when automatic actions like UpdatePing, PurgeLogs, etc are running, the cron of GLPI are not multi-thread and the notifications get delayed a lot. The mail OTG should be sent at the moment of authentication, put MFA mail on a notification queue is not the best option.

OscarBeiro commented 10 months ago

Hi, Sure there are better ways to do it, but this is what we were paid to do. If you are ready to sponsor a new feature, let us know. Regards,

fgendorf commented 10 months ago

OK, I found the bug, you use QueuedNotification::forceSendFor to send immediately, but the forceSendFor use the getPendings in the same time that notification was created. The filter of getPendings became:

Array
(
    [FROM] => glpi_queuednotifications
    [WHERE] => Array
        (
            [is_deleted] => 0
            [mode] => TOFILL
            [send_time] => Array
                (
                    [0] => <
                    [1] => 2023-11-30 11:19:50
                )

            [itemtype] => PluginMfaMfa
            [items_id] => 10
        )

    [ORDER] => send_time ASC
    [START] => 0
    [LIMIT] => 1
)

the time of creation of notification on queue was 2023-11-30 11:19:50, so the getPendings try list pendings with < 2023-11-30 11:19:50 but did not return nothing and the notification continues stay on queue, to be processed to next run cron.

The best way to fix this is fixing glpi/src/QueuedNotification.php on line 531, replace '<' to '<='

   $base_query = [
            'FROM'   => self::getTable(),
            'WHERE'  => [
                'is_deleted'   => 0,
                'mode'         => 'TOFILL',
                'send_time'    => ['<=', $send_time],
            ] +  $extra_where,
            'ORDER'  => 'send_time ASC',
            'START'  => 0,
            'LIMIT'  => $limit
        ];

But is possible to fix the plugin just putting a sleep(1) on line 104;

       $mfa = new PluginMfaMfa();
                $mfa->add(['users_id' => Session::getLoginUserID(), 'code' => PluginMfaMfa::getRandomInt(6)]);
                NotificationEvent::raiseEvent('securitycodegenerate', $mfa, ['entities_id' => 0]);
                sleep(1);
                QueuedNotification::forceSendFor($mfa->getType(), $mfa->fields['id']);
            }

I can push a PR if you want.

No need sponsor me, but you can buy me a juice! 😋

OscarBeiro commented 10 months ago

Please make the PR, and we will review it. You have a couple of juices paid whenever you travel to Pontevedra :) Thank you.

OscarBeiro commented 10 months ago

9