From dab3b6ced0bb24268e3b0d00bb8a4dbcfb2db173 Mon Sep 17 00:00:00 2001 From: Michal Kleiner Date: Wed, 26 Mar 2025 17:05:39 +0100 Subject: [PATCH 01/26] Add initial code for a mechanism to remind users to rotate personal auth tokens --- config/global.ini.php | 4 + .../AuthTokenNotification.php | 79 +++++++++++ .../AuthTokenNotifierTask.php | 86 +++++++++++ .../AuthTokenProviderInterface.php | 23 +++ plugins/UsersManager/AuthTokenProvider.php | 76 ++++++++++ .../Emails/AuthTokenNotificationEmail.php | 134 ++++++++++++++++++ plugins/UsersManager/Model.php | 5 + plugins/UsersManager/lang/en.json | 9 +- .../_authTokenNotificationHtmlEmail.twig | 7 + .../_authTokenNotificationTextEmail.twig | 15 ++ 10 files changed, 437 insertions(+), 1 deletion(-) create mode 100644 plugins/UsersManager/AuthTokenNotifications/AuthTokenNotification.php create mode 100644 plugins/UsersManager/AuthTokenNotifications/AuthTokenNotifierTask.php create mode 100644 plugins/UsersManager/AuthTokenNotifications/AuthTokenProviderInterface.php create mode 100644 plugins/UsersManager/AuthTokenProvider.php create mode 100644 plugins/UsersManager/Emails/AuthTokenNotificationEmail.php create mode 100644 plugins/UsersManager/templates/_authTokenNotificationHtmlEmail.twig create mode 100644 plugins/UsersManager/templates/_authTokenNotificationTextEmail.twig diff --git a/config/global.ini.php b/config/global.ini.php index f89f2148f8b..fbbda97d7fa 100644 --- a/config/global.ini.php +++ b/config/global.ini.php @@ -561,6 +561,10 @@ ; Recommended for best security. only_allow_secure_auth_tokens = 0 +; Number of days after which a personal auth token is recommended to be rotated +; and an email notification will be sent to the user +auth_token_rotation_notification_days = 180 + ; language cookie name for session language_cookie_name = matomo_lang diff --git a/plugins/UsersManager/AuthTokenNotifications/AuthTokenNotification.php b/plugins/UsersManager/AuthTokenNotifications/AuthTokenNotification.php new file mode 100644 index 00000000000..7499dd02da2 --- /dev/null +++ b/plugins/UsersManager/AuthTokenNotifications/AuthTokenNotification.php @@ -0,0 +1,79 @@ +tokenId = $tokenId; + $this->tokenName = $tokenName; + $this->tokenCreationDate = $tokenCreationDate; + $this->login = $login; + $this->email = $email; + $this->onNotificationSent = $onNotificationSent; + } + + public function getTokenName(): string + { + return $this->tokenName; + } + + public function getTokenCreationDate(): string + { + return $this->tokenCreationDate; + } + + public function sendNotification(): void + { + // send email + $email = StaticContainer::getContainer()->make( + AuthTokenNotificationEmail::class, + [ + 'notification' => $this, + 'rotationPeriodDays' => Config::getInstance()->General['auth_token_rotation_notification_days'], + ] + ); + $email->safeSend(); + + if (is_callable($this->onNotificationSent)) { + call_user_func($this->onNotificationSent, $this->tokenId); + } + } +} diff --git a/plugins/UsersManager/AuthTokenNotifications/AuthTokenNotifierTask.php b/plugins/UsersManager/AuthTokenNotifications/AuthTokenNotifierTask.php new file mode 100644 index 00000000000..7ea326d8798 --- /dev/null +++ b/plugins/UsersManager/AuthTokenNotifications/AuthTokenNotifierTask.php @@ -0,0 +1,86 @@ +findComponents( + 'AuthTokenProvider', + AuthTokenProviderInterface::class + ); + + /** @var AuthTokenProvider $provider */ + foreach ($providers as $provider) { + array_push($tokensToNotify, ...$provider->getAuthTokensToNotify()); + } + + return $tokensToNotify; + } + + /** + * Attempts to download new location & ISP GeoIP databases and + * replace the existing ones w/ them. + */ + public function sendEmails() + { + try { + Option::set(self::LAST_RUN_TIME_OPTION_NAME, Date::factory('today')->getTimestamp()); + + $tokensToNotify = $this->getAuthTokensToNotify(); + + // notification emails should be using `safeSend()` method so we don't do try/catch here + foreach ($tokensToNotify as $tokenToNotify) { + $tokenToNotify->sendNotification(); + } + + // reschedule for next run + /** @var Scheduler $scheduler */ + $scheduler = StaticContainer::getContainer()->get(Scheduler::class); + // reschedule to ensure it's not run again in an hour + $scheduler->rescheduleTask(new AuthTokenNotifierTask()); + + } catch (Exception $ex) { + // message will already be prefixed w/ 'GeoIP2AutoUpdater: ' + Log::error($ex); + throw $ex; + } + } +} diff --git a/plugins/UsersManager/AuthTokenNotifications/AuthTokenProviderInterface.php b/plugins/UsersManager/AuthTokenNotifications/AuthTokenProviderInterface.php new file mode 100644 index 00000000000..2143b4406ef --- /dev/null +++ b/plugins/UsersManager/AuthTokenNotifications/AuthTokenProviderInterface.php @@ -0,0 +1,23 @@ +userModel = new UserModel(); + $this->today = Date::factory('today')->getDatetime(); + } + + private function getRotationPeriodThreshold(): string + { + $periodDays = Config::getInstance()->General['auth_token_rotation_notification_days']; + return Date::factory('today')->subDay($periodDays)->getDateTime(); + } + + public function setTokenNotified(string $tokenId): void + { + $this->userModel->setRotationNotificationWasSentForToken($tokenId, $this->today); + } + + public function getAuthTokensToNotify(): array + { + $db = Db::get(); + $sql = "SELECT * FROM " . Common::prefixTable('user_token_auth') + . " WHERE (date_expired is null or date_expired > ?)" + . " AND (date_created <= ?)" + . " AND ts_rotation_notified is null"; + + $tokensToNotify = $db->fetchAll($sql, [ + $this->today, + $this->getRotationPeriodThreshold() + ]); + + $notifications = []; + + foreach ($tokensToNotify as $t) { + $user = $this->userModel->getUser($t->login); + $email = $user['email']; + + $notifications[] = new AuthTokenNotification( + $t->idusertokenauth, + $t->description, + $t->date_created, + $t->login, + $email, + [static::class, 'setTokenNotified'] + ); + } + + return $notifications; + } +} diff --git a/plugins/UsersManager/Emails/AuthTokenNotificationEmail.php b/plugins/UsersManager/Emails/AuthTokenNotificationEmail.php new file mode 100644 index 00000000000..14790e24b2b --- /dev/null +++ b/plugins/UsersManager/Emails/AuthTokenNotificationEmail.php @@ -0,0 +1,134 @@ +login = $login; + $this->emailAddress = $emailAddress; + $this->notification = $notification; + $this->rotationPeriodDays = $rotationPeriodDays; + + $this->setUpEmail(); + } + + private function setUpEmail(): void + { + $this->setDefaultFromPiwik(); + $this->addTo($this->emailAddress); + $this->setSubject($this->getDefaultSubject()); + $this->addReplyTo($this->getFrom(), $this->getFromName()); + $this->setBodyText($this->getDefaultBodyText()); + $this->setWrappedHtmlBody($this->getDefaultBodyView()); + } + + private function getRotationPeriodPretty(): string + { + $startDate = new \DateTime(); + $endDate = (clone $startDate)->add(new \DateInterval("P{$this->rotationPeriodDays}D")); + $diff = $startDate->diff($endDate); + + $parts = []; + + if ($diff->y > 0) { + $parts[] = $diff->y . + ($diff->y === 1 + ? Piwik::translate('Intl_PeriodYear') + : Piwik::translate('Intl_PeriodYears') + ); + } + if ($diff->m > 0) { + $parts[] = $diff->m . + ($diff->m === 1 + ? Piwik::translate('Intl_PeriodMonth') + : Piwik::translate('Intl_PeriodMonths') + ); + } + // Only include days if they're not zero OR if there are no years/months + if ($diff->d > 0 || empty($parts)) { + $parts[] = $diff->d . + ($diff->d === 1 + ? Piwik::translate('Intl_PeriodDay') + : Piwik::translate('Intl_PeriodDays') + ); + } + + return implode(', ', $parts); + } + + protected function getDefaultSubject(): string + { + return Piwik::translate('UsersManager_AuthTokenNotificationEmailSubject'); + } + + protected function getManageAuthTokensLink(): string + { + return Url::getCurrentUrlWithoutQueryString() + . '?module=UsersManager' + . '&action=userSecurity' + . '#authtokens'; + } + + protected function getDefaultBodyText(): string + { + $view = new View('@UsersManager/_authTokenNotificationTextEmail.twig'); + $view->setContentType('text/plain'); + + $this->assignCommonParameters($view); + + return $view->render(); + } + + protected function getDefaultBodyView(): View + { + $view = new View('@UsersManager/_authTokenNotificationHtmlEmail.twig'); + + $this->assignCommonParameters($view); + + return $view; + } + + protected function assignCommonParameters(View $view): void + { + $view->login = $this->login; + $view->tokenName = $this->notification->getTokenName(); + $view->tokenCreationDate = $this->notification->getTokenCreationDate(); + $view->rotationPeriod = $this->getRotationPeriodPretty(); + $view->manageAuthTokensLink = $this->getManageAuthTokensLink(); + } +} diff --git a/plugins/UsersManager/Model.php b/plugins/UsersManager/Model.php index 7cd7a54ef5b..b36a49a6edc 100644 --- a/plugins/UsersManager/Model.php +++ b/plugins/UsersManager/Model.php @@ -536,6 +536,11 @@ public function setTokenAuthWasUsed($tokenAuth, $dateLastUsed) } } + public function setRotationNotificationWasSentForToken(string $tokenId, string $tsRotation) + { + $this->updateTokenAuthTable($tokenId, ['ts_rotation_notified' => $tsRotation]); + } + private function updateTokenAuthTable($idTokenAuth, $fields) { $set = array(); diff --git a/plugins/UsersManager/lang/en.json b/plugins/UsersManager/lang/en.json index 3b69be1a4b9..495047d9145 100644 --- a/plugins/UsersManager/lang/en.json +++ b/plugins/UsersManager/lang/en.json @@ -237,6 +237,13 @@ "AuthTokenSecureOnlyHelp": "Enable this option to only allow this token to be used in a secure way (e.g. POST requests), this is recommended as a best security practice. The token will then not be valid as a URL parameter in GET requests.", "AuthTokenSecureOnlyHelpForced": "The system administrator has configured Matomo to only allow tokens to be created for use in secure way (e.g. via POST requests), you cannot change this token option.", "OnlyAllowSecureRequests": "Only allow secure requests", - "SecureUseOnly": "Secure use only" + "SecureUseOnly": "Secure use only", + "AuthTokenNotificationEmailSubject": "Reminder: Rotate your Matomo personal access token", + "AuthTokenNotificationEmailReminder": "Reminder: Rotate your auth tokens every %1$s", + "AuthTokenNotificationEmail01": "To improve security, we recommend rotating your personal access tokens every %1$s%2$s%3$s.", + "AuthTokenNotificationEmail02": "You’re receiving this email because your %1$s%2$s%3$s personal access token has not been rotated since %4$s%5$s%6$s.", + "AuthTokenNotificationEmail03": "To generate a new token and delete the old one, go to Personal > Security settings in your Matomo instance.", + "AuthTokenNotificationEmail04": "If you believe you received this email in error, please contact your Matomo administrator.", + "AuthTokenNotificationEmail05": "Manage your access tokens" } } diff --git a/plugins/UsersManager/templates/_authTokenNotificationHtmlEmail.twig b/plugins/UsersManager/templates/_authTokenNotificationHtmlEmail.twig new file mode 100644 index 00000000000..297c0ea99d1 --- /dev/null +++ b/plugins/UsersManager/templates/_authTokenNotificationHtmlEmail.twig @@ -0,0 +1,7 @@ +

{{ 'General_HelloUser'|translate(login) }}

+

{{ 'UsersManager_AuthTokenNotificationEmailReminder'|translate(rotationPeriod) }}

+

{{ 'UsersManager_AuthTokenNotificationEmail01'|translate('', rotationPeriod, '') }}

+

{{ 'UsersManager_AuthTokenNotificationEmail02'|translate('', tokenDescription, '', '', tokenCreationDate, '') }}

+

{{ 'UsersManager_AuthTokenNotificationEmail03'|translate }}

+

{{ 'UsersManager_AuthTokenNotificationEmail04'|translate }}

+{{ 'UsersManager_AuthTokenNotificationEmail05'|translate }} diff --git a/plugins/UsersManager/templates/_authTokenNotificationTextEmail.twig b/plugins/UsersManager/templates/_authTokenNotificationTextEmail.twig new file mode 100644 index 00000000000..2016f501287 --- /dev/null +++ b/plugins/UsersManager/templates/_authTokenNotificationTextEmail.twig @@ -0,0 +1,15 @@ +{% autoescape false %} +{{ 'General_HelloUser'|translate(login) }} + +{{ 'UsersManager_AuthTokenNotificationEmailReminder'|translate(rotationPeriod) }} + +{{ 'UsersManager_AuthTokenNotificationEmail01'|translate('', rotationPeriod, '') }} + +{{ 'UsersManager_AuthTokenNotificationEmail02'|translate('', tokenDescription, '', '', tokenCreationDate, '') }} + +{{ 'UsersManager_AuthTokenNotificationEmail03'|translate }} + +{{ 'UsersManager_AuthTokenNotificationEmail04'|translate }} + +{{ 'UsersManager_AuthTokenNotificationEmail05'|translate }} {{ manageAuthTokensLink }} +{% endautoescape %} From 738b03ce08aca85c718563f2467c1b25ea6d00ff Mon Sep 17 00:00:00 2001 From: Michal Kleiner Date: Wed, 26 Mar 2025 17:12:45 +0100 Subject: [PATCH 02/26] Fix CS --- .../AuthTokenNotifications/AuthTokenNotification.php | 3 +-- .../AuthTokenNotifications/AuthTokenNotifierTask.php | 2 -- .../AuthTokenNotifications/AuthTokenProviderInterface.php | 2 -- 3 files changed, 1 insertion(+), 6 deletions(-) diff --git a/plugins/UsersManager/AuthTokenNotifications/AuthTokenNotification.php b/plugins/UsersManager/AuthTokenNotifications/AuthTokenNotification.php index 7499dd02da2..7d3f755e04a 100644 --- a/plugins/UsersManager/AuthTokenNotifications/AuthTokenNotification.php +++ b/plugins/UsersManager/AuthTokenNotifications/AuthTokenNotification.php @@ -40,8 +40,7 @@ public function __construct( string $login, string $email, callable $onNotificationSent - ) - { + ) { $this->tokenId = $tokenId; $this->tokenName = $tokenName; $this->tokenCreationDate = $tokenCreationDate; diff --git a/plugins/UsersManager/AuthTokenNotifications/AuthTokenNotifierTask.php b/plugins/UsersManager/AuthTokenNotifications/AuthTokenNotifierTask.php index 7ea326d8798..6029d4ac669 100644 --- a/plugins/UsersManager/AuthTokenNotifications/AuthTokenNotifierTask.php +++ b/plugins/UsersManager/AuthTokenNotifications/AuthTokenNotifierTask.php @@ -10,7 +10,6 @@ namespace Piwik\Plugins\UsersManager\AuthTokenNotifications; use Exception; -use MaxMind\Exception\AuthenticationException; use Piwik\Container\StaticContainer; use Piwik\Date; use Piwik\Log; @@ -76,7 +75,6 @@ public function sendEmails() $scheduler = StaticContainer::getContainer()->get(Scheduler::class); // reschedule to ensure it's not run again in an hour $scheduler->rescheduleTask(new AuthTokenNotifierTask()); - } catch (Exception $ex) { // message will already be prefixed w/ 'GeoIP2AutoUpdater: ' Log::error($ex); diff --git a/plugins/UsersManager/AuthTokenNotifications/AuthTokenProviderInterface.php b/plugins/UsersManager/AuthTokenNotifications/AuthTokenProviderInterface.php index 2143b4406ef..bd60058b632 100644 --- a/plugins/UsersManager/AuthTokenNotifications/AuthTokenProviderInterface.php +++ b/plugins/UsersManager/AuthTokenNotifications/AuthTokenProviderInterface.php @@ -9,8 +9,6 @@ namespace Piwik\Plugins\UsersManager\AuthTokenNotifications; -use Piwik\Config; - interface AuthTokenProviderInterface { /** From e45fcf2305fa5a429abd9980ef1811491a20e6af Mon Sep 17 00:00:00 2001 From: Michal Kleiner Date: Fri, 28 Mar 2025 12:30:33 +0100 Subject: [PATCH 03/26] Adjust token provider interface, make it more reusable, adjust responsibilities --- .../UsersManager/AuthTokenNotification.php | 21 ++++ .../AuthTokenNotifierTask.php | 84 ---------------- .../AuthTokenProviderInterface.php | 21 ---- .../Emails/AuthTokenNotificationEmail.php | 34 ++----- .../TokenNotification.php} | 47 ++++----- .../TokenNotificationInterface.php | 23 +++++ .../TokenNotifications/TokenNotifierTask.php | 99 +++++++++++++++++++ .../TokenProviderInterface.php | 29 ++++++ ...uthTokenProvider.php => TokenProvider.php} | 23 ++--- 9 files changed, 212 insertions(+), 169 deletions(-) create mode 100644 plugins/UsersManager/AuthTokenNotification.php delete mode 100644 plugins/UsersManager/AuthTokenNotifications/AuthTokenNotifierTask.php delete mode 100644 plugins/UsersManager/AuthTokenNotifications/AuthTokenProviderInterface.php rename plugins/UsersManager/{AuthTokenNotifications/AuthTokenNotification.php => TokenNotifications/TokenNotification.php} (51%) create mode 100644 plugins/UsersManager/TokenNotifications/TokenNotificationInterface.php create mode 100644 plugins/UsersManager/TokenNotifications/TokenNotifierTask.php create mode 100644 plugins/UsersManager/TokenNotifications/TokenProviderInterface.php rename plugins/UsersManager/{AuthTokenProvider.php => TokenProvider.php} (76%) diff --git a/plugins/UsersManager/AuthTokenNotification.php b/plugins/UsersManager/AuthTokenNotification.php new file mode 100644 index 00000000000..93bed1b9294 --- /dev/null +++ b/plugins/UsersManager/AuthTokenNotification.php @@ -0,0 +1,21 @@ +findComponents( - 'AuthTokenProvider', - AuthTokenProviderInterface::class - ); - - /** @var AuthTokenProvider $provider */ - foreach ($providers as $provider) { - array_push($tokensToNotify, ...$provider->getAuthTokensToNotify()); - } - - return $tokensToNotify; - } - - /** - * Attempts to download new location & ISP GeoIP databases and - * replace the existing ones w/ them. - */ - public function sendEmails() - { - try { - Option::set(self::LAST_RUN_TIME_OPTION_NAME, Date::factory('today')->getTimestamp()); - - $tokensToNotify = $this->getAuthTokensToNotify(); - - // notification emails should be using `safeSend()` method so we don't do try/catch here - foreach ($tokensToNotify as $tokenToNotify) { - $tokenToNotify->sendNotification(); - } - - // reschedule for next run - /** @var Scheduler $scheduler */ - $scheduler = StaticContainer::getContainer()->get(Scheduler::class); - // reschedule to ensure it's not run again in an hour - $scheduler->rescheduleTask(new AuthTokenNotifierTask()); - } catch (Exception $ex) { - // message will already be prefixed w/ 'GeoIP2AutoUpdater: ' - Log::error($ex); - throw $ex; - } - } -} diff --git a/plugins/UsersManager/AuthTokenNotifications/AuthTokenProviderInterface.php b/plugins/UsersManager/AuthTokenNotifications/AuthTokenProviderInterface.php deleted file mode 100644 index bd60058b632..00000000000 --- a/plugins/UsersManager/AuthTokenNotifications/AuthTokenProviderInterface.php +++ /dev/null @@ -1,21 +0,0 @@ -login = $login; - $this->emailAddress = $emailAddress; $this->notification = $notification; - $this->rotationPeriodDays = $rotationPeriodDays; - $this->setUpEmail(); } private function setUpEmail(): void { $this->setDefaultFromPiwik(); - $this->addTo($this->emailAddress); + $this->addTo($this->notification->getEmailAddress()); $this->setSubject($this->getDefaultSubject()); $this->addReplyTo($this->getFrom(), $this->getFromName()); $this->setBodyText($this->getDefaultBodyText()); @@ -59,12 +42,13 @@ private function setUpEmail(): void private function getRotationPeriodPretty(): string { + $rotationPeriodDays = Config::getInstance()->General['auth_token_rotation_notification_days']; + $startDate = new \DateTime(); - $endDate = (clone $startDate)->add(new \DateInterval("P{$this->rotationPeriodDays}D")); + $endDate = (clone $startDate)->add(new \DateInterval("P{$rotationPeriodDays}D")); $diff = $startDate->diff($endDate); $parts = []; - if ($diff->y > 0) { $parts[] = $diff->y . ($diff->y === 1 @@ -125,7 +109,7 @@ protected function getDefaultBodyView(): View protected function assignCommonParameters(View $view): void { - $view->login = $this->login; + $view->login = $this->notification->getLogin(); $view->tokenName = $this->notification->getTokenName(); $view->tokenCreationDate = $this->notification->getTokenCreationDate(); $view->rotationPeriod = $this->getRotationPeriodPretty(); diff --git a/plugins/UsersManager/AuthTokenNotifications/AuthTokenNotification.php b/plugins/UsersManager/TokenNotifications/TokenNotification.php similarity index 51% rename from plugins/UsersManager/AuthTokenNotifications/AuthTokenNotification.php rename to plugins/UsersManager/TokenNotifications/TokenNotification.php index 7d3f755e04a..38a4b0eda66 100644 --- a/plugins/UsersManager/AuthTokenNotifications/AuthTokenNotification.php +++ b/plugins/UsersManager/TokenNotifications/TokenNotification.php @@ -7,13 +7,9 @@ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later */ -namespace Piwik\Plugins\UsersManager\AuthTokenNotifications; +namespace Piwik\Plugins\UsersManager\TokenNotifications; -use Piwik\Config; -use Piwik\Container\StaticContainer; -use Piwik\Plugins\UsersManager\Emails\AuthTokenNotificationEmail; - -final class AuthTokenNotification +abstract class TokenNotification implements TokenNotificationInterface { /** @var string */ private $tokenId; @@ -24,29 +20,29 @@ final class AuthTokenNotification /** @var string */ private $tokenCreationDate; - /** @var string */ - private $login; - /** @var string */ private $email; - /** @var callable */ - private $onNotificationSent; + /** @var string */ + private $login; public function __construct( string $tokenId, string $tokenName, string $tokenCreationDate, - string $login, string $email, - callable $onNotificationSent + string $login ) { $this->tokenId = $tokenId; $this->tokenName = $tokenName; $this->tokenCreationDate = $tokenCreationDate; - $this->login = $login; $this->email = $email; - $this->onNotificationSent = $onNotificationSent; + $this->login = $login; + } + + public function getTokenId(): string + { + return $this->tokenId; } public function getTokenName(): string @@ -59,20 +55,15 @@ public function getTokenCreationDate(): string return $this->tokenCreationDate; } - public function sendNotification(): void + public function getEmailAddress(): string { - // send email - $email = StaticContainer::getContainer()->make( - AuthTokenNotificationEmail::class, - [ - 'notification' => $this, - 'rotationPeriodDays' => Config::getInstance()->General['auth_token_rotation_notification_days'], - ] - ); - $email->safeSend(); + return $this->email; + } + + abstract public function getEmailClass(): string; - if (is_callable($this->onNotificationSent)) { - call_user_func($this->onNotificationSent, $this->tokenId); - } + public function getLogin(): string + { + return $this->login; } } diff --git a/plugins/UsersManager/TokenNotifications/TokenNotificationInterface.php b/plugins/UsersManager/TokenNotifications/TokenNotificationInterface.php new file mode 100644 index 00000000000..4fac598c314 --- /dev/null +++ b/plugins/UsersManager/TokenNotifications/TokenNotificationInterface.php @@ -0,0 +1,23 @@ +findComponents( + 'TokenProvider', + TokenProviderInterface::class + ); + + /** @var TokenProvider $provider */ + foreach ($providers as $provider) { + $tokensToNotifyByProvider[get_class($provider)] = [ + 'tokensToNotify' => $provider->getTokensToNotify(), + 'onTokenNotified' => $provider->onTokenNotified(), + ]; + } + + return $tokensToNotifyByProvider; + } + + private function sendNotificationEmail(TokenNotificationInterface $notification): void + { + $email = StaticContainer::getContainer()->make( + $notification->getEmailClass(), + ['notification' => $notification] + ); + $email->safeSend(); + } + + /** + * Send notification email for each provider and its tokens + */ + public function sendEmails() + { + try { + Option::set(self::LAST_RUN_TIME_OPTION_NAME, Date::factory('today')->getTimestamp()); + + $tokensToNotifyByProvider = $this->getTokensToNotifyByProvider(); + + // we use `safeSend()` method (as above) so we don't need to do try/catch here + foreach ($tokensToNotifyByProvider as $providerTokensToNotify) { + /** @var TokenNotificationInterface $tokenToNotify */ + foreach ($providerTokensToNotify['tokensToNotify'] as $tokenToNotify) { + $this->sendNotificationEmail($tokenToNotify); + call_user_func($providerTokensToNotify['onTokenNotified'], $tokenToNotify->getTokenId()); + } + } + + // reschedule for next run + /** @var Scheduler $scheduler */ + $scheduler = StaticContainer::getContainer()->get(Scheduler::class); + // reschedule to ensure it's not run again in an hour + $scheduler->rescheduleTask(new static()); + } catch (Exception $ex) { + StaticContainer::get(LoggerInterface::class)->error($ex); + throw $ex; + } + } +} diff --git a/plugins/UsersManager/TokenNotifications/TokenProviderInterface.php b/plugins/UsersManager/TokenNotifications/TokenProviderInterface.php new file mode 100644 index 00000000000..4707517c8bf --- /dev/null +++ b/plugins/UsersManager/TokenNotifications/TokenProviderInterface.php @@ -0,0 +1,29 @@ +subDay($periodDays)->getDateTime(); } - public function setTokenNotified(string $tokenId): void - { - $this->userModel->setRotationNotificationWasSentForToken($tokenId, $this->today); - } - - public function getAuthTokensToNotify(): array + public function getTokensToNotify(): array { $db = Db::get(); $sql = "SELECT * FROM " . Common::prefixTable('user_token_auth') @@ -66,11 +60,18 @@ public function getAuthTokensToNotify(): array $t->description, $t->date_created, $t->login, - $email, - [static::class, 'setTokenNotified'] + $email ); } return $notifications; } + + public function onTokenNotified(): callable + { + $that = $this; + return function (string $tokenId) use ($that) { + $that->userModel->setRotationNotificationWasSentForToken($tokenId, $that->today); + }; + } } From 8860c8ac670f5f60396d6d5b346df2de49909278 Mon Sep 17 00:00:00 2001 From: Michal Kleiner Date: Fri, 28 Mar 2025 12:49:23 +0100 Subject: [PATCH 04/26] Mark class as final --- plugins/UsersManager/AuthTokenNotification.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/UsersManager/AuthTokenNotification.php b/plugins/UsersManager/AuthTokenNotification.php index 93bed1b9294..bd7d4ca4a52 100644 --- a/plugins/UsersManager/AuthTokenNotification.php +++ b/plugins/UsersManager/AuthTokenNotification.php @@ -12,7 +12,7 @@ use Piwik\Plugins\UsersManager\Emails\AuthTokenNotificationEmail; use Piwik\Plugins\UsersManager\TokenNotifications\TokenNotification; -class AuthTokenNotification extends TokenNotification +final class AuthTokenNotification extends TokenNotification { public function getEmailClass(): string { From 7f0bf88fef66de75626db08e08f08b2d1124a0ec Mon Sep 17 00:00:00 2001 From: Michal Kleiner Date: Fri, 28 Mar 2025 12:54:34 +0100 Subject: [PATCH 05/26] Add migration to create new column and bump core version --- core/Updates/5.4.0-b1.php | 39 +++++++++++++++++++++++++++++++++++++++ core/Version.php | 2 +- 2 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 core/Updates/5.4.0-b1.php diff --git a/core/Updates/5.4.0-b1.php b/core/Updates/5.4.0-b1.php new file mode 100644 index 00000000000..7a6cdfcfd4c --- /dev/null +++ b/core/Updates/5.4.0-b1.php @@ -0,0 +1,39 @@ +migration = $factory; + } + + public function getMigrations(Updater $updater) + { + return [ + $this->migration->db->addColumn('user_token_auth', 'ts_rotation_notified', 'TIMESTAMP NULL'), + ]; + } + + public function doUpdate(Updater $updater) + { + $updater->executeMigrations(__FILE__, $this->getMigrations($updater)); + } +} diff --git a/core/Version.php b/core/Version.php index 2849cbe079f..aaffb4505e9 100644 --- a/core/Version.php +++ b/core/Version.php @@ -22,7 +22,7 @@ final class Version * The current Matomo version. * @var string */ - public const VERSION = '5.4.0-alpha'; + public const VERSION = '5.4.0-b1'; public const MAJOR_VERSION = 5; From d279dd465e668acd6f165f179b356c1ff8afa6bc Mon Sep 17 00:00:00 2001 From: Michal Kleiner Date: Mon, 31 Mar 2025 15:15:01 +0200 Subject: [PATCH 06/26] Rework responsibilities, abstract token notification for allow other types other than email, simplify token provider, remove callbacks --- ...ion.php => AuthTokenEmailNotification.php} | 4 +- .../Emails/AuthTokenNotificationEmail.php | 59 +++++++----------- .../TokenEmailNotification.php | 61 +++++++++++++++++++ .../TokenNotifications/TokenNotification.php | 24 +------- .../TokenNotificationInterface.php | 4 +- .../TokenNotifications/TokenNotifierTask.php | 47 ++++---------- .../TokenProviderInterface.php | 8 +-- plugins/UsersManager/TokenProvider.php | 13 ++-- 8 files changed, 108 insertions(+), 112 deletions(-) rename plugins/UsersManager/{AuthTokenNotification.php => AuthTokenEmailNotification.php} (72%) create mode 100644 plugins/UsersManager/TokenNotifications/TokenEmailNotification.php diff --git a/plugins/UsersManager/AuthTokenNotification.php b/plugins/UsersManager/AuthTokenEmailNotification.php similarity index 72% rename from plugins/UsersManager/AuthTokenNotification.php rename to plugins/UsersManager/AuthTokenEmailNotification.php index bd7d4ca4a52..1fc67d6ca44 100644 --- a/plugins/UsersManager/AuthTokenNotification.php +++ b/plugins/UsersManager/AuthTokenEmailNotification.php @@ -10,9 +10,9 @@ namespace Piwik\Plugins\UsersManager; use Piwik\Plugins\UsersManager\Emails\AuthTokenNotificationEmail; -use Piwik\Plugins\UsersManager\TokenNotifications\TokenNotification; +use Piwik\Plugins\UsersManager\TokenNotifications\TokenEmailNotification; -final class AuthTokenNotification extends TokenNotification +final class AuthTokenEmailNotification extends TokenEmailNotification { public function getEmailClass(): string { diff --git a/plugins/UsersManager/Emails/AuthTokenNotificationEmail.php b/plugins/UsersManager/Emails/AuthTokenNotificationEmail.php index dd7d4dcd202..48e3448bf03 100644 --- a/plugins/UsersManager/Emails/AuthTokenNotificationEmail.php +++ b/plugins/UsersManager/Emails/AuthTokenNotificationEmail.php @@ -23,17 +23,27 @@ class AuthTokenNotificationEmail extends Mail */ private $notification; - public function __construct(TokenNotification $notification) + /** @var string */ + private $recipient; + + /** @var array */ + private $emailData; + + public function __construct(TokenNotification $notification, string $recipient, array $emailData) { parent::__construct(); + $this->notification = $notification; + $this->recipient = $recipient; + $this->emailData = $emailData; + $this->setUpEmail(); } private function setUpEmail(): void { $this->setDefaultFromPiwik(); - $this->addTo($this->notification->getEmailAddress()); + $this->addTo($this->recipient); $this->setSubject($this->getDefaultSubject()); $this->addReplyTo($this->getFrom(), $this->getFromName()); $this->setBodyText($this->getDefaultBodyText()); @@ -44,40 +54,7 @@ private function getRotationPeriodPretty(): string { $rotationPeriodDays = Config::getInstance()->General['auth_token_rotation_notification_days']; - $startDate = new \DateTime(); - $endDate = (clone $startDate)->add(new \DateInterval("P{$rotationPeriodDays}D")); - $diff = $startDate->diff($endDate); - - $parts = []; - if ($diff->y > 0) { - $parts[] = $diff->y . - ($diff->y === 1 - ? Piwik::translate('Intl_PeriodYear') - : Piwik::translate('Intl_PeriodYears') - ); - } - if ($diff->m > 0) { - $parts[] = $diff->m . - ($diff->m === 1 - ? Piwik::translate('Intl_PeriodMonth') - : Piwik::translate('Intl_PeriodMonths') - ); - } - // Only include days if they're not zero OR if there are no years/months - if ($diff->d > 0 || empty($parts)) { - $parts[] = $diff->d . - ($diff->d === 1 - ? Piwik::translate('Intl_PeriodDay') - : Piwik::translate('Intl_PeriodDays') - ); - } - - return implode(', ', $parts); - } - - protected function getDefaultSubject(): string - { - return Piwik::translate('UsersManager_AuthTokenNotificationEmailSubject'); + return Piwik::translate('Intl_PeriodDay' . ($rotationPeriodDays === 1 ? '' : 's')); } protected function getManageAuthTokensLink(): string @@ -87,6 +64,10 @@ protected function getManageAuthTokensLink(): string . '&action=userSecurity' . '#authtokens'; } + protected function getDefaultSubject(): string + { + return Piwik::translate('UsersManager_AuthTokenNotificationEmailSubject'); + } protected function getDefaultBodyText(): string { @@ -109,10 +90,14 @@ protected function getDefaultBodyView(): View protected function assignCommonParameters(View $view): void { - $view->login = $this->notification->getLogin(); $view->tokenName = $this->notification->getTokenName(); $view->tokenCreationDate = $this->notification->getTokenCreationDate(); + $view->rotationPeriod = $this->getRotationPeriodPretty(); $view->manageAuthTokensLink = $this->getManageAuthTokensLink(); + + foreach ($this->emailData as $item => $value) { + $view->assign($item, $value); + } } } diff --git a/plugins/UsersManager/TokenNotifications/TokenEmailNotification.php b/plugins/UsersManager/TokenNotifications/TokenEmailNotification.php new file mode 100644 index 00000000000..a527ae052de --- /dev/null +++ b/plugins/UsersManager/TokenNotifications/TokenEmailNotification.php @@ -0,0 +1,61 @@ + ['item1' => 'value1'], ...] that will be passed to the email class + * + * @var array + */ + private $emailData; + + public function __construct( + string $tokenId, + string $tokenName, + string $tokenCreationDate, + array $recipients, + array $emailData + ) { + parent::__construct($tokenId, $tokenName, $tokenCreationDate); + + $this->recipients = $recipients; + $this->emailData = $emailData; + } + + abstract public function getEmailClass(): string; + + public function dispatch(): bool + { + foreach ($this->recipients as $recipient) { + $email = StaticContainer::getContainer()->make( + $this->getEmailClass(), + [ + 'notification' => $this, + 'recipient' => $recipient, + 'emailData' => $this->emailData[$recipient] ?? [], + ] + ); + $email->safeSend(); + } + + return true; + } +} diff --git a/plugins/UsersManager/TokenNotifications/TokenNotification.php b/plugins/UsersManager/TokenNotifications/TokenNotification.php index 38a4b0eda66..43421367bd8 100644 --- a/plugins/UsersManager/TokenNotifications/TokenNotification.php +++ b/plugins/UsersManager/TokenNotifications/TokenNotification.php @@ -20,24 +20,14 @@ abstract class TokenNotification implements TokenNotificationInterface /** @var string */ private $tokenCreationDate; - /** @var string */ - private $email; - - /** @var string */ - private $login; - public function __construct( string $tokenId, string $tokenName, - string $tokenCreationDate, - string $email, - string $login + string $tokenCreationDate ) { $this->tokenId = $tokenId; $this->tokenName = $tokenName; $this->tokenCreationDate = $tokenCreationDate; - $this->email = $email; - $this->login = $login; } public function getTokenId(): string @@ -55,15 +45,5 @@ public function getTokenCreationDate(): string return $this->tokenCreationDate; } - public function getEmailAddress(): string - { - return $this->email; - } - - abstract public function getEmailClass(): string; - - public function getLogin(): string - { - return $this->login; - } + abstract public function dispatch(): bool; } diff --git a/plugins/UsersManager/TokenNotifications/TokenNotificationInterface.php b/plugins/UsersManager/TokenNotifications/TokenNotificationInterface.php index 4fac598c314..5fd86e84b9a 100644 --- a/plugins/UsersManager/TokenNotifications/TokenNotificationInterface.php +++ b/plugins/UsersManager/TokenNotifications/TokenNotificationInterface.php @@ -17,7 +17,5 @@ public function getTokenName(): string; public function getTokenCreationDate(): string; - public function getEmailClass(): string; - - public function getEmailAddress(): string; + public function dispatch(): bool; } diff --git a/plugins/UsersManager/TokenNotifications/TokenNotifierTask.php b/plugins/UsersManager/TokenNotifications/TokenNotifierTask.php index 3c25d521ce1..5287e5341c6 100644 --- a/plugins/UsersManager/TokenNotifications/TokenNotifierTask.php +++ b/plugins/UsersManager/TokenNotifications/TokenNotifierTask.php @@ -29,60 +29,35 @@ class TokenNotifierTask extends Task public function __construct() { - // all checks whether emails can be sent are done in the actual Mail class - parent::__construct($this, 'sendEmails', null, new Daily()); + parent::__construct($this, 'dispatchNotifications', null, new Daily()); } /** - * Get a list of providers that may require token notifications being sent - * For each of them get a list of tokens and a callback to call when the notification has been sent. + * Get a list of providers that may require token notifications being dispatched * * @return array */ - private function getTokensToNotifyByProvider(): array + private function getTokenProviders(): array { - $tokensToNotifyByProvider = []; - $providers = PluginManager::getInstance()->findComponents( + return PluginManager::getInstance()->findComponents( 'TokenProvider', TokenProviderInterface::class ); - - /** @var TokenProvider $provider */ - foreach ($providers as $provider) { - $tokensToNotifyByProvider[get_class($provider)] = [ - 'tokensToNotify' => $provider->getTokensToNotify(), - 'onTokenNotified' => $provider->onTokenNotified(), - ]; - } - - return $tokensToNotifyByProvider; - } - - private function sendNotificationEmail(TokenNotificationInterface $notification): void - { - $email = StaticContainer::getContainer()->make( - $notification->getEmailClass(), - ['notification' => $notification] - ); - $email->safeSend(); } /** - * Send notification email for each provider and its tokens + * Dispatch notifications for each provider and its tokens */ - public function sendEmails() + public function dispatchNotifications() { try { Option::set(self::LAST_RUN_TIME_OPTION_NAME, Date::factory('today')->getTimestamp()); - $tokensToNotifyByProvider = $this->getTokensToNotifyByProvider(); - - // we use `safeSend()` method (as above) so we don't need to do try/catch here - foreach ($tokensToNotifyByProvider as $providerTokensToNotify) { - /** @var TokenNotificationInterface $tokenToNotify */ - foreach ($providerTokensToNotify['tokensToNotify'] as $tokenToNotify) { - $this->sendNotificationEmail($tokenToNotify); - call_user_func($providerTokensToNotify['onTokenNotified'], $tokenToNotify->getTokenId()); + /** @var TokenProviderInterface $provider */ + foreach ($this->getTokenProviders() as $provider) { + foreach ($provider->getTokensToNotify() as $tokenNotification) { + $tokenNotification->dispatch(); + $provider->setTokenNotified($tokenNotification->getTokenId()); } } diff --git a/plugins/UsersManager/TokenNotifications/TokenProviderInterface.php b/plugins/UsersManager/TokenNotifications/TokenProviderInterface.php index 4707517c8bf..a84990a0904 100644 --- a/plugins/UsersManager/TokenNotifications/TokenProviderInterface.php +++ b/plugins/UsersManager/TokenNotifications/TokenProviderInterface.php @@ -20,10 +20,10 @@ interface TokenProviderInterface public function getTokensToNotify(): array; /** - * Returns a callable that is called when the notification has been sent for a given token. - * The callable is provided with unique token id as its param. + * Sets information that a notification for a given token has been dispatched * - * @return callable + * @param string $tokenId + * @return void */ - public function onTokenNotified(): callable; + public function setTokenNotified(string $tokenId): void; } diff --git a/plugins/UsersManager/TokenProvider.php b/plugins/UsersManager/TokenProvider.php index df345782191..d29cd9826d8 100644 --- a/plugins/UsersManager/TokenProvider.php +++ b/plugins/UsersManager/TokenProvider.php @@ -55,23 +55,20 @@ public function getTokensToNotify(): array $user = $this->userModel->getUser($t->login); $email = $user['email']; - $notifications[] = new AuthTokenNotification( + $notifications[] = new AuthTokenEmailNotification( $t->idusertokenauth, $t->description, $t->date_created, - $t->login, - $email + [$email], + ['login' => $t->login] ); } return $notifications; } - public function onTokenNotified(): callable + public function setTokenNotified(string $tokenId): void { - $that = $this; - return function (string $tokenId) use ($that) { - $that->userModel->setRotationNotificationWasSentForToken($tokenId, $that->today); - }; + $this->userModel->setRotationNotificationWasSentForToken($tokenId, $this->today); } } From 68fa801241858eadf1e292da1d522a32314b36a2 Mon Sep 17 00:00:00 2001 From: Michal Kleiner Date: Mon, 31 Mar 2025 17:30:54 +0200 Subject: [PATCH 07/26] Remove unnecessary use statement --- plugins/UsersManager/TokenNotifications/TokenNotifierTask.php | 1 - 1 file changed, 1 deletion(-) diff --git a/plugins/UsersManager/TokenNotifications/TokenNotifierTask.php b/plugins/UsersManager/TokenNotifications/TokenNotifierTask.php index 5287e5341c6..f1b76ab7b78 100644 --- a/plugins/UsersManager/TokenNotifications/TokenNotifierTask.php +++ b/plugins/UsersManager/TokenNotifications/TokenNotifierTask.php @@ -15,7 +15,6 @@ use Piwik\Log\LoggerInterface; use Piwik\Option; use Piwik\Plugin\Manager as PluginManager; -use Piwik\Plugins\UsersManager\TokenProvider; use Piwik\Scheduler\Schedule\Daily; use Piwik\Scheduler\Scheduler; use Piwik\Scheduler\Task; From 20109720ff56df298c77a32ce784fe8a4333f63e Mon Sep 17 00:00:00 2001 From: Michal Kleiner Date: Fri, 4 Apr 2025 14:24:03 +0200 Subject: [PATCH 08/26] Fix typo --- core/Plugin/Tasks.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/Plugin/Tasks.php b/core/Plugin/Tasks.php index 5130a7b2880..a271c42a464 100644 --- a/core/Plugin/Tasks.php +++ b/core/Plugin/Tasks.php @@ -103,7 +103,7 @@ protected function monthly($methodName, $methodParameter = null, $priority = sel } /** - * Schedules the given tasks/method to run depending at the given scheduled time. Unlike the convenient methods + * Schedules the given tasks/method to run depending on the given scheduled time. Unlike the convenient methods * such as {@link hourly()} you need to specify the object on which the given method should be called. This can be * either an instance of a class or a class name. For more information about these parameters see {@link hourly()} * From 17ed08b6a5469c7231504c10e0b67ce2dccdafd5 Mon Sep 17 00:00:00 2001 From: Michal Kleiner Date: Fri, 4 Apr 2025 14:28:17 +0200 Subject: [PATCH 09/26] Rename interfaces, classes and methods to better suit their intended functionality --- ...ider.php => TokenNotificationProvider.php} | 8 +++--- ...=> TokenNotificationProviderInterface.php} | 10 ++++---- .../TokenNotifications/TokenNotifierTask.php | 25 +++++++++++-------- 3 files changed, 24 insertions(+), 19 deletions(-) rename plugins/UsersManager/{TokenProvider.php => TokenNotificationProvider.php} (85%) rename plugins/UsersManager/TokenNotifications/{TokenProviderInterface.php => TokenNotificationProviderInterface.php} (57%) diff --git a/plugins/UsersManager/TokenProvider.php b/plugins/UsersManager/TokenNotificationProvider.php similarity index 85% rename from plugins/UsersManager/TokenProvider.php rename to plugins/UsersManager/TokenNotificationProvider.php index d29cd9826d8..4d23606c04b 100644 --- a/plugins/UsersManager/TokenProvider.php +++ b/plugins/UsersManager/TokenNotificationProvider.php @@ -13,10 +13,10 @@ use Piwik\Config; use Piwik\Date; use Piwik\Db; -use Piwik\Plugins\UsersManager\TokenNotifications\TokenProviderInterface; +use Piwik\Plugins\UsersManager\TokenNotifications\TokenNotificationProviderInterface; use Piwik\Plugins\UsersManager\Model as UserModel; -class TokenProvider implements TokenProviderInterface +class TokenNotificationProvider implements TokenNotificationProviderInterface { /** @var Model */ private $userModel; @@ -36,7 +36,7 @@ private function getRotationPeriodThreshold(): string return Date::factory('today')->subDay($periodDays)->getDateTime(); } - public function getTokensToNotify(): array + public function getTokenNotificationsForDispatch(): array { $db = Db::get(); $sql = "SELECT * FROM " . Common::prefixTable('user_token_auth') @@ -67,7 +67,7 @@ public function getTokensToNotify(): array return $notifications; } - public function setTokenNotified(string $tokenId): void + public function setTokenNotificationDispatched(string $tokenId): void { $this->userModel->setRotationNotificationWasSentForToken($tokenId, $this->today); } diff --git a/plugins/UsersManager/TokenNotifications/TokenProviderInterface.php b/plugins/UsersManager/TokenNotifications/TokenNotificationProviderInterface.php similarity index 57% rename from plugins/UsersManager/TokenNotifications/TokenProviderInterface.php rename to plugins/UsersManager/TokenNotifications/TokenNotificationProviderInterface.php index a84990a0904..a2e9a61b8d1 100644 --- a/plugins/UsersManager/TokenNotifications/TokenProviderInterface.php +++ b/plugins/UsersManager/TokenNotifications/TokenNotificationProviderInterface.php @@ -9,15 +9,15 @@ namespace Piwik\Plugins\UsersManager\TokenNotifications; -interface TokenProviderInterface +interface TokenNotificationProviderInterface { /** - * Provides a list of tokens to be notified, each with their information - * that can be used to populate the notification email + * Provides a list of token notifications to be dispatched, + * each with their data that can be used e.g. to populate a notification email * * @return TokenNotificationInterface[] */ - public function getTokensToNotify(): array; + public function getTokenNotificationsForDispatch(): array; /** * Sets information that a notification for a given token has been dispatched @@ -25,5 +25,5 @@ public function getTokensToNotify(): array; * @param string $tokenId * @return void */ - public function setTokenNotified(string $tokenId): void; + public function setTokenNotificationDispatched(string $tokenId): void; } diff --git a/plugins/UsersManager/TokenNotifications/TokenNotifierTask.php b/plugins/UsersManager/TokenNotifications/TokenNotifierTask.php index f1b76ab7b78..bf01afec748 100644 --- a/plugins/UsersManager/TokenNotifications/TokenNotifierTask.php +++ b/plugins/UsersManager/TokenNotifications/TokenNotifierTask.php @@ -32,15 +32,15 @@ public function __construct() } /** - * Get a list of providers that may require token notifications being dispatched + * Get a list of providers (class names) that may provide token notifications to be dispatched * * @return array */ - private function getTokenProviders(): array + private function getTokenProviderClasses(): array { return PluginManager::getInstance()->findComponents( 'TokenProvider', - TokenProviderInterface::class + TokenNotificationProviderInterface::class ); } @@ -49,24 +49,29 @@ private function getTokenProviders(): array */ public function dispatchNotifications() { + $container = StaticContainer::getContainer(); + try { Option::set(self::LAST_RUN_TIME_OPTION_NAME, Date::factory('today')->getTimestamp()); - /** @var TokenProviderInterface $provider */ - foreach ($this->getTokenProviders() as $provider) { - foreach ($provider->getTokensToNotify() as $tokenNotification) { - $tokenNotification->dispatch(); - $provider->setTokenNotified($tokenNotification->getTokenId()); + foreach ($this->getTokenProviderClasses() as $providerClass) { + /** @var TokenNotificationProviderInterface $provider */ + $provider = $container->get($providerClass); + foreach ($provider->getTokenNotificationsForDispatch() as $tokenNotification) { + $dispatched = $tokenNotification->dispatch(); + if ($dispatched) { + $provider->setTokenNotificationDispatched($tokenNotification->getTokenId()); + } } } // reschedule for next run /** @var Scheduler $scheduler */ - $scheduler = StaticContainer::getContainer()->get(Scheduler::class); + $scheduler = $container->get(Scheduler::class); // reschedule to ensure it's not run again in an hour $scheduler->rescheduleTask(new static()); } catch (Exception $ex) { - StaticContainer::get(LoggerInterface::class)->error($ex); + $container->get(LoggerInterface::class)->error($ex); throw $ex; } } From 1d938bdec8fd7ec4e45c6d879387a8ab55c94549 Mon Sep 17 00:00:00 2001 From: Michal Kleiner Date: Fri, 4 Apr 2025 14:28:36 +0200 Subject: [PATCH 10/26] Ensure TokenNotifierTask is scheduled --- plugins/UsersManager/Tasks.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/plugins/UsersManager/Tasks.php b/plugins/UsersManager/Tasks.php index 73e74622699..41c385d21f3 100644 --- a/plugins/UsersManager/Tasks.php +++ b/plugins/UsersManager/Tasks.php @@ -11,6 +11,7 @@ use Piwik\Access; use Piwik\Date; +use Piwik\Plugins\UsersManager\TokenNotifications\TokenNotifierTask; class Tasks extends \Piwik\Plugin\Tasks { @@ -35,6 +36,8 @@ public function schedule() $this->daily("cleanupExpiredTokens"); $this->daily("setUserDefaultReportPreference"); $this->daily("cleanUpExpiredInvites"); + + $this->scheduleTask(new TokenNotifierTask()); } public function cleanupExpiredTokens() From 747f3b85bbf0997b7b83f1d5b69d38c1607ee5b4 Mon Sep 17 00:00:00 2001 From: Michal Kleiner Date: Fri, 4 Apr 2025 14:30:53 +0200 Subject: [PATCH 11/26] Correctly use array access instead of object access to db row data --- plugins/UsersManager/TokenNotificationProvider.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/plugins/UsersManager/TokenNotificationProvider.php b/plugins/UsersManager/TokenNotificationProvider.php index 4d23606c04b..67a3827176c 100644 --- a/plugins/UsersManager/TokenNotificationProvider.php +++ b/plugins/UsersManager/TokenNotificationProvider.php @@ -56,11 +56,11 @@ public function getTokenNotificationsForDispatch(): array $email = $user['email']; $notifications[] = new AuthTokenEmailNotification( - $t->idusertokenauth, - $t->description, - $t->date_created, + $t['idusertokenauth'], + $t['description'], + $t['date_created'], [$email], - ['login' => $t->login] + ['login' => $t['login']] ); } From 8963a6e580314d81b9bb67d02eddc308441eb169 Mon Sep 17 00:00:00 2001 From: Michal Kleiner Date: Mon, 7 Apr 2025 14:30:53 +0200 Subject: [PATCH 12/26] Tweaks from further local testing --- plugins/UsersManager/TokenNotificationProvider.php | 4 ++-- plugins/UsersManager/TokenNotifications/TokenNotifierTask.php | 2 +- .../templates/_authTokenNotificationHtmlEmail.twig | 4 ++-- .../templates/_authTokenNotificationTextEmail.twig | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/plugins/UsersManager/TokenNotificationProvider.php b/plugins/UsersManager/TokenNotificationProvider.php index 67a3827176c..524f474f2d3 100644 --- a/plugins/UsersManager/TokenNotificationProvider.php +++ b/plugins/UsersManager/TokenNotificationProvider.php @@ -52,7 +52,7 @@ public function getTokenNotificationsForDispatch(): array $notifications = []; foreach ($tokensToNotify as $t) { - $user = $this->userModel->getUser($t->login); + $user = $this->userModel->getUser($t['login']); $email = $user['email']; $notifications[] = new AuthTokenEmailNotification( @@ -60,7 +60,7 @@ public function getTokenNotificationsForDispatch(): array $t['description'], $t['date_created'], [$email], - ['login' => $t['login']] + [$email => ['login' => $t['login']]] ); } diff --git a/plugins/UsersManager/TokenNotifications/TokenNotifierTask.php b/plugins/UsersManager/TokenNotifications/TokenNotifierTask.php index bf01afec748..75666897908 100644 --- a/plugins/UsersManager/TokenNotifications/TokenNotifierTask.php +++ b/plugins/UsersManager/TokenNotifications/TokenNotifierTask.php @@ -39,7 +39,7 @@ public function __construct() private function getTokenProviderClasses(): array { return PluginManager::getInstance()->findComponents( - 'TokenProvider', + 'TokenNotificationProvider', TokenNotificationProviderInterface::class ); } diff --git a/plugins/UsersManager/templates/_authTokenNotificationHtmlEmail.twig b/plugins/UsersManager/templates/_authTokenNotificationHtmlEmail.twig index 297c0ea99d1..770849b9043 100644 --- a/plugins/UsersManager/templates/_authTokenNotificationHtmlEmail.twig +++ b/plugins/UsersManager/templates/_authTokenNotificationHtmlEmail.twig @@ -1,7 +1,7 @@

{{ 'General_HelloUser'|translate(login) }}

{{ 'UsersManager_AuthTokenNotificationEmailReminder'|translate(rotationPeriod) }}

-

{{ 'UsersManager_AuthTokenNotificationEmail01'|translate('', rotationPeriod, '') }}

-

{{ 'UsersManager_AuthTokenNotificationEmail02'|translate('', tokenDescription, '', '', tokenCreationDate, '') }}

+

{{ 'UsersManager_AuthTokenNotificationEmail01'|translate('', rotationPeriod, '')|raw }}

+

{{ 'UsersManager_AuthTokenNotificationEmail02'|translate('', tokenName, '', '', tokenCreationDate, '')|raw }}

{{ 'UsersManager_AuthTokenNotificationEmail03'|translate }}

{{ 'UsersManager_AuthTokenNotificationEmail04'|translate }}

{{ 'UsersManager_AuthTokenNotificationEmail05'|translate }} diff --git a/plugins/UsersManager/templates/_authTokenNotificationTextEmail.twig b/plugins/UsersManager/templates/_authTokenNotificationTextEmail.twig index 2016f501287..93f0e7cc190 100644 --- a/plugins/UsersManager/templates/_authTokenNotificationTextEmail.twig +++ b/plugins/UsersManager/templates/_authTokenNotificationTextEmail.twig @@ -3,9 +3,9 @@ {{ 'UsersManager_AuthTokenNotificationEmailReminder'|translate(rotationPeriod) }} -{{ 'UsersManager_AuthTokenNotificationEmail01'|translate('', rotationPeriod, '') }} +{{ 'UsersManager_AuthTokenNotificationEmail01'|translate('', rotationPeriod, '')|raw }} -{{ 'UsersManager_AuthTokenNotificationEmail02'|translate('', tokenDescription, '', '', tokenCreationDate, '') }} +{{ 'UsersManager_AuthTokenNotificationEmail02'|translate('', tokenName, '', '', tokenCreationDate, '')|raw }} {{ 'UsersManager_AuthTokenNotificationEmail03'|translate }} From 41b7800d11b26f9ac4b2515377e175bd790704d7 Mon Sep 17 00:00:00 2001 From: Michal Kleiner Date: Mon, 7 Apr 2025 14:31:47 +0200 Subject: [PATCH 13/26] Allow to disable auth token notifications --- config/global.ini.php | 5 +++-- plugins/UsersManager/TokenNotificationProvider.php | 13 +++++++++---- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/config/global.ini.php b/config/global.ini.php index fbbda97d7fa..ae1a647db06 100644 --- a/config/global.ini.php +++ b/config/global.ini.php @@ -561,8 +561,9 @@ ; Recommended for best security. only_allow_secure_auth_tokens = 0 -; Number of days after which a personal auth token is recommended to be rotated -; and an email notification will be sent to the user +; Number of days after which a personal auth token is recommended to be rotated and an email notification will be sent to the user. +; If set to 0 days, notifications won't be sent. +; Recommended to keep enabled for best security. auth_token_rotation_notification_days = 180 ; language cookie name for session diff --git a/plugins/UsersManager/TokenNotificationProvider.php b/plugins/UsersManager/TokenNotificationProvider.php index 524f474f2d3..a242de7b841 100644 --- a/plugins/UsersManager/TokenNotificationProvider.php +++ b/plugins/UsersManager/TokenNotificationProvider.php @@ -30,14 +30,19 @@ public function __construct() $this->today = Date::factory('today')->getDatetime(); } - private function getRotationPeriodThreshold(): string + private function getRotationPeriodThreshold(): ?string { - $periodDays = Config::getInstance()->General['auth_token_rotation_notification_days']; - return Date::factory('today')->subDay($periodDays)->getDateTime(); + $periodDays = (int) Config::getInstance()->General['auth_token_rotation_notification_days']; + return $periodDays ? Date::factory('today')->subDay($periodDays)->getDateTime() : null; } public function getTokenNotificationsForDispatch(): array { + $rotationThreshold = $this->getRotationPeriodThreshold(); + if (null === $rotationThreshold) { + return []; + } + $db = Db::get(); $sql = "SELECT * FROM " . Common::prefixTable('user_token_auth') . " WHERE (date_expired is null or date_expired > ?)" @@ -46,7 +51,7 @@ public function getTokenNotificationsForDispatch(): array $tokensToNotify = $db->fetchAll($sql, [ $this->today, - $this->getRotationPeriodThreshold() + $rotationThreshold ]); $notifications = []; From 06cf8c2461e5089e31b92c48f574c06cdeb6e60d Mon Sep 17 00:00:00 2001 From: Michal Kleiner Date: Mon, 7 Apr 2025 14:31:55 +0200 Subject: [PATCH 14/26] Exclude system tokens --- plugins/UsersManager/TokenNotificationProvider.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/UsersManager/TokenNotificationProvider.php b/plugins/UsersManager/TokenNotificationProvider.php index a242de7b841..dcb377121d1 100644 --- a/plugins/UsersManager/TokenNotificationProvider.php +++ b/plugins/UsersManager/TokenNotificationProvider.php @@ -47,7 +47,8 @@ public function getTokenNotificationsForDispatch(): array $sql = "SELECT * FROM " . Common::prefixTable('user_token_auth') . " WHERE (date_expired is null or date_expired > ?)" . " AND (date_created <= ?)" - . " AND ts_rotation_notified is null"; + . " AND ts_rotation_notified is null" + . " AND system_token = 0"; $tokensToNotify = $db->fetchAll($sql, [ $this->today, From 7953e0b7b9d3f2e72b3bded58f24896670a5a294 Mon Sep 17 00:00:00 2001 From: Michal Kleiner Date: Mon, 7 Apr 2025 14:35:44 +0200 Subject: [PATCH 15/26] Add auth token notification email tests --- core/Db/Schema/Mysql.php | 1 + .../UsersManager/tests/Fixtures/Tokens.php | 73 ++++++++++ .../AuthTokenNotificationEmailTest.php | 133 ++++++++++++++++++ 3 files changed, 207 insertions(+) create mode 100644 plugins/UsersManager/tests/Fixtures/Tokens.php create mode 100644 plugins/UsersManager/tests/Integration/AuthTokenNotificationEmailTest.php diff --git a/core/Db/Schema/Mysql.php b/core/Db/Schema/Mysql.php index 41dc7c5f183..e8277615a21 100644 --- a/core/Db/Schema/Mysql.php +++ b/core/Db/Schema/Mysql.php @@ -74,6 +74,7 @@ public function getTablesCreateSql() date_created DATETIME NOT NULL, date_expired DATETIME NULL, secure_only TINYINT(2) unsigned NOT NULL DEFAULT '0', + ts_rotation_notified DATETIME NULL, PRIMARY KEY(idusertokenauth), UNIQUE KEY uniq_password(password) ) $tableOptions diff --git a/plugins/UsersManager/tests/Fixtures/Tokens.php b/plugins/UsersManager/tests/Fixtures/Tokens.php new file mode 100644 index 00000000000..93b3b4c4947 --- /dev/null +++ b/plugins/UsersManager/tests/Fixtures/Tokens.php @@ -0,0 +1,73 @@ + [ + ['user1, not expired user token, secure only', '2024-01-01 00:00:00', null, 0, 1], + ['user1, not expired user token, not secure only', '2025-01-01 00:00:00', null, 0, 0], + ['user1, not expired system token, secure only', '2024-01-01 00:00:00', null, 1, 1], + ['user1, not expired system token, not secure only', '2025-01-01 00:00:00', null, 1, 0], + ['user1, expired user token, secure only', '2024-01-01 00:00:00', '2024-02-01 00:00:00', 0, 1], + ['user1, expired user token, not secure only', '2025-01-01 00:00:00', '2024-02-01 00:00:00', 0, 0], + ['user1, expired system token, secure only', '2024-01-01 00:00:00', '2024-02-01 00:00:00', 1, 1], + ['user1, expired system token, not secure only', '2025-01-01 00:00:00', '2024-02-01 00:00:00', 1, 0], + ], + 'user2' => [ + ['user2, not expired user token, secure only', '2024-01-01 00:00:00', null, 0, 1], + ['user2, not expired user token, not secure only', '2025-01-01 00:00:00', null, 0, 0], + ['user2, not expired system token, secure only', '2024-01-01 00:00:00', null, 1, 1], + ['user2, not expired system token, not secure only', '2025-01-01 00:00:00', null, 1, 0], + ['user2, expired user token, secure only', '2024-01-01 00:00:00', '2024-02-01 00:00:00', 0, 1], + ['user2, expired user token, not secure only', '2025-01-01 00:00:00', '2024-02-01 00:00:00', 0, 0], + ['user2, expired system token, secure only', '2024-01-01 00:00:00', '2024-02-01 00:00:00', 1, 1], + ['user2, expired system token, not secure only', '2025-01-01 00:00:00', '2024-02-01 00:00:00', 1, 0], + ], + ]; + + public function setUp(): void + { + Fixture::createWebsite('2020-01-11 00:00:00'); + + $this->setUpUsersAndTokens(); + } + + private function setUpUsersAndTokens() + { + $api = API::getInstance(); + $api->addUser('user1', 'password1', 'user1@example.com'); + $api->addUser('user2', 'password2', 'user2@example.com'); + $api->setUserAccess('user2', 'view', [1]); + + $model = new UsersManagerModel(); + + foreach ($this->tokens as $user => $tokens) { + foreach ($tokens as $token) { + $model->addTokenAuth($user, $model->generateRandomTokenAuth(), ...$token); + } + } + } + + public function resetTsRotationNotification() + { + Db::get()->query("UPDATE " . Common::prefixTable('user_token_auth') . " SET ts_rotation_notified = NULL"); + } +} diff --git a/plugins/UsersManager/tests/Integration/AuthTokenNotificationEmailTest.php b/plugins/UsersManager/tests/Integration/AuthTokenNotificationEmailTest.php new file mode 100644 index 00000000000..1df591fab74 --- /dev/null +++ b/plugins/UsersManager/tests/Integration/AuthTokenNotificationEmailTest.php @@ -0,0 +1,133 @@ +task = new TokenNotifierTask(); + } + + /** + * @throws \Exception + */ + private function clearCaptureAndDispatch(bool $resetTsNotified = false): void + { + if ($resetTsNotified) { + self::$fixture->resetTsRotationNotification(); + } + $this->capturedNotifications = []; + $this->task->dispatchNotifications(); + } + + /** + * @throws \Exception + */ + public function testDispatchAuthTokenNotificationEmail() + { + Date::$now = Date::factory('2025-04-01')->getTimestamp(); + + // for standard 180 days, we have one token notification for each of two users + $this->clearCaptureAndDispatch(); + self::assertEquals(2, count($this->capturedNotifications)); + self::assertEquals(['user1', 'user2'], array_column($this->capturedNotifications, 1)); + self::assertEquals(['2024-01-01 00:00:00', '2024-01-01 00:00:00'], array_column($this->capturedNotifications, 3)); + + // all notifications sent already, should be zero now + $this->clearCaptureAndDispatch(); + self::assertEquals(0, count($this->capturedNotifications)); + + // after removing the notification timestamp, we should have two notifications again, both in 2024 + $this->clearCaptureAndDispatch(true); + self::assertEquals(2, count($this->capturedNotifications)); + self::assertEquals(['2024-01-01 00:00:00', '2024-01-01 00:00:00'], array_column($this->capturedNotifications, 3)); + + // change rotation notification interval to 30 days + Config::getInstance()->General['auth_token_rotation_notification_days'] = 30; + + // after changing the date, we get extra two notifications in 2025 + $this->clearCaptureAndDispatch(); + self::assertEquals(2, count($this->capturedNotifications)); + self::assertEquals(['2025-01-01 00:00:00', '2025-01-01 00:00:00'], array_column($this->capturedNotifications, 3)); + } + + /** + * @throws \Exception + */ + public function testNoNotificationIsSentWhenTodayIsBeforeTokensCreated() { + // change rotation notification interval to 1 day and date before first token was created + Date::$now = Date::factory('2023-12-01')->getTimestamp(); + Config::getInstance()->General['auth_token_rotation_notification_days'] = 1; + + $this->clearCaptureAndDispatch(true); + self::assertEquals(0, count($this->capturedNotifications)); + } + + /** + * @throws \Exception + */ + public function testNoNotificationIsSentWhenFeatureDisabled() + { + Date::$now = Date::factory('2025-12-01')->getTimestamp(); + Config::getInstance()->General['auth_token_rotation_notification_days'] = 0; + + $this->clearCaptureAndDispatch(true); + + self::assertEquals(0, count($this->capturedNotifications)); + } + + public function provideContainerConfig(): array + { + return [ + 'Piwik\Access' => new FakeAccess(), + 'observers.global' => \Piwik\DI::add([ + ['Test.Mail.send', \Piwik\DI::value(function (PHPMailer $mail) { + $body = $mail->createBody(); + $body = preg_replace("/=[\r\n]+/", '', $body); + preg_match('|Hello, (.*?)!.*your (.*).*since ([0-9-:\s]+)|isu', $body, $matches); + if (count($matches) === 4) { + unset($matches[0]); + $this->capturedNotifications[] = $matches; + } + })], + ]), + ]; + } +} + +AuthTokenNotificationEmailTest::$fixture = new TokensFixture(); From b195c3b7379bc415cd887af3e7859665513107cd Mon Sep 17 00:00:00 2001 From: Michal Kleiner Date: Mon, 7 Apr 2025 14:43:48 +0200 Subject: [PATCH 16/26] Fix CS --- .../tests/Integration/AuthTokenNotificationEmailTest.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/UsersManager/tests/Integration/AuthTokenNotificationEmailTest.php b/plugins/UsersManager/tests/Integration/AuthTokenNotificationEmailTest.php index 1df591fab74..c501164330b 100644 --- a/plugins/UsersManager/tests/Integration/AuthTokenNotificationEmailTest.php +++ b/plugins/UsersManager/tests/Integration/AuthTokenNotificationEmailTest.php @@ -89,7 +89,8 @@ public function testDispatchAuthTokenNotificationEmail() /** * @throws \Exception */ - public function testNoNotificationIsSentWhenTodayIsBeforeTokensCreated() { + public function testNoNotificationIsSentWhenTodayIsBeforeTokensCreated() + { // change rotation notification interval to 1 day and date before first token was created Date::$now = Date::factory('2023-12-01')->getTimestamp(); Config::getInstance()->General['auth_token_rotation_notification_days'] = 1; From 2f834d996f4912a56f6687f00c5fe2b1bfe48e45 Mon Sep 17 00:00:00 2001 From: Michal Kleiner Date: Tue, 8 Apr 2025 21:24:17 +0200 Subject: [PATCH 17/26] Add log info about number of notifications sent --- .../TokenNotifications/TokenNotifierTask.php | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/plugins/UsersManager/TokenNotifications/TokenNotifierTask.php b/plugins/UsersManager/TokenNotifications/TokenNotifierTask.php index 75666897908..e10fb989130 100644 --- a/plugins/UsersManager/TokenNotifications/TokenNotifierTask.php +++ b/plugins/UsersManager/TokenNotifications/TokenNotifierTask.php @@ -50,17 +50,26 @@ private function getTokenProviderClasses(): array public function dispatchNotifications() { $container = StaticContainer::getContainer(); + $logger = $container->get(LoggerInterface::class); try { Option::set(self::LAST_RUN_TIME_OPTION_NAME, Date::factory('today')->getTimestamp()); + $notificationsToDispatchCount = 0; + $notificationsDispatchedCount = 0; + foreach ($this->getTokenProviderClasses() as $providerClass) { /** @var TokenNotificationProviderInterface $provider */ $provider = $container->get($providerClass); - foreach ($provider->getTokenNotificationsForDispatch() as $tokenNotification) { + + $providerTokenNotificationsForDispatch = $provider->getTokenNotificationsForDispatch(); + $notificationsToDispatchCount += count($providerTokenNotificationsForDispatch); + + foreach ($providerTokenNotificationsForDispatch as $tokenNotification) { $dispatched = $tokenNotification->dispatch(); if ($dispatched) { $provider->setTokenNotificationDispatched($tokenNotification->getTokenId()); + $notificationsDispatchedCount++; } } } @@ -70,6 +79,15 @@ public function dispatchNotifications() $scheduler = $container->get(Scheduler::class); // reschedule to ensure it's not run again in an hour $scheduler->rescheduleTask(new static()); + + if ($notificationsToDispatchCount) { + $logger->info( + "Number of token notifications dispatched: {number} of {total}.", + ['number' => $notificationsDispatchedCount, 'total' => $notificationsToDispatchCount] + ); + } else { + $logger->info("No token notifications to dispatch, task rescheduled."); + } } catch (Exception $ex) { $container->get(LoggerInterface::class)->error($ex); throw $ex; From edc40a619a1077f820e4a088bda51d3ded1cd9a5 Mon Sep 17 00:00:00 2001 From: Michal Kleiner Date: Tue, 8 Apr 2025 21:24:30 +0200 Subject: [PATCH 18/26] Declare test fixture variable --- .../tests/Integration/AuthTokenNotificationEmailTest.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/plugins/UsersManager/tests/Integration/AuthTokenNotificationEmailTest.php b/plugins/UsersManager/tests/Integration/AuthTokenNotificationEmailTest.php index c501164330b..ce8b55a960b 100644 --- a/plugins/UsersManager/tests/Integration/AuthTokenNotificationEmailTest.php +++ b/plugins/UsersManager/tests/Integration/AuthTokenNotificationEmailTest.php @@ -26,6 +26,11 @@ */ class AuthTokenNotificationEmailTest extends IntegrationTestCase { + /** + * @var Fixture + */ + public static $fixture; + protected $capturedNotifications = []; protected $task; From 35a037e1adc9c89839a35439790761a22117c8df Mon Sep 17 00:00:00 2001 From: Michal Kleiner Date: Wed, 9 Apr 2025 08:53:49 +0200 Subject: [PATCH 19/26] Use strings in Tokens fixture --- .../UsersManager/tests/Fixtures/Tokens.php | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/plugins/UsersManager/tests/Fixtures/Tokens.php b/plugins/UsersManager/tests/Fixtures/Tokens.php index 93b3b4c4947..3deba8afa3b 100644 --- a/plugins/UsersManager/tests/Fixtures/Tokens.php +++ b/plugins/UsersManager/tests/Fixtures/Tokens.php @@ -22,24 +22,24 @@ class Tokens extends Fixture { public $tokens = [ 'user1' => [ - ['user1, not expired user token, secure only', '2024-01-01 00:00:00', null, 0, 1], - ['user1, not expired user token, not secure only', '2025-01-01 00:00:00', null, 0, 0], - ['user1, not expired system token, secure only', '2024-01-01 00:00:00', null, 1, 1], - ['user1, not expired system token, not secure only', '2025-01-01 00:00:00', null, 1, 0], - ['user1, expired user token, secure only', '2024-01-01 00:00:00', '2024-02-01 00:00:00', 0, 1], - ['user1, expired user token, not secure only', '2025-01-01 00:00:00', '2024-02-01 00:00:00', 0, 0], - ['user1, expired system token, secure only', '2024-01-01 00:00:00', '2024-02-01 00:00:00', 1, 1], - ['user1, expired system token, not secure only', '2025-01-01 00:00:00', '2024-02-01 00:00:00', 1, 0], + ['user1, not expired user token, secure only', '2024-01-01 00:00:00', null, '0', '1'], + ['user1, not expired user token, not secure only', '2025-01-01 00:00:00', null, '0', '0'], + ['user1, not expired system token, secure only', '2024-01-01 00:00:00', null, '1', '1'], + ['user1, not expired system token, not secure only', '2025-01-01 00:00:00', null, '1', '0'], + ['user1, expired user token, secure only', '2024-01-01 00:00:00', '2024-02-01 00:00:00', '0', '1'], + ['user1, expired user token, not secure only', '2025-01-01 00:00:00', '2024-02-01 00:00:00', '0', '0'], + ['user1, expired system token, secure only', '2024-01-01 00:00:00', '2024-02-01 00:00:00', '1', '1'], + ['user1, expired system token, not secure only', '2025-01-01 00:00:00', '2024-02-01 00:00:00', '1', '0'], ], 'user2' => [ - ['user2, not expired user token, secure only', '2024-01-01 00:00:00', null, 0, 1], - ['user2, not expired user token, not secure only', '2025-01-01 00:00:00', null, 0, 0], - ['user2, not expired system token, secure only', '2024-01-01 00:00:00', null, 1, 1], - ['user2, not expired system token, not secure only', '2025-01-01 00:00:00', null, 1, 0], - ['user2, expired user token, secure only', '2024-01-01 00:00:00', '2024-02-01 00:00:00', 0, 1], - ['user2, expired user token, not secure only', '2025-01-01 00:00:00', '2024-02-01 00:00:00', 0, 0], - ['user2, expired system token, secure only', '2024-01-01 00:00:00', '2024-02-01 00:00:00', 1, 1], - ['user2, expired system token, not secure only', '2025-01-01 00:00:00', '2024-02-01 00:00:00', 1, 0], + ['user2, not expired user token, secure only', '2024-01-01 00:00:00', null, '0', '1'], + ['user2, not expired user token, not secure only', '2025-01-01 00:00:00', null, '0', '0'], + ['user2, not expired system token, secure only', '2024-01-01 00:00:00', null, '1', '1'], + ['user2, not expired system token, not secure only', '2025-01-01 00:00:00', null, '1', '0'], + ['user2, expired user token, secure only', '2024-01-01 00:00:00', '2024-02-01 00:00:00', '0', '1'], + ['user2, expired user token, not secure only', '2025-01-01 00:00:00', '2024-02-01 00:00:00', '0', '0'], + ['user2, expired system token, secure only', '2024-01-01 00:00:00', '2024-02-01 00:00:00', '1', '1'], + ['user2, expired system token, not secure only', '2025-01-01 00:00:00', '2024-02-01 00:00:00', '1', '0'], ], ]; From c2f10508611ee22766f581301600684234b9189d Mon Sep 17 00:00:00 2001 From: Michal Kleiner Date: Wed, 9 Apr 2025 08:55:30 +0200 Subject: [PATCH 20/26] Add ts_rotation_notified field to expected token test data --- plugins/UsersManager/tests/Integration/ModelTest.php | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/plugins/UsersManager/tests/Integration/ModelTest.php b/plugins/UsersManager/tests/Integration/ModelTest.php index 29aa4ecba83..c886b6690d8 100644 --- a/plugins/UsersManager/tests/Integration/ModelTest.php +++ b/plugins/UsersManager/tests/Integration/ModelTest.php @@ -133,7 +133,8 @@ public function testAddTokenAuthMinimal() 'last_used' => null, 'date_created' => '2020-01-02 03:04:05', 'date_expired' => null, - 'secure_only' => '0' + 'secure_only' => '0', + 'ts_rotation_notified' => null, )), $tokens); } @@ -152,7 +153,8 @@ public function testAddTokenAuthExpire() 'last_used' => null, 'date_created' => '2020-01-02 03:04:05', 'date_expired' => '2030-01-05 03:04:05', - 'secure_only' => '0' + 'secure_only' => '0', + 'ts_rotation_notified' => null, )), $tokens); } @@ -214,7 +216,8 @@ public function testGetAllNonSystemTokensForLoginReturnsNotExpiredToken() 'last_used' => null, 'date_created' => '2020-01-02 03:04:05', 'date_expired' => '2030-01-05 03:04:05', - 'secure_only' => '0' + 'secure_only' => '0', + 'ts_rotation_notified' => null, )), $tokens); } From 42f53e0813c7e2dc3a4eed995c6287d35815728f Mon Sep 17 00:00:00 2001 From: Michal Kleiner Date: Thu, 10 Apr 2025 13:27:39 +0200 Subject: [PATCH 21/26] Update UI test screenshot --- .../UIIntegrationTest_admin_diagnostics_configfile.png | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/UI/expected-screenshots/UIIntegrationTest_admin_diagnostics_configfile.png b/tests/UI/expected-screenshots/UIIntegrationTest_admin_diagnostics_configfile.png index a1d02313425..7528204dd37 100644 --- a/tests/UI/expected-screenshots/UIIntegrationTest_admin_diagnostics_configfile.png +++ b/tests/UI/expected-screenshots/UIIntegrationTest_admin_diagnostics_configfile.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:c3f5e13753458e50e740638913f0d725e6c3542fda4e1023c4d1167262afa356 -size 5065984 +oid sha256:5350f97316c87f42efe8346e086cd09feac017e11892c97cf5dee7370cb31f9f +size 5084242 From 61af2b346f7ce8d9efbb5ede085dc52c69c7ab38 Mon Sep 17 00:00:00 2001 From: Michal Kleiner Date: Fri, 11 Apr 2025 19:22:40 +1200 Subject: [PATCH 22/26] Use DATETIME column type in migration Co-authored-by: Marc Neudert --- core/Updates/5.4.0-b1.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/Updates/5.4.0-b1.php b/core/Updates/5.4.0-b1.php index 7a6cdfcfd4c..0dd0aea0c33 100644 --- a/core/Updates/5.4.0-b1.php +++ b/core/Updates/5.4.0-b1.php @@ -28,7 +28,7 @@ public function __construct(MigrationFactory $factory) public function getMigrations(Updater $updater) { return [ - $this->migration->db->addColumn('user_token_auth', 'ts_rotation_notified', 'TIMESTAMP NULL'), + $this->migration->db->addColumn('user_token_auth', 'ts_rotation_notified', 'DATETIME NULL'), ]; } From 30bc5c45aacd085417981a7e719c08ab0cbcf18c Mon Sep 17 00:00:00 2001 From: Michal Kleiner Date: Fri, 11 Apr 2025 10:28:02 +0200 Subject: [PATCH 23/26] Update Manage auth token link URL to behave correctly when task run from CLI --- plugins/UsersManager/Emails/AuthTokenNotificationEmail.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/plugins/UsersManager/Emails/AuthTokenNotificationEmail.php b/plugins/UsersManager/Emails/AuthTokenNotificationEmail.php index 48e3448bf03..2ba5684f16f 100644 --- a/plugins/UsersManager/Emails/AuthTokenNotificationEmail.php +++ b/plugins/UsersManager/Emails/AuthTokenNotificationEmail.php @@ -13,6 +13,7 @@ use Piwik\Mail; use Piwik\Piwik; use Piwik\Plugins\UsersManager\TokenNotifications\TokenNotification; +use Piwik\SettingsPiwik; use Piwik\Url; use Piwik\View; @@ -59,9 +60,9 @@ private function getRotationPeriodPretty(): string protected function getManageAuthTokensLink(): string { - return Url::getCurrentUrlWithoutQueryString() - . '?module=UsersManager' - . '&action=userSecurity' + return SettingsPiwik::getPiwikUrl() + . 'index.php?' + . Url::getQueryStringFromParameters(['module' => 'UsersManager', 'action' => 'userSecurity']) . '#authtokens'; } protected function getDefaultSubject(): string From 223943fdbb2f1a96e23ed9ff6dc5010945f598aa Mon Sep 17 00:00:00 2001 From: Michal Kleiner Date: Fri, 11 Apr 2025 10:28:24 +0200 Subject: [PATCH 24/26] Exclude anonymous user default token from token notifications --- plugins/UsersManager/TokenNotificationProvider.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/plugins/UsersManager/TokenNotificationProvider.php b/plugins/UsersManager/TokenNotificationProvider.php index dcb377121d1..cf9194f2fd0 100644 --- a/plugins/UsersManager/TokenNotificationProvider.php +++ b/plugins/UsersManager/TokenNotificationProvider.php @@ -44,11 +44,13 @@ public function getTokenNotificationsForDispatch(): array } $db = Db::get(); - $sql = "SELECT * FROM " . Common::prefixTable('user_token_auth') + $sql = "SELECT * FROM " . Common::prefixTable('user_token_auth') . " AS uta" + . " JOIN " . Common::prefixTable('user') . " AS u ON uta.login = u.login" . " WHERE (date_expired is null or date_expired > ?)" . " AND (date_created <= ?)" . " AND ts_rotation_notified is null" - . " AND system_token = 0"; + . " AND system_token = 0" + . " AND u.email != 'anonymous@example.org' AND u.invited_by IS NULL"; $tokensToNotify = $db->fetchAll($sql, [ $this->today, From 791d191d0da2718dd6dd8d87cf069804ec2f0df4 Mon Sep 17 00:00:00 2001 From: Michal Kleiner Date: Fri, 11 Apr 2025 10:28:48 +0200 Subject: [PATCH 25/26] Store current datetime when token notification sent --- plugins/UsersManager/TokenNotificationProvider.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/UsersManager/TokenNotificationProvider.php b/plugins/UsersManager/TokenNotificationProvider.php index cf9194f2fd0..156203ba723 100644 --- a/plugins/UsersManager/TokenNotificationProvider.php +++ b/plugins/UsersManager/TokenNotificationProvider.php @@ -77,6 +77,6 @@ public function getTokenNotificationsForDispatch(): array public function setTokenNotificationDispatched(string $tokenId): void { - $this->userModel->setRotationNotificationWasSentForToken($tokenId, $this->today); + $this->userModel->setRotationNotificationWasSentForToken($tokenId, Date::factory('now')->getDatetime()); } } From 491389382e1ac8abedbbc6af9c5279bfafcd8ecf Mon Sep 17 00:00:00 2001 From: Michal Kleiner Date: Fri, 11 Apr 2025 17:25:10 +0200 Subject: [PATCH 26/26] Exclude anonymous user by login --- plugins/UsersManager/TokenNotificationProvider.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/plugins/UsersManager/TokenNotificationProvider.php b/plugins/UsersManager/TokenNotificationProvider.php index 156203ba723..ca2e20e40c0 100644 --- a/plugins/UsersManager/TokenNotificationProvider.php +++ b/plugins/UsersManager/TokenNotificationProvider.php @@ -44,17 +44,17 @@ public function getTokenNotificationsForDispatch(): array } $db = Db::get(); - $sql = "SELECT * FROM " . Common::prefixTable('user_token_auth') . " AS uta" - . " JOIN " . Common::prefixTable('user') . " AS u ON uta.login = u.login" + $sql = "SELECT * FROM " . Common::prefixTable('user_token_auth') . " WHERE (date_expired is null or date_expired > ?)" . " AND (date_created <= ?)" . " AND ts_rotation_notified is null" . " AND system_token = 0" - . " AND u.email != 'anonymous@example.org' AND u.invited_by IS NULL"; + . " AND login != ?"; $tokensToNotify = $db->fetchAll($sql, [ $this->today, - $rotationThreshold + $rotationThreshold, + 'anonymous' ]); $notifications = [];