diff --git a/application/common/components/Emailer.php b/application/common/components/Emailer.php index 850f5966..f45a4011 100644 --- a/application/common/components/Emailer.php +++ b/application/common/components/Emailer.php @@ -194,6 +194,11 @@ protected function assertConfigIsValid() /** * Use the email service to send an email. * + * WARNING: + * You probably shouldn't be calling this directly. Instead, call the + * `sendMessageTo()` method so that the sending of this email will be + * logged. + * * @param string $toAddress The recipient's email address. * @param string $subject The subject. * @param string $htmlBody The email body (as HTML). @@ -261,6 +266,13 @@ protected function getSubjectForMessage(string $messageType, array $data): strin { $subject = $this->subjects[$messageType] ?? ''; + if (empty($subject)) { + \Yii::error(sprintf( + 'No subject known for %s email messages.', + $messageType + )); + } + foreach ($data as $key => $value) { if (is_scalar($value)) { $subject = str_replace('{' . $key . '}', $value, $subject); @@ -313,6 +325,7 @@ public function init() EmailLog::MESSAGE_TYPE_MFA_RATE_LIMIT => $this->subjectForMfaRateLimit, EmailLog::MESSAGE_TYPE_PASSWORD_CHANGED => $this->subjectForPasswordChanged, EmailLog::MESSAGE_TYPE_WELCOME => $this->subjectForWelcome, + EmailLog::MESSAGE_TYPE_ABANDONED_USERS => $this->subjectForAbandonedUsers, EmailLog::MESSAGE_TYPE_EXT_GROUP_SYNC_ERRORS => $this->subjectForExtGroupSyncErrors, EmailLog::MESSAGE_TYPE_GET_BACKUP_CODES => $this->subjectForGetBackupCodes, EmailLog::MESSAGE_TYPE_REFRESH_BACKUP_CODES => $this->subjectForRefreshBackupCodes, @@ -341,7 +354,7 @@ public function init() } /** - * Send the specified type of message to the given User. + * Send the specified type of message to the given User (or non-User address). * * @param string $messageType The message type. Must be one of the * EmailLog::MESSAGE_TYPE_* values. @@ -385,7 +398,9 @@ public function sendMessageTo( $this->email($toAddress, $subject, $htmlBody, strip_tags($htmlBody), $ccAddress, $bccAddress, $delaySeconds); if ($user !== null) { - EmailLog::logMessage($messageType, $user->id); + EmailLog::logMessageToUser($messageType, $user->id); + } else { + EmailLog::logMessageToNonUser($messageType, $toAddress); } } @@ -410,14 +425,13 @@ public function sendDelayedMfaRelatedEmails() } /** - * * Whether the user has already been sent this type of email in the last X days * * @param int $userId * @param string $messageType * @return bool */ - public function hasReceivedMessageRecently($userId, string $messageType) + public function hasUserReceivedMessageRecently(int $userId, string $messageType): bool { $latestEmail = EmailLog::find()->where(['user_id' => $userId, 'message_type' => $messageType]) ->orderBy('sent_utc DESC')->one(); @@ -428,6 +442,49 @@ public function hasReceivedMessageRecently($userId, string $messageType) return MySqlDateTime::dateIsRecent($latestEmail->sent_utc, $this->emailRepeatDelayDays); } + /** + * Whether the non-user address has already been sent this type of email in the last X days + * + * @param string $emailAddress + * @param string $messageType + * @return bool + */ + public function hasNonUserReceivedMessageRecently(string $emailAddress, string $messageType): bool + { + $latestEmail = EmailLog::find()->where([ + 'message_type' => $messageType, + 'non_user_address' => $emailAddress, + 'user_id' => null, + ])->orderBy( + 'sent_utc DESC' + )->one(); + if (empty($latestEmail)) { + return false; + } + + return MySqlDateTime::dateIsRecent($latestEmail->sent_utc, $this->emailRepeatDelayDays); + } + + /** + * Whether we should send an abandoned-users message to HR. + * + * @return bool + */ + public function shouldSendAbandonedUsersMessage(): bool + { + if (empty($this->hrNotificationsEmail)) { + return false; + } + + $haveSentAbandonedUsersEmailRecently = $this->hasNonUserReceivedMessageRecently( + $this->hrNotificationsEmail, + EmailLog::MESSAGE_TYPE_ABANDONED_USERS + ); + + return !$haveSentAbandonedUsersEmailRecently; + } + + /** * Whether we should send an invite message to the given User. * @@ -486,7 +543,7 @@ public function shouldSendGetBackupCodesMessageTo($user) return $this->sendGetBackupCodesEmails && $user->getVerifiedMfaOptionsCount() === 1 && !$user->hasMfaBackupCodes() - && !$this->hasReceivedMessageRecently($user->id, EmailLog::MESSAGE_TYPE_GET_BACKUP_CODES); + && !$this->hasUserReceivedMessageRecently($user->id, EmailLog::MESSAGE_TYPE_GET_BACKUP_CODES); } /** @@ -513,7 +570,7 @@ public function shouldSendLostSecurityKeyMessageTo($user) return false; } - if ($this->hasReceivedMessageRecently($user->id, EmailLog::MESSAGE_TYPE_LOST_SECURITY_KEY)) { + if ($this->hasUserReceivedMessageRecently($user->id, EmailLog::MESSAGE_TYPE_LOST_SECURITY_KEY)) { return false; } @@ -652,7 +709,7 @@ public function sendMethodReminderEmails() foreach ($methods as $method) { $user = $method->user; if (!MySqlDateTime::dateIsRecent($method->created, 3) && - !$this->hasReceivedMessageRecently($user->id, EmailLog::MESSAGE_TYPE_METHOD_REMINDER) + !$this->hasUserReceivedMessageRecently($user->id, EmailLog::MESSAGE_TYPE_METHOD_REMINDER) ) { $this->sendMessageTo( EmailLog::MESSAGE_TYPE_METHOD_REMINDER, @@ -698,7 +755,7 @@ public function sendPasswordExpiringEmails() $passwordExpiry = strtotime($userPassword->getExpiresOn()); if ($passwordExpiry < strtotime(self::PASSWORD_EXPIRING_CUTOFF) && !($passwordExpiry < time()) - && !$this->hasReceivedMessageRecently($user->id, EmailLog::MESSAGE_TYPE_PASSWORD_EXPIRING) + && !$this->hasUserReceivedMessageRecently($user->id, EmailLog::MESSAGE_TYPE_PASSWORD_EXPIRING) ) { $this->sendMessageTo(EmailLog::MESSAGE_TYPE_PASSWORD_EXPIRING, $user); $numEmailsSent++; @@ -740,7 +797,7 @@ public function sendPasswordExpiredEmails() $passwordExpiry = strtotime($userPassword->getExpiresOn()); if ($passwordExpiry < time() && $passwordExpiry > strtotime(self::PASSWORD_EXPIRED_CUTOFF) - && !$this->hasReceivedMessageRecently($user->id, EmailLog::MESSAGE_TYPE_PASSWORD_EXPIRED) + && !$this->hasUserReceivedMessageRecently($user->id, EmailLog::MESSAGE_TYPE_PASSWORD_EXPIRED) ) { $this->sendMessageTo(EmailLog::MESSAGE_TYPE_PASSWORD_EXPIRED, $user); $numEmailsSent++; @@ -801,9 +858,16 @@ public function sendAbandonedUsersEmail() $dataForEmail['users'] = User::getAbandonedUsers(); if (!empty($dataForEmail['users'])) { - $htmlBody = \Yii::$app->view->render('@common/mail/abandoned-users.html.php', $dataForEmail); - - $this->email($this->hrNotificationsEmail, $this->subjectForAbandonedUsers, $htmlBody, strip_tags($htmlBody)); + if ($this->shouldSendAbandonedUsersMessage()) { + $this->sendMessageTo( + EmailLog::MESSAGE_TYPE_ABANDONED_USERS, + null, + ArrayHelper::merge( + $dataForEmail, + ['toAddress' => $this->hrNotificationsEmail] + ) + ); + } } } } diff --git a/application/common/models/EmailLog.php b/application/common/models/EmailLog.php index a2e82068..9b8a98cc 100644 --- a/application/common/models/EmailLog.php +++ b/application/common/models/EmailLog.php @@ -3,6 +3,7 @@ namespace common\models; use common\helpers\MySqlDateTime; +use ReflectionClass; use Yii; use yii\helpers\ArrayHelper; @@ -17,6 +18,7 @@ class EmailLog extends EmailLogBase public const MESSAGE_TYPE_MFA_RATE_LIMIT = 'mfa-rate-limit'; public const MESSAGE_TYPE_PASSWORD_CHANGED = 'password-changed'; public const MESSAGE_TYPE_WELCOME = 'welcome'; + public const MESSAGE_TYPE_ABANDONED_USERS = 'abandoned-users'; public const MESSAGE_TYPE_EXT_GROUP_SYNC_ERRORS = 'ext-group-sync-errors'; public const MESSAGE_TYPE_GET_BACKUP_CODES = 'get-backup-codes'; public const MESSAGE_TYPE_REFRESH_BACKUP_CODES = 'refresh-backup-codes'; @@ -41,35 +43,23 @@ public function attributeLabels() { return ArrayHelper::merge(parent::attributeLabels(), [ 'sent_utc' => Yii::t('app', 'Sent (UTC)'), + 'non_user_address' => Yii::t('app', 'Non-User Address'), ]); } - public static function getMessageTypes() + public static function getMessageTypes(): array { - return [ - self::MESSAGE_TYPE_INVITE, - self::MESSAGE_TYPE_MFA_RATE_LIMIT, - self::MESSAGE_TYPE_PASSWORD_CHANGED, - self::MESSAGE_TYPE_WELCOME, - self::MESSAGE_TYPE_GET_BACKUP_CODES, - self::MESSAGE_TYPE_REFRESH_BACKUP_CODES, - self::MESSAGE_TYPE_LOST_SECURITY_KEY, - self::MESSAGE_TYPE_MFA_OPTION_ADDED, - self::MESSAGE_TYPE_MFA_OPTION_REMOVED, - self::MESSAGE_TYPE_MFA_ENABLED, - self::MESSAGE_TYPE_MFA_DISABLED, - self::MESSAGE_TYPE_METHOD_VERIFY, - self::MESSAGE_TYPE_METHOD_REMINDER, - self::MESSAGE_TYPE_METHOD_PURGED, - self::MESSAGE_TYPE_MFA_MANAGER, - self::MESSAGE_TYPE_MFA_MANAGER_HELP, - self::MESSAGE_TYPE_PASSWORD_EXPIRING, - self::MESSAGE_TYPE_PASSWORD_EXPIRED, - self::MESSAGE_TYPE_PASSWORD_PWNED, - ]; + $reflectionClass = new ReflectionClass(__CLASS__); + $messageTypes = []; + foreach ($reflectionClass->getConstants() as $name => $value) { + if (str_starts_with($name, 'MESSAGE_TYPE_')) { + $messageTypes[] = $value; + } + } + return $messageTypes; } - public static function logMessage(string $messageType, $userId) + public static function logMessageToUser(string $messageType, $userId) { $emailLog = new EmailLog([ 'user_id' => $userId, @@ -88,6 +78,25 @@ public static function logMessage(string $messageType, $userId) } } + public static function logMessageToNonUser(string $messageType, string $emailAddress) + { + $emailLog = new EmailLog([ + 'non_user_address' => $emailAddress, + 'message_type' => $messageType, + ]); + + if (!$emailLog->save()) { + $errorMessage = sprintf( + 'Failed to log %s email to %s: %s', + var_export($messageType, true), + var_export($emailAddress, true), + \json_encode($emailLog->getFirstErrors(), JSON_PRETTY_PRINT) + ); + \Yii::warning($errorMessage); + throw new \Exception($errorMessage, 1730909932); + } + } + /** * {@inheritdoc} */ diff --git a/application/common/models/EmailLogBase.php b/application/common/models/EmailLogBase.php index c7a8ede9..7ffe51ab 100644 --- a/application/common/models/EmailLogBase.php +++ b/application/common/models/EmailLogBase.php @@ -8,9 +8,10 @@ * This is the model class for table "email_log". * * @property int $id - * @property int $user_id + * @property int|null $user_id * @property string|null $message_type * @property string $sent_utc + * @property string|null $non_user_address * * @property User $user */ @@ -30,10 +31,11 @@ public static function tableName() public function rules() { return [ - [['user_id', 'sent_utc'], 'required'], [['user_id'], 'integer'], [['message_type'], 'string'], + [['sent_utc'], 'required'], [['sent_utc'], 'safe'], + [['non_user_address'], 'string', 'max' => 255], [['user_id'], 'exist', 'skipOnError' => true, 'targetClass' => User::class, 'targetAttribute' => ['user_id' => 'id']], ]; } @@ -48,6 +50,7 @@ public function attributeLabels() 'user_id' => Yii::t('app', 'User ID'), 'message_type' => Yii::t('app', 'Message Type'), 'sent_utc' => Yii::t('app', 'Sent Utc'), + 'non_user_address' => Yii::t('app', 'Non User Address'), ]; } diff --git a/application/console/migrations/m241029_200547_add_email_log_message_types.php b/application/console/migrations/m241029_200547_add_email_log_message_types.php new file mode 100644 index 00000000..b80f728e --- /dev/null +++ b/application/console/migrations/m241029_200547_add_email_log_message_types.php @@ -0,0 +1,27 @@ +alterColumn( + '{{email_log}}', + 'message_type', + "enum('invite','welcome','mfa-rate-limit','password-changed','get-backup-codes','refresh-backup-codes','lost-security-key','mfa-option-added','mfa-option-removed','mfa-enabled','mfa-disabled','method-verify','mfa-manager','mfa-manager-help','method-reminder','method-purged','password-expiring','password-expired','password-pwned','ext-group-sync-errors','abandoned-users') null" + ); + } + + public function safeDown() + { + $this->alterColumn( + '{{email_log}}', + 'message_type', + "enum('invite','welcome','mfa-rate-limit','password-changed','get-backup-codes','refresh-backup-codes','lost-security-key','mfa-option-added','mfa-option-removed','mfa-enabled','mfa-disabled','method-verify','mfa-manager','mfa-manager-help','method-reminder','method-purged','password-expiring','password-expired','password-pwned') null" + ); + } +} diff --git a/application/console/migrations/m241106_155829_enable_logging_non_user_emails.php b/application/console/migrations/m241106_155829_enable_logging_non_user_emails.php new file mode 100644 index 00000000..65e8adda --- /dev/null +++ b/application/console/migrations/m241106_155829_enable_logging_non_user_emails.php @@ -0,0 +1,42 @@ +addColumn( + '{{email_log}}', + 'non_user_address', + $this->string()->null() + ); + $this->alterColumn( + '{{email_log}}', + 'user_id', + $this->integer()->null() + ); + } + + public function safeDown() + { + $this->delete( + '{{email_log}}', + [ + 'user_id' => null, + ] + ); + $this->alterColumn( + '{{email_log}}', + 'user_id', + $this->integer()->notNull() + ); + $this->dropColumn( + '{{email_log}}', + 'non_user_address' + ); + } +} diff --git a/application/features/bootstrap/EmailContext.php b/application/features/bootstrap/EmailContext.php index f7c89f94..262c28b3 100644 --- a/application/features/bootstrap/EmailContext.php +++ b/application/features/bootstrap/EmailContext.php @@ -783,7 +783,10 @@ public function iCheckIfAMfaDisabledEmailShouldBeSent() public function iCheckIfAGetBackupCodesEmailHasBeenSentRecently() { $messageType = EmailLog::MESSAGE_TYPE_GET_BACKUP_CODES; - $this->getBackupCodesEmailHasBeenSent = $this->fakeEmailer->hasReceivedMessageRecently($this->tempUser, $messageType); + $this->getBackupCodesEmailHasBeenSent = $this->fakeEmailer->hasUserReceivedMessageRecently( + $this->tempUser->id, + $messageType + ); } /** @@ -952,7 +955,10 @@ public function iSeeThatARefreshBackupCodesEmailShouldBeSent() */ public function iSeeThatTheFirstUserHasReceivedALostSecurityKeyEmail() { - Assert::true($this->fakeEmailer->hasReceivedMessageRecently($this->tempUser->id, EmailLog::MESSAGE_TYPE_LOST_SECURITY_KEY)); + Assert::true($this->fakeEmailer->hasUserReceivedMessageRecently( + $this->tempUser->id, + EmailLog::MESSAGE_TYPE_LOST_SECURITY_KEY + )); } /** @@ -960,7 +966,10 @@ public function iSeeThatTheFirstUserHasReceivedALostSecurityKeyEmail() */ public function iSeeThatTheSecondUserHasReceivedAGetBackupCodesEmail() { - Assert::true($this->fakeEmailer->hasReceivedMessageRecently($this->tempUser2->id, EmailLog::MESSAGE_TYPE_GET_BACKUP_CODES)); + Assert::true($this->fakeEmailer->hasUserReceivedMessageRecently( + $this->tempUser2->id, + EmailLog::MESSAGE_TYPE_GET_BACKUP_CODES + )); } /** @@ -968,7 +977,10 @@ public function iSeeThatTheSecondUserHasReceivedAGetBackupCodesEmail() */ public function iSeeThatTheFirstUserHasNotReceivedALostSecurityKeyEmail() { - Assert::false($this->fakeEmailer->hasReceivedMessageRecently($this->tempUser->id, EmailLog::MESSAGE_TYPE_LOST_SECURITY_KEY)); + Assert::false($this->fakeEmailer->hasUserReceivedMessageRecently( + $this->tempUser->id, + EmailLog::MESSAGE_TYPE_LOST_SECURITY_KEY + )); } /** @@ -976,7 +988,10 @@ public function iSeeThatTheFirstUserHasNotReceivedALostSecurityKeyEmail() */ public function iSeeThatTheSecondUserHasNotReceivedAGetBackupCodesEmail() { - Assert::false($this->fakeEmailer->hasReceivedMessageRecently($this->tempUser2->id, EmailLog::MESSAGE_TYPE_GET_BACKUP_CODES)); + Assert::false($this->fakeEmailer->hasUserReceivedMessageRecently( + $this->tempUser2->id, + EmailLog::MESSAGE_TYPE_GET_BACKUP_CODES + )); } /** @@ -1108,7 +1123,7 @@ public function iSendRecoveryMethodReminderEmails() */ public function iSeeThatARecoveryMethodReminderHasNotBeenSent($hasOrHasNot) { - $hasBeenSent = $this->fakeEmailer->hasReceivedMessageRecently( + $hasBeenSent = $this->fakeEmailer->hasUserReceivedMessageRecently( $this->tempUser->id, EmailLog::MESSAGE_TYPE_METHOD_REMINDER ); @@ -1240,15 +1255,8 @@ public function iSendAbandonedUserEmail() */ public function theAbandonedUserEmailHasOrHasNotBeenSent($hasOrHasNot) { - $emails = $this->fakeEmailer->getFakeEmailsSent(); - $hasBeenSent = false; - - foreach ($emails as $email) { - if ($email[Emailer::PROP_SUBJECT] === $this->fakeEmailer->subjectForAbandonedUsers) { - $hasBeenSent = true; - break; - } - } + $numberSent = $this->countEmailsSent(EmailLog::MESSAGE_TYPE_ABANDONED_USERS); + $hasBeenSent = ($numberSent > 0); if ($hasOrHasNot === 'has') { Assert::true($hasBeenSent); @@ -1257,6 +1265,20 @@ public function theAbandonedUserEmailHasOrHasNotBeenSent($hasOrHasNot) } } + protected function countEmailsSent(string $messageType): int + { + $emails = $this->fakeEmailer->getFakeEmailsSent(); + $actualCount = 0; + + foreach ($emails as $email) { + $subject = $email[Emailer::PROP_SUBJECT]; + if ($this->fakeEmailer->isSubjectForMessageType($subject, $messageType)) { + $actualCount++; + } + } + return $actualCount; + } + /** * @Given the database has been purged */ diff --git a/application/features/bootstrap/fakes/FakeEmailer.php b/application/features/bootstrap/fakes/FakeEmailer.php index ce73f776..eb7603af 100644 --- a/application/features/bootstrap/fakes/FakeEmailer.php +++ b/application/features/bootstrap/fakes/FakeEmailer.php @@ -64,8 +64,11 @@ public function getFakeEmailsSent() return $this->getEmailServiceClient()->emailsSent; } - public function isSubjectForMessageType(string $subject, string $messageType, ?User $user) - { + public function isSubjectForMessageType( + string $subject, + string $messageType, + ?User $user = null + ): bool { $dataForEmail = ArrayHelper::merge( $user ? $user->getAttributesForEmail() : [], $this->otherDataForEmails