Merge pull request #394 from silinternational/feature/improved-msg-pr…

…ocessing Release 7.0.2 -- reduce memory consumption of password expiry processing
silinternational · Feb 12, 2025 · 42d8ebe · 42d8ebe
2 parents f23ac7b + 8a5b4e5
commit 42d8ebe
Show file tree

Hide file tree

Showing 4 changed files with 61 additions and 8 deletions.
diff --git a/application/common/components/Emailer.php b/application/common/components/Emailer.php
@@ -776,10 +776,10 @@ public function sendPasswordExpiringEmails()
             'status' => 'starting',
         ];
 
-        $users = User::getActiveUnlockedUsers();
+        $users = User::getUsersForEmail('password-expiring', $this->emailRepeatDelayDays);
 
         $this->logger->info(array_merge($logData, [
-            'active_users' => count($users)
+            'users' => count($users)
         ]));
 
         $numEmailsSent = 0;
@@ -790,7 +790,6 @@ public function sendPasswordExpiringEmails()
                 $passwordExpiry = strtotime($userPassword->getExpiresOn());
                 if ($passwordExpiry < strtotime(self::PASSWORD_EXPIRING_CUTOFF)
                     && !($passwordExpiry < time())
-                    && !$this->hasUserReceivedMessageRecently($user->id, EmailLog::MESSAGE_TYPE_PASSWORD_EXPIRING)
                 ) {
                     $this->sendMessageTo(EmailLog::MESSAGE_TYPE_PASSWORD_EXPIRING, $user);
                     $numEmailsSent++;
@@ -818,10 +817,10 @@ public function sendPasswordExpiredEmails()
             'status' => 'starting',
         ];
 
-        $users = User::getActiveUnlockedUsers();
+        $users = User::getUsersForEmail('password-expired', $this->emailRepeatDelayDays);
 
         $this->logger->info(array_merge($logData, [
-            'active_users' => count($users)
+            'users' => count($users)
         ]));
 
         $numEmailsSent = 0;
@@ -832,7 +831,6 @@ public function sendPasswordExpiredEmails()
                 $passwordExpiry = strtotime($userPassword->getExpiresOn());
                 if ($passwordExpiry < time()
                     && $passwordExpiry > strtotime(self::PASSWORD_EXPIRED_CUTOFF)
-                    && !$this->hasUserReceivedMessageRecently($user->id, EmailLog::MESSAGE_TYPE_PASSWORD_EXPIRED)
                 ) {
                     $this->sendMessageTo(EmailLog::MESSAGE_TYPE_PASSWORD_EXPIRED, $user);
                     $numEmailsSent++;

diff --git a/application/common/models/User.php b/application/common/models/User.php
@@ -1581,4 +1581,36 @@ public static function exportToSheets()
             'status' => 'finish',
         ]);
     }
+
+    /*
+     * Returns a list of active, unlocked users that haven't recently received a given email message.
+     * @param $template Email template name to use as search criteria
+     * @param $days Number of days to consider an email sent recently
+     * @return User[]
+     */
+    public static function getUsersForEmail(string $template, int $days): array
+    {
+        $usersArray = \Yii::$app->getDb()->createCommand("SELECT u.*
+            FROM `user` u
+                LEFT JOIN `email_log` e ON u.id = e.user_id
+                    AND e.message_type = :template
+                    AND e.sent_utc >= CURRENT_DATE() - INTERVAL :days DAY
+            WHERE u.active = 'yes'
+                AND u.locked = 'no'
+                AND e.id IS NULL
+            GROUP BY u.id
+            HAVING COUNT(*) = 1;")
+            ->bindValue('template', $template)
+            ->bindValue('days', $days)
+            ->queryAll();
+
+        $users = [];
+        foreach ($usersArray as $userData) {
+            $user = new User();
+            User::populateRecord($user, $userData);
+            $users[] = $user;
+        }
+
+        return $users;
+    }
 }
diff --git a/application/console/controllers/CronController.php b/application/console/controllers/CronController.php
@@ -2,12 +2,12 @@
 
 namespace console\controllers;
 
+use common\components\Emailer;
 use common\components\ExternalGroupsSync;
 use common\models\Invite;
 use common\models\Method;
 use common\models\Mfa;
 use common\models\User;
-use common\components\Emailer;
 use yii\console\Controller;
 
 class CronController extends Controller
@@ -86,14 +86,15 @@ public function actionAll()
             'actionSendAbandonedUsersEmail',
             'actionSendDelayedMfaRelatedEmails',
             'actionSendMethodReminderEmails',
-            'actionSendPasswordExpiryEmails',
             'actionSyncExternalGroups',
         ];
 
         if (\Yii::$app->params['google']['enableSheetsExport']) {
             $actions[] = 'actionExportToSheets';
         }
 
+        $actions[] = 'actionSendPasswordExpiryEmails';
+
         foreach ($actions as $action) {
             try {
                 $this->$action();

diff --git a/application/features/email.feature b/application/features/email.feature
@@ -365,6 +365,7 @@ Feature: Email
   Scenario Outline:  When to send password expiring notice email
     Given we are configured <toSendOrNot> password expiring emails
       And I remove records of any emails that have been sent
+      And no mfas exist
       And a user already exists
       And that user has a password that expires in <number> days
       And a "password-expiring" email <hasOrHasNot> been sent to that user
@@ -383,9 +384,30 @@ Feature: Email
       | to send      | 15      | has          | should NOT     |
       | NOT to send  | 15      | has NOT      | should NOT     |
 
+  Scenario Outline:  When to send password expiring notice email for a user with MFA enabled
+    Given we are configured <toSendOrNot> password expiring emails
+    And the database has been purged
+    And a user already exists
+    And that user has a password that expires in <number> days
+    And a totp mfa option does exist
+    And a "password-expiring" email <hasOrHasNot> been sent to that user
+    When I send password expiring emails
+    Then a "password-expiring" email <shouldOrNot> have been sent to them
+
+    Examples:
+      | toSendOrNot | number | hasOrHasNot | shouldOrNot |
+      | to send     | -1462  | has NOT     | should NOT  |
+      | to send     | -1461  | has NOT     | should      |
+      | to send     | -1447  | has NOT     | should      |
+      | to send     | -1446  | has NOT     | should NOT  |
+      | to send     | -1461  | has         | should NOT  |
+      | to send     | -1447  | has         | should NOT  |
+      | NOT to send | -1447  | has NOT     | should NOT  |
+
   Scenario Outline:  When to send password expired notice email
     Given we are configured <toSendOrNot> password expired emails
       And I remove records of any emails that have been sent
+      And no mfas exist
       And a user already exists
       And that user has a password that expires in <number> days
       And a "password-expired" email <hasOrHasNot> been sent to that user